Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KMS Key Aliases are created but state is not always updated #7891

Closed
ghost opened this issue Mar 11, 2019 · 5 comments · Fixed by #7907
Closed

KMS Key Aliases are created but state is not always updated #7891

ghost opened this issue Mar 11, 2019 · 5 comments · Fixed by #7907
Labels
bug Addresses a defect in current functionality. service/kms Issues and PRs that pertain to the kms service.
Milestone

Comments

@ghost
Copy link

ghost commented Mar 11, 2019

This issue was originally opened by @tolidano as hashicorp/terraform#20640. It was migrated here as a result of the provider split. The original body of the issue is below.


Terraform Version

0.11.7

Terraform Configuration Files

data "aws_caller_identity" "current" {}

resource "aws_kms_key" "key" {
  description             = "${var.description}"
  policy                  = "${data.aws_iam_policy_document.key_policy.json}"
  deletion_window_in_days = "30"
  is_enabled              = "true"
  enable_key_rotation     = "true"

  tags {
    description = "${var.description}"
    cost        = "${var.env == "production" ? "production" : "non-production"}"
    name        = "${var.alias_name}"
    managed     = "Terraform"
  }
}

resource "aws_kms_alias" "key_alias" {
  name          = "alias/${var.alias_name}"
  target_key_id = "${aws_kms_key.key.id}"
}

Debug Output

N/A

Crash Output

N/A

Expected Behavior

When I apply the above, it should create a key and an alias and properly update the state in S3

Actual Behavior

The key and alias were created, but the state was only updated with the key. A subsequent plan against the same environment attempted to recreate the key alias, which resulted in an AWS exception stating the alias already existed (it did).

Steps to Reproduce

Over multiple environments, the first run (in QA) worked as expected, but the second run (in production) did not. The key and alias were created, but the state was only updated with the key. A subsequent plan against the same environment attempted to recreate the key alias, which resulted in an AWS exception stating the alias already existed (it did). Import is non-trivial, so I deleted the resource in AWS directly (aws kms delete-alias) and ran the apply again, and this time it worked properly (as it did in QA)

Additional Context

We are using Terragrunt, but that would not explain the erratic behavior.

References

None found.

@nywilken nywilken added the service/kms Issues and PRs that pertain to the kms service. label Mar 12, 2019
@bflad bflad added the bug Addresses a defect in current functionality. label Mar 13, 2019
@bflad
Copy link
Contributor

bflad commented Mar 13, 2019

Hi @tolidano 👋 Sorry for the strange behavior here.

If you enable Terraform debug logging, e.g. TF_LOG=debug terragrunt apply, do you see log messages like the following when this occurs?

[DEBUG] Removing KMS Alias (alias/XXXXXX) as it's already gone

We have seen sporadic failures like these occur in our daily acceptance testing as well and its due to a slight implementation issue in attempting to handle KMS eventual consistency. We can submit a fix for this in the near future.

bflad added a commit that referenced this issue Mar 13, 2019
… after creation due to eventual consistency

References:
* #7891
* #6560
* #7873
* hashicorp/terraform#17220

The KMS service has eventual consistency considerations and the `aws_kms_alias` resource immediately tries to read the KMS alias after creation, which may not find the KMS alias. When not able to find the KMS alias, the resource logic returns an empty API object instead of an error. Since a `nil` check was already performed on the error, the error will always be `nil`. Invoking `return resource.RetryableError(nil)`  is equivalent to `return nil`. The resource during its Read performs an error check first which will skip because its `nil`, then assumes the resource has been deleted outside Terraform and triggers recreation.

Here when we cannot find a KMS alias after allowing some time for eventual consistency, we return a resource not found error and ensure we handle any timeouts due to automatic AWS Go SDK retries.

Output from acceptance testing:

```
--- PASS: TestAccAWSKmsAlias_no_name (37.63s)
--- PASS: TestAccAWSKmsAlias_name_prefix (37.80s)
--- PASS: TestAccAWSKmsAlias_multiple (38.38s)
--- PASS: TestAccAWSKmsAlias_importBasic (40.13s)
--- PASS: TestAccAWSKmsAlias_ArnDiffSuppress (43.61s)
--- PASS: TestAccAWSKmsAlias_basic (46.76s)
```
@bflad
Copy link
Contributor

bflad commented Mar 13, 2019

Fix submitted: #7907

@nywilken
Copy link
Member

The fix has been merged and will release with version 2.2.0 of the Terraform AWS Provider, likely later today.

nywilken pushed a commit that referenced this issue Mar 14, 2019
… after creation due to eventual consistency (#7907)

References:
* #7891
* #6560
* #7873
* hashicorp/terraform#17220

The KMS service has eventual consistency considerations and the `aws_kms_alias` resource immediately tries to read the KMS alias after creation, which may not find the KMS alias. When not able to find the KMS alias, the resource logic returns an empty API object instead of an error. Since a `nil` check was already performed on the error, the error will always be `nil`. Invoking `return resource.RetryableError(nil)`  is equivalent to `return nil`. The resource during its Read performs an error check first which will skip because its `nil`, then assumes the resource has been deleted outside Terraform and triggers recreation.

Here when we cannot find a KMS alias after allowing some time for eventual consistency, we return a resource not found error and ensure we handle any timeouts due to automatic AWS Go SDK retries.

Output from acceptance testing:

```
--- PASS: TestAccAWSKmsAlias_no_name (37.63s)
--- PASS: TestAccAWSKmsAlias_name_prefix (37.80s)
--- PASS: TestAccAWSKmsAlias_multiple (38.38s)
--- PASS: TestAccAWSKmsAlias_importBasic (40.13s)
--- PASS: TestAccAWSKmsAlias_ArnDiffSuppress (43.61s)
--- PASS: TestAccAWSKmsAlias_basic (46.76s)
```
nywilken added a commit that referenced this issue Mar 14, 2019
@bflad bflad added this to the v2.2.0 milestone Mar 15, 2019
@bflad
Copy link
Contributor

bflad commented Mar 15, 2019

This has been released in version 2.2.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

@ghost
Copy link
Author

ghost commented Mar 31, 2020

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 feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Addresses a defect in current functionality. service/kms Issues and PRs that pertain to the kms service.
Projects
None yet
2 participants