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

DFW rule management resource vcd_nsxt_distributed_firewall_rule #1076

Merged
merged 24 commits into from
Jul 14, 2023

Conversation

Didainius
Copy link
Collaborator

@Didainius Didainius commented Jul 3, 2023

Closes #970

This PR introduces a new resource and data source vcd_nsxt_distributed_firewall_rule to manage Distributed firewall rules one by one. We already have a resource vcd_nsxt_distributed_firewall, but that one manages all rules at once and there is a request #970 to manage rules one by one.

Notes

  • There is no API to create a single distributed firewall rule, so this functionality is handled in Go SDK for VCD VdcGroup.CreateDistributedFirewallRule. As a consequence - firewall rules will not be created in parallel, but instead one by one (as the resource will have locks).
  • There is an above_rule_id field in the new resource to handle the firewall rule position during create operation.
  • By default, a distributed firewall gets one default rule created after it is activated in resource vcd_vdc_group. This would make it very inconvenient to work with firewall rules using new resources (although possible by using data source and then field above_rule_id to put all firewall rules above). To make things more convenient, vcd_vdc_group resource has a new flag remove_default_firewall_rule that would remove the default firewall rule.

Note for reviewers - the "internals" of resource are very similar to vcd_nsxt_distributed_firewall. There difference is here we don't have element nesting in a TypeSet

Patch for API V38.0

  • vcd_nsxt_app_port_profile has a patch for similar diff errors in commit 38c907b:
~ resource "vcd_nsxt_app_port_profile" "custom-org1" {
   id          = "urn:vcloud:applicationPortProfile:fbcf286a-4a33-40c7-8312-c5a71449a54f"
   name        = "custom_app_prof"
   # (4 unchanged attributes hidden)

 - app_port {
     - port     = [
         - "any",
       ] -> null
     - protocol = "ICMPv4" -> null
   }
 + app_port {
     + port     = []
     + protocol = "ICMPv4"
   }

Didainius added 3 commits July 3, 2023 17:04
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
Didainius added 4 commits July 3, 2023 17:26
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
@Didainius Didainius changed the title WIP - Standalone DFW rule management resource Standalone DFW rule management resource Jul 5, 2023
Didainius added 3 commits July 5, 2023 10:38
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
@Didainius Didainius changed the title Standalone DFW rule management resource Standalone DFW rule management resource vcd_nsxt_distributed_firewall_rule Jul 5, 2023
@Didainius Didainius changed the title Standalone DFW rule management resource vcd_nsxt_distributed_firewall_rule DFW rule management resource vcd_nsxt_distributed_firewall_rule Jul 5, 2023
@Didainius Didainius marked this pull request as ready for review July 5, 2023 08:28
Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a magical feature! I have provided some "fresh eyes" thoughts.

.changes/v3.10.0/1076-features.md Show resolved Hide resolved
vcd/datasource_vcd_nsxt_distributed_firewall_rule.go Outdated Show resolved Hide resolved
vcd/datasource_vcd_nsxt_distributed_firewall_rule.go Outdated Show resolved Hide resolved
vcd/resource_vcd_nsxt_distributed_firewall_rule.go Outdated Show resolved Hide resolved
vcd/resource_vcd_nsxt_distributed_firewall_rule.go Outdated Show resolved Hide resolved
website/docs/r/nsxt_distributed_firewall.html.markdown Outdated Show resolved Hide resolved
Didainius added 5 commits July 5, 2023 13:40
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
Copy link
Collaborator

@lvirbalas lvirbalas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! The last open item I have is on naming #1076 (comment) , but it will depend on input from a wider audience.

Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
Copy link

@adezxc adezxc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, great PR!

Signed-off-by: Dainius Serplis <[email protected]>
Copy link
Collaborator

@adambarreiro adambarreiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
Signed-off-by: Dainius Serplis <[email protected]>
@Didainius Didainius merged commit e72a489 into vmware:main Jul 14, 2023
@Didainius Didainius deleted the nsxt-dfw-update branch July 14, 2023 20:14
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

Successfully merging this pull request may close these issues.

vcd_nsxt_distributed_firewall_rule resource
5 participants