Support all nginx configmap settings #26
Conversation
sslavic
force-pushed
the
configurable-configmap
branch
from
b92c7ac to
156b233
Compare
sslavic
force-pushed
the
configurable-configmap
branch
from
156b233 to
c21cfe0
Compare
| @@ -1,5 +1,5 @@ | |||
| apiVersion: v1 | |||
| appVersion: v0.27.1 | |||
| appVersion: v0.28.0 | |||
There was a problem hiding this comment.
This was missed in 1.3.0; I've added a note in values.yaml for the tag, when changing the tag to change also appVersion
| @@ -19,44 +17,18 @@ ingressController: | |||
| legacy: false | |||
| replicas: 3 | |||
|
|
|||
| # configmap contains settings that can be overridden with a custom values | |||
| # configmap. | |||
There was a problem hiding this comment.
All of the settings in values.yaml can be overriden with a custom values configmap.
.Values.configmap is now purely for nginx configmap config options. Everything else that doesn't actually belong here moved under different configuration key. Settings which were under configmap just to support overriding but weren't actually overriding upstream defaults were removed since that is no longer needed. Similarly, configmap entries which have same value as upstream default, so not overriding it, were removed. Existing actual overrides were kept. Settings no longer supported upstream were cleaned up.
| configmap: | ||
| disable-access-log: "false" |
There was a problem hiding this comment.
Same as upstream default, so dropped it not to create noise.
| server-name-hash-bucket-size: "1024" | ||
| server-name-hash-max-size: "1024" |
There was a problem hiding this comment.
Same as upstream default, so dropped it not to create noise.
| configmap: | ||
| disable-access-log: "false" | ||
| enable-vts-status: "true" |
There was a problem hiding this comment.
This was dropped ages ago kubernetes/ingress-nginx#2643
| server-tokens: "false" | ||
| worker-processes: "4" | ||
| use-forwarded-headers: "true" | ||
|
|
||
| # optional settings that can be set. |
There was a problem hiding this comment.
This is no longer needed, since all configmap settings can be overriden with custom values, no need to add explicit support for each of them.
| # Enable the specified protocols (cf. http://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_protocols for the list of valid protocols) | ||
| ssl-protocols: "" | ||
|
|
||
| # optional hpa settings |
There was a problem hiding this comment.
This doesn't belong here, it was moved under controller.autoscaling configuration key, like in upstream stable chart.
| hpa-target-cpu-utilization-percentage: 50 | ||
| hpa-target-memory-utilization-percentage: 50 | ||
|
|
||
| # command args options |
There was a problem hiding this comment.
These do not belong here as they are not nginx configmap config options, they were moved under controller config key.
| userID: 101 | ||
| groupID: 101 | ||
|
|
||
| image: | ||
| registry: quay.io | ||
|
|
||
| global: |
There was a problem hiding this comment.
With exception of useProxyProtocol, these weren't used anywhere, dropped them. Dropped also useProxyProtocol since it can be overriden with use-proxy-protocol configmap, and upstream defaults to false so no need to explicitly set it in configmap either.
|
Tested on giraffe (aws, china) for GS platform release 11.0.0, with nginx IC as optional app. |
| @@ -68,6 +40,7 @@ controller: | |||
|
|
|||
| image: | |||
| repository: giantswarm/nginx-ingress-controller | |||
| # when updating tag make sure to also keep appVersion in Chart.yaml in sync | |||
Towards https://github.com/giantswarm/giantswarm/issues/8851
As side-effect also https://github.com/giantswarm/giantswarm/issues/7210 gets addressed.