You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
* typo: Fix a minor typo
* Print error messages to stderr, not stdout
Messages that help the reader make sense of the output are printed to stdout and not to stderr right
now. When using the structured output options (such as `--output json`), this causes the output to
not be valid JSON. It requires the user to filter out the non-JSON lines from the output and then
pipe the output to programs that expect valid JSON input (such as [`jq`])
We are using the Helm diff plugin when deploying to our production environments, and we need to
[filter] these lines out from the output. The logic that we have implemented is fragile and we would
have to keep it in sync with changes in the Helm Diff plugin itself.
A better way to do this would be to print the messages for the reader to standard error, and keep
the standard output purely output that can be piped into another program without any changes. I
believe that this is a common pattern for tools that create structured output.
This PR updates the messages that are printed in the helm-diff upgrade command. I have included the
before and after outputs related to this change. One concern with this change is if plugin users are
using this line in their scripts to detect something (for example: the line `Release was not
present` may be used to detect that a new release will be installed into the cluster) If this change
is accepted, it would be good to add an entry in the changelog or release message about this change
and how it might affect plugin users.
In order to output the commit that was built in the binary, I used the following command:
```sh
$ go build -v -o bin/diff -ldflags="-X github.com/databus23/helm-diff/v3/cmd.Version=$(git rev-parse HEAD)"
```
\### Before
```sh
$ helm diff version
05264b7
$ HELM_DIFF_THREE_WAY_MERGE="true" HELM_DIFF_NORMALIZE_MANIFESTS="true" helm diff upgrade --output json --allow-unreleased wait-for-duration ./ | jq .
jq: parse error: Invalid numeric literal at line 1, column 8
Error: plugin "diff" exited with error
```
\### After
Once the messages are moved to `stderr`, the output can be parsed properly by `jq`. It is also
possible to pipe these messages to `/dev/null`, if we want to see only the structured output.
```sh
$ helm diff version
447393a3f61b0c7747fa284ef73de91d0ac216bf
$ HELM_DIFF_THREE_WAY_MERGE="true" HELM_DIFF_NORMALIZE_MANIFESTS="true" helm diff upgrade --output json --allow-unreleased wait-for-duration ./ | jq .
Enabled three way merge via the envvar
Enabled normalize manifests via the envvar
********************
Release was not present in Helm. Diff will show entire contents as new.
********************
[
{
"api": "batch",
"kind": "Job",
"namespace": "default",
"name": "sample-job-testing-blocking",
"change": "ADD"
}
]
$ HELM_DIFF_THREE_WAY_MERGE="true" HELM_DIFF_NORMALIZE_MANIFESTS="true" helm diff upgrade --output json --allow-unreleased wait-for-duration ./ 2>/dev/null | jq .
[
{
"api": "batch",
"kind": "Job",
"namespace": "default",
"name": "sample-job-testing-blocking",
"change": "ADD"
}
]
```
[`jq`]: https://jqlang.org/
[filter]: https://gitlab.com/gitlab-com/gl-infra/k8s-workloads/gitlab-com/-/blob/18aa8f083d5e45bc97f7745abda6fb50e45eab6a/bin/k-ctl
iferr!=nil&&strings.Contains(err.Error(), "release: not found") {
278
278
ifd.isAllowUnreleased() {
279
-
fmt.Printf("********************\n\n\tRelease was not present in Helm. Diff will show entire contents as new.\n\n********************\n")
279
+
fmt.Fprintf(os.Stderr, "********************\n\n\tRelease was not present in Helm. Diff will show entire contents as new.\n\n********************\n")
280
280
newInstall=true
281
281
err=nil
282
282
} else {
283
-
fmt.Printf("********************\n\n\tRelease was not present in Helm. Include the `--allow-unreleased` to perform diff without exiting in error.\n\n********************\n")
283
+
fmt.Fprintf(os.Stderr, "********************\n\n\tRelease was not present in Helm. Include the `--allow-unreleased` to perform diff without exiting in error.\n\n********************\n")
0 commit comments