From 0747065ba19fb48cf2aef2ef5954bed551f26207 Mon Sep 17 00:00:00 2001 From: horus Date: Wed, 9 Nov 2022 16:23:07 +0800 Subject: [PATCH 1/6] feat: support role IDs in default role resource --- keycloak/default_roles.go | 9 +- provider/resource_keycloak_default_roles.go | 92 +++++++++++++++---- .../resource_keycloak_default_roles_test.go | 7 +- 3 files changed, 84 insertions(+), 24 deletions(-) diff --git a/keycloak/default_roles.go b/keycloak/default_roles.go index f07843d47..4b3c94930 100644 --- a/keycloak/default_roles.go +++ b/keycloak/default_roles.go @@ -6,14 +6,15 @@ import ( ) type DefaultRoles struct { - Id string `json:"id,omitempty"` - RealmId string `json:"-"` - DefaultRoles []string `json:"-"` + Id string `json:"id,omitempty"` + RealmId string `json:"-"` + DefaultRoles []string `json:"-"` + DefaultRoleIds []string `json:"-"` } func (keycloakClient *KeycloakClient) GetDefaultRoles(ctx context.Context, realmId, id string) ([]*Role, error) { var composites []*Role - err := keycloakClient.get(ctx, fmt.Sprintf("/realms/%s/roles-by-id/%s/composites/realm", realmId, id), &composites, nil) + err := keycloakClient.get(ctx, fmt.Sprintf("/realms/%s/roles-by-id/%s/composites", realmId, id), &composites, nil) if err != nil { return nil, err } diff --git a/provider/resource_keycloak_default_roles.go b/provider/resource_keycloak_default_roles.go index 30f8e0fa6..d5fd30e9d 100644 --- a/provider/resource_keycloak_default_roles.go +++ b/provider/resource_keycloak_default_roles.go @@ -4,9 +4,9 @@ import ( "context" "errors" "fmt" - "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "strings" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/mrparkers/terraform-provider-keycloak/keycloak" ) @@ -29,9 +29,15 @@ func resourceKeycloakDefaultRoles() *schema.Resource { "default_roles": { Type: schema.TypeSet, Elem: &schema.Schema{Type: schema.TypeString}, - Description: "Realm level roles assigned to new users.", + Description: "Realm level roles (name) assigned to new users.", Required: true, }, + "default_role_ids": { + Type: schema.TypeSet, + Elem: &schema.Schema{Type: schema.TypeString}, + Description: "Realm level roles (ID) assigned to new users.", + Optional: true, + }, }, } } @@ -44,10 +50,18 @@ func mapFromDataToDefaultRoles(data *schema.ResourceData) *keycloak.DefaultRoles } } + var defaultRoleIds []string + if v, ok := data.GetOk("default_role_ids"); ok { + for _, defaultRole := range v.(*schema.Set).List() { + defaultRoleIds = append(defaultRoleIds, defaultRole.(string)) + } + } + defaultRoles := &keycloak.DefaultRoles{ - Id: data.Id(), - RealmId: data.Get("realm_id").(string), - DefaultRoles: defaultRolesList, + Id: data.Id(), + RealmId: data.Get("realm_id").(string), + DefaultRoles: defaultRolesList, + DefaultRoleIds: defaultRoleIds, } return defaultRoles @@ -58,6 +72,7 @@ func mapFromDefaultRolesToData(data *schema.ResourceData, defaultRoles *keycloak data.Set("realm_id", defaultRoles.RealmId) data.Set("default_roles", defaultRoles.DefaultRoles) + data.Set("default_role_ids", defaultRoles.DefaultRoleIds) } func resourceKeycloakDefaultRolesRead(ctx context.Context, data *schema.ResourceData, meta interface{}) diag.Diagnostics { @@ -71,12 +86,13 @@ func resourceKeycloakDefaultRolesRead(ctx context.Context, data *schema.Resource return handleNotFoundError(ctx, err, data) } - defaultRoleNamesList := getDefaultRoleNames(composites) + defaultRoleNamesList, defaultRoleIds := getDefaultRoles(composites) defaultRoles := &keycloak.DefaultRoles{ - Id: id, - RealmId: realmId, - DefaultRoles: defaultRoleNamesList, + Id: id, + RealmId: realmId, + DefaultRoles: defaultRoleNamesList, + DefaultRoleIds: defaultRoleIds, } mapFromDefaultRolesToData(data, defaultRoles) @@ -110,14 +126,22 @@ func resourceKeycloakDefaultRolesReconcile(ctx context.Context, data *schema.Res return diag.FromErr(err) } - defaultRoleNamesList := getDefaultRoleNames(composites) + defaultRoleNamesList, defaultRoleIds := getDefaultRoles(composites) rolesList, err := keycloakClient.GetRealmRoles(ctx, defaultRoles.RealmId) if err != nil { return diag.FromErr(err) } + for _, roleId := range append(defaultRoleIds, defaultRoles.DefaultRoleIds...) { + role, err := keycloakClient.GetRole(ctx, defaultRoles.RealmId, roleId) + if err != nil { + return diag.FromErr(err) + } + rolesList = append(rolesList, role) + } + // skip if actual default roles in keycloak same as we want - if roleListsEqual(defaultRoleNamesList, defaultRoles.DefaultRoles) { + if roleListsEqual(defaultRoleNamesList, defaultRoles.DefaultRoles) && roleListsEqual(defaultRoleIds, defaultRoles.DefaultRoleIds) { return nil } @@ -131,6 +155,15 @@ func resourceKeycloakDefaultRolesReconcile(ctx context.Context, data *schema.Res putList = append(putList, defaultRoles) } } + for _, roleId := range defaultRoles.DefaultRoleIds { + if !roleListContains(defaultRoleIds, roleId) { + defaultRoles, err := getRoleByIdFromList(rolesList, roleId) + if err != nil { + return diag.FromErr(err) + } + putList = append(putList, defaultRoles) + } + } for _, roleName := range defaultRoleNamesList { if !roleListContains(defaultRoles.DefaultRoles, roleName) { defaultRoles, err := getRoleByNameFromList(rolesList, roleName) @@ -140,6 +173,15 @@ func resourceKeycloakDefaultRolesReconcile(ctx context.Context, data *schema.Res deleteList = append(deleteList, defaultRoles) } } + for _, roleId := range defaultRoleIds { + if !roleListContains(defaultRoles.DefaultRoleIds, roleId) { + defaultRoles, err := getRoleByIdFromList(rolesList, roleId) + if err != nil { + return diag.FromErr(err) + } + deleteList = append(deleteList, defaultRoles) + } + } // apply if not empty if len(putList) > 0 { @@ -200,7 +242,7 @@ func resourceKeycloakDefaultRolesImport(ctx context.Context, d *schema.ResourceD parts := strings.Split(d.Id(), "/") if len(parts) != 2 { - return nil, fmt.Errorf("Invalid import. Supported import format: {{realm}}/{{defaultRoleId}}.") + return nil, fmt.Errorf("invalid import, supported import format: {{realm}}/{{defaultRoleId}}") } _, err := keycloakClient.GetDefaultRoles(ctx, parts[0], parts[1]) @@ -219,21 +261,37 @@ func resourceKeycloakDefaultRolesImport(ctx context.Context, d *schema.ResourceD return []*schema.ResourceData{d}, nil } -func getDefaultRoleNames(roles []*keycloak.Role) []string { - var defaultRolesNames []string +// getDefaultRoles sorts out default realm role names and client role IDs +func getDefaultRoles(roles []*keycloak.Role) ([]string, []string) { + var ( + defaultRolesNames []string + defaultRolesIds []string + ) for _, defaultRoles := range roles { + if defaultRoles.ClientRole { + defaultRolesIds = append(defaultRolesIds, defaultRoles.Id) + continue + } defaultRolesNames = append(defaultRolesNames, defaultRoles.Name) } - return defaultRolesNames + return defaultRolesNames, defaultRolesIds } func getRoleByNameFromList(defaultRoles []*keycloak.Role, name string) (*keycloak.Role, error) { + return getRoleBy(defaultRoles, func(r *keycloak.Role) bool { return r.Name == name }, " by name: "+name) +} + +func getRoleByIdFromList(defaultRoles []*keycloak.Role, id string) (*keycloak.Role, error) { + return getRoleBy(defaultRoles, func(r *keycloak.Role) bool { return r.Id == id }, " by id: "+id) +} + +func getRoleBy(defaultRoles []*keycloak.Role, grep func(*keycloak.Role) bool, msg string) (*keycloak.Role, error) { for _, element := range defaultRoles { - if element.Name == name { + if grep(element) { return element, nil } } - return nil, fmt.Errorf("defaultRoles not found by name") + return nil, fmt.Errorf("defaultRoles not found%s", msg) } func roleListContains(s []string, e string) bool { diff --git a/provider/resource_keycloak_default_roles_test.go b/provider/resource_keycloak_default_roles_test.go index 56ad76aef..f303e8bc9 100644 --- a/provider/resource_keycloak_default_roles_test.go +++ b/provider/resource_keycloak_default_roles_test.go @@ -2,11 +2,12 @@ package provider import ( "fmt" + "testing" + "github.com/hashicorp/terraform-plugin-sdk/v2/helper/acctest" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/terraform" "github.com/mrparkers/terraform-provider-keycloak/keycloak" - "testing" ) func TestAccKeycloakDefaultRoles_basic(t *testing.T) { @@ -84,7 +85,7 @@ func testAccCheckKeycloakDefaultRolesDestroy(realmId string) resource.TestCheckF return fmt.Errorf("error getting defaultRoles with id %s: %s", realm.DefaultRole.Id, err) } - defaultRoles := getDefaultRoleNames(composites) + defaultRoles, _ := getDefaultRoles(composites) if err != nil { return err } @@ -110,7 +111,7 @@ func getKeycloakDefaultRolesFromState(s *terraform.State, resourceName string) ( return nil, fmt.Errorf("error getting defaultRoles with id %s: %s", id, err) } - defaultRoleNamesList := getDefaultRoleNames(composites) + defaultRoleNamesList, _ := getDefaultRoles(composites) defaultRoles := &keycloak.DefaultRoles{ Id: id, From 608052920f7036e6c86be4b4750abcd6a63c5189 Mon Sep 17 00:00:00 2001 From: horus Date: Mon, 27 Nov 2023 19:18:59 +0800 Subject: [PATCH 2/6] use ${client_id}/${role_name} instead of UUIDs --- keycloak/default_roles.go | 48 +++++- provider/resource_keycloak_default_roles.go | 148 +++++------------- .../resource_keycloak_default_roles_test.go | 7 +- 3 files changed, 91 insertions(+), 112 deletions(-) diff --git a/keycloak/default_roles.go b/keycloak/default_roles.go index 4b3c94930..4ee175614 100644 --- a/keycloak/default_roles.go +++ b/keycloak/default_roles.go @@ -6,10 +6,9 @@ import ( ) type DefaultRoles struct { - Id string `json:"id,omitempty"` - RealmId string `json:"-"` - DefaultRoles []string `json:"-"` - DefaultRoleIds []string `json:"-"` + Id string `json:"id,omitempty"` + RealmId string `json:"-"` + DefaultRoles []string `json:"-"` } func (keycloakClient *KeycloakClient) GetDefaultRoles(ctx context.Context, realmId, id string) ([]*Role, error) { @@ -18,6 +17,45 @@ func (keycloakClient *KeycloakClient) GetDefaultRoles(ctx context.Context, realm if err != nil { return nil, err } - + for _, composite := range composites { + if composite.ClientRole && composite.ClientId == "" { + client, err := keycloakClient.GetGenericClient(ctx, realmId, composite.ContainerId) + if err != nil { + return nil, err + } + composite.ClientId = client.ClientId + } + } return composites, nil } + +func (keycloakClient *KeycloakClient) GetQualifiedRoleName(ctx context.Context, realmId string, role *Role) (string, error) { + if !role.ClientRole { + return role.Name, nil + } + if role.ClientId != "" { + return fmt.Sprintf("%s/%s", role.ClientId, role.Name), nil + } + genericClient, err := keycloakClient.GetGenericClient(ctx, realmId, role.ContainerId) + if err != nil { + return "", err + } + role.ClientId = genericClient.ClientId + return fmt.Sprintf("%s/%s", role.ClientId, role.Name), nil +} + +func (keycloakClient *KeycloakClient) GetRoleFullNames(ctx context.Context, realmId string, roles []*Role) ([]string, error) { + fullNames := make([]string, len(roles)) + for i, role := range roles { + if !role.ClientRole { + fullNames[i] = role.Name + continue + } + fullName, err := keycloakClient.GetQualifiedRoleName(ctx, realmId, role) + if err != nil { + return []string{}, err + } + fullNames[i] = fullName + } + return fullNames, nil +} diff --git a/provider/resource_keycloak_default_roles.go b/provider/resource_keycloak_default_roles.go index d5fd30e9d..a60f0eb38 100644 --- a/provider/resource_keycloak_default_roles.go +++ b/provider/resource_keycloak_default_roles.go @@ -32,12 +32,6 @@ func resourceKeycloakDefaultRoles() *schema.Resource { Description: "Realm level roles (name) assigned to new users.", Required: true, }, - "default_role_ids": { - Type: schema.TypeSet, - Elem: &schema.Schema{Type: schema.TypeString}, - Description: "Realm level roles (ID) assigned to new users.", - Optional: true, - }, }, } } @@ -50,31 +44,15 @@ func mapFromDataToDefaultRoles(data *schema.ResourceData) *keycloak.DefaultRoles } } - var defaultRoleIds []string - if v, ok := data.GetOk("default_role_ids"); ok { - for _, defaultRole := range v.(*schema.Set).List() { - defaultRoleIds = append(defaultRoleIds, defaultRole.(string)) - } - } - defaultRoles := &keycloak.DefaultRoles{ - Id: data.Id(), - RealmId: data.Get("realm_id").(string), - DefaultRoles: defaultRolesList, - DefaultRoleIds: defaultRoleIds, + Id: data.Id(), + RealmId: data.Get("realm_id").(string), + DefaultRoles: defaultRolesList, } return defaultRoles } -func mapFromDefaultRolesToData(data *schema.ResourceData, defaultRoles *keycloak.DefaultRoles) { - data.SetId(defaultRoles.Id) - - data.Set("realm_id", defaultRoles.RealmId) - data.Set("default_roles", defaultRoles.DefaultRoles) - data.Set("default_role_ids", defaultRoles.DefaultRoleIds) -} - func resourceKeycloakDefaultRolesRead(ctx context.Context, data *schema.ResourceData, meta interface{}) diag.Diagnostics { keycloakClient := meta.(*keycloak.KeycloakClient) @@ -86,16 +64,20 @@ func resourceKeycloakDefaultRolesRead(ctx context.Context, data *schema.Resource return handleNotFoundError(ctx, err, data) } - defaultRoleNamesList, defaultRoleIds := getDefaultRoles(composites) + defaultRoleNames, err := keycloakClient.GetRoleFullNames(ctx, realmId, composites) + if err != nil { + return handleNotFoundError(ctx, err, data) + } defaultRoles := &keycloak.DefaultRoles{ - Id: id, - RealmId: realmId, - DefaultRoles: defaultRoleNamesList, - DefaultRoleIds: defaultRoleIds, + Id: id, + RealmId: realmId, + DefaultRoles: defaultRoleNames, } - mapFromDefaultRolesToData(data, defaultRoles) + data.SetId(defaultRoles.Id) + data.Set("realm_id", defaultRoles.RealmId) + data.Set("default_roles", defaultRoles.DefaultRoles) return nil } @@ -112,81 +94,69 @@ func resourceKeycloakDefaultRolesReconcile(ctx context.Context, data *schema.Res return diag.FromErr(err) } - defaultRoles := mapFromDataToDefaultRoles(data) + local := mapFromDataToDefaultRoles(data) - realm, err := keycloakClient.GetRealm(ctx, defaultRoles.RealmId) + realm, err := keycloakClient.GetRealm(ctx, local.RealmId) if err != nil { return diag.FromErr(err) } data.SetId(realm.DefaultRole.Id) - composites, err := keycloakClient.GetDefaultRoles(ctx, defaultRoles.RealmId, realm.DefaultRole.Id) + composites, err := keycloakClient.GetDefaultRoles(ctx, local.RealmId, realm.DefaultRole.Id) if err != nil { return diag.FromErr(err) } - defaultRoleNamesList, defaultRoleIds := getDefaultRoles(composites) - rolesList, err := keycloakClient.GetRealmRoles(ctx, defaultRoles.RealmId) + defaultRoleNames, err := keycloakClient.GetRoleFullNames(ctx, local.RealmId, composites) if err != nil { return diag.FromErr(err) } - for _, roleId := range append(defaultRoleIds, defaultRoles.DefaultRoleIds...) { - role, err := keycloakClient.GetRole(ctx, defaultRoles.RealmId, roleId) - if err != nil { - return diag.FromErr(err) - } - rolesList = append(rolesList, role) + defaultRolesMap := make(map[string]*keycloak.Role) + for i, roleName := range defaultRoleNames { + defaultRolesMap[roleName] = composites[i] } // skip if actual default roles in keycloak same as we want - if roleListsEqual(defaultRoleNamesList, defaultRoles.DefaultRoles) && roleListsEqual(defaultRoleIds, defaultRoles.DefaultRoleIds) { + if roleListsEqual(defaultRoleNames, local.DefaultRoles) { return nil } - var putList, deleteList []*keycloak.Role - for _, roleName := range defaultRoles.DefaultRoles { - if !roleListContains(defaultRoleNamesList, roleName) { - defaultRoles, err := getRoleByNameFromList(rolesList, roleName) - if err != nil { - return diag.FromErr(err) - } - putList = append(putList, defaultRoles) + getRole := func(roleName string) (*keycloak.Role, error) { + if !strings.Contains(roleName, "/") { + return keycloakClient.GetRoleByName(ctx, local.RealmId, "", roleName) } - } - for _, roleId := range defaultRoles.DefaultRoleIds { - if !roleListContains(defaultRoleIds, roleId) { - defaultRoles, err := getRoleByIdFromList(rolesList, roleId) - if err != nil { - return diag.FromErr(err) - } - putList = append(putList, defaultRoles) + parts := strings.Split(roleName, "/") + client, err := keycloakClient.GetGenericClientByClientId(ctx, local.RealmId, parts[0]) + if err != nil { + return nil, err } + return keycloakClient.GetRoleByName(ctx, local.RealmId, client.Id, parts[1]) } - for _, roleName := range defaultRoleNamesList { - if !roleListContains(defaultRoles.DefaultRoles, roleName) { - defaultRoles, err := getRoleByNameFromList(rolesList, roleName) + + var putList, deleteList []*keycloak.Role + for _, roleName := range local.DefaultRoles { + // keycloak doesn't have our locally defined roles + if !roleListContains(defaultRoleNames, roleName) { + defaultRoles, err := getRole(roleName) if err != nil { return diag.FromErr(err) } - deleteList = append(deleteList, defaultRoles) + putList = append(putList, defaultRoles) } } - for _, roleId := range defaultRoleIds { - if !roleListContains(defaultRoles.DefaultRoleIds, roleId) { - defaultRoles, err := getRoleByIdFromList(rolesList, roleId) - if err != nil { - return diag.FromErr(err) - } - deleteList = append(deleteList, defaultRoles) + for _, roleName := range defaultRoleNames { + // keycloak have roles we don't want + if !roleListContains(local.DefaultRoles, roleName) { + deleteList = append(deleteList, defaultRolesMap[roleName]) } } // apply if not empty if len(putList) > 0 { role := &keycloak.Role{ - RealmId: defaultRoles.RealmId, + RealmId: local.RealmId, Id: realm.DefaultRole.Id, } err := keycloakClient.AddCompositesToRole(ctx, role, putList) @@ -194,9 +164,10 @@ func resourceKeycloakDefaultRolesReconcile(ctx context.Context, data *schema.Res return diag.FromErr(err) } } + if len(deleteList) > 0 { role := &keycloak.Role{ - RealmId: defaultRoles.RealmId, + RealmId: local.RealmId, Id: realm.DefaultRole.Id, } err := keycloakClient.RemoveCompositesFromRole(ctx, role, deleteList) @@ -261,39 +232,6 @@ func resourceKeycloakDefaultRolesImport(ctx context.Context, d *schema.ResourceD return []*schema.ResourceData{d}, nil } -// getDefaultRoles sorts out default realm role names and client role IDs -func getDefaultRoles(roles []*keycloak.Role) ([]string, []string) { - var ( - defaultRolesNames []string - defaultRolesIds []string - ) - for _, defaultRoles := range roles { - if defaultRoles.ClientRole { - defaultRolesIds = append(defaultRolesIds, defaultRoles.Id) - continue - } - defaultRolesNames = append(defaultRolesNames, defaultRoles.Name) - } - return defaultRolesNames, defaultRolesIds -} - -func getRoleByNameFromList(defaultRoles []*keycloak.Role, name string) (*keycloak.Role, error) { - return getRoleBy(defaultRoles, func(r *keycloak.Role) bool { return r.Name == name }, " by name: "+name) -} - -func getRoleByIdFromList(defaultRoles []*keycloak.Role, id string) (*keycloak.Role, error) { - return getRoleBy(defaultRoles, func(r *keycloak.Role) bool { return r.Id == id }, " by id: "+id) -} - -func getRoleBy(defaultRoles []*keycloak.Role, grep func(*keycloak.Role) bool, msg string) (*keycloak.Role, error) { - for _, element := range defaultRoles { - if grep(element) { - return element, nil - } - } - return nil, fmt.Errorf("defaultRoles not found%s", msg) -} - func roleListContains(s []string, e string) bool { for _, a := range s { if a == e { diff --git a/provider/resource_keycloak_default_roles_test.go b/provider/resource_keycloak_default_roles_test.go index f303e8bc9..1210d5949 100644 --- a/provider/resource_keycloak_default_roles_test.go +++ b/provider/resource_keycloak_default_roles_test.go @@ -85,7 +85,7 @@ func testAccCheckKeycloakDefaultRolesDestroy(realmId string) resource.TestCheckF return fmt.Errorf("error getting defaultRoles with id %s: %s", realm.DefaultRole.Id, err) } - defaultRoles, _ := getDefaultRoles(composites) + defaultRoles, err := keycloakClient.GetRoleFullNames(testCtx, realmId, composites) if err != nil { return err } @@ -111,7 +111,10 @@ func getKeycloakDefaultRolesFromState(s *terraform.State, resourceName string) ( return nil, fmt.Errorf("error getting defaultRoles with id %s: %s", id, err) } - defaultRoleNamesList, _ := getDefaultRoles(composites) + defaultRoleNamesList, err := keycloakClient.GetRoleFullNames(testCtx, realm, composites) + if err != nil { + return nil, fmt.Errorf("error getting defaultRoleNamesList with id %s: %s", id, err) + } defaultRoles := &keycloak.DefaultRoles{ Id: id, From c79496d94bfc728f3fa1ef6d5584b227d3b7aed3 Mon Sep 17 00:00:00 2001 From: horus Date: Thu, 30 Nov 2023 12:19:21 +0800 Subject: [PATCH 3/6] fix: simplify check in default roles destroy test Note: the name list is empty iff len(composites) == 0 --- provider/resource_keycloak_default_roles_test.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/provider/resource_keycloak_default_roles_test.go b/provider/resource_keycloak_default_roles_test.go index 1210d5949..ebebd4140 100644 --- a/provider/resource_keycloak_default_roles_test.go +++ b/provider/resource_keycloak_default_roles_test.go @@ -85,12 +85,8 @@ func testAccCheckKeycloakDefaultRolesDestroy(realmId string) resource.TestCheckF return fmt.Errorf("error getting defaultRoles with id %s: %s", realm.DefaultRole.Id, err) } - defaultRoles, err := keycloakClient.GetRoleFullNames(testCtx, realmId, composites) - if err != nil { - return err - } - if len(defaultRoles) != 0 { - return fmt.Errorf("realm %s still has %d default roles, expected zero", realmId, len(defaultRoles)) + if len(composites) != 0 { + return fmt.Errorf("realm %s still has %d default roles, expected zero", realmId, len(composites)) } return nil From 73dbbf7c849ab5e86fe9a07a0ae4f98e3e8ae4ab Mon Sep 17 00:00:00 2001 From: horus Date: Thu, 30 Nov 2023 13:04:35 +0800 Subject: [PATCH 4/6] refactor: misc changes for comparing default roles lists --- keycloak/default_roles.go | 25 ------- provider/resource_keycloak_default_roles.go | 71 ++++++++++--------- .../resource_keycloak_default_roles_test.go | 10 ++- 3 files changed, 43 insertions(+), 63 deletions(-) diff --git a/keycloak/default_roles.go b/keycloak/default_roles.go index 4ee175614..941661694 100644 --- a/keycloak/default_roles.go +++ b/keycloak/default_roles.go @@ -17,15 +17,6 @@ func (keycloakClient *KeycloakClient) GetDefaultRoles(ctx context.Context, realm if err != nil { return nil, err } - for _, composite := range composites { - if composite.ClientRole && composite.ClientId == "" { - client, err := keycloakClient.GetGenericClient(ctx, realmId, composite.ContainerId) - if err != nil { - return nil, err - } - composite.ClientId = client.ClientId - } - } return composites, nil } @@ -43,19 +34,3 @@ func (keycloakClient *KeycloakClient) GetQualifiedRoleName(ctx context.Context, role.ClientId = genericClient.ClientId return fmt.Sprintf("%s/%s", role.ClientId, role.Name), nil } - -func (keycloakClient *KeycloakClient) GetRoleFullNames(ctx context.Context, realmId string, roles []*Role) ([]string, error) { - fullNames := make([]string, len(roles)) - for i, role := range roles { - if !role.ClientRole { - fullNames[i] = role.Name - continue - } - fullName, err := keycloakClient.GetQualifiedRoleName(ctx, realmId, role) - if err != nil { - return []string{}, err - } - fullNames[i] = fullName - } - return fullNames, nil -} diff --git a/provider/resource_keycloak_default_roles.go b/provider/resource_keycloak_default_roles.go index a60f0eb38..f8486efb9 100644 --- a/provider/resource_keycloak_default_roles.go +++ b/provider/resource_keycloak_default_roles.go @@ -64,9 +64,13 @@ func resourceKeycloakDefaultRolesRead(ctx context.Context, data *schema.Resource return handleNotFoundError(ctx, err, data) } - defaultRoleNames, err := keycloakClient.GetRoleFullNames(ctx, realmId, composites) - if err != nil { - return handleNotFoundError(ctx, err, data) + var defaultRoleNames []string + for _, composite := range composites { + name, err := keycloakClient.GetQualifiedRoleName(ctx, realmId, composite) + if err != nil { + return handleNotFoundError(ctx, err, data) + } + defaultRoleNames = append(defaultRoleNames, name) } defaultRoles := &keycloak.DefaultRoles{ @@ -95,6 +99,10 @@ func resourceKeycloakDefaultRolesReconcile(ctx context.Context, data *schema.Res } local := mapFromDataToDefaultRoles(data) + localDefaultRoles := make(map[string]struct{}, len(local.DefaultRoles)) + for _, defaultRole := range local.DefaultRoles { + localDefaultRoles[defaultRole] = struct{}{} + } realm, err := keycloakClient.GetRealm(ctx, local.RealmId) if err != nil { @@ -108,47 +116,49 @@ func resourceKeycloakDefaultRolesReconcile(ctx context.Context, data *schema.Res return diag.FromErr(err) } - defaultRoleNames, err := keycloakClient.GetRoleFullNames(ctx, local.RealmId, composites) - if err != nil { - return diag.FromErr(err) - } + currentDefaultRoles := make([]string, len(composites)) + defaultRolesMap := make(map[string]*keycloak.Role, len(composites)) - defaultRolesMap := make(map[string]*keycloak.Role) - for i, roleName := range defaultRoleNames { - defaultRolesMap[roleName] = composites[i] + for i, composite := range composites { + name, err := keycloakClient.GetQualifiedRoleName(ctx, local.RealmId, composite) + if err != nil { + return diag.FromErr(err) + } + currentDefaultRoles[i] = name + defaultRolesMap[name] = composite } // skip if actual default roles in keycloak same as we want - if roleListsEqual(defaultRoleNames, local.DefaultRoles) { + if roleListsEqual(currentDefaultRoles, local.DefaultRoles) { return nil } getRole := func(roleName string) (*keycloak.Role, error) { - if !strings.Contains(roleName, "/") { - return keycloakClient.GetRoleByName(ctx, local.RealmId, "", roleName) - } - parts := strings.Split(roleName, "/") - client, err := keycloakClient.GetGenericClientByClientId(ctx, local.RealmId, parts[0]) - if err != nil { - return nil, err + var clientId string + if parts := strings.Split(roleName, "/"); len(parts) == 2 { + client, err := keycloakClient.GetGenericClientByClientId(ctx, local.RealmId, parts[0]) + if err != nil { + return nil, err + } + clientId, roleName = client.Id, parts[1] } - return keycloakClient.GetRoleByName(ctx, local.RealmId, client.Id, parts[1]) + return keycloakClient.GetRoleByName(ctx, local.RealmId, clientId, roleName) } var putList, deleteList []*keycloak.Role + for _, roleName := range local.DefaultRoles { - // keycloak doesn't have our locally defined roles - if !roleListContains(defaultRoleNames, roleName) { - defaultRoles, err := getRole(roleName) + if _, ok := defaultRolesMap[roleName]; !ok { + role, err := getRole(roleName) if err != nil { return diag.FromErr(err) } - putList = append(putList, defaultRoles) + putList = append(putList, role) } } - for _, roleName := range defaultRoleNames { - // keycloak have roles we don't want - if !roleListContains(local.DefaultRoles, roleName) { + + for _, roleName := range currentDefaultRoles { + if _, ok := localDefaultRoles[roleName]; !ok { deleteList = append(deleteList, defaultRolesMap[roleName]) } } @@ -232,15 +242,6 @@ func resourceKeycloakDefaultRolesImport(ctx context.Context, d *schema.ResourceD return []*schema.ResourceData{d}, nil } -func roleListContains(s []string, e string) bool { - for _, a := range s { - if a == e { - return true - } - } - return false -} - func roleListsEqual(a, b []string) bool { if len(a) != len(b) { return false diff --git a/provider/resource_keycloak_default_roles_test.go b/provider/resource_keycloak_default_roles_test.go index ebebd4140..5af3d0ad7 100644 --- a/provider/resource_keycloak_default_roles_test.go +++ b/provider/resource_keycloak_default_roles_test.go @@ -107,9 +107,13 @@ func getKeycloakDefaultRolesFromState(s *terraform.State, resourceName string) ( return nil, fmt.Errorf("error getting defaultRoles with id %s: %s", id, err) } - defaultRoleNamesList, err := keycloakClient.GetRoleFullNames(testCtx, realm, composites) - if err != nil { - return nil, fmt.Errorf("error getting defaultRoleNamesList with id %s: %s", id, err) + var defaultRoleNamesList []string + for _, composite := range composites { + name, err := keycloakClient.GetQualifiedRoleName(testCtx, realm, composite) + if err != nil { + return nil, fmt.Errorf("error getting qualified name for role id %s: %s", composite.Id, err) + } + defaultRoleNamesList = append(defaultRoleNamesList, name) } defaultRoles := &keycloak.DefaultRoles{ From 6e6b115ccd1fef7097518f48e689272a1ccbe3d9 Mon Sep 17 00:00:00 2001 From: horus Date: Thu, 30 Nov 2023 15:20:54 +0800 Subject: [PATCH 5/6] refactor: rename mapFromDataToDefaultRoles to getDefaultRolesFromData for consistency with naming conventions --- provider/resource_keycloak_default_roles.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/provider/resource_keycloak_default_roles.go b/provider/resource_keycloak_default_roles.go index f8486efb9..c4245e2dc 100644 --- a/provider/resource_keycloak_default_roles.go +++ b/provider/resource_keycloak_default_roles.go @@ -36,20 +36,18 @@ func resourceKeycloakDefaultRoles() *schema.Resource { } } -func mapFromDataToDefaultRoles(data *schema.ResourceData) *keycloak.DefaultRoles { - defaultRolesList := make([]string, 0) +func getDefaultRolesFromData(data *schema.ResourceData) *keycloak.DefaultRoles { + var defaultRolesList []string if v, ok := data.GetOk("default_roles"); ok { for _, defaultRole := range v.(*schema.Set).List() { defaultRolesList = append(defaultRolesList, defaultRole.(string)) } } - defaultRoles := &keycloak.DefaultRoles{ Id: data.Id(), RealmId: data.Get("realm_id").(string), DefaultRoles: defaultRolesList, } - return defaultRoles } @@ -98,7 +96,7 @@ func resourceKeycloakDefaultRolesReconcile(ctx context.Context, data *schema.Res return diag.FromErr(err) } - local := mapFromDataToDefaultRoles(data) + local := getDefaultRolesFromData(data) localDefaultRoles := make(map[string]struct{}, len(local.DefaultRoles)) for _, defaultRole := range local.DefaultRoles { localDefaultRoles[defaultRole] = struct{}{} From 2d75e0f85b0f027bbecb27e4c968175adc173b1b Mon Sep 17 00:00:00 2001 From: horus Date: Thu, 30 Nov 2023 15:23:56 +0800 Subject: [PATCH 6/6] refactor: use reflect.DeepEqual() to compare lists equality --- provider/resource_keycloak_default_roles.go | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/provider/resource_keycloak_default_roles.go b/provider/resource_keycloak_default_roles.go index c4245e2dc..52eefdd87 100644 --- a/provider/resource_keycloak_default_roles.go +++ b/provider/resource_keycloak_default_roles.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "reflect" "strings" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" @@ -127,7 +128,7 @@ func resourceKeycloakDefaultRolesReconcile(ctx context.Context, data *schema.Res } // skip if actual default roles in keycloak same as we want - if roleListsEqual(currentDefaultRoles, local.DefaultRoles) { + if reflect.DeepEqual(currentDefaultRoles, local.DefaultRoles) { return nil } @@ -239,15 +240,3 @@ func resourceKeycloakDefaultRolesImport(ctx context.Context, d *schema.ResourceD return []*schema.ResourceData{d}, nil } - -func roleListsEqual(a, b []string) bool { - if len(a) != len(b) { - return false - } - for i, v := range a { - if v != b[i] { - return false - } - } - return true -}