Made changed to ingress controller so that it can take in multiple hosts #135
Conversation
charts/lenses/Chart.yaml
Outdated
| @@ -1,6 +1,6 @@ | |||
| apiVersion: "v1" | |||
| description: A chart for Lenses | |||
| name: lenses | |||
| version: 3.0.6 | |||
| appVersion: 3.0.6 | |||
There was a problem hiding this comment.
hi @gabatwork , app version reflects Lenses version so it should not be changed
charts/lenses/templates/ingress.yaml
Outdated
| @@ -17,21 +17,25 @@ metadata: | |||
| {{- end }} | |||
| spec: | |||
| rules: | |||
| - host: {{ .Values.ingress.host }} | |||
| {{- range .Values.ingress.hosts }} | |||
There was a problem hiding this comment.
what's the goal you are trying to achieve with multiple hosts? Because it's a breaking change and I think we can treat this only with a minor bump of the version and have backward compatible helm charts.
There was a problem hiding this comment.
My need was to basically add CNAMEs. I wanted to deploy an API into production and make sure everything was working before replacing the current API. For this I would need two hosts as follows: api.prod.example.com and api.example.com
There was a problem hiding this comment.
ok got it. let's have this with a backward compatible way then.
There was a problem hiding this comment.
sounds good, should be fine now
|
@gabatwork just rebase to have the latest changes and please squash the commits to keep only 1 commit. |
Hi guys, I noticed that in the ingress controller for this I wasn't able to add multiple hosts so I made this quick change. Hope it's all alright