keycloak_oidc_client import unexpected behaviour

[chrismilson]

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]

The issue seems to stem from the use of the mergo.Merge(client, existingClient) call here:

terraform-provider-keycloak/provider/resource_keycloak_openid_client.go

Lines 536 to 538 in d65c3b8

	if err = mergo.Merge(client, existingClient); err != nil {
	return diag.FromErr(err)
	}

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]

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

[chrismilson]

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.