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

Add support for host_rewrite_header #9608

Merged
merged 21 commits into from
Jun 14, 2024
Merged

Conversation

davidjumani
Copy link
Contributor

@davidjumani davidjumani commented Jun 12, 2024

Description

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

  • N/A

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 davidjumani requested a review from a team as a code owner June 12, 2024 00:35
@solo-changelog-bot
Copy link

Issues linked to changelog:
solo-io#9579

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

github-actions bot commented Jun 12, 2024

Visit the preview URL for this PR (updated for commit 7f5008b):

https://gloo-edge--pr9608-add-host-rewrite-hea-bpwy34fy.web.app

(expires Fri, 21 Jun 2024 15:32:09 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 77c2b86e287749579b7ff9cadb81e099042ef677

@davidjumani
Copy link
Contributor Author

/kick

@davidjumani davidjumani reopened this Jun 13, 2024
nfuden
nfuden previously approved these changes Jun 13, 2024
Copy link
Contributor

@nfuden nfuden left a comment

Choose a reason for hiding this comment

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

While I would like this to be more robust in the header checking area eventually and would really love a follow up issue but this works for me for now

Edit: i am blind: this does link nicely to the validator in envoy

nfuden
nfuden previously approved these changes Jun 13, 2024
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.

A couple notes/questions

changelog/v1.17.0-rc4/add-host-rewrite-header.yaml Outdated Show resolved Hide resolved
pkg/utils/api_conversion/type.go Show resolved Hide resolved
davidjumani and others added 2 commits June 14, 2024 10:44
Co-authored-by: Nathan Fudenberg <nathan.fudenberg@solo.io>
@solo-changelog-bot
Copy link

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

@davidjumani
Copy link
Contributor Author

kick bulldozer

@soloio-bulldozer soloio-bulldozer bot merged commit a6d9362 into main Jun 14, 2024
24 checks passed
@soloio-bulldozer soloio-bulldozer bot deleted the add-host-rewrite-header branch June 14, 2024 16:45
davidjumani added a commit that referenced this pull request Jun 14, 2024
* 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 added a commit that referenced this pull request Jun 14, 2024
* 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 added a commit that referenced this pull request Jun 14, 2024
* Add support for host_rewrite_header (#9608)

* 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>

* move changelog

* codegen

* update changelog

* change changelog type

* revert bumps

---------

Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: Nathan Fudenberg <nathan.fudenberg@solo.io>
soloio-bulldozer bot added a commit that referenced this pull request Jun 17, 2024
* Add support for host_rewrite_header (#9608)

* 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>

* update changelog

* disable ingress tests

* change changelog type

* Trigger CI

---------

Co-authored-by: soloio-bulldozer[bot] <48420018+soloio-bulldozer[bot]@users.noreply.github.com>
Co-authored-by: Nathan Fudenberg <nathan.fudenberg@solo.io>
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.

Headers to add keys are not validated
3 participants