Skip to content

Commit

Permalink
Merge pull request #1805 from maxlaverse/add_x_forwarded_prefix
Browse files Browse the repository at this point in the history
Add X-Forwarded-Prefix on rewrites
  • Loading branch information
aledbf authored Dec 9, 2017
2 parents 6816630 + ce99a5e commit d14d220
Show file tree
Hide file tree
Showing 9 changed files with 151 additions and 25 deletions.
1 change: 1 addition & 0 deletions docs/annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ Key:
| `add-base-url` | Add `<base>` tag to HTML. | | nginx
| `base-url-scheme` | Specify the scheme of the `<base>` tags. | | nginx
| `preserve-host` | Whether to pass the client request host (`true`) or the origin hostname (`false`) in the HTTP Host field. | | trafficserver
| `x-forwarded-prefix` | Add the non-standard `X-Forwarded-Prefix` header to the request with the value of the matched location. | | nginx

## CORS Related
| Name | Meaning | Default | Controller
Expand Down
3 changes: 3 additions & 0 deletions internal/ingress/annotations/annotations.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import (
"k8s.io/ingress-nginx/internal/ingress/annotations/upstreamhashby"
"k8s.io/ingress-nginx/internal/ingress/annotations/upstreamvhost"
"k8s.io/ingress-nginx/internal/ingress/annotations/vtsfilterkey"
"k8s.io/ingress-nginx/internal/ingress/annotations/xforwardedprefix"
"k8s.io/ingress-nginx/internal/ingress/errors"
"k8s.io/ingress-nginx/internal/ingress/resolver"
)
Expand Down Expand Up @@ -81,6 +82,7 @@ type Ingress struct {
UpstreamVhost string
VtsFilterKey string
Whitelist ipwhitelist.SourceRange
XForwardedPrefix bool
}

