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

[1.17] Add support for host_rewrite_header #9623

Merged
merged 5 commits into from
Jun 17, 2024

Conversation

davidjumani
Copy link
Contributor

@davidjumani davidjumani commented Jun 14, 2024

Description

Backport of solo-io#9608

Adds support for the host_rewrite_header route option.
This configures envoy to rewrite the host header with the value of another header based on the value provided
Ref: https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/route/v3/route_components.proto#envoy-v3-api-field-config-route-v3-routeaction-host-rewrite-header

This also fixes a bug where gloo did not reject invalid envoy header keys
vs.yaml

        headerManipulation:
          requestHeadersToAdd:
          - header:
              key: "head(er)"
              value: random

Previously

$ kg apply -f vs.yaml 
virtualservice.gateway.solo.io/default created

$ curl localhost:8080/get
invalid header name: head(er)

Now

$ kg apply -f vs.yaml 

	* Validating *v1.VirtualService failed: validating *v1.VirtualService name:"default" namespace:"gloo-system": 1 error occurred:
	* failed gloo validation resource reports: 2 errors occurred:
	* invalid resource gloo-system.gateway-proxy
	* Route Error: ProcessingError. Reason: *headers.plugin: 'head(er)' is an invalid HTTP header key. Route Name: gloo-system_default-route-0-matcher-0; Route Error: ProcessingError. Reason: *headers.plugin: 'head(er)' is an invalid HTTP header key. Route Name: gloo-system_default-route-0-matcher-0

API changes

  • Adds the host_rewrite_header to the route options

Code changes

  • Adds support for the host_rewrite_header option in the basic route plugin

CI changes

  • Disables ingress tests on the v1.17.x branch

Docs changes

  • N/A

Context

solo-io#9579

Interesting decisions

N/A

Testing steps

Unit tests

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works

BOT NOTES:
resolves solo-io#9622

davidjumani and others added 2 commits June 14, 2024 12:46
* add it to the proto

* add plugin

* add changelog

* Adding changelog file to new location

* Deleting changelog file from old location

* add validation

* remove old entry

* Adding changelog file to new location

* Deleting changelog file from old location

* dont remove exposed method

* update changelog

* update changelog

* Update changelog/v1.18.0-beta1/add-host-rewrite-header.yaml

Co-authored-by: Nathan Fudenberg <nathan.fudenberg@solo.io>

* update changelog

* update changelog

* address comments

---------

Co-authored-by: changelog-bot <changelog-bot>
Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: Nathan Fudenberg <nathan.fudenberg@solo.io>
@davidjumani davidjumani requested a review from a team as a code owner June 14, 2024 16:50
@solo-changelog-bot
Copy link

Issues linked to changelog:
solo-io#9579
solo-io#9622

@github-actions github-actions bot added the keep pr updated signals bulldozer to keep pr up to date with base branch label Jun 14, 2024
@davidjumani
Copy link
Contributor Author

/kick buildbot stuck

Copy link
Contributor

@sheidkamp sheidkamp left a comment

Choose a reason for hiding this comment

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

Question about the regression test change

.github/workflows/regression-tests.yaml Show resolved Hide resolved
@davidjumani
Copy link
Contributor Author

kick bulldozer

@soloio-bulldozer soloio-bulldozer bot merged commit 33f6334 into v1.17.x Jun 17, 2024
22 checks passed
@soloio-bulldozer soloio-bulldozer bot deleted the add-host-rewrite-header-117 branch June 17, 2024 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep pr updated signals bulldozer to keep pr up to date with base branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants