From ed6819468828cdedd97960cbc26cd1a0c006972f Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Sat, 19 Aug 2017 18:13:02 -0300 Subject: [PATCH] Add support for temporal and permanent redirects --- controllers/nginx/pkg/cmd/controller/nginx.go | 24 +++- controllers/nginx/pkg/config/config.go | 1 + controllers/nginx/pkg/template/template.go | 21 ++- .../nginx/pkg/template/template_test.go | 13 +- .../rootfs/etc/nginx/template/nginx.tmpl | 30 +++- .../ingress/annotations/redirect/redirect.go | 131 ++++++++++++++++++ core/pkg/ingress/controller/annotations.go | 6 +- core/pkg/ingress/controller/controller.go | 12 +- core/pkg/ingress/controller/util_test.go | 4 +- core/pkg/ingress/errors/errors.go | 8 ++ core/pkg/ingress/types.go | 12 +- core/pkg/ingress/types_equals.go | 3 + 12 files changed, 232 insertions(+), 33 deletions(-) create mode 100644 core/pkg/ingress/annotations/redirect/redirect.go diff --git a/controllers/nginx/pkg/cmd/controller/nginx.go b/controllers/nginx/pkg/cmd/controller/nginx.go index 189921c14..e299b0bb3 100644 --- a/controllers/nginx/pkg/cmd/controller/nginx.go +++ b/controllers/nginx/pkg/cmd/controller/nginx.go @@ -446,7 +446,6 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error { IP: svc.Spec.ClusterIP, Port: port, ProxyProtocol: false, - }) } @@ -467,11 +466,33 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error { // https://trac.nginx.org/nginx/ticket/631 var longestName int var serverNameBytes int + redirectServers := make(map[string]string) for _, srv := range ingressCfg.Servers { if longestName < len(srv.Hostname) { longestName = len(srv.Hostname) } serverNameBytes += len(srv.Hostname) + if srv.RedirectFromToWWW { + var n string + if strings.HasPrefix(srv.Hostname, "www.") { + n = strings.TrimLeft(srv.Hostname, "www.") + } else { + n = fmt.Sprintf("www.%v", srv.Hostname) + } + glog.V(3).Infof("creating redirect from %v to", srv.Hostname, n) + if _, ok := redirectServers[n]; !ok { + found := false + for _, esrv := range ingressCfg.Servers { + if esrv.Hostname == n { + found = true + break + } + } + if !found { + redirectServers[n] = srv.Hostname + } + } + } } if cfg.ServerNameHashBucketSize == 0 { nameHashBucketSize := nginxHashBucketSize(longestName) @@ -562,6 +583,7 @@ func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) error { CustomErrors: len(cfg.CustomHTTPErrors) > 0, Cfg: cfg, IsIPV6Enabled: n.isIPV6Enabled && !cfg.DisableIpv6, + RedirectServers: redirectServers, } // We need to extract the endpoints to be used in the fastcgi error handler diff --git a/controllers/nginx/pkg/config/config.go b/controllers/nginx/pkg/config/config.go index 190fdb9d6..6154b4aa1 100644 --- a/controllers/nginx/pkg/config/config.go +++ b/controllers/nginx/pkg/config/config.go @@ -428,4 +428,5 @@ type TemplateConfig struct { CustomErrors bool Cfg Configuration IsIPV6Enabled bool + RedirectServers map[string]string } diff --git a/controllers/nginx/pkg/template/template.go b/controllers/nginx/pkg/template/template.go index 1c2a175b8..a3cc83840 100644 --- a/controllers/nginx/pkg/template/template.go +++ b/controllers/nginx/pkg/template/template.go @@ -27,11 +27,10 @@ import ( "strings" text_template "text/template" - "k8s.io/apimachinery/pkg/util/sets" - "github.com/golang/glog" - "github.com/pborman/uuid" + + "k8s.io/apimachinery/pkg/util/sets" "k8s.io/ingress/controllers/nginx/pkg/config" "k8s.io/ingress/core/pkg/ingress" ing_net "k8s.io/ingress/core/pkg/net" @@ -148,7 +147,7 @@ var ( "formatIP": formatIP, "buildNextUpstream": buildNextUpstream, "serverConfig": func(all config.TemplateConfig, server *ingress.Server) interface{} { - return struct { First, Second interface{} } { all, server } + return struct{ First, Second interface{} }{all, server} }, } ) @@ -197,7 +196,7 @@ func buildLocation(input interface{}) string { } path := location.Path - if len(location.Redirect.Target) > 0 && location.Redirect.Target != path { + if len(location.Rewrite.Target) > 0 && location.Rewrite.Target != path { if path == slash { return fmt.Sprintf("~* %s", path) } @@ -290,7 +289,7 @@ func buildProxyPass(host string, b interface{}, loc interface{}) string { // defProxyPass returns the default proxy_pass, just the name of the upstream defProxyPass := fmt.Sprintf("proxy_pass %s://%s;", proto, upstreamName) // if the path in the ingress rule is equals to the target: no special rewrite - if path == location.Redirect.Target { + if path == location.Rewrite.Target { return defProxyPass } @@ -298,9 +297,9 @@ func buildProxyPass(host string, b interface{}, loc interface{}) string { path = fmt.Sprintf("%s/", path) } - if len(location.Redirect.Target) > 0 { + if len(location.Rewrite.Target) > 0 { abu := "" - if location.Redirect.AddBaseURL { + if location.Rewrite.AddBaseURL { // path has a slash suffix, so that it can be connected with baseuri directly bPath := fmt.Sprintf("%s%s", path, "$baseuri") abu = fmt.Sprintf(`subs_filter '' '' r; @@ -308,7 +307,7 @@ func buildProxyPass(host string, b interface{}, loc interface{}) string { `, bPath, bPath) } - if location.Redirect.Target == slash { + if location.Rewrite.Target == slash { // special case redirect to / // ie /something to / return fmt.Sprintf(` @@ -321,7 +320,7 @@ func buildProxyPass(host string, b interface{}, loc interface{}) string { return fmt.Sprintf(` rewrite %s(.*) %s/$1 break; proxy_pass %s://%s; - %v`, path, location.Redirect.Target, proto, upstreamName, abu) + %v`, path, location.Rewrite.Target, proto, upstreamName, abu) } // default proxy_pass @@ -502,4 +501,4 @@ func buildNextUpstream(input interface{}) string { } return strings.Join(nextUpstreamCodes, " ") -} \ No newline at end of file +} diff --git a/controllers/nginx/pkg/template/template_test.go b/controllers/nginx/pkg/template/template_test.go index ac8a1b5c7..d3fa2ee6a 100644 --- a/controllers/nginx/pkg/template/template_test.go +++ b/controllers/nginx/pkg/template/template_test.go @@ -18,14 +18,13 @@ package template import ( "encoding/json" + "io/ioutil" "os" "path" "reflect" "strings" "testing" - "io/ioutil" - "k8s.io/ingress/controllers/nginx/pkg/config" "k8s.io/ingress/core/pkg/ingress" "k8s.io/ingress/core/pkg/ingress/annotations/authreq" @@ -110,8 +109,8 @@ func TestFormatIP(t *testing.T) { func TestBuildLocation(t *testing.T) { for k, tc := range tmplFuncTestcases { loc := &ingress.Location{ - Path: tc.Path, - Redirect: rewrite.Redirect{Target: tc.Target, AddBaseURL: tc.AddBaseURL}, + Path: tc.Path, + Rewrite: rewrite.Redirect{Target: tc.Target, AddBaseURL: tc.AddBaseURL}, } newLoc := buildLocation(loc) @@ -124,9 +123,9 @@ func TestBuildLocation(t *testing.T) { func TestBuildProxyPass(t *testing.T) { for k, tc := range tmplFuncTestcases { loc := &ingress.Location{ - Path: tc.Path, - Redirect: rewrite.Redirect{Target: tc.Target, AddBaseURL: tc.AddBaseURL}, - Backend: "upstream-name", + Path: tc.Path, + Rewrite: rewrite.Redirect{Target: tc.Target, AddBaseURL: tc.AddBaseURL}, + Backend: "upstream-name", } pp := buildProxyPass("", []*ingress.Backend{}, loc) diff --git a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl index 98147223e..1ee7bc9bf 100644 --- a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl +++ b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl @@ -290,6 +290,20 @@ http { {{ $zone }} {{ end }} + {{/* Build server redirects (from/to www) */}} + {{ range $hostname, $to := .RedirectServers }} + server { + listen 80{{ if $all.Cfg.UseProxyProtocol }} proxy_protocol{{ end }}; + listen 442{{ if $all.Cfg.UseProxyProtocol }} proxy_protocol{{ end }} ssl; + {{ if $IsIPV6Enabled }} + listen [::]:80{{ if $all.Cfg.UseProxyProtocol }} proxy_protocol{{ end }}; + listen [::]:442{{ if $all.Cfg.UseProxyProtocol }} proxy_protocol{{ end }}; + {{ end }} + server_name {{ $hostname }}; + return 301 $scheme://{{ $to }}$request_uri; + } + {{ end }} + {{ $backlogSize := .BacklogSize }} {{ range $index, $server := $servers }} server { @@ -510,9 +524,9 @@ stream { ssl_verify_depth {{ $location.CertificateAuth.ValidationDepth }}; {{ end }} - {{ if not (empty $location.Redirect.AppRoot)}} + {{ if not (empty $location.Rewrite.AppRoot)}} if ($uri = /) { - return 302 {{ $location.Redirect.AppRoot }}; + return 302 {{ $location.Rewrite.AppRoot }}; } {{ end }} @@ -536,7 +550,6 @@ stream { client_max_body_size "{{ $location.Proxy.BodySize }}"; - set $target {{ $location.ExternalAuth.URL }}; proxy_pass $target; } @@ -545,7 +558,7 @@ stream { location {{ $path }} { set $proxy_upstream_name "{{ buildUpstreamName $server.Hostname $all.Backends $location }}"; - {{ if (or $location.Redirect.ForceSSLRedirect (and (not (empty $server.SSLCertificate)) $location.Redirect.SSLRedirect)) }} + {{ if (or $location.Rewrite.ForceSSLRedirect (and (not (empty $server.SSLCertificate)) $location.Rewrite.SSLRedirect)) }} # enforce ssl on server side if ($pass_access_scheme = http) { return 301 https://$best_http_host$request_uri; @@ -575,7 +588,6 @@ stream { error_page 401 = {{ $location.ExternalAuth.SigninURL }}?rd=$request_uri; {{ end }} - {{/* if the location contains a rate limit annotation, create one */}} {{ $limits := buildRateLimit $location }} {{ range $limit := $limits }} @@ -596,6 +608,12 @@ stream { {{ template "CORS" }} {{ end }} + {{ if not (empty $location.Redirect.URL) }} + if ($uri ~* {{ $path }}) { + return {{ $location.Redirect.Code }} {{ $location.Redirect.URL }}; + } + {{ end }} + client_max_body_size "{{ $location.Proxy.BodySize }}"; proxy_set_header Host $best_http_host; @@ -644,7 +662,7 @@ stream { proxy_next_upstream {{ buildNextUpstream $location.Proxy.NextUpstream }}{{ if $all.Cfg.RetryNonIdempotent }} non_idempotent{{ end }}; {{/* rewrite only works if the content is not compressed */}} - {{ if $location.Redirect.AddBaseURL }} + {{ if $location.Rewrite.AddBaseURL }} proxy_set_header Accept-Encoding ""; {{ end }} diff --git a/core/pkg/ingress/annotations/redirect/redirect.go b/core/pkg/ingress/annotations/redirect/redirect.go new file mode 100644 index 000000000..b5e366580 --- /dev/null +++ b/core/pkg/ingress/annotations/redirect/redirect.go @@ -0,0 +1,131 @@ +/* +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 redirect + +import ( + "net/http" + "net/url" + "strings" + + extensions "k8s.io/api/extensions/v1beta1" + + "k8s.io/ingress/core/pkg/ingress/annotations/parser" + "k8s.io/ingress/core/pkg/ingress/errors" +) + +const ( + permanent = "ingress.kubernetes.io/permanent-redirect" + temporal = "ingress.kubernetes.io/temporal-redirect" + www = "ingress.kubernetes.io/from-to-www-redirect" +) + +// Redirect returns the redirect configuration for an Ingress rule +type Redirect struct { + URL string `json:"url"` + Code int `json:"code"` + FromToWWW bool `json:"fromToWWW"` +} + +type redirect struct{} + +// NewParser creates a new redirect annotation parser +func NewParser() parser.IngressAnnotation { + return redirect{} +} + +// Parse parses the annotations contained in the ingress +// rule used to create a redirect in the paths defined in the rule. +// If the Ingress containes both annotations the execution order is +// temporal and then permanent +func (a redirect) Parse(ing *extensions.Ingress) (interface{}, error) { + r3w, _ := parser.GetBoolAnnotation(www, ing) + + tr, err := parser.GetStringAnnotation(temporal, ing) + if err != nil && !errors.IsMissingAnnotations(err) { + return nil, err + } + + if tr != "" { + if err := isValidURL(tr); err != nil { + return nil, err + } + + return &Redirect{ + URL: tr, + Code: http.StatusFound, + FromToWWW: r3w, + }, nil + } + + pr, err := parser.GetStringAnnotation(permanent, ing) + if err != nil && !errors.IsMissingAnnotations(err) { + return nil, err + } + + if pr != "" { + if err := isValidURL(pr); err != nil { + return nil, err + } + + return &Redirect{ + URL: pr, + Code: http.StatusMovedPermanently, + FromToWWW: r3w, + }, nil + } + + if r3w { + return &Redirect{ + FromToWWW: r3w, + }, nil + } + + return nil, errors.ErrMissingAnnotations +} + +// Equal tests for equality between two Redirect types +func (r1 *Redirect) Equal(r2 *Redirect) bool { + if r1 == r2 { + return true + } + if r1 == nil || r2 == nil { + return false + } + if r1.URL != r2.URL { + return false + } + if r1.Code != r2.Code { + return false + } + if r1.FromToWWW != r2.FromToWWW { + return false + } + return true +} + +func isValidURL(s string) error { + u, err := url.Parse(s) + if err != nil { + return err + } + + if !strings.HasPrefix(u.Scheme, "http") { + return errors.Errorf("only http and https are valid protocols (%v)", u.Scheme) + } + + return nil +} diff --git a/core/pkg/ingress/controller/annotations.go b/core/pkg/ingress/controller/annotations.go index 01f1cab8b..105e919a0 100644 --- a/core/pkg/ingress/controller/annotations.go +++ b/core/pkg/ingress/controller/annotations.go @@ -19,6 +19,7 @@ package controller import ( "github.com/golang/glog" extensions "k8s.io/api/extensions/v1beta1" + "k8s.io/ingress/core/pkg/ingress/annotations/alias" "k8s.io/ingress/core/pkg/ingress/annotations/auth" "k8s.io/ingress/core/pkg/ingress/annotations/authreq" "k8s.io/ingress/core/pkg/ingress/annotations/authtls" @@ -29,6 +30,7 @@ import ( "k8s.io/ingress/core/pkg/ingress/annotations/portinredirect" "k8s.io/ingress/core/pkg/ingress/annotations/proxy" "k8s.io/ingress/core/pkg/ingress/annotations/ratelimit" + "k8s.io/ingress/core/pkg/ingress/annotations/redirect" "k8s.io/ingress/core/pkg/ingress/annotations/rewrite" "k8s.io/ingress/core/pkg/ingress/annotations/secureupstream" "k8s.io/ingress/core/pkg/ingress/annotations/serviceupstream" @@ -37,7 +39,6 @@ import ( "k8s.io/ingress/core/pkg/ingress/annotations/sslpassthrough" "k8s.io/ingress/core/pkg/ingress/errors" "k8s.io/ingress/core/pkg/ingress/resolver" - "k8s.io/ingress/core/pkg/ingress/annotations/alias" ) type extractorConfig interface { @@ -64,7 +65,8 @@ func newAnnotationExtractor(cfg extractorConfig) annotationExtractor { "UsePortInRedirects": portinredirect.NewParser(cfg), "Proxy": proxy.NewParser(cfg), "RateLimit": ratelimit.NewParser(cfg), - "Redirect": rewrite.NewParser(cfg), + "Redirect": redirect.NewParser(), + "Rewrite": rewrite.NewParser(cfg), "SecureUpstream": secureupstream.NewParser(cfg), "ServiceUpstream": serviceupstream.NewParser(), "SessionAffinity": sessionaffinity.NewParser(), diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index bac29f23f..a6b2d0b54 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -610,11 +610,13 @@ func (ic *GenericController) getBackendServers() ([]*ingress.Backend, []*ingress upstreams := ic.createUpstreams(ings) servers := ic.createServers(ings, upstreams) - // If a server has a hostname equivalent to a pre-existing alias, then we remove the alias + // If a server has a hostname equivalent to a pre-existing alias, then we + // remove the alias to avoid conflicts. for _, server := range servers { for j, alias := range servers { if server.Hostname == alias.Alias { - glog.Warningf("There is a conflict with hostname '%v' and alias of `%v`.", server.Hostname, alias.Hostname) + glog.Warningf("There is a conflict with server hostname '%v' and alias '%v' (in server %v). Removing alias to avoid conflicts.", + server.Hostname, alias.Hostname, alias.Hostname) servers[j].Alias = "" } } @@ -678,6 +680,9 @@ func (ic *GenericController) getBackendServers() ([]*ingress.Backend, []*ingress loc.Port = ups.Port loc.Service = ups.Service mergeLocationAnnotations(loc, anns) + if loc.Redirect.FromToWWW { + server.RedirectFromToWWW = true + } break } } @@ -692,6 +697,9 @@ func (ic *GenericController) getBackendServers() ([]*ingress.Backend, []*ingress Port: ups.Port, } mergeLocationAnnotations(loc, anns) + if loc.Redirect.FromToWWW { + server.RedirectFromToWWW = true + } server.Locations = append(server.Locations, loc) } diff --git a/core/pkg/ingress/controller/util_test.go b/core/pkg/ingress/controller/util_test.go index e49b2803e..af5a1212e 100644 --- a/core/pkg/ingress/controller/util_test.go +++ b/core/pkg/ingress/controller/util_test.go @@ -27,6 +27,7 @@ import ( "k8s.io/ingress/core/pkg/ingress/annotations/ipwhitelist" "k8s.io/ingress/core/pkg/ingress/annotations/proxy" "k8s.io/ingress/core/pkg/ingress/annotations/ratelimit" + "k8s.io/ingress/core/pkg/ingress/annotations/redirect" "k8s.io/ingress/core/pkg/ingress/annotations/rewrite" ) @@ -48,7 +49,8 @@ func TestMergeLocationAnnotations(t *testing.T) { "EnableCORS": true, "ExternalAuth": authreq.External{}, "RateLimit": ratelimit.RateLimit{}, - "Redirect": rewrite.Redirect{}, + "Redirect": redirect.Redirect{}, + "Rewrite": rewrite.Redirect{}, "Whitelist": ipwhitelist.SourceRange{}, "Proxy": proxy.Configuration{}, "CertificateAuth": authtls.AuthSSLConfig{}, diff --git a/core/pkg/ingress/errors/errors.go b/core/pkg/ingress/errors/errors.go index 635df3944..1a9db9ba4 100644 --- a/core/pkg/ingress/errors/errors.go +++ b/core/pkg/ingress/errors/errors.go @@ -83,3 +83,11 @@ func IsInvalidContent(e error) bool { _, ok := e.(InvalidContent) return ok } + +func New(m string) error { + return errors.New(m) +} + +func Errorf(format string, args ...interface{}) error { + return errors.Errorf(format, args) +} diff --git a/core/pkg/ingress/types.go b/core/pkg/ingress/types.go index 00f5afa21..ba316532a 100644 --- a/core/pkg/ingress/types.go +++ b/core/pkg/ingress/types.go @@ -32,6 +32,7 @@ import ( "k8s.io/ingress/core/pkg/ingress/annotations/ipwhitelist" "k8s.io/ingress/core/pkg/ingress/annotations/proxy" "k8s.io/ingress/core/pkg/ingress/annotations/ratelimit" + "k8s.io/ingress/core/pkg/ingress/annotations/redirect" "k8s.io/ingress/core/pkg/ingress/annotations/rewrite" "k8s.io/ingress/core/pkg/ingress/defaults" "k8s.io/ingress/core/pkg/ingress/resolver" @@ -220,8 +221,10 @@ type Server struct { SSLPemChecksum string `json:"sslPemChecksum"` // Locations list of URIs configured in the server. Locations []*Location `json:"locations,omitempty"` - // return the alias of the server name + // Alias return the alias of the server name Alias string `json:"alias,omitempty"` + // RedirectFromToWWW returns if a redirect to/from prefix www is required + RedirectFromToWWW bool `json:"redirectFromToWWW,omitempty"` } // Location describes an URI inside a server. @@ -276,9 +279,12 @@ type Location struct { // The Redirect annotation precedes RateLimit // +optional RateLimit ratelimit.RateLimit `json:"rateLimit,omitempty"` - // Redirect describes the redirection this location. + // Redirect describes a temporal o permanent redirection this location. // +optional - Redirect rewrite.Redirect `json:"redirect,omitempty"` + Redirect redirect.Redirect `json:"redirect,omitempty"` + // Rewrite describes the redirection this location. + // +optional + Rewrite rewrite.Redirect `json:"rewrite,omitempty"` // Whitelist indicates only connections from certain client // addresses or networks are allowed. // +optional diff --git a/core/pkg/ingress/types_equals.go b/core/pkg/ingress/types_equals.go index 81cd4409a..5244e6f86 100644 --- a/core/pkg/ingress/types_equals.go +++ b/core/pkg/ingress/types_equals.go @@ -361,6 +361,9 @@ func (l1 *Location) Equal(l2 *Location) bool { if !(&l1.Redirect).Equal(&l2.Redirect) { return false } + if !(&l1.Rewrite).Equal(&l2.Rewrite) { + return false + } if !(&l1.Whitelist).Equal(&l2.Whitelist) { return false }