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

r/aws_rds_cluster_role_association - New resource #12370

Merged

Conversation

mantoine96
Copy link
Contributor

@mantoine96 mantoine96 commented Mar 12, 2020

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment 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 pull request followers and do not help prioritize the request

Closes #9552
Closes #13641

Release note for CHANGELOG:

* **New Resource**: `aws_db_cluster_role_association` (#12370)

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSDbClusterRoleAssociation_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -count 1 -parallel 20 -run=TestAccAWSDbClusterRoleAssociation_basic -timeout 120m
?       github.com/terraform-providers/terraform-provider-aws   [no test files]
=== RUN   TestAccAWSDbClusterRoleAssociation_basic
=== PAUSE TestAccAWSDbClusterRoleAssociation_basic
=== CONT  TestAccAWSDbClusterRoleAssociation_basic
--- FAIL: TestAccAWSDbClusterRoleAssociation_basic (145.77s)
    testing.go:654: Step 0 error: After applying this step and refreshing, the plan was not empty:
        
        DIFF:
        
        UPDATE: aws_rds_cluster.test
          arn:                                 "arn:aws:rds:us-west-2:335060465476:cluster:tf-acc-test-5141842218216975802" => "arn:aws:rds:us-west-2:335060465476:cluster:tf-acc-test-5141842218216975802"
          availability_zones.#:                "3" => "3"
          availability_zones.0:                "us-west-2a" => "us-west-2a"
          availability_zones.1:                "us-west-2b" => "us-west-2b"
          availability_zones.2:                "us-west-2c" => "us-west-2c"
          backtrack_window:                    "0" => "0"
          backup_retention_period:             "5" => "5"
          cluster_identifier:                  "tf-acc-test-5141842218216975802" => "tf-acc-test-5141842218216975802"
          cluster_members.#:                   "0" => "0"
          cluster_resource_id:                 "cluster-YPHXLKR7QXJN4VTFQQUM6HVETQ" => "cluster-YPHXLKR7QXJN4VTFQQUM6HVETQ"
          copy_tags_to_snapshot:               "false" => "false"
          database_name:                       "mydb" => "mydb"
          db_cluster_parameter_group_name:     "default.aurora-postgresql10" => "default.aurora-postgresql10"
          db_subnet_group_name:                "default" => "default"
          deletion_protection:                 "false" => "false"
          enable_http_endpoint:                "false" => "false"
          enabled_cloudwatch_logs_exports.#:   "0" => "0"
          endpoint:                            "tf-acc-test-5141842218216975802.cluster-cskifp8xdxvd.us-west-2.rds.amazonaws.com" => "tf-acc-test-5141842218216975802.cluster-cskifp8xdxvd.us-west-2.rds.amazonaws.com"
          engine:                              "aurora-postgresql" => "aurora-postgresql"
          engine_mode:                         "provisioned" => "provisioned"
          engine_version:                      "10.7" => "10.7"
          global_cluster_identifier:           "" => ""
          hosted_zone_id:                      "Z1PVIF0B656C1W" => "Z1PVIF0B656C1W"
          iam_database_authentication_enabled: "false" => "false"
          iam_roles.#:                         "1" => "0"
          iam_roles.0:                         "arn:aws:iam::335060465476:role/tf-acc-test-5141842218216975802" => ""
          id:                                  "tf-acc-test-5141842218216975802" => "tf-acc-test-5141842218216975802"
          kms_key_id:                          "" => ""
          master_password:                     "foobarfoobarfoobar" => "foobarfoobarfoobar"
          master_username:                     "foo" => "foo"
          port:                                "5432" => "5432"
          preferred_backup_window:             "07:00-09:00" => "07:00-09:00"
          preferred_maintenance_window:        "fri:13:12-fri:13:42" => "fri:13:12-fri:13:42"
          reader_endpoint:                     "tf-acc-test-5141842218216975802.cluster-ro-cskifp8xdxvd.us-west-2.rds.amazonaws.com" => "tf-acc-test-5141842218216975802.cluster-ro-cskifp8xdxvd.us-west-2.rds.amazonaws.com"
          replication_source_identifier:       "" => ""
          s3_import.#:                         "0" => "0"
          scaling_configuration.#:             "0" => "0"
          skip_final_snapshot:                 "true" => "true"
          storage_encrypted:                   "false" => "false"
          vpc_security_group_ids.#:            "1" => "1"
          vpc_security_group_ids.0:            "sg-01cc8f45" => "sg-01cc8f45"
        
        
        
        STATE:
        
        data.aws_iam_policy_document.rds_assume_role_policy:
          ID = 1077238154
          provider = provider.aws
          json = {
          "Version": "2012-10-17",
          "Statement": [
            {
              "Sid": "",
              "Effect": "Allow",
              "Action": "sts:AssumeRole",
              "Principal": {
                "Service": "rds.amazonaws.com"
              }
            }
          ]
        }
          statement.# = 1
          statement.0.actions.# = 1
          statement.0.actions.0 = sts:AssumeRole
          statement.0.effect = Allow
          statement.0.principals.# = 1
          statement.0.principals.0.identifiers.# = 1
          statement.0.principals.0.identifiers.0 = rds.amazonaws.com
          statement.0.principals.0.type = Service
          statement.0.sid = 
          version = 2012-10-17
        aws_db_cluster_role_association.test:
          ID = tf-acc-test-5141842218216975802,arn:aws:iam::335060465476:role/tf-acc-test-5141842218216975802
          provider = provider.aws
          db_cluster_identifier = tf-acc-test-5141842218216975802
          feature_name = s3Import
          role_arn = arn:aws:iam::335060465476:role/tf-acc-test-5141842218216975802
        
          Dependencies:
            aws_iam_role.test
            aws_rds_cluster.test
        aws_iam_role.test:
          ID = tf-acc-test-5141842218216975802
          provider = provider.aws
          arn = arn:aws:iam::335060465476:role/tf-acc-test-5141842218216975802
          assume_role_policy = {"Version":"2012-10-17","Statement":[{"Sid":"","Effect":"Allow","Principal":{"Service":"rds.amazonaws.com"},"Action":"sts:AssumeRole"}]}
          create_date = 2020-03-12T15:29:02Z
          description = 
          force_detach_policies = false
          max_session_duration = 3600
          name = tf-acc-test-5141842218216975802
          path = /
          unique_id = AROAU4AZI65CJXCNSES36
        
          Dependencies:
            data.aws_iam_policy_document.rds_assume_role_policy
        aws_rds_cluster.test:
          ID = tf-acc-test-5141842218216975802
          provider = provider.aws
          arn = arn:aws:rds:us-west-2:335060465476:cluster:tf-acc-test-5141842218216975802
          availability_zones.# = 3
          availability_zones.0 = us-west-2a
          availability_zones.1 = us-west-2b
          availability_zones.2 = us-west-2c
          backtrack_window = 0
          backup_retention_period = 5
          cluster_identifier = tf-acc-test-5141842218216975802
          cluster_resource_id = cluster-YPHXLKR7QXJN4VTFQQUM6HVETQ
          copy_tags_to_snapshot = false
          database_name = mydb
          db_cluster_parameter_group_name = default.aurora-postgresql10
          db_subnet_group_name = default
          deletion_protection = false
          enable_http_endpoint = false
          endpoint = tf-acc-test-5141842218216975802.cluster-cskifp8xdxvd.us-west-2.rds.amazonaws.com
          engine = aurora-postgresql
          engine_mode = provisioned
          engine_version = 10.7
          global_cluster_identifier = 
          hosted_zone_id = Z1PVIF0B656C1W
          iam_database_authentication_enabled = false
          iam_roles.# = 1
          iam_roles.0 = arn:aws:iam::335060465476:role/tf-acc-test-5141842218216975802
          kms_key_id = 
          master_password = foobarfoobarfoobar
          master_username = foo
          port = 5432
          preferred_backup_window = 07:00-09:00
          preferred_maintenance_window = fri:13:12-fri:13:42
          reader_endpoint = tf-acc-test-5141842218216975802.cluster-ro-cskifp8xdxvd.us-west-2.rds.amazonaws.com
          replication_source_identifier = 
          skip_final_snapshot = true
          storage_encrypted = false
          vpc_security_group_ids.# = 1
          vpc_security_group_ids.0 = sg-01cc8f45
    testing.go:715: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.
        
        Error: Check failed: DBClusterNotFoundFault: DBCluster tf-acc-test-5141842218216975802 not found.
                status code: 404, request id: b8033c62-46ec-4c15-972d-d779c0968fc1
        
        State: <no state>
FAIL
FAIL    github.com/terraform-providers/terraform-provider-aws/aws       147.225s
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws/internal/flatmap      0.079s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags 0.241s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws/internal/naming       0.551s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws/internal/service/batch/equivalency    0.213s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws/internal/service/eks/token    0.293s [no tests to run]
?       github.com/terraform-providers/terraform-provider-aws/awsproviderlint   [no test files]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes    0.300s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/AWSAT001   0.780s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/AWSR001    0.125s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/awsproviderlint/passes/fmtsprintfcallexpr 0.129s [no tests to run]
FAIL
make: *** [testacc] Error 1

There seems to a problem with the iam_roles argument in aws_rds_cluster, which can also be used to associate a role but only with Aurora MySQL (and doesn't allow passing a feature with the role) and I'd love to have feedback / advice on this.

I have two ideas on how to fix this, but both would be breaking changes:

  • Abandon this resource and modify iam_roles structure in aws_rds_cluster to be list(object({role_arn=string,feature=string})) which would allow for it to work on both MySQL and PostgreSQL.
    • Pro: No need for an additional resource
  • Keep this resource and delete iam_roles argument from aws_rds_cluster
    • Pro: Making the implementation similar to what is done for aws_db_instance and aws_db_instance_role_association

Additionally, I am not sure on the naming. Of course, Aurora clusters use the aws_rds_* namespace for most of their resources, but there are some using aws_db_cluster_*. Happy either way.

@mantoine96 mantoine96 requested a review from a team March 12, 2020 15:00
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. needs-triage Waiting for first response or review from a maintainer. service/rds Issues and PRs that pertain to the rds service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. labels Mar 12, 2020
@mantoine96 mantoine96 force-pushed the Add-db_cluster_role_association_resource branch 2 times, most recently from 33aa34c to ad113f4 Compare March 13, 2020 09:01
@mantoine96 mantoine96 force-pushed the Add-db_cluster_role_association_resource branch from ad113f4 to cafcf3e Compare April 8, 2020 05:56
@teamterraform

This comment has been minimized.

Base automatically changed from master to main January 23, 2021 00:57
@breathingdust breathingdust requested a review from a team as a code owner January 23, 2021 00:57
@shiron-babi
Copy link

@breathingdust - do you think you will be able to merge it somewhen soon? we really need this feature

@ewbankkit ewbankkit self-assigned this Jun 25, 2021
@ewbankkit ewbankkit removed the needs-triage Waiting for first response or review from a maintainer. label Jul 2, 2021
@ewbankkit ewbankkit force-pushed the Add-db_cluster_role_association_resource branch from cafcf3e to 39306ae Compare July 2, 2021 20:57
@github-actions github-actions bot added service/ec2 Issues and PRs that pertain to the ec2 service. service/transfer Issues and PRs that pertain to the transfer service. labels Jul 2, 2021
@ewbankkit ewbankkit force-pushed the Add-db_cluster_role_association_resource branch from a5894a5 to d90b347 Compare July 2, 2021 21:04
@github-actions github-actions bot removed service/ec2 Issues and PRs that pertain to the ec2 service. service/transfer Issues and PRs that pertain to the transfer service. provider Pertains to the provider itself, rather than any interaction with AWS. labels Jul 2, 2021
@ewbankkit ewbankkit force-pushed the Add-db_cluster_role_association_resource branch from d90b347 to b56d161 Compare July 6, 2021 13:37
@ewbankkit ewbankkit changed the title r/aws_db_cluster_role_association - New resource r/aws_rds_cluster_role_association - New resource Jul 6, 2021
@github-actions github-actions bot added the provider Pertains to the provider itself, rather than any interaction with AWS. label Jul 6, 2021
ewbankkit added 3 commits July 6, 2021 10:54
Note that RDS Cluster IAM Role Associations can now be managed via either the aws_rds_cluster.iam_roles attribute or the aws_rds_cluster_role_association resource, not both.
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀.

Commercial
% make testacc TEST=./aws TESTARGS='-run=TestAccAWSRDSClusterRoleAssociation_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSRDSClusterRoleAssociation_ -timeout 180m
=== RUN   TestAccAWSRDSClusterRoleAssociation_basic
=== PAUSE TestAccAWSRDSClusterRoleAssociation_basic
=== RUN   TestAccAWSRDSClusterRoleAssociation_disappears
=== PAUSE TestAccAWSRDSClusterRoleAssociation_disappears
=== RUN   TestAccAWSRDSClusterRoleAssociation_disappears_cluster
=== PAUSE TestAccAWSRDSClusterRoleAssociation_disappears_cluster
=== RUN   TestAccAWSRDSClusterRoleAssociation_disappears_role
=== PAUSE TestAccAWSRDSClusterRoleAssociation_disappears_role
=== CONT  TestAccAWSRDSClusterRoleAssociation_basic
=== CONT  TestAccAWSRDSClusterRoleAssociation_disappears_role
=== CONT  TestAccAWSRDSClusterRoleAssociation_disappears
=== CONT  TestAccAWSRDSClusterRoleAssociation_disappears_cluster
--- PASS: TestAccAWSRDSClusterRoleAssociation_disappears_cluster (131.22s)
--- PASS: TestAccAWSRDSClusterRoleAssociation_disappears (163.37s)
--- PASS: TestAccAWSRDSClusterRoleAssociation_disappears_role (163.77s)
--- PASS: TestAccAWSRDSClusterRoleAssociation_basic (165.71s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	168.798s
GovCloud
% make testacc TEST=./aws TESTARGS='-run=TestAccAWSRDSClusterRoleAssociation_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSRDSClusterRoleAssociation_ -timeout 180m
=== RUN   TestAccAWSRDSClusterRoleAssociation_basic
=== PAUSE TestAccAWSRDSClusterRoleAssociation_basic
=== RUN   TestAccAWSRDSClusterRoleAssociation_disappears
=== PAUSE TestAccAWSRDSClusterRoleAssociation_disappears
=== RUN   TestAccAWSRDSClusterRoleAssociation_disappears_cluster
=== PAUSE TestAccAWSRDSClusterRoleAssociation_disappears_cluster
=== RUN   TestAccAWSRDSClusterRoleAssociation_disappears_role
=== PAUSE TestAccAWSRDSClusterRoleAssociation_disappears_role
=== CONT  TestAccAWSRDSClusterRoleAssociation_basic
=== CONT  TestAccAWSRDSClusterRoleAssociation_disappears_role
=== CONT  TestAccAWSRDSClusterRoleAssociation_disappears
=== CONT  TestAccAWSRDSClusterRoleAssociation_disappears_cluster
--- PASS: TestAccAWSRDSClusterRoleAssociation_disappears (124.68s)
--- PASS: TestAccAWSRDSClusterRoleAssociation_disappears_cluster (135.43s)
--- PASS: TestAccAWSRDSClusterRoleAssociation_disappears_role (146.71s)
--- PASS: TestAccAWSRDSClusterRoleAssociation_basic (147.35s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	150.454s

@ewbankkit
Copy link
Contributor

@thehunt33r Thanks for the contribution 🎉 👏.
To get this merged more quickly, I went ahead and renamed the resource to aws_rds_cluster_role_association to match our new resource naming guidelines and added a couple more acceptance tests.

@mantoine96
Copy link
Contributor Author

Thank you @ewbankkit for all the work you've poured in this PR! I had indeed hesitated on the naming of the resource, and I also agree with aws_rds_cluster_role_association 👍

Looking forward to getting this PR merged and to start using it!

@ewbankkit ewbankkit merged commit 54d77ae into hashicorp:main Jul 6, 2021
@shiron-babi
Copy link

@ewbankkit & @thehunt33r - Thanks for the hard work, this will be very useful for us!

@github-actions
Copy link

github-actions bot commented Aug 6, 2021

I'm going to lock this pull request 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 related to this change, 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 Aug 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. provider Pertains to the provider itself, rather than any interaction with AWS. service/rds Issues and PRs that pertain to the rds service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
4 participants