mrparkers / terraform-provider-keycloak

Terraform provider for Keycloak
https://registry.terraform.io/providers/mrparkers/keycloak/latest/docs
MIT License
604 stars 294 forks source link

keycloak_realm : custom attributes are not imported with terraform import command #592

Open daviddelannoy opened 2 years ago

daviddelannoy commented 2 years ago

Hi,

Expected Behavior

Can import existing realm attributes with resource keycloak_realm

Current Behavior

Existing realm attributes are not imported with the import command of keycloak_realm

Steps to Reproduce

Have an existing test realm with some custom attributes.

resource "keycloak_realm" "test" {
  realm = "test"

  attributes = {
    "key1" = "value1"
    "key2" = "value2"
  }
}

Try to import this realm :

terraform init
terraform import keycloak_realm.test test

Attributes are not imported with import command. terraform.tfstatecontent with empty attributes :

{
  "version": 4,
  "terraform_version": "1.0.4",
  "serial": 1,
  "lineage": "18bc35c0-e0f3-4c8e-eb8e-49876113b5d9",
  "outputs": {},
  "resources": [
    {
      "mode": "managed",
      "type": "keycloak_realm",
      "name": "test",
      "provider": "provider[\"registry.terraform.io/mrparkers/keycloak\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "access_code_lifespan": "1m0s",
            "access_code_lifespan_login": "30m0s",
            "access_code_lifespan_user_action": "5m0s",
            "access_token_lifespan": "5m0s",
            "access_token_lifespan_for_implicit_flow": "15m0s",
            "account_theme": "",
            "action_token_generated_by_admin_lifespan": "12h0m0s",
            "action_token_generated_by_user_lifespan": "5m0s",
            "admin_theme": "",
            "attributes": {},
            "browser_flow": "browser",
[...]
}

So, terraform apply suggest to create these attributes :

$ terraform apply

keycloak_realm.test: Refreshing state... [id=test]

Terraform used the selected providers to generate the following execution plan. Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  # keycloak_realm.test will be updated in-place
  ~ resource "keycloak_realm" "test" {
      ~ attributes                               = {
          + "key1" = "value1"
          + "key2" = "value2"
        }
        id                                       = "test"
        # (37 unchanged attributes hidden)

        # (2 unchanged blocks hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Context (Environment)

We are doing retro-engineering on actual configuration to provision everything with this provider

Tested with latest version 3.3.0

Possible Solution

Problem seems to be here :

https://github.com/mrparkers/terraform-provider-keycloak/blob/cb74346df6c3e4bb4e2696824d60b65932a1bea3/provider/resource_keycloak_realm.go#L1144

as data.GetOk("attributes") will always be empty when running initial import command

Import works fine by replacing

attributes := map[string]interface{}{}
if v, ok := data.GetOk("attributes"); ok {
  for key := range v.(map[string]interface{}) {
    attributes[key] = realm.Attributes[key]
    //We are only interested in attributes managed in terraform (Keycloak returns a lot of doubles values in the attributes...)
  }
}

by

attributes := map[string]interface{}{}
for k, v := range realm.Attributes {
  attributes[k] = v
  //We are only interested in attributes managed in terraform (Keycloak returns a lot of doubles values in the attributes...)
}

but I was wondering about the above comment mentioning duplicates values ? @mrparkers ?

After another import, tfstate is ok :

{
  "version": 4,
  "terraform_version": "1.0.4",
  "serial": 1,
  "lineage": "f4a49074-1577-1311-4da8-19f7ebe53a61",
  "outputs": {},
  "resources": [
    {
      "mode": "managed",
      "type": "keycloak_realm",
      "name": "test",
      "provider": "provider[\"registry.terraform.io/mrparkers/keycloak\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "access_code_lifespan": "1m0s",
            "access_code_lifespan_login": "30m0s",
            "access_code_lifespan_user_action": "5m0s",
            "access_token_lifespan": "5m0s",
            "access_token_lifespan_for_implicit_flow": "15m0s",
            "account_theme": "",
            "action_token_generated_by_admin_lifespan": "12h0m0s",
            "action_token_generated_by_user_lifespan": "5m0s",
            "admin_theme": "",
            "attributes": {
              "key1": "value1",
              "key2": "value2"
            },
            "browser_flow": "browser",
mrparkers commented 2 years ago

I was able to reproduce this issue myself. I tried your suggestion locally, but it presents other problems, like a non-empty plan following an apply operation (which automatically causes tests to fail).

The issue with the attributes field for this resource is that we're in this weird state where we're trying to partially manage it. Some official Keycloak features use the attributes object in the realm JSON (such as the relatively new CIBA Policy features), and it's also used for custom attributes like the ones you're trying to use. So we're trying to manage some of this attributes object, without overwriting all of it. This causes weird issues like the one you're describing.

I spent some time trying to find a good compromise with no luck. I think it might be possible to treat this field similarly to the extra_config argument that we support on some of the other resources. That might produce a better result.

@tomrutsaert do you have any thoughts on this?

daviddelannoy commented 2 years ago

Thanks for the comment @mrparkers

We also have discussed about this problem between our team members and also think that it may be easier to manage custom attributes with extra_config as you suggest.

It makes sense to be able to distinguish attributes used by keycloak for internal features from pure custom attributes.

We can try to make a PR like #579 and following ones you have made on other resources ?

tomrutsaert commented 2 years ago

It is very annoying that keycloak has it own extra attributes derived form other config settings. As indicated by @mrparkers , this makes the whole attributes lifecycle way more complex. We could indeed add the extra_config for all custom attributes. But we need to be aware people will be already be using the attributes variable for custom attributes. as it does work for custom attributes. It only seems that importing is not working. I have several projects where I set custom attributes via realm>attributes, so if custom attributes need to be moved to realm>extra_config. this should be at least mentioned in the release notes and preferable be shown in the " terraform plan"

Is there any harm to import all realm attributes initially? As indicated in opening post? The only thing that will be strange is the first apply, after the first apply to tf state should be correct..... (we could warn the user about this....) Import should could be adapted to do the apply twice....

(I typed this comment without looking at the code, so I could be missing something)