hashicorp / terraform-provider-vault

Terraform Vault provider
https://www.terraform.io/docs/providers/vault/
Mozilla Public License 2.0
467 stars 542 forks source link

[Bug]: vault_ldap_auth_backend resources imported without namespace #2329

Closed crusstu closed 1 month ago

crusstu commented 2 months ago

Terraform Core Version

1.9.5

Terraform Vault Provider Version

4.2.0

Vault Server Version

1.16.6+ent

Affected Resource(s)

Expected Behavior

Importing a namespaced LDAP auth method should set the namespace attribute for the resource.

Provided a matching resource configuration, post-import terraform apply commands should indicate no changes are to be applied, and should definitely not force-recreate the LDAP auth method.

Actual Behavior

Namespaced LDAP auth methods are imported without a namespace attribute, so subsequent terraform apply commands force-recreate the LDAP auth method as changing the namespace attribute requires recreation of the resource.

Relevant Error/Panic Output Snippet

No response

Terraform Configuration Files

terraform {
  required_providers {
    vault = {
      source  = "hashicorp/vault"
      version = "~>4.2.0"
    }
  }
}

provider "vault" {
    # namespace = ... # could be set via VAULT_NAMESPACE
}

resource "vault_namespace" "namespace" {
  path            = "testing"
}

resource "vault_ldap_auth_backend" "ldap" {
  namespace       = resource.vault_namespace.namespace.path
  path            = "ldap"
  url             = "ldap://127.0.0.1"
  insecure_tls    = true
  starttls        = false
  binddn          = "..."
  bindpass        = "..."
  userdn          = "..."
  userattr        = "..."
  groupattr       = "..."
  groupdn         = "..."
  groupfilter     = "..."
}

Steps to Reproduce

  1. terraform init
  2. terraform apply
  3. terraform state rm 'vault_ldap_auth_backend.ldap'
  4. terraform import 'vault_ldap_auth_backend.ldap' 'ldap'
  5. Import should be successful
  6. terraform plan
  7. Note in the plan that Terraform will try to recreate the LDAP auth method because the namespace attribute has changed:
  # vault_ldap_auth_backend.ldap must be replaced
-/+ resource "vault_ldap_auth_backend" "ldap" {
      ~ accessor                = "auth_ldap_xxxxxxxx" -> (known after apply)
      + bindpass                = (sensitive value)
      ~ case_sensitive_names    = false -> (known after apply)
      + certificate             = (known after apply)
      + client_tls_cert         = (known after apply)
      + client_tls_key          = (sensitive value)
      ~ deny_null_bind          = true -> (known after apply)
      + description             = (known after apply)
      ~ discoverdn              = false -> (known after apply)
      ~ id                      = "ldap" -> (known after apply)
      + namespace               = "testing" # forces replacement
      + upndomain               = (known after apply)
      ~ use_token_groups        = false -> (known after apply)
      + userfilter              = (known after apply)
      ~ username_as_alias       = false -> (known after apply)
        # (24 unchanged attributes hidden)
    }

Debug Output

No response

Panic Output

No response

Important Factoids

No response

References

When namespace support was introduced in #1479 it doesn't seem like any of the import machinery was re-evaluated. The implementation for vault_ldap_auth_backend relies on schema.ImportStatePassthroughContext:

https://github.com/hashicorp/terraform-provider-vault/blob/9af82cbd8aa395363f7cdf711c06f4d6f2818d7d/vault/resource_ldap_auth_backend.go#L214-L216

where the doc comment says

This should be used only in the case that an ID-only refresh is possible.

Since LDAP auth methods (actually, any auth method) is unaware of its containing namespace, I don't believe ID-only refreshes are possible for namespaced auth methods.

This is the same issue raised in #2030, so I believe all auth methods are likewise impacted.

Would you like to implement a fix?

None

crusstu commented 1 month ago

While investigating this further, the correct behaviour is achieved by the use of the TERRAFORM_VAULT_NAMESPACE_IMPORT environment variable during import operations as documented in the provider: https://github.com/hashicorp/terraform-provider-vault/blob/0318b6b4523ef8a47a7b2ec8f120126166dad9ac/website/docs/index.html.markdown#namespace-support

I think I was expecting to use the VAULT_NAMESPACE environment variable instead of an additional var to configure the namespace property during import.

Important to note that the docs say

The import namespace will always be made relative to the namespace of the provider{} block.

but it's not just the provider block; if both VAULT_NAMESPACE and TERRAFORM_VAULT_NAMESPACE_IMPORT are set, TERRAFORM_VAULT_NAMESPACE_IMPORT will be relative to VAULT_NAMESPACE (which I think should be expected, but it's not specifically stipulated).