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

Generalize Rewrite Block Creation and Deprecate AddBaseUrl (not backwards compatible) #3174

Merged
merged 1 commit into from
Jan 2, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 14 additions & 47 deletions docs/examples/rewrite/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,21 @@ Rewriting can be controlled using the following annotations:
|Name|Description|Values|
| --- | --- | --- |
|nginx.ingress.kubernetes.io/rewrite-target|Target URI where the traffic must be redirected|string|
|nginx.ingress.kubernetes.io/add-base-url|indicates if is required to add a base tag in the head of the responses from the upstream servers|bool|
|nginx.ingress.kubernetes.io/base-url-scheme|Override for the scheme passed to the base tag|string|
|nginx.ingress.kubernetes.io/ssl-redirect|Indicates if the location section is accessible SSL only (defaults to True when Ingress contains a Certificate)|bool|
|nginx.ingress.kubernetes.io/force-ssl-redirect|Forces the redirection to HTTPS even if the Ingress is not TLS Enabled|bool|
|nginx.ingress.kubernetes.io/app-root|Defines the Application Root that the Controller must redirect if it's in '/' context|string|
|nginx.ingress.kubernetes.io/use-regex|Indicates if the paths defined on an Ingress use regular expressions|bool|

## Validation
## Examples

### Rewrite Target

