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

Multi Region KMS Key Replicas don't allow Replication Region to be configured #23964

Closed
ryancormack opened this issue Mar 31, 2022 · 3 comments
Closed
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/kms Issues and PRs that pertain to the kms service.

Comments

@ryancormack
Copy link
Contributor

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

Description

The Multi Region KMS key resource (https://github.com/hashicorp/terraform-provider-aws/blob/main/internal/service/kms/replica_key.go#L33) doesn't allow a Replica Region to be passed into it, instead using the Region set in the provide https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/kms_replica_key and https://github.com/hashicorp/terraform-provider-aws/blob/main/internal/service/kms/replica_key.go#L108.
This means that for each Replica you'd like to create, you need an extra provider that the replica resource can reference, where the difference is only the Region set in the provider. This can make the Terraform messy and overly complex.

I'd like to propose allowing a replica_region to be exposed in the module allowing this to be set without having to create additional provider blocks.

I'd propose to read the value passed in and then fall back to the value from the client/provider block as is current behaviour making this new argument an optional one.

If this seems like an appropriate approach, I'm happy to do the work needed.

New or Affected Resource(s)

Potential Terraform Configuration

Example from https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/kms_replica_key

provider "aws" {

  #alias  = "primary" //no longer needed
  region = "us-east-1"
}

# note this would now no longer be needed
#provider "aws" {
#  region = "us-west-2"
#}

resource "aws_kms_key" "primary" {
  #provider = aws.primary //this would no longer be needed as this block would default to the only provider

  description             = "Multi-Region primary key"
  deletion_window_in_days = 30
  multi_region            = true
}

resource "aws_kms_replica_key" "replica-europe" {
  description             = "Multi-Region replica key"
  deletion_window_in_days = 7
  primary_key_arn         = aws_kms_key.primary.arn
  replica_region          = "eu-west-1"
}

resource "aws_kms_replica_key" "replica-anz" {
  description             = "Multi-Region replica key"
  deletion_window_in_days = 7
  primary_key_arn         = aws_kms_key.primary.arn
  replica_region          = "ap-southeast-2"
}

References

  • #0000
@ryancormack ryancormack added the enhancement Requests to existing resources that expand the functionality or scope. label Mar 31, 2022
@github-actions github-actions bot added needs-triage Waiting for first response or review from a maintainer. service/kms Issues and PRs that pertain to the kms service. labels Mar 31, 2022
@ryancormack
Copy link
Contributor Author

I've noticed that a similar approach has been taken with the S3 bucket replication, https://registry.terraform.io/providers/hashicorp/aws/latest/docs/resources/s3_bucket_replication_configuration

That makes me think this is a deliberate design choice, rather than something that has organically grown - in which case I can close this issue and this would deviate from that. But it would be interesting to understand the reasoning behind that choice.

Thanks

@ewbankkit
Copy link
Contributor

Relates: #19896.

@ryancormack Thanks for raising this issue.
You are correct in that choice being a design decision. There may well be value in adding something like a separate aws_kms_multi_region_key resource which would manage the primary and all replicas via a single Terraform resource instance.
Please feel free to open an Issue for that if appropriate.

@ewbankkit ewbankkit removed the needs-triage Waiting for first response or review from a maintainer. label Mar 31, 2022
@github-actions
Copy link

github-actions bot commented May 5, 2022

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 May 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Requests to existing resources that expand the functionality or scope. service/kms Issues and PRs that pertain to the kms service.
Projects
None yet
Development

No branches or pull requests

2 participants