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_oidc_client import unexpected behaviour #1007

Open
chrismilson opened this issue Oct 22, 2024 · 3 comments · May be fixed by #1008
Open

keycloak_oidc_client import unexpected behaviour #1007

chrismilson opened this issue Oct 22, 2024 · 3 comments · May be fixed by #1008

Comments

@chrismilson
Copy link

When setting import = true on an openid client, values defined in the terraform config for other fields are not applied on create, and appear as changes in a subsequent apply.

For example:

# Assume this was applied and then removed from state
resource "keycloak_openid_client" "example" {
  client_id   = "example"
  enabled     = true
  access_type = "CONFIDENTIAL"
}

###

# Then the following code was deployed
resource "keycloak_openid_client" "example" {
  client_id   = "example"
  enabled     = false
  access_type = "PUBLIC"
  import      = true
}

On the first plan the resource shows that enabled will be false and on apply the client is imported into state. On a subsequent plan, however we see:

  # keycloak_openid_client.example will be updated in-place
  ~ resource "keycloak_openid_client" "account_console" {
      ~ enabled                                    = true -> false
      ...
    }

This is an inconsistency between plan and apply, and since there is no error on the first apply, it may not be obvious that the client is still enabled, which could be a security concern in some use cases.

@chrismilson
Copy link
Author

chrismilson commented Oct 22, 2024

The issue seems to stem from the use of the mergo.Merge(client, existingClient) call here:
https://github.com/mrparkers/terraform-provider-keycloak/blob/d65c3b87b47fc609fe0252bf6d4a4a26fcac7f67/provider/resource_keycloak_openid_client.go#L536-L538

Since false is the default value for Bool, mergo assumes the field was unset in the client object, and overwrites it with whatever is in the existingClient object. See darccio/mergo#129.

Since the Update method doesn't know what the config was at import time, a subsequent apply updates the field to the expected value.

@chrismilson
Copy link
Author

Because Update wouldn't respect any previous configuration anyway, would it be possible to remove the "merge with existing client config" altogether?

@chrismilson
Copy link
Author

chrismilson commented Oct 23, 2024

To avoid silently failing, a postcondition can be added to the resource. This ensures an error will be thrown when the bug is encountered:

resource "keycloak_openid_client" "example" {
  client_id   = "example"
  enabled     = false
  access_type = "PUBLIC"
  import      = true

  lifecycle {
    postcondition {
      condition = self.enabled == false # Add `&& self.<field> == <expected value>` for each field whose value is important. Of course, you should probably also use a local variable to store the target value for each field and reference the same local in this condition to ensure they are consistent.
      error_message = <<-EOT
        There is a bug with the keycloak provider that causes some fields to be set to unexpected values when `import = true`.
        See https://github.com/mrparkers/terraform-provider-keycloak/issues/1007

        This bug has just occurred. You must perform another terraform apply for the expected values to be applied.
        EOT
    }
  }
}

On the first apply, the resource is created, but fails the postcondition. On a subsequent apply, the resource already exists in the terraform state, and the update sets the field to the expected value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant