From 78cb4021122cb8911d1b714173ffe0d0c2c05f48 Mon Sep 17 00:00:00 2001 From: Akshay Gupta Date: Fri, 6 May 2022 13:56:04 +0530 Subject: [PATCH] fix: assign rule priority automatically to routing rule (#1390) (#1391) * assign rule priority * skip rule with priority * adding non-working tests * find next free priority * adjust jump; fix test * remove comment * add buffer to sig channel Co-authored-by: Alex Z. Yang Co-authored-by: Alex Z. Yang --- cmd/appgw-ingress/main.go | 2 +- docs/setup/install-new.md | 2 +- functional_tests/cookie_name.json | 1 + functional_tests/duplicate_ports.json | 2 + .../empty_cluster_with_private_ip.json | 1 + ...obes_same_labels_different_namespaces.json | 2 + .../one_ingress_https_backend.json | 2 + ...ttps_backend_without_backend_protocol.json | 1 + .../one_ingress_slash_nothing.json | 1 + .../one_ingress_slash_slashnothing.json | 1 + .../one_ingress_with_multiple_path_rules.json | 1 + functional_tests/three_ingresses.json | 3 + .../two_ingresses_same_domain_tls_notls.json | 2 + ...me_hostname_value_different_locations.json | 1 + .../two_ingresses_slash_slashsomething.json | 1 + ...es_with_and_without_extended_hostname.json | 2 + functional_tests/waf_annotation.json | 1 + go.mod | 2 +- go.sum | 22 +++- pkg/appgw/configbuilder_test.go | 3 + pkg/appgw/frontend_listeners.go | 22 ++++ pkg/appgw/frontend_listeners_test.go | 65 ++++++++++ pkg/appgw/requestroutingrules.go | 63 ++++++++++ pkg/appgw/requestroutingrules_test.go | 116 ++++++++++++++++++ 24 files changed, 310 insertions(+), 9 deletions(-) diff --git a/cmd/appgw-ingress/main.go b/cmd/appgw-ingress/main.go index e06966d57..f20db4d77 100644 --- a/cmd/appgw-ingress/main.go +++ b/cmd/appgw-ingress/main.go @@ -224,7 +224,7 @@ func main() { klog.Fatal(errorLine) } - sigChan := make(chan os.Signal) + sigChan := make(chan os.Signal, 1) signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM) <-sigChan diff --git a/docs/setup/install-new.md b/docs/setup/install-new.md index ba3013a0d..6b6b566f5 100644 --- a/docs/setup/install-new.md +++ b/docs/setup/install-new.md @@ -236,7 +236,7 @@ Values: - `rbac.enabled`: Make sure to set this to true if you have a AKS cluster that is RBAC enabled. Note on Identity: The `identityResourceID` and `identityClientID` are values that were created - during the [Create an Identity](https://github.com/Azure/application-gateway-kubernetes-ingress/blob/072626cb4e37f7b7a1b0c4578c38d1eadc3e8701/docs/setup/install-new.md#create-an-identity) + during the [Create an Identity](#create-an-identity) steps, and could be obtained again using the following command: ```bash diff --git a/functional_tests/cookie_name.json b/functional_tests/cookie_name.json index 3892d7272..966c6b250 100644 --- a/functional_tests/cookie_name.json +++ b/functional_tests/cookie_name.json @@ -207,6 +207,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-e1903c8aa3446b7b3207aec6d6ecba8a" }, + "priority": 19500, "ruleType": "PathBasedRouting", "urlPathMap": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/urlPathMaps/url-e1903c8aa3446b7b3207aec6d6ecba8a" diff --git a/functional_tests/duplicate_ports.json b/functional_tests/duplicate_ports.json index c9f272326..a51800c83 100644 --- a/functional_tests/duplicate_ports.json +++ b/functional_tests/duplicate_ports.json @@ -198,6 +198,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-08ba2bb7da9df5927d900fca8ce96ba5" }, + "priority": 19000, "redirectConfiguration": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/redirectConfigurations/sslr-fl-175bc9385b96d7116527143f35cdfdea" }, @@ -217,6 +218,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-175bc9385b96d7116527143f35cdfdea" }, + "priority": 19005, "ruleType": "Basic" } } diff --git a/functional_tests/empty_cluster_with_private_ip.json b/functional_tests/empty_cluster_with_private_ip.json index 230fa04ed..070f2f0e0 100644 --- a/functional_tests/empty_cluster_with_private_ip.json +++ b/functional_tests/empty_cluster_with_private_ip.json @@ -116,6 +116,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-ac133916d522c571f2d9ef8f3b8a25d3" }, + "priority": 19500, "ruleType": "Basic" } } diff --git a/functional_tests/health_probes_same_labels_different_namespaces.json b/functional_tests/health_probes_same_labels_different_namespaces.json index 633e18c7e..680c78672 100644 --- a/functional_tests/health_probes_same_labels_different_namespaces.json +++ b/functional_tests/health_probes_same_labels_different_namespaces.json @@ -244,6 +244,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-08ba2bb7da9df5927d900fca8ce96ba5" }, + "priority": 19000, "ruleType": "PathBasedRouting", "urlPathMap": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/urlPathMaps/url-08ba2bb7da9df5927d900fca8ce96ba5" @@ -263,6 +264,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-175bc9385b96d7116527143f35cdfdea" }, + "priority": 19005, "ruleType": "Basic" } } diff --git a/functional_tests/one_ingress_https_backend.json b/functional_tests/one_ingress_https_backend.json index d0f8c723d..f9f6d9651 100644 --- a/functional_tests/one_ingress_https_backend.json +++ b/functional_tests/one_ingress_https_backend.json @@ -199,6 +199,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-c61dda7ff9748ec5f51989767db5afd0" }, + "priority": 19500, "ruleType": "PathBasedRouting", "urlPathMap": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/urlPathMaps/url-c61dda7ff9748ec5f51989767db5afd0" @@ -212,6 +213,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-e1903c8aa3446b7b3207aec6d6ecba8a" }, + "priority": 19505, "ruleType": "PathBasedRouting", "urlPathMap": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/urlPathMaps/url-e1903c8aa3446b7b3207aec6d6ecba8a" diff --git a/functional_tests/one_ingress_https_backend_without_backend_protocol.json b/functional_tests/one_ingress_https_backend_without_backend_protocol.json index d86428d2d..51726904a 100644 --- a/functional_tests/one_ingress_https_backend_without_backend_protocol.json +++ b/functional_tests/one_ingress_https_backend_without_backend_protocol.json @@ -159,6 +159,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-c61dda7ff9748ec5f51989767db5afd0" }, + "priority": 19500, "ruleType": "PathBasedRouting", "urlPathMap": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/urlPathMaps/url-c61dda7ff9748ec5f51989767db5afd0" diff --git a/functional_tests/one_ingress_slash_nothing.json b/functional_tests/one_ingress_slash_nothing.json index 0b727840b..cfecd0265 100644 --- a/functional_tests/one_ingress_slash_nothing.json +++ b/functional_tests/one_ingress_slash_nothing.json @@ -162,6 +162,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-e1903c8aa3446b7b3207aec6d6ecba8a" }, + "priority": 19500, "ruleType": "Basic" } } diff --git a/functional_tests/one_ingress_slash_slashnothing.json b/functional_tests/one_ingress_slash_slashnothing.json index 083396c94..23e02bbe7 100644 --- a/functional_tests/one_ingress_slash_slashnothing.json +++ b/functional_tests/one_ingress_slash_slashnothing.json @@ -202,6 +202,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-e1903c8aa3446b7b3207aec6d6ecba8a" }, + "priority": 19500, "ruleType": "PathBasedRouting", "urlPathMap": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/urlPathMaps/url-e1903c8aa3446b7b3207aec6d6ecba8a" diff --git a/functional_tests/one_ingress_with_multiple_path_rules.json b/functional_tests/one_ingress_with_multiple_path_rules.json index aef536a2a..8cf274aeb 100644 --- a/functional_tests/one_ingress_with_multiple_path_rules.json +++ b/functional_tests/one_ingress_with_multiple_path_rules.json @@ -248,6 +248,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-e1903c8aa3446b7b3207aec6d6ecba8a" }, + "priority": 19500, "ruleType": "PathBasedRouting", "urlPathMap": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/urlPathMaps/url-e1903c8aa3446b7b3207aec6d6ecba8a" diff --git a/functional_tests/three_ingresses.json b/functional_tests/three_ingresses.json index 734ed27b9..70bc9be2b 100644 --- a/functional_tests/three_ingresses.json +++ b/functional_tests/three_ingresses.json @@ -305,6 +305,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-08ba2bb7da9df5927d900fca8ce96ba5" }, + "priority": 19000, "redirectConfiguration": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/redirectConfigurations/sslr-fl-175bc9385b96d7116527143f35cdfdea" }, @@ -324,6 +325,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-175bc9385b96d7116527143f35cdfdea" }, + "priority": 19005, "ruleType": "Basic" } }, @@ -334,6 +336,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-e1903c8aa3446b7b3207aec6d6ecba8a" }, + "priority": 19500, "ruleType": "PathBasedRouting", "urlPathMap": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/urlPathMaps/url-e1903c8aa3446b7b3207aec6d6ecba8a" diff --git a/functional_tests/two_ingresses_same_domain_tls_notls.json b/functional_tests/two_ingresses_same_domain_tls_notls.json index 46e9e18c1..213e78e40 100644 --- a/functional_tests/two_ingresses_same_domain_tls_notls.json +++ b/functional_tests/two_ingresses_same_domain_tls_notls.json @@ -244,6 +244,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-08ba2bb7da9df5927d900fca8ce96ba5" }, + "priority": 19000, "ruleType": "PathBasedRouting", "urlPathMap": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/urlPathMaps/url-08ba2bb7da9df5927d900fca8ce96ba5" @@ -263,6 +264,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-175bc9385b96d7116527143f35cdfdea" }, + "priority": 19005, "ruleType": "Basic" } } diff --git a/functional_tests/two_ingresses_same_hostname_value_different_locations.json b/functional_tests/two_ingresses_same_hostname_value_different_locations.json index f7d98f25f..948b5db5c 100644 --- a/functional_tests/two_ingresses_same_hostname_value_different_locations.json +++ b/functional_tests/two_ingresses_same_hostname_value_different_locations.json @@ -187,6 +187,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-08ba2bb7da9df5927d900fca8ce96ba5" }, + "priority": 19000, "ruleType": "PathBasedRouting", "urlPathMap": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/urlPathMaps/url-08ba2bb7da9df5927d900fca8ce96ba5" diff --git a/functional_tests/two_ingresses_slash_slashsomething.json b/functional_tests/two_ingresses_slash_slashsomething.json index 083396c94..23e02bbe7 100644 --- a/functional_tests/two_ingresses_slash_slashsomething.json +++ b/functional_tests/two_ingresses_slash_slashsomething.json @@ -202,6 +202,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-e1903c8aa3446b7b3207aec6d6ecba8a" }, + "priority": 19500, "ruleType": "PathBasedRouting", "urlPathMap": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/urlPathMaps/url-e1903c8aa3446b7b3207aec6d6ecba8a" diff --git a/functional_tests/two_ingresses_with_and_without_extended_hostname.json b/functional_tests/two_ingresses_with_and_without_extended_hostname.json index 135b9e00c..14b7bc1d2 100644 --- a/functional_tests/two_ingresses_with_and_without_extended_hostname.json +++ b/functional_tests/two_ingresses_with_and_without_extended_hostname.json @@ -220,6 +220,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-ac80e40addbf0c0bac6b5c79b565a989" }, + "priority": 19000, "ruleType": "PathBasedRouting", "urlPathMap": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/urlPathMaps/url-ac80e40addbf0c0bac6b5c79b565a989" @@ -233,6 +234,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-e1903c8aa3446b7b3207aec6d6ecba8a" }, + "priority": 19500, "ruleType": "PathBasedRouting", "urlPathMap": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/urlPathMaps/url-e1903c8aa3446b7b3207aec6d6ecba8a" diff --git a/functional_tests/waf_annotation.json b/functional_tests/waf_annotation.json index bc3a629b6..b581ef602 100644 --- a/functional_tests/waf_annotation.json +++ b/functional_tests/waf_annotation.json @@ -205,6 +205,7 @@ "httpListener": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-e1903c8aa3446b7b3207aec6d6ecba8a" }, + "priority": 19500, "ruleType": "PathBasedRouting", "urlPathMap": { "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/urlPathMaps/url-e1903c8aa3446b7b3207aec6d6ecba8a" diff --git a/go.mod b/go.mod index be37e273e..1d690babc 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/knative/pkg v0.0.0-20190619032946-d90a9bc97dde github.com/kylelemons/godebug v1.1.0 github.com/onsi/ginkgo v1.16.4 - github.com/onsi/gomega v1.14.0 + github.com/onsi/gomega v1.19.0 github.com/prometheus/client_golang v1.11.0 github.com/spf13/pflag v1.0.5 gopkg.in/yaml.v2 v2.4.0 diff --git a/go.sum b/go.sum index 34a611da5..5aa676be7 100644 --- a/go.sum +++ b/go.sum @@ -164,6 +164,7 @@ github.com/google/pprof v0.0.0-20190515194954-54271f7e092f/go.mod h1:zfwlbNMJ+OI github.com/google/pprof v0.0.0-20191218002539-d4f498aebedc/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM= github.com/google/pprof v0.0.0-20200212024743-f11f1df84d12/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM= github.com/google/pprof v0.0.0-20200229191704-1ebb73c60ed3/go.mod h1:ZgVRPoUq/hfqzAqh7sHMqb3I9Rq5C59dIz2SbBwJ4eM= +github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= github.com/google/renameio v0.1.0/go.mod h1:KWCgfxg9yswjAJkECMjeO8J8rahYeXnNhOm40UhjYkI= github.com/google/uuid v1.1.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/google/uuid v1.1.2 h1:EVhdT+1Kseyi1/pUmXKaFxYsDNy9RQYkMWRH68J/W7Y= @@ -179,6 +180,7 @@ github.com/hashicorp/golang-lru v0.5.1 h1:0hERBMJE1eitiLkihrMvRVBYAkpHzc/J3QdDN+ github.com/hashicorp/golang-lru v0.5.1/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hpcloud/tail v1.0.0/go.mod h1:ab1qPbhIpdTxEkNHXyeSf5vhxWSCs/tWer42PpOxQnU= github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= +github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc= github.com/imdario/mergo v0.3.5 h1:JboBksRwiiAJWvIYJVo46AfV+IAIKZpfrSzVKj42R4Q= github.com/imdario/mergo v0.3.5/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA= github.com/jpillora/backoff v1.0.0/go.mod h1:J/6gKK9jxlEcS3zixgDgUAsiuZ7yrSoa/FX5e0EB2j4= @@ -235,12 +237,15 @@ github.com/onsi/ginkgo v1.11.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+ github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk= github.com/onsi/ginkgo v1.16.4 h1:29JGrr5oVBm5ulCWet69zQkzWipVXIol6ygQUe/EzNc= github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0= +github.com/onsi/ginkgo/v2 v2.1.3 h1:e/3Cwtogj0HA+25nMP1jCMDIf8RtRYbGwGGuBIFztkc= +github.com/onsi/ginkgo/v2 v2.1.3/go.mod h1:vw5CSIxN1JObi/U8gcbwft7ZxR2dgaR70JSE3/PpL4c= github.com/onsi/gomega v0.0.0-20170829124025-dcabb60a477c/go.mod h1:C1qb7wdrVGGVU+Z6iS04AVkA3Q65CEZX59MT0QO5uiA= github.com/onsi/gomega v1.7.0/go.mod h1:ex+gbHU/CVuBBDIJjb2X0qEXbFg53c61hWP/1CpauHY= github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= -github.com/onsi/gomega v1.14.0 h1:ep6kpPVwmr/nTbklSx2nrLNSIO62DoYAhnPNIMhK8gI= -github.com/onsi/gomega v1.14.0/go.mod h1:cIuvLEne0aoVhAgh/O6ac0Op8WWw9H6eYCriF+tEHG0= +github.com/onsi/gomega v1.17.0/go.mod h1:HnhC7FXeEQY45zxNK3PPoIUhzk/80Xly9PcubAlGdZY= +github.com/onsi/gomega v1.19.0 h1:4ieX6qQjPP/BfC3mpsAtIGGlxTWPeA3Inl/7DtXw1tw= +github.com/onsi/gomega v1.19.0/go.mod h1:LY+I3pBVzYsTBU1AnDwOSxaYi9WoWiqgwooUqq9yPro= github.com/peterbourgon/diskv v2.0.1+incompatible/go.mod h1:uqqh8zWWbv1HBMNONnaR/tNboyR3/BZd58JJSHlUSCU= github.com/pkg/errors v0.8.0/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= @@ -358,8 +363,9 @@ golang.org/x/net v0.0.0-20200625001655-4c5254603344/go.mod h1:/O7V0waA8r7cgGh81R golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= golang.org/x/net v0.0.0-20210224082022-3d97a244fca7/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= -golang.org/x/net v0.0.0-20210428140749-89ef3d95e781 h1:DzZ89McO9/gWPsQXS/FVKAlG02ZjaQ6AlZRBimEYOd0= golang.org/x/net v0.0.0-20210428140749-89ef3d95e781/go.mod h1:OJAsFXCWl8Ukc7SiCT/9KSuxbyM7479/AVlXFRxuMCk= +golang.org/x/net v0.0.0-20220225172249-27dd8689420f h1:oA4XRj0qtSt8Yo1Zms0CUlsT3KG69V2UGQWPBxujDmc= +golang.org/x/net v0.0.0-20220225172249-27dd8689420f/go.mod h1:CfG3xpIq0wQ8r1q4Su4UZFWDARRcnwPjda9FqA0JpMk= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -411,20 +417,24 @@ golang.org/x/sys v0.0.0-20210112080510-489259a85091/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210423082822-04245dca01da/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20210426230700-d19ff857e887/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40 h1:JWgyZ1qgdTaF3N3oxC+MdTV7qvEEgHo3otj+HB5CM7Q= golang.org/x/sys v0.0.0-20210603081109-ebe580a85c40/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e h1:fLOSk5Q00efkSvAm+4xcoXD+RRmLmmulPn5I3Y9F2EM= +golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= -golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d h1:SZxvLBoTP5yHO3Frd4z4vrF+DBX9vMVanchswa69toE= golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= +golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 h1:JGgROgKl9N8DuW20oFS5gxc+lE67/N3FcwmBPMe7ArY= +golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= -golang.org/x/text v0.3.6 h1:aRYxNxv6iGQlyVaZmk6ZgYEDa+Jg18DxebPSrd6bg1M= golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/text v0.3.7 h1:olpwvP2KacW1ZWvsR7uQhoyTYvKAupfQrRGBFM352Gk= +golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= diff --git a/pkg/appgw/configbuilder_test.go b/pkg/appgw/configbuilder_test.go index ad88ca4db..f4d523253 100644 --- a/pkg/appgw/configbuilder_test.go +++ b/pkg/appgw/configbuilder_test.go @@ -412,6 +412,7 @@ var _ = Describe("Tests `appgw.ConfigBuilder`", func() { -- "httpListener": { -- "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-08ba2bb7da9df5927d900fca8ce96ba5" -- }, +-- "priority": 19000, -- "ruleType": "Basic" -- } -- } @@ -645,6 +646,7 @@ var _ = Describe("Tests `appgw.ConfigBuilder`", func() { -- "httpListener": { -- "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-e1903c8aa3446b7b3207aec6d6ecba8a" -- }, +-- "priority": 19500, -- "ruleType": "Basic" -- } -- } @@ -925,6 +927,7 @@ var _ = Describe("Tests `appgw.ConfigBuilder`", func() { -- "httpListener": { -- "id": "/subscriptions/--subscription--/resourceGroups/--resource-group--/providers/Microsoft.Network/applicationGateways/--app-gw-name--/httpListeners/fl-b2dba5a02f61d8615cc23d8df120ac20" -- }, +-- "priority": 19000, -- "ruleType": "Basic" -- } -- } diff --git a/pkg/appgw/frontend_listeners.go b/pkg/appgw/frontend_listeners.go index 5ee87043e..5d60b854b 100644 --- a/pkg/appgw/frontend_listeners.go +++ b/pkg/appgw/frontend_listeners.go @@ -222,3 +222,25 @@ func (c *appGwConfigBuilder) groupListenersByListenerIdentifier(cbCtx *ConfigBui return listenersByID } + +// LookupListener gets by ID. +func LookupListenerByID(listeners *[]n.ApplicationGatewayHTTPListener, ID *string) *n.ApplicationGatewayHTTPListener { + for _, listener := range *listeners { + if *listener.ID == *ID { + return &listener + } + } + return nil +} + +func IsMutliSiteListener(listener *n.ApplicationGatewayHTTPListener) bool { + if listener == nil { + return false + } + + if listener.HostName != nil && *listener.HostName != "" { + return true + } + + return listener.HostNames != nil && len(*listener.HostNames) > 0 +} diff --git a/pkg/appgw/frontend_listeners_test.go b/pkg/appgw/frontend_listeners_test.go index 543834748..03b123420 100644 --- a/pkg/appgw/frontend_listeners_test.go +++ b/pkg/appgw/frontend_listeners_test.go @@ -523,4 +523,69 @@ var _ = Describe("MutateAppGateway ingress rules and parse frontend listener con Expect(*listeners).To(ContainElement(expectedListener80)) }) }) + + Context("LookupListenerByID tests", func() { + It("should return nil when listener for ID is not found", func() { + listeners := []n.ApplicationGatewayHTTPListener{ + expectedListener443, + expectedListener80, + } + + listener := LookupListenerByID(&listeners, to.StringPtr("missing-listener")) + Expect(listener).To(BeNil()) + }) + + It("should return listener when listener for ID is found", func() { + listeners := []n.ApplicationGatewayHTTPListener{ + expectedListener443, + expectedListener80, + } + + listener443ID := to.StringPtr(resPref + "httpListeners/" + listenerID443Name) + + listener := LookupListenerByID(&listeners, listener443ID) + Expect(*listener).To(Equal(expectedListener443)) + }) + }) + + Context("IsMutliSiteListener tests", func() { + It("should return false for basic listener", func() { + listener := &n.ApplicationGatewayHTTPListener{ + ApplicationGatewayHTTPListenerPropertiesFormat: &n.ApplicationGatewayHTTPListenerPropertiesFormat{}, + } + + Expect(IsMutliSiteListener(listener)).To(BeFalse()) + + listener = &n.ApplicationGatewayHTTPListener{ + ApplicationGatewayHTTPListenerPropertiesFormat: &n.ApplicationGatewayHTTPListenerPropertiesFormat{ + HostName: to.StringPtr(""), + HostNames: to.StringSlicePtr([]string{}), + }, + } + + Expect(IsMutliSiteListener(listener)).To(BeFalse()) + }) + + It("should return true if listener has hostname populated", func() { + listener := &n.ApplicationGatewayHTTPListener{ + ApplicationGatewayHTTPListenerPropertiesFormat: &n.ApplicationGatewayHTTPListenerPropertiesFormat{ + HostName: to.StringPtr("foo"), + }, + } + + Expect(IsMutliSiteListener(listener)).To(BeTrue()) + }) + + It("should return true if listener has hostnames populated", func() { + listener := &n.ApplicationGatewayHTTPListener{ + ApplicationGatewayHTTPListenerPropertiesFormat: &n.ApplicationGatewayHTTPListenerPropertiesFormat{ + HostNames: to.StringSlicePtr([]string{ + "foo", + }), + }, + } + + Expect(IsMutliSiteListener(listener)).To(BeTrue()) + }) + }) }) diff --git a/pkg/appgw/requestroutingrules.go b/pkg/appgw/requestroutingrules.go index 11b9e0935..ded476bbf 100755 --- a/pkg/appgw/requestroutingrules.go +++ b/pkg/appgw/requestroutingrules.go @@ -21,6 +21,10 @@ import ( "github.com/Azure/application-gateway-kubernetes-ingress/pkg/utils" ) +const ( + MaxAllowedPriority = 20000 +) + func (c *appGwConfigBuilder) RequestRoutingRules(cbCtx *ConfigBuilderContext) error { requestRoutingRules, pathMaps := c.getRules(cbCtx) @@ -56,6 +60,10 @@ func (c *appGwConfigBuilder) RequestRoutingRules(cbCtx *ConfigBuilderContext) er } sort.Sort(sorter.ByRequestRoutingRuleName(requestRoutingRules)) + + // Apply rule priority after sorting to come up with stable priority. + requestRoutingRules = c.assignPriorityWhereMissing(requestRoutingRules) + c.appGw.RequestRoutingRules = &requestRoutingRules return nil @@ -463,3 +471,58 @@ func preparePathFromPathType(path string, pathType *networking.PathType) string func isPathCatchAll(path string, pathType *networking.PathType) bool { return len(path) == 0 || path == "/*" || path == "/" } + +// assignPriorityWhereMissing assigns priority to rules that don't have rule priority assigned +// by the user. +// This logic is similar to how AppGW populates rule priority internally. +// Multisite rule is given higher priority than basic rule. +func (c *appGwConfigBuilder) assignPriorityWhereMissing(rules []n.ApplicationGatewayRequestRoutingRule) []n.ApplicationGatewayRequestRoutingRule { + + usedUpPriorities := make(map[int32]interface{}) + for _, rule := range rules { + if rule.Priority != nil { + usedUpPriorities[*rule.Priority] = nil + } + } + + var lastMultiSiteRulePriority int32 = 19000 + var lastBasicRulePriority int32 = 19500 + var priorityJump int32 = 5 + for _, rule := range rules { + listener := LookupListenerByID(c.appGw.HTTPListeners, rule.HTTPListener.ID) + + if rule.Priority != nil { + // rule already has a priority assigned + continue + } + + // find priority to assign to the rule + var priority int32 + if IsMutliSiteListener(listener) { + priority = findNextFreePriority(usedUpPriorities, lastMultiSiteRulePriority, priorityJump) + lastMultiSiteRulePriority = priority + } else { + priority = findNextFreePriority(usedUpPriorities, lastBasicRulePriority, priorityJump) + lastBasicRulePriority = priority + } + + rule.Priority = to.Int32Ptr(priority) + usedUpPriorities[priority] = nil + } + + return rules +} + +func findNextFreePriority(usedUpPriorities map[int32]interface{}, lastPriority int32, jump int32) int32 { + priority := lastPriority + for { + if _, exists := usedUpPriorities[priority]; !exists { + return priority + } + + priority = priority + jump + if priority > MaxAllowedPriority { + return MaxAllowedPriority + } + } +} diff --git a/pkg/appgw/requestroutingrules_test.go b/pkg/appgw/requestroutingrules_test.go index a36b552fa..1d9ad9db8 100755 --- a/pkg/appgw/requestroutingrules_test.go +++ b/pkg/appgw/requestroutingrules_test.go @@ -23,6 +23,8 @@ import ( ) var _ = Describe("Test routing rules generations", func() { + defer GinkgoRecover() + checkPathRules := func(urlPathMap *n.ApplicationGatewayURLPathMap, pathRuleCount int) { if pathRuleCount == 0 { Expect(urlPathMap.PathRules).To(BeNil()) @@ -496,6 +498,7 @@ var _ = Describe("Test routing rules generations", func() { "/providers/Microsoft.Network/applicationGateways/--app-gw-name--" + "/redirectConfigurations/sslr-" + expectedListenerID443Name)}, ProvisioningState: "", + Priority: to.Int32Ptr(19000), }, Name: to.StringPtr("rr-" + utils.GetHashCode(expectedListenerID80)), Etag: to.StringPtr("*"), @@ -524,6 +527,7 @@ var _ = Describe("Test routing rules generations", func() { RewriteRuleSet: nil, RedirectConfiguration: nil, ProvisioningState: "", + Priority: to.Int32Ptr(19005), }, Name: to.StringPtr("rr-" + utils.GetHashCode(expectedListenerID443)), Etag: to.StringPtr("*"), @@ -1001,6 +1005,108 @@ var _ = Describe("Test routing rules generations", func() { }) }) + Describe("test auto assigning missing routing rule prirority", func() { + var configBuilder appGwConfigBuilder + var ingress *networking.Ingress + var cbCtx *ConfigBuilderContext + // var prohibitedTargets []*ptv1.AzureIngressProhibitedTarget + var ruleCount int + var ingresses []*networking.Ingress + + BeforeEach(func() { + ingresses = make([]*networking.Ingress, 0) + configBuilder = newConfigBuilderFixture(nil) + backend := tests.NewIngressBackendFixture(tests.ServiceName, int32(80)) + + endpoint := tests.NewEndpointsFixture() + service := tests.NewServiceFixture(*tests.NewServicePortsFixture()...) + + Expect(configBuilder.k8sContext.Caches.Endpoints.Add(endpoint)).To(Succeed()) + Expect(configBuilder.k8sContext.Caches.Service.Add(service)).To(Succeed()) + + // The upper bound for rule priority defined as 20000 in NRP. + // With multisite listeners, we auto generate priority starting from + // 19000 with an increment of 10. Thus, AGIC's bahivor in + // scenarios where customers define over 50 rules for basic + // listeners is undefined. + ruleCount = 50 + for i := 0; i < ruleCount; i++ { + ingress = &networking.Ingress{} + ingress.Name = fmt.Sprint(i) + ingress.Namespace = tests.Namespace + ingress.Annotations = map[string]string{ + annotations.OverrideFrontendPortKey: fmt.Sprint(i), + annotations.IngressClassKey: tests.IngressClassController, + } + rule := tests.NewIngressRuleFixture("", "/", *backend) + ingress.Spec.Rules = []networking.IngressRule{rule} + + // Turn off TLS so we have better control over the number of + // generated request routing rules. + ingress.Spec.TLS = nil + ingresses = append(ingresses, ingress) + Expect(configBuilder.k8sContext.Caches.Ingress.Add(ingress)).To(Succeed()) + } + + cbCtx = &ConfigBuilderContext{ + IngressList: ingresses, + DefaultAddressPoolID: to.StringPtr("xx"), + DefaultHTTPSettingsID: to.StringPtr("yy"), + } + }) + + Context("when listeners are of multi-site type", func() { + var minPriority, maxPriority int32 = 19000, 19500 + + BeforeEach(func() { + for idx, ingress := range ingresses { + ingress.Annotations[annotations.HostNameExtensionKey] = fmt.Sprintf("host-%d", idx) + } + + Expect(configBuilder.Listeners(cbCtx)).To(Succeed()) + Expect(configBuilder.RequestRoutingRules(cbCtx)).To(Succeed()) + }) + + It("should have the expected number of request routing rules", func() { + Expect(*configBuilder.appGw.RequestRoutingRules).To(HaveLen(ruleCount)) + }) + + It("should all have unique priorities assigned", func() { + Expect(*configBuilder.appGw.RequestRoutingRules).To(Satisfy(haveUniquePriorities)) + }) + + It("should all have priorities in range", func() { + Expect(*configBuilder.appGw.RequestRoutingRules).To(HaveEach(Satisfy(func(r n.ApplicationGatewayRequestRoutingRule) bool { + return minPriority <= *r.Priority && *r.Priority < maxPriority + }))) + }) + }) + + Context("when listeners are of basic type", func() { + var minPriority, maxPriority int32 = 19500, 20000 + + BeforeEach(func() { + Expect(configBuilder.Listeners(cbCtx)).To(Succeed()) + Expect(configBuilder.RequestRoutingRules(cbCtx)).To(Succeed()) + }) + + It("should have the expected number of request routing rules", func() { + Expect(*configBuilder.appGw.RequestRoutingRules).To(HaveLen(ruleCount)) + }) + + It("should all have unique priorities assigned", func() { + Expect(*configBuilder.appGw.RequestRoutingRules).To(Satisfy(haveUniquePriorities)) + }) + + It("should all have priorities in range", func() { + Expect(*configBuilder.appGw.RequestRoutingRules).To(HaveEach(Satisfy(func(r n.ApplicationGatewayRequestRoutingRule) bool { + return minPriority <= *r.Priority && *r.Priority < maxPriority + }))) + }) + }) + + }) + Context("test preparePathFromPathType", func() { It("should append * when pathType is Prefix", func() { pathType := networking.PathTypePrefix @@ -1055,3 +1161,13 @@ var _ = Describe("Test routing rules generations", func() { }) }) }) + +func haveUniquePriorities(rules []n.ApplicationGatewayRequestRoutingRule) bool { + priorities := make(map[int32]bool) + for _, r := range rules { + if _, ok := priorities[*r.Priority]; ok { + return false + } + } + return true +}