Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
GraysonWu committed Dec 13, 2023
1 parent d509e97 commit ece03e4
Show file tree
Hide file tree
Showing 10 changed files with 84 additions and 74 deletions.
3 changes: 2 additions & 1 deletion nsxt/policy_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ func getSecurityPolicyAndGatewayRuleSchema(scopeRequired bool, isIds bool, nsxID
"nsx_id": getFlexNsxIDSchema(nsxIDReadOnly),
"display_name": getDisplayNameSchema(),
"description": getDescriptionSchema(),
"path": getPathSchema(),
"revision": getRevisionSchema(),
"destination_groups": {
Type: schema.TypeSet,
Expand Down Expand Up @@ -296,7 +297,6 @@ func getSecurityPolicyAndGatewayRuleSchema(scopeRequired bool, isIds bool, nsxID
Required: true,
}
ruleSchema["context"] = getContextSchema()
ruleSchema["path"] = getPathSchema()
} else {
ruleSchema["sequence_number"] = &schema.Schema{
Type: schema.TypeInt,
Expand Down Expand Up @@ -399,6 +399,7 @@ func setPolicyRulesInSchema(d *schema.ResourceData, rules []model.Rule) error {
elem := make(map[string]interface{})
elem["display_name"] = rule.DisplayName
elem["description"] = rule.Description
elem["path"] = rule.Path
elem["notes"] = rule.Notes
elem["logged"] = rule.Logged
elem["log_label"] = rule.Tag
Expand Down
4 changes: 4 additions & 0 deletions nsxt/policy_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,10 @@ func getDomainFromResourcePath(rPath string) string {
return getResourceIDFromResourcePath(rPath, "domains")
}

func getProjectIDFromResourcePath(rPath string) string {
return getResourceIDFromResourcePath(rPath, "projects")
}

func getResourceIDFromResourcePath(rPath string, rType string) string {
segments := strings.Split(rPath, "/")
for i, seg := range segments {
Expand Down
2 changes: 1 addition & 1 deletion nsxt/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ func Provider() *schema.Provider {
"nsxt_policy_host_transport_node_collection": resourceNsxtPolicyHostTransportNodeCollection(),
"nsxt_policy_lb_client_ssl_profile": resourceNsxtPolicyLBClientSslProfile(),
"nsxt_policy_security_policy_rule": resourceNsxtPolicySecurityPolicyRule(),
"nsxt_policy_security_policy_no_rule": resourceNsxtPolicySecurityPolicyNoRule(),
"nsxt_policy_parent_security_policy": resourceNsxtPolicyParentSecurityPolicy(),
},

ConfigureFunc: providerConfigure,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,20 @@ import (
"github.com/vmware/terraform-provider-nsxt/api/infra/domains"
)

func resourceNsxtPolicySecurityPolicyNoRule() *schema.Resource {
func resourceNsxtPolicyParentSecurityPolicy() *schema.Resource {
return &schema.Resource{
Create: resourceNsxtPolicySecurityPolicyNoRuleCreate,
Read: resourceNsxtPolicySecurityPolicyNoRuleRead,
Update: resourceNsxtPolicySecurityPolicyNoRuleUpdate,
Delete: resourceNsxtPolicySecurityPolicyNoRuleDelete,
Create: resourceNsxtPolicyParentSecurityPolicyCreate,
Read: resourceNsxtPolicyParentSecurityPolicyRead,
Update: resourceNsxtPolicyParentSecurityPolicyUpdate,
Delete: resourceNsxtPolicyParentSecurityPolicyDelete,
Importer: &schema.ResourceImporter{
State: nsxtDomainResourceImporter,
},
Schema: getPolicySecurityPolicySchema(false, true, false),
}
}

func resourceNsxtPolicySecurityPolicyNoRuleCreate(d *schema.ResourceData, m interface{}) error {
func resourceNsxtPolicyParentSecurityPolicyCreate(d *schema.ResourceData, m interface{}) error {
connector := getPolicyConnector(m)

// Initialize resource Id and verify this ID is not yet used
Expand All @@ -39,7 +39,7 @@ func resourceNsxtPolicySecurityPolicyNoRuleCreate(d *schema.ResourceData, m inte
domain := d.Get("domain").(string)
client := domains.NewSecurityPoliciesClient(getSessionContext(d, m), connector)

obj := securityPolicySchemaToModelNoRule(d, id)
obj := parentSecurityPolicySchemaToModel(d, id)
err = client.Patch(domain, id, obj)
if err != nil {
return handleCreateError("Security Policy", id, err)
Expand All @@ -48,10 +48,10 @@ func resourceNsxtPolicySecurityPolicyNoRuleCreate(d *schema.ResourceData, m inte
d.SetId(id)
d.Set("nsx_id", id)

return resourceNsxtPolicySecurityPolicyNoRuleRead(d, m)
return resourceNsxtPolicyParentSecurityPolicyRead(d, m)
}

func securityPolicySchemaToModelNoRule(d *schema.ResourceData, id string) model.SecurityPolicy {
func parentSecurityPolicySchemaToModel(d *schema.ResourceData, id string) model.SecurityPolicy {
displayName := d.Get("display_name").(string)
description := d.Get("description").(string)
tags := getPolicyTagsFromSchema(d)
Expand Down Expand Up @@ -80,12 +80,12 @@ func securityPolicySchemaToModelNoRule(d *schema.ResourceData, id string) model.
}
}

func resourceNsxtPolicySecurityPolicyNoRuleRead(d *schema.ResourceData, m interface{}) error {
_, err := securityPolicyModelToSchemaNoRule(d, m)
func resourceNsxtPolicyParentSecurityPolicyRead(d *schema.ResourceData, m interface{}) error {
_, err := parentSecurityPolicyModelToSchema(d, m)
return err
}

func securityPolicyModelToSchemaNoRule(d *schema.ResourceData, m interface{}) (*model.SecurityPolicy, error) {
func parentSecurityPolicyModelToSchema(d *schema.ResourceData, m interface{}) (*model.SecurityPolicy, error) {
connector := getPolicyConnector(m)
id := d.Id()
domainName := d.Get("domain").(string)
Expand Down Expand Up @@ -118,7 +118,7 @@ func securityPolicyModelToSchemaNoRule(d *schema.ResourceData, m interface{}) (*
return &obj, nil
}

func resourceNsxtPolicySecurityPolicyNoRuleUpdate(d *schema.ResourceData, m interface{}) error {
func resourceNsxtPolicyParentSecurityPolicyUpdate(d *schema.ResourceData, m interface{}) error {
connector := getPolicyConnector(m)

id := d.Id()
Expand All @@ -134,16 +134,16 @@ func resourceNsxtPolicySecurityPolicyNoRuleUpdate(d *schema.ResourceData, m inte
return handleUpdateError("Security Policy", id, err)
}

obj := securityPolicySchemaToModelNoRule(d, id)
obj := parentSecurityPolicySchemaToModel(d, id)
obj.Rules = remoteObj.Rules
err = client.Patch(domain, id, obj)
if err != nil {
return handleUpdateError("Security Policy", id, err)
}

return resourceNsxtPolicySecurityPolicyNoRuleRead(d, m)
return resourceNsxtPolicyParentSecurityPolicyRead(d, m)
}

func resourceNsxtPolicySecurityPolicyNoRuleDelete(d *schema.ResourceData, m interface{}) error {
func resourceNsxtPolicyParentSecurityPolicyDelete(d *schema.ResourceData, m interface{}) error {
return resourceNsxtPolicySecurityPolicyDelete(d, m)
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,21 @@ import (
"github.com/hashicorp/terraform-plugin-sdk/v2/terraform"
)

func TestAccResourceNsxtPolicySecurityPolicyNoRule_basic(t *testing.T) {
testAccResourceNsxtPolicySecurityPolicyNoRuleBasic(t, false, func() {
func TestAccResourceNsxtPolicyParentSecurityPolicy_basic(t *testing.T) {
testAccResourceNsxtPolicyParentSecurityPolicyBasic(t, false, func() {
testAccPreCheck(t)
})
}

func TestAccResourceNsxtPolicySecurityPolicyNoRule_multitenancy(t *testing.T) {
testAccResourceNsxtPolicySecurityPolicyNoRuleBasic(t, true, func() {
func TestAccResourceNsxtPolicyParentSecurityPolicy_multitenancy(t *testing.T) {
testAccResourceNsxtPolicyParentSecurityPolicyBasic(t, true, func() {
testAccPreCheck(t)
testAccOnlyMultitenancy(t)
})
}

func testAccResourceNsxtPolicySecurityPolicyNoRuleBasic(t *testing.T, withContext bool, preCheck func()) {
testResourceName := "nsxt_policy_security_policy_no_rule.test"
func testAccResourceNsxtPolicyParentSecurityPolicyBasic(t *testing.T, withContext bool, preCheck func()) {
testResourceName := "nsxt_policy_parent_security_policy.test"

name := getAccTestResourceName()
updatedName := getAccTestResourceName()
Expand All @@ -40,11 +40,11 @@ func testAccResourceNsxtPolicySecurityPolicyNoRuleBasic(t *testing.T, withContex
PreCheck: preCheck,
Providers: testAccProviders,
CheckDestroy: func(state *terraform.State) error {
return testAccNsxtPolicySecurityPolicyNoRuleCheckDestroy(state, updatedName)
return testAccNsxtPolicyParentSecurityPolicyCheckDestroy(state, updatedName)
},
Steps: []resource.TestStep{
{
Config: testAccNsxtPolicySecurityPolicyNoRuleTemplate(withContext, name, locked, seqNum, tcpStrict),
Config: testAccNsxtPolicyParentSecurityPolicyTemplate(withContext, name, locked, seqNum, tcpStrict),
Check: resource.ComposeTestCheckFunc(
testAccNsxtPolicySecurityPolicyExists(testResourceName, defaultDomain),
resource.TestCheckResourceAttr(testResourceName, "display_name", name),
Expand All @@ -54,7 +54,7 @@ func testAccResourceNsxtPolicySecurityPolicyNoRuleBasic(t *testing.T, withContex
),
},
{
Config: testAccNsxtPolicySecurityPolicyNoRuleTemplate(withContext, updatedName, updatedLocked, updatedSeqNum, updatedTCPStrict),
Config: testAccNsxtPolicyParentSecurityPolicyTemplate(withContext, updatedName, updatedLocked, updatedSeqNum, updatedTCPStrict),
Check: resource.ComposeTestCheckFunc(
testAccNsxtPolicySecurityPolicyExists(testResourceName, defaultDomain),
resource.TestCheckResourceAttr(testResourceName, "display_name", updatedName),
Expand All @@ -67,19 +67,19 @@ func testAccResourceNsxtPolicySecurityPolicyNoRuleBasic(t *testing.T, withContex
})
}

func TestAccResourceNsxtPolicySecurityPolicyNoRule_importBasic(t *testing.T) {
func TestAccResourceNsxtPolicyParentSecurityPolicy_importBasic(t *testing.T) {
name := getAccTestResourceName()
testResourceName := "nsxt_policy_security_policy_no_rule.test"
testResourceName := "nsxt_policy_parent_security_policy.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: func(state *terraform.State) error {
return testAccNsxtPolicySecurityPolicyNoRuleCheckDestroy(state, name)
return testAccNsxtPolicyParentSecurityPolicyCheckDestroy(state, name)
},
Steps: []resource.TestStep{
{
Config: testAccNsxtPolicySecurityPolicyNoRuleTemplate(false, name, "true", "1", "true"),
Config: testAccNsxtPolicyParentSecurityPolicyTemplate(false, name, "true", "1", "true"),
},
{
ResourceName: testResourceName,
Expand All @@ -91,9 +91,9 @@ func TestAccResourceNsxtPolicySecurityPolicyNoRule_importBasic(t *testing.T) {
})
}

func TestAccResourceNsxtPolicySecurityPolicyNoRule_importBasic_multitenancy(t *testing.T) {
func TestAccResourceNsxtPolicyParentSecurityPolicy_importBasic_multitenancy(t *testing.T) {
name := getAccTestResourceName()
testResourceName := "nsxt_policy_security_policy_no_rule.test"
testResourceName := "nsxt_policy_parent_security_policy.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() {
Expand All @@ -106,7 +106,7 @@ func TestAccResourceNsxtPolicySecurityPolicyNoRule_importBasic_multitenancy(t *t
},
Steps: []resource.TestStep{
{
Config: testAccNsxtPolicySecurityPolicyNoRuleTemplate(true, name, "true", "1", "true"),
Config: testAccNsxtPolicyParentSecurityPolicyTemplate(true, name, "true", "1", "true"),
},
{
ResourceName: testResourceName,
Expand All @@ -118,11 +118,11 @@ func TestAccResourceNsxtPolicySecurityPolicyNoRule_importBasic_multitenancy(t *t
})
}

func testAccNsxtPolicySecurityPolicyNoRuleCheckDestroy(state *terraform.State, displayName string) error {
func testAccNsxtPolicyParentSecurityPolicyCheckDestroy(state *terraform.State, displayName string) error {
connector := getPolicyConnector(testAccProvider.Meta().(nsxtClients))
for _, rs := range state.RootModule().Resources {

if rs.Type != "nsxt_policy_security_policy_no_rule" {
if rs.Type != "nsxt_policy_parent_security_policy" {
continue
}

Expand All @@ -139,13 +139,13 @@ func testAccNsxtPolicySecurityPolicyNoRuleCheckDestroy(state *terraform.State, d
return nil
}

func testAccNsxtPolicySecurityPolicyNoRuleTemplate(withContext bool, name, locked, seqNum, tcpStrict string) string {
func testAccNsxtPolicyParentSecurityPolicyTemplate(withContext bool, name, locked, seqNum, tcpStrict string) string {
context := ""
if withContext {
context = testAccNsxtPolicyMultitenancyContext()
}
return testAccNsxtPolicySecurityPolicyDeps() + fmt.Sprintf(`
resource "nsxt_policy_security_policy_no_rule" "test" {
resource "nsxt_policy_parent_security_policy" "test" {
%s
display_name = "%s"
description = "Acceptance Test"
Expand Down
4 changes: 2 additions & 2 deletions nsxt/resource_nsxt_policy_security_policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func resourceNsxtPolicySecurityPolicyExistsPartial(domainName string) func(sessi

func policySecurityPolicyBuildAndPatch(d *schema.ResourceData, m interface{}, connector client.Connector, isGlobalManager bool, id string, createFlow bool) error {

obj := securityPolicySchemaToModelNoRule(d, id)
obj := parentSecurityPolicySchemaToModel(d, id)
domain := d.Get("domain").(string)
revision := int64(d.Get("revision").(int))
log.Printf("[INFO] Creating Security Policy with ID %s", id)
Expand Down Expand Up @@ -105,7 +105,7 @@ func resourceNsxtPolicySecurityPolicyCreate(d *schema.ResourceData, m interface{
}

func resourceNsxtPolicySecurityPolicyRead(d *schema.ResourceData, m interface{}) error {
obj, err := securityPolicyModelToSchemaNoRule(d, m)
obj, err := parentSecurityPolicyModelToSchema(d, m)
if err != nil {
return err
}
Expand Down
31 changes: 23 additions & 8 deletions nsxt/resource_nsxt_policy_security_policy_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,26 @@ func resourceNsxtPolicySecurityPolicyRule() *schema.Resource {
func resourceNsxtPolicySecurityPolicyRuleCreate(d *schema.ResourceData, m interface{}) error {
connector := getPolicyConnector(m)

policyPath := d.Get("policy_path").(string)
projectID := getProjectIDFromResourcePath(policyPath)
domain := getDomainFromResourcePath(policyPath)
policyID := getPolicyIDFromPath(policyPath)

if len(d.Get("context").([]interface{})) == 0 {
contexts := make([]interface{}, 1)
ctxMap := make(map[string]interface{})
ctxMap["project_id"] = projectID
contexts[0] = ctxMap
d.Set("context", contexts)
}

// Initialize resource Id and verify this ID is not yet used
id, err := getOrGenerateID2(d, m, resourceNsxtPolicySecurityPolicyRuleExistsPartial(d.Get("policy_path").(string)))
if err != nil {
return err
}

policyPath := d.Get("policy_path").(string)
log.Printf("[INFO] Creating Security Policy Rule with ID %s under policy %s", id, policyPath)
domain := getDomainFromResourcePath(policyPath)
policyID := getPolicyIDFromPath(policyPath)

client := securitypolicies.NewRulesClient(getSessionContext(d, m), connector)
rule := securityPolicyRuleSchemaToModel(d, id)
err = client.Patch(domain, policyID, id, rule)
Expand Down Expand Up @@ -134,8 +143,18 @@ func resourceNsxtPolicySecurityPolicyRuleRead(d *schema.ResourceData, m interfac
}

policyPath := d.Get("policy_path").(string)
projectID := getProjectIDFromResourcePath(policyPath)
domain := getDomainFromResourcePath(policyPath)
policyID := getPolicyIDFromPath(policyPath)

if len(d.Get("context").([]interface{})) == 0 {
contexts := make([]interface{}, 1)
ctxMap := make(map[string]interface{})
ctxMap["project_id"] = projectID
contexts[0] = ctxMap
d.Set("context", contexts)
}

client := securitypolicies.NewRulesClient(getSessionContext(d, m), connector)
rule, err := client.Get(domain, policyID, id)
if err != nil {
Expand Down Expand Up @@ -218,10 +237,6 @@ func resourceNsxtPolicySecurityPolicyRuleDelete(d *schema.ResourceData, m interf

func nsxtSecurityPolicyRuleImporter(d *schema.ResourceData, m interface{}) ([]*schema.ResourceData, error) {
importID := d.Id()
// Example of Rule path: /infra/domains/default/security-policies/04e862ad-ddce-434c-8453-229e2740982e/rules/b971bdc3-9e8f-442d-a694-846cbbb46ca5
if strings.Count(importID, "/") != 7 {
return nil, fmt.Errorf("Invalid SecurityPolicyRule path %s", importID)
}
rd, err := nsxtPolicyPathResourceImporterHelper(d, m)
if err != nil {
return rd, err
Expand Down
8 changes: 4 additions & 4 deletions nsxt/resource_nsxt_policy_security_policy_rule_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ func testAccNsxtPolicySecurityPolicyRuleDeps(withContext bool) string {
context = testAccNsxtPolicyMultitenancyContext()
}
return testAccNsxtPolicySecurityPolicyDeps() + fmt.Sprintf(`
resource "nsxt_policy_security_policy_no_rule" "policy1" {
resource "nsxt_policy_parent_security_policy" "policy1" {
%s
display_name = "no-rule-policy"
description = "Acceptance Test"
Expand Down Expand Up @@ -205,7 +205,7 @@ func testAccNsxtPolicySecurityPolicyRuleTemplate(withContext bool, name, action,
resource "nsxt_policy_security_policy_rule" "test" {
%s
display_name = "%s"
policy_path = nsxt_policy_security_policy_no_rule.policy1.path
policy_path = nsxt_policy_parent_security_policy.policy1.path
action = "%s"
direction = "%s"
ip_version = "%s"
Expand All @@ -217,13 +217,13 @@ resource "nsxt_policy_security_policy_rule" "test" {
tag = "orange"
}
depends_on = [nsxt_policy_security_policy_no_rule.policy1, nsxt_policy_group.group2]
depends_on = [nsxt_policy_parent_security_policy.policy1, nsxt_policy_group.group2]
}
data "nsxt_policy_security_policy_rule" "test" {
%s
display_name = "%s"
policy_path = nsxt_policy_security_policy_no_rule.policy1.path
policy_path = nsxt_policy_parent_security_policy.policy1.path
depends_on = [nsxt_policy_security_policy_rule.test]
}`, context, name, action, direction, ipVersion, seqNum, context, name)
}
Loading

0 comments on commit ece03e4

Please sign in to comment.