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.

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

keycloak_openid_client_optional_scopes does not remove scopes on first apply #498

Closed
marknelissen opened this issue Mar 15, 2021 · 4 comments
Labels
good first issue Good for newcomers

Comments

@marknelissen
Copy link

The keycloak_openid_client_optional_scopes resource does not apply the list exactly as described on the first run of a new environment. It does add the elements listed, but does not remove the not listed default entries. This makes it so that following example requires two applies (of which one a failed one) to actually get the wanted situation: move the offline_access scope from the optional to the default scope.

resource "keycloak_openid_client_optional_scopes" "opt" {
  realm_id  = keycloak_realm.tenant.id
  client_id = keycloak_openid_client.client.id
  optional_scopes = [
    "address",
    "phone",
    "microprofile-jwt",
  ]
}

resource "keycloak_openid_client_default_scopes" "def" {
  realm_id  = keycloak_realm.tenant.id
  client_id = keycloak_openid_client.client.id
  default_scopes = [
    "profile",
    "email",
    "roles",
    "web-origins",
    "offline_access",
  ]
  depends_on = [
    keycloak_openid_client_optional_scopes.opt
  ]
}
@hamiltont
Copy link
Contributor

hamiltont commented Aug 28, 2021

keycloak_openid_client_default_scopes has exactly the same issue - the first apply will add your own listed scopes, but it takes a second apply for TF to notice that the KC added some additional default "default scopes" on the server side. Never get an apply failure though, it runs happily both times

I wonder if the KC API returns a representation of the object that was just created? If yes, then perhaps a general solution that would work for multiple resource types would be to have some central logic to test if the returned resource equals the requested resource, and auto-apply that resource a second time if it appears the server added/customized the creation request based on server-side defaults. It's a fairly common problem with REST APIs that they expand a POST request and you need a followup PUT/PATCH to get what you actually requested.

@mrparkers
Copy link
Owner

This was a deliberate decision I made back when creating these resources. At the time, it felt weird to me that a "create" for a resource could result in a potentially destructive action (a client scope being removed from the list).

However, I think I've changed my mind on this, and I agree that it would be better (and more CI friendly) for this resource to fully reconcile on both create and update. We already do this for the keycloak_user_groups resource, for example.

If someone is interested in contributing this change, I'd be happy to review and merge a PR for it. I think it is a relatively simple change - we'd want to remove the create function for this resource, rename the update function to something like resourceKeycloakOpenidClientDefaultScopesReconcile, then use this function for the create operation as well as the update operation.

Otherwise, I can try to fix this when I have some spare time.

@Vlad-Kirichenko
Copy link
Contributor

Hello @mrparkers.
I would start doing this. PR will soon be created

@mrparkers
Copy link
Owner

Closed via #594

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants