Skip to content
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 default path to Ingress paths #449

Merged
merged 1 commit into from
Nov 11, 2020
Merged

Add default path to Ingress paths #449

merged 1 commit into from
Nov 11, 2020

Conversation

sergeyshaykhullin
Copy link
Contributor

@sergeyshaykhullin sergeyshaykhullin commented Oct 11, 2020

Problem

For now you are using empty array for ingress paths option as a default

In ingress helm template used range, so empty array by default renders as invalid k8s Ingress resource and to deploy this chart i have to specify paths. In most cases default path is /

Examples

Some examples of ingress values, most of them using / as default:

Another approach is to use fallback for paths when empty, but it is overkill for charts is not overriding application path

That will is fixed

This fixes k8s validation error on helm install (Ansible used):
fatal: [node1]: FAILED! => {"changed": false, "command": "/usr/local/bin/helm --namespace=akhq --version=0.1.3 upgrade -i --reset-values -f=/tmp/tmpvs_giipm.yml akhq akhq/akhq", "msg": "Failure when executing Helm command. Exited 1.\nstdout: \nstderr: Error: UPGRADE FAILED: error validating \"\": error validating data: ValidationError(Ingress.spec.rules[0].http): missing required field \"paths\" in io.k8s.api.networking.v1beta1.HTTPIngressRuleValue\n", "stderr": "Error: UPGRADE FAILED: error validating \"\": error validating data: ValidationError(Ingress.spec.rules[0].http): missing required field \"paths\" in io.k8s.api.networking.v1beta1.HTTPIngressRuleValue\n", "stderr_lines": ["Error: UPGRADE FAILED: error validating \"\": error validating data: ValidationError(Ingress.spec.rules[0].http): missing required field \"paths\" in io.k8s.api.networking.v1beta1.HTTPIngressRuleValue"], "stdout": "", "stdout_lines": []}

@tchiotludo
Copy link
Owner

Sorry I forgot this one.
To be honest, I've a doubt about this one ?
You are change [] to [ - "/" ], how the user can override (remove) this default path ?
Is this not better to add a control (if empty set to /) on the helm charts ?

@sergeyshaykhullin
Copy link
Contributor Author

sergeyshaykhullin commented Nov 6, 2020

@tchiotludo To override default path just use [/override]. You can set [],but it is generating invalid manifest and can't be applied, so this behavior by default is strange

No, there is no need for fallbacks

@tchiotludo tchiotludo merged commit f8894b8 into tchiotludo:dev Nov 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants