the provider continues to reveal sensitive variables during destroy or update in-place

[cdtzabra]

Terraform, Provider, Kubernetes and Helm Versions

Terraform version: v1.6.0
Provider version: 2.11.0
Kubernetes version: 1.26.4

Affected Resource(s)

• helm_release

Terraform Configuration Files

• Deploying vsphere-csi chart from https://vsphere-tmm.github.io/helm-charts

resource "helm_release" "vsphere-csi" {
  name             = "csi"

  # https://github.com/vsphere-tmm/helm-charts/tree/master/charts/vsphere-csi
  # https://artifacthub.io/packages/helm/vsphere-tmm/vsphere-csi
  repository       = "https://vsphere-tmm.github.io/helm-charts"
  chart            = "vsphere-csi"
  version          = "3.2.3"
  namespace        = vsphere-csi
  create_namespace = true
  values = [
    templatefile(("${path.module}/files/vsphere-csi-values.yaml.tpl"),
      {
        vcenter_server      = var.vsphere_server
        vcenter_user        = var.vsphere_user   # sensitive variable
        vcenter_password    = var.vsphere_password  # sensitive variable
        vcenter_datacenters = yamlencode(var.vsphere_csi_datacenters)
      })
  ]
}

variable "vsphere_server" {
  type        = string
  description = "The PCC vsphere ID"
}

variable "vsphere_user" {
  type        = string
  description = "PCC user for authentication"
  sensitive   = true
}

variable "vsphere_password" {
  type        = string
  description = "PCC user password"
    sensitive = true
}

variable "vsphere_csi_datacenters" {
  type        = list(string)
  description = "PCC datacenters to use for CSI"
}

files/vsphere-csi-values.yaml.tpl looks like

global:
  config:
    storageClass: thin-csi
    storageclass:
      enabled: false
      expansion: true
      default: false
    vcenter:
      ${vcenter_server}:
          server: ${vcenter_server}
          user: ${vcenter_user}
          password: ${vcenter_password}
          datacenters:
            ${indent(12, vcenter_datacenters)}

controller:
  nodeSelector:
    node-role.kubernetes.io/control-plane: "true"
  tolerations: {}
  config:
    csi-migration: true
    csi-auth-check: true
    online-volume-extend: true
    trigger-csi-fullsync: false
    async-query-volume: true
    block-volume-snapshot: false # changed # to recheck
    csi-windows-support: false # changed
    use-csinode-id: true
    list-volumes: true
    pv-to-backingdiskobjectid-mapping: false
    cnsmgr-suspend-create-volume: true
    topology-preferential-datastores: false
    multi-vcenter-csi-topology: false # changed
    max-pvscsi-targets-per-vm: true
    csi-internal-generated-cluster-id: true
    listview-tasks: false

Debug Output

NOTE: In addition to Terraform debugging, please set HELM_DEBUG=1 to enable debugging info from helm.

Panic Output

