Update FW policy rules only if needed

nsxt/policy_common.go

@@ -390,7 +390,7 @@ func setPolicyRulesInSchema(d *schema.ResourceData, rules []model.Rule) error {
 	return d.Set("rule", rulesList)
 }
 
-func getPolicyRulesFromSchema(d *schema.ResourceData, setNsxID bool) []model.Rule {
+func getPolicyRulesFromSchema(d *schema.ResourceData) []model.Rule {
 	rules := d.Get("rule").([]interface{})
 	var ruleList []model.Rule
 	seq := 0
@@ -410,9 +410,11 @@ func getPolicyRulesFromSchema(d *schema.ResourceData, setNsxID bool) []model.Rul
 		sequenceNumber := int64(seq)
 		tagStructs := getPolicyTagsFromSet(data["tag"].(*schema.Set))
 
-		// Use a different random Id each time, otherwise Update requires revision
-		// to be set for existing rules, and NOT be set for new rules
 		id := newUUID()
+		nsxID := data["nsx_id"].(string)
+		if nsxID != "" {
+			id = nsxID
+		}
 
 		resourceType := "Rule"
 		elem := model.Rule{
@@ -438,13 +440,6 @@ func getPolicyRulesFromSchema(d *schema.ResourceData, setNsxID bool) []model.Rul
 			SequenceNumber:       &sequenceNumber,
 		}
 
-		if setNsxID {
-			nsxID := data["nsx_id"].(string)
-			if nsxID != "" {
-				elem.Id = &nsxID
-			}
-		}
-
 		ruleList = append(ruleList, elem)
 		seq = seq + 1
 	}

nsxt/resource_nsxt_policy_gateway_policy.go

@@ -8,6 +8,7 @@ import (
 	"log"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/vmware/vsphere-automation-sdk-go/runtime/data"
 	"github.com/vmware/vsphere-automation-sdk-go/runtime/protocol/client"
 	gm_domains "github.com/vmware/vsphere-automation-sdk-go/services/nsxt-gm/global_infra/domains"
 	gm_model "github.com/vmware/vsphere-automation-sdk-go/services/nsxt-gm/model"
@@ -67,15 +68,67 @@ func resourceNsxtPolicyGatewayPolicyExistsPartial(domainName string) func(id str
 	}
 }
 
-func resourceNsxtPolicyGatewayPolicyCreate(d *schema.ResourceData, m interface{}) error {
-	connector := getPolicyConnector(m)
+func getUpdatedRuleChildren(d *schema.ResourceData) ([]*data.StructValue, error) {
+	var policyChildren []*data.StructValue
 
-	// Initialize resource Id and verify this ID is not yet used
-	id, err := getOrGenerateID(d, m, resourceNsxtPolicyGatewayPolicyExistsPartial(d.Get("domain").(string)))
-	if err != nil {
-		return err
+	if !d.HasChange("rule") {
+		return nil, nil
 	}
 
+	oldRules, newRules := d.GetChange("rule")
+	rules := getPolicyRulesFromSchema(d)
+	newRulesCount := len(newRules.([]interface{}))
+	oldRulesCount := len(oldRules.([]interface{}))
+	for ruleNo := 0; ruleNo < newRulesCount; ruleNo++ {
+		ruleIndicator := fmt.Sprintf("rule.%d", ruleNo)
+		if d.HasChange(ruleIndicator) {
+			rule := rules[ruleNo]
+			// New or updated rule
+			ruleID := newUUID()
+			if rule.Id != nil {
+				ruleID = *rule.Id
+				log.Printf("[DEBUG]: Updating child rule with id %s", *rule.Id)
+			} else {
+				log.Printf("[DEBUG]: Adding child rule with id %s", ruleID)
+				rule.Id = &ruleID
+			}
+
+			childRule, err := createPolicyChildRule(ruleID, rule, false)
+			if err != nil {
+				return policyChildren, err
+			}
+			policyChildren = append(policyChildren, childRule)
+		}
+	}
+
+	resourceType := "Rule"
+	for ruleNo := newRulesCount; ruleNo < oldRulesCount; ruleNo++ {
+		// delete
+		ruleIndicator := fmt.Sprintf("rule.%d", ruleNo)
+		oldRule, _ := d.GetChange(ruleIndicator)
+		oldRuleMap := oldRule.(map[string]interface{})
+		oldRuleID := oldRuleMap["nsx_id"].(string)
+		rule := model.Rule{
+			Id:           &oldRuleID,
+			ResourceType: &resourceType,
+		}
+
+		childRule, err := createPolicyChildRule(oldRuleID, rule, true)
+		if err != nil {
+			return policyChildren, err
+		}
+		log.Printf("[DEBUG]: Deleting child rule with id %s", oldRuleID)
+		policyChildren = append(policyChildren, childRule)
+
+	}
+
+	return policyChildren, nil
+
+}
+
+func policyGatewayPolicyBuildAndPatch(d *schema.ResourceData, m interface{}, connector *client.RestConnector, isGlobalManager bool, id string) error {
+
+	domain := d.Get("domain").(string)
 	displayName := d.Get("display_name").(string)
 	description := d.Get("description").(string)
 	tags := getPolicyTagsFromSchema(d)
@@ -84,7 +137,8 @@ func resourceNsxtPolicyGatewayPolicyCreate(d *schema.ResourceData, m interface{}
 	locked := d.Get("locked").(bool)
 	sequenceNumber := int64(d.Get("sequence_number").(int))
 	stateful := d.Get("stateful").(bool)
-	rules := getPolicyRulesFromSchema(d, false)
+	revision := int64(d.Get("revision").(int))
+	objType := "GatewayPolicy"
 
 	obj := model.GatewayPolicy{
 		DisplayName:    &displayName,
@@ -95,27 +149,41 @@ func resourceNsxtPolicyGatewayPolicyCreate(d *schema.ResourceData, m interface{}
 		Locked:         &locked,
 		SequenceNumber: &sequenceNumber,
 		Stateful:       &stateful,
-		Rules:          rules,
+		ResourceType:   &objType,
+		Id:             &id,
 	}
-
 	_, isSet := d.GetOkExists("tcp_strict")
 	if isSet {
 		tcpStrict := d.Get("tcp_strict").(bool)
 		obj.TcpStrict = &tcpStrict
 	}
 
-	log.Printf("[INFO] Creating Gateway Policy with ID %s", id)
-	if isPolicyGlobalManager(m) {
-		client := gm_domains.NewGatewayPoliciesClient(connector)
-		gmObj, err1 := convertModelBindingType(obj, model.GatewayPolicyBindingType(), gm_model.GatewayPolicyBindingType())
-		if err1 != nil {
-			return err1
-		}
-		err = client.Patch(d.Get("domain").(string), id, gmObj.(gm_model.GatewayPolicy))
-	} else {
-		client := domains.NewGatewayPoliciesClient(connector)
-		err = client.Patch(d.Get("domain").(string), id, obj)
+	if len(d.Id()) > 0 {
+		// This is update flow
+		obj.Revision = &revision
 	}
+
+	policyChildren, err := getUpdatedRuleChildren(d)
+	if err != nil {
+		return err
+	}
+	if len(policyChildren) > 0 {
+		obj.Children = policyChildren
+	}
+
+	return gatewayPolicyInfraPatch(obj, domain, m)
+}
+
+func resourceNsxtPolicyGatewayPolicyCreate(d *schema.ResourceData, m interface{}) error {
+	connector := getPolicyConnector(m)
+
+	// Initialize resource Id and verify this ID is not yet used
+	id, err := getOrGenerateID(d, m, resourceNsxtPolicyGatewayPolicyExistsPartial(d.Get("domain").(string)))
+	if err != nil {
+		return err
+	}
+
+	err = policyGatewayPolicyBuildAndPatch(d, m, connector, isPolicyGlobalManager(m), id)
 	if err != nil {
 		return handleCreateError("Gateway Policy", id, err)
 	}
@@ -166,48 +234,7 @@ func resourceNsxtPolicyGatewayPolicyUpdate(d *schema.ResourceData, m interface{}
 		return fmt.Errorf("Error obtaining Gateway Policy ID")
 	}
 
-	displayName := d.Get("display_name").(string)
-	description := d.Get("description").(string)
-	tags := getPolicyTagsFromSchema(d)
-	category := d.Get("category").(string)
-	comments := d.Get("comments").(string)
-	locked := d.Get("locked").(bool)
-	sequenceNumber := int64(d.Get("sequence_number").(int))
-	stateful := d.Get("stateful").(bool)
-	tcpStrict := d.Get("tcp_strict").(bool)
-	rules := getPolicyRulesFromSchema(d, false)
-	revision := int64(d.Get("revision").(int))
-
-	obj := model.GatewayPolicy{
-		DisplayName:    &displayName,
-		Description:    &description,
-		Tags:           tags,
-		Category:       &category,
-		Comments:       &comments,
-		Locked:         &locked,
-		SequenceNumber: &sequenceNumber,
-		Stateful:       &stateful,
-		TcpStrict:      &tcpStrict,
-		Revision:       &revision,
-		Rules:          rules,
-	}
-
-	var err error
-	if isPolicyGlobalManager(m) {
-		rawObj, err1 := convertModelBindingType(obj, model.GatewayPolicyBindingType(), gm_model.GatewayPolicyBindingType())
-		if err1 != nil {
-			return err1
-		}
-		gmObj := rawObj.(gm_model.GatewayPolicy)
-		client := gm_domains.NewGatewayPoliciesClient(connector)
-		// We need to use PUT, because PATCH will not replace the whole rule list
-		_, err = client.Update(d.Get("domain").(string), id, gmObj)
-	} else {
-		client := domains.NewGatewayPoliciesClient(connector)
-		// We need to use PUT, because PATCH will not replace the whole rule list
-		_, err = client.Update(d.Get("domain").(string), id, obj)
-	}
-
+	err := policyGatewayPolicyBuildAndPatch(d, m, connector, isPolicyGlobalManager(m), id)
 	if err != nil {
 		return handleUpdateError("Gateway Policy", id, err)
 	}

nsxt/resource_nsxt_policy_predefined_gateway_policy.go

@@ -310,7 +310,7 @@ func updatePolicyPredefinedGatewayPolicy(id string, d *schema.ResourceData, m in
 	var childRules []*data.StructValue
 	if d.HasChange("rule") {
 		oldRules, _ := d.GetChange("rule")
-		rules := getPolicyRulesFromSchema(d, true)
+		rules := getPolicyRulesFromSchema(d)
 
 		existingRules := make(map[string]bool)
 		for _, rule := range rules {

nsxt/resource_nsxt_policy_predefined_security_policy.go

@@ -235,7 +235,7 @@ func updatePolicyPredefinedSecurityPolicy(id string, d *schema.ResourceData, m i
 	var childRules []*data.StructValue
 	if d.HasChange("rule") {
 		oldRules, _ := d.GetChange("rule")
-		rules := getPolicyRulesFromSchema(d, true)
+		rules := getPolicyRulesFromSchema(d)
 
 		existingRules := make(map[string]bool)
 		for _, rule := range rules {

nsxt/resource_nsxt_policy_security_policy.go

@@ -73,15 +73,9 @@ func resourceNsxtPolicySecurityPolicyExistsPartial(domainName string) func(id st
 	}
 }
 
-func resourceNsxtPolicySecurityPolicyCreate(d *schema.ResourceData, m interface{}) error {
-	connector := getPolicyConnector(m)
-
-	// Initialize resource Id and verify this ID is not yet used
-	id, err := getOrGenerateID(d, m, resourceNsxtPolicySecurityPolicyExistsPartial(d.Get("domain").(string)))
-	if err != nil {
-		return err
-	}
+func policySecurityPolicyBuildAndPatch(d *schema.ResourceData, m interface{}, connector *client.RestConnector, isGlobalManager bool, id string) error {
 
+	domain := d.Get("domain").(string)
 	displayName := d.Get("display_name").(string)
 	description := d.Get("description").(string)
 	tags := getPolicyTagsFromSchema(d)
@@ -92,9 +86,11 @@ func resourceNsxtPolicySecurityPolicyCreate(d *schema.ResourceData, m interface{
 	sequenceNumber := int64(d.Get("sequence_number").(int))
 	stateful := d.Get("stateful").(bool)
 	tcpStrict := d.Get("tcp_strict").(bool)
-	rules := getPolicyRulesFromSchema(d, false)
+	revision := int64(d.Get("revision").(int))
+	objType := "SecurityPolicy"
 
 	obj := model.SecurityPolicy{
+		Id:             &id,
 		DisplayName:    &displayName,
 		Description:    &description,
 		Tags:           tags,
@@ -105,21 +101,37 @@ func resourceNsxtPolicySecurityPolicyCreate(d *schema.ResourceData, m interface{
 		SequenceNumber: &sequenceNumber,
 		Stateful:       &stateful,
 		TcpStrict:      &tcpStrict,
-		Rules:          rules,
+		ResourceType:   &objType,
 	}
 	log.Printf("[INFO] Creating Security Policy with ID %s", id)
-	if isPolicyGlobalManager(m) {
-		gmObj, err1 := convertModelBindingType(obj, model.SecurityPolicyBindingType(), gm_model.SecurityPolicyBindingType())
-		if err1 != nil {
-			return err1
-		}
-		client := gm_domains.NewSecurityPoliciesClient(connector)
-		err = client.Patch(d.Get("domain").(string), id, gmObj.(gm_model.SecurityPolicy))
-	} else {
-		client := domains.NewSecurityPoliciesClient(connector)
-		err = client.Patch(d.Get("domain").(string), id, obj)
+
+	if len(d.Id()) > 0 {
+		// This is update flow
+		obj.Revision = &revision
+	}
+
+	policyChildren, err := getUpdatedRuleChildren(d)
+	if err != nil {
+		return err
+	}
+	if len(policyChildren) > 0 {
+		obj.Children = policyChildren
 	}
 
+	return securityPolicyInfraPatch(obj, domain, m)
+}
+
+func resourceNsxtPolicySecurityPolicyCreate(d *schema.ResourceData, m interface{}) error {
+	connector := getPolicyConnector(m)
+
+	// Initialize resource Id and verify this ID is not yet used
+	id, err := getOrGenerateID(d, m, resourceNsxtPolicySecurityPolicyExistsPartial(d.Get("domain").(string)))
+	if err != nil {
+		return err
+	}
+
+	err = policySecurityPolicyBuildAndPatch(d, m, connector, isPolicyGlobalManager(m), id)
+
 	if err != nil {
 		return handleCreateError("Security Policy", id, err)
 	}
@@ -185,52 +197,7 @@ func resourceNsxtPolicySecurityPolicyUpdate(d *schema.ResourceData, m interface{
 	if id == "" {
 		return fmt.Errorf("Error obtaining Security Policy id")
 	}
-
-	displayName := d.Get("display_name").(string)
-	description := d.Get("description").(string)
-	tags := getPolicyTagsFromSchema(d)
-	category := d.Get("category").(string)
-	comments := d.Get("comments").(string)
-	locked := d.Get("locked").(bool)
-	scope := getStringListFromSchemaSet(d, "scope")
-	sequenceNumber := int64(d.Get("sequence_number").(int))
-	stateful := d.Get("stateful").(bool)
-	tcpStrict := d.Get("tcp_strict").(bool)
-	rules := getPolicyRulesFromSchema(d, false)
-	revision := int64(d.Get("revision").(int))
-
-	obj := model.SecurityPolicy{
-		DisplayName:    &displayName,
-		Description:    &description,
-		Tags:           tags,
-		Category:       &category,
-		Comments:       &comments,
-		Locked:         &locked,
-		Scope:          scope,
-		SequenceNumber: &sequenceNumber,
-		Stateful:       &stateful,
-		TcpStrict:      &tcpStrict,
-		Revision:       &revision,
-		Rules:          rules,
-	}
-
-	var err error
-	if isPolicyGlobalManager(m) {
-		gmObj, err1 := convertModelBindingType(obj, model.SecurityPolicyBindingType(), gm_model.SecurityPolicyBindingType())
-		if err1 != nil {
-			return err1
-		}
-		gmSecurityPolicy := gmObj.(gm_model.SecurityPolicy)
-		client := gm_domains.NewSecurityPoliciesClient(connector)
-
-		// We need to use PUT, because PATCH will not replace the whole rule list
-		_, err = client.Update(d.Get("domain").(string), id, gmSecurityPolicy)
-	} else {
-		client := domains.NewSecurityPoliciesClient(connector)
-
-		// We need to use PUT, because PATCH will not replace the whole rule list
-		_, err = client.Update(d.Get("domain").(string), id, obj)
-	}
+	err := policySecurityPolicyBuildAndPatch(d, m, connector, isPolicyGlobalManager(m), id)
 	if err != nil {
 		return handleUpdateError("Security Policy", id, err)
 	}