lgallard / terraform-aws-secrets-manager

Terraform module to create Amazon Secrets Manager resources.
Apache License 2.0
63 stars 50 forks source link

aws_secretsmanager_secrets_version fails if name_prefix is set #30

Closed Polpetta closed 11 months ago

Polpetta commented 1 year ago

Hi, I've noticed that with the latest release (0.6.2) the deployment fails if name_prefix is set.

To reproduce it just create a secrets map like that:

secrets = {
    license-database-creds = {
      name_prefix = "dev"
      description      = "test"
      secret_key_value = {
        username = "test"
        password = "test"
        engine = "test"
        host = "example.com"
        port = 42
        dbname = "test"
        dbInstanceIdentifier = "db_id"
      }
      recovery_window_in_days = 7
    }
  }

When deploying, you'll have the following error:

Terraform will perform the following actions:

  # aws_secretsmanager_secret.sm["license-database-creds"] will be created
  + resource "aws_secretsmanager_secret" "sm" {
      + arn                            = (known after apply)
      + description                    = "test"
      + force_overwrite_replica_secret = false
      + id                             = (known after apply)
      + name                           = (known after apply)
      + name_prefix                    = "dev"
      + policy                         = (known after apply)
      + recovery_window_in_days        = 7
      + rotation_enabled               = (known after apply)
      + rotation_lambda_arn            = (known after apply)
      + tags                           = {
          ~~REDACTED~~
        }
      + tags_all                       = {
          ~~REDACTED~~
        }

      + replica {
          + kms_key_id         = (known after apply)
          + last_accessed_date = (known after apply)
          + region             = (known after apply)
          + status             = (known after apply)
          + status_message     = (known after apply)
        }

      + rotation_rules {
          + automatically_after_days = (known after apply)
        }
    }

  # aws_secretsmanager_secret_version.sm-sv["license-database-creds"] will be created
  + resource "aws_secretsmanager_secret_version" "sm-sv" {
      + arn            = (known after apply)
      + id             = (known after apply)
      + secret_id      = "license-database-creds"
      + secret_string  = (sensitive value)
      + version_id     = (known after apply)
      + version_stages = (known after apply)
    }

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

Changes to Outputs:
  + rotate_secret_arns = {}
  + rotate_secret_ids  = {}
  + secret_arns        = {
      + license-database-creds = (known after apply)
    }
  + secret_ids         = {
      + license-database-creds = (known after apply)
    }
data.aws_iam_policy.vpc[0]: Still reading... [10s elapsed]
aws_secretsmanager_secret.sm["license-database-creds"]: Creating...
data.aws_iam_policy.vpc[0]: Read complete after 10s [id=arn:aws:iam::aws:policy/service-role/AWSLambdaENIManagementAccess]
aws_iam_policy.vpc[0]: Refreshing state... [id=arn:aws:iam::651768230362:policy/license-provider-dev-vpc]
aws_secretsmanager_secret.sm["license-database-creds"]: Creation complete after 0s [id=arn:aws:secretsmanager:eu-west-1:651768230362:secret:dev20230116142516958800000001-6EsApX]
aws_secretsmanager_secret_version.sm-sv["license-database-creds"]: Creating...
aws_iam_role_policy_attachment.vpc[0]: Refreshing state... [id=license-provider-dev-20230116093643617600000002]

Warning: Argument is deprecated

  with provider["registry.terraform.io/hashicorp/aws"],
  on main_providers.tf line 6, in provider "aws":
   6:   skip_get_ec2_platforms      = true

With the retirement of EC2-Classic the skip_get_ec2_platforms attribute has
been deprecated and will be removed in a future version.

(and 2 more similar warnings elsewhere)

Warning: Attribute Deprecated

  with provider["registry.terraform.io/hashicorp/aws"],
  on main_providers.tf line 6, in provider "aws":
   6:   skip_get_ec2_platforms      = true

With the retirement of EC2-Classic the skip_get_ec2_platforms attribute has
been deprecated and will be removed in a future version.

(and 2 more similar warnings elsewhere)

Error: error putting Secrets Manager Secret value: ResourceNotFoundException: Secrets Manager can't find the specified secret.

  with aws_secretsmanager_secret_version.sm-sv["license-database-creds"],
  on main.tf line 19, in resource "aws_secretsmanager_secret_version" "sm-sv":
  19: resource "aws_secretsmanager_secret_version" "sm-sv" {

I'm not sure, but giving a quick look a the terraform codebase I think the problem is in the following line, where the key is taken without keeping in consideration that the real id could be generated differently (such as in this case, where I have different stages and I'll need to have different ids even if the key will be the same...) https://github.com/lgallard/terraform-aws-secrets-manager/blob/37fa067663455528f7a7bfafa97dd3184d8e399b/main.tf#L21

I noticed the same behavior is done in the next resource block too: https://github.com/lgallard/terraform-aws-secrets-manager/blob/37fa067663455528f7a7bfafa97dd3184d8e399b/main.tf#L34

ThomasHemery commented 1 year ago

I have also experienced issues with this. Using name_prefix results in the error: "Error: putting Secrets Manager Secret value: ResourceNotFoundException: Secrets Manager can't find the specified secret." The secret manager is created with a name reflecting the prefix, but the values are not set as it appears to be attempting to set the values on the resource based only on the fixed name of the block, ignoring the prefix.

surendarrajasekaran commented 1 year ago
resource "aws_secretsmanager_secret_version" "sm-sv" {
  for_each      = { for k, v in var.secrets : k => v if !var.unmanaged }
  secret_id     =  aws_secretsmanager_secret.sm[each.key].id
  secret_string = lookup(each.value, "secret_string", null) != null ? lookup(each.value, "secret_string", null) : (lookup(each.value, "secret_key_value", null) != null ? jsonencode(lookup(each.value, "secret_key_value", {})) : null)
  secret_binary = lookup(each.value, "secret_binary", null) != null ? base64encode(lookup(each.value, "secret_binary")) : null
  depends_on    = [aws_secretsmanager_secret.sm]
  lifecycle {
    ignore_changes = [
       secret_id,
    ]
  }
}

secret_id value should be like I mentioned above, because each has to fetch from sm resources, instead of checking the parameter.

lgallard commented 11 months ago

@Polpetta @surendarrajasekaran latest release has this change. Closing this, but if the error persist feel free to open another issue or leave a comment here.