terraform destroy or terraform plan --with some changes (update in place)

 ~ resource "helm_release" "vsphere-csi" {
        id                         = "csi"
      ~ metadata                   = [
          - {
              - app_version = "3.0.2"
              - chart       = "vsphere-csi"
              - name        = "csi"
              - namespace   = "infra-vmware-system-csi"
              - revision    = 2
              - values      = jsonencode(
                    {
                      - controller = {
                          - config       = {
                              - async-query-volume                = true
                              - block-volume-snapshot             = false
                              - cnsmgr-suspend-create-volume      = true
                              - csi-auth-check                    = true
                              - csi-internal-generated-cluster-id = true
                              - csi-migration                     = true
                              - csi-windows-support               = false
                              - list-volumes                      = true
                              - listview-tasks                    = false
                              - max-pvscsi-targets-per-vm         = true
                              - multi-vcenter-csi-topology        = false
                              - online-volume-extend              = true
                              - pv-to-backingdiskobjectid-mapping = false
                              - topology-preferential-datastores  = false
                              - trigger-csi-fullsync              = false
                              - use-csinode-id                    = true
                            }
                          - nodeSelector = {
                              - "node-role.kubernetes.io/control-plane" = "true"
                            }
                          - tolerations  = {}
                        }
                      - global     = {
                          - config = {
                              - storageClass = "thin-csi"
                              - storageclass = {
                                  - default   = false
                                  - enabled   = false
                                  - expansion = true
                                }
                              - vcenter      = {
                                  - "xxxxxx" = {
                                      - datacenters = [
                                          - "xxxxxxxx",
                                        ]
                                      - password    = SENSITIVE_VALUE_REVELEAD
                                      - server      = "xxxxx"
                                      - user        = SENSITIVE_VALUE_REVELEAD
                                    }
                                }
                            }
                        }
                    }
                )
              - version     = "3.2.3"
            },
        ] -> (known after apply)
        name                       = "csi"
      ~ values                     = [
          - (sensitive value),
          + (sensitive value),
        ]
        # (26 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

Expected Behavior

Sensitive variable should not be revelead in metatadata field

Issue already reported here: #793
With fix explanations from terraform

Actual Behavior

Sensitive data get display in metadata fields

More info https://github.com/hashicorp/terraform-provider-helm/issues/new?assignees=&labels=bug&projects=&template=bug-report.md&title=

[daniel-butler-irl]

It is my understanding that sensitive values should be passed with set_sensitive, not the standard values input... https://registry.terraform.io/providers/hashicorp/helm/latest/docs/resources/release#set_sensitive

[lorelei-rupp-imprivata]

@daniel-butler-irl interesting, I did not get that impression, but I will test that theory out if I get some time today

[daniel-butler-irl]

Not sure if it's supposed to work like that, but it's how I handle it. It would be better if it could respect the sensitive field...

[cdtzabra]

@daniel-butler-irl

The problem is that this field doesn't work with templatefile.
As far as I'm concerned, as soon as a variable is marked as sensitive, the provider should treat it as such everywhere.
set_sensitive isn't enough in my opinion, doesn't work when using file/templatefile.

Whereas it's very common to use one values file with Helm rather than a multitude of sets

[BBBmau]

Thank you for bringing this to our attention @cdtzabra, based off of the comment from a previous issue this is more due to how Terraform handles metadata information and thus treats it all as is regardless of whether it's sensitive or not.

The fix for this would be to implement a helm_release data source that still provides metadata info if needed instead of relying on the helm_release resource for it. This will be something to look into to solve this problem.

[rym-dd]

Hi @BBBmau

Do you know if this issue has been selected for development?

Thank you 🙏

[ianarsenault]

A potential hacky workaround to this is to add metadata to lifecycle ignore_changes, however this will spit out a warning indicating it's decided by the provider and not an argument set so it's redundant. Also unaware of any issues that may come from this if you're outputting anything from the metadata block , and then there's drift going unnoticed.

Before

# helm_release.release will be updated in-place
  ~ resource "helm_release" "release" {
        id                         = "my-chart"
      ~ metadata                   = [
          - {
              - chart       = "my-chart"
              ....
              - values      = jsonencode(
                    {
                      - config           = {
                          - password = <redacted>
                          - name   = "my-config"
                        }
                    }
                )
            },
        ] -> (known after apply)
        name                       = "my-chart-name"
      ~ values                     = [
          - (sensitive value),
          + (sensitive value),
        ]
        # (26 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.

After

  lifecycle {
    ignore_changes = [metadata]
  }

Terraform will perform the following actions:

  # helm_release.release will be updated in-place
  ~ resource "helm_release" "release" {
        id                         = "my-chart"
        name                       = "my-chart-name"
      ~ values                     = [
          - (sensitive value),
          + (sensitive value),
        ]
        # (27 unchanged attributes hidden)
    }

Plan: 0 to add, 1 to change, 0 to destroy.
╷
│ Warning: Redundant ignore_changes element
│
│   on ../../../my_helm_chart/0.1.0/main.tf line 6, in resource "helm_release" "release":
│    6: resource "helm_release" "release" {
│
│ Adding an attribute name to ignore_changes tells Terraform to ignore future changes to the argument in configuration after the object has been created, retaining the value originally configured.
│
│ The attribute metadata is decided by the provider alone and therefore there can be no configured value to compare with. Including this attribute in ignore_changes has no effect. Remove the attribute from
│ ignore_changes to quiet this warning.
╵