Skip to content

Commit

Permalink
Merge pull request #75 from thetechnick/hsts
Browse files Browse the repository at this point in the history
Added hsts settings, Refactored bool/integer parsing
  • Loading branch information
pleshakov authored Nov 23, 2016
2 parents 8e42b83 + faf07d5 commit 69a005a
Show file tree
Hide file tree
Showing 7 changed files with 294 additions and 38 deletions.
49 changes: 36 additions & 13 deletions nginx-controller/controller/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package controller
import (
"fmt"
"reflect"
"strconv"
"strings"
"time"

Expand Down Expand Up @@ -325,22 +324,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
Expand Down
4 changes: 4 additions & 0 deletions nginx-controller/nginx/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -23,5 +26,6 @@ func NewDefaultConfig() *Config {
ClientMaxBodySize: "1m",
MainServerNamesHashMaxSize: "512",
ProxyBuffering: true,
HSTSMaxAge: 2592000,
}
}
63 changes: 46 additions & 17 deletions nginx-controller/nginx/configurator.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package nginx

import (
"fmt"
"strconv"
"strings"
"sync"

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand Down
46 changes: 46 additions & 0 deletions nginx-controller/nginx/convert.go
Original file line number Diff line number Diff line change
@@ -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
}
149 changes: 149 additions & 0 deletions nginx-controller/nginx/convert_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
}
4 changes: 3 additions & 1 deletion nginx-controller/nginx/ingress.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -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}} {
Expand Down
Loading

0 comments on commit 69a005a

Please sign in to comment.