From 2fa173f44483b1bfc2ed63b805550ff87f194d05 Mon Sep 17 00:00:00 2001 From: Dennis Kniep Date: Fri, 27 Dec 2024 15:30:24 +0100 Subject: [PATCH] feat:(roles): Importable role by rolename Signed-off-by: Dennis Kniep --- docs/resources/role.md | 1 + provider/resource_keycloak_role.go | 94 +++++- provider/resource_keycloak_role_test.go | 404 +++++++++++++++++++++++- 3 files changed, 475 insertions(+), 24 deletions(-) diff --git a/docs/resources/role.md b/docs/resources/role.md index e64e27912..8553c43aa 100644 --- a/docs/resources/role.md +++ b/docs/resources/role.md @@ -152,6 +152,7 @@ resource "keycloak_role" "admin_role" { - `description` - (Optional) The description of the role - `composite_roles` - (Optional) When specified, this role will be a composite role, composed of all roles that have an ID present within this list. - `attributes` - (Optional) A map representing attributes for the role. In order to add multivalue attributes, use `##` to seperate the values. Max length for each value is 255 chars +- `import` - (Optional) When `true`, the role with the specified `name` is assumed to already exist, and it will be imported into state instead of being created. This attribute is useful when dealing with roles that Keycloak creates automatically during realm creation, such as the client roles `create-client`, `view-realm`, ... for the client `realm-management` created per realm. Note, that the role will not be removed during destruction if `import` is `true`. ## Import diff --git a/provider/resource_keycloak_role.go b/provider/resource_keycloak_role.go index f87a89b47..d9dcf07a9 100644 --- a/provider/resource_keycloak_role.go +++ b/provider/resource_keycloak_role.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" + "github.com/imdario/mergo" "strings" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" @@ -39,18 +40,26 @@ func resourceKeycloakRole() *schema.Resource { "description": { Type: schema.TypeString, Optional: true, + Computed: true, }, "composite_roles": { Type: schema.TypeSet, Elem: &schema.Schema{Type: schema.TypeString}, - MinItems: 1, Set: schema.HashString, Optional: true, + Computed: true, }, // misc attributes "attributes": { Type: schema.TypeMap, Optional: true, + Computed: true, + }, + "import": { + Type: schema.TypeBool, + Optional: true, + Default: false, + ForceNew: true, }, }, } @@ -95,34 +104,66 @@ func resourceKeycloakRoleCreate(ctx context.Context, data *schema.ResourceData, role := mapFromDataToRole(data) - var compositeRoles []*keycloak.Role - if v, ok := data.GetOk("composite_roles"); ok { - compositeRolesTf := v.(*schema.Set).List() + if data.Get("import").(bool) { + realmId := data.Get("realm_id").(string) + name := data.Get("name").(string) + clientId := data.Get("client_id").(string) + compositeRolesValue, compositeRolesSpecified := data.GetOk("composite_roles") - for _, compositeRoleId := range compositeRolesTf { - compositeRoleToAdd, err := keycloakClient.GetRole(ctx, role.RealmId, compositeRoleId.(string)) - if err != nil { + compositeRoles, err := mapCompositeRoleIdsToRoleObjects(ctx, keycloakClient, compositeRolesValue.(*schema.Set).List(), role.RealmId) + if err != nil { + return diag.FromErr(err) + } + + if len(compositeRoles) != 0 { + role.Composite = true + } + + existingRole, err := keycloakClient.GetRoleByName(ctx, realmId, clientId, name) + if err != nil { + return diag.FromErr(err) + } + + if err = mergo.Merge(role, existingRole); err != nil { + return diag.FromErr(err) + } + if err = keycloakClient.UpdateRole(ctx, role); err != nil { + return diag.FromErr(err) + } + + existingCompositeRoles, err := keycloakClient.GetRoleComposites(ctx, role) + if err != nil { + return diag.FromErr(err) + } + + if role.Composite && compositeRolesSpecified { + if err = keycloakClient.RemoveCompositesFromRole(ctx, role, existingCompositeRoles); err != nil { return diag.FromErr(err) } + if err = keycloakClient.AddCompositesToRole(ctx, role, compositeRoles); err != nil { + return diag.FromErr(err) + } + } - compositeRoles = append(compositeRoles, compositeRoleToAdd) + } else { + compositeRoles, err := mapCompositeRoleIdsToRoleObjects(ctx, keycloakClient, data.Get("composite_roles").(*schema.Set).List(), role.RealmId) + if err != nil { + return diag.FromErr(err) } if len(compositeRoles) != 0 { // technically you can still specify composite_roles = [] in HCL role.Composite = true } - } - err := keycloakClient.CreateRole(ctx, role) - if err != nil { - return diag.FromErr(err) - } - - if role.Composite { - err = keycloakClient.AddCompositesToRole(ctx, role, compositeRoles) - if err != nil { + if err = keycloakClient.CreateRole(ctx, role); err != nil { return diag.FromErr(err) } + + if role.Composite { + if err = keycloakClient.AddCompositesToRole(ctx, role, compositeRoles); err != nil { + return diag.FromErr(err) + } + } } mapFromRoleToData(data, role) @@ -130,6 +171,20 @@ func resourceKeycloakRoleCreate(ctx context.Context, data *schema.ResourceData, return resourceKeycloakRoleRead(ctx, data, meta) } +func mapCompositeRoleIdsToRoleObjects(ctx context.Context, keycloakClient *keycloak.KeycloakClient, compositeRoleIds []interface{}, realmId string) ([]*keycloak.Role, error) { + var compositeRoles []*keycloak.Role + + for _, compositeRoleId := range compositeRoleIds { + compositeRoleToAdd, err := keycloakClient.GetRole(ctx, realmId, compositeRoleId.(string)) + if err != nil { + return nil, err + } + + compositeRoles = append(compositeRoles, compositeRoleToAdd) + } + return compositeRoles, nil +} + func resourceKeycloakRoleRead(ctx context.Context, data *schema.ResourceData, meta interface{}) diag.Diagnostics { keycloakClient := meta.(*keycloak.KeycloakClient) @@ -232,6 +287,10 @@ func resourceKeycloakRoleUpdate(ctx context.Context, data *schema.ResourceData, } func resourceKeycloakRoleDelete(ctx context.Context, data *schema.ResourceData, meta interface{}) diag.Diagnostics { + if data.Get("import").(bool) { + return nil + } + keycloakClient := meta.(*keycloak.KeycloakClient) realmId := data.Get("realm_id").(string) @@ -254,6 +313,7 @@ func resourceKeycloakRoleImport(ctx context.Context, d *schema.ResourceData, met } d.Set("realm_id", parts[0]) + d.Set("import", false) d.SetId(parts[1]) diagnostics := resourceKeycloakRoleRead(ctx, d, meta) diff --git a/provider/resource_keycloak_role_test.go b/provider/resource_keycloak_role_test.go index 8900d9c75..ee0363983 100644 --- a/provider/resource_keycloak_role_test.go +++ b/provider/resource_keycloak_role_test.go @@ -6,6 +6,7 @@ import ( "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/keycloak/terraform-provider-keycloak/keycloak" + "regexp" "strings" "testing" ) @@ -117,15 +118,26 @@ func TestAccKeycloakRole_basicRealmUpdate(t *testing.T) { Steps: []resource.TestStep{ { Config: testKeycloakRole_basicRealmWithDescription(roleName, descriptionOne), - Check: testAccCheckKeycloakRoleExists("keycloak_role.role"), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeycloakRoleExists("keycloak_role.role"), + resource.TestCheckResourceAttr("keycloak_role.role", "description", descriptionOne), + testAccCheckKeycloakRoleHasComposites("keycloak_role.role", []string{}), + ), }, { Config: testKeycloakRole_basicRealmWithDescription(roleName, descriptionTwo), - Check: testAccCheckKeycloakRoleExists("keycloak_role.role"), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeycloakRoleExists("keycloak_role.role"), + resource.TestCheckResourceAttr("keycloak_role.role", "description", descriptionTwo), + testAccCheckKeycloakRoleHasComposites("keycloak_role.role", []string{}), + ), }, { Config: testKeycloakRole_basicRealm(roleName), - Check: testAccCheckKeycloakRoleExists("keycloak_role.role"), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeycloakRoleExists("keycloak_role.role"), + testAccCheckKeycloakRoleHasComposites("keycloak_role.role", []string{}), + ), }, }, }) @@ -292,6 +304,157 @@ func TestAccKeycloakRole_basicWithAttributes(t *testing.T) { }) } +func TestAccKeycloakRole_importWithAttributes(t *testing.T) { + t.Parallel() + roleName := acctest.RandomWithPrefix("tf-acc") + attributeName := acctest.RandomWithPrefix("tf-acc") + attributeValue := acctest.RandomWithPrefix("tf-acc") + + resource.Test(t, resource.TestCase{ + ProviderFactories: testAccProviderFactories, + PreCheck: func() { testAccPreCheck(t) }, + CheckDestroy: testAccCheckKeycloakRoleDestroy(), + Steps: []resource.TestStep{ + { + Config: testKeycloakRole_basicWithAttributes(roleName, attributeName, attributeValue), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeycloakRoleExists("keycloak_role.role"), + testAccCheckKeycloakRoleHasAttribute("keycloak_role.role", attributeName, attributeValue), + ), + }, + { + Config: testKeycloakRole_importRealmRoleWithAttributes(roleName, attributeName, attributeValue), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeycloakRoleExists("keycloak_role.imported-realm-role"), + testAccCheckKeycloakRoleHasAttribute("keycloak_role.imported-realm-role", attributeName, attributeValue), + ), + }, + { + Config: testKeycloakRole_importRealmRoleWithoutAttributes(roleName, attributeName, attributeValue), + Check: resource.ComposeTestCheckFunc( + testAccCheckKeycloakRoleExists("keycloak_role.imported-realm-role"), + testAccCheckKeycloakRoleHasNoAttributes("keycloak_role.imported-realm-role"), + ), + }, + }, + }) +} + +func TestAccKeycloakRole_import(t *testing.T) { + t.Parallel() + + resource.Test(t, resource.TestCase{ + ProviderFactories: testAccProviderFactories, + PreCheck: func() { testAccPreCheck(t) }, + CheckDestroy: testAccCheckKeycloakRoleNotDestroyed(), + Steps: []resource.TestStep{ + { + Config: testKeycloakRole_importRealmRole("non-existing-role"), + ExpectError: regexp.MustCompile("Could not find role"), + }, + { + Config: testKeycloakRole_importClientRole("view-profile", "non-existing-client"), + ExpectError: regexp.MustCompile("openid client with name non-existing-client does not exist"), + }, + { + Config: testKeycloakRole_importClientRole("non-existing-role", "account"), + ExpectError: regexp.MustCompile("Could not find role"), + }, + { + Config: testKeycloakRole_importRealmRole("offline_access"), + Check: testAccCheckKeycloakRoleHasDescription("keycloak_role.imported-realm-role", "${role_offline-access}"), + }, + { + Config: testKeycloakRole_importClientRole("view-profile", "account"), + Check: testAccCheckKeycloakRoleHasDescription("keycloak_role.imported-client-role", "${role_view-profile}"), + }, + { + Config: testKeycloakRole_importAndModifyRealmRole("offline_access", "$${role_offline-access}"), // $ needs to be escaped + Check: testAccCheckKeycloakRoleHasDescription("keycloak_role.imported-realm-role", "${role_offline-access}"), + }, + { + Config: testKeycloakRole_importAndModifyRealmRole("offline_access", "updated-descr"), + Check: testAccCheckKeycloakRoleHasDescription("keycloak_role.imported-realm-role", "updated-descr"), + }, + { + Config: testKeycloakRole_importAndModifyClientRole("view-profile", "account", "updated-descr"), + Check: testAccCheckKeycloakRoleHasDescription("keycloak_role.imported-client-role", "updated-descr"), + }, + { + Config: testKeycloakRole_importCompositeRole("default-roles-" + testAccRealm.Realm), + Check: testAccCheckKeycloakRoleHasComposites("keycloak_role.imported-composite-role", []string{"uma_authorization", "offline_access", "view-profile", "manage-account"}), + }, + { + Config: testKeycloakRole_importAndModifyCompositeRole("default-roles-"+testAccRealm.Realm, []NestedRole{{Name: "offline_access"}, {Name: "uma_authorization"}}), + Check: testAccCheckKeycloakRoleHasComposites("keycloak_role.imported-composite-role", []string{"offline_access", "uma_authorization"}), + }, + { + Config: testKeycloakRole_importCompositeRole("default-roles-" + testAccRealm.Realm), + Check: testAccCheckKeycloakRoleHasComposites("keycloak_role.imported-composite-role", []string{"offline_access", "uma_authorization"}), + }, + { + Config: testKeycloakRole_importAndModifyCompositeRole("default-roles-"+testAccRealm.Realm, []NestedRole{{Name: "uma_authorization"}}), + Check: testAccCheckKeycloakRoleHasComposites("keycloak_role.imported-composite-role", []string{"uma_authorization"}), + }, + { + Config: testKeycloakRole_importCompositeRole("default-roles-" + testAccRealm.Realm), + Check: testAccCheckKeycloakRoleHasComposites("keycloak_role.imported-composite-role", []string{"uma_authorization"}), + }, + { + Config: testKeycloakRole_importAndModifyCompositeRole("default-roles-"+testAccRealm.Realm, []NestedRole{{Name: "uma_authorization"}, {Name: "manage-account", ClientId: stringPointer("account")}}), + Check: testAccCheckKeycloakRoleHasComposites("keycloak_role.imported-composite-role", []string{"uma_authorization", "manage-account"}), + }, + { + Config: testKeycloakRole_importCompositeRole("default-roles-" + testAccRealm.Realm), + Check: testAccCheckKeycloakRoleHasComposites("keycloak_role.imported-composite-role", []string{"uma_authorization", "manage-account"}), + }, + { + Config: testKeycloakRole_importNoCompositeRole("default-roles-" + testAccRealm.Realm), + Check: testAccCheckKeycloakRoleHasComposites("keycloak_role.imported-composite-role", []string{}), + }, + { + Config: testKeycloakRole_importCompositeRole("default-roles-" + testAccRealm.Realm), + Check: testAccCheckKeycloakRoleHasComposites("keycloak_role.imported-composite-role", []string{}), + }, + }, + }) +} + +func testAccCheckKeycloakRoleNotDestroyed() resource.TestCheckFunc { + return func(s *terraform.State) error { + for _, rs := range s.RootModule().Resources { + if rs.Type != "keycloak_role" { + continue + } + + id := rs.Primary.ID + realm := rs.Primary.Attributes["realm_id"] + + role, _ := keycloakClient.GetRole(testCtx, realm, id) + if role == nil { + return fmt.Errorf("role %s does not exists", id) + } + } + + return nil + } +} + +func testAccCheckKeycloakRoleHasDescription(resourceName, expectedDescription string) resource.TestCheckFunc { + return func(state *terraform.State) error { + role, err := getRoleFromState(state, resourceName) + if err != nil { + return err + } + + if role.Description != expectedDescription { + return fmt.Errorf("expected role's description to be %s, but was %s", expectedDescription, role.Description) + } + + return nil + } +} + func testAccCheckKeycloakRoleExists(resourceName string) resource.TestCheckFunc { return func(s *terraform.State) error { _, err := getRoleFromState(s, resourceName) @@ -354,6 +517,21 @@ func testAccCheckKeycloakRoleHasAttribute(resourceName, attributeName, attribute } } +func testAccCheckKeycloakRoleHasNoAttributes(resourceName string) resource.TestCheckFunc { + return func(state *terraform.State) error { + role, err := getRoleFromState(state, resourceName) + if err != nil { + return err + } + + if len(role.Attributes) != 0 { + return fmt.Errorf("expected role %s to have no attributes", role.Name) + } + + return nil + } +} + func testAccCheckKeycloakRoleHasComposites(resourceName string, compositeRoleNames []string) resource.TestCheckFunc { return func(state *terraform.State) error { role, err := getRoleFromState(state, resourceName) @@ -511,10 +689,7 @@ resource "keycloak_role" "role" { } func testKeycloakRole_composites(clientOne, clientTwo, roleOne, roleTwo, roleThree, roleFour, roleWithComposites string, composites []string) string { - var tfComposites string - if len(composites) != 0 { - tfComposites = fmt.Sprintf("composite_roles = %s", arrayOfStringsForTerraformResource(composites)) - } + tfComposites := fmt.Sprintf("composite_roles = %s", arrayOfStringsForTerraformResource(composites)) return fmt.Sprintf(` data "keycloak_realm" "realm" { @@ -579,3 +754,218 @@ resource "keycloak_role" "role" { } `, testAccRealm.Realm, role, attributeName, attributeValue) } + +func testKeycloakRole_importRealmRoleWithAttributes(role, attributeName, attributeValue string) string { + return fmt.Sprintf(` +data "keycloak_realm" "realm" { + realm = "%s" +} + +resource "keycloak_role" "role" { + name = "%s" + realm_id = data.keycloak_realm.realm.id + attributes = { + "%s" = "%s" + } +} + +resource "keycloak_role" "imported-realm-role" { + name = "%s" + realm_id = data.keycloak_realm.realm.id + import = true + depends_on = [ + keycloak_role.role, + ] +} + `, testAccRealm.Realm, role, attributeName, attributeValue, role) +} + +func testKeycloakRole_importRealmRoleWithoutAttributes(role, attributeName, attributeValue string) string { + return fmt.Sprintf(` +data "keycloak_realm" "realm" { + realm = "%s" +} + +resource "keycloak_role" "role" { + name = "%s" + realm_id = data.keycloak_realm.realm.id + attributes = { + "%s" = "%s" + } + lifecycle { + ignore_changes = [ + attributes, + ] + } +} + +resource "keycloak_role" "imported-realm-role" { + name = "%s" + realm_id = data.keycloak_realm.realm.id + import = true + attributes = {} + depends_on = [ + keycloak_role.role, + ] +} + `, testAccRealm.Realm, role, attributeName, attributeValue, role) +} + +func testKeycloakRole_importRealmRole(name string) string { + return fmt.Sprintf(` +data "keycloak_realm" "realm" { + realm = "%s" +} +resource "keycloak_role" "imported-realm-role" { + name = "%s" + realm_id = data.keycloak_realm.realm.id + import = true +} + `, testAccRealm.Realm, name) +} + +func testKeycloakRole_importAndModifyRealmRole(name, newDescription string) string { + return fmt.Sprintf(` +data "keycloak_realm" "realm" { + realm = "%s" +} +resource "keycloak_role" "imported-realm-role" { + name = "%s" + realm_id = data.keycloak_realm.realm.id + import = true + description = "%s" +} + `, testAccRealm.Realm, name, newDescription) +} + +func testKeycloakRole_importClientRole(name, clientId string) string { + return fmt.Sprintf(` +data "keycloak_realm" "realm" { + realm = "%s" +} +resource "keycloak_openid_client" "imported-client" { + realm_id = data.keycloak_realm.realm.id + client_id = "%s" + import = true + access_type = "PUBLIC" +} +resource "keycloak_role" "imported-client-role" { + realm_id = data.keycloak_realm.realm.id + client_id = keycloak_openid_client.imported-client.id + name = "%s" + import = true +} + `, testAccRealm.Realm, clientId, name) +} + +func testKeycloakRole_importAndModifyClientRole(name, clientId, newDescription string) string { + return fmt.Sprintf(` +data "keycloak_realm" "realm" { + realm = "%s" +} +resource "keycloak_openid_client" "imported-client" { + realm_id = data.keycloak_realm.realm.id + client_id = "%s" + import = true + access_type = "PUBLIC" +} +resource "keycloak_role" "imported-client-role" { + realm_id = data.keycloak_realm.realm.id + client_id = keycloak_openid_client.imported-client.id + name = "%s" + import = true + description = "%s" +} + `, testAccRealm.Realm, clientId, name, newDescription) +} + +type NestedRole struct { + Name string + ClientId *string +} + +func testKeycloakRole_importAndModifyCompositeRole(name string, nestedRoles []NestedRole) string { + importedRoles := "" + importedRoleRefs := "" + + for i, nestedRole := range nestedRoles { + importedRoleRef := fmt.Sprintf("imported-noncomposite-role-%d", i) + if nestedRole.ClientId == nil { + importedRoles += fmt.Sprintf(` +resource "keycloak_role" "%s" { + name = "%s" + realm_id = data.keycloak_realm.realm.id + import = true +} +`, importedRoleRef, nestedRole.Name) + } else { + + importedClientIdRef := fmt.Sprintf("imported-client-%d", i) + + importedRoles += fmt.Sprintf(` +resource "keycloak_openid_client" "%s" { + realm_id = data.keycloak_realm.realm.id + client_id = "%s" + import = true + access_type = "PUBLIC" +} + +resource "keycloak_role" "%s" { + realm_id = data.keycloak_realm.realm.id + client_id = keycloak_openid_client.%s.id + name = "%s" + import = true +} +`, importedClientIdRef, *nestedRole.ClientId, importedRoleRef, importedClientIdRef, nestedRole.Name) + } + + if i != 0 { + importedRoleRefs += ", " + } + importedRoleRefs += fmt.Sprintf("keycloak_role.%s.id", importedRoleRef) + } + + return fmt.Sprintf(` +data "keycloak_realm" "realm" { + realm = "%s" +} + +%s + +resource "keycloak_role" "imported-composite-role" { + name = "%s" + realm_id = data.keycloak_realm.realm.id + import = true + composite_roles = [%s] +} + `, testAccRealm.Realm, importedRoles, name, importedRoleRefs) +} + +func testKeycloakRole_importCompositeRole(name string) string { + return fmt.Sprintf(` +data "keycloak_realm" "realm" { + realm = "%s" +} + +resource "keycloak_role" "imported-composite-role" { + name = "%s" + realm_id = data.keycloak_realm.realm.id + import = true +} + `, testAccRealm.Realm, name) +} + +func testKeycloakRole_importNoCompositeRole(name string) string { + return fmt.Sprintf(` +data "keycloak_realm" "realm" { + realm = "%s" +} + +resource "keycloak_role" "imported-composite-role" { + name = "%s" + realm_id = data.keycloak_realm.realm.id + import = true + composite_roles = [] +} + `, testAccRealm.Realm, name) +}