!!! attention
Starting in Version 0.2.2, ingress definitions using the annotation `nginx.ingress.kubernetes.io/rewrite-target` are not backwards compatible with previous versions. In Version 0.2.2 and beyond, any substrings within the request URI that need to be passed to the rewritten path must explicitly be defined in a [capture group](https://www.regular-expressions.info/refcapture.html).

!!! note
[Captured groups](https://www.regular-expressions.info/refcapture.html) are saved in numbered placeholders, chronologically, in the form `$1`, `$2` ... `$n`. These placeholders can be used as parameters in the `rewrite-target` annotation.

Create an Ingress rule with a rewrite annotation:

```console
Expand All @@ -34,7 +38,7 @@ apiVersion: extensions/v1beta1
kind: Ingress
metadata:
annotations:
nginx.ingress.kubernetes.io/rewrite-target: /
nginx.ingress.kubernetes.io/rewrite-target: /$1
name: rewrite
namespace: default
spec:
Expand All @@ -45,53 +49,16 @@ spec:
- backend:
serviceName: http-svc
servicePort: 80
path: /something
path: /something/?(.*)
" | kubectl create -f -
```

Check the rewrite is working
In this ingress definition, any characters captured by `(.*)` will be assigned to the placeholder `$1`, which is then used as a parameter in the `rewrite-target` annotation.

```
$ curl -v http://172.17.4.99/something -H 'Host: rewrite.bar.com'
* Trying 172.17.4.99...
* Connected to 172.17.4.99 (172.17.4.99) port 80 (#0)
> GET /something HTTP/1.1
> Host: rewrite.bar.com
> User-Agent: curl/7.43.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Server: nginx/1.11.0
< Date: Tue, 31 May 2016 16:07:31 GMT
< Content-Type: text/plain
< Transfer-Encoding: chunked
< Connection: keep-alive
<
CLIENT VALUES:
client_address=10.2.56.9
command=GET
real path=/
query=nil
request_version=1.1
request_uri=http://rewrite.bar.com:8080/

SERVER VALUES:
server_version=nginx: 1.9.11 - lua: 10001

HEADERS RECEIVED:
accept=*/*
connection=close
host=rewrite.bar.com
user-agent=curl/7.43.0
x-forwarded-for=10.2.56.1
x-forwarded-host=rewrite.bar.com
x-forwarded-port=80
x-forwarded-proto=http
x-real-ip=10.2.56.1
BODY:
* Connection #0 to host 172.17.4.99 left intact
-no body in request-
```
For example, the ingress definition above will result in the following rewrites:
- `rewrite.bar.com/something` rewrites to `rewrite.bar.com/`
- `rewrite.bar.com/something/` rewrites to `rewrite.bar.com/`
- `rewrite.bar.com/something/new` rewrites to `rewrite.bar.com/new`

### App Root

Expand Down
15 changes: 7 additions & 8 deletions docs/user-guide/ingress-path-matching.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,11 @@ spec:
paths:
- path: /foo/bar
backend:
serviceName: test
serviceName: service1
servicePort: 80
- path: /foo/bar/
backend:
serviceName: test
serviceName: service2
servicePort: 80
```

Expand All @@ -76,14 +76,14 @@ spec:
paths:
- path: /foo/bar/.+
backend:
serviceName: test
serviceName: service3
servicePort: 80
```

The ingress controller would define the following location blocks, in order of descending length, within the NGINX template for the `test.com` server:
Copy link
Member

Choose a reason for hiding this comment

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

Why would it define more than single location block? Setting path to /foo/bar/.+ means I require all paths to be prefixed with /foo/bar/ and followed by at least a single character.

If in addition to above I want to match /foo/bar and /foo/bar/ I'd define my as as /foo/bar/?.*

Copy link
Contributor Author

@zrdaley zrdaley Oct 26, 2018

Choose a reason for hiding this comment

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

These should all have different service backends, otherwise you're right, it wouldn't make sense to have 3 locations. My mistake, will fix!

Copy link
Member

Choose a reason for hiding this comment

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

but there's a concrete example above and it has just single path and backend service, so why not fix this?


```txt
location ~* "^/foo/bar/.+\/?(?<baseuri>.*)" {
location ~* ^/foo/bar/.+ {
...
}

Expand All @@ -98,13 +98,12 @@ location ~* "^/foo/bar" {

The following request URI's would match the corresponding location blocks:

- `test.com/foo/bar/1` matches `~* "^/foo/bar/.+\/?(?<baseuri>.*)"`
- `test.com/foo/bar/` matches `~* "^/foo/bar/"`
- `test.com/foo/bar` matches `~* "^/foo/bar"`
- `test.com/foo/bar/1` matches `~* ^/foo/bar/.+` and will go to service 3.
- `test.com/foo/bar/` matches `~* ^/foo/bar/` and will go to service 2.
- `test.com/foo/bar` matches `~* ^/foo/bar` and will go to service 1.

**IMPORTANT NOTES**:

- paths created under the `rewrite-ingress` are sorted before `\/?(?<baseuri>.*)` is appended. For example if the path defined within `test-ingress-2` was `/foo/.+` then the location block for `^/foo/.+\/?(?<baseuri>.*)` would be the LAST block listed.
- If the `use-regex` OR `rewrite-target` annotation is used on any Ingress for a given host, then the case insensitive regular expression [location modifier](https://nginx.org/en/docs/http/ngx_http_core_module.html#location) will be enforced on ALL paths for a given host regardless of what Ingress they are defined on.

## Warning
Expand Down
6 changes: 0 additions & 6 deletions docs/user-guide/nginx-configuration/annotations.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz

|Name | type |
|---------------------------|------|
|[nginx.ingress.kubernetes.io/add-base-url](#rewrite)|"true" or "false"|
|[nginx.ingress.kubernetes.io/app-root](#rewrite)|string|
|[nginx.ingress.kubernetes.io/affinity](#session-affinity)|cookie|
|[nginx.ingress.kubernetes.io/auth-realm](#authentication)|string|
Expand All @@ -29,7 +28,6 @@ You can add these Kubernetes annotations to specific Ingress objects to customiz
|[nginx.ingress.kubernetes.io/auth-url](#external-authentication)|string|
|[nginx.ingress.kubernetes.io/auth-snippet](#external-authentication)|string|
|[nginx.ingress.kubernetes.io/backend-protocol](#backend-protocol)|string|HTTP,HTTPS,GRPC,GRPCS,AJP|
|[nginx.ingress.kubernetes.io/base-url-scheme](#rewrite)|string|
|[nginx.ingress.kubernetes.io/canary](#canary)|"true" or "false"|
|[nginx.ingress.kubernetes.io/canary-by-header](#canary)|string|
|[nginx.ingress.kubernetes.io/canary-by-cookie](#canary)|string|
Expand Down Expand Up @@ -125,10 +123,6 @@ Currently a maximum of one canary ingress can be applied per Ingress rule.
In some scenarios the exposed URL in the backend service differs from the specified path in the Ingress rule. Without a rewrite any request will return 404.
Set the annotation `nginx.ingress.kubernetes.io/rewrite-target` to the path expected by the service.

If the application contains relative links it is possible to add an additional annotation `nginx.ingress.kubernetes.io/add-base-url` that will prepend a [`base` tag](https://developer.mozilla.org/en/docs/Web/HTML/Element/base) in the header of the returned HTML from the backend.

If the scheme of [`base` tag](https://developer.mozilla.org/en/docs/Web/HTML/Element/base) need to be specific, set the annotation `nginx.ingress.kubernetes.io/base-url-scheme` to the scheme such as `http` and `https`.

If the Application Root is exposed in a different path and needs to be redirected, set the annotation `nginx.ingress.kubernetes.io/app-root` to redirect requests for `/`.

!!! example
Expand Down
13 changes: 0 additions & 13 deletions internal/ingress/annotations/rewrite/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,6 @@ import (
type Config struct {
// Target URI where the traffic must be redirected
Target string `json:"target"`
// AddBaseURL indicates if is required to add a base tag in the head
// of the responses from the upstream servers
AddBaseURL bool `json:"addBaseUrl"`
// BaseURLScheme override for the scheme passed to the base tag
BaseURLScheme string `json:"baseUrlScheme"`
// SSLRedirect indicates if the location section is accessible SSL only
SSLRedirect bool `json:"sslRedirect"`
// ForceSSLRedirect indicates if the location section is accessible SSL only
Expand All @@ -53,12 +48,6 @@ func (r1 *Config) Equal(r2 *Config) bool {
if r1.Target != r2.Target {
return false
}
if r1.AddBaseURL != r2.AddBaseURL {
return false
}
if r1.BaseURLScheme != r2.BaseURLScheme {
return false
}
if r1.SSLRedirect != r2.SSLRedirect {
return false
}
Expand Down Expand Up @@ -101,8 +90,6 @@ func (a rewrite) Parse(ing *extensions.Ingress) (interface{}, error) {
config.ForceSSLRedirect = a.r.GetDefaultBackend().ForceSSLRedirect
}

config.AddBaseURL, _ = parser.GetBoolAnnotation("add-base-url", ing)
config.BaseURLScheme, _ = parser.GetStringAnnotation("base-url-scheme", ing)
config.AppRoot, _ = parser.GetStringAnnotation("app-root", ing)
config.UseRegex, _ = parser.GetBoolAnnotation("use-regex", ing)

Expand Down
49 changes: 2 additions & 47 deletions internal/ingress/controller/template/template.go
Original file line number Diff line number Diff line change
Expand Up @@ -338,19 +338,6 @@ func buildLocation(input interface{}, enforceRegex bool) string {
}

path := location.Path
if needsRewrite(location) {
if path == slash {
return fmt.Sprintf("~* ^%s", path)
}
// baseuri regex will parse basename from the given location
baseuri := `(?<baseuri>.*)`
if !strings.HasSuffix(path, slash) {
// Not treat the slash after "location path" as a part of baseuri
baseuri = fmt.Sprintf(`\/?%s`, baseuri)
}
return fmt.Sprintf(`~* "^%s%s"`, path, baseuri)
}

if enforceRegex {
return fmt.Sprintf(`~* "^%s"`, path)
}
Expand Down Expand Up @@ -465,48 +452,16 @@ func buildProxyPass(host string, b interface{}, loc interface{}) string {
return defProxyPass
}

if !strings.HasSuffix(path, slash) {
path = fmt.Sprintf("%s/", path)
}

if len(location.Rewrite.Target) > 0 {
var abu string
var xForwardedPrefix string

if location.Rewrite.AddBaseURL {
bPath := fmt.Sprintf("%s$escaped_base_uri", path)
regex := `(<(?:H|h)(?:E|e)(?:A|a)(?:D|d)(?:[^">]|"[^"]*")*>)`
scheme := "$scheme"

if len(location.Rewrite.BaseURLScheme) > 0 {
scheme = location.Rewrite.BaseURLScheme
}

abu = fmt.Sprintf(`
set_escape_uri $escaped_base_uri $baseuri;
subs_filter '%v' '$1<base href="%v://$http_host%v">' ro;
`, regex, scheme, bPath)
}

if location.XForwardedPrefix {
xForwardedPrefix = fmt.Sprintf("proxy_set_header X-Forwarded-Prefix \"%s\";\n", path)
}

if location.Rewrite.Target == slash {
// special case redirect to /
// ie /something to /
return fmt.Sprintf(`
rewrite "(?i)%s(.*)" /$1 break;
rewrite "(?i)%s$" / break;
%v%v %s%s;
%v`, path, location.Path, xForwardedPrefix, proxyPass, proto, upstreamName, abu)
}

return fmt.Sprintf(`
rewrite "(?i)%s(.*)" %s/$1 break;
rewrite "(?i)%s$" %s/ break;
%v%v %s%s;
%v`, path, location.Rewrite.Target, location.Path, location.Rewrite.Target, xForwardedPrefix, proxyPass, proto, upstreamName, abu)
rewrite "(?i)%s" %s break;
%v%v %s%s;`, path, location.Rewrite.Target, xForwardedPrefix, proxyPass, proto, upstreamName)
}

// default proxy_pass
Expand Down
Loading