-
Notifications
You must be signed in to change notification settings - Fork 71
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
action anticipates a given env #44
Comments
This hack works: master...davidkarlsen:fixKubeConfig - should we just go with something similar? Probably the property needs renaming and it should be a v2? |
Signed-off-by: David Karlsen <david@davidkarlsen.com>
I wouldn't consider this a hack and use this as fix. Doesn't sound like that's a breaking change to me that needs a v2. |
Agree - looks not only reasonable but also backwards compatible 👍 |
Ait, then I'll cancel #45 and come back with the initial version |
Signed-off-by: David Karlsen <david@davidkarlsen.com>
The action runs docker via the ct.sh command, which is not ideal because the dockerized env needs a bit of configuration. For instance in https://github.com/helm/chart-testing-action/blob/master/ct.sh#L128 only kubeconfig is copied, not the tls ca etc.
For actions that are purely js-based this is not a problem, since the action run's under whatever environment is set at the time it is launched, and for docker-based actions gh will mount a given set of directories, and pass on some env to the launched container.
Maybe we should consider going with purely js-based action, or a purely docker-based action. IMHO the latter is favourable, as chart-testing relies on a set of tooling, which is encapsulated in the image: https://github.com/helm/chart-testing/blob/master/Dockerfile.
Either that, or do a bit more work on setting up the kube-env (copy the full .kube dir, rather than only the config-file) - but there could be more traps down the road.
When running the action on self-hosted runners, it fails with:
The text was updated successfully, but these errors were encountered: