-
Notifications
You must be signed in to change notification settings - Fork 141
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
Add validate argument to helm_template #587
base: main
Are you sure you want to change the base?
Conversation
I would like to add a unittest but I couldn't figure out how to run them. Does this make sense? # tests/unit/modules/test_helm_template.py
def test_template_with_validate():
my_chart_ref = "testref"
helm_cmd = "helm"
parser = argparse.ArgumentParser()
# to "simulate" helm template options, include two optional parameters NAME and CHART.
# if parsed string contains only one parameter, the value will be passed
# to CHART and NAME will be set to default value "release-name" as in helm template
parser.add_argument("cmd")
parser.add_argument("template")
parser.add_argument("NAME", nargs="?", default="release-name")
parser.add_argument("CHART", nargs="+")
parser.add_argument("--validate", dest="validate", action="store_true")
parser.set_defaults(validate=False)
mytemplate = template(cmd=helm_cmd, chart_ref=my_chart_ref, validate=True)
args, unknown = parser.parse_known_args(mytemplate.split())
assert args.validate is True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pauvos thanks for your pull request,
could you please add a changelog? here is an example https://github.com/ansible-collections/kubernetes.core/pull/575/files#diff-6d789f5197c05bc24880f186dfc451b420fc61f8666d969d2c7309473aa1e3f1
@abikouo , yes sure thing! I have 2 other pull requests:
Should i merge everything in one pull requests or create individual changelog fragments? |
71e9d81
to
aaeeb1d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR, and sorry for taking so long to get back to this. Since this is going to connect to a cluster, you'll need to add some connection parameters to the module. This should be pretty easy, though, and you can look at how the helm module does it. I think it should be sufficient to just extend the doc string as it's done in
kubernetes.core/plugins/modules/helm.py
Lines 221 to 222 in 8640c16
extends_documentation_fragment: | |
- kubernetes.core.helm_common_options |
kubernetes.core/plugins/modules/helm.py
Line 695 in deb4859
arg_spec = copy.deepcopy(HELM_AUTH_ARG_SPEC) |
Since this is adding functionality to connect to a cluster, it would be a good idea to add an integration test or two for this. You can add those in
- name: Render templates |
Your suggestion of the unit test seems reasonable. If you want to run unit tests locally you can do so with ansible-test units --docker
.
There is an alternative way to achieve the desired results here which is to add an api versions parameter to the helm_template module (this is often useful when there isn't a kubernetes cluster to connect to e.g. during CI - this is how we generate all of our templates) e.g. for the alb-controller we pass |
SUMMARY
Support
helm template --validate
:It's sometimes necessary to render templates against the kubernetes API because helm charts increasingly start to use conditions:
Without checking the capabilities of the cluster the above template will render a deprecated extensions/v1beta1 Ingress resource.
ISSUE TYPE
COMPONENT NAME
kubernetes.core.helm_template
ADDITIONAL INFORMATION