RDS Aurora Cross Region replication for encrypted clusters #2111
Conversation
radeksimko
added
the
enhancement
|
Any objections to merging this? This is really holding my work back as far as being able to deploy encrypted cross region RDS clusters. |
|
@radeksimko I believe there should also be a bug label on this since it was missing source_region causing it to fail when trying to deploy a cross region replication cluster that's encrypted. |
There was a problem hiding this comment.
Hi @mitchelldavis44
thanks for the PR.
In addition to what I mentioned in the code, do you mind adding an acceptance test or modifying and existing one to exercise this new field?
aws/resource_aws_rds_cluster.go
Outdated
| "source_region": { | ||
| Type: schema.TypeString, | ||
| Optional: true, | ||
| }, |
There was a problem hiding this comment.
I believe we should mark this as ConflictsWith: []string{"snapshot_identifier"} because this field will be ignored if the DB is restored, instead of created from scratch.
There was a problem hiding this comment.
Also it seems it's not updatable, so we should mark it as ForceNew: true.
radeksimko
added
the
waiting-response
mitchelldavis44
force-pushed
the
aurora_cross_region_encryption
branch
from
ab29929
to
c423a24
Compare
|
Hi @radeksimko I have made the changes you've requested, I've never added an acceptance test before but I tried my hand at adding one. Let me know if you need anything else, thanks! |
radeksimko
removed
the
waiting-response
|
@radeksimko is this ready to merge, or are there more changes needed? |
|
@mitchelldavis44 thanks for modifying the schema, I think we'll need a bit more testing there - i.e. a separate test which spins up an Aurora cluster with both encryption and |
radeksimko
added
waiting-response
mitchelldavis44
force-pushed
the
aurora_cross_region_encryption
branch
from
8b5cd56
to
7874ae7
Compare
|
@radeksimko I added a completely new test for cross region replicas that checks that |
radeksimko
removed
the
waiting-response
| CheckDestroy: testAccCheckAWSClusterDestroy, | ||
| Steps: []resource.TestStep{ | ||
| { | ||
| Config: testAccAWSClusterConfig(acctest.RandInt()), |
There was a problem hiding this comment.
I think we'll also need to create a config which sets up the cross-region replication, testAccAWSClusterConfig doesn't do it. Any minimalistic configuration will do in this context, but it should actually setup the replication and test this new behaviour/field - ideally we'd be checking the value of source_region, not just whether it's set.
There was a problem hiding this comment.
This is a little out of my comfort zone. Unfortunately I don't really have the experience to do what you're asking, I'm not very experienced in writing go.
There was a problem hiding this comment.
@radeksimko is there any way we can have this merged? I'm not the guy to make more complex tests, but this is definitely a needed flag in terraform.
radeksimko
added
the
waiting-response
|
It looks like there are 2 conflicting PR's solving the same issue - This one, and another #2808 by @dselvan. Both are requesting help with the test - although @dselvan has an almost working Terraform test setup, it doesn't work with both providers (which is necessary to deploy to both east and west for cross-region replication). Is there a different way this test can be composed - is it somehow related to I have tried searching the tests to find an example of doing something similar (2 providers while calling |
|
This is getting kinda ridiculous, is there someone that can help with the tests or at the very least a timetable on when you guys will release this? The code I have in this pull request works, for both east and west and we've been using our "own" provider since this hasn't been merged. Again, cross region replication works completely with this pull request, any assistance on the test would be greatly appreciated. |
|
This has been released in version 1.10.0 of the AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. |
|
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
locked and limited conversation to collaborators
breathingdust
removed
the
waiting-response
This fixes issue RDS Aurora Cross-Region replication for encrypted cluster failing #630. Issue was it was missing source_region within the module which was making it fail when trying to create a cross region replication cluster that was encrypted with the errror "aws_rds_cluster.replica: InvalidParameterCombination: Source cluster arn:aws:rds:us-east-1::cluster:<cluster_name> is encrypted; pre-signed URL has to be specified status code: 400, request id: b59ab0b3-812b-11e7-8467-c192637e53bf"