Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reverse-proxy: T6370: Set custom HTTP headers in reverse-proxy responses #3487

Merged
merged 1 commit into from
May 23, 2024

Conversation

Embezzle
Copy link
Contributor

Change Summary

Add the option to set custom HTTP headers in reverse-proxy responses.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

https://vyos.dev/T6370

Related PR(s)

Component(s) name

load-balancing -> reverse-proxy

Proposed changes

How to test

  1. Create a reverse-proxy configuration using the new options:
set load-balancing reverse-proxy backend bk-01 mode 'http'
set load-balancing reverse-proxy backend bk-01 server srv-01 address '192.0.2.12'
set load-balancing reverse-proxy backend bk-01 server srv-01 port '80'
set load-balancing reverse-proxy backend bk-01 http-response-headers Proxy-Backend-ID value bk-01

set load-balancing reverse-proxy service fe-https backend 'bk-01'
set load-balancing reverse-proxy service fe-https listen-address '192.0.2.11'
set load-balancing reverse-proxy service fe-https mode 'http'
set load-balancing reverse-proxy service fe-https port '443'
set load-balancing reverse-proxy service fe-https ssl certificate cert
set load-balancing reverse-proxy service fe-https http-response-headers Strict-Transport-Security value 'max-age=31536000; includeSubDomains;'
  1. Check the HAProxy backend server configuration is showing the correct options:
vyos@vyos:~$ cat /var/run/haproxy/haproxy.cfg | grep 'Proxy-Backend-ID\|Strict-Transport-Security'
	http-response set-header Strict-Transport-Security 'max-age=31536000; includeSubDomains;'
	http-response set-header Proxy-Backend-ID 'bk-01'

Smoketest result

vyos@vyos:~$ /usr/libexec/vyos/tests/smoke/cli/test_load-balancing_reverse-proxy.py
test_01_lb_reverse_proxy_domain (__main__.TestLoadBalancingReverseProxy.test_01_lb_reverse_proxy_domain) ... ok
test_02_lb_reverse_proxy_cert_not_exists (__main__.TestLoadBalancingReverseProxy.test_02_lb_reverse_proxy_cert_not_exists) ...
PKI does not contain any certificates!


Certificate "cert" not found in configuration!

ok
test_03_lb_reverse_proxy_ca_not_exists (__main__.TestLoadBalancingReverseProxy.test_03_lb_reverse_proxy_ca_not_exists) ... ok
test_04_lb_reverse_proxy_backend_ssl_no_verify (__main__.TestLoadBalancingReverseProxy.test_04_lb_reverse_proxy_backend_ssl_no_verify) ...
backend bk-01 cannot have both ssl options no-verify and ca-certificate
set!

ok
test_05_lb_reverse_proxy_backend_http_check (__main__.TestLoadBalancingReverseProxy.test_05_lb_reverse_proxy_backend_http_check) ... ok
test_06_lb_reverse_proxy_tcp_mode (__main__.TestLoadBalancingReverseProxy.test_06_lb_reverse_proxy_tcp_mode) ... ok
test_07_lb_reverse_proxy_http_response_headers (__main__.TestLoadBalancingReverseProxy.test_07_lb_reverse_proxy_http_response_headers) ... ok

----------------------------------------------------------------------
Ran 7 tests in 29.581s

OK

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • I have run the components SMOKETESTS if applicable
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

@c-po c-po requested review from a team, dmbaturin, sarthurdev, zdc, jestabro, sever-sever and c-po and removed request for a team May 20, 2024 05:18
@jack9603301
Copy link
Contributor

jack9603301 commented May 20, 2024 via email

@c-po
Copy link
Member

c-po commented May 20, 2024

I remember that I once had an LVS load balancing solution that had not yet been merged. What’s going on now?

IIRC PR stalled

Copy link
Member

@sever-sever sever-sever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t see another best solution to integrate them as headers could be completely different

@c-po
Copy link
Member

c-po commented May 22, 2024

I wonder if we can not have a precompiled list of allowed response headers. I guess they are more common and less exotic?

Copy link
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Custom headers are acceptable in responses (say, X-Clacks-Overhead) and do no harm even if clients don't use them in any way, so I see no reason to limit it to a pre-determined list.

@c-po c-po merged commit 3e69d8b into vyos:current May 23, 2024
8 checks passed
@c-po
Copy link
Member

c-po commented May 23, 2024

@Mergifyio backport sagitta

Copy link
Contributor

mergify bot commented May 23, 2024

backport sagitta

✅ Backports have been created

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants