From 12e74ff3e53d8886a08fa4f568f51d55aeb25136 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Mon, 25 Jul 2022 15:56:36 +0100 Subject: [PATCH 01/20] Example test to display error event for invalid annotation name --- examples/complete-example/cafe-ingress.yaml | 2 ++ internal/k8s/validation.go | 15 +++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/examples/complete-example/cafe-ingress.yaml b/examples/complete-example/cafe-ingress.yaml index 50a8893990..f78ea3b42c 100644 --- a/examples/complete-example/cafe-ingress.yaml +++ b/examples/complete-example/cafe-ingress.yaml @@ -2,6 +2,8 @@ apiVersion: networking.k8s.io/v1 kind: Ingress metadata: name: cafe-ingress + annotations: + nginx.org/lb-met: "round_robin" spec: ingressClassName: nginx tls: diff --git a/internal/k8s/validation.go b/internal/k8s/validation.go index 98a74e5235..820384cb88 100644 --- a/internal/k8s/validation.go +++ b/internal/k8s/validation.go @@ -484,6 +484,12 @@ func validateIngressAnnotations( ) field.ErrorList { allErrs := field.ErrorList{} + for key := range annotations { + if !stringInSlice(key, annotationNames) { + allErrs = append(allErrs, field.Invalid(fieldPath, key, fmt.Sprintf("invalid annotation provided"))) + } + } + for _, name := range annotationNames { if value, exists := annotations[name]; exists { context := &annotationValidationContext{ @@ -505,6 +511,15 @@ func validateIngressAnnotations( return allErrs } +func stringInSlice(a string, list []string) bool { + for _, b := range list { + if b == a { + return true + } + } + return false +} + func validateIngressAnnotation(context *annotationValidationContext) field.ErrorList { allErrs := field.ErrorList{} if validationFuncs, exists := annotationValidations[context.name]; exists { From fa2fd08ddfa84ccd818a66442504df143f5c6173 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Mon, 25 Jul 2022 16:30:48 +0100 Subject: [PATCH 02/20] Remove validation test --- internal/k8s/validation.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/internal/k8s/validation.go b/internal/k8s/validation.go index 820384cb88..2486e24c7b 100644 --- a/internal/k8s/validation.go +++ b/internal/k8s/validation.go @@ -484,12 +484,6 @@ func validateIngressAnnotations( ) field.ErrorList { allErrs := field.ErrorList{} - for key := range annotations { - if !stringInSlice(key, annotationNames) { - allErrs = append(allErrs, field.Invalid(fieldPath, key, fmt.Sprintf("invalid annotation provided"))) - } - } - for _, name := range annotationNames { if value, exists := annotations[name]; exists { context := &annotationValidationContext{ From 99422d7d985a4607de41eacdd1ce4aba5985dddb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Tue, 16 Aug 2022 14:24:32 +0100 Subject: [PATCH 03/20] Add support for wildcard hostnames in VirtualServer --- deployments/service/loadbalancer-aws-elb.yaml | 4 ++-- pkg/apis/configuration/validation/virtualserver.go | 7 +++++-- pkg/apis/configuration/validation/virtualserver_test.go | 1 + 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/deployments/service/loadbalancer-aws-elb.yaml b/deployments/service/loadbalancer-aws-elb.yaml index d8b8aec359..ee66f46244 100644 --- a/deployments/service/loadbalancer-aws-elb.yaml +++ b/deployments/service/loadbalancer-aws-elb.yaml @@ -4,8 +4,8 @@ metadata: name: nginx-ingress namespace: nginx-ingress annotations: - service.beta.kubernetes.io/aws-load-balancer-backend-protocol: "tcp" - service.beta.kubernetes.io/aws-load-balancer-proxy-protocol: "*" + service.beta.kubernetes.io/aws-load-balancer-type: "nlb" + service.beta.kubernetes.io/aws-load-balancer-nlb-target-type: "ip" spec: type: LoadBalancer ports: diff --git a/pkg/apis/configuration/validation/virtualserver.go b/pkg/apis/configuration/validation/virtualserver.go index 29a937f7cb..7ece2b9c8b 100644 --- a/pkg/apis/configuration/validation/virtualserver.go +++ b/pkg/apis/configuration/validation/virtualserver.go @@ -99,8 +99,11 @@ func validateHost(host string, fieldPath *field.Path) field.ErrorList { return append(allErrs, field.Required(fieldPath, "")) } - for _, msg := range validation.IsDNS1123Subdomain(host) { - allErrs = append(allErrs, field.Invalid(fieldPath, host, msg)) + // Allow wildcard hosts e.g. *.example.com + if !strings.HasPrefix(host, "*.") { + for _, msg := range validation.IsDNS1123Subdomain(host) { + allErrs = append(allErrs, field.Invalid(fieldPath, host, msg)) + } } return allErrs diff --git a/pkg/apis/configuration/validation/virtualserver_test.go b/pkg/apis/configuration/validation/virtualserver_test.go index d982b905b9..00f60859ce 100644 --- a/pkg/apis/configuration/validation/virtualserver_test.go +++ b/pkg/apis/configuration/validation/virtualserver_test.go @@ -74,6 +74,7 @@ func TestValidateHost(t *testing.T) { "hello", "example.com", "hello-world-1", + "*.example.com", } for _, h := range validHosts { From 04f78dcdc51dbcaedb6407719803da070e6925c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Thu, 18 Aug 2022 10:51:10 +0100 Subject: [PATCH 04/20] Update buildDNSEndpoint to use vs.ObjectMeta.Name instead of vs.Spec.Host as the name --- internal/externaldns/sync.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/externaldns/sync.go b/internal/externaldns/sync.go index cd131ce3ad..78b3846359 100644 --- a/internal/externaldns/sync.go +++ b/internal/externaldns/sync.go @@ -139,7 +139,7 @@ func getValidTargets(endpoints []vsapi.ExternalEndpoint) (extdnsapi.Targets, str func buildDNSEndpoint(extdnsLister extdnslisters.DNSEndpointLister, vs *vsapi.VirtualServer, targets extdnsapi.Targets, recordType string) (*extdnsapi.DNSEndpoint, *extdnsapi.DNSEndpoint, error) { var updateDNSEndpoint *extdnsapi.DNSEndpoint var newDNSEndpoint *extdnsapi.DNSEndpoint - existingDNSEndpoint, err := extdnsLister.DNSEndpoints(vs.Namespace).Get(vs.Spec.Host) + existingDNSEndpoint, err := extdnsLister.DNSEndpoints(vs.Namespace).Get(vs.ObjectMeta.Name) if !apierrors.IsNotFound(err) && err != nil { return nil, nil, err } @@ -147,7 +147,7 @@ func buildDNSEndpoint(extdnsLister extdnslisters.DNSEndpointLister, vs *vsapi.Vi dnsEndpoint := &extdnsapi.DNSEndpoint{ ObjectMeta: metav1.ObjectMeta{ - Name: vs.Spec.Host, + Name: vs.ObjectMeta.Name, Namespace: vs.Namespace, Labels: vs.Labels, OwnerReferences: []metav1.OwnerReference{*metav1.NewControllerRef(vs, controllerGVK)}, From fa37731e62dfc1365a2438e81b79ba4f73debcae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Thu, 18 Aug 2022 10:54:25 +0100 Subject: [PATCH 05/20] Remove unused function --- internal/k8s/validation.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/internal/k8s/validation.go b/internal/k8s/validation.go index 2486e24c7b..98a74e5235 100644 --- a/internal/k8s/validation.go +++ b/internal/k8s/validation.go @@ -505,15 +505,6 @@ func validateIngressAnnotations( return allErrs } -func stringInSlice(a string, list []string) bool { - for _, b := range list { - if b == a { - return true - } - } - return false -} - func validateIngressAnnotation(context *annotationValidationContext) field.ErrorList { allErrs := field.ErrorList{} if validationFuncs, exists := annotationValidations[context.name]; exists { From 4f96e56f58157eca27b2a3a39435e5d69bfc7630 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Thu, 18 Aug 2022 10:57:42 +0100 Subject: [PATCH 06/20] Revert example configuration --- examples/complete-example/cafe-ingress.yaml | 2 -- pkg/apis/configuration/validation/virtualserver.go | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/examples/complete-example/cafe-ingress.yaml b/examples/complete-example/cafe-ingress.yaml index f78ea3b42c..50a8893990 100644 --- a/examples/complete-example/cafe-ingress.yaml +++ b/examples/complete-example/cafe-ingress.yaml @@ -2,8 +2,6 @@ apiVersion: networking.k8s.io/v1 kind: Ingress metadata: name: cafe-ingress - annotations: - nginx.org/lb-met: "round_robin" spec: ingressClassName: nginx tls: diff --git a/pkg/apis/configuration/validation/virtualserver.go b/pkg/apis/configuration/validation/virtualserver.go index 7ece2b9c8b..2af4b00915 100644 --- a/pkg/apis/configuration/validation/virtualserver.go +++ b/pkg/apis/configuration/validation/virtualserver.go @@ -98,8 +98,7 @@ func validateHost(host string, fieldPath *field.Path) field.ErrorList { if host == "" { return append(allErrs, field.Required(fieldPath, "")) } - - // Allow wildcard hosts e.g. *.example.com + if !strings.HasPrefix(host, "*.") { for _, msg := range validation.IsDNS1123Subdomain(host) { allErrs = append(allErrs, field.Invalid(fieldPath, host, msg)) From 3dd09b0bedf8d19c7a5e50d263ce184d6e486040 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Thu, 18 Aug 2022 11:00:25 +0100 Subject: [PATCH 07/20] Add a const for wildcardHost symbol --- pkg/apis/configuration/validation/virtualserver.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/pkg/apis/configuration/validation/virtualserver.go b/pkg/apis/configuration/validation/virtualserver.go index 2af4b00915..113967353d 100644 --- a/pkg/apis/configuration/validation/virtualserver.go +++ b/pkg/apis/configuration/validation/virtualserver.go @@ -92,14 +92,16 @@ func (vsv *VirtualServerValidator) validateVirtualServerSpec(spec *v1.VirtualSer return allErrs } +const wildcardHost = "*." + func validateHost(host string, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} if host == "" { return append(allErrs, field.Required(fieldPath, "")) } - - if !strings.HasPrefix(host, "*.") { + + if !strings.HasPrefix(host, wildcardHost) { for _, msg := range validation.IsDNS1123Subdomain(host) { allErrs = append(allErrs, field.Invalid(fieldPath, host, msg)) } From 6c72b3c5e5ffecc2c5702e278dd1b37f028e9886 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Mon, 22 Aug 2022 17:00:57 +0100 Subject: [PATCH 08/20] Add check for host name collision with wildcards --- .../deployment/nginx-plus-ingress.yaml | 2 +- .../cafe-virtual-server.yaml | 4 +- .../external-dns/cafe-virtual-server.yaml | 2 +- internal/k8s/configuration.go | 44 ++++++++++++++++++- 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/deployments/deployment/nginx-plus-ingress.yaml b/deployments/deployment/nginx-plus-ingress.yaml index 0d8096b642..c7b1346e2b 100644 --- a/deployments/deployment/nginx-plus-ingress.yaml +++ b/deployments/deployment/nginx-plus-ingress.yaml @@ -19,7 +19,7 @@ spec: spec: serviceAccountName: nginx-ingress containers: - - image: nginx-plus-ingress:2.3.0 + - image: nginx/nginx-ingress:2.3.0-SNAPSHOT-3dd09b0 imagePullPolicy: IfNotPresent name: nginx-plus-ingress ports: diff --git a/examples/custom-resources/basic-configuration/cafe-virtual-server.yaml b/examples/custom-resources/basic-configuration/cafe-virtual-server.yaml index c87af04890..5835456439 100644 --- a/examples/custom-resources/basic-configuration/cafe-virtual-server.yaml +++ b/examples/custom-resources/basic-configuration/cafe-virtual-server.yaml @@ -1,9 +1,9 @@ apiVersion: k8s.nginx.org/v1 kind: VirtualServer metadata: - name: cafe + name: cafe-1 spec: - host: cafe.example.com + host: "*.example.com" tls: secret: cafe-secret upstreams: diff --git a/examples/custom-resources/external-dns/cafe-virtual-server.yaml b/examples/custom-resources/external-dns/cafe-virtual-server.yaml index b7f6f6e47d..a3ab6f0526 100644 --- a/examples/custom-resources/external-dns/cafe-virtual-server.yaml +++ b/examples/custom-resources/external-dns/cafe-virtual-server.yaml @@ -3,7 +3,7 @@ kind: VirtualServer metadata: name: cafe spec: - host: cafe.example.com + host: cafe.kic.nginx.com tls: secret: cafe-secret externalDNS: diff --git a/internal/k8s/configuration.go b/internal/k8s/configuration.go index 46aee6fefc..cff6ffbe7d 100644 --- a/internal/k8s/configuration.go +++ b/internal/k8s/configuration.go @@ -1054,7 +1054,9 @@ func (c *Configuration) addProblemsForResourcesWithoutActiveHost(resources map[s problems[r.GetKeyWithKind()] = p } case *VirtualServerConfiguration: - res := c.hosts[impl.VirtualServer.Spec.Host] + //res := c.hosts[impl.VirtualServer.Spec.Host] + + res, _ := checkHostCollision(impl.VirtualServer.Spec.Host, c.hosts) if res.GetKeyWithKind() != r.GetKeyWithKind() { p := ConfigurationProblem{ @@ -1346,11 +1348,24 @@ func (c *Configuration) buildHostsAndResources() (newHosts map[string]Resource, newResources[resource.GetKeyWithKind()] = resource holder, exists := newHosts[vs.Spec.Host] - if !exists { + var isHostCollision bool + if len(newHosts) > 0 { + if holder, isHostCollision = checkHostCollision(vs.Spec.Host, newHosts); !exists && !isHostCollision { + newHosts[vs.Spec.Host] = resource + continue + } + } else { newHosts[vs.Spec.Host] = resource continue } + //holder, exists := newHosts[vs.Spec.Host] + // + //if !exists { + // newHosts[vs.Spec.Host] = resource + // continue + //} + warning := fmt.Sprintf("host %s is taken by another resource", vs.Spec.Host) if !holder.Wins(resource) { @@ -1394,6 +1409,31 @@ func (c *Configuration) buildHostsAndResources() (newHosts map[string]Resource, return newHosts, newResources } +func checkHostCollision(newHost string, existingHosts map[string]Resource) (Resource, bool) { + wildcardPrefix := "*." + isHostCollision := false + var holder Resource + if strings.HasPrefix(newHost, wildcardPrefix) { + newHostSuffix := strings.TrimPrefix(newHost, wildcardPrefix) + for existingHost := range existingHosts { + if strings.HasSuffix(existingHost, newHostSuffix) { + isHostCollision = true + holder = existingHosts[existingHost] + } + } + } else { + for existingHost := range existingHosts { + existingHostSuffix := strings.TrimPrefix(existingHost, wildcardPrefix) + if strings.HasSuffix(newHost, existingHostSuffix) { + isHostCollision = true + holder = existingHosts[existingHost] + } + } + } + return holder, isHostCollision + +} + func (c *Configuration) isChallengeIngress(ing *networking.Ingress) bool { if !c.isCertManagerEnabled { return false From bc405c5f20f037d635d8943fe79c6a4d6c8973a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Mon, 22 Aug 2022 17:03:02 +0100 Subject: [PATCH 09/20] Revert host collision update --- internal/k8s/configuration.go | 45 +++-------------------------------- 1 file changed, 3 insertions(+), 42 deletions(-) diff --git a/internal/k8s/configuration.go b/internal/k8s/configuration.go index cff6ffbe7d..3eec1edfd0 100644 --- a/internal/k8s/configuration.go +++ b/internal/k8s/configuration.go @@ -1054,9 +1054,7 @@ func (c *Configuration) addProblemsForResourcesWithoutActiveHost(resources map[s problems[r.GetKeyWithKind()] = p } case *VirtualServerConfiguration: - //res := c.hosts[impl.VirtualServer.Spec.Host] - - res, _ := checkHostCollision(impl.VirtualServer.Spec.Host, c.hosts) + res := c.hosts[impl.VirtualServer.Spec.Host] if res.GetKeyWithKind() != r.GetKeyWithKind() { p := ConfigurationProblem{ @@ -1348,24 +1346,12 @@ func (c *Configuration) buildHostsAndResources() (newHosts map[string]Resource, newResources[resource.GetKeyWithKind()] = resource holder, exists := newHosts[vs.Spec.Host] - var isHostCollision bool - if len(newHosts) > 0 { - if holder, isHostCollision = checkHostCollision(vs.Spec.Host, newHosts); !exists && !isHostCollision { - newHosts[vs.Spec.Host] = resource - continue - } - } else { + + if !exists { newHosts[vs.Spec.Host] = resource continue } - //holder, exists := newHosts[vs.Spec.Host] - // - //if !exists { - // newHosts[vs.Spec.Host] = resource - // continue - //} - warning := fmt.Sprintf("host %s is taken by another resource", vs.Spec.Host) if !holder.Wins(resource) { @@ -1409,31 +1395,6 @@ func (c *Configuration) buildHostsAndResources() (newHosts map[string]Resource, return newHosts, newResources } -func checkHostCollision(newHost string, existingHosts map[string]Resource) (Resource, bool) { - wildcardPrefix := "*." - isHostCollision := false - var holder Resource - if strings.HasPrefix(newHost, wildcardPrefix) { - newHostSuffix := strings.TrimPrefix(newHost, wildcardPrefix) - for existingHost := range existingHosts { - if strings.HasSuffix(existingHost, newHostSuffix) { - isHostCollision = true - holder = existingHosts[existingHost] - } - } - } else { - for existingHost := range existingHosts { - existingHostSuffix := strings.TrimPrefix(existingHost, wildcardPrefix) - if strings.HasSuffix(newHost, existingHostSuffix) { - isHostCollision = true - holder = existingHosts[existingHost] - } - } - } - return holder, isHostCollision - -} - func (c *Configuration) isChallengeIngress(ing *networking.Ingress) bool { if !c.isCertManagerEnabled { return false From 36c34614f8f9434610bbe9faff117fbdcc3dbde1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Mon, 22 Aug 2022 17:07:32 +0100 Subject: [PATCH 10/20] Revert example files --- deployments/deployment/nginx-plus-ingress.yaml | 2 +- .../basic-configuration/cafe-virtual-server.yaml | 4 ++-- .../custom-resources/external-dns/cafe-virtual-server.yaml | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/deployments/deployment/nginx-plus-ingress.yaml b/deployments/deployment/nginx-plus-ingress.yaml index c7b1346e2b..0d8096b642 100644 --- a/deployments/deployment/nginx-plus-ingress.yaml +++ b/deployments/deployment/nginx-plus-ingress.yaml @@ -19,7 +19,7 @@ spec: spec: serviceAccountName: nginx-ingress containers: - - image: nginx/nginx-ingress:2.3.0-SNAPSHOT-3dd09b0 + - image: nginx-plus-ingress:2.3.0 imagePullPolicy: IfNotPresent name: nginx-plus-ingress ports: diff --git a/examples/custom-resources/basic-configuration/cafe-virtual-server.yaml b/examples/custom-resources/basic-configuration/cafe-virtual-server.yaml index 5835456439..c87af04890 100644 --- a/examples/custom-resources/basic-configuration/cafe-virtual-server.yaml +++ b/examples/custom-resources/basic-configuration/cafe-virtual-server.yaml @@ -1,9 +1,9 @@ apiVersion: k8s.nginx.org/v1 kind: VirtualServer metadata: - name: cafe-1 + name: cafe spec: - host: "*.example.com" + host: cafe.example.com tls: secret: cafe-secret upstreams: diff --git a/examples/custom-resources/external-dns/cafe-virtual-server.yaml b/examples/custom-resources/external-dns/cafe-virtual-server.yaml index a3ab6f0526..b7f6f6e47d 100644 --- a/examples/custom-resources/external-dns/cafe-virtual-server.yaml +++ b/examples/custom-resources/external-dns/cafe-virtual-server.yaml @@ -3,7 +3,7 @@ kind: VirtualServer metadata: name: cafe spec: - host: cafe.kic.nginx.com + host: cafe.example.com tls: secret: cafe-secret externalDNS: From ef31cb3ea921585e8121919b331ebed69dff8906 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Tue, 23 Aug 2022 10:20:08 +0100 Subject: [PATCH 11/20] Update python test for externaldns --- deployments/deployment/nginx-plus-ingress.yaml | 2 +- tests/suite/test_virtual_server_externaldns.py | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/deployments/deployment/nginx-plus-ingress.yaml b/deployments/deployment/nginx-plus-ingress.yaml index 0d8096b642..65aa7a717c 100644 --- a/deployments/deployment/nginx-plus-ingress.yaml +++ b/deployments/deployment/nginx-plus-ingress.yaml @@ -19,7 +19,7 @@ spec: spec: serviceAccountName: nginx-ingress containers: - - image: nginx-plus-ingress:2.3.0 + - image: nginx/nginx-ingress:2.3.0-SNAPSHOT-36c3461 imagePullPolicy: IfNotPresent name: nginx-plus-ingress ports: diff --git a/tests/suite/test_virtual_server_externaldns.py b/tests/suite/test_virtual_server_externaldns.py index 574a92c6f2..7e0860dff5 100644 --- a/tests/suite/test_virtual_server_externaldns.py +++ b/tests/suite/test_virtual_server_externaldns.py @@ -5,7 +5,7 @@ from suite.custom_resources_utils import is_dnsendpoint_present from suite.resources_utils import wait_before_test, get_events from suite.vs_vsr_resources_utils import patch_virtual_server_from_yaml -from suite.yaml_utils import get_first_host_from_yaml, get_namespace_from_yaml +from suite.yaml_utils import get_name_from_yaml, get_namespace_from_yaml VS_YAML = f"{TEST_DATA}/virtual-server-external-dns/standard/virtual-server.yaml" @@ -19,11 +19,11 @@ class TestExternalDNSVirtualServer: def test_responses_after_setup(self, kube_apis, crd_ingress_controller_with_ed, create_externaldns, virtual_server_setup): print("\nStep 1: Verify DNSEndpoint exists") - dns_name = get_first_host_from_yaml(VS_YAML) + dns_ep_name = get_name_from_yaml(VS_YAML) retry = 0 - dep = is_dnsendpoint_present(kube_apis.custom_objects, dns_name, virtual_server_setup.namespace) + dep = is_dnsendpoint_present(kube_apis.custom_objects, dns_ep_name, virtual_server_setup.namespace) while dep == False and retry <= 60: - dep = is_dnsendpoint_present(kube_apis.custom_objects, dns_name, virtual_server_setup.namespace) + dep = is_dnsendpoint_present(kube_apis.custom_objects, dns_ep_name, virtual_server_setup.namespace) retry += 1 wait_before_test(1) print(f"DNSEndpoint not created, retrying... #{retry}") From 3b5abb8d74061e81ff83af476185b3c5026286fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Tue, 23 Aug 2022 11:30:55 +0100 Subject: [PATCH 12/20] Fix error vs.ObjectMeta.Host undefined --- deployments/deployment/nginx-plus-ingress.yaml | 2 +- internal/externaldns/sync.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/deployments/deployment/nginx-plus-ingress.yaml b/deployments/deployment/nginx-plus-ingress.yaml index 65aa7a717c..0d8096b642 100644 --- a/deployments/deployment/nginx-plus-ingress.yaml +++ b/deployments/deployment/nginx-plus-ingress.yaml @@ -19,7 +19,7 @@ spec: spec: serviceAccountName: nginx-ingress containers: - - image: nginx/nginx-ingress:2.3.0-SNAPSHOT-36c3461 + - image: nginx-plus-ingress:2.3.0 imagePullPolicy: IfNotPresent name: nginx-plus-ingress ports: diff --git a/internal/externaldns/sync.go b/internal/externaldns/sync.go index 08a8548ee8..debaf6a6c1 100644 --- a/internal/externaldns/sync.go +++ b/internal/externaldns/sync.go @@ -142,7 +142,7 @@ func buildDNSEndpoint(extdnsLister []extdnslisters.DNSEndpointLister, vs *vsapi. var existingDNSEndpoint *extdnsapi.DNSEndpoint var err error for _, el := range extdnsLister { - existingDNSEndpoint, err = el.DNSEndpoints(vs.Namespace).Get(vs.ObjectMeta.Host) + existingDNSEndpoint, err = el.DNSEndpoints(vs.Namespace).Get(vs.ObjectMeta.Name) if err == nil { break } From 908782af46faaa89c59d67d8f077311644bb6549 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Tue, 23 Aug 2022 14:16:28 +0100 Subject: [PATCH 13/20] Remove space --- internal/k8s/configuration.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/k8s/configuration.go b/internal/k8s/configuration.go index 3eec1edfd0..46aee6fefc 100644 --- a/internal/k8s/configuration.go +++ b/internal/k8s/configuration.go @@ -1346,7 +1346,6 @@ func (c *Configuration) buildHostsAndResources() (newHosts map[string]Resource, newResources[resource.GetKeyWithKind()] = resource holder, exists := newHosts[vs.Spec.Host] - if !exists { newHosts[vs.Spec.Host] = resource continue From 8cd30af60c652b67b079dbb22545b1f89b32c255 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Wed, 24 Aug 2022 12:01:47 +0100 Subject: [PATCH 14/20] Update documentation --- .../virtualserver-and-virtualserverroute-resources.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/content/configuration/virtualserver-and-virtualserverroute-resources.md b/docs/content/configuration/virtualserver-and-virtualserverroute-resources.md index 3fa0462a5d..3d01f280f3 100644 --- a/docs/content/configuration/virtualserver-and-virtualserverroute-resources.md +++ b/docs/content/configuration/virtualserver-and-virtualserverroute-resources.md @@ -51,7 +51,7 @@ spec: {{% table %}} |Field | Description | Type | Required | | ---| ---| ---| --- | -|``host`` | The host (domain name) of the server. Must be a valid subdomain as defined in RFC 1123, such as ``my-app`` or ``hello.example.com``. Wildcard domains like ``*.example.com`` are not allowed. The ``host`` value needs to be unique among all Ingress and VirtualServer resources. See also [Handling Host and Listener Collisions](/nginx-ingress-controller/configuration/handling-host-and-listener-collisions). | ``string`` | Yes | +|``host`` | The host (domain name) of the server. Must be a valid subdomain as defined in RFC 1123, such as ``my-app`` or ``hello.example.com``. When using a wildcard domain like ``*.example.com`` the domain must be contained in double quotes. The ``host`` value needs to be unique among all Ingress and VirtualServer resources. See also [Handling Host and Listener Collisions](/nginx-ingress-controller/configuration/handling-host-and-listener-collisions). | ``string`` | Yes | |``tls`` | The TLS termination configuration. | [tls](#virtualservertls) | No | |``externalDNS`` | The externalDNS configuration for a VirtualServer. | [externalDNS](#virtualserverexternaldns) | No | |``dos`` | A reference to a DosProtectedResource, setting this enables DOS protection of the VirtualServer. | ``string`` | No | @@ -249,7 +249,7 @@ Note that each subroute must have a `path` that starts with the same prefix (her {{% table %}} |Field | Description | Type | Required | | ---| ---| ---| --- | -|``host`` | The host (domain name) of the server. Must be a valid subdomain as defined in RFC 1123, such as ``my-app`` or ``hello.example.com``. Wildcard domains like ``*.example.com`` are not allowed. Must be the same as the ``host`` of the VirtualServer that references this resource. | ``string`` | Yes | +|``host`` | The host (domain name) of the server. Must be a valid subdomain as defined in RFC 1123, such as ``my-app`` or ``hello.example.com``. When using a wildcard domain like ``*.example.com`` the domain must be contained in double quotes. Must be the same as the ``host`` of the VirtualServer that references this resource. | ``string`` | Yes | |``upstreams`` | A list of upstreams. | [[]upstream](#upstream) | No | |``subroutes`` | A list of subroutes. | [[]subroute](#virtualserverroutesubroute) | No | |``ingressClassName`` | Specifies which Ingress Controller must handle the VirtualServerRoute resource. Must be the same as the ``ingressClassName`` of the VirtualServer that references this resource. | ``string``_ | No | From b25d67609a66cc80f2b1e9ddfca976e12ce5be16 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 24 Aug 2022 12:34:48 +0100 Subject: [PATCH 15/20] add tests for vs wildcard support --- .../virtual-server-wildcard.yaml | 20 +++++++ tests/suite/test_virtual_server_wildcard.py | 52 +++++++++++++++++++ 2 files changed, 72 insertions(+) create mode 100644 tests/data/virtual-server-wildcard/virtual-server-wildcard.yaml create mode 100644 tests/suite/test_virtual_server_wildcard.py diff --git a/tests/data/virtual-server-wildcard/virtual-server-wildcard.yaml b/tests/data/virtual-server-wildcard/virtual-server-wildcard.yaml new file mode 100644 index 0000000000..c3dcffdf36 --- /dev/null +++ b/tests/data/virtual-server-wildcard/virtual-server-wildcard.yaml @@ -0,0 +1,20 @@ +apiVersion: k8s.nginx.org/v1 +kind: VirtualServer +metadata: + name: virtual-server-wildcard +spec: + host: "*.example.com" + upstreams: + - name: backend2 + service: backend2-svc + port: 80 + - name: backend1 + service: backend1-svc + port: 80 + routes: + - path: "/backend1" + action: + pass: backend1 + - path: "/backend2" + action: + pass: backend2 \ No newline at end of file diff --git a/tests/suite/test_virtual_server_wildcard.py b/tests/suite/test_virtual_server_wildcard.py new file mode 100644 index 0000000000..2758e7f3f2 --- /dev/null +++ b/tests/suite/test_virtual_server_wildcard.py @@ -0,0 +1,52 @@ +import pytest +import requests +from settings import TEST_DATA +from suite.custom_assertions import wait_and_assert_status_code +from suite.custom_resources_utils import read_custom_resource +from suite.vs_vsr_resources_utils import ( + create_virtual_server_from_yaml, + delete_virtual_server, +) +from suite.resources_utils import wait_before_test + +@pytest.mark.vs +@pytest.mark.parametrize( + "crd_ingress_controller, virtual_server_setup", + [ + ( + {"type": "complete", "extra_args": [f"-enable-custom-resources"]}, + {"example": "virtual-server", "app_type": "simple"}, + ) + ], + indirect=True, +) +class TestVirtualServerWildcard: + + def test_vs_status(self, kube_apis, crd_ingress_controller, virtual_server_setup): + + wait_and_assert_status_code(200, virtual_server_setup.backend_1_url, virtual_server_setup.vs_host) + wait_and_assert_status_code(200, virtual_server_setup.backend_2_url, virtual_server_setup.vs_host) + wait_and_assert_status_code(404, virtual_server_setup.backend_1_url, "test.example.com") + wait_and_assert_status_code(404, virtual_server_setup.backend_2_url, "test.example.com") + + # create virtual server with wildcard hostname + manifest_vs_wc = f"{TEST_DATA}/virtual-server-wildcard/virtual-server-wildcard.yaml" + vs_wc_name = create_virtual_server_from_yaml(kube_apis.custom_objects, manifest_vs_wc, virtual_server_setup.namespace) + wait_before_test() + response = read_custom_resource( + kube_apis.custom_objects, + virtual_server_setup.namespace, + "virtualservers", + vs_wc_name, + ) + assert ( + response["status"] + and response["status"]["reason"] == "AddedOrUpdated" + and response["status"]["state"] == "Valid" + ) + wait_and_assert_status_code(200, virtual_server_setup.backend_1_url, "test.example.com") + wait_and_assert_status_code(200, virtual_server_setup.backend_2_url, "test.example.com") + wait_and_assert_status_code(404, virtual_server_setup.backend_1_url, "test.xexample.com") + wait_and_assert_status_code(404, virtual_server_setup.backend_2_url, "test.xexample.com") + + delete_virtual_server(kube_apis.custom_objects, vs_wc_name, virtual_server_setup.namespace) \ No newline at end of file From 29a19b90b5daf4b8cb073bd5de32d07921362353 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 24 Aug 2022 13:32:37 +0100 Subject: [PATCH 16/20] lint and retry for vs status --- tests/suite/test_virtual_server_wildcard.py | 30 +++++++++++---------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/tests/suite/test_virtual_server_wildcard.py b/tests/suite/test_virtual_server_wildcard.py index 2758e7f3f2..454bebe652 100644 --- a/tests/suite/test_virtual_server_wildcard.py +++ b/tests/suite/test_virtual_server_wildcard.py @@ -1,13 +1,10 @@ import pytest -import requests from settings import TEST_DATA from suite.custom_assertions import wait_and_assert_status_code from suite.custom_resources_utils import read_custom_resource -from suite.vs_vsr_resources_utils import ( - create_virtual_server_from_yaml, - delete_virtual_server, -) from suite.resources_utils import wait_before_test +from suite.vs_vsr_resources_utils import create_virtual_server_from_yaml, delete_virtual_server + @pytest.mark.vs @pytest.mark.parametrize( @@ -21,9 +18,8 @@ indirect=True, ) class TestVirtualServerWildcard: - def test_vs_status(self, kube_apis, crd_ingress_controller, virtual_server_setup): - + wait_and_assert_status_code(200, virtual_server_setup.backend_1_url, virtual_server_setup.vs_host) wait_and_assert_status_code(200, virtual_server_setup.backend_2_url, virtual_server_setup.vs_host) wait_and_assert_status_code(404, virtual_server_setup.backend_1_url, "test.example.com") @@ -31,7 +27,9 @@ def test_vs_status(self, kube_apis, crd_ingress_controller, virtual_server_setup # create virtual server with wildcard hostname manifest_vs_wc = f"{TEST_DATA}/virtual-server-wildcard/virtual-server-wildcard.yaml" - vs_wc_name = create_virtual_server_from_yaml(kube_apis.custom_objects, manifest_vs_wc, virtual_server_setup.namespace) + vs_wc_name = create_virtual_server_from_yaml( + kube_apis.custom_objects, manifest_vs_wc, virtual_server_setup.namespace + ) wait_before_test() response = read_custom_resource( kube_apis.custom_objects, @@ -39,14 +37,18 @@ def test_vs_status(self, kube_apis, crd_ingress_controller, virtual_server_setup "virtualservers", vs_wc_name, ) - assert ( - response["status"] - and response["status"]["reason"] == "AddedOrUpdated" - and response["status"]["state"] == "Valid" - ) + while not response["status"]: + response = read_custom_resource( + kube_apis.custom_objects, + virtual_server_setup.namespace, + "virtualservers", + vs_wc_name, + ) + + assert response["status"]["reason"] == "AddedOrUpdated" and response["status"]["state"] == "Valid" wait_and_assert_status_code(200, virtual_server_setup.backend_1_url, "test.example.com") wait_and_assert_status_code(200, virtual_server_setup.backend_2_url, "test.example.com") wait_and_assert_status_code(404, virtual_server_setup.backend_1_url, "test.xexample.com") wait_and_assert_status_code(404, virtual_server_setup.backend_2_url, "test.xexample.com") - delete_virtual_server(kube_apis.custom_objects, vs_wc_name, virtual_server_setup.namespace) \ No newline at end of file + delete_virtual_server(kube_apis.custom_objects, vs_wc_name, virtual_server_setup.namespace) From 56aaca26a7b70cf09d430acf15d0e475c8e9a578 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 24 Aug 2022 14:43:32 +0100 Subject: [PATCH 17/20] fix retries --- tests/suite/test_virtual_server_wildcard.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/suite/test_virtual_server_wildcard.py b/tests/suite/test_virtual_server_wildcard.py index 454bebe652..67bd1aca0f 100644 --- a/tests/suite/test_virtual_server_wildcard.py +++ b/tests/suite/test_virtual_server_wildcard.py @@ -1,3 +1,4 @@ +import time import pytest from settings import TEST_DATA from suite.custom_assertions import wait_and_assert_status_code @@ -26,18 +27,17 @@ def test_vs_status(self, kube_apis, crd_ingress_controller, virtual_server_setup wait_and_assert_status_code(404, virtual_server_setup.backend_2_url, "test.example.com") # create virtual server with wildcard hostname + retry = 0 manifest_vs_wc = f"{TEST_DATA}/virtual-server-wildcard/virtual-server-wildcard.yaml" vs_wc_name = create_virtual_server_from_yaml( kube_apis.custom_objects, manifest_vs_wc, virtual_server_setup.namespace ) wait_before_test() - response = read_custom_resource( - kube_apis.custom_objects, - virtual_server_setup.namespace, - "virtualservers", - vs_wc_name, - ) - while not response["status"]: + response = {} + while ("status" not in response) and (retry <= 30): + print("Waiting for VS status update...") + time.sleep(1) + retry += 1 response = read_custom_resource( kube_apis.custom_objects, virtual_server_setup.namespace, From 9d06ca5595fada6d61364b2807f8c03144efbea6 Mon Sep 17 00:00:00 2001 From: Venktesh Date: Wed, 24 Aug 2022 14:46:17 +0100 Subject: [PATCH 18/20] isort --- tests/suite/test_virtual_server_wildcard.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/suite/test_virtual_server_wildcard.py b/tests/suite/test_virtual_server_wildcard.py index 67bd1aca0f..79a92d87cc 100644 --- a/tests/suite/test_virtual_server_wildcard.py +++ b/tests/suite/test_virtual_server_wildcard.py @@ -1,4 +1,5 @@ import time + import pytest from settings import TEST_DATA from suite.custom_assertions import wait_and_assert_status_code From c301b4e55ec3c42d82402bbccb8d72d6d0cecbfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Wed, 24 Aug 2022 15:04:53 +0100 Subject: [PATCH 19/20] Validate wildcard host --- pkg/apis/configuration/validation/virtualserver.go | 8 ++++++-- pkg/apis/configuration/validation/virtualserver_test.go | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/pkg/apis/configuration/validation/virtualserver.go b/pkg/apis/configuration/validation/virtualserver.go index 113967353d..aaa8de14c9 100644 --- a/pkg/apis/configuration/validation/virtualserver.go +++ b/pkg/apis/configuration/validation/virtualserver.go @@ -92,7 +92,7 @@ func (vsv *VirtualServerValidator) validateVirtualServerSpec(spec *v1.VirtualSer return allErrs } -const wildcardHost = "*." +const wildcardPrefix = "*." func validateHost(host string, fieldPath *field.Path) field.ErrorList { allErrs := field.ErrorList{} @@ -101,10 +101,14 @@ func validateHost(host string, fieldPath *field.Path) field.ErrorList { return append(allErrs, field.Required(fieldPath, "")) } - if !strings.HasPrefix(host, wildcardHost) { + if !strings.HasPrefix(host, wildcardPrefix) { for _, msg := range validation.IsDNS1123Subdomain(host) { allErrs = append(allErrs, field.Invalid(fieldPath, host, msg)) } + } else { + for _, msg := range validation.IsWildcardDNS1123Subdomain(host) { + allErrs = append(allErrs, field.Invalid(fieldPath, host, msg)) + } } return allErrs diff --git a/pkg/apis/configuration/validation/virtualserver_test.go b/pkg/apis/configuration/validation/virtualserver_test.go index 00f60859ce..de16fef91a 100644 --- a/pkg/apis/configuration/validation/virtualserver_test.go +++ b/pkg/apis/configuration/validation/virtualserver_test.go @@ -90,6 +90,7 @@ func TestValidateHost(t *testing.T) { "..", ".example.com", "-hello-world-1", + "*.-example.com", } for _, h := range invalidHosts { From cecc19576c6199fc32ad27e22cbfea4455e3ed7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E2=80=9Cshaun-nx=E2=80=9D?= <“s.odonovan@f5.com”> Date: Thu, 25 Aug 2022 14:43:33 +0100 Subject: [PATCH 20/20] Invert logic --- pkg/apis/configuration/validation/virtualserver.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/apis/configuration/validation/virtualserver.go b/pkg/apis/configuration/validation/virtualserver.go index aaa8de14c9..8a8805f690 100644 --- a/pkg/apis/configuration/validation/virtualserver.go +++ b/pkg/apis/configuration/validation/virtualserver.go @@ -101,12 +101,12 @@ func validateHost(host string, fieldPath *field.Path) field.ErrorList { return append(allErrs, field.Required(fieldPath, "")) } - if !strings.HasPrefix(host, wildcardPrefix) { - for _, msg := range validation.IsDNS1123Subdomain(host) { + if strings.HasPrefix(host, wildcardPrefix) { + for _, msg := range validation.IsWildcardDNS1123Subdomain(host) { allErrs = append(allErrs, field.Invalid(fieldPath, host, msg)) } } else { - for _, msg := range validation.IsWildcardDNS1123Subdomain(host) { + for _, msg := range validation.IsDNS1123Subdomain(host) { allErrs = append(allErrs, field.Invalid(fieldPath, host, msg)) } }