Enable overriding of ingress api version for systems where detection doesn't work #252

2021-11-21T14:31:07Z

iMartyn commented

2021-11-21 14:31:07 +00:00

(Migrated from gitea.com)

fixes #251

The rendering is a bit more programatic but the result is the same if you don't have an override. This makes the code a little easier at the end of the template, and slightly less easier to read at the beginning, which I think is a valid tradeoff.

fixes #251 The rendering is a bit more programatic but the result is the same if you don't have an override. This makes the code a little easier at the end of the template, and slightly less easier to read at the beginning, which I think is a valid tradeoff.

justusbunsi commented

2021-11-21 19:56:45 +00:00

(Migrated from gitea.com)

+{{- if .Values.ingress.APIVersionOverride -}}

The "override" suffix is not necessary, I'd say. As long as it is commented out and well documented that in most cases this gets automatically detected, one can treat it like image.tag value.

Please add this option to the README section (table) for ingress.

? Wrong line commented. The new option was meant.

``` +{{- if .Values.ingress.APIVersionOverride -}} ``` The "override" suffix is not necessary, I'd say. As long as it is commented out and well documented that in most cases this gets automatically detected, one can treat it like image.tag value. Please add this option to the README section (table) for ingress. ? Wrong line commented. The new option was meant.

👍 1

justusbunsi commented

2021-11-26 05:13:53 +00:00

(Migrated from gitea.com)

+| `ingress.APIVersion`                 | specify APIVersion of ingress object           | version indicated by helm's `Capabilities` object.  Mostly for argocd |

Sorry for not mentioning this earlier, haven't realized it before: The most settings in values.yaml are lowerCamelCase. Let's be consistent with the apiVersion as well. ?

``` +| `ingress.APIVersion` | specify APIVersion of ingress object | version indicated by helm's `Capabilities` object. Mostly for argocd | ``` Sorry for not mentioning this earlier, haven't realized it before: The most settings in values.yaml are lowerCamelCase. Let's be consistent with the `apiVersion` as well. ?

iMartyn commented

2021-11-26 09:12:36 +00:00

(Migrated from gitea.com)

Sure thing, consistency is good. Fixed.

luhahn commented

2021-12-17 09:18:07 +00:00

(Migrated from gitea.com)

Will have a look today at your PR :)

justusbunsi (Migrated from gitea.com) approved these changes 2021-12-18 10:51:22 +00:00

justusbunsi (Migrated from gitea.com) left a comment

Somehow I missed to actually approve this one. LGTM.

luhahn (Migrated from gitea.com) approved these changes 2021-12-20 11:53:34 +00:00

luhahn (Migrated from gitea.com) left a comment

LGTM

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: lunny/helm-chart#252