nginx-gateway-fabric Add support for ResponseHeaderModifier for HTTPRouteRule objects

trafficstars

Proposed changes

Problem: Users want to add, set, and remove response headers.

Solution: Use add_header NGINX directive to support ResponseHeaderModifier.

If the action is set, we simply configure the add_header directive with the given value in the HTTPRoute spec.
If the action is remove, we configure the add_header directive with the value set to an empty string.
If the action is add, we create a mapping for the $http_header_name variable appending a comma at the end of any client provided headers (if present) and prepend this to the given value in the HTTPRoute spec.

Testing: Describe any testing that you did.

Please focus on (optional): If you any specific areas where you would like reviewers to focus their attention or provide specific feedback, add them here.

Closes #1397

Checklist

Before creating a PR, run through this checklist and mark each as complete.

[x] I have read the CONTRIBUTING doc
[x] I have added tests that prove my fix is effective or that my feature works
[x] I have checked that all unit tests pass after adding my changes
[x] I have updated necessary documentation
[x] I have rebased my branch onto main
[x] I will ensure my PR is targeting the main branch and pulling from my branch from my own fork

Release notes

If this PR introduces a change that affects users and needs to be mentioned in the release notes, please add a brief note that summarizes the change.

Apr 25 '24 15:04 salonichf5

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 86.83%. Comparing base (0098b07) to head (987a368).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1880      +/-   ##
==========================================
+ Coverage   86.71%   86.83%   +0.11%     
==========================================
  Files          88       88              
  Lines        5971     6025      +54     
  Branches       50       50              
==========================================
+ Hits         5178     5232      +54     
  Misses        741      741              
  Partials       52       52

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Apr 25 '24 15:04 codecov[bot]

Kevin's PR - https://github.com/nginxinc/nginx-gateway-fabric/pull/1494

Apr 25 '24 19:04 salonichf5

Shouldn't we be adding the responseHeaderModifier validation here ? @kate-osborn

Apr 30 '24 03:04 salonichf5

Shouldn't we be adding the responseHeaderModifier validation here ? @kate-osborn

@salonichf5 it looks like the intent was to make the HTTPRequestHeaderValidator more generic so it could validate both request and response headers, and then add the response header-specific validation separately in the validateFilterResponseHeaderModifier function in httproute.go.

I like your suggestion of adding a new struct to the HTTPValidator that validates response headers. I would move the functionality of validateFilterResponseHeaderModifier to that new validator.

Edit: @salonichf5 after looking through the code again, I think there will be less code duplication if we keep the approach the same. I would just rename HTTPRequestHeaderValidator to HTTPHeaderValidator and keep everything else the same.