Added hsts settings, Refactored bool/integer parsing

nginx-controller/controller/controller.go

@@ -19,7 +19,6 @@ package controller
 import (
 	"fmt"
 	"reflect"
-	"strconv"
 	"strings"
 	"time"
 
@@ -322,22 +321,46 @@ func (lbc *LoadBalancerController) syncCfgm(key string) {
 		if serverNamesHashMaxSize, exists := cfgm.Data["server-names-hash-max-size"]; exists {
 			cfg.MainServerNamesHashMaxSize = serverNamesHashMaxSize
 		}
-		if HTTP2Str, exists := cfgm.Data["http2"]; exists {
-			if HTTP2, err := strconv.ParseBool(HTTP2Str); err == nil {
-				cfg.HTTP2 = HTTP2
-			} else {
-				glog.Errorf("In configmap %v/%v 'http2' contains invalid declaration: %v, ignoring", cfgm.Namespace, cfgm.Name, err)
-			}
+		HTTP2, err := nginx.GetMapKeyAsBool(cfgm.Data, "http2", cfgm)
+		if err != nil && err != nginx.ErrorKeyNotFound {
+			glog.Error(err)
+		} else {
+			cfg.HTTP2 = HTTP2
+		}
+
+		// HSTS block
+		HSTSErrors := false
+		HSTS, err := nginx.GetMapKeyAsBool(cfgm.Data, "hsts", cfgm)
+		if err != nil && err != nginx.ErrorKeyNotFound {
+			glog.Error(err)
+			HSTSErrors = true
+		}
+		HSTSMaxAge, err := nginx.GetMapKeyAsInt(cfgm.Data, "hsts-max-age", cfgm)
+		if err != nil && err != nginx.ErrorKeyNotFound {
+			glog.Error(err)
+			HSTSErrors = true
+		}
+		HSTSIncludeSubdomains, err := nginx.GetMapKeyAsBool(cfgm.Data, "hsts-include-subdomains", cfgm)
+		if err != nil && err != nginx.ErrorKeyNotFound {
+			glog.Error(err)
+			HSTSErrors = true
 		}
+		if HSTSErrors {
+			glog.Warningf("Configmap %s/%s: There are configuration issues with hsts annotations, skipping options for all hsts settings", cfgm.GetNamespace(), cfgm.GetName())
+		} else {
+			cfg.HSTS = HSTS
+			cfg.HSTSMaxAge = HSTSMaxAge
+			cfg.HSTSIncludeSubdomains = HSTSIncludeSubdomains
+		}
+
 		if logFormat, exists := cfgm.Data["log-format"]; exists {
 			cfg.MainLogFormat = logFormat
 		}
-		if proxyBufferingStr, exists := cfgm.Data["proxy-buffering"]; exists {
-			if ProxyBuffering, err := strconv.ParseBool(proxyBufferingStr); err == nil {
-				cfg.ProxyBuffering = ProxyBuffering
-			} else {
-				glog.Errorf("In configmap %v/%v 'proxy-buffering' contains invalid declaration: %v, ignoring", cfgm.Namespace, cfgm.Name, err)
-			}
+		ProxyBuffering, err := nginx.GetMapKeyAsBool(cfgm.Data, "proxy-buffering", cfgm)
+		if err != nil && err != nginx.ErrorKeyNotFound {
+			glog.Error(err)
+		} else {
+			cfg.ProxyBuffering = ProxyBuffering
 		}
 		if proxyBuffers, exists := cfgm.Data["proxy-buffers"]; exists {
 			cfg.ProxyBuffers = proxyBuffers

nginx-controller/nginx/config.go

@@ -13,6 +13,9 @@ type Config struct {
 	ProxyBuffers                  string
 	ProxyBufferSize               string
 	ProxyMaxTempFileSize          string
+	HSTS                          bool
+	HSTSMaxAge                    int64
+	HSTSIncludeSubdomains         bool
 }
 
 // NewDefaultConfig creates a Config with default values
@@ -23,5 +26,6 @@ func NewDefaultConfig() *Config {
 		ClientMaxBodySize:          "1m",
 		MainServerNamesHashMaxSize: "512",
 		ProxyBuffering:             true,
+		HSTSMaxAge:                 2592000,
 	}
 }

nginx-controller/nginx/configurator.go

@@ -2,7 +2,6 @@ package nginx
 
 import (
 	"fmt"
-	"strconv"
 	"strings"
 	"sync"
 
@@ -104,8 +103,11 @@ func (cnf *Configurator) generateNginxCfg(ingEx *IngressEx, pems map[string]stri
 		}
 
 		server := Server{
-			Name:  serverName,
-			HTTP2: ingCfg.HTTP2,
+			Name:                  serverName,
+			HTTP2:                 ingCfg.HTTP2,
+			HSTS:                  ingCfg.HSTS,
+			HSTSMaxAge:            ingCfg.HSTSMaxAge,
+			HSTSIncludeSubdomains: ingCfg.HSTSIncludeSubdomains,
 		}
 
 		if pemFile, ok := pems[serverName]; ok {
@@ -145,8 +147,11 @@ func (cnf *Configurator) generateNginxCfg(ingEx *IngressEx, pems map[string]stri
 
 	if len(ingEx.Ingress.Spec.Rules) == 0 && ingEx.Ingress.Spec.Backend != nil {
 		server := Server{
-			Name:  emptyHost,
-			HTTP2: ingCfg.HTTP2,
+			Name:                  emptyHost,
+			HTTP2:                 ingCfg.HTTP2,
+			HSTS:                  ingCfg.HSTS,
+			HSTSMaxAge:            ingCfg.HSTSMaxAge,
+			HSTSIncludeSubdomains: ingCfg.HSTSIncludeSubdomains,
 		}
 
 		if pemFile, ok := pems[emptyHost]; ok {
@@ -180,20 +185,44 @@ func (cnf *Configurator) createConfig(ingEx *IngressEx) Config {
 	if clientMaxBodySize, exists := ingEx.Ingress.Annotations["nginx.org/client-max-body-size"]; exists {
 		ingCfg.ClientMaxBodySize = clientMaxBodySize
 	}
-	if HTTP2Str, exists := ingEx.Ingress.Annotations["nginx.org/http2"]; exists {
-		if HTTP2, err := strconv.ParseBool(HTTP2Str); err == nil {
-			ingCfg.HTTP2 = HTTP2
-		} else {
-			glog.Errorf("In %v/%v nginx.org/http2 contains invalid declaration: %v, ignoring", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err)
-		}
+	HTTP2, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "nginx.org/http2", ingEx.Ingress)
+	if err != nil && err != ErrorKeyNotFound {
+		glog.Error(err)
+	} else {
+		ingCfg.HTTP2 = HTTP2
 	}
-	if proxyBufferingStr, exists := ingEx.Ingress.Annotations["nginx.org/proxy-buffering"]; exists {
-		if ProxyBuffering, err := strconv.ParseBool(proxyBufferingStr); err == nil {
-			ingCfg.ProxyBuffering = ProxyBuffering
-		} else {
-			glog.Errorf("In %v/%v nginx.org/proxy-buffering contains invalid declaration: %v, ignoring", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err)
-		}
+	ProxyBuffering, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "nginx.org/proxy-buffering", ingEx.Ingress)
+	if err != nil && err != ErrorKeyNotFound {
+		glog.Error(err)
+	} else {
+		ingCfg.ProxyBuffering = ProxyBuffering
+	}
+
+	// HSTS block
+	HSTSErrors := false
+	HSTS, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "nginx.org/hsts", ingEx.Ingress)
+	if err != nil && err != ErrorKeyNotFound {
+		glog.Error(err)
+		HSTSErrors = true
 	}
+	HSTSMaxAge, err := GetMapKeyAsInt(ingEx.Ingress.Annotations, "nginx.org/hsts-max-age", ingEx.Ingress)
+	if err != nil && err != ErrorKeyNotFound {
+		glog.Error(err)
+		HSTSErrors = true
+	}
+	HSTSIncludeSubdomains, err := GetMapKeyAsBool(ingEx.Ingress.Annotations, "nginx.org/hsts-include-subdomains", ingEx.Ingress)
+	if err != nil && err != ErrorKeyNotFound {
+		glog.Error(err)
+		HSTSErrors = true
+	}
+	if HSTSErrors {
+		glog.Warningf("Ingress %s/%s: There are configuration issues with hsts annotations, skipping annotions for all hsts settings", ingEx.Ingress.GetNamespace(), ingEx.Ingress.GetName())
+	} else {
+		ingCfg.HSTS = HSTS
+		ingCfg.HSTSMaxAge = HSTSMaxAge
+		ingCfg.HSTSIncludeSubdomains = HSTSIncludeSubdomains
+	}
+
 	if proxyBuffers, exists := ingEx.Ingress.Annotations["nginx.org/proxy-buffers"]; exists {
 		ingCfg.ProxyBuffers = proxyBuffers
 	}

nginx-controller/nginx/convert.go

@@ -0,0 +1,46 @@
+package nginx
+
+import (
+	"errors"
+	"fmt"
+	"strconv"
+
+	"k8s.io/kubernetes/pkg/api/meta"
+	"k8s.io/kubernetes/pkg/runtime"
+)
+
+var (
+	// ErrorKeyNotFound is returned if the key was not found in the map
+	ErrorKeyNotFound = errors.New("Key not found in map")
+)
+
+// There seems to be no composite interface in the kubernetes api package,
+// so we have to declare our own.
+type apiObject interface {
+	meta.Object
+	runtime.Object
+}
+
+// GetMapKeyAsBool searches the map for the given key and parses the key as bool
+func GetMapKeyAsBool(m map[string]string, key string, context apiObject) (bool, error) {
+	if str, exists := m[key]; exists {
+		b, err := strconv.ParseBool(str)
+		if err != nil {
+			return false, fmt.Errorf("%s %v/%v '%s' contains invalid bool: %v, ignoring", context.GetObjectKind().GroupVersionKind().Kind, context.GetNamespace(), context.GetName(), key, err)
+		}
+		return b, nil
+	}
+	return false, ErrorKeyNotFound
+}
+
+// GetMapKeyAsInt tries to find and parse a key in a map as int64
+func GetMapKeyAsInt(m map[string]string, key string, context apiObject) (int64, error) {
+	if str, exists := m[key]; exists {
+		i, err := strconv.ParseInt(str, 10, 64)
+		if err != nil {
+			return 0, fmt.Errorf("%s %v/%v '%s' contains invalid integer: %v, ignoring", context.GetObjectKind().GroupVersionKind().Kind, context.GetNamespace(), context.GetName(), key, err)
+		}
+		return i, nil
+	}
+	return 0, ErrorKeyNotFound
+}

nginx-controller/nginx/convert_test.go

@@ -0,0 +1,149 @@
+package nginx
+
+import (
+	"testing"
+
+	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/api/unversioned"
+	"k8s.io/kubernetes/pkg/apis/extensions"
+)
+
+var configMap = api.ConfigMap{
+	ObjectMeta: api.ObjectMeta{
+		Name:      "test",
+		Namespace: "default",
+	},
+	TypeMeta: unversioned.TypeMeta{
+		Kind:       "ConfigMap",
+		APIVersion: "v1",
+	},
+}
+var ingress = extensions.Ingress{
+	ObjectMeta: api.ObjectMeta{
+		Name:      "test",
+		Namespace: "kube-system",
+	},
+	TypeMeta: unversioned.TypeMeta{
+		Kind:       "Ingress",
+		APIVersion: "extensions/v1beta1",
+	},
+}
+
+//
+// GetMapKeyAsBool
+//
+func TestGetMapKeyAsBool(t *testing.T) {
+	configMap := configMap
+	configMap.Data = map[string]string{
+		"key": "True",
+	}
+
+	b, err := GetMapKeyAsBool(configMap.Data, "key", &configMap)
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+	if b != true {
+		t.Errorf("Result should be true")
+	}
+}
+
+func TestGetMapKeyAsBoolNotFound(t *testing.T) {
+	configMap := configMap
+	configMap.Data = map[string]string{}
+
+	_, err := GetMapKeyAsBool(configMap.Data, "key", &configMap)
+	if err != ErrorKeyNotFound {
+		t.Errorf("ErrorKeyNotFound was expected, got: %v", err)
+	}
+}
+
+func TestGetMapKeyAsBoolErrorMessage(t *testing.T) {
+	cfgm := configMap
+	cfgm.Data = map[string]string{
+		"key": "string",
+	}
+
+	// Test with configmap
+	_, err := GetMapKeyAsBool(cfgm.Data, "key", &cfgm)
+	if err == nil {
+		t.Error("An error was expected")
+	}
+	expected := `ConfigMap default/test 'key' contains invalid bool: strconv.ParseBool: parsing "string": invalid syntax, ignoring`
+	if err.Error() != expected {
+		t.Errorf("The error message does not match expectations:\nGot: %v\nExpected: %v", err, expected)
+	}
+
+	// Test with ingress object
+	ingress := ingress
+	ingress.Annotations = map[string]string{
+		"key": "other_string",
+	}
+	_, err = GetMapKeyAsBool(ingress.Annotations, "key", &ingress)
+	if err == nil {
+		t.Error("An error was expected")
+	}
+	expected = `Ingress kube-system/test 'key' contains invalid bool: strconv.ParseBool: parsing "other_string": invalid syntax, ignoring`
+	if err.Error() != expected {
+		t.Errorf("The error message does not match expectations:\nGot: %v\nExpected: %v", err, expected)
+	}
+}
+
+//
+// GetMapKeyAsInt
+//
+func TestGetMapKeyAsInt(t *testing.T) {
+	configMap := configMap
+	configMap.Data = map[string]string{
+		"key": "123456789",
+	}
+
+	i, err := GetMapKeyAsInt(configMap.Data, "key", &configMap)
+	if err != nil {
+		t.Errorf("Unexpected error: %v", err)
+	}
+	var expected int64 = 123456789
+	if i != expected {
+		t.Errorf("Unexpected return value:\nGot: %v\nExpected: %v", i, expected)
+	}
+}
+
+func TestGetMapKeyAsIntNotFound(t *testing.T) {
+	configMap := configMap
+	configMap.Data = map[string]string{}
+
+	_, err := GetMapKeyAsInt(configMap.Data, "key", &configMap)
+	if err != ErrorKeyNotFound {
+		t.Errorf("ErrorKeyNotFound was expected, got: %v", err)
+	}
+}
+
+func TestGetMapKeyAsIntErrorMessage(t *testing.T) {
+	cfgm := configMap
+	cfgm.Data = map[string]string{
+		"key": "string",
+	}
+
+	// Test with configmap
+	_, err := GetMapKeyAsInt(cfgm.Data, "key", &cfgm)
+	if err == nil {
+		t.Error("An error was expected")
+	}
+	expected := `ConfigMap default/test 'key' contains invalid integer: strconv.ParseInt: parsing "string": invalid syntax, ignoring`
+	if err.Error() != expected {
+		t.Errorf("The error message does not match expectations:\nGot: %v\nExpected: %v", err, expected)
+	}
+
+	// Test with ingress object
+	ingress := ingress
+	ingress.Annotations = map[string]string{
+		"key": "other_string",
+	}
+	_, err = GetMapKeyAsInt(ingress.Annotations, "key", &ingress)
+	if err == nil {
+		t.Error("An error was expected")
+	}
+	expected = `Ingress kube-system/test 'key' contains invalid integer: strconv.ParseInt: parsing "other_string": invalid syntax, ignoring`
+	if err.Error() != expected {
+		t.Errorf("The error message does not match expectations:\nGot: %v\nExpected: %v", err, expected)
+	}
+}

nginx-controller/nginx/ingress.tmpl

@@ -21,7 +21,9 @@ server {
 	if ($scheme = http) {
 		return 301 https://$host$request_uri;
 	}
-	{{end}}
+	{{- if $server.HSTS}}
+	add_header Strict-Transport-Security "max-age={{$server.HSTSMaxAge}}; {{if $server.HSTSIncludeSubdomains}}includeSubDomains; {{end}}preload" always;{{end}}
+	{{- end}}
 
 	{{range $location := $server.Locations}}
 	location {{$location.Path}} {