ingress: adds configurable SSL redirect nginx controller

* add global value to config map
* add per ingress value as annotation to ingress resources

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
-

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
 

controllers/nginx/controller.go

@@ -692,6 +692,11 @@ func (lbc *loadBalancerController) getUpstreamServers(ngxCfg config.Configuratio
 				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
@@ -721,13 +726,8 @@ func (lbc *loadBalancerController) getUpstreamServers(ngxCfg config.Configuratio
 						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
@@ -742,10 +742,6 @@ func (lbc *loadBalancerController) getUpstreamServers(ngxCfg config.Configuratio
 				}
 
 				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,

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 }}

controllers/nginx/nginx/rewrite/main.go

@@ -21,22 +21,36 @@ import (
 	"strconv"
 
 	"k8s.io/kubernetes/pkg/apis/extensions"
+
+	"k8s.io/contrib/ingress/controllers/nginx/nginx/config"
 )
 
 const (
-	rewriteTo  = "ingress.kubernetes.io/rewrite-target"
+	rewriteTo   = "ingress.kubernetes.io/rewrite-target"
-	addBaseURL = "ingress.kubernetes.io/add-base-url"
+	addBaseURL  = "ingress.kubernetes.io/add-base-url"
+	sslRedirect = "ingress.kubernetes.io/ssl-redirect"
 )
 
-// Redirect returns authentication configuration for an Ingress rule
+// Redirect describes the per location redirect config
 type Redirect struct {
 	// Target URI where the traffic must be redirected
 	Target string
 	// AddBaseURL indicates if is required to add a base tag in the head
 	// of the responses from the upstream servers
 	AddBaseURL bool
+	// Should indicates if the location section should be accessible SSL only
+	SSLRedirect bool
 }
 
+var (
+	// ErrMissingSSLRedirect returned error when the ingress does not contains the
+	// ssl-redirect annotation
+	ErrMissingSSLRedirect = errors.New("ssl-redirect annotations is missing")
+
+	// ErrInvalidBool gets returned when the str value is not convertible to a bool
+	ErrInvalidBool = errors.New("ssl-redirect annotations has invalid value")
+)
+
 type ingAnnotations map[string]string
 
 func (a ingAnnotations) addBaseURL() bool {
@@ -57,17 +71,39 @@ func (a ingAnnotations) rewriteTo() string {
 	return ""
 }
 
+func (a ingAnnotations) sslRedirect() (bool, error) {
+	val, ok := a[sslRedirect]
+	if !ok {
+		return false, ErrMissingSSLRedirect
+	}
+
+	sr, err := strconv.ParseBool(val)
+	if err != nil {
+		return false, ErrInvalidBool
+	}
+
+	return sr, nil
+}
+
 // ParseAnnotations parses the annotations contained in the ingress
 // rule used to rewrite the defined paths
-func ParseAnnotations(ing *extensions.Ingress) (*Redirect, error) {
+func ParseAnnotations(cfg config.Configuration, ing *extensions.Ingress) (*Redirect, error) {
 	if ing.GetAnnotations() == nil {
 		return &Redirect{}, errors.New("no annotations present")
 	}
 
-	rt := ingAnnotations(ing.GetAnnotations()).rewriteTo()
+	annotations := ingAnnotations(ing.GetAnnotations())
-	abu := ingAnnotations(ing.GetAnnotations()).addBaseURL()
+
+	sslRe, err := annotations.sslRedirect()
+	if err != nil {
+		sslRe = cfg.SSLRedirect
+	}
+
+	rt := annotations.rewriteTo()
+	abu := annotations.addBaseURL()
 	return &Redirect{
-		Target:     rt,
+		Target:      rt,
-		AddBaseURL: abu,
+		AddBaseURL:  abu,
+		SSLRedirect: sslRe,
 	}, nil
 }

controllers/nginx/nginx/rewrite/main_test.go

@@ -118,3 +118,28 @@ func TestRedirect(t *testing.T) {
 		t.Errorf("Expected %v as redirect but returned %s", defRoute, redirect.Target)
 	}
 }
+
+func TestSSLRedirect(t *testing.T) {
+	ing := buildIngress()
+
+	cfg := config.Configuration{SSLRedirect: true}
+
+	data := map[string]string{}
+
+	ing.SetAnnotations(data)
+
+	redirect, _ := ParseAnnotations(cfg, ing)
+
+	if !redirect.SSLRedirect {
+		t.Errorf("Expected true but returned false")
+	}
+
+	data[sslRedirect] = "false"
+	ing.SetAnnotations(data)
+
+	redirect, _ = ParseAnnotations(cfg, ing)
+
+	if redirect.SSLRedirect {
+		t.Errorf("Expected false but returned true")
+	}
+}