Allow for multiple values to be passed to ALB traffic shaping header match condition

[schlags]

Summary

How to reproduce

Today, it doesn't seem to be possible to allow for multiple values to be passed for a given header in setHeaderRoute matches.

The below manifest, for example, would create two matches that would be impossible to route. (origin header with exact match of one value AND origin header with exact match of another value).

        - setHeaderRoute: # Sets requests from our internal slugs to go to the canary service
            name: "example-header-route-1"
            match:
              - headerName: origin
                headerValue:
                  exact: https://testing-one.example.com
              - headerName: origin
                headerValue:
                  exact: https://testing-two.example.com

This results in the definition of the ingress annotation being the following:

    alb.ingress.kubernetes.io/conditions.example-header-route-1: >
      [{"field":"http-header","httpHeaderConfig":{"httpHeaderName": "origin", "values":["https://testing-one.example.com"]}},{"field":"http-header","httpHeaderConfig":{"httpHeaderName": "origin", "values":["https://testing-two.example.com"]}}]

Desired end state

Instead, we'd prefer argo-rollouts to capture that multiple exact matches are being accepted for a given header name, and should hope to see the values array in use to specify this like so:

    alb.ingress.kubernetes.io/conditions.testinator-canary: >
      [{"field":"http-header","httpHeaderConfig":{"httpHeaderName": "origin", "values":["https://testing-one.example.com", "https://testing-two.example.com"]}}]

Proposed solution

The ALB LBC defines the annotation as accepting an array of values, but argo-rollouts doesn't support it since the following is written:

		condition := ingressutil.ALBCondition{
			Field: "http-header",
			HttpHeaderConfig: ingressutil.HttpHeaderConfig{
				HttpHeaderName: match.HeaderName,
				Values:         []string{match.HeaderValue.Exact},
			},
		}

If we instead define an upsert function that checks to see if that headerName is already present in the conditions of res before appending a new condition, we could handle this. I cannot think of any use cases where it would be intentional to define separate conditions with the same HttpHeaderName - since we only support Exact matches in the traffic routing for ALB.

// 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 {
		res = upsertCondition(res, match)
	}
	bytes := jsonutil.MustMarshal(res)
	return string(bytes), nil
}

Use Cases

If we wanted to define multiple values for a request header to route to the canary service during a rollout, we would be able to ensure that any matches in setHeaderRoute for that headerName would be reconciled correctly in the resulting ingress annotation.

---

Message from the maintainers:

Impacted by this bug? Give it a 👍. We prioritize the issues with the most 👍.