-
Notifications
You must be signed in to change notification settings - Fork 218
Description
Describe the bug
Not exactly a bug, but I wasn't sure what other issue type fit.
In https://github.com/argoproj-labs/argocd-vault-plugin/blob/main/manifests/cmp-configmap/argocd-cm.yaml#L31 helm/kustomize are called and their output is piped to argocd-vault-plugin.
This works fine assuming the user has verified the helm chart or kustomize resources are valid and render successfully beforehand.
However if they did not do so, helm/kustomize will exit with non-0 exit code and typically with some relevant message on stderr.
On Argo without AVP, this stderr gets displayed to the user prompting them to act.
With AVP the pipe to argocd-vault-plugin continues, argocd-vault-plugin gets essentially nothing on stdin which is still valid, sees nothing to replace/lookup and returns success (exitcode 0). Argocd then thinks the refresh/render was successful and there are no resources, i.e. an inconsistent behaviour vs not using AVP.
An easy fix is to specify bash instead of sh and do set -o pipefail && before invoking kustomize or helm. Example of what I mean for kustomize:
original:
- name: argocd-vault-plugin-kustomize
generate:
command: ["sh", "-c"]
args: ["kustomize build . | argocd-vault-plugin generate -"]
revised:
- name: argocd-vault-plugin-kustomize
generate:
command: ["bash", "-c"]
args: ["set -o pipefail && kustomize build . | argocd-vault-plugin generate -"]
To Reproduce
Steps to reproduce the behavior:
- Create a valid kustomize or helm project
- Create app on argo without specifying AVP as plugin
- Verify your expected resources are rendered by Argo
- Update the project to have some invalid content that will fail to render
- See error
- Revert the change in 4
- Update the Argo app definition to now specify it should pass the project through AVP
- Make the same change as 4
- See Argo report status as green but resources have all disappeared without error message being shown
Expected behavior
Helm/Kustomize error behaviour should be consistent with/without AVP.