hashicorp / terraform

Terraform enables you to safely and predictably create, change, and improve infrastructure. It is a source-available tool that codifies APIs into declarative configuration files that can be shared amongst team members, treated as code, edited, reviewed, and versioned.
https://www.terraform.io/
Other
42.47k stars 9.51k forks source link

Terraform crash on CDKTF generated import #35416

Closed bmendric closed 3 months ago

bmendric commented 3 months ago

Terraform Version

Terraform v1.9.0
on linux_amd64
+ provider registry.terraform.io/hashicorp/vault v4.3.0

Terraform Configuration Files

{
  "//": {
    "metadata": {
      "backend": "local",
      "importsGeneratingConfiguration": [
        "minimal-vault_minimalLDAP_ldap_D4B205B5",
        "minimal-vault_minimalLDAP_admins_500762DE"
      ],
      "stackName": "vault-unit",
      "version": "0.20.8"
    },
    "outputs": {
    }
  },
  "import": [
    {
      "id": "minimal-ldap",
      "to": "vault_ldap_auth_backend.minimal-vault_minimalLDAP_ldap_D4B205B5"
    },
    {
      "id": "auth/${vault_ldap_auth_backend.minimal-vault_minimalLDAP_ldap_D4B205B5.path}/groups/${vault_ldap_auth_backend_group.minimal-vault_minimalLDAP_admins_500762DE.groupname}",
      "to": "vault_ldap_auth_backend_group.minimal-vault_minimalLDAP_admins_500762DE"
    }
  ],
  "provider": {
    "vault": [
      {
        "address": "http://localhost:8200"
      }
    ]
  },
  "resource": {
    "vault_ldap_auth_backend": {
      "minimal-vault_minimalLDAP_ldap_D4B205B5": {
        "//": {
          "metadata": {
            "path": "vault-unit/minimal-vault/minimalLDAP/ldap",
            "uniqueId": "minimal-vault_minimalLDAP_ldap_D4B205B5"
          }
        },
        "binddn": "CN=Service Bind hshvlt,OU=ServiceAccounts,OU=Core,DC=Test,DC=Lab",
        "bindpass": "hunter2",
        "description": "(minimal) LDAP authentication for TestLab",
        "groupdn": "DC=Test,DC=Lab",
        "groupfilter": "(&(objectClass=group)(member:1.2.840.113556.1.4.1941:={{.UserDN}}))",
        "path": "minimal-ldap",
        "starttls": true,
        "tls_min_version": "tls12",
        "token_max_ttl": 14400,
        "token_no_default_policy": true,
        "upndomain": "test.lab",
        "url": "ldap://test.lab",
        "userattr": "sAMAccountName",
        "userdn": "OU=Admins,OU=Tier0,DC=Test,DC=Lab"
      }
    },
    "vault_ldap_auth_backend_group": {
      "minimal-vault_minimalLDAP_admins_500762DE": {
        "//": {
          "metadata": {
            "path": "vault-unit/minimal-vault/minimalLDAP/admins",
            "uniqueId": "minimal-vault_minimalLDAP_admins_500762DE"
          }
        },
        "backend": "${vault_ldap_auth_backend.minimal-vault_minimalLDAP_ldap_D4B205B5.path}",
        "groupname": "secret-admins",
        "policies": [
          "minimal-admin"
        ]
      }
    }
  },
  "terraform": {
    "backend": {
      "local": {
      }
    },
    "required_providers": {
      "vault": {
        "source": "hashicorp/vault",
        "version": "4.3.0"
      }
    }
  }
}

Debug Output

https://gist.github.com/bmendric/72014ddfda85ecf6ab55a5ed56ba224c

Expected Behavior

No crash

Actual Behavior

It crashed

Steps to Reproduce

  1. terraform init
  2. terraform apply

For completeness, linked terraform logs generated with TF_LOG=trace terraform apply -no-color 2>&1 | tee trace.log

Additional Context

This is a minimal example to reproduce the error. Removing the import line for the LDAP group does not cause a crash. Actual contents of the attached manifest are generated via CDKTF.

I also tested this with 1.8.5 and observed the same behavior

References

No response

liamcervante commented 3 months ago

Hi @bmendric, thanks for filing this.

The reason for the crash is that your id attribute is self referencing the item you want to import:

    {
      "id": "auth/${vault_ldap_auth_backend.minimal-vault_minimalLDAP_ldap_D4B205B5.path}/groups/${vault_ldap_auth_backend_group.minimal-vault_minimalLDAP_admins_500762DE.groupname}",
      "to": "vault_ldap_auth_backend_group.minimal-vault_minimalLDAP_admins_500762DE"
    }

Your reference to vault_ldap_auth_backend_group.minimal-vault_minimalLDAP_admins_500762DE.groupname in the id field is not valid. You can't refer to the to-be-imported value while working out the id for that value as Terraform can't know what the value for that reference will be until after the import has completed, and the import can't complete until it knows what the id should be.

However, Terraform definitely shouldn't crash in this situation. We should be able to produce a better error message that highlights the issue for you.

We'll get a fix in a for the crash, and in the meantime you should be able to make progress if you remove the self-reference from the id attribute.

liamcervante commented 3 months ago

On the technical side, the crash is occurring as we are trying to read from a resource that has yet to be processed in the Terraform graph. It hasn't been processed yet as the import operation is being processed before the resource, so the crash is somewhat expected.

We should extract the to attribute first, and then pass it as an argument to the evaluateImportIdExpression function. Inside the function we can then inspect the expression and make sure none of the references point to the resource to-be-imported.

bmendric commented 3 months ago

@liamcervante That makes a lot of sense to me; thank you for the information and quick turn around here. I've also gone ahead and opened a related issue on CDKTF to attempt to better validate the fields.

github-actions[bot] commented 2 months ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.