fix path for canary by header + add e2e

e2e/ingress_nginx_canary_test.go

@@ -74,5 +74,125 @@ func TestIngressNGINXCanary(t *testing.T) {
 				},
 			})
 		})
+
+		t.Run("canary by header at path", func(t *testing.T) {
+			suffix, err := randString()
+			require.NoError(t, err)
+			host := fmt.Sprintf("canary-header-path-%s.com", suffix)
+			runTestCase(t, &testCase{
+				gatewayImplementation: istio.ProviderName,
+				providers:             []string{ingressnginx.Name},
+				providerFlags: map[string]map[string]string{
+					ingressnginx.Name: {
+						ingressnginx.NginxIngressClassFlag: ingressnginx.NginxIngressClass,
+					},
+				},
+				ingresses: []*networkingv1.Ingress{
+					basicIngress().
+						withName("main").
+						withHost(host).
+						withPath("/hostname").
+						withIngressClass(ingressnginx.NginxIngressClass).
+						withBackend(DummyAppName1).
+						build(),
+					basicIngress().
+						withName("canary-header").
+						withHost(host).
+						withPath("/hostname").
+						withIngressClass(ingressnginx.NginxIngressClass).
+						withAnnotation("nginx.ingress.kubernetes.io/canary", "true").
+						withAnnotation("nginx.ingress.kubernetes.io/canary-by-header", "X-Canary").
+						withBackend(DummyAppName2).
+						build(),
+				},
+				verifiers: map[string][]verifier{
+					"main": {
+						// With the canary header set to "always", all requests at the
+						// canary path should go to the canary backend.
+						&httpRequestVerifier{
+							host: host,
+							path: "/hostname",
+							requestHeaders: map[string]string{
+								"X-Canary": "always",
+							},
+							bodyRegex: regexp.MustCompile("^dummy-app2"),
+						},
+						// Without the header, requests should go to the main backend.
+						&httpRequestVerifier{
+							host:      host,
+							path:      "/hostname",
+							bodyRegex: regexp.MustCompile("^dummy-app1"),
+						},
+					},
+				},
+			})
+		})
+
+		t.Run("canary weight and header combined", func(t *testing.T) {
+			suffix, err := randString()
+			require.NoError(t, err)
+			host := fmt.Sprintf("canary-combined-%s.com", suffix)
+			runTestCase(t, &testCase{
+				gatewayImplementation: istio.ProviderName,
+				providers:             []string{ingressnginx.Name},
+				providerFlags: map[string]map[string]string{
+					ingressnginx.Name: {
+						ingressnginx.NginxIngressClassFlag: ingressnginx.NginxIngressClass,
+					},
+				},
+				ingresses: []*networkingv1.Ingress{
+					basicIngress().
+						withName("prod").
+						withHost(host).
+						withIngressClass(ingressnginx.NginxIngressClass).
+						withBackend(DummyAppName1).
+						build(),
+					basicIngress().
+						withName("canary-combined").
+						withHost(host).
+						withIngressClass(ingressnginx.NginxIngressClass).
+						withAnnotation("nginx.ingress.kubernetes.io/canary", "true").
+						withAnnotation("nginx.ingress.kubernetes.io/canary-weight", "20").
+						withAnnotation("nginx.ingress.kubernetes.io/canary-by-header", "X-Canary").
+						withBackend(DummyAppName2).
+						build(),
+				},
+				verifiers: map[string][]verifier{
+					"prod": {
+						// With the canary header set to "always", 100% of requests
+						// should go to the canary backend regardless of weight.
+						&httpRequestVerifier{
+							host: host,
+							path: "/hostname",
+							requestHeaders: map[string]string{
+								"X-Canary": "always",
+							},
+							bodyRegex: regexp.MustCompile("^dummy-app2"),
+						},
+						// With the canary header set to "never", 0% of requests
+						// should go to the canary backend regardless of weight.
+						&httpRequestVerifier{
+							host: host,
+							path: "/hostname",
+							requestHeaders: map[string]string{
+								"X-Canary": "never",
+							},
+							bodyRegex: regexp.MustCompile("^dummy-app1"),
+						},
+						// Without any header, the canary-weight (20%) applies.
+						&canaryVerifier{
+							verifier: &httpRequestVerifier{
+								host:      host,
+								path:      "/hostname",
+								bodyRegex: regexp.MustCompile("^dummy-app2"),
+							},
+							runs:         200,
+							minSuccesses: 0.1,
+							maxSuccesses: 0.3,
+						},
+					},
+				},
+			})
+		})
 	})
 }

pkg/i2gw/providers/ingressnginx/canary.go

@@ -94,6 +94,24 @@ func parseCanaryConfig(ingress *networkingv1.Ingress) (canaryConfig, error) {
 	return config, nil
 }
 
