terraform-provider-helm icon indicating copy to clipboard operation
terraform-provider-helm copied to clipboard

Provide richer diff of values changes

Open jcogilvie opened this issue 1 year ago • 33 comments

Description

When I have an existing helm_release with a set of values[], and the values change, often the values are only changing by a few lines. I would like to see a more narrow diff of the selected part of the values that have changed from my last apply, rather than the current view of "old values file replaced by new values file."

I'm not sure if this is feasible. But, if I have a long values file, the diff becomes meaningless if it just prints the 100 lines of old file as being replaced by 101 lines of new file, without any indicator of which line has changed.

Take this example diff given a small values file change. Can you immediately tell which line has changed?

Terraform will perform the following actions:

  # helm_release.datadog[0] will be updated in-place
!   resource "helm_release" "datadog" {
        id                         = "datadog"
        name                       = "datadog"
!       values                     = [
-           <<-EOT
                  dogstatsd:
                    originDetection: true
                    useHostPort: true
                    useHostPID: true
                    useSocketVolume: true
                    nonLocalTraffic: true
                    # unix domain socket
                    # keep this value in sync with helm-charts DD_DOGSTATSD_SOCKET (default is in flux)
                    socketPath: /var/run/datadog/statsd.sock
            EOT,
+           <<-EOT
                  dogstatsd:
                    originDetection: true
                    useHostPort: true
                    useHostPID: true
                    useSocketVolume: true
                    nonLocalTraffic: true
                    # unix domain socket
                    # keep this value in sync with helm-charts DD_DOGSTATSD_SOCKET (default is in flux)
                    socketPath: /var/run/datadog/statsd.sock
                    someAddedLine: foo  # for example
            EOT,
        ]
        # (25 unchanged attributes hidden)

        # (3 unchanged blocks hidden)
    }

Versus a more narrowly scoped change:

Terraform will perform the following actions:

  # helm_release.datadog[0] will be updated in-place
!   resource "helm_release" "datadog" {
        id                         = "datadog"
        name                       = "datadog"
!       values                     = [
!                 dogstatsd:
                    # unix domain socket
                    # keep this value in sync with helm-charts DD_DOGSTATSD_SOCKET (default is in flux)
                    socketPath: /var/run/datadog/statsd.sock
+                   someAddedLine: foo  # for example
        ]
        # (25 unchanged attributes hidden)

        # (3 unchanged blocks hidden)
    }

I know that 'smarter' diffs are possible in terraform, because I am able to see line by line diffs in helm manifests with helm_template if I set them as outputs. Maybe this could be achieved by changing values to a map internally with its map key as the index or something, to allow diffs between objects that may not be possible in the current list form.

Potential Terraform Configuration

Same as today.

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

jcogilvie avatar Apr 18 '23 16:04 jcogilvie

Hi @jcogilvie, can you explain why you defined values with a block of yaml instead of HCL?

sheneska avatar Apr 26 '23 14:04 sheneska

Sure I can.

First, the interface for values takes a list of strings. And using a file ref to load that string is the first thing shown in the documentation for the resource. So, that's how we started.

We did try HCL in a local in some cases. But helm charts come bundled with values as yaml, showing us a template of how to use the chart. That meant it was a context switch and a mental translation any time we wanted to include new values. We couldn't just copy and paste, and it was problematic having people context switch between yaml and HCL (and yaml is a little bit lighter). And then we'd just have to yamlencode the HCL before passing it into values anyway. So that's reason number 2.

We also separately tried to use a bunch of set blocks but that's a lot less legible, and meant changes could live in two places (in the values file or in the helm_release). This became more problematic when we started using a sister helm_template that's a duplicate of the helm_release (in order to get plan-time manifests as an output that can be diffed), as it meant we'd have to perform the set on two objects.

I hope this helps.

jcogilvie avatar Apr 26 '23 14:04 jcogilvie

Hi @jcogilvie, Thanks for sharing your use case! However the block of yaml is treated as plain text by terraform so there really isn't any way to break it down further.

