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

Normalising YAML before diffing #257

Closed
sarahhodne opened this issue Jan 21, 2021 · 6 comments · Fixed by #273
Closed

Normalising YAML before diffing #257

sarahhodne opened this issue Jan 21, 2021 · 6 comments · Fixed by #273

Comments

@sarahhodne
Copy link

Sometimes when I upgrade Helm charts there's a change that doesn't really cause something to be changed, like an indentation change, or re-ordering the keys in a file. It would be nice if the YAML was "normalised" (ensure a consistent indentation level, sort all the keys by name) before diffing, to make it easier to see if there's actually a change being made.

@deiga
Copy link
Contributor

deiga commented Jan 22, 2021

Would that not cause a mismatch between what's actually in kube and what you are diffing with?

@lifelofranco
Copy link

This issue might be related.

@schollii
Copy link

schollii commented Feb 2, 2021

@deiga not if the values.yaml are semantically equivalent. Indentation, comments, blank lines, using or not using braces (recall yaml is superset of json so you can actually use braces to delimit blocks), and order of keys, do not change the actual configuration of the system. Renaming keys, and changing values of keys do change the configuration. Changing the order of items in a list, likely change it but there are cases where it will not (depends what the template code loop does).

@sarahhodne, it would be useful if you posted an example of 2 values files that you consider identical but lead helm diff to show differences.

@sarahhodne
Copy link
Author

The thing that made me create this ticket wasn't a change in the values file, but the chart we were using had changed (among a few other things) the indentation level, so a Deployment diff ended up something like this:

        containers:
-       - name: fluent-bit
+         - name: fluent-bit

There were also some key-reorderings, causing a diff like this:

+       volumes:
+         - name: config
+           configMap:
+             name: fluent-bit
        tolerations:
          - effect: NoSchedule
            key: pool-name
            operator: Exists
-       volumes:
-         - name: config
-           configMap:
-             name: fluent-bit

@deiga brings up a good point, but I think that's handled in #176 more than here, since helm-diff only diffs against Helm's internal state and not the actual live state (unlike Helm 3, which does a 3-way merge), this is already a problem orthogonal to this issue.

@stale
Copy link

stale bot commented Jun 2, 2021

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.

@stale stale bot added the stale label Jun 2, 2021
@davidholsgrove
Copy link

PR #273 if merged will close issue #257

@stale stale bot removed the stale label Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants