Skip to content

Commit 5a25310

Browse files
Fix ACL ruleset Read function to prevent state mutation and handle out-of-band rules correctly
Two important fixes: 1. Create new rule maps instead of mutating existing state - The Read function was directly modifying rule maps from the state - This could cause Terraform to see spurious changes - Now creates fresh maps with updated values from the API 2. Don't persist out-of-band rules as dummy entries in state - Previously, unknown rules in managed mode were added as dummy rules - This polluted the state with fake entries - Now logs warnings about out-of-band rules but doesn't add them to state - The Update function will delete them on the next apply
1 parent 06543e2 commit 5a25310

File tree

1 file changed

+23
-32
lines changed

1 file changed

+23
-32
lines changed

cloudstack/resource_cloudstack_network_acl_ruleset.go

Lines changed: 23 additions & 32 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" {
@@ -449,30 +453,17 @@ func resourceCloudStackNetworkACLRulesetRead(d *schema.ResourceData, meta interf
449453
}
450454
}
451455

452-
// If this is a managed resource, add all unknown rules to dummy rules
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.
453458
managed := d.Get("managed").(bool)
454459
if managed && len(ruleMap) > 0 {
455-
for uuid := range ruleMap {
456-
// Make a dummy rule to hold the unknown UUID
457-
cidrs := &schema.Set{F: schema.HashString}
458-
cidrs.Add(uuid)
459-
460-
rule := map[string]interface{}{
461-
"cidr_list": cidrs,
462-
"protocol": uuid,
463-
"uuid": uuid,
464-
"rule_number": 0,
465-
"action": "allow",
466-
"traffic_type": "ingress",
467-
"icmp_type": 0,
468-
"icmp_code": 0,
469-
"description": "",
470-
"port": "",
471-
}
472-
473-
// Add the dummy rule to the rules set
474-
rules.Add(rule)
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())
461+
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)
475463
}
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.
476467
}
477468

478469
if rules.Len() > 0 {

0 commit comments

Comments
 (0)