From dfc9aca04eb253e2d24a6652ee750a59e1a6858b Mon Sep 17 00:00:00 2001
From: Joao Morais <jcmoraisjr@gmail.com>
Date: Sat, 20 Oct 2018 16:37:54 -0300
Subject: [PATCH] Add annotation and configmap validations

Add validation on:
* balance algorithm annotation and configmap
* proxy body size annotation and configmap
* timeout queue
---
 README.md                                      |  2 ++
 pkg/common/ingress/annotations/balance/main.go | 18 +++++++++++++++++-
 .../ingress/annotations/connection/main.go     | 10 ++++++++++
 pkg/common/ingress/annotations/proxy/main.go   | 17 +++++++++++++++++
 pkg/common/ingress/annotations/waf/main.go     |  2 +-
 pkg/controller/config.go                       | 18 ++++++++++++++++++
 6 files changed, 65 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index a0827b37c..81f3ba5ee 100644
--- a/README.md
+++ b/README.md
@@ -621,6 +621,8 @@ resource.
 Since 0.4 a suffix can be added to the size, so `10m` means
 `10 * 1024 * 1024` bytes. Supported suffix are: `k`, `m` and `g`.
 
+Since 0.7 `unlimited` can be used to overwrite any global body size limit.
+
 http://cbonte.github.io/haproxy-dconv/1.8/configuration.html#7.3.6-req.body_size
 
 ### ssl-ciphers
diff --git a/pkg/common/ingress/annotations/balance/main.go b/pkg/common/ingress/annotations/balance/main.go
index fc1f8f910..d5d7314fc 100644
--- a/pkg/common/ingress/annotations/balance/main.go
+++ b/pkg/common/ingress/annotations/balance/main.go
@@ -17,15 +17,21 @@ limitations under the License.
 package balance
 
 import (
+	"github.com/golang/glog"
 	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/ingress/annotations/parser"
 	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/ingress/resolver"
 	extensions "k8s.io/api/extensions/v1beta1"
+	"regexp"
 )
 
 const (
 	balanceAnn = "ingress.kubernetes.io/balance-algorithm"
 )
 
+var (
+	balanceRegex = regexp.MustCompile(`^(roundrobin$|static-rr$|leastconn$|first$|source$|uri|url_param|hdr\(|rdp-cookie)`)
+)
+
 type balance struct {
 	resolver resolver.DefaultBackend
 }
