Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: client default/optional scopes first apply problem #594

Merged
merged 5 commits into from
Sep 23, 2021
Merged

fix: client default/optional scopes first apply problem #594

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4dff08a
Fixed client default/optional scopes first apply problem
Vlad-Kirichenko Sep 13, 2021
477a628
Merge branch 'master' of github.com:NapoleonGames/terraform-provider-…
Vlad-Kirichenko Sep 13, 2021
c685919
Removed test with expectations of resource behavior that was earlier …
Vlad-Kirichenko Sep 13, 2021
e0ead74
Merge branch 'master' of github.com:NapoleonGames/terraform-provider-…
Vlad-Kirichenko Sep 14, 2021
82c5599
Merge branch 'master' of github.com:NapoleonGames/terraform-provider-…
Vlad-Kirichenko Sep 20, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion keycloak/openid_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,9 @@ func (keycloakClient *KeycloakClient) getOpenidClientScopes(realmId, clientId, t
var scopes []*OpenidClientScope

err := keycloakClient.get(fmt.Sprintf("/realms/%s/clients/%s/%s-client-scopes", realmId, clientId, t), &scopes, nil)
if err != nil {
if err != nil && ErrorIs404(err) {
return nil, fmt.Errorf("validation error: client with id %s does not exist", clientId)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was there a particular reason that this is needed? normally, we check for 404 errors within the provider package.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I saw a problem in the TestAccKeycloakOpenidClientDefaultScopes_validateClientDoesNotExist test, and saw that the validation is missing when getting for scopes. Therefore, I added it.

It's strange that this check didn't work here.

} else if err != nil {
return nil, err
}

Expand Down
22 changes: 3 additions & 19 deletions provider/resource_keycloak_openid_client_default_scopes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import (

func resourceKeycloakOpenidClientDefaultScopes() *schema.Resource {
return &schema.Resource{
Create: resourceKeycloakOpenidClientDefaultScopesCreate,
Create: resourceKeycloakOpenidClientDefaultScopesReconcile,
Read: resourceKeycloakOpenidClientDefaultScopesRead,
Delete: resourceKeycloakOpenidClientDefaultScopesDelete,
Update: resourceKeycloakOpenidClientDefaultScopesUpdate,
Update: resourceKeycloakOpenidClientDefaultScopesReconcile,
Schema: map[string]*schema.Schema{
"realm_id": {
Type: schema.TypeString,
Expand All @@ -33,22 +33,6 @@ func resourceKeycloakOpenidClientDefaultScopes() *schema.Resource {
}
}

func resourceKeycloakOpenidClientDefaultScopesCreate(data *schema.ResourceData, meta interface{}) error {
keycloakClient := meta.(*keycloak.KeycloakClient)
realmId := data.Get("realm_id").(string)
clientId := data.Get("client_id").(string)
defaultScopes := data.Get("default_scopes").(*schema.Set)

err := keycloakClient.AttachOpenidClientDefaultScopes(realmId, clientId, interfaceSliceToStringSlice(defaultScopes.List()))
if err != nil {
return err
}

data.SetId(openidClientDefaultScopesId(realmId, clientId))

return resourceKeycloakOpenidClientDefaultScopesRead(data, meta)
}

func openidClientDefaultScopesId(realmId string, clientId string) string {
return fmt.Sprintf("%s/%s", realmId, clientId)
}
Expand All @@ -75,7 +59,7 @@ func resourceKeycloakOpenidClientDefaultScopesRead(data *schema.ResourceData, me
return nil
}

func resourceKeycloakOpenidClientDefaultScopesUpdate(data *schema.ResourceData, meta interface{}) error {
func resourceKeycloakOpenidClientDefaultScopesReconcile(data *schema.ResourceData, meta interface{}) error {
keycloakClient := meta.(*keycloak.KeycloakClient)

realmId := data.Get("realm_id").(string)
Expand Down
23 changes: 0 additions & 23 deletions provider/resource_keycloak_openid_client_default_scopes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,29 +258,6 @@ func TestAccKeycloakOpenidClientDefaultScopes_noImportNeeded(t *testing.T) {
})
}

// by default, keycloak clients have the default scopes "profile", "email", "roles",
// and "web-origins" attached. if you create this resource with only one scope, it
// won't remove these two scopes, because the creation of a new resource should not
// result in anything destructive. thus, a following plan will not be empty, as terraform
// will think it needs to remove these scopes, which is okay to do during an update
func TestAccKeycloakOpenidClientDefaultScopes_profileAndEmailDefaultScopes(t *testing.T) {
t.Parallel()
client := acctest.RandomWithPrefix("tf-acc")
clientScope := acctest.RandomWithPrefix("tf-acc")

resource.Test(t, resource.TestCase{
ProviderFactories: testAccProviderFactories,
PreCheck: func() { testAccPreCheck(t) },
Steps: []resource.TestStep{
{
Config: testKeycloakOpenidClientDefaultScopes_listOfScopes(client, clientScope, []string{clientScope}),
Check: testAccCheckKeycloakOpenidClientHasDefaultScopes("keycloak_openid_client.client", append(preAssignedDefaultClientScopes, clientScope)),
ExpectNonEmptyPlan: true,
},
},
})
}

// Keycloak throws a 500 if you attempt to attach an optional scope that is already attached as an optional scope
func TestAccKeycloakOpenidClientDefaultScopes_validateDuplicateScopeAssignment(t *testing.T) {
t.Parallel()
Expand Down
22 changes: 3 additions & 19 deletions provider/resource_keycloak_openid_client_optional_scopes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ import (

func resourceKeycloakOpenidClientOptionalScopes() *schema.Resource {
return &schema.Resource{
Create: resourceKeycloakOpenidClientOptionalScopesCreate,
Create: resourceKeycloakOpenidClientOptionalScopesReconcile,
Read: resourceKeycloakOpenidClientOptionalScopesRead,
Delete: resourceKeycloakOpenidClientOptionalScopesDelete,
Update: resourceKeycloakOpenidClientOptionalScopesUpdate,
Update: resourceKeycloakOpenidClientOptionalScopesReconcile,
Schema: map[string]*schema.Schema{
"realm_id": {
Type: schema.TypeString,
Expand All @@ -33,22 +33,6 @@ func resourceKeycloakOpenidClientOptionalScopes() *schema.Resource {
}
}

func resourceKeycloakOpenidClientOptionalScopesCreate(data *schema.ResourceData, meta interface{}) error {
keycloakClient := meta.(*keycloak.KeycloakClient)
realmId := data.Get("realm_id").(string)
clientId := data.Get("client_id").(string)
optionalScopes := data.Get("optional_scopes").(*schema.Set)

err := keycloakClient.AttachOpenidClientOptionalScopes(realmId, clientId, interfaceSliceToStringSlice(optionalScopes.List()))
if err != nil {
return err
}

data.SetId(openidClientOptionalScopesId(realmId, clientId))

return resourceKeycloakOpenidClientOptionalScopesRead(data, meta)
}

func openidClientOptionalScopesId(realmId string, clientId string) string {
return fmt.Sprintf("%s/%s", realmId, clientId)
}
Expand All @@ -75,7 +59,7 @@ func resourceKeycloakOpenidClientOptionalScopesRead(data *schema.ResourceData, m
return nil
}

func resourceKeycloakOpenidClientOptionalScopesUpdate(data *schema.ResourceData, meta interface{}) error {
func resourceKeycloakOpenidClientOptionalScopesReconcile(data *schema.ResourceData, meta interface{}) error {
keycloakClient := meta.(*keycloak.KeycloakClient)

realmId := data.Get("realm_id").(string)
Expand Down
24 changes: 0 additions & 24 deletions provider/resource_keycloak_openid_client_optional_scopes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,30 +264,6 @@ func TestAccKeycloakOpenidClientOptionalScopes_noImportNeeded(t *testing.T) {
})
}

// by optional, keycloak clients have the optional scopes "address", "phone" and
// "offline_access" "microprofile-jwt" attached. if you create this resource with only one scope, it
// won't remove these two scopes, because the creation of a new resource should
// not result in anything destructive. thus, a following plan will not be empty,
// as terraform will think it needs to remove these scopes, which is okay to do
// during an update
func TestAccKeycloakOpenidClientOptionalScopes_profileAndEmailOptionalScopes(t *testing.T) {
t.Parallel()
client := acctest.RandomWithPrefix("tf-acc")
clientScope := acctest.RandomWithPrefix("tf-acc")

resource.Test(t, resource.TestCase{
ProviderFactories: testAccProviderFactories,
PreCheck: func() { testAccPreCheck(t) },
Steps: []resource.TestStep{
{
Config: testKeycloakOpenidClientOptionalScopes_listOfScopes(client, clientScope, []string{clientScope}),
Check: testAccCheckKeycloakOpenidClientHasOptionalScopes("keycloak_openid_client.client", append(getPreAssignedOptionalClientScopes(), clientScope)),
ExpectNonEmptyPlan: true,
},
},
})
}

// Keycloak throws a 500 if you attempt to attach an optional scope that is already attached as a default scope
func TestAccKeycloakOpenidClientOptionalScopes_validateDuplicateScopeAssignment(t *testing.T) {
t.Parallel()
Expand Down