sheneska avatar Apr 27 '23 16:04 sheneska

I could be off base here but I think I've seen this happen with lists of strings.

I think if there were an option for a singular values file, that might cause terraform to diff the value instead of treating it like a replacement of one string with another.

jcogilvie avatar Apr 27 '23 17:04 jcogilvie

I can confirm that a more robust diff behavior is possible if the provider were to expose a single-string variant of values that can accept a merged yaml instead of a list.

I know this from usage of the argocd_application in the argocd provider, which does exactly that: it accepts values as a singular string on the interface, and therefore does a nice clean diff of that string.

jcogilvie avatar May 22 '23 17:05 jcogilvie

As a intermediate solution i have created a small python script which converts the plan output to make it easier to read

Without script image

With script image

Download: https://gist.github.com/Kenterfie/a7ec9e50f17a749b8bb6469f21a6be4f

Maybe it will help others as well, as long as no solution exists

Kenterfie avatar Aug 01 '23 18:08 Kenterfie

i think a proper diff view is really neccessary here.

e.g. kube-prometheus-stack helm chart contains a approx 3000 line values file. changing one line of code outputs 6000 lines of "terraform plan" which is nearly impossible to understand/diff.

rd-michel avatar Aug 08 '23 11:08 rd-michel

In addition to the missing diff for Helm values, the provider also now started printing all metadata as invalidated now, since 2.10 or so. This makes reading the plan output for a large Helm chart very painful.

I mean the stuff that starts with this:

image

... and thousands of lines later ends with this:

image

towolf avatar Sep 13 '23 22:09 towolf

The diff has been an issue for this module for a while. Sometimes it shows the difference and sometimes it just prints the entire yaml twice like @rd-michel is saying.

I wrote a little thing like @Kenterfie to diff the helm_release resource using the tfplan file.

➜ terraform-diff helm_release.onepassword
Generating a diff of 'helm_release.onepassword' in '/var/folders/r1/xx/T/tmp.REIC8M5s'
INFO[0003] Executing hook: create_plugin_directory  prefix=[/Users/xxx/asdf/1password]
@@ -23,6 +23,13 @@
     - "hosts":
       - "1password.xxx.io"
       "secretName": "tls-io-xxx-1password"
+  "redis":
+    "master":
+      "resources":
+        "limits":
+          "cpu": 512
+        "requests":
+          "cpu": "250m"
   "service":
     "type": "ClusterIP"
function terraform-diff () {
    TEMP=$(mktemp -d)

    echo "Generating a diff of '$1' in '$TEMP'"

    terraform plan -out=$TEMP/tfplan >/dev/null
    terraform show -json $TEMP/tfplan | jq -r '.resource_changes[] | select(.address=="'"$1"'") | .change.before.values | add' > $TEMP/before.txt
    terraform show -json $TEMP/tfplan | jq -r '.resource_changes[] | select(.address=="'"$1"'") | .change.after.values | add' > $TEMP/after.txt

    diff -u --color=always $TEMP/before.txt $TEMP/after.txt | sed -e '1,2d'
}

rayjanoka avatar Sep 14 '23 15:09 rayjanoka

The metadata diffs is present in versions >= 2.10.0. I run a workaround by wrapping the values in sensitive so it doesn't display in the helm resource, then adding a null resource which prints the diffs more nicely. One can use a templatefile for this, or just jsonencode values directly.

terraform {
  required_providers {
    helm = {
      source  = "hashicorp/helm"
      version = "< 2.10.0"
    }
  }
}

resource "helm_release" "chart" {
  name       = var.chart_name
  namespace  = var.chart_namespace
  chart      = var.helm_chart
  repository = var.chart_repo
  version    = var.chart_version
  values     = [sensitive(local.chart_values)]
}

resource "null_resource" "helm_values" {
  triggers = {
    contents = local.helm_values
  }
}

locals {
  chart_values = templatefile(format("%s/values.yaml.tpl", path.module), {
    value1 = var.value1,
    value2 = "some-value",
  })
}

alyssahardy avatar Sep 14 '23 17:09 alyssahardy

