Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor hierarchical firewall policy variables #1570

Closed
ludoo opened this issue Aug 5, 2023 · 6 comments
Closed

Refactor hierarchical firewall policy variables #1570

ludoo opened this issue Aug 5, 2023 · 6 comments
Assignees

Comments

@ludoo
Copy link
Collaborator

ludoo commented Aug 5, 2023

In the organization/folder modules:

  • the firewall_policies variable mixes policy and policy rules
  • the locals in the firewall-policies.tf file are very hard to parse

One example while naming mixes policies and rules

_merged_rules = flatten([
    for policy, rules in local.firewall_policies : [
      for name, rule in rules : merge(rule, {
        policy = policy
        name   = name
      })
    ]
  ])

Also check description fields for all resource management modules.

@ludoo ludoo self-assigned this Aug 5, 2023
@juliocc
Copy link
Collaborator

juliocc commented Aug 6, 2023

What do you think if we move hfw policies to its own module and we leave only the attachment in the resman modules?

@ludoo
Copy link
Collaborator Author

ludoo commented Aug 6, 2023

What do you think if we move hfw policies to its own module and we leave only the attachment in the resman modules?

Hmmmm the factory is pretty convenient to have in the org/folder modules, but I agree that it has very little relationship with resman. I both like splitting fw policies into a separate module, and the convenience/simplicity of the current approach, so whatever you prefer works for me.

@ludoo
Copy link
Collaborator Author

ludoo commented Aug 6, 2023

In hindsight, if we leverage the existing net firewall policy to create the policy (and optionally use it for attachment too), and only leave attachments in the resource management modules, that is probably the right way to go.

+1 to your approach

@ludoo
Copy link
Collaborator Author

ludoo commented Aug 7, 2023

Had a look at the VPC Network Firewall Policy module, and the resource attributes for network policies/rules are a 1:1 match for hirerachical policies/rules.

What we could do is integrate hierarchical policies in the same module and backport the factory. Then replace the project_id variable with a root_node one which accepts a project, folder, or organization. The first type of node creates a network policy, the second and third one a hierarchical policy. The region variable would of course be only valid for network policies.

Attachments should be optionally managed in the module itself via an attachments variable which would replace the current target_vpcs one and take VPC ids for network policy, org/folder ids for hierarchical policy. Attachments should still possible from individual VPCs/folders/org by passing in the policy id.

This is the variable set of the current module

image

I agree with @juliocc that this would result in a much cleaner design. I'm going to draft this unless you all don't like the approach.

@juliocc
Copy link
Collaborator

juliocc commented Aug 7, 2023

sounds great, go for it!

@ludoo
Copy link
Collaborator Author

ludoo commented Aug 11, 2023

This has been completed with the latest merge.

@ludoo ludoo closed this as completed Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants