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_data_key_reuse_period_seconds in SQS is updated all the time #19786

Closed
fumantsu opened this issue Jun 14, 2021 · 9 comments · Fixed by #19834
Closed

kms_data_key_reuse_period_seconds in SQS is updated all the time #19786

fumantsu opened this issue Jun 14, 2021 · 9 comments · Fixed by #19834
Labels
bug Addresses a defect in current functionality. regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. service/sqs Issues and PRs that pertain to the sqs service.
Milestone

Comments

@fumantsu
Copy link

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform CLI and Terraform AWS Provider Version

Terraform v0.14.11
AWS provider: 3.45.0

Affected Resource(s)

*aws_sqs_queue

Terraform Configuration Files

Please include all Terraform configurations required to reproduce the bug. Bug reports without a functional reproduction may be closed without investigation.

resource "aws_sqs_queue" "this" {
  name        = "ELB-LB_S3_Queue_DLQ"
   id         = "https://sqs.eu-west-1.amazonaws.com/xxxxxx/ELB-LB_S3_Queue_DLQ"

  tags = {
    "Name"          = "ELB/LB Access Logs SQS DLQ Queue",
	"ResourceGroup" = "Access Logs Lambda",
	"terraform"     = "true"
  }
}

Expected Behavior

Either stay to 0 or update based on if encryption is enabled or not

Actual Behavior

Updating all time from 0 to 300 which is the default from the aws provider

Steps to Reproduce

Create a SQS without encryption and without value for the property

@github-actions github-actions bot added needs-triage Waiting for first response or review from a maintainer. service/sqs Issues and PRs that pertain to the sqs service. labels Jun 14, 2021
@ewbankkit ewbankkit added waiting-response Maintainers are waiting on response from community or contributor. and removed needs-triage Waiting for first response or review from a maintainer. labels Jun 14, 2021
@ewbankkit
Copy link
Contributor

@fumantsu Thanks for raising this issue.
Do you have a terraform plan or terraform apply output that shows the change to kms_data_key_reuse_period_seconds?

@grace-hunter
Copy link

I have the same issue. Here is a sample plan for a sqs queue without encryption:

# aws_sqs_queue.queue will be updated in-place
  ~ resource "aws_sqs_queue" "queue" {
    <...>
      ~ kms_data_key_reuse_period_seconds = 0 -> 300
    <...>
    }

The change persists even after running terraform apply.

@ewbankkit
Copy link
Contributor

@grace-instrumental Thanks for the quick response.
Are you able to add the corresponding Terraform code? I am having trouble reproducing this.

@fumantsu
Copy link
Author

I have exactly the same output message.
As for code, personally I'm using the terraform-aws-modules code in some kind of "local version" but especially the one below is very basic.

module "lb_access_logs_dlq" {
  source = "../../modules/sqs"

  name                              = "ELB-LB_S3_Queue_DLQ"
  create_policy                     = false
  tags = merge(
    {
      "Name"          = "ELB/LB Access Logs SQS DLQ Queue",
      "ResourceGroup" = "Access Logs Lambda"
    },
  var.globals.global_common_tags)
}

and the module is:

resource "aws_sqs_queue" "this" {
  count = var.create ? 1 : 0

  name        = var.name
  name_prefix = var.name_prefix

  visibility_timeout_seconds = var.visibility_timeout_seconds
  message_retention_seconds  = var.message_retention_seconds
  max_message_size           = var.max_message_size
  delay_seconds              = var.delay_seconds
  receive_wait_time_seconds  = var.receive_wait_time_seconds
  #   policy                      = var.policy
  redrive_policy              = var.redrive_policy
  fifo_queue                  = var.fifo_queue
  content_based_deduplication = var.content_based_deduplication

  kms_master_key_id                 = var.kms_master_key_id
  kms_data_key_reuse_period_seconds = var.kms_data_key_reuse_period_seconds

  tags = var.tags
}

The variables which are set for the kms:

variable "kms_master_key_id" {
  description = "The ID of an AWS-managed customer master key (CMK) for Amazon SQS or a custom CMK"
  type        = string
  default     = null
}