Hey there. Was looking for that feature and looks like there is no reason to use terraform for helm at all. I'd switch to helmfile or whatever, because terraform's way to manage helm is not so good for complicated manifests.

Punkoivan avatar Sep 14 '23 18:09 Punkoivan

@Punkoivan I would usually prefer that if I didn't need to create additional resources such as buckets or keys, and I didn't need values that I obtain from Terraform.

alyssahardy avatar Sep 14 '23 19:09 alyssahardy

@alyssahardy you can try to create just a raw configmap within terraform and then use helm's "lookup" function to get the values.

Punkoivan avatar Sep 14 '23 20:09 Punkoivan

Hi. Coming from a helm world, where helm diff works very well, and even comparing with the normal plans available on terraform, having 10s and 100s of lines marked as diff (and in our case, not even being able to see the planned changes) does indeed making working with the kubernetes_helm provider very challenging... Workaround cost time and introduce variability in the processes, which is why it would be preferable that the diff produced more useful results.

joaocc avatar Sep 27 '23 08:09 joaocc

I agree with the benefits of using collections of yamls over hcl since it's more helm-native. been suffering from this diff issue for a while and would love to see it being properly addressed!

thx in advance!

duxing avatar Jan 06 '24 00:01 duxing

One alternative would be to provide a method to convert a data structure (in our case obtained from yamldecode) into keys/values that could be put in a dynamic set { block. This would also save people who don't use values files from having to deal with the headache of rewriting keys to use the \\. syntax.

eytanhanig avatar Jan 06 '24 00:01 eytanhanig

BTW, this seems to be better in TF 1.8.0.

towolf avatar Apr 15 '24 09:04 towolf

nothing changed for me with tf 1.8.0

example:

  • deploy kube-prometheus-stack helm chart
  • afterwards add a single new line to the helm values file
  • rollout again

result: ❯ terraform plan |wc -l 8372

8372 lines of diff ... minus some terraform description output

rd-michel avatar Apr 16 '24 14:04 rd-michel

@rd-michel which helm Provider version? Please try pinning down to Version 2.9.0

towolf avatar Apr 16 '24 15:04 towolf

I'm using Terraform 1.8.0 and version 2.13.0 of the provider, and there's definitely been an improvement. It's definitely still a lot of output though. I see a "minus" diff for the entire old set of values and then the entire new set of values with the actual proper diffs within it. It would be great if it just showed the proper diff with some context.

tanadeau avatar Apr 16 '24 15:04 tanadeau

With new versions of the provider you also get this problem:

https://github.com/hashicorp/terraform-provider-helm/issues/1315

I mentioned that earlier in this issue:

https://github.com/hashicorp/terraform-provider-helm/issues/1121#issuecomment-1718409101

towolf avatar Apr 16 '24 15:04 towolf

you can try enabling manifest from the helm provider. note it's experimental.

when it works, it's a big improvement. however, it has provided me with some weird errors that can only be bypassed by turning this off.

duxing avatar Apr 16 '24 17:04 duxing

@towolf we always use the latest version of the helm provider (2.13.1 atm). why is it necessary to downgrade to 2.9.0?

rd-michel avatar Apr 17 '24 10:04 rd-michel

@rd-michel see my previous comment. I refer to the metadata change introduced here: Always recompute metadata when a release is updated.

But I don't know exactly what you are complaining about. Too much clutter from metadata or too much context in the actual values diff?

towolf avatar Apr 17 '24 10:04 towolf

Thanks @towolf

Ive pretty much the same problem as described by @tanadeau

The displayed helm values context for large helm values files can be overwhelming during a terraform plan/apply

Perfect would be:

  • show me the actual diff + some context

rd-michel avatar Apr 17 '24 13:04 rd-michel

Ideally, I'd like similar output to helm diff -C3 upgrade when running an terraform plan. Unlike the solution from @rayjanoka, helm diff shows the difference in the actual cluster resources, rather than the values yaml.

alexgottscha avatar May 23 '24 16:05 alexgottscha