[GH-280] Add support for networking.k8s.io/v1 ingress

[andrewodri]

Summary

This PR adds support for networking.k8s.io/v1 Ingress objects while retaining support for unserved networking.k8s.io/v1beta1 and extensions/v1beta1 Ingress objects as well.

Out of neccessity, it also adds support for Helm v3 while retaining support for Helm v2. While Helm v2 does support the .Capabilities.KubeVersion.GitVersion object, Helm v3 does not support it, and only supports .Capabilities.KubeVersion.Version. Rather sifting through Helm versions and Kubernetes versions to identify support, supported API versions are validated using .Capabilities.APIVersions.Has which is supported by Helm v2 and v3.

This has only been tested to a limited extent since I wanted to get this up; if there is a good chance of this being merged, I would be happy to do some further testing.

Ticket Link

Fixes mattermost-helm#280

[mattermod]

Hello @andrewodri,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

[JosefWN]

Great @andrewodri, both increased compatibility and readability! It just seems that you forgot to bump the chart versions.

I don't know if it's out of scope for this PR, but ingressClassName is missing from the chart as well, for Kubernetes 1.18+.

Would be great to get this merged, I've been blocked by it for months and had to run an ugly-patched chart of my own. Not sure if it will help, but I can try to chase the repo maintainers once this is ready for merge.

[andrewodri]

Thanks @JosefWN! Chart versions have been bumped.

It seems like the ingressClassName directive is supported in the enterprise edition chart... I think it would be pretty straightforward moving that across, although it might be out of the scope of this issues; let's see what our friendly neighborhood code reviewer has to say though! (See https://github.com/andrewodri/mattermost-helm/blob/c206b067083fc964ebc199fbba7ae5fb8667322f/charts/mattermost-enterprise-edition/templates/ingress-mattermost-app.yaml#L24-L26)

Would be great to get this merged, I've been blocked by it for months and had to run an ugly-patched chart of my own. Not sure if it will help, but I can try to chase the repo maintainers once this is ready for merge.

Hah, yeah I saw the writing on the wall with this one... Hopefully following the contribution guidelines and keeping the backwards compatibility will help get this merged!

I'm happy to do additional clean up as well, since we could like move to feature testing and progressive enhancement for other Kubernetes objects and clean up the _helpers.tpl, provided that is a pattern the team is comfortable with 🙂

[JosefWN]

@stylianosrigas, @pfltdv, @spirosoik: any chance of getting this merged, and also support for ingressClassName? Like I said in my comment above, been a blocker for months... With the release of Kubernetes 1.24 in a couple of weeks, Mattermost will only support old and unmaintained Kubernetes versions (pre-1.22).

[phoinixgrr]

Awesome work there @andrewodri 🎖️

Performing helm template errors out. Focalboard example:

$ cd charts
$ helm template focalboard --values focalboard/values.yaml
Error: YAML parse error on focalboard/templates/ingress.yaml: error converting YAML to JSON: yaml: line 16: mapping values are not allowed in this context

Use --debug flag to render out invalid YAML

Tested with the default values.yaml ingress config, ingress.enabled: true:

ingress:
  enabled: true
  annotations:
    kubernetes.io/ingress.class: nginx
    kubernetes.io/tls-acme: "true"
  hosts:
    - host: focalboard.example.com
      paths:
      - path: /
        backend:
          serviceName: focalboard
          servicePort: 80

My local helm version:

 helm version
version.BuildInfo{Version:"v3.8.1", GitCommit:"5cb9af4b1b271d11d7a97a71df3ac337dd94ad37", GitTreeState:"clean", GoVersion:"go1.17.8"}

Is this expected?

[andrewodri]

Thanks all! Hah; not expected @phoinixgrr... I had left some block chomping indicators in that if statement 😬

A fix has been pushed up for that.