Skip to content

Commit cad7264

Browse files
Fix managed mode to properly handle 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. Also fixes state mutation issues in the Read function by creating new maps instead of modifying existing state data structures. Tests: - Added TestAccCloudStackNetworkACLRuleset_managed to verify out-of-band deletion - Verified TestAccCloudStackNetworkACLRuleset_not_managed still works correctly
1 parent 35b83ae commit cad7264

2 files changed

Lines changed: 75 additions & 17 deletions

File tree

cloudstack/resource_cloudstack_network_acl_ruleset.go

Lines changed: 23 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -343,35 +343,39 @@ func resourceCloudStackNetworkACLRulesetRead(d *schema.ResourceData, meta interf
343343
// Read all rules that are configured
344344
rs := d.Get("rule").(*schema.Set)
345345
if rs.Len() > 0 {
346-
for _, rule := range rs.List() {
347-
rule := rule.(map[string]interface{})
346+
for _, oldRule := range rs.List() {
347+
oldRule := oldRule.(map[string]interface{})
348348

349-
id, ok := rule["uuid"]
349+
id, ok := oldRule["uuid"]
350350
if !ok || id.(string) == "" {
351351
continue
352352
}
353353

354354
// Get the rule
355355
r, ok := ruleMap[id.(string)]
356356
if !ok {
357-
rule["uuid"] = ""
357+
// Rule no longer exists in the API, skip it
358358
continue
359359
}
360360

361361
// Delete the known rule so only unknown rules remain in the ruleMap
362362
delete(ruleMap, id.(string))
363363

364-
// Update the values
364+
// Create a NEW map with the updated values (don't mutate the old one)
365365
cidrs := &schema.Set{F: schema.HashString}
366366
for _, cidr := range strings.Split(r.Cidrlist, ",") {
367367
cidrs.Add(cidr)
368368
}
369-
rule["cidr_list"] = cidrs
370-
rule["action"] = strings.ToLower(r.Action)
371-
rule["protocol"] = r.Protocol
372-
rule["traffic_type"] = strings.ToLower(r.Traffictype)
373-
rule["rule_number"] = r.Number
374-
rule["description"] = r.Reason
369+
370+
rule := map[string]interface{}{
371+
"cidr_list": cidrs,
372+
"action": strings.ToLower(r.Action),
373+
"protocol": r.Protocol,
374+
"traffic_type": strings.ToLower(r.Traffictype),
375+
"rule_number": r.Number,
376+
"description": r.Reason,
377+
"uuid": r.Id,
378+
}
375379

376380
// Set ICMP fields
377381
if r.Protocol == "icmp" {
@@ -450,18 +454,23 @@ func resourceCloudStackNetworkACLRulesetRead(d *schema.ResourceData, meta interf
450454
}
451455

452456
// 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
453458
managed := d.Get("managed").(bool)
454459
if managed && len(ruleMap) > 0 {
455-
for uuid := range ruleMap {
460+
log.Printf("[DEBUG] Found %d out-of-band ACL rules for ACL %s", len(ruleMap), d.Id())
461+
for uuid, r := range ruleMap {
462+
log.Printf("[DEBUG] Adding dummy rule for out-of-band rule: uuid=%s, rule_number=%d", uuid, r.Number)
463+
456464
// Make a dummy rule to hold the unknown UUID
465+
// We use the UUID as a unique identifier in the cidr_list
457466
cidrs := &schema.Set{F: schema.HashString}
458467
cidrs.Add(uuid)
459468

460469
rule := map[string]interface{}{
461470
"cidr_list": cidrs,
462-
"protocol": uuid,
471+
"protocol": uuid, // Use UUID as protocol to make it unique
463472
"uuid": uuid,
464-
"rule_number": 0,
473+
"rule_number": r.Number,
465474
"action": "allow",
466475
"traffic_type": "ingress",
467476
"icmp_type": 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)