From cfc90c98ef557ed7965063d7a17db59cac1707cb Mon Sep 17 00:00:00 2001 From: Manuel Alejandro de Brito Fontes Date: Mon, 20 Apr 2020 17:38:50 -0400 Subject: [PATCH] Add support for IngressClass and ingress.class annotation --- .../ingress-nginx/templates/clusterrole.yaml | 8 ++++ .../templates/controller-role.yaml | 8 ++++ cmd/nginx/main.go | 23 ++++++++-- .../provider/aws/deploy-tls-termination.yaml | 16 +++++++ deploy/static/provider/aws/deploy.yaml | 16 +++++++ deploy/static/provider/baremetal/deploy.yaml | 16 +++++++ deploy/static/provider/cloud/deploy.yaml | 16 +++++++ deploy/static/provider/kind/deploy.yaml | 16 +++++++ internal/ingress/annotations/class/main.go | 43 +++++++++++-------- .../ingress/annotations/class/main_test.go | 6 +-- .../ingress/controller/controller_test.go | 6 +-- internal/ingress/controller/store/store.go | 12 ------ internal/k8s/main.go | 8 ++++ 13 files changed, 154 insertions(+), 40 deletions(-) diff --git a/charts/ingress-nginx/templates/clusterrole.yaml b/charts/ingress-nginx/templates/clusterrole.yaml index fe7c5f511316..2035f549a232 100644 --- a/charts/ingress-nginx/templates/clusterrole.yaml +++ b/charts/ingress-nginx/templates/clusterrole.yaml @@ -65,4 +65,12 @@ rules: - ingresses/status verbs: - update + - apiGroups: + - "networking.k8s.io" # k8s 1.14+ + resources: + - ingressclasses + verbs: + - get + - list + - watch {{- end }} diff --git a/charts/ingress-nginx/templates/controller-role.yaml b/charts/ingress-nginx/templates/controller-role.yaml index 4d313a96191a..f2e39274483c 100644 --- a/charts/ingress-nginx/templates/controller-role.yaml +++ b/charts/ingress-nginx/templates/controller-role.yaml @@ -49,6 +49,14 @@ rules: - ingresses/status verbs: - update + - apiGroups: + - "networking.k8s.io" # k8s 1.14+ + resources: + - ingressclasses + verbs: + - get + - list + - watch - apiGroups: - "" resources: diff --git a/cmd/nginx/main.go b/cmd/nginx/main.go index b6811d394f25..e3974b87b6d2 100644 --- a/cmd/nginx/main.go +++ b/cmd/nginx/main.go @@ -29,8 +29,8 @@ import ( "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promhttp" - "k8s.io/apimachinery/pkg/api/errors" + apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/wait" discovery "k8s.io/apimachinery/pkg/version" @@ -42,6 +42,7 @@ import ( "k8s.io/klog" "k8s.io/ingress-nginx/internal/file" + "k8s.io/ingress-nginx/internal/ingress/annotations/class" "k8s.io/ingress-nginx/internal/ingress/controller" "k8s.io/ingress-nginx/internal/ingress/metric" "k8s.io/ingress-nginx/internal/k8s" @@ -107,8 +108,24 @@ func main() { klog.Warningf("Using deprecated \"k8s.io/api/extensions/v1beta1\" package because Kubernetes version is < v1.14.0") } - if !k8s.IsIngressV1Ready { - klog.Infof("Enabling new Ingress features availables since v1.18.0") + if k8s.IsIngressV1Ready { + klog.Infof("Enabling new Ingress features available since Kubernetes v1.18") + k8s.IngressClass, err = kubeClient.NetworkingV1beta1().IngressClasses(). + Get(context.TODO(), class.IngressClass, metav1.GetOptions{}) + if err != nil { + if !apierrors.IsNotFound(err) { + klog.Fatalf("Error searching IngressClass: %v", err) + } + + klog.Warningf("No IngressClass resource with name %v found. Only annotation will be used.", class.IngressClass) + + // TODO: remove once this is fixed in client-go + k8s.IngressClass = nil + } + + if k8s.IngressClass != nil && k8s.IngressClass.Spec.Controller != k8s.IngressNGINXControllerClass { + klog.Fatalf("IngressClass with name %v is not valid for ingress-nginx (invalid Spec.Controller)", class.IngressClass) + } } conf.Client = kubeClient diff --git a/deploy/static/provider/aws/deploy-tls-termination.yaml b/deploy/static/provider/aws/deploy-tls-termination.yaml index 4aed810e227b..a302405967be 100644 --- a/deploy/static/provider/aws/deploy-tls-termination.yaml +++ b/deploy/static/provider/aws/deploy-tls-termination.yaml @@ -106,6 +106,14 @@ rules: - ingresses/status verbs: - update + - apiGroups: + - networking.k8s.io # k8s 1.14+ + resources: + - ingressclasses + verbs: + - get + - list + - watch --- # Source: ingress-nginx/templates/clusterrolebinding.yaml apiVersion: rbac.authorization.k8s.io/v1 @@ -184,6 +192,14 @@ rules: - ingresses/status verbs: - update + - apiGroups: + - networking.k8s.io # k8s 1.14+ + resources: + - ingressclasses + verbs: + - get + - list + - watch - apiGroups: - '' resources: diff --git a/deploy/static/provider/aws/deploy.yaml b/deploy/static/provider/aws/deploy.yaml index 518457978472..1dc44cb0b19e 100644 --- a/deploy/static/provider/aws/deploy.yaml +++ b/deploy/static/provider/aws/deploy.yaml @@ -99,6 +99,14 @@ rules: - ingresses/status verbs: - update + - apiGroups: + - networking.k8s.io # k8s 1.14+ + resources: + - ingressclasses + verbs: + - get + - list + - watch --- # Source: ingress-nginx/templates/clusterrolebinding.yaml apiVersion: rbac.authorization.k8s.io/v1 @@ -177,6 +185,14 @@ rules: - ingresses/status verbs: - update + - apiGroups: + - networking.k8s.io # k8s 1.14+ + resources: + - ingressclasses + verbs: + - get + - list + - watch - apiGroups: - '' resources: diff --git a/deploy/static/provider/baremetal/deploy.yaml b/deploy/static/provider/baremetal/deploy.yaml index ff9de1efd01c..406bf18be412 100644 --- a/deploy/static/provider/baremetal/deploy.yaml +++ b/deploy/static/provider/baremetal/deploy.yaml @@ -99,6 +99,14 @@ rules: - ingresses/status verbs: - update + - apiGroups: + - networking.k8s.io # k8s 1.14+ + resources: + - ingressclasses + verbs: + - get + - list + - watch --- # Source: ingress-nginx/templates/clusterrolebinding.yaml apiVersion: rbac.authorization.k8s.io/v1 @@ -177,6 +185,14 @@ rules: - ingresses/status verbs: - update + - apiGroups: + - networking.k8s.io # k8s 1.14+ + resources: + - ingressclasses + verbs: + - get + - list + - watch - apiGroups: - '' resources: diff --git a/deploy/static/provider/cloud/deploy.yaml b/deploy/static/provider/cloud/deploy.yaml index 845e5d4fbce8..27b6ac38bdde 100644 --- a/deploy/static/provider/cloud/deploy.yaml +++ b/deploy/static/provider/cloud/deploy.yaml @@ -99,6 +99,14 @@ rules: - ingresses/status verbs: - update + - apiGroups: + - networking.k8s.io # k8s 1.14+ + resources: + - ingressclasses + verbs: + - get + - list + - watch --- # Source: ingress-nginx/templates/clusterrolebinding.yaml apiVersion: rbac.authorization.k8s.io/v1 @@ -177,6 +185,14 @@ rules: - ingresses/status verbs: - update + - apiGroups: + - networking.k8s.io # k8s 1.14+ + resources: + - ingressclasses + verbs: + - get + - list + - watch - apiGroups: - '' resources: diff --git a/deploy/static/provider/kind/deploy.yaml b/deploy/static/provider/kind/deploy.yaml index 2cf3661bc74a..29140c213731 100644 --- a/deploy/static/provider/kind/deploy.yaml +++ b/deploy/static/provider/kind/deploy.yaml @@ -99,6 +99,14 @@ rules: - ingresses/status verbs: - update + - apiGroups: + - networking.k8s.io # k8s 1.14+ + resources: + - ingressclasses + verbs: + - get + - list + - watch --- # Source: ingress-nginx/templates/clusterrolebinding.yaml apiVersion: rbac.authorization.k8s.io/v1 @@ -177,6 +185,14 @@ rules: - ingresses/status verbs: - update + - apiGroups: + - networking.k8s.io # k8s 1.14+ + resources: + - ingressclasses + verbs: + - get + - list + - watch - apiGroups: - '' resources: diff --git a/internal/ingress/annotations/class/main.go b/internal/ingress/annotations/class/main.go index a20507d135cf..af4ca6afeed7 100644 --- a/internal/ingress/annotations/class/main.go +++ b/internal/ingress/annotations/class/main.go @@ -18,6 +18,7 @@ package class import ( networking "k8s.io/api/networking/v1beta1" + "k8s.io/ingress-nginx/internal/k8s" ) const ( @@ -37,30 +38,36 @@ var ( IngressClass = "nginx" ) -// IsValid returns true if the given Ingress either doesn't specify -// the ingress.class annotation, or it's set to the configured in the -// ingress controller. +// IsValid returns true if the given Ingress specify the ingress.class +// annotation or IngressClassName resource for Kubernetes >= v1.18 func IsValid(ing *networking.Ingress) bool { - className := ing.Spec.IngressClassName + // 1. with annotation + ingress, ok := ing.GetAnnotations()[IngressKey] + if ok { + // we have 2 valid combinations + // 1 - ingress with default class | blank annotation on ingress + // 2 - ingress with specific class | same annotation on ingress + // + // and 2 invalid combinations + // 3 - ingress with default class | fixed annotation on ingress + // 4 - ingress with specific class | different annotation on ingress + if ingress == "" && IngressClass == DefaultClass { + return true + } - // we have 2 valid combinations - // 1 - ingress with default class | blank annotation on ingress - // 2 - ingress with specific class | same annotation on ingress - // - // and 2 invalid combinations - // 3 - ingress with default class | fixed annotation on ingress - // 4 - ingress with specific class | different annotation on ingress - if className != nil { - return *className == IngressClass + return ingress == IngressClass } - if IngressClass == DefaultClass { - return true + // 2. k8s < v1.18. Check default annotation + if !k8s.IsIngressV1Ready { + return IngressClass == DefaultClass } - if IngressClass == "" { - return true + // 3. without annotation and IngressClass. Check default annotation + if k8s.IngressClass == nil { + return IngressClass == DefaultClass } - return false + // 4. with IngressClass + return k8s.IngressClass.Name == IngressClass } diff --git a/internal/ingress/annotations/class/main_test.go b/internal/ingress/annotations/class/main_test.go index c4c95e39ebec..f38eeb790d3b 100644 --- a/internal/ingress/annotations/class/main_test.go +++ b/internal/ingress/annotations/class/main_test.go @@ -54,10 +54,10 @@ func TestIsValidClass(t *testing.T) { }, } + data := map[string]string{} + ing.SetAnnotations(data) for _, test := range tests { - if test.ingress != "" { - ing.Spec.IngressClassName = &test.ingress - } + ing.Annotations[IngressKey] = test.ingress IngressClass = test.controller DefaultClass = test.defClass diff --git a/internal/ingress/controller/controller_test.go b/internal/ingress/controller/controller_test.go index ac56542985ac..356c088a2678 100644 --- a/internal/ingress/controller/controller_test.go +++ b/internal/ingress/controller/controller_test.go @@ -190,8 +190,7 @@ func TestCheckIngress(t *testing.T) { } t.Run("When the ingress class differs from nginx", func(t *testing.T) { - class := "different" - ing.Spec.IngressClassName = &class + ing.ObjectMeta.Annotations["kubernetes.io/ingress.class"] = "different" nginx.command = testNginxTestCommand{ t: t, err: fmt.Errorf("test error"), @@ -202,8 +201,7 @@ func TestCheckIngress(t *testing.T) { }) t.Run("when the class is the nginx one", func(t *testing.T) { - class := "nginx" - ing.Spec.IngressClassName = &class + ing.ObjectMeta.Annotations["kubernetes.io/ingress.class"] = "nginx" nginx.command = testNginxTestCommand{ t: t, err: nil, diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index 0a8fb117f302..1cb6c4d5f927 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -949,30 +949,18 @@ func toIngress(obj interface{}) (*networkingv1beta1.Ingress, bool) { return nil, false } - ing.Spec.IngressClassName = extractClassName(ing) setDefaultPathTypeIfEmpty(ing) - return ing, true } if ing, ok := obj.(*networkingv1beta1.Ingress); ok { - ing.Spec.IngressClassName = extractClassName(ing) setDefaultPathTypeIfEmpty(ing) - return ing, true } return nil, false } -func extractClassName(ing *networkingv1beta1.Ingress) *string { - if c, ok := ing.Annotations[class.IngressKey]; ok { - return &c - } - - return nil -} - // Default path type is Prefix to not break existing definitions var defaultPathType = networkingv1beta1.PathTypePrefix diff --git a/internal/k8s/main.go b/internal/k8s/main.go index a62bcf5310a2..05ddcc33d08a 100644 --- a/internal/k8s/main.go +++ b/internal/k8s/main.go @@ -25,6 +25,7 @@ import ( "k8s.io/klog" apiv1 "k8s.io/api/core/v1" + networkingv1beta1 "k8s.io/api/networking/v1beta1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/version" clientset "k8s.io/client-go/kubernetes" @@ -121,6 +122,13 @@ var IsNetworkingIngressAvailable bool // IsIngressV1Ready indicates if the running Kubernetes version is at least v1.18.0 var IsIngressV1Ready bool +// IngressClass indicates the class of the Ingress to use as filter +var IngressClass *networkingv1beta1.IngressClass + +// IngressNGINXControllerClass defines the valid value of IngressClass +// Controller field for ingress-nginx +const IngressNGINXControllerClass = "k8s.io/ingress-nginx" + // NetworkingIngressAvailable checks if the package "k8s.io/api/networking/v1beta1" // is available or not and if Ingress V1 is supported (k8s >= v1.18.0) func NetworkingIngressAvailable(client clientset.Interface) (bool, bool) {