[ingress] allow api-version spec

[tony-kerz]

helm template --debug -n ae-system --set server.ingress.enabled=1 my-release .

---
# Source: vault/templates/server-ingress.yaml
apiVersion: networking.k8s.io/v1beta1


kind: Ingress
metadata:
  name: my-release-vault
  namespace: ae-system
  labels:
    helm.sh/chart: vault-0.10.0
    app.kubernetes.io/name: vault
    app.kubernetes.io/instance: my-release
    app.kubernetes.io/managed-by: Helm
spec:
  rules:
    - host: "chart-example.local"
      http:
        paths:
          - path: /
            backend:
              serviceName: my-release-vault
              servicePort: 8200
---

helm template --debug -n ae-system --set server.ingress.enabled=1 --set server.ingress.apiVersion=networking.k8s.io/v1 my-release .

---
# Source: vault/templates/server-ingress.yaml
apiVersion: networking.k8s.io/v1

kind: Ingress
metadata:
  name: my-release-vault
  namespace: ae-system
  labels:
    helm.sh/chart: vault-0.10.0
    app.kubernetes.io/name: vault
    app.kubernetes.io/instance: my-release
    app.kubernetes.io/managed-by: Helm
spec:
  rules:
    - host: "chart-example.local"
      http:
        paths:
          - path: /
            backend:
              serviceName: my-release-vault
              servicePort: 8200
---

[hashicorp-cla]

All committers have signed the CLA.

[jasonodonnell]

Hi @tony-kerz, thanks for the contribution!

Can you help me understand why this change is being introduced? We don't offer api version configurables on any other objects so I'm curious what issues you are running into?

Additionally for any PR with configurables to be merged, there must be unit test coverage. You can find the bats unit tests in test/unit, but please hold off until we understand the scope of this change more.

[tony-kerz]

hi @jasonodonnell,

sure, yeah, the helm chart won't install on k8s v1.19.x, see this issue, is that sufficient motivation?

best,
tony.

[jasonodonnell]

Hi @tony-kerz, thanks for pointing me to that issue. I've now linked it to this PR.

I'm very hesitant to allow users to configure this on their own. Instead I'm thinking this should use if else statements and lookup the capability of the server. The APIs don't change very often so we can expand when needed, but it would be one less thing for the user to configure.

Thoughts?

[tony-kerz]

the capabilities lookup is cool, but u can see how it fell short this time around and now the chart is unusable in certain contexts until a new version is released.

i've been bitten by the shifting api version situation numerous times in the past, and custom forks are always an extra hassle, so i might endorse a belt-and-suspenders approach which uses a "case" type thing with all known versions at a point in time, but also allows an override to keep the chart in play during periods of change.

[jasonodonnell]

@tony-kerz, I'm fine with a switch case. Would you mind updating the PR to support it?

[tony-kerz]

sure, will do, thanks jason!

on a side note, i tried to run the ingress bats test and choked on the (undocumented?) dependency on python yq because i myself have migrated to the go version of yq to reduce the surface area of my runtimes.

any thoughts on cutting over to the go portable yq?

[tony-kerz]

thx for looking at this @jasonodonnell i think #498 should be fine for this, let's just focus on merging that ;)