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

helm_release metadata not cloaked when key contains "." character #737

Closed
camlow325 opened this issue Apr 24, 2021 · 5 comments
Closed

helm_release metadata not cloaked when key contains "." character #737

camlow325 opened this issue Apr 24, 2021 · 5 comments

Comments

@camlow325
Copy link

Terraform, Provider, Kubernetes and Helm Versions

Terraform version: 0.15.0, 0.14.0
Provider version: 2.1.1
Kubernetes version: 1.17.12

Affected Resource(s)

  • helm_release

Terraform Configuration Files

resource "helm_release" "example" {
  name = "redis"

  repository = "https://charts.bitnami.com/bitnami"
  chart      = "redis"

  set {
    name  = "replica.replicaCount"
    value = 0
  }

  set_sensitive {
    name  = "auth.password"
    value = "foo"
  }

  set_sensitive {
    name  = "foo\\.bar"
    value = "baz"
  }
}

Steps to Reproduce

  1. terraform apply
  2. terraform destroy

Expected Behavior

Apply succeeds. In the plan output for the destroy, the value for the foo.bar key under the metadata attribute is marked as sensitive:

  # helm_release.example will be destroyed
  - resource "helm_release" "example" {
...
      - metadata                   = [
          - {
... 
             - values      = jsonencode(
                    {
...
                      - foo.bar = "(sensitive value)"

Actual Behavior

Apply succeeds. In the plan output for the destroy, the value for the foo.bar key under the metadata attribute is shown in original plain text (not marked sensitive):

  # helm_release.example will be destroyed
  - resource "helm_release" "example" {
...
      - metadata                   = [
          - {
... 
             - values      = jsonencode(
                    {
...
                      - foo.bar = "baz"

Important Factoids

The value is also shown in original plain text in Terraform 0.14.x and earlier in the plan output ahead of an apply when the concise diff feature is disabled (TF_X_CONCISE_DIFF=0).

When the key does not have a "." character in it, (e.g., using name = "foo.bar" in the example above, the value is shown as "(sensitive value)", as expected. In our case, though we need to set a key with a "." in its name, due to how the underlying Helm chart has been designed. We use \\. to escape the "." character, in order to avoid having it be misinterpreted as a delimiter for a YAML "sub-map".

It doesn't appear that using the new sensitive function in Terraform 0.15 would help in this case since the sensitivity only applies to the set_sensitive block, not the metadata attribute.

References

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment
@pviniciusfm
Copy link

@camlow325 we are also facing this issue and I found that line 879 from the file resource_release.go is the culprit.

pathKeys := strings.Split(valuePath, ".")
sensitiveKey := pathKeys[len(pathKeys)-1]
parentPathKeys := pathKeys[:len(pathKeys)-1]

When this line was introduced I think it was not considering field names with backslashes. Thus, it is matching any field names with dots but we only should be matching only the dots that are not prefixed with backslashes "".

I considered using a regex split but Go regex implementation doesn't support the look backward matching.

So maybe we can write a custom tokenization algorithm to solve this issue.

I will make a PR with that and see what the maintainers say.

pviniciusfm pushed a commit to pviniciusfm/terraform-provider-helm that referenced this issue May 15, 2021
pviniciusfm added a commit to pviniciusfm/terraform-provider-helm that referenced this issue May 15, 2021
@pviniciusfm
Copy link

@camlow325 Does #746 fix your issue?

@camlow325
Copy link
Author

@pviniciusfm, your fix worked great for me. Thanks for doing this!

@github-actions
Copy link

Marking this issue as stale due to inactivity. If this issue receives no comments in the next 30 days it will automatically be closed. If this issue was automatically closed and you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. This helps our maintainers find and focus on the active issues. Maintainers may also remove the stale label at their discretion. Thank you!

@github-actions
Copy link

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 have found a problem that seems similar to this, 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 Oct 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants