diff --git a/tests/framework/crossplane.go b/tests/framework/crossplane.go index e6b968946..f9e109a47 100644 --- a/tests/framework/crossplane.go +++ b/tests/framework/crossplane.go @@ -28,10 +28,10 @@ type ExpectedNginxField struct { File string // Location is the location name that the directive should exist in. Location string - // Servers are the server names that the directive should exist in. - Servers []string - // Upstreams are the upstream names that the directive should exist in. - Upstreams []string + // Server is the server name that the directive should exist in. + Server string + // Upstream is the upstream name that the directive should exist in. + Upstream string // ValueSubstringAllowed allows the expected value to be a substring of the real value. // This makes it easier for cases when real values are complex file names or contain things we // don't care about, and we just want to check if a substring exists. @@ -41,64 +41,74 @@ type ExpectedNginxField struct { // ValidateNginxFieldExists accepts the nginx config and the configuration for the expected field, // and returns whether or not that field exists where it should. func ValidateNginxFieldExists(conf *Payload, expFieldCfg ExpectedNginxField) error { + var directiveFoundInServer, directiveFoundInUpstream bool + + b, err := json.Marshal(conf) + if err != nil { + return fmt.Errorf("error marshaling nginx config: %w", err) + } + for _, config := range conf.Config { if !strings.Contains(config.File, expFieldCfg.File) { continue } for _, directive := range config.Parsed { - if len(expFieldCfg.Servers) == 0 { + if len(expFieldCfg.Server) == 0 && len(expFieldCfg.Upstream) == 0 { if expFieldCfg.fieldFound(directive) { return nil } continue } - if err := validateServerBlockDirectives(expFieldCfg, *directive); err != nil { - return err + directiveFoundInServer = validateServerBlockDirectives(expFieldCfg, *directive) + + directiveFoundInUpstream = validateUpstreamDirectives(expFieldCfg, *directive) + + if len(expFieldCfg.Server) > 0 && directiveFoundInServer { + return nil } - if err := validateUpstreamDirectives(expFieldCfg, directive); err != nil { - return err + if len(expFieldCfg.Upstream) > 0 && directiveFoundInUpstream { + return nil } } } - b, err := json.Marshal(conf) - if err != nil { - return fmt.Errorf("error marshaling nginx config: %w", err) - } - - return fmt.Errorf("field not found; expected: %+v\nNGINX conf: %s", expFieldCfg, string(b)) + return fmt.Errorf("directive %s not found in: nginx config %s", expFieldCfg.Directive, string(b)) } -func validateServerBlockDirectives(expFieldCfg ExpectedNginxField, directive Directive) error { - for _, serverName := range expFieldCfg.Servers { - if directive.Directive == "server" && getServerName(directive.Block) == serverName { - for _, serverDirective := range directive.Block { - if expFieldCfg.Location == "" && !expFieldCfg.fieldFound(serverDirective) { - return fmt.Errorf("field not found; expected: %+v\nNGINX conf: %s", expFieldCfg, serverDirective.Directive) - } else if serverDirective.Directive == "location" && - !fieldExistsInLocation(serverDirective, expFieldCfg) { - return fmt.Errorf("field not found; expected: %+v\nNGINX conf: %s", expFieldCfg, serverDirective.Directive) - } +func validateServerBlockDirectives( + expFieldCfg ExpectedNginxField, + directive Directive, +) bool { + var fieldFound bool + if directive.Directive == "server" && getServerName(directive.Block) == expFieldCfg.Server { + for _, serverDirective := range directive.Block { + if expFieldCfg.Location == "" && expFieldCfg.fieldFound(serverDirective) { + fieldFound = true + } else if serverDirective.Directive == "location" && + fieldExistsInLocation(serverDirective, expFieldCfg) { + fieldFound = true } } } - return nil + return fieldFound } -func validateUpstreamDirectives(expFieldCfg ExpectedNginxField, directive *Directive) error { - for _, upstreamName := range expFieldCfg.Upstreams { - if directive.Directive == "upstream" && directive.Args[0] == upstreamName { - for _, upstreamDirective := range directive.Block { - if !expFieldCfg.fieldFound(upstreamDirective) { - return fmt.Errorf("field not found; expected: %+v\nNGINX conf: %s", expFieldCfg, upstreamDirective.Directive) - } +func validateUpstreamDirectives( + expFieldCfg ExpectedNginxField, + directive Directive, +) bool { + var fieldFound bool + if directive.Directive == "upstream" && directive.Args[0] == expFieldCfg.Upstream { + for _, directive := range directive.Block { + if expFieldCfg.fieldFound(directive) { + fieldFound = true } } } - return nil + return fieldFound } func getServerName(serverBlock Directives) string { diff --git a/tests/suite/client_settings_test.go b/tests/suite/client_settings_test.go index b5e5a8ec4..f7ab05c88 100644 --- a/tests/suite/client_settings_test.go +++ b/tests/suite/client_settings_test.go @@ -117,7 +117,13 @@ var _ = Describe("ClientSettingsPolicy", Ordered, Label("functional", "cspolicy" Directive: "include", Value: fmt.Sprintf("%s_gw-csp.conf", filePrefix), File: "http.conf", - Servers: []string{"*.example.com", "cafe.example.com"}, + Server: "*.example.com", + }, + { + Directive: "include", + Value: fmt.Sprintf("%s_gw-csp.conf", filePrefix), + File: "http.conf", + Server: "cafe.example.com", }, { Directive: "client_max_body_size", @@ -150,7 +156,7 @@ var _ = Describe("ClientSettingsPolicy", Ordered, Label("functional", "cspolicy" Directive: "include", Value: fmt.Sprintf("%s_coffee-route-csp.conf", filePrefix), File: "http.conf", - Servers: []string{"cafe.example.com"}, + Server: "cafe.example.com", Location: "/coffee", }, { @@ -164,7 +170,7 @@ var _ = Describe("ClientSettingsPolicy", Ordered, Label("functional", "cspolicy" Directive: "include", Value: fmt.Sprintf("%s_tea-route-csp.conf", filePrefix), File: "http.conf", - Servers: []string{"cafe.example.com"}, + Server: "cafe.example.com", Location: "/tea", }, { @@ -178,7 +184,7 @@ var _ = Describe("ClientSettingsPolicy", Ordered, Label("functional", "cspolicy" Directive: "include", Value: fmt.Sprintf("%s_soda-route-csp.conf", filePrefix), File: "http.conf", - Servers: []string{"cafe.example.com"}, + Server: "cafe.example.com", Location: "/soda", }, { @@ -192,7 +198,7 @@ var _ = Describe("ClientSettingsPolicy", Ordered, Label("functional", "cspolicy" Directive: "include", Value: fmt.Sprintf("%s_grpc-route-csp.conf", filePrefix), File: "http.conf", - Servers: []string{"*.example.com"}, + Server: "*.example.com", Location: "/helloworld.Greeter/SayHello", }, { diff --git a/tests/suite/manifests/upstream-settings-policy/invalid-target-usps.yaml b/tests/suite/manifests/upstream-settings-policy/invalid-target-usps.yaml index f10b3bb94..5a223f2f3 100644 --- a/tests/suite/manifests/upstream-settings-policy/invalid-target-usps.yaml +++ b/tests/suite/manifests/upstream-settings-policy/invalid-target-usps.yaml @@ -4,12 +4,13 @@ metadata: name: gateway-not-valid spec: gatewayClassName: nginx - addresses: "10.0.0.1" + addresses: + - value: "10.0.0.1" listeners: - - name: http - port: 80 - protocol: HTTP - hostname: "soda.example.com" + - name: http + port: 80 + protocol: HTTP + hostname: "soda.example.com" --- apiVersion: apps/v1 kind: Deployment @@ -26,10 +27,10 @@ spec: app: soda spec: containers: - - name: soda - image: nginxdemos/nginx-hello:plain-text - ports: - - containerPort: 8080 + - name: soda + image: nginxdemos/nginx-hello:plain-text + ports: + - containerPort: 8080 --- apiVersion: v1 kind: Service @@ -37,31 +38,31 @@ metadata: name: soda spec: ports: - - port: 80 - targetPort: 8080 - protocol: TCP - name: http + - port: 80 + targetPort: 8080 + protocol: TCP + name: http selector: app: soda --- apiVersion: gateway.networking.k8s.io/v1 kind: HTTPRoute metadata: - name: soda + name: sode spec: parentRefs: - - name: gateway-httproute-allowed - sectionName: http + - name: gateway-not-valid + sectionName: http hostnames: - - "soda.example.com" + - "soda.example.com" rules: - - matches: - - path: - type: Exact - value: /soda - backendRefs: - - name: soda - port: 80 + - matches: + - path: + type: Exact + value: /soda + backendRefs: + - name: soda + port: 80 --- apiVersion: gateway.nginx.org/v1alpha1 kind: UpstreamSettingsPolicy @@ -70,9 +71,9 @@ metadata: spec: zoneSize: 512k targetRefs: - - group: core - kind: Service - name: soda + - group: core + kind: Service + name: soda keepAlive: connections: 10 requests: 3 diff --git a/tests/suite/manifests/upstream-settings-policy/routes.yaml b/tests/suite/manifests/upstream-settings-policy/routes.yaml index 37defe1a3..f26e705a4 100644 --- a/tests/suite/manifests/upstream-settings-policy/routes.yaml +++ b/tests/suite/manifests/upstream-settings-policy/routes.yaml @@ -18,6 +18,25 @@ spec: port: 80 --- apiVersion: gateway.networking.k8s.io/v1 +kind: HTTPRoute +metadata: + name: tea +spec: + parentRefs: + - name: gateway + sectionName: http + hostnames: + - "cafe.example.com" + rules: + - matches: + - path: + type: Exact + value: /tea + backendRefs: + - name: tea + port: 80 +--- +apiVersion: gateway.networking.k8s.io/v1 kind: GRPCRoute metadata: name: grpc-route diff --git a/tests/suite/manifests/upstream-settings-policy/valid-merge-usps.yaml b/tests/suite/manifests/upstream-settings-policy/valid-merge-usps.yaml index bc232fa20..cdbab6c26 100644 --- a/tests/suite/manifests/upstream-settings-policy/valid-merge-usps.yaml +++ b/tests/suite/manifests/upstream-settings-policy/valid-merge-usps.yaml @@ -1,18 +1,20 @@ apiVersion: gateway.nginx.org/v1alpha1 kind: UpstreamSettingsPolicy metadata: - name: coffee-svc-usp-1 + name: merge-usp-1 spec: - zoneSize: 1g targetRefs: - group: core kind: Service name: coffee + keepAlive: + time: 1m + timeout: 5h --- apiVersion: gateway.nginx.org/v1alpha1 kind: UpstreamSettingsPolicy metadata: - name: coffee-svc-usp-2 + name: merge-usp-2 spec: targetRefs: - group: core @@ -21,5 +23,25 @@ spec: keepAlive: connections: 100 requests: 55 - time: 1m - timeout: 5h +--- +apiVersion: gateway.nginx.org/v1alpha1 +kind: UpstreamSettingsPolicy +metadata: + name: z-usp-wins +spec: + zoneSize: 64k + targetRefs: + - group: core + kind: Service + name: tea +--- +apiVersion: gateway.nginx.org/v1alpha1 +kind: UpstreamSettingsPolicy +metadata: + name: a-usp +spec: + zoneSize: 128k + targetRefs: + - group: core + kind: Service + name: tea diff --git a/tests/suite/manifests/upstream-settings-policy/valid-usps-first-wins.yaml b/tests/suite/manifests/upstream-settings-policy/valid-usps-first-wins.yaml deleted file mode 100644 index 3bdf1dabb..000000000 --- a/tests/suite/manifests/upstream-settings-policy/valid-usps-first-wins.yaml +++ /dev/null @@ -1,21 +0,0 @@ -apiVersion: gateway.nginx.org/v1alpha1 -kind: UpstreamSettingsPolicy -metadata: - name: z-coffee-svc-usp -spec: - zoneSize: 64k - targetRefs: - - group: core - kind: Service - name: coffee ---- -apiVersion: gateway.nginx.org/v1alpha1 -kind: UpstreamSettingsPolicy -metadata: - name: a-coffee-svc-usp -spec: - zoneSize: 128k - targetRefs: - - group: core - kind: Service - name: coffee diff --git a/tests/suite/manifests/upstream-settings-policy/valid-usps.yaml b/tests/suite/manifests/upstream-settings-policy/valid-usps.yaml index 6865c5df7..cb4e4bf05 100644 --- a/tests/suite/manifests/upstream-settings-policy/valid-usps.yaml +++ b/tests/suite/manifests/upstream-settings-policy/valid-usps.yaml @@ -21,10 +21,10 @@ kind: UpstreamSettingsPolicy metadata: name: grpc-svc-usp spec: - targetRef: - group: core - kind: Service - name: grpc-backend + targetRefs: + - group: core + kind: Service + name: grpc-backend zoneSize: 64k keepAlive: connections: 100 diff --git a/tests/suite/snippets_filter_test.go b/tests/suite/snippets_filter_test.go index 632a8199e..e92e44d0a 100644 --- a/tests/suite/snippets_filter_test.go +++ b/tests/suite/snippets_filter_test.go @@ -149,7 +149,7 @@ var _ = Describe("SnippetsFilter", Ordered, Label("functional", "snippets-filter { Directive: "include", Value: fmt.Sprintf("%s%s", httpServerContext, httpRouteSuffix), - Servers: []string{"cafe.example.com"}, + Server: "cafe.example.com", File: "http.conf", }, { @@ -157,7 +157,7 @@ var _ = Describe("SnippetsFilter", Ordered, Label("functional", "snippets-filter Value: fmt.Sprintf("%s%s", httpServerLocationContext, httpRouteSuffix), File: "http.conf", Location: "/coffee", - Servers: []string{"cafe.example.com"}, + Server: "cafe.example.com", }, { Directive: "keepalive_time", @@ -194,7 +194,7 @@ var _ = Describe("SnippetsFilter", Ordered, Label("functional", "snippets-filter { Directive: "include", Value: fmt.Sprintf("%s%s", httpServerContext, grpcRouteSuffix), - Servers: []string{"*.example.com"}, + Server: "*.example.com", File: "http.conf", }, { @@ -207,7 +207,7 @@ var _ = Describe("SnippetsFilter", Ordered, Label("functional", "snippets-filter Value: fmt.Sprintf("%s%s", httpServerLocationContext, grpcRouteSuffix), File: "http.conf", Location: "/helloworld.Greeter/SayHello", - Servers: []string{"*.example.com"}, + Server: "*.example.com", }, }), ) diff --git a/tests/suite/upstream_settings_test.go b/tests/suite/upstream_settings_test.go index c9b50358d..4b77ed7b3 100644 --- a/tests/suite/upstream_settings_test.go +++ b/tests/suite/upstream_settings_test.go @@ -61,22 +61,28 @@ var _ = Describe("UpstreamSettingsPolicy", Ordered, Label("uspolicy"), func() { }) Specify("they are accepted", func() { - usPolicies := []string{ - "multiple-http-svc-usp", - "grpc-svc-usp", + usPolicies := map[string]int{ + "multiple-http-svc-usp": 2, + "grpc-svc-usp": 1, } - for _, name := range usPolicies { + for name, ancestorCount := range usPolicies { uspolicyNsName := types.NamespacedName{Name: name, Namespace: namespace} gatewayNsName := types.NamespacedName{Name: "gateway", Namespace: namespace} - err := waitForUSPolicyStatus(uspolicyNsName, gatewayNsName, metav1.ConditionTrue, v1alpha2.PolicyReasonAccepted) + err := waitForUSPolicyStatus( + uspolicyNsName, + gatewayNsName, + metav1.ConditionTrue, + v1alpha2.PolicyReasonAccepted, + ancestorCount, + ) Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("%s was not accepted", name)) } }) Context("verify working traffic", func() { - It("should return a 200 response for HTTPRoute", func() { + It("should return a 200 response for HTTPRoutes", func() { port := 80 if portFwdPort != 0 { port = portFwdPort @@ -116,9 +122,6 @@ var _ = Describe("UpstreamSettingsPolicy", Ordered, Label("uspolicy"), func() { Expect(err).ToNot(HaveOccurred()) }) - // TODO: important - // The directive file and field value need to be updated based on the - // implementation of the UpstreamSettingsPolicy and how they are specified in the config files. DescribeTable("are set properly for", func(expCfgs []framework.ExpectedNginxField) { for _, expCfg := range expCfgs { @@ -128,70 +131,88 @@ var _ = Describe("UpstreamSettingsPolicy", Ordered, Label("uspolicy"), func() { Entry("HTTP upstreams", []framework.ExpectedNginxField{ { Directive: "zone", - Value: "default_coffee_80 512k", - Upstreams: []string{"default_coffee_80"}, + Value: "uspolicy_coffee_80 512k", + Upstream: "uspolicy_coffee_80", File: "http.conf", }, { Directive: "zone", - Value: "default_tea_80 512k", - Upstreams: []string{"default_tea_80"}, + Value: "uspolicy_tea_80 512k", + Upstream: "uspolicy_tea_80", File: "http.conf", }, { Directive: "keepalive", Value: "10", - Upstreams: []string{"default_coffee_80", "default_tea_80"}, + Upstream: "uspolicy_coffee_80", + File: "http.conf", + }, + { + Directive: "keepalive_requests", + Value: "3", + Upstream: "uspolicy_coffee_80", File: "http.conf", }, { Directive: "keepalive_requests", Value: "3", - Upstreams: []string{"default_coffee_80", "default_tea_80"}, + Upstream: "uspolicy_tea_80", + File: "http.conf", + }, + { + Directive: "keepalive_time", + Value: "10s", + Upstream: "uspolicy_coffee_80", File: "http.conf", }, { Directive: "keepalive_time", Value: "10s", - Upstreams: []string{"default_coffee_80", "default_tea_80"}, + Upstream: "uspolicy_tea_80", + File: "http.conf", + }, + { + Directive: "keepalive_timeout", + Value: "50s", + Upstream: "uspolicy_coffee_80", File: "http.conf", }, { Directive: "keepalive_timeout", Value: "50s", - Upstreams: []string{"default_coffee_80", "default_tea_80"}, + Upstream: "uspolicy_tea_80", File: "http.conf", }, }), Entry("GRPC upstreams", []framework.ExpectedNginxField{ { Directive: "zone", - Value: "default_grpc-backend_8080 64k", - Upstreams: []string{"default_grpc-backend_8080"}, + Value: "uspolicy_grpc-backend_8080 64k", + Upstream: "uspolicy_grpc-backend_8080", File: "http.conf", }, { Directive: "keepalive", Value: "100", - Upstreams: []string{"default_grpc-backend_8080"}, + Upstream: "uspolicy_grpc-backend_8080", File: "http.conf", }, { Directive: "keepalive_requests", Value: "45", - Upstreams: []string{"default_grpc-backend_8080"}, + Upstream: "uspolicy_grpc-backend_8080", File: "http.conf", }, { Directive: "keepalive_time", Value: "1m", - Upstreams: []string{"default_grpc-backend_8080"}, + Upstream: "uspolicy_grpc-backend_8080", File: "http.conf", }, { Directive: "keepalive_timeout", Value: "5h", - Upstreams: []string{"default_grpc-backend_8080"}, + Upstream: "uspolicy_grpc-backend_8080", File: "http.conf", }, }), @@ -200,219 +221,151 @@ var _ = Describe("UpstreamSettingsPolicy", Ordered, Label("uspolicy"), func() { }) When("multiple UpstreamSettingsPolicies with overlapping settings target the same Service", func() { - Specify("configuring distinct settings, the policies are merged", func() { - files := []string{ - "upstream-settings-policy/valid-merge-usps.yaml", - } - Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed()) + usps := []string{ + "upstream-settings-policy/valid-merge-usps.yaml", + } - gatewayNsName := types.NamespacedName{Name: "gateway", Namespace: namespace} - nsname := types.NamespacedName{Name: "coffee-svc-usp-1", Namespace: namespace} - Expect(waitForUSPolicyStatus( - nsname, - gatewayNsName, - metav1.ConditionTrue, - v1alpha2.PolicyReasonAccepted, - )).To(Succeed()) - - nsname = types.NamespacedName{Name: "coffee-svc-usp-2", Namespace: namespace} - Expect(waitForUSPolicyStatus( - nsname, - gatewayNsName, - metav1.ConditionTrue, - v1alpha2.PolicyReasonAccepted, - )).To(Succeed()) - - Context("verify working traffic", func() { - It("should return a 200 response for HTTPRoutes", func() { - port := 80 - if portFwdPort != 0 { - port = portFwdPort - } - baseCoffeeURL := fmt.Sprintf("http://cafe.example.com:%d%s", port, "/coffee") - - Eventually( - func() error { - return expectRequestToSucceed(baseCoffeeURL, address, "URI: /coffee") - }). - WithTimeout(timeoutConfig.RequestTimeout). - WithPolling(500 * time.Millisecond). - Should(Succeed()) - }) - }) + BeforeAll(func() { + Expect(resourceManager.ApplyFromFiles(usps, namespace)).To(Succeed()) + }) - Context("nginx directives", func() { - var conf *framework.Payload + AfterAll(func() { + Expect(resourceManager.DeleteFromFiles(usps, namespace)).To(Succeed()) + }) - BeforeAll(func() { - podNames, err := framework.GetReadyNGFPodNames(k8sClient, ngfNamespace, releaseName, timeoutConfig.GetTimeout) - Expect(err).ToNot(HaveOccurred()) - Expect(podNames).To(HaveLen(1)) + DescribeTable("upstreamSettingsPolicy status is set as expected", + func(name string, status metav1.ConditionStatus, condReason v1alpha2.PolicyConditionReason, ancestorCount int) { + gatewayNsName := types.NamespacedName{Name: "gateway", Namespace: namespace} + nsname := types.NamespacedName{Name: name, Namespace: namespace} + Expect(waitForUSPolicyStatus(nsname, gatewayNsName, status, condReason, ancestorCount)).To(Succeed()) + }, + Entry("uspolicy merge-usp-1", "merge-usp-1", metav1.ConditionTrue, v1alpha2.PolicyReasonAccepted, 1), + Entry("uspolicy merge-usp-2", "merge-usp-2", metav1.ConditionTrue, v1alpha2.PolicyReasonAccepted, 1), + Entry("uspolicy a-usp", "a-usp", metav1.ConditionTrue, v1alpha2.PolicyReasonAccepted, 1), + Entry("uspolicy z-usp", "z-usp-wins", metav1.ConditionFalse, v1alpha2.PolicyReasonConflicted, 1), + ) - ngfPodName := podNames[0] + Context("verify working traffic", func() { + It("should return a 200 response for HTTPRoutes", func() { + port := 80 + if portFwdPort != 0 { + port = portFwdPort + } + baseCoffeeURL := fmt.Sprintf("http://cafe.example.com:%d%s", port, "/coffee") + baseTeaURL := fmt.Sprintf("http://cafe.example.com:%d%s", port, "/tea") - conf, err = resourceManager.GetNginxConfig(ngfPodName, ngfNamespace) - Expect(err).ToNot(HaveOccurred()) - }) + Eventually( + func() error { + return expectRequestToSucceed(baseCoffeeURL, address, "URI: /coffee") + }). + WithTimeout(timeoutConfig.RequestTimeout). + WithPolling(1000 * time.Millisecond). + Should(Succeed()) - // TODO: important - // The directive file and field value need to be updated based on the - // implementation of the UpstreamSettingsPolicy and how they are specified in the config files. - DescribeTable("are set properly for", - func(expCfgs []framework.ExpectedNginxField) { - for _, expCfg := range expCfgs { - Expect(framework.ValidateNginxFieldExists(conf, expCfg)).To(Succeed()) - } - }, - Entry("Coffee upstream", []framework.ExpectedNginxField{ - { - Directive: "zone", - Value: "default_coffee_80 1g", - Upstreams: []string{"default_coffee_80"}, - File: "http.conf", - }, - { - Directive: "keepalive", - Value: "100", - Upstreams: []string{"default_coffee_80"}, - File: "http.conf", - }, - { - Directive: "keepalive_requests", - Value: "55", - Upstreams: []string{"default_coffee_80"}, - File: "http.conf", - }, - { - Directive: "keepalive_time", - Value: "1m", - Upstreams: []string{"default_coffee_80"}, - File: "http.conf", - }, - { - Directive: "keepalive_timeout", - Value: "5h", - Upstreams: []string{"default_coffee_80"}, - File: "http.conf", - }, - }), - ) + Eventually( + func() error { + return expectRequestToSucceed(baseTeaURL, address, "URI: /tea") + }). + WithTimeout(timeoutConfig.RequestTimeout). + WithPolling(1000 * time.Millisecond). + Should(Succeed()) }) }) - Specify("the policy created first wins", func() { - files := []string{"upstream-settings-policy/valid-usps-first-wins.yaml"} - Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed()) - - gatewayNsName := types.NamespacedName{Name: "gateway", Namespace: namespace} - nsname := types.NamespacedName{Name: "a-coffee-svc-usp", Namespace: namespace} - Expect(waitForUSPolicyStatus( - nsname, - gatewayNsName, - metav1.ConditionTrue, - v1alpha2.PolicyReasonAccepted, - )).To(Succeed()) - - nsname = types.NamespacedName{Name: "z-coffee-svc-usp", Namespace: namespace} - Expect(waitForUSPolicyStatus( - nsname, - gatewayNsName, - metav1.ConditionTrue, - v1alpha2.PolicyReasonAccepted, - )).To(Succeed()) - - Context("verify working traffic", func() { - It("should return a 200 response for HTTPRoute", func() { - port := 80 - if portFwdPort != 0 { - port = portFwdPort - } - baseURL := fmt.Sprintf("http://cafe.example.com:%d%s", port, "/coffee") - - Eventually( - func() error { - return expectRequestToSucceed(baseURL, address, "URI: /coffee") - }). - WithTimeout(timeoutConfig.RequestTimeout). - WithPolling(500 * time.Millisecond). - Should(Succeed()) - }) - }) - - Context("nginx directives", func() { - var conf *framework.Payload - - BeforeAll(func() { - podNames, err := framework.GetReadyNGFPodNames(k8sClient, ngfNamespace, releaseName, timeoutConfig.GetTimeout) - Expect(err).ToNot(HaveOccurred()) - Expect(podNames).To(HaveLen(1)) + Context("nginx directives", func() { + var conf *framework.Payload - ngfPodName := podNames[0] + BeforeAll(func() { + podNames, err := framework.GetReadyNGFPodNames(k8sClient, ngfNamespace, releaseName, timeoutConfig.GetTimeout) + Expect(err).ToNot(HaveOccurred()) + Expect(podNames).To(HaveLen(1)) - conf, err = resourceManager.GetNginxConfig(ngfPodName, ngfNamespace) - Expect(err).ToNot(HaveOccurred()) - }) + ngfPodName := podNames[0] - // TODO: important - // The directive file and field value need to be updated based on the - // implementation of the UpstreamSettingsPolicy and how they are specified in the config files. - DescribeTable("are set properly for", - func(expCfgs []framework.ExpectedNginxField) { - for _, expCfg := range expCfgs { - Expect(framework.ValidateNginxFieldExists(conf, expCfg)).To(Succeed()) - } - }, - Entry("Coffee upstream", []framework.ExpectedNginxField{ - { - Directive: "zone", - Value: "default_coffee_80 128k", - Upstreams: []string{"default_coffee_80"}, - File: "http.conf", - }, - }), - ) + conf, err = resourceManager.GetNginxConfig(ngfPodName, ngfNamespace) + Expect(err).ToNot(HaveOccurred()) }) - }) - AfterEach(func() { - Expect(resourceManager.DeleteFromFiles(files, namespace)).To(Succeed()) + DescribeTable("are set properly for", + func(expCfgs []framework.ExpectedNginxField) { + for _, expCfg := range expCfgs { + Expect(framework.ValidateNginxFieldExists(conf, expCfg)).To(Succeed()) + } + }, + Entry("Coffee upstream", []framework.ExpectedNginxField{ + { + Directive: "zone", + Value: "uspolicy_tea_80 128k", + Upstream: "uspolicy_tea_80", + File: "http.conf", + }, + { + Directive: "zone", + Value: "uspolicy_coffee_80 512k", + Upstream: "uspolicy_coffee_80", + File: "http.conf", + }, + { + Directive: "keepalive", + Value: "100", + Upstream: "uspolicy_coffee_80", + File: "http.conf", + }, + { + Directive: "keepalive_requests", + Value: "55", + Upstream: "uspolicy_coffee_80", + File: "http.conf", + }, + { + Directive: "keepalive_time", + Value: "1m", + Upstream: "uspolicy_coffee_80", + File: "http.conf", + }, + { + Directive: "keepalive_timeout", + Value: "5h", + Upstream: "uspolicy_coffee_80", + File: "http.conf", + }, + }), + ) }) }) - When("UpstreamSettingsPolicy targets a Service that has an invalid Gateway", func() { - Specify("upstreamSettingsPolicy has a condition TargetNotFound", func() { - files := []string{"upstream-settings-policy/invalid-usps.yaml"} + When("UpstreamSettingsPolicy targets a Service that does not exists", func() { + Specify("upstreamSettingsPolicy has no condition set", func() { + files := []string{"upstream-settings-policy/invalid-svc-usps.yaml"} Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed()) - nsname := types.NamespacedName{Name: "soda-svc-usp", Namespace: namespace} + nsname := types.NamespacedName{Name: "does-not-exist", Namespace: namespace} gatewayNsName := types.NamespacedName{Name: "gateway", Namespace: namespace} - Expect(waitForUSPolicyStatus( - nsname, - gatewayNsName, - metav1.ConditionFalse, - v1alpha2.PolicyReasonTargetNotFound, - )).To(Succeed()) + Consistently( + func() bool { + return waitForUSPolicyStatus( + nsname, + gatewayNsName, + metav1.ConditionTrue, + v1alpha2.PolicyReasonAccepted, + 1, + ) != nil + }).WithTimeout(timeoutConfig.GetTimeout). + WithPolling(500 * time.Millisecond). + Should(BeTrue()) Expect(resourceManager.DeleteFromFiles(files, namespace)).To(Succeed()) }) }) - When("UpstreamSettingsPolicy targets a Service that does not exist", func() { - Specify("usptreamSettingsPolicy has no condition", func() { - files := []string{"upstream-settings-policy/invalid-svc-usps.yaml"} + When("UpstreamSettingsPolicy targets a Service that has an invalid Gateway", func() { + Specify("upstreamSettingsPolicy has no condition set", func() { + files := []string{"upstream-settings-policy/invalid-target-usps.yaml"} Expect(resourceManager.ApplyFromFiles(files, namespace)).To(Succeed()) - nsname := types.NamespacedName{Name: "does-not-exists", Namespace: namespace} - gatewayNsName := types.NamespacedName{Name: "gateway", Namespace: namespace} - - Expect(waitForUSPolicyStatus( - nsname, - gatewayNsName, - metav1.ConditionFalse, - v1alpha2.PolicyReasonAccepted, - )).To(BeFalse()) - + nsname := types.NamespacedName{Name: "soda-svc-usp", Namespace: namespace} + gatewayNsName := types.NamespacedName{Name: "gateway-not-valid", Namespace: namespace} Consistently( func() bool { return waitForUSPolicyStatus( @@ -420,6 +373,7 @@ var _ = Describe("UpstreamSettingsPolicy", Ordered, Label("uspolicy"), func() { gatewayNsName, metav1.ConditionTrue, v1alpha2.PolicyReasonAccepted, + 1, ) != nil }).WithTimeout(timeoutConfig.GetTimeout). WithPolling(500 * time.Millisecond). @@ -435,8 +389,9 @@ func waitForUSPolicyStatus( gatewayNsName types.NamespacedName, condStatus metav1.ConditionStatus, condReason v1alpha2.PolicyConditionReason, + expectedAncestorCount int, ) error { - ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetStatusTimeout) + ctx, cancel := context.WithTimeout(context.Background(), timeoutConfig.GetStatusTimeout*2) defer cancel() GinkgoWriter.Printf( @@ -448,7 +403,7 @@ func waitForUSPolicyStatus( return wait.PollUntilContextCancel( ctx, - 500*time.Millisecond, + 2000*time.Millisecond, true, /* poll immediately */ func(ctx context.Context) (bool, error) { var usPolicy ngfAPI.UpstreamSettingsPolicy @@ -464,7 +419,7 @@ func waitForUSPolicyStatus( return false, nil } - if len(usPolicy.Status.Ancestors) != 1 { + if len(usPolicy.Status.Ancestors) != expectedAncestorCount { return false, fmt.Errorf("policy has %d ancestors, expected 1", len(usPolicy.Status.Ancestors)) }