hashicorp / terraform-provider-helm

Terraform Helm provider
https://www.terraform.io/docs/providers/helm/
Mozilla Public License 2.0
989 stars 364 forks source link

Provide richer diff of values changes #1121

Open jcogilvie opened 1 year ago

jcogilvie commented 1 year ago

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

sheneska commented 1 year ago

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

jcogilvie commented 1 year ago

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.

sheneska commented 1 year ago

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.

jcogilvie commented 1 year ago

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 commented 1 year ago

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.

Kenterfie commented 11 months ago

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

rd-michel commented 11 months ago

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.

towolf commented 9 months ago

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

rayjanoka commented 9 months ago

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'
}
alyssahardy commented 9 months ago

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",
  })
}
Punkoivan commented 9 months ago

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.

alyssahardy commented 9 months ago

@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.

Punkoivan commented 9 months ago

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

joaocc commented 9 months ago

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.

duxing commented 6 months ago

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!

eytanhanig commented 6 months ago

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.

towolf commented 2 months ago

BTW, this seems to be better in TF 1.8.0.

rd-michel commented 2 months ago

nothing changed for me with tf 1.8.0

example:

result: ❯ terraform plan |wc -l 8372

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

towolf commented 2 months ago

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

tanadeau commented 2 months ago

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.

towolf commented 2 months ago

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

duxing commented 2 months ago

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.

rd-michel commented 2 months ago

@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?

towolf commented 2 months ago

@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?

rd-michel commented 2 months ago

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:

alexgottscha commented 1 month ago

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.