From 7e65ad3e432bd2837c17e3e63e85dcbcc30f4a8a Mon Sep 17 00:00:00 2001 From: Mike Eves Date: Thu, 10 Jun 2021 18:02:42 +0100 Subject: [PATCH 1/6] Add class label to kubernetes ingress discovery Signed-off-by: Mike Eves --- discovery/kubernetes/ingress.go | 6 ++++ discovery/kubernetes/ingress_test.go | 54 +++++++++++++++++++++------- docs/configuration/configuration.md | 1 + 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/discovery/kubernetes/ingress.go b/discovery/kubernetes/ingress.go index a6ef746e47..b638421f3b 100644 --- a/discovery/kubernetes/ingress.go +++ b/discovery/kubernetes/ingress.go @@ -147,6 +147,7 @@ const ( ingressSchemeLabel = metaLabelPrefix + "ingress_scheme" ingressHostLabel = metaLabelPrefix + "ingress_host" ingressPathLabel = metaLabelPrefix + "ingress_path" + ingressClassLabel = metaLabelPrefix + "ingress_class" ) func ingressLabels(ingress *v1beta1.Ingress) model.LabelSet { @@ -154,6 +155,11 @@ func ingressLabels(ingress *v1beta1.Ingress) model.LabelSet { ls := make(model.LabelSet, 2*(len(ingress.Labels)+len(ingress.Annotations))+2) ls[ingressNameLabel] = lv(ingress.Name) ls[namespaceLabel] = lv(ingress.Namespace) + if ingress.Spec.IngressClassName == nil { + ls[ingressClassLabel] = lv("") + } else { + ls[ingressClassLabel] = lv(*ingress.Spec.IngressClassName) + } for k, v := range ingress.Labels { ln := strutil.SanitizeLabelName(k) diff --git a/discovery/kubernetes/ingress_test.go b/discovery/kubernetes/ingress_test.go index 27c0cf92b8..ff0cd46e84 100644 --- a/discovery/kubernetes/ingress_test.go +++ b/discovery/kubernetes/ingress_test.go @@ -33,7 +33,7 @@ const ( TLSMixed ) -func makeIngress(tls TLSMode) *v1beta1.Ingress { +func makeIngress(tls TLSMode, excludeClass bool) *v1beta1.Ingress { ret := &v1beta1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: "testingress", @@ -42,7 +42,8 @@ func makeIngress(tls TLSMode) *v1beta1.Ingress { Annotations: map[string]string{"test/annotation": "testannotationvalue"}, }, Spec: v1beta1.IngressSpec{ - TLS: nil, + IngressClassName: classString("testclass"), + TLS: nil, Rules: []v1beta1.IngressRule{ { Host: "example.com", @@ -81,13 +82,23 @@ func makeIngress(tls TLSMode) *v1beta1.Ingress { ret.Spec.TLS = []v1beta1.IngressTLS{{Hosts: []string{"example.com"}}} } + if excludeClass { + ret.Spec.IngressClassName = nil + } + return ret } -func expectedTargetGroups(ns string, tls TLSMode) map[string]*targetgroup.Group { +func classString(v string) *string { + return &v +} + +func expectedTargetGroups(ns string, tls TLSMode, excludeClass bool) map[string]*targetgroup.Group { scheme1 := "http" scheme2 := "http" + ingressClassName := "testclass" + switch tls { case TLSYes: scheme1 = "https" @@ -96,6 +107,10 @@ func expectedTargetGroups(ns string, tls TLSMode) map[string]*targetgroup.Group scheme1 = "https" } + if excludeClass { + ingressClassName = "" + } + key := fmt.Sprintf("ingress/%s/testingress", ns) return map[string]*targetgroup.Group{ key: { @@ -126,6 +141,7 @@ func expectedTargetGroups(ns string, tls TLSMode) map[string]*targetgroup.Group "__meta_kubernetes_ingress_labelpresent_test_label": "true", "__meta_kubernetes_ingress_annotation_test_annotation": "testannotationvalue", "__meta_kubernetes_ingress_annotationpresent_test_annotation": "true", + "__meta_kubernetes_ingress_class": lv(ingressClassName), }, Source: key, }, @@ -138,11 +154,11 @@ func TestIngressDiscoveryAdd(t *testing.T) { k8sDiscoveryTest{ discovery: n, afterStart: func() { - obj := makeIngress(TLSNo) + obj := makeIngress(TLSNo, false) c.NetworkingV1beta1().Ingresses("default").Create(context.Background(), obj, metav1.CreateOptions{}) }, expectedMaxItems: 1, - expectedRes: expectedTargetGroups("default", TLSNo), + expectedRes: expectedTargetGroups("default", TLSNo, false), }.Run(t) } @@ -152,11 +168,11 @@ func TestIngressDiscoveryAddTLS(t *testing.T) { k8sDiscoveryTest{ discovery: n, afterStart: func() { - obj := makeIngress(TLSYes) + obj := makeIngress(TLSYes, false) c.NetworkingV1beta1().Ingresses("default").Create(context.Background(), obj, metav1.CreateOptions{}) }, expectedMaxItems: 1, - expectedRes: expectedTargetGroups("default", TLSYes), + expectedRes: expectedTargetGroups("default", TLSYes, false), }.Run(t) } @@ -166,26 +182,40 @@ func TestIngressDiscoveryAddMixed(t *testing.T) { k8sDiscoveryTest{ discovery: n, afterStart: func() { - obj := makeIngress(TLSMixed) + obj := makeIngress(TLSMixed, false) c.NetworkingV1beta1().Ingresses("default").Create(context.Background(), obj, metav1.CreateOptions{}) }, expectedMaxItems: 1, - expectedRes: expectedTargetGroups("default", TLSMixed), + expectedRes: expectedTargetGroups("default", TLSMixed, false), + }.Run(t) +} + +func TestIngressDiscoveryAddNoClass(t *testing.T) { + n, c := makeDiscovery(RoleIngress, NamespaceDiscovery{Names: []string{"default"}}) + + k8sDiscoveryTest{ + discovery: n, + afterStart: func() { + obj := makeIngress(TLSMixed, true) + c.NetworkingV1beta1().Ingresses("default").Create(context.Background(), obj, metav1.CreateOptions{}) + }, + expectedMaxItems: 1, + expectedRes: expectedTargetGroups("default", TLSMixed, true), }.Run(t) } func TestIngressDiscoveryNamespaces(t *testing.T) { n, c := makeDiscovery(RoleIngress, NamespaceDiscovery{Names: []string{"ns1", "ns2"}}) - expected := expectedTargetGroups("ns1", TLSNo) - for k, v := range expectedTargetGroups("ns2", TLSNo) { + expected := expectedTargetGroups("ns1", TLSNo, false) + for k, v := range expectedTargetGroups("ns2", TLSNo, false) { expected[k] = v } k8sDiscoveryTest{ discovery: n, afterStart: func() { for _, ns := range []string{"ns1", "ns2"} { - obj := makeIngress(TLSNo) + obj := makeIngress(TLSNo, false) obj.Namespace = ns c.NetworkingV1beta1().Ingresses(obj.Namespace).Create(context.Background(), obj, metav1.CreateOptions{}) } diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index af94de42a4..542e90a95e 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -1342,6 +1342,7 @@ Available meta labels: * `__meta_kubernetes_ingress_labelpresent_`: `true` for each label from the ingress object. * `__meta_kubernetes_ingress_annotation_`: Each annotation from the ingress object. * `__meta_kubernetes_ingress_annotationpresent_`: `true` for each annotation from the ingress object. +* `__meta_kubernetes_ingress_class`: Class from the ingress spec, if present. Default to `""`. * `__meta_kubernetes_ingress_scheme`: Protocol scheme of ingress, `https` if TLS config is set. Defaults to `http`. * `__meta_kubernetes_ingress_path`: Path from ingress spec. Defaults to `/`. From 22b16c30def14bd27b835db46d1e578a5dbf1ea3 Mon Sep 17 00:00:00 2001 From: Mike Eves Date: Thu, 10 Jun 2021 18:06:26 +0100 Subject: [PATCH 2/6] Fix typo Signed-off-by: Mike Eves --- docs/configuration/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index 542e90a95e..fad606ae2d 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -1342,7 +1342,7 @@ Available meta labels: * `__meta_kubernetes_ingress_labelpresent_`: `true` for each label from the ingress object. * `__meta_kubernetes_ingress_annotation_`: Each annotation from the ingress object. * `__meta_kubernetes_ingress_annotationpresent_`: `true` for each annotation from the ingress object. -* `__meta_kubernetes_ingress_class`: Class from the ingress spec, if present. Default to `""`. +* `__meta_kubernetes_ingress_class`: Class from the ingress spec, if present. Defaults to `""`. * `__meta_kubernetes_ingress_scheme`: Protocol scheme of ingress, `https` if TLS config is set. Defaults to `http`. * `__meta_kubernetes_ingress_path`: Path from ingress spec. Defaults to `/`. From aab51ffe2ae7490e18e5f8aa4c435b55a21fafcf Mon Sep 17 00:00:00 2001 From: Mike Eves Date: Fri, 11 Jun 2021 11:26:27 +0100 Subject: [PATCH 3/6] Tweak docs Signed-off-by: Mike Eves --- docs/configuration/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index fad606ae2d..70fd570c44 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -1342,7 +1342,7 @@ Available meta labels: * `__meta_kubernetes_ingress_labelpresent_`: `true` for each label from the ingress object. * `__meta_kubernetes_ingress_annotation_`: Each annotation from the ingress object. * `__meta_kubernetes_ingress_annotationpresent_`: `true` for each annotation from the ingress object. -* `__meta_kubernetes_ingress_class`: Class from the ingress spec, if present. Defaults to `""`. +* `__meta_kubernetes_ingress_class`: Class from ingress spec, if present. * `__meta_kubernetes_ingress_scheme`: Protocol scheme of ingress, `https` if TLS config is set. Defaults to `http`. * `__meta_kubernetes_ingress_path`: Path from ingress spec. Defaults to `/`. From 7e1111ff141f19887013977f570eb02ec002705c Mon Sep 17 00:00:00 2001 From: Mike Eves Date: Fri, 11 Jun 2021 13:43:22 +0100 Subject: [PATCH 4/6] Update label from `class` to `class_name` Signed-off-by: Mike Eves --- discovery/kubernetes/ingress.go | 6 +++--- discovery/kubernetes/ingress_test.go | 10 +++++----- docs/configuration/configuration.md | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/discovery/kubernetes/ingress.go b/discovery/kubernetes/ingress.go index b638421f3b..68b4797ed2 100644 --- a/discovery/kubernetes/ingress.go +++ b/discovery/kubernetes/ingress.go @@ -147,7 +147,7 @@ const ( ingressSchemeLabel = metaLabelPrefix + "ingress_scheme" ingressHostLabel = metaLabelPrefix + "ingress_host" ingressPathLabel = metaLabelPrefix + "ingress_path" - ingressClassLabel = metaLabelPrefix + "ingress_class" + ingressClassNameLabel = metaLabelPrefix + "ingress_class_name" ) func ingressLabels(ingress *v1beta1.Ingress) model.LabelSet { @@ -156,9 +156,9 @@ func ingressLabels(ingress *v1beta1.Ingress) model.LabelSet { ls[ingressNameLabel] = lv(ingress.Name) ls[namespaceLabel] = lv(ingress.Namespace) if ingress.Spec.IngressClassName == nil { - ls[ingressClassLabel] = lv("") + ls[ingressClassNameLabel] = lv("") } else { - ls[ingressClassLabel] = lv(*ingress.Spec.IngressClassName) + ls[ingressClassNameLabel] = lv(*ingress.Spec.IngressClassName) } for k, v := range ingress.Labels { diff --git a/discovery/kubernetes/ingress_test.go b/discovery/kubernetes/ingress_test.go index ff0cd46e84..02c566468a 100644 --- a/discovery/kubernetes/ingress_test.go +++ b/discovery/kubernetes/ingress_test.go @@ -33,7 +33,7 @@ const ( TLSMixed ) -func makeIngress(tls TLSMode, excludeClass bool) *v1beta1.Ingress { +func makeIngress(tls TLSMode, excludeClassName bool) *v1beta1.Ingress { ret := &v1beta1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: "testingress", @@ -82,7 +82,7 @@ func makeIngress(tls TLSMode, excludeClass bool) *v1beta1.Ingress { ret.Spec.TLS = []v1beta1.IngressTLS{{Hosts: []string{"example.com"}}} } - if excludeClass { + if excludeClassName { ret.Spec.IngressClassName = nil } @@ -93,7 +93,7 @@ func classString(v string) *string { return &v } -func expectedTargetGroups(ns string, tls TLSMode, excludeClass bool) map[string]*targetgroup.Group { +func expectedTargetGroups(ns string, tls TLSMode, excludeClassName bool) map[string]*targetgroup.Group { scheme1 := "http" scheme2 := "http" @@ -107,7 +107,7 @@ func expectedTargetGroups(ns string, tls TLSMode, excludeClass bool) map[string] scheme1 = "https" } - if excludeClass { + if excludeClassName { ingressClassName = "" } @@ -141,7 +141,7 @@ func expectedTargetGroups(ns string, tls TLSMode, excludeClass bool) map[string] "__meta_kubernetes_ingress_labelpresent_test_label": "true", "__meta_kubernetes_ingress_annotation_test_annotation": "testannotationvalue", "__meta_kubernetes_ingress_annotationpresent_test_annotation": "true", - "__meta_kubernetes_ingress_class": lv(ingressClassName), + "__meta_kubernetes_ingress_class_name": lv(ingressClassName), }, Source: key, }, diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index 70fd570c44..87a55051e8 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -1342,7 +1342,7 @@ Available meta labels: * `__meta_kubernetes_ingress_labelpresent_`: `true` for each label from the ingress object. * `__meta_kubernetes_ingress_annotation_`: Each annotation from the ingress object. * `__meta_kubernetes_ingress_annotationpresent_`: `true` for each annotation from the ingress object. -* `__meta_kubernetes_ingress_class`: Class from ingress spec, if present. +* `__meta_kubernetes_ingress_class_name`: Class name from ingress spec, if present. * `__meta_kubernetes_ingress_scheme`: Protocol scheme of ingress, `https` if TLS config is set. Defaults to `http`. * `__meta_kubernetes_ingress_path`: Path from ingress spec. Defaults to `/`. From 7941b350ba1b03b92defcf4cf828bcabec27d414 Mon Sep 17 00:00:00 2001 From: Mike Eves Date: Fri, 11 Jun 2021 15:14:23 +0100 Subject: [PATCH 5/6] Don't set label if ingressClassName is not set Signed-off-by: Mike Eves --- discovery/kubernetes/ingress.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/discovery/kubernetes/ingress.go b/discovery/kubernetes/ingress.go index 68b4797ed2..c09fb70904 100644 --- a/discovery/kubernetes/ingress.go +++ b/discovery/kubernetes/ingress.go @@ -155,9 +155,7 @@ func ingressLabels(ingress *v1beta1.Ingress) model.LabelSet { ls := make(model.LabelSet, 2*(len(ingress.Labels)+len(ingress.Annotations))+2) ls[ingressNameLabel] = lv(ingress.Name) ls[namespaceLabel] = lv(ingress.Namespace) - if ingress.Spec.IngressClassName == nil { - ls[ingressClassNameLabel] = lv("") - } else { + if ingress.Spec.IngressClassName != nil { ls[ingressClassNameLabel] = lv(*ingress.Spec.IngressClassName) } From ae081886fa23060fea34f8b598fbe06be236d1eb Mon Sep 17 00:00:00 2001 From: Mike Eves Date: Fri, 11 Jun 2021 15:49:13 +0100 Subject: [PATCH 6/6] Fix up tests Signed-off-by: Mike Eves --- discovery/kubernetes/ingress_test.go | 48 +++++++--------------------- 1 file changed, 12 insertions(+), 36 deletions(-) diff --git a/discovery/kubernetes/ingress_test.go b/discovery/kubernetes/ingress_test.go index 02c566468a..5ae5d4980c 100644 --- a/discovery/kubernetes/ingress_test.go +++ b/discovery/kubernetes/ingress_test.go @@ -33,7 +33,7 @@ const ( TLSMixed ) -func makeIngress(tls TLSMode, excludeClassName bool) *v1beta1.Ingress { +func makeIngress(tls TLSMode) *v1beta1.Ingress { ret := &v1beta1.Ingress{ ObjectMeta: metav1.ObjectMeta{ Name: "testingress", @@ -82,10 +82,6 @@ func makeIngress(tls TLSMode, excludeClassName bool) *v1beta1.Ingress { ret.Spec.TLS = []v1beta1.IngressTLS{{Hosts: []string{"example.com"}}} } - if excludeClassName { - ret.Spec.IngressClassName = nil - } - return ret } @@ -93,12 +89,10 @@ func classString(v string) *string { return &v } -func expectedTargetGroups(ns string, tls TLSMode, excludeClassName bool) map[string]*targetgroup.Group { +func expectedTargetGroups(ns string, tls TLSMode) map[string]*targetgroup.Group { scheme1 := "http" scheme2 := "http" - ingressClassName := "testclass" - switch tls { case TLSYes: scheme1 = "https" @@ -107,10 +101,6 @@ func expectedTargetGroups(ns string, tls TLSMode, excludeClassName bool) map[str scheme1 = "https" } - if excludeClassName { - ingressClassName = "" - } - key := fmt.Sprintf("ingress/%s/testingress", ns) return map[string]*targetgroup.Group{ key: { @@ -141,7 +131,7 @@ func expectedTargetGroups(ns string, tls TLSMode, excludeClassName bool) map[str "__meta_kubernetes_ingress_labelpresent_test_label": "true", "__meta_kubernetes_ingress_annotation_test_annotation": "testannotationvalue", "__meta_kubernetes_ingress_annotationpresent_test_annotation": "true", - "__meta_kubernetes_ingress_class_name": lv(ingressClassName), + "__meta_kubernetes_ingress_class_name": "testclass", }, Source: key, }, @@ -154,11 +144,11 @@ func TestIngressDiscoveryAdd(t *testing.T) { k8sDiscoveryTest{ discovery: n, afterStart: func() { - obj := makeIngress(TLSNo, false) + obj := makeIngress(TLSNo) c.NetworkingV1beta1().Ingresses("default").Create(context.Background(), obj, metav1.CreateOptions{}) }, expectedMaxItems: 1, - expectedRes: expectedTargetGroups("default", TLSNo, false), + expectedRes: expectedTargetGroups("default", TLSNo), }.Run(t) } @@ -168,11 +158,11 @@ func TestIngressDiscoveryAddTLS(t *testing.T) { k8sDiscoveryTest{ discovery: n, afterStart: func() { - obj := makeIngress(TLSYes, false) + obj := makeIngress(TLSYes) c.NetworkingV1beta1().Ingresses("default").Create(context.Background(), obj, metav1.CreateOptions{}) }, expectedMaxItems: 1, - expectedRes: expectedTargetGroups("default", TLSYes, false), + expectedRes: expectedTargetGroups("default", TLSYes), }.Run(t) } @@ -182,40 +172,26 @@ func TestIngressDiscoveryAddMixed(t *testing.T) { k8sDiscoveryTest{ discovery: n, afterStart: func() { - obj := makeIngress(TLSMixed, false) + obj := makeIngress(TLSMixed) c.NetworkingV1beta1().Ingresses("default").Create(context.Background(), obj, metav1.CreateOptions{}) }, expectedMaxItems: 1, - expectedRes: expectedTargetGroups("default", TLSMixed, false), - }.Run(t) -} - -func TestIngressDiscoveryAddNoClass(t *testing.T) { - n, c := makeDiscovery(RoleIngress, NamespaceDiscovery{Names: []string{"default"}}) - - k8sDiscoveryTest{ - discovery: n, - afterStart: func() { - obj := makeIngress(TLSMixed, true) - c.NetworkingV1beta1().Ingresses("default").Create(context.Background(), obj, metav1.CreateOptions{}) - }, - expectedMaxItems: 1, - expectedRes: expectedTargetGroups("default", TLSMixed, true), + expectedRes: expectedTargetGroups("default", TLSMixed), }.Run(t) } func TestIngressDiscoveryNamespaces(t *testing.T) { n, c := makeDiscovery(RoleIngress, NamespaceDiscovery{Names: []string{"ns1", "ns2"}}) - expected := expectedTargetGroups("ns1", TLSNo, false) - for k, v := range expectedTargetGroups("ns2", TLSNo, false) { + expected := expectedTargetGroups("ns1", TLSNo) + for k, v := range expectedTargetGroups("ns2", TLSNo) { expected[k] = v } k8sDiscoveryTest{ discovery: n, afterStart: func() { for _, ns := range []string{"ns1", "ns2"} { - obj := makeIngress(TLSNo, false) + obj := makeIngress(TLSNo) obj.Namespace = ns c.NetworkingV1beta1().Ingresses(obj.Namespace).Create(context.Background(), obj, metav1.CreateOptions{}) }