fix: update apiversion for ingress. fixes #75

[domdepasquale]

This change updates the maximum apiVersion for ingress if the Kubernetes cluster Atlantis is running on supports it.

[domdepasquale]

Also, this updates the method for checking capabilities since .Capabilities.KubeVersion.GitVersion is deprecated. See: https://github.com/helm/helm/blob/7d13cd5ca1f1eee080c180be6c3201e3a81ff6e9/pkg/chartutil/capabilities.go#L80

[tszym]

Hi, thanks for this patch!
It's a nice first step but the chart might still fail on a stable API because of changes in supported fields.

The chart is using spec.rules[0].http.paths[0].backend.serviceName and spec.rules[0].http.paths[0].backend.servicePort with became spec.rules[0].http.paths[0].backend.service.name and spec.rules[0].http.paths[0].backend.service.port.number or .name

Also spec.rules[0].http.paths[0].pathType became a required value and is not provided yet. This one might be heavier to change, as path values are advised to use a * wildcard and I don't know if one of the possible filePaths support it now.

Do you think this patch could also address these parts so the linked issue is actually fixed?

[domdepasquale]

Yes. Totally forgot about the spec change. I'll work on it and reply soon.

[domdepasquale]

FWIW, I've set this up to generate based off of what's available in the cluster. @tszym you made a note that "path values are advised to use a * wildcard" and I couldn't find that in the reference for paths but did see it for hostnames.

[tszym]

Oh right, I meant the readme of this chart currently describes ingress paths with

Path to use in the Ingress. Should be set to /* if using gce-ingress in Google Cloud.

I don't know if that is still possible with the stable API as I did not find it either in the reference.

[tszym]

Do you know if that would be appropriate to reference an ingress class, or provide a way to do it, in the ingress resource?

[domdepasquale]

Oh right, I meant the readme of this chart currently describes ingress paths with

Path to use in the Ingress. Should be set to /* if using gce-ingress in Google Cloud.

I don't know if that is still possible with the stable API as I did not find it either in the reference.

The documentation here makes me think that the readme of the chart is still applicable

[domdepasquale]

Do you know if that would be appropriate to reference an ingress class, or provide a way to do it, in the ingress resource?

Were you thinking of this? https://kubernetes.io/docs/concepts/services-networking/ingress/#deprecated-annotation

I could see creating the option to define it, but to leave it nil for now. Thoughts?

[tszym]

I was thinking about adding an optional ingressClassName field if that's ok to you. This should be enough, I don't feel like the IngressClass resource should be part of the chart but as I understand the stable API, the field on the ingress should be useful.

[domdepasquale]

Check it out when you have time and let me know.

[tszym]

Looks good to me! Maybe need to add the field to the table in the readme, or it's automatic idk.

[kobejn-jb]

Looks, good. I've started to work on this change myself but found your PR.

Tested on k8s 1.20.7 with traefik 1.87.1

Maybe somebody with write access can approve :)

[jurgen-weber-deltatre]

Can we please get this merged.

[jamengual]

@dad264 could you push a change to bump the version of the chart? like https://github.com/runatlantis/helm-charts /pull/88
otherwise we will not get a new release

[jurgen-weber-deltatre]

yeah, new version needed. ;)

[jamengual]

New version is out, enjoy.