RoadieHQ / kubewise

Get Helm notifications in your team chat
Apache License 2.0
59 stars 6 forks source link

Output helm values diff #35

Open cep21 opened 4 years ago

cep21 commented 4 years ago

We often modify charts just by changing values. It would be useful to show a diff of the values from the current release to the previous one. For example, here it is for my kubewise helm chart from the CLI.

< helm get values kubewise-kubewise --revision 2 > /tmp/2
< helm get values kubewise-kubewise --revision 1 > /tmp/1
< diff /tmp/1 /tmp/2
6c6
<   channel: '#k8s'
---
>   channel: '#k8s-staging'
dtuite commented 4 years ago

Interesting. Slightly worried it might get very long, especially if the diff covered multiple files. There's obviously a small cost to having to scroll through too many pages of Slack (or whatever the tool is) messages.

There are ways to solve that problem of course. I've noticed that both Slack and Google Chat have a concept of threaded messages. A quick Google tells me that Microsoft Teams has this feature too. We could put the main notification in the channel and then add supplementary data in a thread on the message.

What are your thoughts on how this might work?

cep21 commented 4 years ago

Threads work fine. For our use cases, the NOTES of a helm chart are interesting when a chart is first installed, but not so helpful when a chart is updated. What people really want to see is how the helm values change (and the helm chart version itself). Usually helm values change very little for our case

cep21 commented 4 years ago

Slightly worried it might get very long, especially if the diff covered multiple files

To clarify, I'm talking about helm get values, not the entire template (helm get all)

dtuite commented 4 years ago

Is sensitive data in the values a concern? There would very likely be Slack API tokens in there for example. You might not want that broadcasted to anyone who was in your Slack channel.

cep21 commented 4 years ago

We follow a best practice of never putting sensitive values in helm charts. They are usually secrets that are created with either https://github.com/godaddy/kubernetes-external-secrets or https://github.com/bitnami-labs/sealed-secrets

dtuite commented 4 years ago

Hey @cep21. I made some progress on this. Here's how it looks in Slack.

Screenshot 2020-04-09 at 22 05 30

Let me know what you think please.

BTW, would you mind emailing me at github@davidtuite.com please?

cep21 commented 4 years ago

Looks awesome! I'm guessing it's configurable somehow?

dtuite commented 4 years ago

I was going to put it behind a flag like --set showValuesDiff=true and default it to false because I do want to continue to support teams who are not confident that secrets won't end up in the diff.

What other configuration do you have in mind?

The example I have shown uses the https://github.com/pmezard/go-difflib library to render the diff. Unfortunately that package is not maintained and hasn't been updated in 4 years. I couldn't find anything else which had that nice +- diff with separated lines.

cep21 commented 4 years ago

A parameter with default false sounds reasonable.

I'm unsure if this is too much configuration, but this would be perfect for us except I just realized that the datadog helm chart requires you to embed the postgres secret into the configuration, so I would want some way to disable it for specific charts, or enable it for others.

Two ways would be some parameter of chart names to not show diffs for, or maybe a k8s metadata I can attach to the chart somehow.

Another option would be a no-op helm value of "kubewise-meta: supress-diff" that is seen by kubewise. I wonder if this works as a general way to configure kubewise inside helm charts.

dtuite commented 4 years ago

The ability to output a diff of the values on upgrade or rollback is now released in helm chart version 0.11.1. It is disabled by default. To enable it: --set chartValuesDiff.enabled=true.

There is no ability to blacklist packages yet. That can be added in a separate PR on this same issue.

Implementation wise, I'm leaning towards a blacklist rather than a whitelist. There are k8s users with hundreds of applications running in their clusters, a whitelist would be difficult to maintain and the feature would end up not being used.

With a blacklist, the likelihood of accidentally leaking an API key or something goes up but

  1. I would hope that this is caught in the dev clusters where there should be different keys to prod.
  2. The likelihood of an attacker finding the API key in your Slack (or whatever) and using it against your clusters is low.
  3. The key can be rotated.

Open to further thoughts if you have any?

TJM commented 4 years ago

blacklist sounds good, but realistically, secrets shouldn't be in values (not saying that we comply with that yet) ;)

dtuite commented 4 years ago

Apologies for going quiet on this. I've been busy with other things and COVID isn't helping. I'll get back to it but it will take some time.