Helm Chart Option for Use Without Proxy#12671
Conversation
tijder
changed the title
| # - secretName: chart-example-tls | ||
| # hosts: | ||
| # - chart-example.local |
There was a problem hiding this comment.
It might be clearer to uncomment these and add comments what needs to be changed before deploying the chart.
There was a problem hiding this comment.
I see this happening in many more helmet charts. That's why I did it this way.
But I can change it.
There was a problem hiding this comment.
If uncommented, then it probably needs to be applied in values-ci and readme as well.
helm/templates/www.yaml
Outdated
| @@ -1,4 +1,5 @@ | |||
| --- | |||
| {{- if not .Values.www.directlyToService }} | |||
There was a problem hiding this comment.
I'm not sure I'm a fan of directlyToService, it raises the question: what/who directly to what service? How about deployProxy with a default value of true?
There was a problem hiding this comment.
I think it's a good suggestion. I have changed it.
| optional: true | ||
| - name: LDAP_SEARCH_FILTER # override to make sure that double dollar signs are processed like in docker | ||
| value: {{ .Values.api_server.env.LDAP_SEARCH_FILTER }} | ||
| {{- if .Values.api_server.extraVolumeMounts }} |
There was a problem hiding this comment.
Can you include the used options in values.yaml or document their usage? I'd be interested to see how it is applied exactly, now they're just empty lists. Also, why not make api_server.extraVolumes a list of dictionaries which contain both volume name and mountpoint instead of two separate lists?
| - SETUID | ||
| drop: | ||
| - ALL | ||
| {{- toYaml .Values.database.securityContext | nindent 12 }} |
There was a problem hiding this comment.
Perhaps surround the option with {{- if .Values.database.securityContext }}, make the whole thing optional (i.e. for local deployment)
| {{- end }} | ||
| --- | ||
| {{- if .Values.www.ingress -}} | ||
| {{ if .Values.www.ingress.enabled -}} |
There was a problem hiding this comment.
Is there a use case for having ingress disabled, but still wanting to specify www.ingress options?
And is there a reason to the different approach to trimming before/after in the various blocks?
| {{- range $key, $value := .Values.www.ingress.annotations }} | ||
| {{ $key }}: {{ $value | quote }} | ||
| {{- end }} | ||
| {{- toYaml . | nindent 4 }} |
There was a problem hiding this comment.
Does this include the |quote functionality, otherwise good to document either in values or readme
| {{- end }} | ||
| rules: | ||
| - host: {{ printf "%s" $.Values.www.ingress.hostname }} | ||
| {{- if .Values.www.ingress.hostname }} |
There was a problem hiding this comment.
So if there is no hostname provided, then the rules block is empty - correct? Does that work?
| # - secretName: chart-example-tls | ||
| # hosts: | ||
| # - chart-example.local |
There was a problem hiding this comment.
If uncommented, then it probably needs to be applied in values-ci and readme as well.
| repository: "ictu/quality-time_collector" | ||
|
|
||
| database: | ||
| securityContext: |
There was a problem hiding this comment.
Shouldn't the context override possibility also be applied to all other pods?
| tls: | ||
| {{ toYaml .Values.www.ingress.tls | indent 4 }} | ||
| {{- end }} | ||
| {{- range .Values.www.ingress.tls }} |
There was a problem hiding this comment.
Why does this one use range instead of {{- with .Values.www.ingress.tls }}?
3 participants
In this pull request, I implemented the necessary changes for #12559 and #12560.
false, the proxy is bypassed, allowing direct access to the API and frontendapi_server.securityContextin the database has been introduced.