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 X-Forwarded-Prefix on rewrites #1805

Merged
merged 2 commits into from
Dec 9, 2017

Conversation

maxlaverse
Copy link
Contributor

@maxlaverse maxlaverse commented Dec 6, 2017

What this PR does / why we need it:
Add an annotation x-forwarded-prefix that activate the addition of a X-Forwarded-Prefix to the request sent to the upstream. This prefix has the value of the Nginx location that matched the rewrite.

Which issue this PR fixes
fixes #1774

@k8s-reviewable
Copy link

This change is Reviewable

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 6, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 36.721% when pulling ce99a5e on maxlaverse:add_x_forwarded_prefix into 6816630 on kubernetes:master.

@aledbf
Copy link
Member

aledbf commented Dec 9, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Dec 9, 2017
@aledbf
Copy link
Member

aledbf commented Dec 9, 2017

@maxlaverse thanks!

@aledbf aledbf merged commit d14d220 into kubernetes:master Dec 9, 2017
@maxlaverse maxlaverse deleted the add_x_forwarded_prefix branch December 9, 2017 18:22
@metaflow
Copy link

HI @maxlaverse ! I am trying to figure how to set this annotation on. Should it look like:

apiVersion: extensions/v1beta1                           
kind: Ingress                                            
metadata:                                                
  annotations:                                           
    ingress.kubernetes.io/x-forwarded-prefix: "true"
    kubernetes.io/ingress.class: "nginx"

? Or there is a different syntax?

@pieterlange
Copy link
Contributor

pieterlange commented Feb 20, 2018

@metaflow yes, it's a boolean but you've got the annotation wrong (nginx.ingress.kubernetes.io/x-forwarded-prefix: "true"):
https://github.com/kubernetes/ingress-nginx/pull/1805/files#diff-589e0b18c15aeaf9e008c8bc9131c317R85

@metaflow
Copy link

Thank you @pieterlange, it works! Side question: what are the translation rules from XForwardedPrefix to x-forwarded-prefix? I have not seen the direct mapping between these strings.

spencergibb pushed a commit to spring-cloud/spring-cloud-gateway that referenced this pull request Aug 14, 2018
If the path of the url that the gw is routing to is a subset of the url that it is routing from then the difference is the prefix. Also applies if the path of the url being routed to is empty and the routed-from url path is non-empty. 

Included quite a few null checks as it should allow for the various different route definition styles and types of request. I've been [using this](https://github.com/ryandawsonuk/activiti-cloud-gateway/tree/develop/src/main/java/org/springframework/cloud/gateway/filter/headers) in k8s by overriding the class and found the null checks to be necessary.

Was thinking it could be disabled by default as that's how it is for [nginx ingress](kubernetes/ingress-nginx#1805) but I haven't thought of any specific reason not to enable by default.

fixes gh-314
go2max pushed a commit to go2max/spring-microservice-api that referenced this pull request May 15, 2021
If the path of the url that the gw is routing to is a subset of the url that it is routing from then the difference is the prefix. Also applies if the path of the url being routed to is empty and the routed-from url path is non-empty. 

Included quite a few null checks as it should allow for the various different route definition styles and types of request. I've been [using this](https://github.com/ryandawsonuk/activiti-cloud-gateway/tree/develop/src/main/java/org/springframework/cloud/gateway/filter/headers) in k8s by overriding the class and found the null checks to be necessary.

Was thinking it could be disabled by default as that's how it is for [nginx ingress](kubernetes/ingress-nginx#1805) but I haven't thought of any specific reason not to enable by default.

fixes gh-314
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: X-Forwarded-Prefix set to the Path value
7 participants