// Extractor defines the annotation parsers to be used in the extraction of annotations
Expand Down Expand Up @@ -115,6 +117,7 @@ func NewAnnotationExtractor(cfg resolver.Resolver) Extractor {
"UpstreamVhost": upstreamvhost.NewParser(cfg),
"VtsFilterKey": vtsfilterkey.NewParser(cfg),
"Whitelist": ipwhitelist.NewParser(cfg),
"XForwardedPrefix": xforwardedprefix.NewParser(cfg),
},
}
}
Expand Down
39 changes: 39 additions & 0 deletions internal/ingress/annotations/xforwardedprefix/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
/*
Copyright 2017 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package xforwardedprefix

import (
extensions "k8s.io/api/extensions/v1beta1"

"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/ingress/resolver"
)

type xforwardedprefix struct {
r resolver.Resolver
}

// NewParser creates a new xforwardedprefix annotation parser
func NewParser(r resolver.Resolver) parser.IngressAnnotation {
return xforwardedprefix{r}
}

// Parse parses the annotations contained in the ingress rule
// used to add an x-forwarded-prefix header to the request
func (cbbs xforwardedprefix) Parse(ing *extensions.Ingress) (interface{}, error) {
return parser.GetBoolAnnotation("x-forwarded-prefix", ing)
}
62 changes: 62 additions & 0 deletions internal/ingress/annotations/xforwardedprefix/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
Copyright 2017 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package xforwardedprefix

import (
"testing"

api "k8s.io/api/core/v1"
extensions "k8s.io/api/extensions/v1beta1"
meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/ingress-nginx/internal/ingress/annotations/parser"
"k8s.io/ingress-nginx/internal/ingress/resolver"
)

func TestParse(t *testing.T) {
annotation := parser.GetAnnotationWithPrefix("x-forwarded-prefix")
ap := NewParser(&resolver.Mock{})
if ap == nil {
t.Fatalf("expected a parser.IngressAnnotation but returned nil")
}

testCases := []struct {
annotations map[string]string
expected bool
}{
{map[string]string{annotation: "true"}, true},
{map[string]string{annotation: "1"}, true},
{map[string]string{annotation: ""}, false},
{map[string]string{}, false},
{nil, false},
}

ing := &extensions.Ingress{
ObjectMeta: meta_v1.ObjectMeta{
Name: "foo",
Namespace: api.NamespaceDefault,
},
Spec: extensions.IngressSpec{},
}

for _, testCase := range testCases {
ing.SetAnnotations(testCase.annotations)
result, _ := ap.Parse(ing)
if result != testCase.expected {
t.Errorf("expected %v but returned %v, annotations: %s", testCase.expected, result, testCase.annotations)
}
}
}
2 changes: 2 additions & 0 deletions internal/ingress/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,7 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]
loc.VtsFilterKey = anns.VtsFilterKey
loc.Whitelist = anns.Whitelist
loc.Denied = anns.Denied
loc.XForwardedPrefix = anns.XForwardedPrefix

if loc.Redirect.FromToWWW {
server.RedirectFromToWWW = true
Expand Down Expand Up @@ -503,6 +504,7 @@ func (n *NGINXController) getBackendServers(ingresses []*extensions.Ingress) ([]
VtsFilterKey: anns.VtsFilterKey,
Whitelist: anns.Whitelist,
Denied: anns.Denied,
XForwardedPrefix: anns.XForwardedPrefix,
}

if loc.Redirect.FromToWWW {
Expand Down
13 changes: 9 additions & 4 deletions internal/ingress/controller/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -324,20 +324,25 @@ func buildProxyPass(host string, b interface{}, loc interface{}) string {
}
}

xForwardedPrefix := ""
if location.XForwardedPrefix {
xForwardedPrefix = fmt.Sprintf(`proxy_set_header X-Forwarded-Prefix "%s";
`, path)
}
if location.Rewrite.Target == slash {
// special case redirect to /
// ie /something to /
return fmt.Sprintf(`
rewrite %s(.*) /$1 break;
rewrite %s / break;
proxy_pass %s://%s;
%v`, path, location.Path, proto, upstreamName, abu)
%vproxy_pass %s://%s;
%v`, path, location.Path, xForwardedPrefix, proto, upstreamName, abu)
}

return fmt.Sprintf(`
rewrite %s(.*) %s/$1 break;
proxy_pass %s://%s;
%v`, path, location.Rewrite.Target, proto, upstreamName, abu)
%vproxy_pass %s://%s;
%v`, path, location.Rewrite.Target, xForwardedPrefix, proto, upstreamName, abu)
}

// default proxy_pass
Expand Down
49 changes: 28 additions & 21 deletions internal/ingress/controller/template/template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,64 +36,70 @@ import (
var (
// TODO: add tests for secure endpoints
tmplFuncTestcases = map[string]struct {
Path string
Target string
Location string
ProxyPass string
AddBaseURL bool
BaseURLScheme string
Sticky bool
Path string
Target string
Location string
ProxyPass string
AddBaseURL bool
BaseURLScheme string
Sticky bool
XForwardedPrefix bool
}{
"invalid redirect / to /": {"/", "/", "/", "proxy_pass http://upstream-name;", false, "", false},
"invalid redirect / to /": {"/", "/", "/", "proxy_pass http://upstream-name;", false, "", false, false},
"redirect / to /jenkins": {"/", "/jenkins", "~* /",
`
rewrite /(.*) /jenkins/$1 break;
proxy_pass http://upstream-name;
`, false, "", false},
`, false, "", false, false},
"redirect /something to /": {"/something", "/", `~* ^/something\/?(?<baseuri>.*)`, `
rewrite /something/(.*) /$1 break;
rewrite /something / break;
proxy_pass http://upstream-name;
`, false, "", false},
`, false, "", false, false},
"redirect /end-with-slash/ to /not-root": {"/end-with-slash/", "/not-root", "~* ^/end-with-slash/(?<baseuri>.*)", `
rewrite /end-with-slash/(.*) /not-root/$1 break;
proxy_pass http://upstream-name;
`, false, "", false},
`, false, "", false, false},
"redirect /something-complex to /not-root": {"/something-complex", "/not-root", `~* ^/something-complex\/?(?<baseuri>.*)`, `
rewrite /something-complex/(.*) /not-root/$1 break;
proxy_pass http://upstream-name;
`, false, "", false},
`, false, "", false, false},
"redirect / to /jenkins and rewrite": {"/", "/jenkins", "~* /", `
rewrite /(.*) /jenkins/$1 break;
proxy_pass http://upstream-name;
subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="$scheme://$http_host/$baseuri">' ro;
`, true, "", false},
`, true, "", false, false},
"redirect /something to / and rewrite": {"/something", "/", `~* ^/something\/?(?<baseuri>.*)`, `
rewrite /something/(.*) /$1 break;
rewrite /something / break;
proxy_pass http://upstream-name;
subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="$scheme://$http_host/something/$baseuri">' ro;
`, true, "", false},
`, true, "", false, false},
"redirect /end-with-slash/ to /not-root and rewrite": {"/end-with-slash/", "/not-root", `~* ^/end-with-slash/(?<baseuri>.*)`, `
rewrite /end-with-slash/(.*) /not-root/$1 break;
proxy_pass http://upstream-name;
subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="$scheme://$http_host/end-with-slash/$baseuri">' ro;
`, true, "", false},
`, true, "", false, false},
"redirect /something-complex to /not-root and rewrite": {"/something-complex", "/not-root", `~* ^/something-complex\/?(?<baseuri>.*)`, `
rewrite /something-complex/(.*) /not-root/$1 break;
proxy_pass http://upstream-name;
subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="$scheme://$http_host/something-complex/$baseuri">' ro;
`, true, "", false},
`, true, "", false, false},
"redirect /something to / and rewrite with specific scheme": {"/something", "/", `~* ^/something\/?(?<baseuri>.*)`, `
rewrite /something/(.*) /$1 break;
rewrite /something / break;
proxy_pass http://upstream-name;
subs_filter '(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)' '$1<base href="http://$http_host/something/$baseuri">' ro;
`, true, "http", false},
`, true, "http", false, false},
"redirect / to /something with sticky enabled": {"/", "/something", `~* /`, `
rewrite /(.*) /something/$1 break;
proxy_pass http://sticky-upstream-name;
`, false, "http", true},
`, false, "http", true, false},
"add the X-Forwarded-Prefix header": {"/there", "/something", `~* ^/there\/?(?<baseuri>.*)`, `
rewrite /there/(.*) /something/$1 break;
proxy_set_header X-Forwarded-Prefix "/there/";
proxy_pass http://sticky-upstream-name;
`, false, "http", true, true},
}
)

Expand Down Expand Up @@ -136,9 +142,10 @@ func TestBuildProxyPass(t *testing.T) {

for k, tc := range tmplFuncTestcases {
loc := &ingress.Location{
Path: tc.Path,
Rewrite: rewrite.Config{Target: tc.Target, AddBaseURL: tc.AddBaseURL, BaseURLScheme: tc.BaseURLScheme},
Backend: defaultBackend,
Path: tc.Path,
Rewrite: rewrite.Config{Target: tc.Target, AddBaseURL: tc.AddBaseURL, BaseURLScheme: tc.BaseURLScheme},
Backend: defaultBackend,
XForwardedPrefix: tc.XForwardedPrefix,
}

backends := []*ingress.Backend{}
Expand Down
4 changes: 4 additions & 0 deletions internal/ingress/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,10 @@ type Location struct {
// DefaultBackend allows the use of a custom default backend for this location.
// +optional
DefaultBackend *apiv1.Service `json:"defaultBackend,omitempty"`
// XForwardedPrefix allows to add a header X-Forwarded-Prefix to the request with the
// original location.
// +optional
XForwardedPrefix bool `json:"xForwardedPrefix,omitempty"`
}

// SSLPassthroughBackend describes a SSL upstream server configured
Expand Down
3 changes: 3 additions & 0 deletions internal/ingress/types_equals.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,9 @@ func (l1 *Location) Equal(l2 *Location) bool {
if l1.UpstreamVhost != l2.UpstreamVhost {
return false
}
if l1.XForwardedPrefix != l2.XForwardedPrefix {
return false
}

return true
}
Expand Down

0 comments on commit d14d220

Please sign in to comment.