@@ -38,8 +44,18 @@ func NewParser(resolver resolver.DefaultBackend) parser.IngressAnnotation {
 // Parse parses balance-algorithm annotation
 func (b balance) Parse(ing *extensions.Ingress) (interface{}, error) {
 	s, err := parser.GetStringAnnotation(balanceAnn, ing)
+	def := b.resolver.GetDefaultBackend().BalanceAlgorithm
 	if err != nil {
-		return b.resolver.GetDefaultBackend().BalanceAlgorithm, nil
+		return def, nil
+	}
+	if !balanceRegex.MatchString(s) {
+		glog.Warningf("invalid balance algorithm '%v' on %v/%v, using default: %v", s, ing.Namespace, ing.Name, def)
+		return def, nil
 	}
 	return s, nil
 }
+
+// IsValidBalance return true if b is a valid load balance algorithm
+func IsValidBalance(b string) bool {
+	return balanceRegex.MatchString(b)
+}
diff --git a/pkg/common/ingress/annotations/connection/main.go b/pkg/common/ingress/annotations/connection/main.go
index 358486ca0..6f02548b4 100644
--- a/pkg/common/ingress/annotations/connection/main.go
+++ b/pkg/common/ingress/annotations/connection/main.go
@@ -17,8 +17,10 @@ limitations under the License.
 package connection
 
 import (
+	"github.com/golang/glog"
 	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/ingress/annotations/parser"
 	extensions "k8s.io/api/extensions/v1beta1"
+	"regexp"
 )
 
 const (
@@ -27,6 +29,10 @@ const (
 	timeoutQueueAnn   = "ingress.kubernetes.io/timeout-queue"
 )
 
+var (
+	timeoutQueueRegex = regexp.MustCompile(`^([0-9]+(us|ms|[smhd])?)$`)
+)
+
 // Config is the connection configuration
 type Config struct {
 	MaxConnServer  int
@@ -56,6 +62,10 @@ func (c conn) Parse(ing *extensions.Ingress) (interface{}, error) {
 	if err != nil {
 		timeoutqueue = ""
 	}
+	if timeoutqueue != "" && !timeoutQueueRegex.MatchString(timeoutqueue) {
+		glog.Warningf("ignoring invalid timeout queue %v on %v/%v", timeoutqueue, ing.Namespace, ing.Name)
+		timeoutqueue = ""
+	}
 	return &Config{
 		MaxConnServer:  maxconn,
 		MaxQueueServer: maxqueue,
diff --git a/pkg/common/ingress/annotations/proxy/main.go b/pkg/common/ingress/annotations/proxy/main.go
index f98504627..83cd6e625 100644
--- a/pkg/common/ingress/annotations/proxy/main.go
+++ b/pkg/common/ingress/annotations/proxy/main.go
@@ -17,7 +17,9 @@ limitations under the License.
 package proxy
 
 import (
+	"github.com/golang/glog"
 	extensions "k8s.io/api/extensions/v1beta1"
+	"regexp"
 
 	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/ingress/annotations/parser"
 	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/ingress/resolver"
@@ -36,6 +38,10 @@ const (
 	requestBuffering = "ingress.kubernetes.io/proxy-request-buffering"
 )
 
+var (
+	bodySizeRegex = regexp.MustCompile(`^(|([0-9]+[kmg]?))$`)
+)
+
 // Configuration returns the proxy timeout to use in the upstream server/s
 type Configuration struct {
 	BodySize         string `json:"bodySize"`
@@ -140,6 +146,12 @@ func (a proxy) Parse(ing *extensions.Ingress) (interface{}, error) {
 	if err != nil || bs == "" {
 		bs = defBackend.ProxyBodySize
 	}
+	if !bodySizeRegex.MatchString(bs) {
+		if bs != "unlimited" {
+			glog.Warningf("ignoring invalid body size '%v' on %v/%v", bs, ing.Namespace, ing.Name)
+		}
+		bs = ""
+	}
 
 	nu, err := parser.GetStringAnnotation(nextUpstream, ing)
 	if err != nil || nu == "" {
@@ -158,3 +170,8 @@ func (a proxy) Parse(ing *extensions.Ingress) (interface{}, error) {
 
 	return &Configuration{bs, ct, st, rt, bufs, cd, cp, nu, pp, rb}, nil
 }
+
+// IsValidProxyBodySize return true if s is a valid proxy body size string
+func IsValidProxyBodySize(s string) bool {
+	return bodySizeRegex.MatchString(s)
+}
diff --git a/pkg/common/ingress/annotations/waf/main.go b/pkg/common/ingress/annotations/waf/main.go
index b9c1e7c1f..a92c9004a 100644
--- a/pkg/common/ingress/annotations/waf/main.go
+++ b/pkg/common/ingress/annotations/waf/main.go
@@ -50,7 +50,7 @@ func (w waf) Parse(ing *extensions.Ingress) (interface{}, error) {
 		return Config{}, nil
 	}
 	if !wafAnnRegex.MatchString(s) {
-		glog.Warningf("ignoring invalid WAF option: %v", s)
+		glog.Warningf("ignoring invalid WAF option '%v' on %v/%v", s, ing.Namespace, ing.Name)
 		return Config{}, nil
 	}
 	return Config{
diff --git a/pkg/controller/config.go b/pkg/controller/config.go
index 28c60e464..85cd4c4da 100644
--- a/pkg/controller/config.go
+++ b/pkg/controller/config.go
@@ -23,9 +23,11 @@ import (
 	"github.com/golang/glog"
 	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/file"
 	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/ingress"
+	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/ingress/annotations/balance"
 	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/ingress/annotations/cors"
 	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/ingress/annotations/dnsresolvers"
 	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/ingress/annotations/hsts"
+	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/ingress/annotations/proxy"
 	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/ingress/annotations/waf"
 	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/ingress/defaults"
 	"github.com/jcmoraisjr/haproxy-ingress/pkg/common/net/ssl"
@@ -162,9 +164,25 @@ func newHAProxyConfig(haproxyController *HAProxyController) *types.HAProxyConfig
 		configDHParam(haproxyController, &conf)
 		configForwardfor(&conf)
 	}
+	validateConfig(&conf)
 	return &conf
 }
 
+func validateConfig(conf *types.HAProxyConfig) {
+	b := conf.BalanceAlgorithm
+	if !balance.IsValidBalance(b) {
+		glog.Warningf("invalid default algorithm '%v', using roundrobin instead", b)
+		conf.BalanceAlgorithm = "roundrobin"
+	}
+	bs := conf.ProxyBodySize
+	if !proxy.IsValidProxyBodySize(bs) {
+		if bs != "unlimited" {
+			glog.Warningf("invalid proxy body size '%v', using unlimited", bs)
+		}
+		conf.ProxyBodySize = ""
+	}
+}
+
 // TODO Ingress core should provide this
 // read ssl-dh-param secret
 func configDHParam(haproxyController *HAProxyController, conf *types.HAProxyConfig) {