+func createHeaderMatchRule(header string, value string, existingMatches []gatewayv1.HTTPRouteMatch, backend gatewayv1.HTTPBackendRef) gatewayv1.HTTPRouteRule {
+	headerMatch := gatewayv1.HTTPRouteMatch{
+		Headers: []gatewayv1.HTTPHeaderMatch{
+			{
+				Name:  gatewayv1.HTTPHeaderName(header),
+				Value: value,
+			},
+		},
+	}
+	if len(existingMatches) > 0 && existingMatches[0].Path != nil {
+		headerMatch.Path = existingMatches[0].Path
+	}
+	return gatewayv1.HTTPRouteRule{
+		Matches:     []gatewayv1.HTTPRouteMatch{headerMatch},
+		BackendRefs: []gatewayv1.HTTPBackendRef{backend},
+	}
+}
+
 func canaryFeature(ingresses []networkingv1.Ingress, _ map[types.NamespacedName]map[string]int32, ir *providerir.ProviderIR) field.ErrorList {
 	ruleGroups := common.GetRuleGroups(ingresses)
 	var errList field.ErrorList
@@ -107,7 +125,7 @@ func canaryFeature(ingresses []networkingv1.Ingress, _ map[types.NamespacedName]
 
 			for ruleIdx := 0; ruleIdx < len(httpRouteContext.HTTPRoute.Spec.Rules); ruleIdx++ {
 				backendSources := httpRouteContext.RuleBackendSources[ruleIdx]
-
+				existingMatches := httpRouteContext.HTTPRoute.Spec.Rules[ruleIdx].Matches
 				canaryWeight, nonCanaryWeight, config, canarySourceIngress, canaryBackendIdx, nonCanaryBackendIdx, parseErrs := getCanaryInfo(backendSources, "httproute", httpRouteContext.HTTPRoute.Name, ruleIdx)
 				errList = append(errList, parseErrs...)
 				if canaryBackendIdx != -1 && nonCanaryBackendIdx != -1 {
@@ -125,45 +143,25 @@ func canaryFeature(ingresses []networkingv1.Ingress, _ map[types.NamespacedName]
 						canaryBackendSource := backendSources[canaryBackendIdx]
 						nonCanaryBackendSource := backendSources[nonCanaryBackendIdx]
 
-						var header = "always"
+						var canaryHeaderValue = "always"
 						if config.headerValue != "" {
-							header = config.headerValue
+							canaryHeaderValue = config.headerValue
 						}
 						canaryBackendCopy := canaryBackend
 						canaryBackendCopy.Weight = nil
-						newRule := gatewayv1.HTTPRouteRule{
-							Matches: []gatewayv1.HTTPRouteMatch{
-								{
-									Headers: []gatewayv1.HTTPHeaderMatch{
-										{
-											Name:  gatewayv1.HTTPHeaderName(config.header),
-											Value: header,
-										},
-									},
-								},
-							},
-							BackendRefs: []gatewayv1.HTTPBackendRef{canaryBackendCopy},
-						}
-						rulesToAdd = append(rulesToAdd, newRule)
+
+						var canaryMatchRule = createHeaderMatchRule(config.header, canaryHeaderValue, existingMatches, canaryBackendCopy)
+						rulesToAdd = append(rulesToAdd, canaryMatchRule)
 						sourcesToAdd = append(sourcesToAdd, []providerir.BackendSource{canaryBackendSource, nonCanaryBackendSource})
 
+						notify(notifications.InfoNotification, fmt.Sprintf("parsed canary annotations of ingress %s/%s and set header \"%s\" with value \"%s\" for canary backend",
+							canarySourceIngress.Namespace, canarySourceIngress.Name, config.header, canaryHeaderValue), &httpRouteContext.HTTPRoute)
+
 						if config.headerValue == "" {
 							nonCanaryBackendCopy := nonCanaryBackend
 							nonCanaryBackendCopy.Weight = nil
-							neverRule := gatewayv1.HTTPRouteRule{
-								Matches: []gatewayv1.HTTPRouteMatch{
-									{
-										Headers: []gatewayv1.HTTPHeaderMatch{
-											{
-												Name:  gatewayv1.HTTPHeaderName(config.header),
-												Value: "never",
-											},
-										},
-									},
-								},
-								BackendRefs: []gatewayv1.HTTPBackendRef{nonCanaryBackendCopy},
-							}
-							rulesToAdd = append(rulesToAdd, neverRule)
+							var nonCanaryMatchRule = createHeaderMatchRule(config.header, "never", existingMatches, nonCanaryBackendCopy)
+							rulesToAdd = append(rulesToAdd, nonCanaryMatchRule)
 							sourcesToAdd = append(sourcesToAdd, []providerir.BackendSource{nonCanaryBackendSource})
 
 							notify(notifications.InfoNotification, fmt.Sprintf("parsed canary annotations of ingress %s/%s and set header \"%s\" with value \"never\" for non-canary backend",
@@ -183,8 +181,6 @@ func canaryFeature(ingresses []networkingv1.Ingress, _ map[types.NamespacedName]
 							httpRouteContext.HTTPRoute.Spec.Rules[ruleIdx].BackendRefs = filteredBackendRefs
 							httpRouteContext.RuleBackendSources[ruleIdx] = filteredBackendSources
 						}
-						notify(notifications.InfoNotification, fmt.Sprintf("parsed canary annotations of ingress %s/%s and set header \"%s\" with value \"%s\"",
-							canarySourceIngress.Namespace, canarySourceIngress.Name, config.header, config.headerValue), &httpRouteContext.HTTPRoute)
 					}
 				}
 			}