From 0a4f032f275814738e02d9772c2be0a3947b18a2 Mon Sep 17 00:00:00 2001 From: Akshay Gupta Date: Fri, 18 Feb 2022 14:51:01 -0800 Subject: [PATCH] Merge master into release/1.5 (#1349) * fixed subscriptionId description (#1345) * k8s: Add retry logic to get server version (#1344) * add retry logic to get server version * add output of server version Co-authored-by: Akshay Gupta * networking/v1: add support for ingress path type (#1346) * add support for ingress path type * add test for / * fix: don't initialize ingress class when k8s doesn't support v1/ingress (#1347) * fix: panic when ingress class is not supported on k8s * add test * docs: add changelog 1.5 (#1348) * add changelog 1.5 * add retry fix * fix reference Co-authored-by: Carlos Mendible Co-authored-by: Shuanglu --- CHANGELOG/CHANGELOG-1.5.md | 69 +++++++ docs/developers/build.md | 2 +- pkg/appgw/configbuilder.go | 3 +- pkg/appgw/ingress_rules.go | 2 +- pkg/appgw/requestroutingrules.go | 30 +++- pkg/appgw/requestroutingrules_test.go | 117 ++++++++++++ pkg/k8scontext/context.go | 18 +- pkg/k8scontext/k8scontext_test.go | 40 ++++- pkg/k8scontext/supported_apiversion.go | 27 ++- pkg/tests/fixtures.go | 159 ++++++++++++++++ ...g-v1-mfu_one_namespace_one_ingress_test.go | 50 +++++- .../path-type/app.yaml | 169 ++++++++++++++++++ 12 files changed, 666 insertions(+), 20 deletions(-) create mode 100644 CHANGELOG/CHANGELOG-1.5.md create mode 100644 scripts/e2e/cmd/runner/testdata/networking-v1/one-namespace-one-ingress/path-type/app.yaml diff --git a/CHANGELOG/CHANGELOG-1.5.md b/CHANGELOG/CHANGELOG-1.5.md new file mode 100644 index 000000000..76618d539 --- /dev/null +++ b/CHANGELOG/CHANGELOG-1.5.md @@ -0,0 +1,69 @@ +- [How to try](#how-to-try) +- [v1.5.1](#v151) + - [Features](#features) + - [Fixes](#fixes) +- [v1.5.0](#v150) + - [Features](#features-1) + - [Fixes](#fixes-1) +- [v1.5.0-rc1](#v150-rc1) + - [Features](#features-2) + - [Fixes](#fixes-2) + +# v1.5.1 + +## Features +* [[#1122](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1346) Add support for ingress.rules.path.pathType property + +## Fixes +* [#1347](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1347) fix(v1/ingress): fix panic due to ingress class when k8s <= 1.19 +* [#1344](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1344) fix(v1/ingress): retry getting server version to get past transient issues + +# v1.5.0 + +## Features +* [#1329](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1329) Support for ingress class resource in v1/ingress +* [#1280](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1280) Add cookie-based-affinity-distinct-name annotation +* [#1287](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1287) Add rewrite rule set annotation to reference an existing rewrite rule on Application Gateway + +## Fixes +* [#1322](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1322) fix(port): port reference by name in ingress + +# v1.5.0-rc1 + +## Features +* [#1197](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1197) Support for v1/ingress (maintaining compatibility with v1beta1/ingress for clsuters <= 1.22) +* [#1324](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1324) Support for multi-arch images: amd64/arm64 +* [#1169](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1169) Resource quota exposed through helm chart +> Note: +> * Support for ingress class resource will added in a future release +> * Support for ingress.rules.path.pathType property will added in a future release + +## Fixes +* [#1282](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1282) fix(ingress): Modify cloned ingress instead of original ingress +* [#1271](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1271) fix(private clouds): Use correct endpoints for AAD and Azure in private clouds +* [#1273](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1273) fix(config): panic when processing duplicate paths in urlpathmap +* [#1220](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1220) fix(crd): Upgrade prohibited target CRD api-version and add tests +* [#1278](https://github.com/Azure/application-gateway-kubernetes-ingress/pull/1278) fix(prohibited target): incorrect merge when rules being merged reference the same path map + +## How to try: +```bash +# Add helm repo / update AGIC repo +helm repo add application-gateway-kubernetes-ingress https://appgwingress.blob.core.windows.net/ingress-azure-helm-package/ +helm repo update + +# Install +helm install \ + \ + -f helm-config.yaml \ + application-gateway-kubernetes-ingress/ingress-azure \ + +# or + +# Upgrade +# https://github.com/Azure/application-gateway-kubernetes-ingress/blob/master/docs/how-tos/helm-upgrade.md +# --reuse-values when upgrading, reuse the last release's values and merge in any overrides from the command line via --set and -f. If '--reset-values' is specified, this is ignored +helm upgrade \ + \ + application-gateway-kubernetes-ingress/ingress-azure \ + --reuse-values +``` diff --git a/docs/developers/build.md b/docs/developers/build.md index ce9746f2b..5d63891c6 100644 --- a/docs/developers/build.md +++ b/docs/developers/build.md @@ -33,7 +33,7 @@ The file will contain a JSON blob with the following shape: { "clientId": "...", "clientSecret": "...", - "subscriptionId": "", + "subscriptionId": "", "tenantId": "...", "activeDirectoryEndpointUrl": "https://login.microsoftonline.com", "resourceManagerEndpointUrl": "https://management.azure.com/", diff --git a/pkg/appgw/configbuilder.go b/pkg/appgw/configbuilder.go index e07abe30b..eb096e133 100644 --- a/pkg/appgw/configbuilder.go +++ b/pkg/appgw/configbuilder.go @@ -217,8 +217,9 @@ func generateListenerID(ingress *networking.Ingress, rule *networking.IngressRul if overridePort != nil { if *overridePort > 0 && *overridePort < 65000 { frontendPort = *overridePort + klog.V(5).Infof("Using custom port specified in the override annotation: %d", *overridePort) } else { - klog.V(5).Infof("Invalid custom port configuration (%d). Setting listener port to default : %d", *overridePort, frontendPort) + klog.V(5).Infof("Derived listener port from ingress: %d", frontendPort) } } diff --git a/pkg/appgw/ingress_rules.go b/pkg/appgw/ingress_rules.go index 30da57e84..3f23f7b92 100644 --- a/pkg/appgw/ingress_rules.go +++ b/pkg/appgw/ingress_rules.go @@ -52,7 +52,7 @@ func (c *appGwConfigBuilder) applyToListener(rule *networking.IngressRule) bool for pathIdx := range rule.HTTP.Paths { path := &rule.HTTP.Paths[pathIdx] // if there is path that is /, /* , empty string, then apply the waf policy to the listener. - if len(path.Path) == 0 || path.Path == "/" || path.Path == "/*" { + if isPathCatchAll(path.Path, path.PathType) { return true } } diff --git a/pkg/appgw/requestroutingrules.go b/pkg/appgw/requestroutingrules.go index 7fc7eeb42..11b9e0935 100755 --- a/pkg/appgw/requestroutingrules.go +++ b/pkg/appgw/requestroutingrules.go @@ -269,7 +269,7 @@ func (c *appGwConfigBuilder) getDefaultFromRule(cbCtx *ConfigBuilderContext, lis defBackend := ingress.Spec.DefaultBackend for pathIdx := range rule.HTTP.Paths { path := &rule.HTTP.Paths[pathIdx] - if path.Path == "" || path.Path == "/*" || path.Path == "/" { + if isPathCatchAll(path.Path, path.PathType) { defBackend = &path.Backend defPath = path defRule = rule @@ -303,7 +303,7 @@ func (c *appGwConfigBuilder) getPathRules(cbCtx *ConfigBuilderContext, listenerI pathRules := make([]n.ApplicationGatewayPathRule, 0) for pathIdx := range rule.HTTP.Paths { path := &rule.HTTP.Paths[pathIdx] - if len(path.Path) == 0 || path.Path == "/*" || path.Path == "/" { + if isPathCatchAll(path.Path, path.PathType) { continue } @@ -314,7 +314,7 @@ func (c *appGwConfigBuilder) getPathRules(cbCtx *ConfigBuilderContext, listenerI Name: to.StringPtr(pathRuleName), ID: to.StringPtr(c.appGwIdentifier.pathRuleID(pathMapName, pathRuleName)), ApplicationGatewayPathRulePropertiesFormat: &n.ApplicationGatewayPathRulePropertiesFormat{ - Paths: &[]string{path.Path}, + Paths: &[]string{preparePathFromPathType(path.Path, path.PathType)}, }, } @@ -439,3 +439,27 @@ func printPathRule(pathRule n.ApplicationGatewayPathRule) string { return s } + +// preparePathFromPathType uses pathType property to modify ingress.Path to append/remove "*" +// if pathType == Prefix, "*" is appended if not provided by the user +// if pathType == Exact, "*" is trimmed +func preparePathFromPathType(path string, pathType *networking.PathType) string { + if pathType != nil { + if *pathType == networking.PathTypeExact { + return strings.TrimSuffix(path, "*") + } + + if *pathType == networking.PathTypePrefix && !strings.HasSuffix(path, "*") { + return path + "*" + } + } + return path +} + +// Application Gateway doesn't allow exact path for "/" +// Code="ApplicationGatewayPathRulePathShouldHaveNonEmptyMatchValue" Message="Path / should have a nonempty match value followed by '/' in PathRule xxxx +// So, Path "/" with pathType:Exact cannot be added into the path rule. Thus, we don't support it. +// "/" for any path type will be treated as a prefix match. +func isPathCatchAll(path string, pathType *networking.PathType) bool { + return len(path) == 0 || path == "/*" || path == "/" +} diff --git a/pkg/appgw/requestroutingrules_test.go b/pkg/appgw/requestroutingrules_test.go index 1ff90cb85..a36b552fa 100755 --- a/pkg/appgw/requestroutingrules_test.go +++ b/pkg/appgw/requestroutingrules_test.go @@ -937,4 +937,121 @@ var _ = Describe("Test routing rules generations", func() { }) }) + + Context("test pathType in ingress", func() { + configBuilder := newConfigBuilderFixture(nil) + endpoint := tests.NewEndpointsFixture() + service := tests.NewServiceFixture(*tests.NewServicePortsFixture()...) + ingress := tests.NewIngressTestWithVariousPathTypeFixture(tests.Namespace, "ingress-with-path-type") + + _ = configBuilder.k8sContext.Caches.Endpoints.Add(endpoint) + _ = configBuilder.k8sContext.Caches.Service.Add(service) + _ = configBuilder.k8sContext.Caches.Ingress.Add(ingress) + + cbCtx := &ConfigBuilderContext{ + IngressList: []*networking.Ingress{&ingress}, + ServiceList: []*v1.Service{service}, + DefaultAddressPoolID: to.StringPtr("xx"), + DefaultHTTPSettingsID: to.StringPtr("yy"), + } + + _, urlPathMaps := configBuilder.getRules(cbCtx) + urlPathMap := urlPathMaps[0] + pathRules := *urlPathMaps[0].PathRules + + It("should have 8 paths", func() { + Expect(len(pathRules)).To(Equal(8)) + }) + + It("should add * to paths with pathType:prefix", func() { + paths := *(pathRules[0].Paths) + Expect(paths[0]).To(Equal("/prefix0*")) + + paths = *(pathRules[1].Paths) + Expect(paths[0]).To(Equal("/prefix1*")) + }) + + It("should trim * from paths with pathType:exact", func() { + paths := *(pathRules[2].Paths) + Expect(paths[0]).To(Equal("/exact2")) + + paths = *(pathRules[3].Paths) + Expect(paths[0]).To(Equal("/exact3")) + }) + + It("should not modify paths with pathType:implementationSpecific", func() { + paths := *(pathRules[4].Paths) + Expect(paths[0]).To(Equal("/ims4*")) + + paths = *(pathRules[5].Paths) + Expect(paths[0]).To(Equal("/ims5")) + }) + + It("should not modify paths with pathType:nil", func() { + paths := *(pathRules[6].Paths) + Expect(paths[0]).To(Equal("/nil6*")) + + paths = *(pathRules[7].Paths) + Expect(paths[0]).To(Equal("/nil7")) + }) + + It("should have default matching /*", func() { + Expect(*urlPathMap.DefaultBackendAddressPool.ID).To(HaveSuffix("pool---namespace-----service-name---80-bp-9876")) + Expect(*urlPathMap.DefaultBackendHTTPSettings.ID).To(HaveSuffix("bp---namespace-----service-name---80-9876-ingress-with-path-type")) + }) + }) + + Context("test preparePathFromPathType", func() { + It("should append * when pathType is Prefix", func() { + pathType := networking.PathTypePrefix + + Expect(preparePathFromPathType("/path", &pathType)).To(Equal("/path*")) + Expect(preparePathFromPathType("/path*", &pathType)).To(Equal("/path*")) + }) + + It("should trim when pathType is Exact", func() { + pathType := networking.PathTypeExact + + Expect(preparePathFromPathType("/path", &pathType)).To(Equal("/path")) + Expect(preparePathFromPathType("/path*", &pathType)).To(Equal("/path")) + }) + + It("should not modify when pathType is ImplementationSpecific", func() { + pathType := networking.PathTypeImplementationSpecific + + Expect(preparePathFromPathType("/path", &pathType)).To(Equal("/path")) + Expect(preparePathFromPathType("/path*", &pathType)).To(Equal("/path*")) + }) + + It("should not modify when pathType is nil", func() { + Expect(preparePathFromPathType("/path", nil)).To(Equal("/path")) + Expect(preparePathFromPathType("/path*", nil)).To(Equal("/path*")) + }) + }) + + Context("test isPathCatchAll", func() { + // Application Gateway doesn't allow exact path for "/" + It("should be false if / and pathType:exact", func() { + pathTypeExact := networking.PathTypeExact + Expect(isPathCatchAll("/", &pathTypeExact)).To(BeTrue()) + Expect(isPathCatchAll("/*", &pathTypeExact)).To(BeTrue()) + }) + + It("should be true if / and pathType:nil", func() { + Expect(isPathCatchAll("/", nil)).To(BeTrue()) + Expect(isPathCatchAll("/*", nil)).To(BeTrue()) + }) + + It("should be true if / and pathType:prefix", func() { + pathTypePrefix := networking.PathTypePrefix + Expect(isPathCatchAll("/", &pathTypePrefix)).To(BeTrue()) + Expect(isPathCatchAll("/*", &pathTypePrefix)).To(BeTrue()) + }) + + It("should be true if / and pathType:implementationSpecific", func() { + pathTypeIMS := networking.PathTypeImplementationSpecific + Expect(isPathCatchAll("/", &pathTypeIMS)).To(BeTrue()) + Expect(isPathCatchAll("/*", &pathTypeIMS)).To(BeTrue()) + }) + }) }) diff --git a/pkg/k8scontext/context.go b/pkg/k8scontext/context.go index d331f622d..db3f45292 100644 --- a/pkg/k8scontext/context.go +++ b/pkg/k8scontext/context.go @@ -76,7 +76,6 @@ func NewContext(kubeClient kubernetes.Interface, crdClient versioned.Interface, if IsNetworkingV1PackageSupported { informerCollection.Ingress = informerFactory.Networking().V1().Ingresses().Informer() - informerCollection.IngressClass = informerFactory.Networking().V1().IngressClasses().Informer() } else { informerCollection.Ingress = informerFactory.Extensions().V1beta1().Ingresses().Informer() } @@ -84,7 +83,6 @@ func NewContext(kubeClient kubernetes.Interface, crdClient versioned.Interface, cacheCollection := CacheCollection{ Endpoints: informerCollection.Endpoints.GetStore(), Ingress: informerCollection.Ingress.GetStore(), - IngressClass: informerCollection.IngressClass.GetStore(), Pods: informerCollection.Pods.GetStore(), Secret: informerCollection.Secret.GetStore(), Service: informerCollection.Service.GetStore(), @@ -146,7 +144,6 @@ func NewContext(kubeClient kubernetes.Interface, crdClient versioned.Interface, // Register event handlers. informerCollection.Endpoints.AddEventHandler(resourceHandler) informerCollection.Ingress.AddEventHandler(ingressResourceHandler) - informerCollection.IngressClass.AddEventHandler(resourceHandler) informerCollection.Pods.AddEventHandler(resourceHandler) informerCollection.Secret.AddEventHandler(secretResourceHandler) informerCollection.Service.AddEventHandler(resourceHandler) @@ -156,6 +153,12 @@ func NewContext(kubeClient kubernetes.Interface, crdClient versioned.Interface, informerCollection.MultiClusterService.AddEventHandler(resourceHandler) informerCollection.MultiClusterIngress.AddEventHandler(resourceHandler) + if IsNetworkingV1PackageSupported { + informerCollection.IngressClass = informerFactory.Networking().V1().IngressClasses().Informer() + informerCollection.IngressClass.AddEventHandler(resourceHandler) + cacheCollection.IngressClass = informerCollection.IngressClass.GetStore() + } + return context } @@ -188,13 +191,16 @@ func (c *Context) Run(stopChannel chan struct{}, omitCRDs bool, envVariables env c.informers.Service, c.informers.Secret, c.informers.Ingress, - c.informers.IngressClass, //TODO: enabled by ccp feature flag // c.informers.AzureApplicationGatewayBackendPool, // c.informers.AzureApplicationGatewayInstanceUpdateStatus, } + if IsNetworkingV1PackageSupported { + sharedInformers = append(sharedInformers, c.informers.IngressClass) + } + // For AGIC to watch for these CRDs the EnableBrownfieldDeploymentVarName env variable must be set to true if envVariables.EnableBrownfieldDeployment { sharedInformers = append(sharedInformers, c.informers.AzureIngressProhibitedTarget) @@ -870,6 +876,10 @@ func (c *Context) isServiceReferencedByAnyIngress(service *v1.Service) bool { // getIngressClassResource gets ingress class object with specified name func (c *Context) getIngressClassResource(ingressClassName string) *networking.IngressClass { + if c.Caches.IngressClass == nil { + return nil + } + ingressClassInterface, exist, err := c.Caches.IngressClass.GetByKey(ingressClassName) if err != nil { return nil diff --git a/pkg/k8scontext/k8scontext_test.go b/pkg/k8scontext/k8scontext_test.go index 4578760ea..8171f32ab 100644 --- a/pkg/k8scontext/k8scontext_test.go +++ b/pkg/k8scontext/k8scontext_test.go @@ -21,9 +21,12 @@ import ( testclient "k8s.io/client-go/kubernetes/fake" "github.com/Azure/application-gateway-kubernetes-ingress/pkg/annotations" - "github.com/Azure/application-gateway-kubernetes-ingress/pkg/crd_client/agic_crd_client/clientset/versioned/fake" - multiClusterFake "github.com/Azure/application-gateway-kubernetes-ingress/pkg/crd_client/azure_multicluster_crd_client/clientset/versioned/fake" - istioFake "github.com/Azure/application-gateway-kubernetes-ingress/pkg/crd_client/istio_crd_client/clientset/versioned/fake" + agiccrd "github.com/Azure/application-gateway-kubernetes-ingress/pkg/crd_client/agic_crd_client/clientset/versioned" + agiccrdFake "github.com/Azure/application-gateway-kubernetes-ingress/pkg/crd_client/agic_crd_client/clientset/versioned/fake" + mcscrd "github.com/Azure/application-gateway-kubernetes-ingress/pkg/crd_client/azure_multicluster_crd_client/clientset/versioned" + mcsfake "github.com/Azure/application-gateway-kubernetes-ingress/pkg/crd_client/azure_multicluster_crd_client/clientset/versioned/fake" + istiocrd "github.com/Azure/application-gateway-kubernetes-ingress/pkg/crd_client/istio_crd_client/clientset/versioned" + istiofake "github.com/Azure/application-gateway-kubernetes-ingress/pkg/crd_client/istio_crd_client/clientset/versioned/fake" "github.com/Azure/application-gateway-kubernetes-ingress/pkg/environment" "github.com/Azure/application-gateway-kubernetes-ingress/pkg/metricstore" "github.com/Azure/application-gateway-kubernetes-ingress/pkg/tests" @@ -32,6 +35,9 @@ import ( var _ = ginkgo.Describe("K8scontext", func() { var k8sClient kubernetes.Interface + var crdClient agiccrd.Interface + var istioCrdClient istiocrd.Interface + var multiClusterCrdClient mcscrd.Interface var ctxt *Context ingressNS := "test-ingress-controller" ingressName := "hello-world" @@ -81,9 +87,9 @@ var _ = ginkgo.Describe("K8scontext", func() { // Create the mock K8s client. k8sClient = testclient.NewSimpleClientset() - crdClient := fake.NewSimpleClientset() - istioCrdClient := istioFake.NewSimpleClientset() - multiClusterCrdClient := multiClusterFake.NewSimpleClientset() + crdClient = agiccrdFake.NewSimpleClientset() + istioCrdClient = istiofake.NewSimpleClientset() + multiClusterCrdClient = mcsfake.NewSimpleClientset() _, err := k8sClient.CoreV1().Namespaces().Create(context.TODO(), ns, metav1.CreateOptions{}) Expect(err).ToNot(HaveOccurred(), "Unable to create the namespace %s: %v", ingressNS, err) @@ -623,4 +629,26 @@ var _ = ginkgo.Describe("K8scontext", func() { Expect(testIngresses[1]).To(Equal(&ingressWithoutIngressClass), "Expected to retrieve the same ingress that we inserted, but it seems we found the following ingress: %v", testIngresses[1]) }) }) + + ginkgo.Context("Checking when server doesn't support v1/ingress", func() { + + ginkgo.BeforeEach(func() { + // Create a `k8scontext` to start listening to ingress resources. + IsNetworkingV1PackageSupported = false + ctxt = NewContext(k8sClient, crdClient, multiClusterCrdClient, istioCrdClient, []string{ingressNS}, 1000*time.Second, metricstore.NewFakeMetricStore(), environment.GetFakeEnv()) + + Expect(ctxt).ShouldNot(BeNil(), "Unable to create `k8scontext`") + }) + + // e2e covers tests that make sure that v1beta1/ingress works with AGIC + ginkgo.It("should start context successfully", func() { + // start the informers. This will sync the cache with the latest ingress. + runErr := ctxt.Run(stopChannel, true, environment.GetFakeEnv()) + Expect(runErr).ToNot(HaveOccurred()) + + // ingress class informer should be nil + Expect(ctxt.informers.IngressClass).To(BeNil()) + Expect(ctxt.Caches.IngressClass).To(BeNil()) + }) + }) }) diff --git a/pkg/k8scontext/supported_apiversion.go b/pkg/k8scontext/supported_apiversion.go index 6088c2690..e87e9981b 100644 --- a/pkg/k8scontext/supported_apiversion.go +++ b/pkg/k8scontext/supported_apiversion.go @@ -6,12 +6,23 @@ package k8scontext import ( + "time" + + "github.com/Azure/application-gateway-kubernetes-ingress/pkg/utils" + extensionsv1beta1 "k8s.io/api/extensions/v1beta1" networkingv1 "k8s.io/api/networking/v1" k8sruntime "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/version" clientset "k8s.io/client-go/kubernetes" "k8s.io/klog/v2" + + serverversion "k8s.io/apimachinery/pkg/version" +) + +const ( + maxRetryCount = 10 + retryPause = 1 * time.Second ) var ( @@ -36,9 +47,19 @@ func SupportsNetworkingPackage(client clientset.Interface) bool { // check kubernetes version to use new ingress package or not version119, _ := version.ParseGeneric("v1.19.0") - serverVersion, err := client.Discovery().ServerVersion() + var serverVersion *serverversion.Info + var err error + utils.Retry(maxRetryCount, retryPause, func() (utils.Retriable, error) { + serverVersion, err = client.Discovery().ServerVersion() + if err != nil { + klog.Warningf("Failed to get server version of the cluster: %s", err) + return utils.Retriable(true), err + } else { + return false, err + } + }) if err != nil { - return false + klog.Fatalf("Failed to get server version of the cluster: %s", err) } runningVersion, err := version.ParseGeneric(serverVersion.String()) @@ -46,6 +67,6 @@ func SupportsNetworkingPackage(client clientset.Interface) bool { klog.Errorf("unexpected error parsing running Kubernetes version: %v", err) return false } - + klog.V(1).Infof("server version is: %s", runningVersion.String()) return runningVersion.AtLeast(version119) } diff --git a/pkg/tests/fixtures.go b/pkg/tests/fixtures.go index cbff64ea7..94575e898 100644 --- a/pkg/tests/fixtures.go +++ b/pkg/tests/fixtures.go @@ -696,6 +696,165 @@ func NewIngressTestFixtureBasic(namespace string, ingressName string, tls bool) return ingress } +// NewIngressTestFixture creates a new Ingress struct for testing. +func NewIngressTestWithVariousPathTypeFixture(namespace string, ingressName string) networking.Ingress { + prefixType := networking.PathTypePrefix + exactType := networking.PathTypeExact + implementationSpecificType := networking.PathTypeImplementationSpecific + return networking.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: ingressName, + Namespace: namespace, + Annotations: map[string]string{ + annotations.IngressClassKey: IngressClassController, + }, + }, + Spec: networking.IngressSpec{ + Rules: []networking.IngressRule{ + { + Host: "hello.com", + IngressRuleValue: networking.IngressRuleValue{ + HTTP: &networking.HTTPIngressRuleValue{ + Paths: []networking.HTTPIngressPath{ + { + // type:prefix with * + Path: "/prefix0*", + PathType: &prefixType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: ServiceName, + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + { + // type:prefix without * + Path: "/prefix1", + PathType: &prefixType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: ServiceName, + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + { + // type:exact with * + Path: "/exact2*", + PathType: &exactType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: ServiceName, + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + { + // type:exact without * + Path: "/exact3", + PathType: &implementationSpecificType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: ServiceName, + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + { + // type:implementationSpecific with * + Path: "/ims4*", + PathType: &implementationSpecificType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: ServiceName, + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + { + // type:implementationSpecific without * + Path: "/ims5", + PathType: &implementationSpecificType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: ServiceName, + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + { + // type:nil with * + Path: "/nil6*", + PathType: nil, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: ServiceName, + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + { + // type:nil without * + Path: "/nil7", + PathType: nil, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: ServiceName, + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + { + // "/" path with pathType:prefix + Path: "/", + PathType: &prefixType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: ServiceName, + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + { + // "/*" path with pathType:exact + Path: "/*", + PathType: &exactType, + Backend: networking.IngressBackend{ + Service: &networking.IngressServiceBackend{ + Name: ServiceName, + Port: networking.ServiceBackendPort{ + Number: 80, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + } +} + // NewPodTestFixture creates a new Pod struct for testing. func NewPodTestFixture(namespace string, podName string) v1.Pod { return v1.Pod{ diff --git a/scripts/e2e/cmd/runner/networking-v1-mfu_one_namespace_one_ingress_test.go b/scripts/e2e/cmd/runner/networking-v1-mfu_one_namespace_one_ingress_test.go index cb1281624..d0f8c0cfe 100644 --- a/scripts/e2e/cmd/runner/networking-v1-mfu_one_namespace_one_ingress_test.go +++ b/scripts/e2e/cmd/runner/networking-v1-mfu_one_namespace_one_ingress_test.go @@ -405,7 +405,7 @@ var _ = Describe("networking-v1-MFU", func() { }) It("[ingress-class-resource] ingress class resource should work with ingress v1", func() { - namespaceName := "ingress-class-resource" + namespaceName := "e2e-ingress-class-resource" ns := &v1.Namespace{ ObjectMeta: metav1.ObjectMeta{ Name: namespaceName, @@ -468,6 +468,54 @@ var _ = Describe("networking-v1-MFU", func() { Expect(testHeader).To(Equal("test-value")) }) + It("[path-type] Path Type should correctly convert path to app gateway and respond correctly", func() { + namespaceName := "e2e-path-type" + ns := &v1.Namespace{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespaceName, + }, + } + klog.Info("Creating namespace: ", namespaceName) + _, err = clientset.CoreV1().Namespaces().Create(context.TODO(), ns, metav1.CreateOptions{}) + Expect(err).To(BeNil()) + + yamlPath := "testdata/networking-v1/one-namespace-one-ingress/path-type/app.yaml" + klog.Info("Applying yaml: ", yamlPath) + err = applyYaml(clientset, namespaceName, yamlPath) + Expect(err).To(BeNil()) + time.Sleep(10 * time.Second) + + // get ip address for 1 ingress + klog.Info("Getting public IP from Ingress...") + publicIP, _ := getPublicIP(clientset, namespaceName) + Expect(publicIP).ToNot(Equal("")) + + urlHttps := fmt.Sprintf("http://%s", publicIP) + + respondedWithColor := func(path string, body string) { + resp, err := makeGetRequest(urlHttps+path, "example.com", 200, true) + Expect(readBody(resp)).To(ContainSubstring(body), "path: %s", path) + Expect(err).To(BeNil()) + } + + // PathType:Prefix + respondedWithColor("/prefix", "correct-app") + respondedWithColor("/prefixSuffix", "correct-app") + + // PathType:Exact + respondedWithColor("/exact", "correct-app") + respondedWithColor("/exact/asd", "catch-all") + + // PathType:ImplementationSpecific + respondedWithColor("/ims", "correct-app") + respondedWithColor("/imsSuffix", "correct-app") + + // Path / with pathType:exact + // AppGW doesn't allow / with pathType:exact + respondedWithColor("/", "catch-all") + respondedWithColor("/Suffix", "catch-all") + }) + AfterEach(func() { // clear all namespaces cleanUp(clientset) diff --git a/scripts/e2e/cmd/runner/testdata/networking-v1/one-namespace-one-ingress/path-type/app.yaml b/scripts/e2e/cmd/runner/testdata/networking-v1/one-namespace-one-ingress/path-type/app.yaml new file mode 100644 index 000000000..d6ee20e78 --- /dev/null +++ b/scripts/e2e/cmd/runner/testdata/networking-v1/one-namespace-one-ingress/path-type/app.yaml @@ -0,0 +1,169 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: catch-all +data: + default.conf: |- + server { + listen 80 default_server; + location / { + return 200 "catch-all"; + } + } +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: catch-all +spec: + selector: + matchLabels: + app: catch-all + replicas: 1 + template: + metadata: + labels: + app: catch-all + spec: + containers: + - name: nginx + imagePullPolicy: Always + image: nginx:latest + ports: + - containerPort: 80 + name: http + volumeMounts: + - mountPath: /etc/nginx/conf.d + name: configmap-volume + volumes: + - name: configmap-volume + configMap: + name: catch-all +--- +apiVersion: v1 +kind: Service +metadata: + name: catch-all +spec: + selector: + app: catch-all + ports: + - protocol: TCP + port: 80 + targetPort: 80 +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: correct-app +data: + default.conf: |- + server { + listen 80 default_server; + location / { + return 200 "correct-app"; + } + } +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: correct-app +spec: + selector: + matchLabels: + app: correct-app + replicas: 1 + template: + metadata: + labels: + app: correct-app + spec: + containers: + - name: nginx + imagePullPolicy: Always + image: nginx:latest + ports: + - containerPort: 80 + name: http + volumeMounts: + - mountPath: /etc/nginx/conf.d + name: configmap-volume + volumes: + - name: configmap-volume + configMap: + name: correct-app +--- +apiVersion: v1 +kind: Service +metadata: + name: correct-app +spec: + selector: + app: correct-app + ports: + - protocol: TCP + port: 80 + targetPort: 80 +--- +apiVersion: networking.k8s.io/v1 +kind: Ingress +metadata: + name: ingress + annotations: + kubernetes.io/ingress.class: azure/application-gateway + appgw.ingress.kubernetes.io/health-probe-path: / +spec: + rules: + - host: example.com + http: + paths: + - path: / + backend: + service: + name: catch-all + port: + number: 80 + pathType: Prefix + - path: /prefix + backend: + service: + name: correct-app + port: + number: 80 + pathType: Prefix + - path: /prefix* + backend: + service: + name: correct-app + port: + number: 80 + pathType: Prefix + - path: /exact + backend: + service: + name: correct-app + port: + number: 80 + pathType: Exact + - path: /exact* + backend: + service: + name: correct-app + port: + number: 80 + pathType: Exact + - path: /ims + backend: + service: + name: correct-app + port: + number: 80 + pathType: ImplementationSpecific + - path: /ims* + backend: + service: + name: correct-app + port: + number: 80 + pathType: ImplementationSpecific