From f96542889359b8ff517e59c4ea436985e5f18564 Mon Sep 17 00:00:00 2001 From: Shawn Wang Date: Tue, 12 Dec 2023 13:47:26 -0800 Subject: [PATCH] Fix role for path as Set and add Acc Test Signed-off-by: Shawn Wang --- nsxt/resource_nsxt_policy_role_binding.go | 12 +- .../resource_nsxt_policy_role_binding_test.go | 121 ++++++++++++++++-- ...user_management_role_binding.html.markdown | 2 +- 3 files changed, 115 insertions(+), 20 deletions(-) diff --git a/nsxt/resource_nsxt_policy_role_binding.go b/nsxt/resource_nsxt_policy_role_binding.go index 37e3b254a..ccfdc4c2c 100644 --- a/nsxt/resource_nsxt_policy_role_binding.go +++ b/nsxt/resource_nsxt_policy_role_binding.go @@ -6,6 +6,7 @@ package nsxt import ( "fmt" "log" + "strings" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" @@ -85,7 +86,7 @@ func resourceNsxtPolicyUserManagementRoleBinding() *schema.Resource { // getRolesForPathSchema return schema for RolesForPath, which is shared between role bindings and PI func getRolesForPathSchema(forceNew bool) *schema.Schema { return &schema.Schema{ - Type: schema.TypeList, + Type: schema.TypeSet, Description: "List of roles that are associated with the user, limiting them to a path", Required: true, ForceNew: forceNew, @@ -123,7 +124,7 @@ func (r rolesPerPath) getAnyRole() *string { func getRolesForPathFromSchema(d *schema.ResourceData) rolesForPath { rolesForPathMap := make(rolesForPath) - rolesForPathInput := d.Get("roles_for_path").([]interface{}) + rolesForPathInput := d.Get("roles_for_path").(*schema.Set).List() for _, rolesPerPathInput := range rolesForPathInput { data := rolesPerPathInput.(map[string]interface{}) path := data["path"].(string) @@ -142,7 +143,7 @@ func setRolesForPathInSchema(d *schema.ResourceData, nsxRolesForPathList []nsxMo var rolesForPathList []map[string]interface{} for _, nsxRolesForPath := range nsxRolesForPathList { elem := make(map[string]interface{}) - elem["path"] = nsxRolesForPath.Path + elem["path"] = *nsxRolesForPath.Path roles := make([]string, 0, len(nsxRolesForPath.Roles)) for _, nsxRole := range nsxRolesForPath.Roles { roles = append(roles, *nsxRole.Role) @@ -196,7 +197,7 @@ func getRolesForPathList(d *schema.ResourceData, rolesToRemove rolesForPath) []n // Handle deletion of entire paths on change if d.HasChange("roles_for_path") { o, _ := d.GetChange("roles_for_path") - oldRoles := o.([]interface{}) + oldRoles := o.(*schema.Set).List() for _, oldRole := range oldRoles { data := oldRole.(map[string]interface{}) path := data["path"].(string) @@ -307,8 +308,9 @@ func revertRoleBinding(d *schema.ResourceData, m interface{}) error { if path == defaultRolePath { continue } + rPath := strings.Clone(path) nsxRolesForPaths = append(nsxRolesForPaths, nsxModel.RolesForPath{ - Path: &path, + Path: &rPath, DeletePath: &boolTrue, Roles: []nsxModel.Role{{Role: roleMap.getAnyRole()}}, }) diff --git a/nsxt/resource_nsxt_policy_role_binding_test.go b/nsxt/resource_nsxt_policy_role_binding_test.go index 523a77b94..f0086b39c 100644 --- a/nsxt/resource_nsxt_policy_role_binding_test.go +++ b/nsxt/resource_nsxt_policy_role_binding_test.go @@ -40,7 +40,7 @@ func TestAccResourceNsxtPolicyRoleBinding_basic(t *testing.T) { }, Steps: []resource.TestStep{ { - Config: testAccNsxtPolicyRoleBindingCreate(getTestLdapUser(), userType, identType), + Config: testAccNsxtPolicyRoleBindingCreate(getTestLdapUser(), userType, identType, "false", ""), Check: resource.ComposeTestCheckFunc( testAccNsxtPolicyRoleBindingExists(accTestPolicyRoleBindingCreateAttributes["display_name"], testResourceName), resource.TestCheckResourceAttr(testResourceName, "display_name", accTestPolicyRoleBindingCreateAttributes["display_name"]), @@ -51,7 +51,7 @@ func TestAccResourceNsxtPolicyRoleBinding_basic(t *testing.T) { resource.TestCheckResourceAttr(testResourceName, "roles_for_path.#", "2"), resource.TestCheckResourceAttr(testResourceName, "roles_for_path.0.path", "/"), resource.TestCheckResourceAttr(testResourceName, "roles_for_path.0.roles.#", "1"), - resource.TestCheckResourceAttr(testResourceName, "roles_for_path.0.roles.0", "auditor"), + resource.TestCheckResourceAttr(testResourceName, "roles_for_path.0.roles.0", "network_engineer"), resource.TestCheckResourceAttr(testResourceName, "roles_for_path.1.path", "/orgs/default"), resource.TestCheckResourceAttr(testResourceName, "roles_for_path.1.roles.#", "1"), resource.TestCheckResourceAttr(testResourceName, "roles_for_path.1.roles.0", "org_admin"), @@ -60,7 +60,7 @@ func TestAccResourceNsxtPolicyRoleBinding_basic(t *testing.T) { ), }, { - Config: testAccNsxtPolicyRoleBindingUpdate(getTestLdapUser(), userType, identType), + Config: testAccNsxtPolicyRoleBindingUpdate(getTestLdapUser(), userType, identType, "false", ""), Check: resource.ComposeTestCheckFunc( testAccNsxtPolicyRoleBindingExists(accTestPolicyRoleBindingUpdateAttributes["display_name"], testResourceName), resource.TestCheckResourceAttr(testResourceName, "display_name", accTestPolicyRoleBindingUpdateAttributes["display_name"]), @@ -71,7 +71,61 @@ func TestAccResourceNsxtPolicyRoleBinding_basic(t *testing.T) { resource.TestCheckResourceAttr(testResourceName, "roles_for_path.#", "1"), resource.TestCheckResourceAttr(testResourceName, "roles_for_path.0.path", "/"), resource.TestCheckResourceAttr(testResourceName, "roles_for_path.0.roles.#", "1"), - resource.TestCheckResourceAttr(testResourceName, "roles_for_path.0.roles.0", "auditor"), + resource.TestCheckResourceAttr(testResourceName, "roles_for_path.0.roles.0", "security_engineer"), + + resource.TestCheckResourceAttrSet(testResourceName, "revision"), + ), + }, + }, + }) +} + +func TestAccResourceNsxtPolicyRoleBinding_local_user(t *testing.T) { + testResourceName := "nsxt_policy_user_management_role_binding.test" + userType := nsxModel.RoleBinding_TYPE_LOCAL_USER + testUsername := "terraform-" + getAccTestRandomString(10) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { + testAccPreCheck(t) + testAccOnlyLocalManager(t) + }, + Providers: testAccProviders, + CheckDestroy: func(state *terraform.State) error { + return testAccNsxtPolicyRoleBindingCheckDestroy(state, accTestPolicyRoleUpdateAttributes["display_name"]) + }, + Steps: []resource.TestStep{ + { + Config: testAccNsxtPolicyRoleBindingLocalOverwrite(testUsername), + Check: resource.ComposeTestCheckFunc( + testAccNsxtPolicyRoleBindingExists(accTestPolicyRoleBindingCreateAttributes["display_name"], testResourceName), + resource.TestCheckResourceAttr(testResourceName, "display_name", accTestPolicyRoleBindingCreateAttributes["display_name"]), + resource.TestCheckResourceAttr(testResourceName, "description", accTestPolicyRoleBindingCreateAttributes["description"]), + resource.TestCheckResourceAttr(testResourceName, "name", testUsername), + resource.TestCheckResourceAttr(testResourceName, "type", userType), + resource.TestCheckResourceAttr(testResourceName, "roles_for_path.#", "2"), + resource.TestCheckResourceAttr(testResourceName, "roles_for_path.0.path", "/"), + resource.TestCheckResourceAttr(testResourceName, "roles_for_path.0.roles.#", "1"), + resource.TestCheckResourceAttr(testResourceName, "roles_for_path.0.roles.0", "network_engineer"), + resource.TestCheckResourceAttr(testResourceName, "roles_for_path.1.path", "/orgs/default"), + resource.TestCheckResourceAttr(testResourceName, "roles_for_path.1.roles.#", "1"), + resource.TestCheckResourceAttr(testResourceName, "roles_for_path.1.roles.0", "org_admin"), + + resource.TestCheckResourceAttrSet(testResourceName, "revision"), + ), + }, + { + Config: testAccNsxtPolicyRoleBindingLocalUpdate(testUsername), + Check: resource.ComposeTestCheckFunc( + testAccNsxtPolicyRoleBindingExists(accTestPolicyRoleBindingUpdateAttributes["display_name"], testResourceName), + resource.TestCheckResourceAttr(testResourceName, "display_name", accTestPolicyRoleBindingUpdateAttributes["display_name"]), + resource.TestCheckResourceAttr(testResourceName, "description", accTestPolicyRoleBindingUpdateAttributes["description"]), + resource.TestCheckResourceAttr(testResourceName, "name", testUsername), + resource.TestCheckResourceAttr(testResourceName, "type", userType), + resource.TestCheckResourceAttr(testResourceName, "roles_for_path.#", "1"), + resource.TestCheckResourceAttr(testResourceName, "roles_for_path.0.path", "/"), + resource.TestCheckResourceAttr(testResourceName, "roles_for_path.0.roles.#", "1"), + resource.TestCheckResourceAttr(testResourceName, "roles_for_path.0.roles.0", "security_engineer"), resource.TestCheckResourceAttrSet(testResourceName, "revision"), ), @@ -97,7 +151,7 @@ func TestAccResourceNsxtPolicyRoleBinding_import_basic(t *testing.T) { }, Steps: []resource.TestStep{ { - Config: testAccNsxtPolicyRoleBindingCreate(getTestLdapUser(), userType, identType), + Config: testAccNsxtPolicyRoleBindingCreate(getTestLdapUser(), userType, identType, "false", ""), }, { ResourceName: testResourceName, @@ -159,41 +213,80 @@ func testAccNsxtPolicyRoleBindingCheckDestroy(state *terraform.State, displayNam return nil } -func testAccNsxtPolicyRoleBindingCreate(user, userType, identType string) string { +func testAccNsxtPolicyRoleBindingLocalOverwrite(user string) string { + return fmt.Sprintf("%s\n%s", testAccNodeUserCreate(user), + testAccNsxtPolicyRoleBindingCreate( + user, + nsxModel.RoleBinding_TYPE_LOCAL_USER, + "", + "true", + "nsxt_node_user.test")) +} + +func testAccNsxtPolicyRoleBindingLocalUpdate(user string) string { + return fmt.Sprintf("%s\n%s", testAccNodeUserCreate(user), + testAccNsxtPolicyRoleBindingUpdate( + user, + nsxModel.RoleBinding_TYPE_LOCAL_USER, + "", + "true", + "nsxt_node_user.test")) +} + +func testAccNsxtPolicyRoleBindingCreate(user, userType, identType, overwrite, dependsOn string) string { attrMap := accTestPolicyRoleBindingCreateAttributes + var identLine, dependsOnLine string + if len(identType) > 0 { + identLine = fmt.Sprintf("identity_source_type = \"%s\"", identType) + } + if len(dependsOn) > 0 { + dependsOnLine = fmt.Sprintf("depends_on = [%s]", dependsOn) + } + return fmt.Sprintf(` resource "nsxt_policy_user_management_role_binding" "test" { display_name = "%s" description = "%s" name = "%s" type = "%s" - identity_source_type = "%s" + overwrite_local_user = %s + %s + %s roles_for_path { path = "/" - roles = ["auditor", "security_engineer"] + roles = ["network_engineer"] } roles_for_path { path = "/orgs/default" - roles = ["org_admin", "network_engineer"] + roles = ["org_admin"] } -}`, attrMap["display_name"], attrMap["description"], user, userType, identType) +}`, attrMap["display_name"], attrMap["description"], user, userType, overwrite, identLine, dependsOnLine) } -func testAccNsxtPolicyRoleBindingUpdate(user, userType, identType string) string { +func testAccNsxtPolicyRoleBindingUpdate(user, userType, identType, overwrite, dependsOn string) string { attrMap := accTestPolicyRoleBindingUpdateAttributes + var identLine, dependsOnLine string + if len(identType) > 0 { + identLine = fmt.Sprintf("identity_source_type = \"%s\"", identType) + } + if len(dependsOn) > 0 { + dependsOnLine = fmt.Sprintf("depends_on = [%s]", dependsOn) + } return fmt.Sprintf(` resource "nsxt_policy_user_management_role_binding" "test" { display_name = "%s" description = "%s" name = "%s" type = "%s" - identity_source_type = "%s" + overwrite_local_user = %s + %s + %s roles_for_path { path = "/" - roles = ["auditor"] + roles = ["security_engineer"] } -}`, attrMap["display_name"], attrMap["description"], user, userType, identType) +}`, attrMap["display_name"], attrMap["description"], user, userType, overwrite, identLine, dependsOnLine) } diff --git a/website/docs/r/policy_user_management_role_binding.html.markdown b/website/docs/r/policy_user_management_role_binding.html.markdown index 11a5f84a5..fa96e375b 100644 --- a/website/docs/r/policy_user_management_role_binding.html.markdown +++ b/website/docs/r/policy_user_management_role_binding.html.markdown @@ -47,7 +47,7 @@ The following arguments are supported: * `roles_for_path` - (Required) A list of The roles that are associated with the user, limiting them to a path. In case the path is '/', the roles apply everywhere. * `path` - (Required) Path of the entity in parent hierarchy. * `roles` - (Required) A list of identifiers for the roles to associate with the given user limited to a path. -* `overwrite_local_user` - (Optional) Flag to allow overwriting existing role bindings for local user with terraform definition. On deletion, the user's role will be reverted to auditor only. Any exising configuration will be lost. +* `overwrite_local_user` - (Optional) Flag to allow overwriting existing role bindings for local user with terraform definition. On deletion, the user's role will be reverted to auditor only. Any existing configuration will be lost. ## Attributes Reference