Merge pull request #1001 from vmware/fix-sequence-number-validation

Validate rule sequence numbers on create only
vmware · Oct 23, 2023 · 9f08061 · 9f08061
2 parents 1c6a26a + eab9f72
commit 9f08061
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 16 deletions.
diff --git a/nsxt/policy_common.go b/nsxt/policy_common.go
@@ -2,6 +2,7 @@ package nsxt
 
 import (
 	"fmt"
+	"log"
 	"strings"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -423,8 +424,9 @@ func validatePolicyRuleSequence(d *schema.ResourceData) error {
 	for _, rule := range rules {
 		data := rule.(map[string]interface{})
 		sequenceNumber := int64(data["sequence_number"].(int))
+		displayName := data["display_name"].(string)
 		if sequenceNumber > 0 && sequenceNumber <= latestNum {
-			return fmt.Errorf("when sequence_number is specified in a rule, it must be consistent with rule order. To avoid confusion, it is recommended to either specify sequence numbers in all rules, or none")
+			return fmt.Errorf("when sequence_number is specified in a rule, it must be consistent with rule order. To avoid confusion, it is recommended to either specify sequence numbers in all rules, or none. Error detected with rule %s: %v <= %v", displayName, sequenceNumber, latestNum)
 		}
 
 		if sequenceNumber == 0 {
@@ -470,7 +472,13 @@ func getPolicyRulesFromSchema(d *schema.ResourceData) []model.Rule {
 		}
 
 		resourceType := "Rule"
-		if sequenceNumber == 0 {
+		if sequenceNumber == 0 || sequenceNumber <= lastSequence {
+			// We overwrite sequence number in case its not specified,
+			// or out of order, which might be due to provider upgrade
+			// or bad user configuration
+			if sequenceNumber <= lastSequence {
+				log.Printf("[WARNING] Sequence_number %v for rule %s is out of order - overriding with sequence number %v", sequenceNumber, displayName, lastSequence+1)
+			}
 			sequenceNumber = lastSequence + 1
 		}
 		lastSequence = sequenceNumber

diff --git a/nsxt/resource_nsxt_policy_predefined_gateway_policy.go b/nsxt/resource_nsxt_policy_predefined_gateway_policy.go
@@ -314,10 +314,6 @@ func updatePolicyPredefinedGatewayPolicy(id string, d *schema.ResourceData, m in
 	var childRules []*data.StructValue
 	if d.HasChange("rule") {
 		oldRules, _ := d.GetChange("rule")
-		err1 := validatePolicyRuleSequence(d)
-		if err1 != nil {
-			return err1
-		}
 		rules := getPolicyRulesFromSchema(d)
 
 		existingRules := make(map[string]bool)

diff --git a/nsxt/resource_nsxt_policy_predefined_security_policy.go b/nsxt/resource_nsxt_policy_predefined_security_policy.go
@@ -236,10 +236,6 @@ func updatePolicyPredefinedSecurityPolicy(id string, d *schema.ResourceData, m i
 	var childRules []*data.StructValue
 	if d.HasChange("rule") {
 		oldRules, _ := d.GetChange("rule")
-		err1 := validatePolicyRuleSequence(d)
-		if err1 != nil {
-			return err1
-		}
 		rules := getPolicyRulesFromSchema(d)
 
 		existingRules := make(map[string]bool)

diff --git a/nsxt/resource_nsxt_policy_security_policy.go b/nsxt/resource_nsxt_policy_security_policy.go
@@ -87,16 +87,15 @@ func policySecurityPolicyBuildAndPatch(d *schema.ResourceData, m interface{}, co
 	}
 	log.Printf("[INFO] Creating Security Policy with ID %s", id)
 
-	if !createFlow {
+	if createFlow {
+		if err := validatePolicyRuleSequence(d); err != nil {
+			return err
+		}
+	} else {
 		// This is update flow
 		obj.Revision = &revision
 	}
 
-	err := validatePolicyRuleSequence(d)
-	if err != nil {
-		return err
-	}
-
 	policyChildren, err := getUpdatedRuleChildren(d)
 	if err != nil {
 		return err