fix: handle multi-value response headers as comma-separated per RFC 7230

[patrostkowski]

What type of PR is this?

fix

What this PR does / why we need it:

This change updates the translation logic of ResponseHeaderModifier so that when multiple values are specified for a header, they are joined into a single, comma-separated value as required by RFC 7230. As a result, the generated Envoy config now emits one HeaderValueOption per header, with all values joined by commas.

Tested using following config:

apiVersion: gateway.networking.k8s.io/v1
kind: HTTPRoute
metadata:
  name: backend
  namespace: default
spec:
  hostnames:
  - www.example.com
  parentRefs:
  - group: gateway.networking.k8s.io
    kind: Gateway
    name: eg
  rules:
  - backendRefs:
    - group: ""
      kind: Service
      name: backend
      port: 3000
      weight: 1
    filters:
    - responseHeaderModifier:
        set:
        - name: Cache-Control
          value: private, no-store
      type: ResponseHeaderModifier
    matches:
    - path:
        type: PathPrefix
        value: /

# curl latest image
root@ubuntu:/$ curl -i http://10.96.20.98/ -H 'Host: www.example.com' -H 'X-Echo-Set-Header: Cache-control: value1' | grep ^cache-control
cache-control:  no-store

# curl e4c8f89 image
root@ubuntu:/$ curl -i http://10.96.20.98/ -H 'Host: www.example.com' -H 'X-Echo-Set-Header: Cache-control: value1' | grep ^cache-control
cache-control: private, no-store

Which issue(s) this PR fixes:

Fixes #5733

Release Notes: Yes/No

[arkodg]

this will break the logic for requestHeaders

for responseHeaders, is the fix to avoid the split in the gateway-api layer

gateway/internal/gatewayapi/filters.go

Line 620 in 32d61c4

Value: strings.Split(addHeader.Value, ","),

?

please add a test case in the gateway-api and xds/translator layer

[arkodg]

hey @patrostkowski we use YAML based testcases for better readability

• e.g. for gateway-api lib - internal/gatewayapi/testdata/grpcroute-with-request-header-modifier.in.yaml
• for xds translator - internal/xds/translator/testdata/in/xds-ir/http-route-request-headers.yaml

if you run make testdata it will even generate the out files for you

[patrostkowski]

@arkodg sure, let me adjust the tests then

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.66%. Comparing base (e0e8a29) to head (2b8a179).
⚠️ Report is 589 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6281      +/-   ##
==========================================
+ Coverage   70.60%   70.66%   +0.06%     
==========================================
  Files         220      220              
  Lines       36829    36832       +3     
==========================================
+ Hits        26004    26029      +25     
+ Misses       9292     9275      -17     
+ Partials     1533     1528       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions!

[github-actions]

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions!

[arkodg]

@patrostkowski still working on this ?

[jasmin-terrien]

@patrostkowski any news/time to finish your fix ?

[VonNao]

This still seems to be worked on right?

[arkodg]

@jasmin-terrien @VonNao feel free to pick this one up, there's been inactivity for a long time

[arkodg]

fixed with #7436