variable "kms_data_key_reuse_period_seconds" {
  description = "The length of time, in seconds, for which Amazon SQS can reuse a data key to encrypt or decrypt messages before calling AWS KMS again. An integer representing seconds, between 60 seconds (1 minute) and 86,400 seconds (24 hours)"
  type        = number
  default     = 300
}

Not quite sure if the 'null' is triggering something nasty so I doubt there is a connection between the module and the issue.

Interesting can be the state file:

resource "aws_sqs_queue" "this" {
    arn                               = "arn:aws:sqs:eu-west-1:xxxxxxx:ELB-LB_S3_Queue_DLQ"
    content_based_deduplication       = false
    delay_seconds                     = 0
    fifo_queue                        = false
    id                                = "https://sqs.eu-west-1.amazonaws.com/xxxxxxxxx/ELB-LB_S3_Queue_DLQ"
    kms_data_key_reuse_period_seconds = 0
    max_message_size                  = 262144
    message_retention_seconds         = 345600
    name                              = "ELB-LB_S3_Queue_DLQ"
    policy                            = jsonencode(
        {
            Id        = "example-ID"
            Statement = [
                {
                    Action    = "SQS:SendMessage"
                    Condition = {
                        ArnLike      = {
                            aws:SourceArn = "arn:aws:s3:::blabla-logs"
                        }
                        StringEquals = {
                            aws:SourceAccount = "xxxxxxxxx"
                        }
                    }
                    Effect    = "Allow"
                    Principal = {
                        AWS = "*"
                    }
                    Resource  = "arn:aws:sqs:eu-west-1:xxxxxxxxxxxxx:ELB-LB_S3_Queue_DLQ"
                    Sid       = "sendMessage"
                },
            ]
            Version   = "2012-10-17"
        }
    )
    receive_wait_time_seconds         = 0
    tags                              = {
        "Name"          = "ELB/LB Access Logs SQS DLQ Queue"
        "ResourceGroup" = "Access Logs Lambda"
        "owner"         = "blabla"
        "team"          = "blabla"
        "terraform"     = "true"
    }
    tags_all                          = {
        "Name"          = "ELB/LB Access Logs SQS DLQ Queue"
        "ResourceGroup" = "Access Logs Lambda"
        "owner"         = "blabla"
        "team"          = "blabla"
        "terraform"     = "true"
    }
    url                               = "https://sqs.eu-west-1.amazonaws.com/xxxxxxxxxxxxx/ELB-LB_S3_Queue_DLQ"
    visibility_timeout_seconds        = 30
}

It seems that it tries to set the number to 300 but in the end because the KMS is not enabled is end-up as 0

@tanasegabriel
Copy link

tanasegabriel commented Jun 16, 2021

This happens if you send any value to the kms_data_key_reuse_period_seconds argument in your queue/module definition while encryption is disabled. I imagine the provider always sends 300 (the default value) when this optional argument is not set by the user.

I have a hunch that something changed on the AWS side of things, where they always report back kms_data_key_reuse_period_seconds to be 0 for queues with encryption disabled, triggering changes during the refresh operation.

Setting the value of kms_data_key_reuse_period_seconds to null seems to be a valid workaround.

@ewbankkit ewbankkit added bug Addresses a defect in current functionality. regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. and removed waiting-response Maintainers are waiting on response from community or contributor. labels Jun 16, 2021
@ewbankkit
Copy link
Contributor

ewbankkit commented Jun 16, 2021

Yes, setting to null or 0 is a workaround.
It turns out AWS doesn't send back any value for the KmsDataKeyReusePeriodSeconds queue attribute if kms_master_key_id isn't set and there was a change to the default value handling in the AWS Provider (because the attribute does not have an explicit default value assigned in the schema).
Anyway, as this is a regression I will make the required code fixes and add acceptance test(s).

@fumantsu
Copy link
Author

fumantsu commented Jun 16, 2021 via email

@github-actions
Copy link

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

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2021
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. regression Pertains to a degraded workflow resulting from an upstream patch or internal enhancement. service/sqs Issues and PRs that pertain to the sqs service.
Projects
None yet
4 participants