resource/aws_rds_cluster: Add support for iam_roles to rds_cluster #1093
Conversation
stack72
added
the
enhancement
Fixes: #382 We store the state of the role that was added so that we can understand if the status of the role is `invalid` and thus an error has occurred ``` % make testacc TEST=./aws TESTARGS='-run=TestAccAWSRDSCluster_updateIamRoles' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -run=TestAccAWSRDSCluster_updateIamRoles -timeout 120m === RUN TestAccAWSRDSCluster_updateIamRoles --- PASS: TestAccAWSRDSCluster_updateIamRoles (190.73s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 190.769s ```
stack72
force-pushed
the
f-aws-rds-cluster-iam_roles-382
branch
from
83f11d3 to
02c6a22
Compare
| }, | ||
| "role_status": { | ||
| Type: schema.TypeString, | ||
| Computed: true, |
There was a problem hiding this comment.
Computed values in sets are a bit weird, since it's pretty hard to actually use them (no first-class interpolation syntax for set members) and it makes diffs confusing.
If interpolating the role status is important I'd suggest instead having a separate attribute iam_role_statuses that is a computed map of strings, and expose them in there with the role arn as the key. This way the syntax for interpolating them will be more convenient, and perhaps also the iam_roles attribute itself could be just a set of strings.
There was a problem hiding this comment.
@apparentlymart this is purely for information only - it's nothing to do with the actual diff. It just allows people to see the status of a role.
FYI, the feedback from the original PR was to add this - thus i've added it
There was a problem hiding this comment.
@stack72 did you mean this PR? hashicorp/terraform#10822 I can't find any suggestion around status.
I can't think of an example where this field would be useful - and if there's one, then Set will effectively prevent users from doing so due to indexes (i.e. I agree with @apparentlymart ).
There was a problem hiding this comment.
There was a problem hiding this comment.
@radeksimko / @apparentlymart back in hashicorp/terraform#10822 I essentially suggested this inclusion. It would seem I didn't do my homework enough and I now think I should not have.
I'll take your suggestion and rework this back to being a set of strings, and add another param to expose the statuses, if we're all on that page?
|
@catsby / @radeksimko / @apparentlymart / @grubernaut I am going to close this and reopen it from my fork with the correct changes |
Rework of hashicorp#1093 Fixes: hashicorp#382 ``` % make testacc TEST=./aws TESTARGS='-run=TestAccAWSRDSCluster_updateIamRoles' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -run=TestAccAWSRDSCluster_updateIamRoles -timeout 120m === RUN TestAccAWSRDSCluster_updateIamRoles --- PASS: TestAccAWSRDSCluster_updateIamRoles (170.35s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 170.374s ```
Rework of hashicorp#1093 Fixes: hashicorp#382 ``` % make testacc TEST=./aws TESTARGS='-run=TestAccAWSRDSCluster_updateIamRoles' ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./aws -v -run=TestAccAWSRDSCluster_updateIamRoles -timeout 120m === RUN TestAccAWSRDSCluster_updateIamRoles --- PASS: TestAccAWSRDSCluster_updateIamRoles (170.35s) PASS ok github.com/terraform-providers/terraform-provider-aws/aws 170.374s ```
|
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
Fixes: #382
We store the state of the role that was added so that we can understand
if the status of the role is
invalidand thus an error has occurred