Incorrect validation or description for HTTPHeaderFilter in HTTPRoute

[snorwin]

What happened:
The current documentation for the HTTPHeaderFilter type in the HTTPRoute Custom Resource (CR) specifies the validation rules as follows:

Only one action for a given header name is permitted. Filters specifying multiple actions of the same or different type for any one header name are invalid and will be rejected by CRD validation.

However, this description does not accurately reflect the actual validation behavior:

• Filters specifying multiple actions of the same or different type for any one header name are invalid:

  requestHeaderModifier:
    add:
      - name: my-header-name
        value: my-header-value
      - name: my-header-name
        value: my-header-value
  

  Error: The HTTPRoute "backend" is invalid: spec.rules[0].filters[0].requestHeaderModifier.add[1]: Duplicate value: map[string]interface {}{"name":"my-header-name"}

• Filters specifying multiple actions of the same or different type for any one header name are invalid:

  requestHeaderModifier:
    add:
      - name: my-header-name
        value: my-header-value
    set:
      - name: my-header-name
        value: my-header-value
  

  No Error: httproute.gateway.networking.k8s.io/backend configured

What you expected to happen:
The documentation should accurately describe the validation rules. If multiple actions of different types are indeed permitted for the same header, this should be clearly stated in the documentation. Otherwise, if the current behavior is unintended, the validation logic should be updated to match the documented behavior.

How to reproduce it (as minimally and precisely as possible):
Define an HTTPRoute with the requestHeaderModifier mentioned above.

Anything else we need to know?:
N/A

[snorwin]

This issue could potential be combined with: #2277

[robscott]

@snorwin thanks for the detailed writeup! This is actually more nuanced than it seems. In some cases we have stated in the spec that something is invalid despite not having corresponding CEL validation, this may be one of those cases. Essentially CEL came into existence after much of the spec was written, so in some cases we had to rely on controllers to do this kind of validation instead of CEL. While this does seem like something we could tighten with CEL validation, now that the API is already GA, a chance like that would be not be backwards compatible.

As far as this specific case, it seems like it could at least theoretically be desirably to remove and then add a header if some implementations don't support set, but otherwise I'm struggling to find a use case for allowing different operations on the same header name.

/cc @youngnick @mlavacca