databus23 / helm-diff

A helm plugin that shows a diff explaining what a helm upgrade would change
Apache License 2.0
2.65k stars 277 forks source link

Chart/version label noise on each chart upgrade #392

Closed danopia closed 7 months ago

danopia commented 2 years ago

I've been using helm-diff via helmfile for some time now to post a diff into a Github PR comment. I'd like to do something about the nuisance label change that shows up for every single resource when a chart version changes. I don't mind these labels changing on upgrade, and I feel other helm users view these same labels as noise too.

I didn't see any open discussions around reducing diff noise. I hope opening a new issue is acceptable. This is a pretty straightforward usecase at least.

For example, updating jetstack/cert-manager from 1.8.0 to 1.8.1. The only actual change is the image tag in the deployments. But the printed diff is huge and quickly exceeds Github's max comment size, basically repeating this section for all 48 resources:

$ helmfile [...] diff --context 5

[...]
cert-manager, cert-manager-cainjector, ClusterRole (rbac.authorization.k8s.io) has changed:
...
    labels:
      app: cainjector
      app.kubernetes.io/name: cainjector
      app.kubernetes.io/instance: cert-manager
      app.kubernetes.io/component: "cainjector"
-     app.kubernetes.io/version: "v1.8.0"
+     app.kubernetes.io/version: "v1.8.1"
      app.kubernetes.io/managed-by: Helm
-     helm.sh/chart: cert-manager-v1.8.0
+     helm.sh/chart: cert-manager-v1.8.1
  rules:
    - apiGroups: ["cert-manager.io"]
      resources: ["certificates"]
      verbs: ["get", "list", "watch"]
    - apiGroups: [""]
...
cert-manager, cert-manager-cainjector, ClusterRoleBinding (rbac.authorization.k8s.io) has changed:
...
    labels:
      app: cainjector
      app.kubernetes.io/name: cainjector
      app.kubernetes.io/instance: cert-manager
      app.kubernetes.io/component: "cainjector"
-     app.kubernetes.io/version: "v1.8.0"
+     app.kubernetes.io/version: "v1.8.1"
      app.kubernetes.io/managed-by: Helm
-     helm.sh/chart: cert-manager-v1.8.0
+     helm.sh/chart: cert-manager-v1.8.1
  roleRef:
    apiGroup: rbac.authorization.k8s.io
    kind: ClusterRole
    name: cert-manager-cainjector
  subjects:
[...]

Potentially related:

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

z0rc commented 1 year ago

There is still interest in this.

stale[bot] commented 1 year ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

z0rc commented 1 year ago

Not stale.

LatVAlY commented 1 year ago

Not stale.

tolleiv commented 10 months ago

This would be very valuable

yxxhero commented 7 months ago

hi. all. I will try to optimize code. what do you think of the value of the feature? @danopia @tolleiv @z0rc @LatVAlY @Sajfer

LatVAlY commented 7 months ago

I think as already explained above, reducing the noise on the diff, plus having a checksum by modifying a value the whole checksum would be showed as changed, which is logical but the diff would look so huge, I would say improving this without having to compromise the code quality.💭 as a thought :) Again thank you so much for the efforts!

yxxhero commented 7 months ago

@LatVAlY thanks for your advice. I will add features on top of the code quality prompts.

z0rc commented 7 months ago

For my case https://github.com/databus23/helm-diff/pull/475 was sufficient.

yxxhero commented 7 months ago

yeah. @z0rc it's ok.

tolleiv commented 6 months ago

Comming late for this one, to add some value - --suppress-output-line-regex "(app.kubernetes.io/version|chart):.*" got rid of all the noise so far. Thanks!