Allow creation of Ethernet category rules

Creation of Ethernet rules fails when ip_protocol is not null Fixes: #841 Signed-off-by: Kobi Samoray <[email protected]>
vmware · Mar 6, 2023 · 3d92539 · 3d92539
1 parent 8a79218
commit 3d92539
Show file tree

Hide file tree

Showing 3 changed files with 114 additions and 5 deletions.
diff --git a/nsxt/policy_common.go b/nsxt/policy_common.go
@@ -13,7 +13,7 @@ var defaultDomain = "default"
 var defaultSite = "default"
 var securityPolicyCategoryValues = []string{"Ethernet", "Emergency", "Infrastructure", "Environment", "Application"}
 var securityPolicyDirectionValues = []string{model.Rule_DIRECTION_IN, model.Rule_DIRECTION_OUT, model.Rule_DIRECTION_IN_OUT}
-var securityPolicyIPProtocolValues = []string{model.Rule_IP_PROTOCOL_IPV4, model.Rule_IP_PROTOCOL_IPV6, model.Rule_IP_PROTOCOL_IPV4_IPV6}
+var securityPolicyIPProtocolValues = []string{"NONE", model.Rule_IP_PROTOCOL_IPV4, model.Rule_IP_PROTOCOL_IPV6, model.Rule_IP_PROTOCOL_IPV4_IPV6}
 
 // TODO: change last string to sdk constant when available
 var securityPolicyActionValues = []string{model.Rule_ACTION_ALLOW, model.Rule_ACTION_DROP, model.Rule_ACTION_REJECT, "JUMP_TO_APPLICATION"}
@@ -373,7 +373,11 @@ func setPolicyRulesInSchema(d *schema.ResourceData, rules []model.Rule) error {
 		elem["action"] = rule.Action
 		elem["destinations_excluded"] = rule.DestinationsExcluded
 		elem["sources_excluded"] = rule.SourcesExcluded
-		elem["ip_version"] = rule.IpProtocol
+		if rule.IpProtocol == nil {
+			elem["ip_version"] = "NONE"
+		} else {
+			elem["ip_version"] = rule.IpProtocol
+		}
 		elem["direction"] = rule.Direction
 		elem["disabled"] = rule.Disabled
 		elem["revision"] = rule.Revision
@@ -414,7 +418,12 @@ func getPolicyRulesFromSchema(d *schema.ResourceData) []model.Rule {
 		disabled := data["disabled"].(bool)
 		sourcesExcluded := data["sources_excluded"].(bool)
 		destinationsExcluded := data["destinations_excluded"].(bool)
-		ipProtocol := data["ip_version"].(string)
+
+		var ipProtocol *string
+		ipp := data["ip_version"].(string)
+		if ipp != "NONE" {
+			ipProtocol = &ipp
+		}
 		direction := data["direction"].(string)
 		notes := data["notes"].(string)
 		seq := data["sequence_number"].(int)
@@ -441,7 +450,7 @@ func getPolicyRulesFromSchema(d *schema.ResourceData) []model.Rule {
 			Disabled:             &disabled,
 			SourcesExcluded:      &sourcesExcluded,
 			DestinationsExcluded: &destinationsExcluded,
-			IpProtocol:           &ipProtocol,
+			IpProtocol:           ipProtocol,
 			Direction:            &direction,
 			SourceGroups:         getPathListFromMap(data, "source_groups"),
 			DestinationGroups:    getPathListFromMap(data, "destination_groups"),

diff --git a/nsxt/resource_nsxt_policy_security_policy_test.go b/nsxt/resource_nsxt_policy_security_policy_test.go
@@ -141,6 +141,45 @@ func TestAccResourceNsxtPolicySecurityPolicy_basic(t *testing.T) {
 		},
 	})
 }
