clarify behavior of multiple query param matches for the same key

[skriss]

The spec does not indicate what should be done if a route rule match has multiple query param matches for the same key.

For header matches, guidance is given to use the first match for a given header name, and to ignore subsequent ones for the same name (see https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1alpha2.HTTPHeaderMatch).

I'd suggest adding the same guidance for query params, with the exception that query param names be case-sensitive. If this makes sense, I'll open a PR to add this to the spec. (I should also be able to add a conformance test for query params that includes this soon).

[shaneutt]

So basically today we say that []HTTPHeaderMatch and []HTTPQueryParamMatch are logically treated as AND operated lists, but with the caveat that multiple HTTPHeaderMatch entries with the same name have the ability to "overload" themselves to enable an OR on their values. Your suggestion that we should also prescribe the same for params makes sense to me, but I am left thinking that overall this may become a bit confusing for both implementors AND users 🤔

Perhaps we would be better served by rethinking how we enable the expression of the values OR? Perhaps it would be better to have an explicit values []string on the HTTPHeaderMatch type as a clear and easier to document way to express the OR need? I'm not sure I'm just thinking out loud.

Please let me know your thoughts?

[skriss]

@shaneutt thanks for the comments!

with the caveat that multiple HTTPHeaderMatch entries with the same name have the ability to "overload" themselves to enable an OR on their values

This is not my understanding -- the spec says "If multiple entries specify equivalent header names, only the first entry with an equivalent name MUST be considered for a match. Subsequent entries with an equivalent header name MUST be ignored. Due to the case-insensitivity of header names, “foo” and “Foo” are considered equivalent." (see name field doc under https://gateway-api.sigs.k8s.io/v1alpha2/references/spec/#gateway.networking.k8s.io/v1alpha2.HTTPHeaderMatch). Am I missing something?

[shaneutt]

You know now that you mention it, I think I've been historically confused about this bit. After doing some re-reading I think what we're suggesting is that when there are multiples you "must only consider the first entry with name X, and all others are ignored". As written that seems to mean that duplicates are allowed, but entirely pointless, but we still expect downstream to handle that in their implementation. It's making me wonder if we should put that burden on the API instead and simply add validation that disallows duplicates?

[skriss]

Yeah, I think it makes sense to add logic to the webhook for this since as you say, duplicates are pointless, though as an implementor I'd still want clear guidance on what to do if duplicates do make it to my implementation. I'm fine with the "use the first, ignore the rest" guidance for now. It sounds like you are in agreement that whatever we do for header matches, probably makes sense to do for query param matches too?

[shaneutt]

Yeah, I'm with you on that 👍

[youngnick]

I agree that we should both have the webhook validation try to stop duplicate matches for either header or query params, and that we should have the behavior in case something makes it through that all but the first must be ignored.

[robscott]

+1 on consensus here, added to v0.5.0 milestone, appreciate if anyone has time to pick this one up in next day or two.

[skriss]

I should be able to knock this out.

[shaneutt]

/assign @skriss