Make Ingress generators K8s 1.18 compatible

[alexellis]

In Kubernetes 1.18 the Ingress definitions have changed slightly.

In particular the @paurosello mentioned the change about ingressClass becoming part of the spec, instead of being an annotation.

If we can add both, perhaps that will help keep backwards compatibility for the time being?

https://kubernetes.io/docs/setup/release/notes/#extending-ingress-with-and-replacing-a-deprecated-annotation-with-ingressclass

Alex

[Waterdrips]

We can't add both. A 1.18 cluster:

Error from server (Invalid): error when creating "/tmp/.arkade/temp_openfaas_ingress.yaml": Ingress.extensions "openfaas-gateway" is invalid: annotations.kubernetes.io/ingress.class: Invalid value: "nginx": can not be set when the class field is also set

a 1.17 gives a validation error if the field exists but we can set --validate-false to get past that.

We might have to get the k8s version when installing ingress and decide if we are doing annotation or field.

Ill have a look at checking the version using the go client or something..

[alexellis]

You can't set an annotation for backwards compatibility? Since when were annotations something the Kubernetes API rejected deployments over.

If there's no workaround, then like you say the code will need an additional flag or test to determine the API version.

@viveksyngh cc

[alexellis]

Please don't add a dependency to the Kubernetes go client to arkade, the size of the binary will balloon out of control.

Rob Scott is saying that it is supported to have both the annotation and the spec, despite the hard error. Perhaps the best solution for arkade is to run kubectl apply with --validate=false.

https://twitter.com/robertjscott/status/1262053313786216449?s=20

For the Ingress Operator (that creates ingress, we have the Kubernetes Go client already there, so it seems like we should either set both there, or set either depending on the API server version. openfaas/ingress-operator#27)

[aledbf]

The validate=false will help to bypass the validation https://github.com/kubernetes/kubernetes/blob/master/pkg/apis/networking/validation/validation.go#L216

[alexellis]

https://twitter.com/robertjscott/status/1262053313786216449?s=20

It's unclear whether this is intended or unintended behaviour from the K8s team, it certainly doesn't look like both the spec attribute and the legacy annotation are compatible.

Thanks @aledbf for echoing what @Waterdrips said above. We should try this for arkade, and then see what kind of errors we get in the ingress-operator and decide how to move on from there.

[alexellis]

IngressClassName is the name of the IngressClass cluster resource. The associated IngressClass defines which controller will implement the resource. This replaces the deprecated kubernetes.io/ingress.class annotation. For backwards compatibility, when that annotation is set, it must be given precedence over this field. The controller may emit a warning if the field and annotation have different values. Implementations of this API should ignore Ingresses without a class specified. An IngressClass resource may be marked as default, which can be used to set a default value for this field. For more information, refer to the IngressClass documentation.

https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#ingressspec-v1beta1-networking-k8s-io

[Waterdrips]

I was already using --validate=false

Error from server (Invalid): error when creating "/tmp/.arkade/temp_openfaas_ingress.yaml": Ingress.extensions "openfaas-gateway" is invalid: annotations.kubernetes.io/ingress.class: Invalid value: "nginx": can not be set when the class field is also set

The error is ^ there

the yaml:

apiVersion: extensions/v1beta1 
kind: Ingress
metadata:
  name: openfaas-gateway
  namespace: openfaas
  annotations:
    cert-manager.io/cluster-issuer: letsencrypt-staging
    kubernetes.io/ingress.class: nginx
spec:
  ingressClassName: nginx
  rules:
  - host: example.heyal.uk
    http:
      paths:
      - backend:
          serviceName: gateway
          servicePort: 8080
        path: /
  tls:
  - hosts:
    - example.heyal.uk
    secretName: openfaas-gateway
---
apiVersion: cert-manager.io/v1alpha2
kind: ClusterIssuer
metadata:
  name: letsencrypt-staging
spec:
  acme:
    email: [email protected]
    server: https://acme-staging-v02.api.letsencrypt.org/directory
    privateKeySecretRef:
      name: example-issuer-account-key
    solvers:
    - http01:
        ingress:
          class: nginx

[robscott]

To clarify here, the only reason both a class field and annotation should exist on an Ingress resource at the same time would be part of an upgrade from one approach to the other. Only one of these values will be read from at any given time. When both are set, the annotation is given precedence. Why set both on new resources if only one is going to be used?

[alexellis]

There's more, apparently the ingressClass has to be defined separately, if it's a miss on a K8s 1.18 cluster, what happens then? @aledbf do you know?

[alexellis]

Sorry to mix two issues here, but whilst you have eyes on @robscott

What would you recommend we do for both separate projects? I'd be interested in your opinion if you're open to giving it.

openfaas/ingress-operator#27 (comment)