Skip to content

Commit ca79086

Browse files
Fix managed mode to properly delete out-of-band ACL rules
When managed=true, the provider now correctly detects and deletes ACL rules that were created outside of Terraform (out-of-band rules). Implementation: - Read function adds 'dummy' rules to state for out-of-band rules when managed=true - These dummy rules contain the UUID of the out-of-band rule - On next apply, Terraform detects the diff (dummy rule in state, not in config) - Update function deletes the dummy rules, which deletes the actual out-of-band rules This approach is consistent with the legacy resource_cloudstack_network_acl_rule.go and ensures that managed ACLs stay in sync with the configuration. Tests: - Added TestAccCloudStackNetworkACLRuleset_managed to verify out-of-band deletion - Verified TestAccCloudStackNetworkACLRuleset_not_managed still works correctly
1 parent 5a25310 commit ca79086

File tree

2 files changed

+77
-10
lines changed

2 files changed

+77
-10
lines changed

cloudstack/resource_cloudstack_network_acl_ruleset.go

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -453,17 +453,35 @@ func resourceCloudStackNetworkACLRulesetRead(d *schema.ResourceData, meta interf
453453
}
454454
}
455455

456-
// If this is a managed resource and there are unknown rules (out-of-band rules),
457-
// log them but DON'T add them to the state. They will be deleted on the next apply.
456+
// If this is a managed resource, add all unknown rules to dummy rules
457+
// This allows Terraform to detect them and trigger an update to delete them
458458
managed := d.Get("managed").(bool)
459459
if managed && len(ruleMap) > 0 {
460-
log.Printf("[WARN] Found %d out-of-band ACL rules for ACL %s that are not managed by Terraform. These will be deleted on the next apply.", len(ruleMap), d.Id())
460+
log.Printf("[DEBUG] Found %d out-of-band ACL rules for ACL %s", len(ruleMap), d.Id())
461461
for uuid, r := range ruleMap {
462-
log.Printf("[WARN] Out-of-band rule: uuid=%s, rule_number=%d, protocol=%s, action=%s", uuid, r.Number, r.Protocol, r.Action)
462+
log.Printf("[DEBUG] Adding dummy rule for out-of-band rule: uuid=%s, rule_number=%d", uuid, r.Number)
463+
464+
// Make a dummy rule to hold the unknown UUID
465+
// We use the UUID as a unique identifier in the cidr_list
466+
cidrs := &schema.Set{F: schema.HashString}
467+
cidrs.Add(uuid)
468+
469+
rule := map[string]interface{}{
470+
"cidr_list": cidrs,
471+
"protocol": uuid, // Use UUID as protocol to make it unique
472+
"uuid": uuid,
473+
"rule_number": r.Number,
474+
"action": "allow",
475+
"traffic_type": "ingress",
476+
"icmp_type": 0,
477+
"icmp_code": 0,
478+
"description": "",
479+
"port": "",
480+
}
481+
482+
// Add the dummy rule to the rules set
483+
rules.Add(rule)
463484
}
464-
// Note: We do NOT add these to the rules set. They should not be in the state.
465-
// On the next terraform apply, the Update function will detect that these rules
466-
// exist in the API but not in the config, and will delete them.
467485
}
468486

469487
if rules.Len() > 0 {

cloudstack/resource_cloudstack_network_acl_ruleset_test.go

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,28 @@ func testAccCheckOutOfBandACLRuleExists(aclID string) error {
143143
return fmt.Errorf("Out-of-band rule (number 30) was deleted even though managed=false")
144144
}
145145

146+
// testAccCheckOutOfBandACLRuleDeleted verifies that the out-of-band rule was deleted
147+
func testAccCheckOutOfBandACLRuleDeleted(aclID string) error {
148+
client := testAccProvider.Meta().(*cloudstack.CloudStackClient)
149+
150+
p := client.NetworkACL.NewListNetworkACLsParams()
151+
p.SetAclid(aclID)
152+
153+
resp, err := client.NetworkACL.ListNetworkACLs(p)
154+
if err != nil {
155+
return fmt.Errorf("Failed to list ACL rules: %v", err)
156+
}
157+
158+
// Check that the out-of-band rule (rule number 30) was deleted
159+
for _, rule := range resp.NetworkACLs {
160+
if rule.Number == 30 {
161+
return fmt.Errorf("Out-of-band rule (number 30) still exists even though managed=true")
162+
}
163+
}
164+
165+
return nil // Not found - success!
166+
}
167+
146168
func TestAccCloudStackNetworkACLRuleset_basic(t *testing.T) {
147169
resource.Test(t, resource.TestCase{
148170
PreCheck: func() { testAccPreCheck(t) },
@@ -500,6 +522,8 @@ resource "cloudstack_network_acl_ruleset" "bar" {
500522
}`
501523

502524
func TestAccCloudStackNetworkACLRuleset_managed(t *testing.T) {
525+
var aclID string
526+
503527
resource.Test(t, resource.TestCase{
504528
PreCheck: func() { testAccPreCheck(t) },
505529
Providers: testAccProviders,
@@ -513,13 +537,38 @@ func TestAccCloudStackNetworkACLRuleset_managed(t *testing.T) {
513537
"cloudstack_network_acl_ruleset.managed", "managed", "true"),
514538
resource.TestCheckResourceAttr(
515539
"cloudstack_network_acl_ruleset.managed", "rule.#", "2"),
540+
// Capture the ACL ID for later use
541+
func(s *terraform.State) error {
542+
rs, ok := s.RootModule().Resources["cloudstack_network_acl_ruleset.managed"]
543+
if !ok {
544+
return fmt.Errorf("Not found: cloudstack_network_acl_ruleset.managed")
545+
}
546+
aclID = rs.Primary.ID
547+
return nil
548+
},
549+
),
550+
},
551+
{
552+
// Add an out-of-band rule via the API
553+
PreConfig: func() {
554+
// Create a rule outside of Terraform
555+
testAccCreateOutOfBandACLRule(t, aclID)
556+
},
557+
Config: testAccCloudStackNetworkACLRuleset_managed,
558+
Check: resource.ComposeTestCheckFunc(
559+
testAccCheckCloudStackNetworkACLRulesetExists("cloudstack_network_acl_ruleset.managed"),
560+
// With managed=true, the out-of-band rule should be DELETED
561+
// Verify only the 2 configured rules exist
562+
resource.TestCheckResourceAttr(
563+
"cloudstack_network_acl_ruleset.managed", "rule.#", "2"),
564+
// Verify the out-of-band rule was deleted
565+
func(s *terraform.State) error {
566+
return testAccCheckOutOfBandACLRuleDeleted(aclID)
567+
},
516568
),
517569
},
518570
},
519571
})
520-
521-
// After the test, manually create an out-of-band rule and verify it's detected
522-
// This would require additional test infrastructure, so we'll skip for now
523572
}
524573

525574
const testAccCloudStackNetworkACLRuleset_managed = `

0 commit comments

Comments
 (0)