Merge pull request #850 from simonswine/feature-ssl-redirect

ingress: adds configurable SSL redirect nginx controller

hack/verify-flags/exceptions.txt

@@ -2,7 +2,7 @@ ingress/controllers/nginx/README.md:Enables which HTTP codes should be passed fo
 ingress/controllers/nginx/README.md:Setting at least one code this also enables [proxy_intercept_errors](http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_intercept_errors) (required to process error_page)
 ingress/controllers/nginx/nginx.tmpl:        require("error_page")
 ingress/controllers/nginx/nginx.tmpl:    error_page {{ $errCode }} = @custom_{{ $errCode }};{{ end }}
-ingress/controllers/nginx/nginx/main.go:	// enables which HTTP codes should be passed for processing with the error_page directive
+ingress/controllers/nginx/nginx/config/config.go:	// enables which HTTP codes should be passed for processing with the error_page directive
 mungegithub/mungers/submit-queue.go:		sq.e2e = &fake_e2e.FakeE2ETester{
 mungegithub/mungers/submit-queue.go:	fake_e2e "k8s.io/contrib/mungegithub/mungers/e2e/fake"
 mungegithub/mungers/submit-queue_test.go:	fake_e2e "k8s.io/contrib/mungegithub/mungers/e2e/fake"

ingress/controllers/nginx/Changelog.md

@@ -1,5 +1,10 @@
 Changelog
 
+### next
+
+- [X] [#1063](https://github.com/kubernetes/contrib/pull/1063) watches referenced tls secrets
+- [X] [#850](https://github.com/kubernetes/contrib/pull/850) adds configurable SSL redirect nginx controller
+
 ### 0.7
 
 - [X] [#898](https://github.com/kubernetes/contrib/pull/898) reorder locations. Location / must be the last one to avoid errors routing to subroutes
@@ -16,10 +21,3 @@ Changelog
 - [X] [#1102](https://github.com/kubernetes/contrib/pull/1102) geolocation of traffic in stats
 - [X] [#884](https://github.com/kubernetes/contrib/issues/884) support services running ssl
 - [X] [#930](https://github.com/kubernetes/contrib/issues/930) detect changes in configuration configmaps
-
-
-TODO
-
-- [ ] [#1063](https://github.com/kubernetes/contrib/pull/1063) watches referenced tls secrets
-- [ ] [#850](https://github.com/kubernetes/contrib/pull/850) adds configurable SSL redirect nginx controller
-

ingress/controllers/nginx/README.md

@@ -131,6 +131,11 @@ To disable this behavior use `hsts=false` in the NGINX ConfigMap.
 
 NGINX provides the configuration option [ssl_buffer_size](http://nginx.org/en/docs/http/ngx_http_ssl_module.html#ssl_buffer_size) to allow the optimization of the TLS record size. This improves the [Time To First Byte](https://www.igvita.com/2013/12/16/optimizing-nginx-tls-time-to-first-byte/) (TTTFB). The default value in the Ingress controller is `4k` (nginx default is `16k`);
 
+### Server-side HTTPS enforcement through redirect
+
+By default the controller redirects (301) to HTTPS if TLS is enabled for that ingress . If you want to disable that behaviour globally, you can use `ssl-redirect: "false"` in the NGINX ConfigMap.
+
+To configure this feature for specfic ingress resources, you can use the `ingress.kubernetes.io/ssl-redirect: "false"` annotation in theparticular resource.
 
 ## Proxy Protocol
 

ingress/controllers/nginx/controller.go

@@ -42,6 +42,7 @@ import (
 
 	"k8s.io/contrib/ingress/controllers/nginx/nginx"
 	"k8s.io/contrib/ingress/controllers/nginx/nginx/auth"
+	"k8s.io/contrib/ingress/controllers/nginx/nginx/config"
 	"k8s.io/contrib/ingress/controllers/nginx/nginx/healthcheck"
 	"k8s.io/contrib/ingress/controllers/nginx/nginx/ratelimit"
 	"k8s.io/contrib/ingress/controllers/nginx/nginx/rewrite"
@@ -647,7 +648,7 @@ func (lbc *loadBalancerController) getDefaultUpstream() *nginx.Upstream {
 	return upstream
 }
 
-func (lbc *loadBalancerController) getUpstreamServers(ngxCfg nginx.Configuration, data []interface{}) ([]*nginx.Upstream, []*nginx.Server) {
+func (lbc *loadBalancerController) getUpstreamServers(ngxCfg config.Configuration, data []interface{}) ([]*nginx.Upstream, []*nginx.Server) {
 	upstreams := lbc.createUpstreams(ngxCfg, data)
 	upstreams[defUpstreamName] = lbc.getDefaultUpstream()
 
@@ -691,6 +692,11 @@ func (lbc *loadBalancerController) getUpstreamServers(ngxCfg nginx.Configuration
 				glog.V(3).Infof("error reading secure upstream in Ingress %v/%v: %v", ing.GetNamespace(), ing.GetName(), err)
 			}
 
+			locRew, err := rewrite.ParseAnnotations(ngxCfg, ing)
+			if err != nil {
+				glog.V(3).Infof("error parsing rewrite annotations for Ingress rule %v/%v: %v", ing.GetNamespace(), ing.GetName(), err)
+			}
+
 			host := rule.Host
 			if host == "" {
 				host = defServerName
@@ -720,13 +726,8 @@ func (lbc *loadBalancerController) getUpstreamServers(ngxCfg nginx.Configuration
 						loc.Upstream = *ups
 						loc.Auth = *nginxAuth
 						loc.RateLimit = *rl
-						loc.SecureUpstream = secUpstream
-
-						locRew, err := rewrite.ParseAnnotations(ing)
-						if err != nil {
-							glog.V(3).Infof("error parsing rewrite annotations for Ingress rule %v/%v: %v", ing.GetNamespace(), ing.GetName(), err)
-						}
 						loc.Redirect = *locRew
+						loc.SecureUpstream = secUpstream
 
 						addLoc = false
 						continue
@@ -741,10 +742,6 @@ func (lbc *loadBalancerController) getUpstreamServers(ngxCfg nginx.Configuration
 				}
 
 				if addLoc {
-					locRew, err := rewrite.ParseAnnotations(ing)
-					if err != nil {
-						glog.V(3).Infof("error parsing rewrite annotations for Ingress rule %v/%v: %v", ing.GetNamespace(), ing.GetName(), err)
-					}
 
 					server.Locations = append(server.Locations, &nginx.Location{
 						Path:           nginxPath,
@@ -785,7 +782,7 @@ func (lbc *loadBalancerController) getUpstreamServers(ngxCfg nginx.Configuration
 
 // createUpstreams creates the NGINX upstreams for each service referenced in
 // Ingress rules. The servers inside the upstream are endpoints.
-func (lbc *loadBalancerController) createUpstreams(ngxCfg nginx.Configuration, data []interface{}) map[string]*nginx.Upstream {
+func (lbc *loadBalancerController) createUpstreams(ngxCfg config.Configuration, data []interface{}) map[string]*nginx.Upstream {
 	upstreams := make(map[string]*nginx.Upstream)
 
 	for _, ingIf := range data {

ingress/controllers/nginx/nginx.tmpl

@@ -172,10 +172,6 @@ http {
         {{- end }}
 
         {{ if (and $server.SSL $cfg.hsts) -}}
-        if ($scheme = http) {
-            return 301 https://$host$request_uri;
-        }
-
         more_set_headers                            "Strict-Transport-Security: max-age={{ $cfg.hstsMaxAge }}{{ if $cfg.hstsIncludeSubdomains }}; includeSubDomains{{ end }}; preload";
         {{- end }}
 
@@ -184,6 +180,12 @@ http {
         {{- range $location := $server.Locations }}
         {{ $path := buildLocation $location }}
         location {{ $path }} {
+            {{ if (and $server.SSL $location.Redirect.SSLRedirect) -}}
+            # enforce ssl on server side
+            if ($scheme = http) {
+                return 301 https://$host$request_uri;
+            }
+            {{- end }}
             {{/* if the location contains a rate limit annotation, create one */}}
             {{ $limits := buildRateLimit $location }}
             {{- range $limit := $limits }}

ingress/controllers/nginx/nginx/command.go

@@ -25,6 +25,8 @@ import (
 	"github.com/golang/glog"
 
 	"k8s.io/kubernetes/pkg/healthz"
+
+	"k8s.io/contrib/ingress/controllers/nginx/nginx/config"
 )
 
 // Start starts a nginx (master process) and waits. If the process ends
@@ -54,7 +56,7 @@ func (ngx *Manager) Start() {
 // shut down, stop accepting new connections and continue to service current requests
 // until all such requests are serviced. After that, the old worker processes exit.
 // http://nginx.org/en/docs/beginners_guide.html#control
-func (ngx *Manager) CheckAndReload(cfg Configuration, ingressCfg IngressConfig) {
+func (ngx *Manager) CheckAndReload(cfg config.Configuration, ingressCfg IngressConfig) {
 	ngx.reloadRateLimiter.Accept()
 
 	ngx.reloadLock.Lock()