From b8b9ccf067d01cd8e1da6d4218d6cdb209a9d3e4 Mon Sep 17 00:00:00 2001 From: Bobby Iliev <bobby@bobbyiliev.com> Date: Mon, 8 Jul 2024 18:18:35 +0300 Subject: [PATCH] Only fetch the list of Frontegg roles once per TF provider invocation (#595) * Only fetch the list of Frontegg roles once per TF provider invocation * Fix failing GetSCIMGroupByID requests --- pkg/frontegg/scim_groups.go | 2 +- pkg/frontegg/sso_default_roles.go | 2 +- pkg/frontegg/sso_default_roles_test.go | 4 ++-- pkg/provider/provider.go | 8 ++++++++ pkg/resources/resource_app_password.go | 17 ++++------------- pkg/resources/resource_scim_group_roles.go | 14 +++++--------- pkg/resources/resource_scim_group_roles_test.go | 4 ++++ pkg/resources/resource_sso_default_roles.go | 10 ++-------- .../resource_sso_default_roles_test.go | 12 ++++++++++++ pkg/resources/resource_sso_group_mapping.go | 17 +++-------------- .../resource_sso_group_mapping_test.go | 8 ++++++++ pkg/resources/resource_user.go | 7 +------ pkg/testhelpers/helpers.go | 4 ++++ pkg/utils/provider_meta.go | 4 ++++ 14 files changed, 59 insertions(+), 54 deletions(-) diff --git a/pkg/frontegg/scim_groups.go b/pkg/frontegg/scim_groups.go index 6f1c0109..ce232295 100644 --- a/pkg/frontegg/scim_groups.go +++ b/pkg/frontegg/scim_groups.go @@ -131,7 +131,7 @@ func DeleteSCIMGroup(ctx context.Context, client *clients.FronteggClient, groupI // GetSCIMGroupByID fetches a single SCIM group by its ID. func GetSCIMGroupByID(ctx context.Context, client *clients.FronteggClient, groupID string) (*ScimGroup, error) { - endpoint := fmt.Sprintf("%s%s/%s?_groupsRelations=rolesAndUsers", client.Endpoint, SCIMGroupsApiPathV1, groupID) + endpoint := fmt.Sprintf("%s%s/%s/?_groupsRelations=rolesAndUsers", client.Endpoint, SCIMGroupsApiPathV1, groupID) resp, err := doRequest(ctx, client, "GET", endpoint, nil) if err != nil { return nil, err diff --git a/pkg/frontegg/sso_default_roles.go b/pkg/frontegg/sso_default_roles.go index 8d3b090b..307bbaa1 100644 --- a/pkg/frontegg/sso_default_roles.go +++ b/pkg/frontegg/sso_default_roles.go @@ -33,7 +33,7 @@ type FronteggRole struct { } // ListRoles fetches roles from the Frontegg API and returns a map of role names to their IDs. -func ListSSORoles(ctx context.Context, client *clients.FronteggClient) (map[string]string, error) { +func ListFronteggRoles(ctx context.Context, client *clients.FronteggClient) (map[string]string, error) { endpoint := fmt.Sprintf("%s%s", client.Endpoint, SSORolesApiPathV2) resp, err := doRequest(ctx, client, "GET", endpoint, nil) if err != nil { diff --git a/pkg/frontegg/sso_default_roles_test.go b/pkg/frontegg/sso_default_roles_test.go index 6047bfd2..32f42c1c 100644 --- a/pkg/frontegg/sso_default_roles_test.go +++ b/pkg/frontegg/sso_default_roles_test.go @@ -11,7 +11,7 @@ import ( "github.com/stretchr/testify/assert" ) -func TestListSSORolesSuccess(t *testing.T) { +func TestListFronteggRolesSuccess(t *testing.T) { assert := assert.New(t) rolesResponse := FronteggRolesResponse{ @@ -33,7 +33,7 @@ func TestListSSORolesSuccess(t *testing.T) { HTTPClient: mockServer.Client(), } - roles, err := ListSSORoles(context.Background(), client) + roles, err := ListFronteggRoles(context.Background(), client) assert.NoError(err) assert.Equal(2, len(roles)) assert.Equal("role-id-1", roles["Admin"]) diff --git a/pkg/provider/provider.go b/pkg/provider/provider.go index 2ee91ac4..aa576ce5 100644 --- a/pkg/provider/provider.go +++ b/pkg/provider/provider.go @@ -7,6 +7,7 @@ import ( "github.com/MaterializeInc/terraform-provider-materialize/pkg/clients" "github.com/MaterializeInc/terraform-provider-materialize/pkg/datasources" + "github.com/MaterializeInc/terraform-provider-materialize/pkg/frontegg" "github.com/MaterializeInc/terraform-provider-materialize/pkg/resources" "github.com/MaterializeInc/terraform-provider-materialize/pkg/utils" @@ -229,6 +230,12 @@ func providerConfigure(ctx context.Context, d *schema.ResourceData, version stri log.Printf("[DEBUG] Initialized DB clients for regions: %v\n", dbClients) + // Fetch Frontegg roles and store them in the provider meta + fronteggRoles, err := frontegg.ListFronteggRoles(ctx, fronteggClient) + if err != nil { + return nil, diag.Errorf("Unable to fetch Frontegg roles: %s", err) + } + // Construct and return the provider meta with all clients initialized. providerMeta := &utils.ProviderMeta{ DB: dbClients, @@ -236,6 +243,7 @@ func providerConfigure(ctx context.Context, d *schema.ResourceData, version stri CloudAPI: cloudAPIClient, DefaultRegion: clients.Region(defaultRegion), RegionsEnabled: regionsEnabled, + FronteggRoles: fronteggRoles, } return providerMeta, nil diff --git a/pkg/resources/resource_app_password.go b/pkg/resources/resource_app_password.go index b49fdb16..01c0e719 100644 --- a/pkg/resources/resource_app_password.go +++ b/pkg/resources/resource_app_password.go @@ -2,7 +2,6 @@ package resources import ( "context" - "fmt" "sort" "time" @@ -124,12 +123,8 @@ func appPasswordCreate(ctx context.Context, d *schema.ResourceData, meta interfa return diag.Errorf("at least one role is required for a service-type app password") } - // TODO: only fetch the list of SSO roles once per TF provider - // invocation. - roleMap, err := frontegg.ListSSORoles(ctx, client) - if err != nil { - return diag.FromErr(fmt.Errorf("error fetching roles: %s", err)) - } + roleMap := providerMeta.FronteggRoles + var roleIDs []string for _, role := range roles { if roleID, ok := roleMap[role]; ok { @@ -201,12 +196,8 @@ func appPasswordRead(ctx context.Context, d *schema.ResourceData, meta interface // We don't update secret and password because those fields can only be // determined at creation time. } else { - // TODO: only fetch the list of SSO roles once per TF provider - // invocation. - roleMap, err := frontegg.ListSSORoles(ctx, client) - if err != nil { - return diag.FromErr(fmt.Errorf("error fetching roles: %s", err)) - } + roleMap := providerMeta.FronteggRoles + roleReverseMap := make(map[string]string) for roleName, roleId := range roleMap { roleReverseMap[roleId] = roleName diff --git a/pkg/resources/resource_scim_group_roles.go b/pkg/resources/resource_scim_group_roles.go index 72855bd9..f4707366 100644 --- a/pkg/resources/resource_scim_group_roles.go +++ b/pkg/resources/resource_scim_group_roles.go @@ -6,7 +6,6 @@ import ( "log" "strings" - "github.com/MaterializeInc/terraform-provider-materialize/pkg/clients" "github.com/MaterializeInc/terraform-provider-materialize/pkg/frontegg" "github.com/MaterializeInc/terraform-provider-materialize/pkg/utils" "github.com/hashicorp/terraform-plugin-sdk/v2/diag" @@ -54,7 +53,7 @@ func scimGroupRoleCreate(ctx context.Context, d *schema.ResourceData, meta inter } client := providerMeta.Frontegg - roleIDs, err := getRoleIDsByName(ctx, client, roleNames) + roleIDs, err := getRoleIDsByName(ctx, providerMeta, roleNames) if err != nil { return diag.FromErr(fmt.Errorf("error getting role IDs: %s", err)) } @@ -141,7 +140,7 @@ func scimGroupRoleUpdate(ctx context.Context, d *schema.ResourceData, meta inter // Add the new roles to the group newRoleNames := expandStringSet(d.Get("roles").(*schema.Set)) - newRoleIDs, err := getRoleIDsByName(ctx, client, newRoleNames) + newRoleIDs, err := getRoleIDsByName(ctx, providerMeta, newRoleNames) if err != nil { return diag.FromErr(fmt.Errorf("error getting role IDs: %s", err)) } @@ -169,7 +168,7 @@ func scimGroupRoleDelete(ctx context.Context, d *schema.ResourceData, meta inter } client := providerMeta.Frontegg - roleIDs, err := getRoleIDsByName(ctx, client, roleNames) + roleIDs, err := getRoleIDsByName(ctx, providerMeta, roleNames) if err != nil { return diag.FromErr(fmt.Errorf("error getting role IDs: %s", err)) } @@ -186,11 +185,8 @@ func scimGroupRoleDelete(ctx context.Context, d *schema.ResourceData, meta inter } // Helper function to expand a set of role names to a list of role IDs -func getRoleIDsByName(ctx context.Context, client *clients.FronteggClient, roleNames []string) ([]string, error) { - roleMap, err := frontegg.ListSSORoles(ctx, client) - if err != nil { - return nil, err - } +func getRoleIDsByName(ctx context.Context, providerMeta *utils.ProviderMeta, roleNames []string) ([]string, error) { + roleMap := providerMeta.FronteggRoles var roleIDs []string for _, roleName := range roleNames { diff --git a/pkg/resources/resource_scim_group_roles_test.go b/pkg/resources/resource_scim_group_roles_test.go index 5ef4ecb7..e13aebed 100644 --- a/pkg/resources/resource_scim_group_roles_test.go +++ b/pkg/resources/resource_scim_group_roles_test.go @@ -32,6 +32,10 @@ func TestScimGroupRoleResourceCreate(t *testing.T) { providerMeta := &utils.ProviderMeta{ Frontegg: client, + FronteggRoles: map[string]string{ + "Admin": "1", + "Member": "2", + }, } if err := scimGroupRoleCreate(context.TODO(), d, providerMeta); err != nil { diff --git a/pkg/resources/resource_sso_default_roles.go b/pkg/resources/resource_sso_default_roles.go index 98950361..70687216 100644 --- a/pkg/resources/resource_sso_default_roles.go +++ b/pkg/resources/resource_sso_default_roles.go @@ -54,10 +54,7 @@ func ssoDefaultRolesCreateOrUpdate(ctx context.Context, d *schema.ResourceData, ssoConfigID := d.Get("sso_config_id").(string) roleNames := convertToStringSlice(d.Get("roles").(*schema.Set).List()) - roleMap, err := frontegg.ListSSORoles(ctx, client) - if err != nil { - return diag.FromErr(err) - } + roleMap := providerMeta.FronteggRoles var roleIDs []string for _, roleName := range roleNames { @@ -95,10 +92,7 @@ func ssoDefaultRolesRead(ctx context.Context, d *schema.ResourceData, meta inter return diag.FromErr(err) } - roleMap, err := frontegg.ListSSORoles(ctx, client) - if err != nil { - return diag.FromErr(err) - } + roleMap := providerMeta.FronteggRoles var roleNames []string for _, roleID := range roleIDs { diff --git a/pkg/resources/resource_sso_default_roles_test.go b/pkg/resources/resource_sso_default_roles_test.go index df4e9d14..b281d3cf 100644 --- a/pkg/resources/resource_sso_default_roles_test.go +++ b/pkg/resources/resource_sso_default_roles_test.go @@ -26,6 +26,10 @@ func TestSSODefaultRolesCreateOrUpdate(t *testing.T) { providerMeta := &utils.ProviderMeta{ Frontegg: client, + FronteggRoles: map[string]string{ + "Admin": "1", + "Member": "2", + }, } // Create a new ResourceData object @@ -70,6 +74,10 @@ func TestSSODefaultRolesRead(t *testing.T) { providerMeta := &utils.ProviderMeta{ Frontegg: client, + FronteggRoles: map[string]string{ + "Admin": "1", + "Member": "2", + }, } // Set the initial state with "sso_config_id" and "roles" as a list of strings @@ -110,6 +118,10 @@ func TestSSODefaultRolesDelete(t *testing.T) { providerMeta := &utils.ProviderMeta{ Frontegg: client, + FronteggRoles: map[string]string{ + "Admin": "1", + "Member": "2", + }, } // Set the initial state with "sso_config_id" and "roles" as a list of strings diff --git a/pkg/resources/resource_sso_group_mapping.go b/pkg/resources/resource_sso_group_mapping.go index 29f724cb..ad15da53 100644 --- a/pkg/resources/resource_sso_group_mapping.go +++ b/pkg/resources/resource_sso_group_mapping.go @@ -63,11 +63,7 @@ func ssoGroupMappingCreate(ctx context.Context, d *schema.ResourceData, meta int group := d.Get("group").(string) roleNames := convertToStringSlice(d.Get("roles").(*schema.Set).List()) - // Fetch role IDs based on role names. - roleMap, err := frontegg.ListSSORoles(ctx, client) - if err != nil { - return diag.FromErr(fmt.Errorf("error fetching roles: %s", err)) - } + roleMap := providerMeta.FronteggRoles var roleIDs []string for _, roleName := range roleNames { @@ -106,11 +102,7 @@ func ssoGroupMappingRead(ctx context.Context, d *schema.ResourceData, meta inter return diag.FromErr(err) } - // Fetch role mappings from the API. - roleMap, err := frontegg.ListSSORoles(ctx, client) - if err != nil { - return diag.FromErr(fmt.Errorf("error fetching roles: %s", err)) - } + roleMap := providerMeta.FronteggRoles for _, group := range *groups { if group.ID == groupID { @@ -153,10 +145,7 @@ func ssoGroupMappingUpdate(ctx context.Context, d *schema.ResourceData, meta int groupID := d.Id() roleNames := convertToStringSlice(d.Get("roles").(*schema.Set).List()) - roleMap, err := frontegg.ListSSORoles(ctx, client) - if err != nil { - return diag.FromErr(fmt.Errorf("error fetching roles: %s", err)) - } + roleMap := providerMeta.FronteggRoles var roleIDs []string for _, roleName := range roleNames { diff --git a/pkg/resources/resource_sso_group_mapping_test.go b/pkg/resources/resource_sso_group_mapping_test.go index b24cbc01..a1e4a71d 100644 --- a/pkg/resources/resource_sso_group_mapping_test.go +++ b/pkg/resources/resource_sso_group_mapping_test.go @@ -25,6 +25,10 @@ func TestSSORoleGroupMappingCreate(t *testing.T) { providerMeta := &utils.ProviderMeta{ Frontegg: client, + FronteggRoles: map[string]string{ + "Admin": "1", + "Member": "2", + }, } // Set the expected values for sso_config_id, group, and roles @@ -96,6 +100,10 @@ func TestSSORoleGroupMappingUpdate(t *testing.T) { providerMeta := &utils.ProviderMeta{ Frontegg: client, + FronteggRoles: map[string]string{ + "Admin": "1", + "Member": "2", + }, } // Set the initial state with "group" and "roles" diff --git a/pkg/resources/resource_user.go b/pkg/resources/resource_user.go index 46434951..79e83f7b 100644 --- a/pkg/resources/resource_user.go +++ b/pkg/resources/resource_user.go @@ -2,7 +2,6 @@ package resources import ( "context" - "fmt" "strings" "github.com/MaterializeInc/terraform-provider-materialize/pkg/frontegg" @@ -82,11 +81,7 @@ func userCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) d } } - // Fetch role IDs based on role names. - roleMap, err := frontegg.ListSSORoles(ctx, client) - if err != nil { - return diag.FromErr(fmt.Errorf("error fetching roles: %s", err)) - } + roleMap := providerMeta.FronteggRoles var roleIDs []string for _, roleName := range roleNames { diff --git a/pkg/testhelpers/helpers.go b/pkg/testhelpers/helpers.go index 5f43c650..c8f1c7c4 100644 --- a/pkg/testhelpers/helpers.go +++ b/pkg/testhelpers/helpers.go @@ -121,6 +121,10 @@ func WithMockProviderMeta(t *testing.T, f func(*utils.ProviderMeta, sqlmock.Sqlm TokenExpiry: time.Date(9999, 1, 1, 0, 0, 0, 0, time.UTC), }, CloudAPI: nil, + FronteggRoles: map[string]string{ + "Admin": "1", + "Member": "2", + }, } mock.MatchExpectationsInOrder(true) diff --git a/pkg/utils/provider_meta.go b/pkg/utils/provider_meta.go index 85a77112..682b38e7 100644 --- a/pkg/utils/provider_meta.go +++ b/pkg/utils/provider_meta.go @@ -32,6 +32,10 @@ type ProviderMeta struct { // RegionsEnabled is a map indicating which regions are currently enabled // for use. This can be used to quickly check the availability in different regions. RegionsEnabled map[clients.Region]bool + + // Frontegg Roles is a map that associates each Frontegg role with its corresponding ID. + // This is used to map role names to role IDs when creating/updating users. + FronteggRoles map[string]string } var DefaultRegion string