+func TestAccResourceNsxtPolicySecurityPolicy_EthernetRule(t *testing.T) {
+	name := getAccTestResourceName()
+	testResourceName := "nsxt_policy_security_policy.test"
+	direction1 := "IN"
+	proto1 := "NONE"
+	defaultAction := "ALLOW"
+	tag1 := "abc"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:  func() { testAccOnlyLocalManager(t); testAccPreCheck(t) },
+		Providers: testAccProviders,
+		CheckDestroy: func(state *terraform.State) error {
+			return testAccNsxtPolicySecurityPolicyCheckDestroy(state, name, defaultDomain)
+		},
+		Steps: []resource.TestStep{
+			{
+				Config: testAccNsxtPolicySecurityPolicyWithEthernetRule(name, direction1, proto1, tag1, defaultDomain, ""),
+				Check: resource.ComposeTestCheckFunc(
+					testAccNsxtPolicySecurityPolicyExists(testResourceName, defaultDomain),
+					resource.TestCheckResourceAttr(testResourceName, "display_name", name),
+					resource.TestCheckResourceAttr(testResourceName, "description", "Acceptance Test"),
+					resource.TestCheckResourceAttr(testResourceName, "category", "Ethernet"),
+					resource.TestCheckResourceAttr(testResourceName, "domain", defaultDomain),
+					resource.TestCheckResourceAttr(testResourceName, "comments", ""),
+					resource.TestCheckResourceAttr(testResourceName, "locked", "false"),
+					resource.TestCheckResourceAttr(testResourceName, "scope.#", "0"),
+					resource.TestCheckResourceAttr(testResourceName, "rule.#", "1"),
+					resource.TestCheckResourceAttr(testResourceName, "rule.0.display_name", name),
+					resource.TestCheckResourceAttr(testResourceName, "rule.0.direction", direction1),
+					resource.TestCheckResourceAttr(testResourceName, "rule.0.ip_version", proto1),
+					resource.TestCheckResourceAttr(testResourceName, "rule.0.action", defaultAction),
+					resource.TestCheckResourceAttr(testResourceName, "rule.0.log_label", tag1),
+					resource.TestCheckResourceAttrSet(testResourceName, "rule.0.rule_id"),
+					resource.TestCheckResourceAttr(testResourceName, "rule.0.tag.#", "1"),
+				),
+			},
+		},
+	})
+}
 
 func TestAccResourceNsxtPolicySecurityPolicy_withDependencies(t *testing.T) {
 	name := getAccTestResourceName()
@@ -678,6 +717,67 @@ resource "nsxt_policy_security_policy" "test" {
 }`, name, name, direction, protocol, ruleTag, profiles)
 }
 
+func testAccNsxtPolicySecurityPolicyWithEthernetRule(name string, direction string, protocol string, ruleTag string, domainName string, profiles string) string {
+	if domainName == defaultDomain {
+		return fmt.Sprintf(`
+resource "nsxt_policy_security_policy" "test" {
+  display_name    = "%s"
+  description     = "Acceptance Test"
+  category        = "Ethernet"
+  locked          = false
+  stateful        = false
+
+  tag {
+    scope = "color"
+    tag   = "orange"
+  }
+
+  rule {
+    display_name = "%s"
+    direction    = "%s"
+    ip_version   = "%s"
+    log_label    = "%s"
+
+    tag {
+      scope = "color"
+      tag   = "blue"
+    }
+    %s
+  }
+}`, name, name, direction, protocol, ruleTag, profiles)
+	}
+	return testAccNsxtGlobalPolicyGroupIPAddressCreateTemplate("group", domainName) + fmt.Sprintf(`
+resource "nsxt_policy_security_policy" "test" {
+  display_name    = "%s"
+  description     = "Acceptance Test"
+  category        = "Application"
+  locked          = false
+  sequence_number = 3
+  stateful        = true
+  tcp_strict      = false
+  domain          = data.nsxt_policy_site.test.id
+
+  tag {
+    scope = "color"
+    tag   = "orange"
+  }
+
+  rule {
+    display_name = "%s"
+    direction    = "%s"
+    ip_version   = "%s"
+    log_label    = "%s"
+    source_groups = [nsxt_policy_group.test.path]
+
+    tag {
+      scope = "color"
+      tag   = "blue"
+    }
+    %s
+  }
+}`, name, name, direction, protocol, ruleTag, profiles)
+}
+
 func testAccNsxtPolicySecurityPolicyDeps() string {
 	return `
 resource "nsxt_policy_group" "group1" {

diff --git a/website/docs/r/policy_security_policy.html.markdown b/website/docs/r/policy_security_policy.html.markdown
@@ -119,7 +119,7 @@ The following arguments are supported:
   * `sources_excluded` - (Optional) A boolean value indicating negation of source groups.
   * `direction` - (Optional) Traffic direction, one of `IN`, `OUT` or `IN_OUT`. Default is `IN_OUT`.
   * `disabled` - (Optional) Flag to disable this rule. Default is false.
-  * `ip_version` - (Optional) Version of IP protocol, one of `IPV4`, `IPV6`, `IPV4_IPV6`. Default is `IPV4_IPV6`.
+  * `ip_version` - (Optional) Version of IP protocol, one of `NONE`, IPV4`, `IPV6`, `IPV4_IPV6`. Default is `IPV4_IPV6`. For `Ethernet` category rules, use `NONE` value.
   * `logged` - (Optional) Flag to enable packet logging. Default is false.
   * `notes` - (Optional) Additional notes on changes.
   * `profiles` - (Optional) Set of profile paths relevant for this rule.