diff --git a/rollout/trafficrouting/alb/alb.go b/rollout/trafficrouting/alb/alb.go index b1b9ff0f25..d91f211655 100644 --- a/rollout/trafficrouting/alb/alb.go +++ b/rollout/trafficrouting/alb/alb.go @@ -515,17 +515,28 @@ func getTrafficForwardActionString(r *v1alpha1.Rollout, port int32) (string, err return string(bytes), nil } +// Two exact matches with the same header name should be merged into the values array of the same condition +func upsertCondition(res []ingressutil.ALBCondition, match v1alpha1.HeaderRoutingMatch) []ingressutil.ALBCondition { + for i, condition := range res { + if condition.HttpHeaderConfig.HttpHeaderName == match.HeaderName { + res[i].HttpHeaderConfig.Values = append(res[i].HttpHeaderConfig.Values, match.HeaderValue.Exact) + return res + } + } + condition := ingressutil.ALBCondition{ + Field: "http-header", + HttpHeaderConfig: ingressutil.HttpHeaderConfig{ + HttpHeaderName: match.HeaderName, + Values: []string{match.HeaderValue.Exact}, + }, + } + return append(res, condition) +} + func getTrafficForwardConditionString(headerRoute *v1alpha1.SetHeaderRoute) (string, error) { var res []ingressutil.ALBCondition for _, match := range headerRoute.Match { - condition := ingressutil.ALBCondition{ - Field: "http-header", - HttpHeaderConfig: ingressutil.HttpHeaderConfig{ - HttpHeaderName: match.HeaderName, - Values: []string{match.HeaderValue.Exact}, - }, - } - res = append(res, condition) + res = upsertCondition(res, match) } bytes := jsonutil.MustMarshal(res) return string(bytes), nil diff --git a/rollout/trafficrouting/alb/alb_test.go b/rollout/trafficrouting/alb/alb_test.go index e81b456941..52fd5a6ec7 100644 --- a/rollout/trafficrouting/alb/alb_test.go +++ b/rollout/trafficrouting/alb/alb_test.go @@ -1866,6 +1866,108 @@ func TestSetHeaderRoute(t *testing.T) { assert.Len(t, client.Actions(), 1) } +func TestSetHeaderRouteWithDifferentHeaderNames(t *testing.T) { + ro := fakeRollout(STABLE_SVC, CANARY_SVC, nil, "ingress", 443) + ro.Spec.Strategy.Canary.TrafficRouting.ManagedRoutes = []v1alpha1.MangedRoutes{ + {Name: "header-route"}, + } + + i := ingress("ingress", STABLE_SVC, CANARY_SVC, "action1", 443, 10, ro.Name, false) + client := fake.NewSimpleClientset(i) + k8sI := kubeinformers.NewSharedInformerFactory(client, 0) + k8sI.Extensions().V1beta1().Ingresses().Informer().GetIndexer().Add(i) + + ingressWrapper, err := ingressutil.NewIngressWrapper(ingressutil.IngressModeExtensions, client, k8sI) + if err != nil { + t.Fatal(err) + } + + r, err := NewReconciler(ReconcilerConfig{ + Rollout: ro, + Client: client, + Recorder: record.NewFakeEventRecorder(), + ControllerKind: schema.GroupVersionKind{Group: "foo", Version: "v1", Kind: "Bar"}, + IngressWrapper: ingressWrapper, + }) + assert.NoError(t, err) + + err = r.SetHeaderRoute(&v1alpha1.SetHeaderRoute{ + Name: "header-route", + Match: []v1alpha1.HeaderRoutingMatch{ + { + HeaderName: "origin", + HeaderValue: &v1alpha1.StringMatch{ + Exact: "https://www.fake-origin1.com", + }, + }, + { + HeaderName: "Agent", + HeaderValue: &v1alpha1.StringMatch{ + Exact: "Chrome", + }, + }, + }, + }) + assert.Nil(t, err) + assert.Len(t, client.Actions(), 1) + + // no managed routes, no changes expected + err = r.RemoveManagedRoutes() + assert.Nil(t, err) + assert.Len(t, client.Actions(), 1) +} + +func TestSetHeaderRouteWithDuplicateHeaderNameMatches(t *testing.T) { + ro := fakeRollout(STABLE_SVC, CANARY_SVC, nil, "ingress", 443) + ro.Spec.Strategy.Canary.TrafficRouting.ManagedRoutes = []v1alpha1.MangedRoutes{ + {Name: "header-route"}, + } + + i := ingress("ingress", STABLE_SVC, CANARY_SVC, "action1", 443, 10, ro.Name, false) + client := fake.NewSimpleClientset(i) + k8sI := kubeinformers.NewSharedInformerFactory(client, 0) + k8sI.Extensions().V1beta1().Ingresses().Informer().GetIndexer().Add(i) + + ingressWrapper, err := ingressutil.NewIngressWrapper(ingressutil.IngressModeExtensions, client, k8sI) + if err != nil { + t.Fatal(err) + } + + r, err := NewReconciler(ReconcilerConfig{ + Rollout: ro, + Client: client, + Recorder: record.NewFakeEventRecorder(), + ControllerKind: schema.GroupVersionKind{Group: "foo", Version: "v1", Kind: "Bar"}, + IngressWrapper: ingressWrapper, + }) + assert.NoError(t, err) + + err = r.SetHeaderRoute(&v1alpha1.SetHeaderRoute{ + Name: "header-route", + Match: []v1alpha1.HeaderRoutingMatch{ + { + HeaderName: "origin", + HeaderValue: &v1alpha1.StringMatch{ + Exact: "https://www.fake-origin1.com", + }, + }, + { + HeaderName: "origin", + HeaderValue: &v1alpha1.StringMatch{ + Exact: "https://www.fake-origin2.com", + }, + }, + }, + }) + assert.Nil(t, err) + assert.Len(t, client.Actions(), 1) + + // no managed routes, no changes expected + err = r.RemoveManagedRoutes() + assert.Nil(t, err) + assert.Len(t, client.Actions(), 1) +} + func TestSetHeaderRouteMultiIngress(t *testing.T) { ro := fakeRolloutWithMultiIngress(STABLE_SVC, CANARY_SVC, nil, []string{"ingress", "multi-ingress"}, 443) ro.Spec.Strategy.Canary.TrafficRouting.ManagedRoutes = []v1alpha1.MangedRoutes{