Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

vault_database_secret_backend_connection - allow mysql_rds,mysql_aurora,mysql_legacy to specifying tls_ca and tls_certificate_key #2106

Merged

Conversation

ram-parameswaran
Copy link
Contributor

@ram-parameswaran ram-parameswaran commented Dec 7, 2023

Description

Initially raised by(on behalf of) ent customer via Zendesk in an enterprise support engagement.

allow mysql_rds,mysql_aurora,mysql_legacy options of vault_database_secret_backend_connection terraform resource to allow specifying tls_ca and tls_certificate_key

Checklist

Output from acceptance testing:

resource "vault_database_secret_backend_connection" "mysql_aurora" {
  backend       = "database"
  name          = "mysql_aurora"
  allowed_roles = ["dev", "prod"]

  mysql_aurora {
    connection_url = "{{username}}:{{password}}@@tcp(ndh-hvr-mysqlauroratestdb-1.cluster-cmk3lyxo3cny.ap-southeast-2.rds.amazonaws.com:3306)/"
    tls_ca = "<cert_content_in_PEM>"
  }
}

resource "vault_database_secret_backend_connection" "mysql_rds" {
  backend       = "database"
  name          = "mysql_rds"
  allowed_roles = ["dev", "prod"]

  mysql_rds {
    connection_url = "{{username}}:{{password}}@@tcp(ndh-hvr-mysqlauroratestdb-1.cluster-cmk3lyxo3cny.ap-southeast-2.rds.amazonaws.com:3306)/"
    tls_ca = "<cert_content_in_PEM>"
  }
}
# Adding config read outputs from Vault

# mysql_aurora

`$ vault read database/config/mysql_aurora | jq -r .data
{
  "allowed_roles": [
    "dev",
    "prod"
  ],
  "connection_details": {
    "connection_url": "{{username}}:{{password}}@@tcp(ndh-hvr-mysqlauroratestdb-1.cluster-yyyy.ap-southeast-2.rds.amazonaws.com:3306)/",
    "max_open_connections": 2,
    "tls_ca": "<removed_for_sake_of_brevity>",
    "username": ""
  },
  "password_policy": "",
  "plugin_name": "mysql-aurora-database-plugin",
  "plugin_version": "",
  "root_credentials_rotate_statements": []
}`

# mysql_rds

`$ vault read database/config/mysql_rds | jq -r .data
{
  "allowed_roles": [
    "dev",
    "prod"
  ],
  "connection_details": {
    "connection_url": "{{username}}:{{password}}@@tcp(ndh-hvr-mysqlrdsdb-1.cluster-xxxxx.ap-southeast-2.rds.amazonaws.com:3306)/",
    "max_open_connections": 2,
    "tls_ca": "<removed_for_sake_of_brevity>",
    "username": ""
  },
  "password_policy": "",
  "plugin_name": "mysql-rds-database-plugin",
  "plugin_version": "",
  "root_credentials_rotate_statements": []
}`

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" comments, they generate extra noise for pull request followers and do not help prioritize the request

ram-parameswaran and others added 2 commits December 7, 2023 15:29
…cy options of vault_database_secret_backend_connection terraform resource to allow specifying tls_ca and tls_certificate_key
Add entry for - allow mysql_rds,mysql_aurora,mysql_legacy options of vault_database_secret_backend_connection terraform resource to allow specifying tls_ca and tls_certificate_key
@github-actions github-actions bot added the size/S label Dec 7, 2023
@ram-parameswaran ram-parameswaran changed the title mysql database secret engine: allow mysql_rds,mysql_aurora,mysql_legacy options of vault_database_secret_backend_connection terraform resource to allow specifying tls_ca and tls_certificate_key vault_database_secret_backend_connection - allow mysql_rds,mysql_aurora,mysql_legacy to specifying tls_ca and tls_certificate_key Dec 7, 2023
@ram-parameswaran ram-parameswaran self-assigned this Dec 7, 2023
@fairclothjm
Copy link
Contributor

Thanks for the contribution @ram-parameswaran ! Are there any tests we could add to verify this behavior?

@ram-parameswaran
Copy link
Contributor Author

@fairclothjm I have added acceptance test results back into the PR description. Let me know if anything further is needed. Thanks!

@vinay-gopalan
Copy link
Contributor

Hi @ram-parameswaran, thanks for adding the output from the Vault CLI. We would also like to validate that the tests within the Terraform Vault Provider pass with these updates. Could we also add an additional test step for MySQL Aurora to TestAccDatabaseSecretBackendConnection_mysql_tls on this line in the test file to confirm that the fields tls_ca and tls_certificate_key can be set to the resource via these configs? I think adding just the one test step for Aurora should suffice for RDS and Legacy as well, since the changes are the same. You will need a config function for the new test step, and we can add a testAccDatabaseSecretBackendConnectionConfig_mysql_aurora_tls function that is basically the same as the MySQL TLS config function in the same file.

Please let us know if you have any questions, and thanks once again for contributing the to Terraform Vault Provider!

@github-actions github-actions bot added size/M and removed size/S labels Dec 14, 2023
@ram-parameswaran
Copy link
Contributor Author

@vinay-gopalan thanks for your valuable comments. I have added the requested tests. Please review and let me know if you would like to me to change anything.

Remove unnecessary TODO
Copy link
Contributor

@vinay-gopalan vinay-gopalan left a comment

Choose a reason for hiding this comment

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

Thanks for adding the test @ram-parameswaran! I had some comments on why tests in CI might be failing, and how we can consolidate the new test into existing infrastructure

vault/resource_database_secret_backend_connection_test.go Outdated Show resolved Hide resolved
vault/resource_database_secret_backend_connection_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@vinay-gopalan vinay-gopalan left a comment

Choose a reason for hiding this comment

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

Looking great! Had 1 last fix request and then we should be ready to get this in! Thanks for all the work on this 🙏🏼

vault/resource_database_secret_backend_connection_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@vinay-gopalan vinay-gopalan left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for all the effort on this! 🙏🏼

@vinay-gopalan vinay-gopalan merged commit 0043a4e into main Jan 3, 2024
10 checks passed
@vinay-gopalan vinay-gopalan deleted the resource_database_secret_backend_connection_mysql_tls_fix branch January 3, 2024 23:00
@fairclothjm fairclothjm added this to the 3.24.0 milestone Jan 17, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants