terraform-aws-modules / terraform-aws-vpn-gateway

Terraform module to create AWS VPN gateway resources 🇺🇦
https://registry.terraform.io/modules/terraform-aws-modules/vpn-gateway/aws
Apache License 2.0
111 stars 156 forks source link

fix: Set `vpn_connection_customer_gateway_configuration` output to sensitive #71

Closed lmpardey closed 2 years ago

lmpardey commented 2 years ago

Description

The module output vpn_connection_customer_gateway_configuration is set as sensitive.

Motivation and Context

Referencing the output vpn_connection_customer_gateway_configuration in another output results in an error message that this referenced value contains sensitive information and should be marked as sensitive.

Using the updated example minimal-vpn-gateway to reference the output results in the following error (before the PR):

$ terraform apply
╷
│ Error: Output refers to sensitive values
│
│   on outputs.tf line 36:
│   36: output "vpn_connection_customer_gateway_configuration" {
│
│ To reduce the risk of accidentally exporting sensitive data that was intended to be only
│ internal, Terraform requires that any root module output containing sensitive data be
│ explicitly marked as sensitive, to confirm your intent.
│
│ If you do intend to export this data, annotate the output value as sensitive by adding
│ the following argument:
│     sensitive = true

Following the advice shown by Terraform, this PR sets the output in the root module to sensitive.

Breaking Changes

None known.

How Has This Been Tested?

iamnicolasvdb commented 2 years ago

Maybe instead of setting this output to sensitive you could use the nonsensitive() to mimic the default behavior without hiding the output as sensitive. https://www.terraform.io/language/functions/nonsensitive

lmpardey commented 2 years ago

@iamnicolasvdb it should be the choice of the implementer whether the sensitive value is displayed in the command-line output. I would not make it a default. Forcing the output as nonsensitive forces this choice on the implementer of the module.

The AWS provider may expose all the attributes of a resource as nonsensitive by default, but this does not mean it is a good idea. Terraform shows the warning for this attribute for a reason. The Terraform documentation recommends to mark any outputs which contain sensitive information as sensitive.

This PR makes the module comply with the requirements by Terraform that outputs containing sensitive data should use the sensitive function. This will let the root module be referenced in other modules or used in other tools that integrate Terraform without breaking, since it is not following the requirements of marking the output as sensitive.

github-actions[bot] commented 2 years ago

This PR has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this PR will be closed in 10 days

lmpardey commented 2 years ago

Still awaiting for review. Comment to keep the PR open.

github-actions[bot] commented 2 years ago

This PR has been automatically marked as stale because it has been open 30 days with no activity. Remove stale label or comment or this PR will be closed in 10 days

github-actions[bot] commented 2 years ago

This PR was automatically closed because of stale in 10 days

github-actions[bot] commented 1 year ago

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.