Use default K8S ingress annotations

[jonatasbaldin]

Expected Behaviour

Instead of using spec.ingressType to select an Ingress class, we should use the default annotation kubernetes.io/ingress.class, since #7 was implemented.

Current Behaviour

In the version 0.4.0, by the docs, we need to define the Ingress class using the spec.ingressType configuration.

BUT

After merging #7 we actually don't need that. I tested two cases:

Empty annotation and spec.ingressType

Deploying the following FunctionIngress without spec.ingressType and without kubernetes.io/ingress.class annotation defaults the Ingress class to nginx and works. I believe this should be considered a bug and specifying an Ingress class should be mandatory.

apiVersion: openfaas.com/v1alpha2
kind: FunctionIngress
metadata:
  name: nodeinfo
  namespace: openfaas
spec:
  domain: "nodeinfo.deployeveryday.com"
  function: "nodeinfo"
  # ingressType: "nginx"  # disable ingressType

Use kubernetes.io/ingress.class annotation

Deploying the following FunctionIngress without spec.ingressType but using the kubernetes.io/ingress.class works like a charm. I believe this should be the default way of doing things.

apiVersion: openfaas.com/v1alpha2
kind: FunctionIngress
metadata:
  name: nodeinfo
  namespace: openfaas
  annotations:
    # Uses default K8S annotation
    kubernetes.io/ingress.class: "nginx"
spec:
  domain: "nodeinfo.deployeveryday.com"
  function: "nodeinfo"
  # ingressType: "nginx"  # disable ingressType

Possible Solution

1. getClass should yield an error if not class is defined.
2. makeAnnotations should get the Class from annotations and not from function.Spec.IngressType.

Context

Using the default K8S annotation to select an Ingress class requires lower learning curve and it's more a standard across other K8S controllers.

[alexellis]

Thanks for your suggestion.

I have mixed feelings about this.

[alexellis]

What do you think about using both sources and having a priority to use the spec? 🤷‍♂

[jonatasbaldin]

I usually prefer one and only one way of doing things, if possible, since it leads to less confusion.

In the currency scenario, we already have two ways of picking the class. And even a third way, which is not defining anything (and the class by default is nginx).

It depends also on what is the intended design for the FunctionIngress. Was it made to be as close as possible to an Ingress (since it actually creates one) or to have its own different structure?

Personally, as a user, when I saw the FunctionIngress for the first time, I was expecting it to behave as an Ingress as much as possible.

(I'm loving these discussions btw, thinking about DX is so nice!)