kubernetes-ingress icon indicating copy to clipboard operation
kubernetes-ingress copied to clipboard

Allow default gRPC/proxy headers to be overwritten in VirtualServer

Open centromere opened this issue 2 years ago • 2 comments

Proposed changes

When using the VirtualServer CRD, it is not possible to override the default values of the following headers:

  • X-Real-IP
  • X-Forwarded-For
  • X-Forwarded-Host
  • X-Forwarded-Port
  • X-Forwarded-Proto

For example:

  - action:
      proxy:
        requestHeaders:
          set:
            - name: X-Forwarded-For
              value: ${http_cf_connecting_ip}
            - name: X-Forwarded-Proto
              value: ${scheme}-foo

Will result in the following:

        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header X-Forwarded-Host $host;
        proxy_set_header X-Forwarded-Port $server_port;
        proxy_set_header X-Forwarded-Proto $scheme;

        proxy_set_header X-Forwarded-For "${http_cf_connecting_ip}";

        proxy_set_header X-Forwarded-Proto "${scheme}-foo";

This causes the headers to be sent upstream twice: once with the default value and once with the overwritten value. This change prevents this behavior from happening.

Checklist

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

  • [x] I have read the CONTRIBUTING doc
  • [ ] 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

centromere avatar Jun 03 '22 21:06 centromere

Codecov Report

Merging #2735 (d593c2a) into main (f70cf54) will increase coverage by 0.04%. The diff coverage is 72.72%.

@@            Coverage Diff             @@
##             main    #2735      +/-   ##
==========================================
+ Coverage   52.52%   52.57%   +0.04%     
==========================================
  Files          58       59       +1     
  Lines       16070    16080      +10     
==========================================
+ Hits         8441     8454      +13     
+ Misses       7351     7349       -2     
+ Partials      278      277       -1     
Impacted Files Coverage Δ
internal/configs/version2/template_helper.go 70.00% <70.00%> (ø)
internal/configs/version2/template_executor.go 60.46% <100.00%> (ø)
internal/k8s/configuration.go 95.76% <0.00%> (+0.36%) :arrow_up:
...ternal/k8s/appprotect/app_protect_configuration.go 86.74% <0.00%> (+0.57%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

codecov-commenter avatar Jun 30 '22 03:06 codecov-commenter

Is someone available to review this PR?

centromere avatar Aug 08 '22 16:08 centromere

@centromere Many thanks for your PR on this one. Approved and merged.

tomasohaodha avatar Nov 07 '22 14:11 tomasohaodha