From 1a72b3f775988aa388b74183df297d8c27f8290b Mon Sep 17 00:00:00 2001 From: Peter Wilson Date: Fri, 3 Mar 2017 12:44:45 +1100 Subject: [PATCH] add ForceSSLRedirect ingress annotation --- controllers/nginx/pkg/config/config.go | 1 + .../rootfs/etc/nginx/template/nginx.tmpl | 48 +++++++++---------- core/pkg/ingress/annotations/rewrite/main.go | 20 +++++--- .../ingress/annotations/rewrite/main_test.go | 33 +++++++++++-- core/pkg/ingress/defaults/main.go | 4 ++ 5 files changed, 72 insertions(+), 34 deletions(-) diff --git a/controllers/nginx/pkg/config/config.go b/controllers/nginx/pkg/config/config.go index c23c158677..0a86e15048 100644 --- a/controllers/nginx/pkg/config/config.go +++ b/controllers/nginx/pkg/config/config.go @@ -292,6 +292,7 @@ func NewDefault() Configuration { ProxyCookieDomain: "off", ProxyCookiePath: "off", SSLRedirect: true, + ForceSSLRedirect: false, CustomHTTPErrors: []int{}, WhitelistSourceRange: []string{}, SkipAccessLogURLs: []string{}, diff --git a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl index 9d29e9a5d1..3c04b295b7 100644 --- a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl +++ b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl @@ -14,7 +14,7 @@ worker_rlimit_nofile {{ .MaxOpenFiles }}; events { multi_accept on; worker_connections {{ $cfg.MaxWorkerConnections }}; - use epoll; + use epoll; } http { @@ -26,7 +26,7 @@ http { real_ip_header X-Forwarded-For; set_real_ip_from 0.0.0.0/0; {{ end }} - + real_ip_recursive on; {{/* databases used to determine the country depending on the client IP address */}} @@ -51,7 +51,7 @@ http { aio threads; tcp_nopush on; tcp_nodelay on; - + log_subrequest on; reset_timedout_connection on; @@ -73,7 +73,7 @@ http { gzip_comp_level 5; gzip_http_version 1.1; gzip_min_length 256; - gzip_types {{ $cfg.GzipTypes }}; + gzip_types {{ $cfg.GzipTypes }}; gzip_proxied any; {{ end }} @@ -241,16 +241,16 @@ http { proxy_pass_request_body off; proxy_set_header Content-Length ""; {{ end }} - {{ if not (empty $location.ExternalAuth.Method) }} + {{ if not (empty $location.ExternalAuth.Method) }} proxy_method {{ $location.ExternalAuth.Method }}; {{ end }} - proxy_set_header Host $host; + proxy_set_header Host $host; proxy_pass_request_headers on; set $target {{ $location.ExternalAuth.URL }}; proxy_pass $target; } {{ end }} - + location {{ $path }} { set $proxy_upstream_name "{{ $location.Backend }}"; @@ -260,17 +260,17 @@ http { allow {{ $ip }};{{ end }} deny all; {{ end }} - + port_in_redirect {{ if $location.UsePortInRedirects }}on{{ else }}off{{ end }}; {{ if not (empty $authPath) }} # this location requires authentication auth_request {{ $authPath }}; {{ end }} - - {{ if (and (not (empty $server.SSLCertificate)) $location.Redirect.SSLRedirect) }} + + {{ if (or $location.Redirect.ForceSSLRedirect (and (not (empty $server.SSLCertificate)) $location.Redirect.SSLRedirect)) }} # enforce ssl on server side - if ($scheme = http) { + if ($pass_access_scheme = http) { return 301 https://$host$request_uri; } {{ end }} @@ -278,7 +278,7 @@ http { {{ $limits := buildRateLimit $location }} {{ range $limit := $limits }} {{ $limit }}{{ end }} - + {{ if $location.BasicDigestAuth.Secured }} {{ if eq $location.BasicDigestAuth.Type "basic" }} auth_basic "{{ $location.BasicDigestAuth.Realm }}"; @@ -289,7 +289,7 @@ http { {{ end }} proxy_set_header Authorization ""; {{ end }} - + {{ if $location.EnableCORS }} {{ template "CORS" }} {{ end }} @@ -353,7 +353,7 @@ http { {{ end }} } {{ end }} - + {{ if eq $server.Hostname "_" }} # health checks in cloud providers require the use of port 80 location {{ $healthzURI }} { @@ -375,9 +375,9 @@ http { {{ template "CUSTOM_ERRORS" $cfg }} } - + {{ end }} - + # default server, used for NGINX healthcheck and access to nginx stats server { # Use the port 18080 (random value just to avoid known ports) as default port for nginx. @@ -389,7 +389,7 @@ http { access_log off; return 200; } - + location /nginx_status { {{ if $cfg.EnableVtsStatus }} vhost_traffic_status_display; @@ -443,7 +443,7 @@ stream { {{ range $i, $passthrough := .PassthroughBackends }} {{ $passthrough.Hostname }} {{ $passthrough.Backend }}; {{ end }} - # send SSL traffic to this nginx in a different port + # send SSL traffic to this nginx in a different port default nginx-ssl-backend; } @@ -470,15 +470,15 @@ stream { ssl_preread on; } {{ end }} - - # TCP services + + # TCP services {{ range $i, $tcpServer := .TCPBackends }} upstream {{ $tcpServer.Backend.Namespace }}-{{ $tcpServer.Backend.Name }}-{{ $tcpServer.Backend.Port }} { {{ range $j, $endpoint := $tcpServer.Endpoints }} server {{ $endpoint.Address }}:{{ $endpoint.Port }}; {{ end }} } - + server { listen {{ $tcpServer.Port }}; proxy_pass {{ $tcpServer.Backend.Namespace }}-{{ $tcpServer.Backend.Name }}-{{ $tcpServer.Backend.Port }}; @@ -492,11 +492,11 @@ stream { server {{ $endpoint.Address }}:{{ $endpoint.Port }}; {{ end }} } - + server { listen {{ $udpServer.Port }}; proxy_responses 1; - proxy_pass {{ $udpServer.Backend.Namespace }}-{{ $udpServer.Backend.Name }}-{{ $udpServer.Backend.Port }}; + proxy_pass {{ $udpServer.Backend.Namespace }}-{{ $udpServer.Backend.Name }}-{{ $udpServer.Backend.Port }}; } {{ end }} } @@ -509,7 +509,7 @@ stream { content_by_lua_block { openURL(ngx.req.get_headers(0), {{ $errCode }}) } - } + } {{ end }} {{ end }} diff --git a/core/pkg/ingress/annotations/rewrite/main.go b/core/pkg/ingress/annotations/rewrite/main.go index 14dec6616b..999ef78447 100644 --- a/core/pkg/ingress/annotations/rewrite/main.go +++ b/core/pkg/ingress/annotations/rewrite/main.go @@ -24,9 +24,10 @@ import ( ) const ( - rewriteTo = "ingress.kubernetes.io/rewrite-target" - addBaseURL = "ingress.kubernetes.io/add-base-url" - sslRedirect = "ingress.kubernetes.io/ssl-redirect" + rewriteTo = "ingress.kubernetes.io/rewrite-target" + addBaseURL = "ingress.kubernetes.io/add-base-url" + sslRedirect = "ingress.kubernetes.io/ssl-redirect" + forceSSLRedirect = "ingress.kubernetes.io/force-ssl-redirect" ) // Redirect describes the per location redirect config @@ -38,6 +39,8 @@ type Redirect struct { AddBaseURL bool `json:"addBaseUrl"` // SSLRedirect indicates if the location section is accessible SSL only SSLRedirect bool `json:"sslRedirect"` + // ForceSSLRedirect indicates if the location section is accessible SSL only + ForceSSLRedirect bool `json:"forceSSLRedirect"` } type rewrite struct { @@ -57,10 +60,15 @@ func (a rewrite) Parse(ing *extensions.Ingress) (interface{}, error) { if err != nil { sslRe = a.backendResolver.GetDefaultBackend().SSLRedirect } + fSslRe, err := parser.GetBoolAnnotation(forceSSLRedirect, ing) + if err != nil { + fSslRe = a.backendResolver.GetDefaultBackend().ForceSSLRedirect + } abu, _ := parser.GetBoolAnnotation(addBaseURL, ing) return &Redirect{ - Target: rt, - AddBaseURL: abu, - SSLRedirect: sslRe, + Target: rt, + AddBaseURL: abu, + SSLRedirect: sslRe, + ForceSSLRedirect: fSslRe, }, nil } diff --git a/core/pkg/ingress/annotations/rewrite/main_test.go b/core/pkg/ingress/annotations/rewrite/main_test.go index f4f0ed973d..75daf01bcd 100644 --- a/core/pkg/ingress/annotations/rewrite/main_test.go +++ b/core/pkg/ingress/annotations/rewrite/main_test.go @@ -117,10 +117,6 @@ func TestSSLRedirect(t *testing.T) { t.Errorf("Expected true but returned false") } - if !redirect.SSLRedirect { - t.Errorf("Expected true but returned false") - } - data[sslRedirect] = "false" ing.SetAnnotations(data) @@ -133,3 +129,32 @@ func TestSSLRedirect(t *testing.T) { t.Errorf("Expected false but returned true") } } + +func TestForceSSLRedirect(t *testing.T) { + ing := buildIngress() + + data := map[string]string{} + data[rewriteTo] = defRoute + ing.SetAnnotations(data) + + i, _ := NewParser(mockBackend{true}).Parse(ing) + redirect, ok := i.(*Redirect) + if !ok { + t.Errorf("expected a Redirect type") + } + if redirect.ForceSSLRedirect { + t.Errorf("Expected false but returned true") + } + + data[forceSSLRedirect] = "true" + ing.SetAnnotations(data) + + i, _ = NewParser(mockBackend{false}).Parse(ing) + redirect, ok = i.(*Redirect) + if !ok { + t.Errorf("expected a Redirect type") + } + if !redirect.ForceSSLRedirect { + t.Errorf("Expected true but returned false") + } +} diff --git a/core/pkg/ingress/defaults/main.go b/core/pkg/ingress/defaults/main.go index 19b6c110b0..d8420da046 100644 --- a/core/pkg/ingress/defaults/main.go +++ b/core/pkg/ingress/defaults/main.go @@ -59,6 +59,10 @@ type Backend struct { // Enables or disables the redirect (301) to the HTTPS port SSLRedirect bool `json:"ssl-redirect"` + // Enables or disables the redirect (301) to the HTTPS port even without TLS cert + // This is useful if doing SSL offloading outside of cluster eg AWS ELB + ForceSSLRedirect bool `json:"force-ssl-redirect"` + // Enables or disables the specification of port in redirects // Default: false UsePortInRedirects bool `json:"use-port-in-redirects"`