From ad2d02b55281a47a3260e2e7a508d88d505e1e95 Mon Sep 17 00:00:00 2001
From: Anna Khmelnitsky <akhmelnitsky@vmware.com>
Date: Thu, 2 Jan 2025 16:44:38 -0800
Subject: [PATCH] Remove member type validation in group criteria

This follows NSX allowing mixing some member types within
same expression block, which was not allowed earlier.

Rather then following NSX validation for which member types mixes are
permitted, terraform will let NSX do the validation for this part.
Other validations on terraform side - such as same expression type -
are not affected by this change.

Signed-off-by: Anna Khmelnitsky <akhmelnitsky@vmware.com>
---
 nsxt/resource_nsxt_policy_group.go      | 34 +----------------
 nsxt/resource_nsxt_policy_group_test.go | 49 +++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 33 deletions(-)

diff --git a/nsxt/resource_nsxt_policy_group.go b/nsxt/resource_nsxt_policy_group.go
index 1ec613fd8..8154f56ae 100644
--- a/nsxt/resource_nsxt_policy_group.go
+++ b/nsxt/resource_nsxt_policy_group.go
@@ -6,7 +6,6 @@ package nsxt
 import (
 	"fmt"
 	"log"
-	"strings"
 
 	"github.com/vmware/terraform-provider-nsxt/api/infra/domains"
 	utl "github.com/vmware/terraform-provider-nsxt/api/utl"
@@ -352,22 +351,8 @@ func resourceNsxtPolicyGroupExistsInDomainPartial(domain string) func(sessionCon
 	}
 }
 
-func validateNestedGroupConditions(conditions []interface{}) (string, error) {
-	memberType := ""
-	for _, cond := range conditions {
-		condMap := cond.(map[string]interface{})
-		condMemberType := condMap["member_type"].(string)
-		if memberType != "" && condMemberType != memberType {
-			return "", fmt.Errorf("Nested conditions must all use the same member_type, but found '%v' with '%v'", condMemberType, memberType)
-		}
-		memberType = condMemberType
-	}
-	return memberType, nil
-}
-
 type criteriaMeta struct {
 	ExpressionType string
-	MemberType     string
 	IsNested       bool
 	criteriaBlocks []interface{}
 }
@@ -381,25 +366,12 @@ func validateGroupCriteriaSets(criteriaSets []interface{}) ([]criteriaMeta, erro
 		seenExp := ""
 		criteriaMap := criteriaBlock.(map[string]interface{})
 		for expName, expVal := range criteriaMap {
-			memberType := ""
 			expValList := expVal.([]interface{})
 			if len(expValList) > 0 {
 				if seenExp != "" {
-					return nil, fmt.Errorf("Criteria blocks are homogeneous, but found '%v' with '%v'", expName, seenExp)
-				}
-				if expName == "condition" {
-					mType, err := validateNestedGroupConditions(expValList)
-					if err != nil {
-						return nil, err
-					}
-					memberType = mType
-				} else if strings.HasSuffix(expName, "_expression") {
-					memberType = ""
-				} else {
-					return nil, fmt.Errorf("Unknown criteria: %v", expName)
+					return nil, fmt.Errorf("Criteria blocks should be homogeneous, but found '%v' with '%v'", expName, seenExp)
 				}
 				criteriaType := criteriaMeta{
-					MemberType:     memberType,
 					ExpressionType: expName,
 					IsNested:       len(expValList) > 1,
 					criteriaBlocks: expValList}
@@ -422,10 +394,6 @@ func validateGroupConjunctions(conjunctions []interface{}, criteriaMeta []criter
 				return fmt.Errorf("AND conjunctions must use the same types of criteria expressions, but got %v and %v",
 					metaA.ExpressionType, metaB.ExpressionType)
 			}
-			if metaA.MemberType != metaB.MemberType {
-				return fmt.Errorf("AND conjunctions with conditions must have the same member types, but got %v and %v",
-					metaA.MemberType, metaB.MemberType)
-			}
 		}
 	}
 	return nil
diff --git a/nsxt/resource_nsxt_policy_group_test.go b/nsxt/resource_nsxt_policy_group_test.go
index 8a490c6fe..fc5d37cf6 100644
--- a/nsxt/resource_nsxt_policy_group_test.go
+++ b/nsxt/resource_nsxt_policy_group_test.go
@@ -59,6 +59,33 @@ func TestAccResourceNsxtPolicyGroup_basicImport_multitenancy(t *testing.T) {
 	})
 }
 
+func TestAccResourceNsxtPolicyGroup_mixedCriteria(t *testing.T) {
+	name := getAccTestResourceName()
+	resourceName := "nsxt_policy_group"
+	testResourceName := fmt.Sprintf("%s.test", resourceName)
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:  func() { testAccPreCheck(t) },
+		Providers: testAccProviders,
+		CheckDestroy: func(state *terraform.State) error {
+			return testAccNsxtPolicyGroupCheckDestroy(state, name, defaultDomain)
+		},
+		Steps: []resource.TestStep{
+			{
+				Config: testAccNsxtPolicyGroupMixedCriteriaTemplate(name),
+				Check: resource.ComposeTestCheckFunc(
+					testAccNsxtPolicyGroupExists(testResourceName, defaultDomain),
+					resource.TestCheckResourceAttr(testResourceName, "display_name", name),
+					resource.TestCheckResourceAttrSet(testResourceName, "path"),
+					resource.TestCheckResourceAttrSet(testResourceName, "revision"),
+					resource.TestCheckResourceAttr(testResourceName, "criteria.#", "1"),
+					resource.TestCheckResourceAttr(testResourceName, "criteria.0.condition.#", "2"),
+				),
+			},
+		},
+	})
+}
+
 func TestAccResourceNsxtPolicyGroup_empty(t *testing.T) {
 	testAccResourceNsxtPolicyGroupEmpty(t, false, func() {
 		testAccPreCheck(t)
@@ -1575,3 +1602,25 @@ resource "nsxt_policy_group" "test" {
 }
 `, name)
 }
+
+func testAccNsxtPolicyGroupMixedCriteriaTemplate(name string) string {
+	return fmt.Sprintf(`
+resource "nsxt_policy_group" "test" {
+  display_name = "%s"
+
+  criteria {
+    condition {
+      key         = "Tag"
+      member_type = "Segment"
+      operator    = "EQUALS"
+      value       = "blue"
+    }
+    condition {
+      key         = "Tag"
+      member_type = "SegmentPort"
+      operator    = "EQUALS"
+      value       = "orange"
+    }
+  }
+}`, name)
+}