Add configuration and annotation for port_in_redirect

controllers/nginx/pkg/cmd/controller/nginx.go

@@ -29,6 +29,8 @@ import (
 	"time"
 
 	"github.com/golang/glog"
+	"github.com/mitchellh/mapstructure"
+
 	"k8s.io/kubernetes/pkg/api"
 
 	"k8s.io/ingress/controllers/nginx/pkg/config"
@@ -58,7 +60,10 @@ func newNGINXController() ingress.Controller {
 	if ngx == "" {
 		ngx = binary
 	}
-	n := NGINXController{binary: ngx}
+	n := NGINXController{
+		binary:    ngx,
+		configmap: &api.ConfigMap{},
+	}
 
 	var onChange func()
 	onChange = func() {
@@ -87,13 +92,15 @@ Error loading new template : %v
 
 	go n.Start()
 
-	return n
+	return ingress.Controller(&n)
 }
 
 // NGINXController ...
 type NGINXController struct {
 	t *ngx_template.Template
 
+	configmap *api.ConfigMap
+
 	binary string
 }
 
@@ -170,11 +177,31 @@ func (n NGINXController) Reload(data []byte) ([]byte, bool, error) {
 
 // BackendDefaults returns the nginx defaults
 func (n NGINXController) BackendDefaults() defaults.Backend {
+	if n.configmap == nil {
+		d := config.NewDefault()
+		return d.Backend
+	}
+
+	return n.backendDefaults()
+}
+
+func (n *NGINXController) backendDefaults() defaults.Backend {
 	d := config.NewDefault()
+	config := &mapstructure.DecoderConfig{
+		Metadata:         nil,
+		WeaklyTypedInput: true,
+		Result:           &d,
+		TagName:          "json",
+	}
+	decoder, err := mapstructure.NewDecoder(config)
+	if err != nil {
+		glog.Warningf("unexpected error merging defaults: %v", err)
+	}
+	decoder.Decode(n.configmap.Data)
 	return d.Backend
 }
 
-// IsReloadRequired check if the new configuration file is different
+// isReloadRequired check if the new configuration file is different
 // from the current one.
 func (n NGINXController) isReloadRequired(data []byte) bool {
 	in, err := os.Open(cfgPath)
@@ -249,6 +276,11 @@ Error: %v
 	return nil
 }
 
+// SetConfig ...
+func (n *NGINXController) SetConfig(cmap *api.ConfigMap) {
+	n.configmap = cmap
+}
+
 // OnUpdate is called by syncQueue in https://github.com/aledbf/ingress-controller/blob/master/pkg/ingress/controller/controller.go#L82
 // periodically to keep the configuration in sync.
 //
@@ -257,7 +289,7 @@ Error: %v
 // write the configuration file
 // returning nill implies the backend will be reloaded.
 // if an error is returned means requeue the update
-func (n NGINXController) OnUpdate(cmap *api.ConfigMap, ingressCfg ingress.Configuration) ([]byte, error) {
+func (n *NGINXController) OnUpdate(ingressCfg ingress.Configuration) ([]byte, error) {
 	var longestName int
 	var serverNames int
 	for _, srv := range ingressCfg.Servers {
@@ -267,7 +299,7 @@ func (n NGINXController) OnUpdate(cmap *api.ConfigMap, ingressCfg ingress.Config
 		}
 	}
 
-	cfg := ngx_template.ReadConfig(cmap)
+	cfg := ngx_template.ReadConfig(n.configmap.Data)
 
 	// NGINX cannot resize the has tables used to store server names.
 	// For this reason we check if the defined size defined is correct

controllers/nginx/pkg/config/config.go

@@ -270,6 +270,7 @@ func NewDefault() Configuration {
 			CustomHTTPErrors:     []int{},
 			WhitelistSourceRange: []string{},
 			SkipAccessLogURLs:    []string{},
+			UsePortInRedirects:   false,
 		},
 	}
 

controllers/nginx/pkg/template/configmap.go

@@ -23,8 +23,6 @@ import (
 	"github.com/golang/glog"
 	"github.com/mitchellh/mapstructure"
 
-	"k8s.io/kubernetes/pkg/api"
-
 	"k8s.io/ingress/controllers/nginx/pkg/config"
 	"k8s.io/ingress/core/pkg/net/dns"
 )
@@ -36,13 +34,21 @@ const (
 )
 
 // ReadConfig obtains the configuration defined by the user merged with the defaults.
-func ReadConfig(conf *api.ConfigMap) config.Configuration {
+func ReadConfig(src map[string]string) config.Configuration {
+	conf := map[string]string{}
+	if src != nil {
+		// we need to copy the configmap data because the content is altered
+		for k, v := range src {
+			conf[k] = v
+		}
+	}
+
 	errors := make([]int, 0)
 	skipUrls := make([]string, 0)
 	whitelist := make([]string, 0)
 
-	if val, ok := conf.Data[customHTTPErrors]; ok {
+	if val, ok := conf[customHTTPErrors]; ok {
-		delete(conf.Data, customHTTPErrors)
+		delete(conf, customHTTPErrors)
 		for _, i := range strings.Split(val, ",") {
 			j, err := strconv.Atoi(i)
 			if err != nil {
@@ -52,12 +58,12 @@ func ReadConfig(conf *api.ConfigMap) config.Configuration {
 			}
 		}
 	}
-	if val, ok := conf.Data[skipAccessLogUrls]; ok {
+	if val, ok := conf[skipAccessLogUrls]; ok {
-		delete(conf.Data, skipAccessLogUrls)
+		delete(conf, skipAccessLogUrls)
 		skipUrls = strings.Split(val, ",")
 	}
-	if val, ok := conf.Data[whitelistSourceRange]; ok {
+	if val, ok := conf[whitelistSourceRange]; ok {
-		delete(conf.Data, whitelistSourceRange)
+		delete(conf, whitelistSourceRange)
 		whitelist = append(whitelist, strings.Split(val, ",")...)
 	}
 
@@ -84,7 +90,7 @@ func ReadConfig(conf *api.ConfigMap) config.Configuration {
 	if err != nil {
 		glog.Warningf("unexpected error merging defaults: %v", err)
 	}
-	err = decoder.Decode(conf.Data)
+	err = decoder.Decode(conf)
 	if err != nil {
 		glog.Warningf("unexpected error merging defaults: %v", err)
 	}

controllers/nginx/pkg/template/configmap_test.go

@@ -23,7 +23,6 @@ import (
 
 	"k8s.io/ingress/controllers/nginx/pkg/config"
 	"k8s.io/ingress/core/pkg/net/dns"
-	"k8s.io/kubernetes/pkg/api"
 )
 
 func TestFilterErrors(t *testing.T) {
@@ -34,17 +33,15 @@ func TestFilterErrors(t *testing.T) {
 }
 
 func TestMergeConfigMapToStruct(t *testing.T) {
-	conf := &api.ConfigMap{
+	conf := map[string]string{
-		Data: map[string]string{
+		"custom-http-errors":         "300,400,demo",
-			"custom-http-errors":         "300,400,demo",
+		"proxy-read-timeout":         "1",
-			"proxy-read-timeout":         "1",
+		"proxy-send-timeout":         "2",
-			"proxy-send-timeout":         "2",
+		"skip-access-log-urls":       "/log,/demo,/test",
-			"skip-access-log-urls":       "/log,/demo,/test",
+		"use-proxy-protocol":         "true",
-			"use-proxy-protocol":         "true",
+		"use-gzip":                   "true",
-			"use-gzip":                   "true",
+		"enable-dynamic-tls-records": "false",
-			"enable-dynamic-tls-records": "false",
+		"gzip-types":                 "text/html",
-			"gzip-types":                 "text/html",
-		},
 	}
 	def := config.NewDefault()
 	def.CustomHTTPErrors = []int{300, 400}
@@ -68,7 +65,7 @@ func TestMergeConfigMapToStruct(t *testing.T) {
 
 	def = config.NewDefault()
 	def.Resolver = h
-	to = ReadConfig(&api.ConfigMap{})
+	to = ReadConfig(map[string]string{})
 	if diff := pretty.Compare(to, def); diff != "" {
 		t.Errorf("unexpected diff: (-got +want)\n%s", diff)
 	}
@@ -76,10 +73,8 @@ func TestMergeConfigMapToStruct(t *testing.T) {
 	def = config.NewDefault()
 	def.Resolver = h
 	def.WhitelistSourceRange = []string{"1.1.1.1/32"}
-	to = ReadConfig(&api.ConfigMap{
+	to = ReadConfig(map[string]string{
-		Data: map[string]string{
+		"whitelist-source-range": "1.1.1.1/32",
-			"whitelist-source-range": "1.1.1.1/32",
-		},
 	})
 
 	if diff := pretty.Compare(to, def); diff != "" {

controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl

@@ -251,6 +251,8 @@ http {
             deny all;
             {{ end }}
             
+            port_in_redirect {{ if $location.UsePortInRedirects }}on{{ else }}off{{ end }};
+
             {{ if not (empty $authPath) }}
             # this location requires authentication
             auth_request {{ $authPath }};

core/pkg/ingress/annotations/parser/main.go

@@ -36,7 +36,7 @@ func (a ingAnnotations) parseBool(name string) (bool, error) {
 	if ok {
 		b, err := strconv.ParseBool(val)
 		if err != nil {
-			return false, errors.NewInvalidAnnotationContent(name)
+			return false, errors.NewInvalidAnnotationContent(name, val)
 		}
 		return b, nil
 	}
@@ -56,7 +56,7 @@ func (a ingAnnotations) parseInt(name string) (int, error) {
 	if ok {
 		i, err := strconv.Atoi(val)
 		if err != nil {
-			return 0, errors.NewInvalidAnnotationContent(name)
+			return 0, errors.NewInvalidAnnotationContent(name, val)
 		}
 		return i, nil
 	}

core/pkg/ingress/annotations/portinredirect/main.go

@@ -14,28 +14,35 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package cors
+package portinredirect
 
 import (
 	"k8s.io/kubernetes/pkg/apis/extensions"
 
 	"k8s.io/ingress/core/pkg/ingress/annotations/parser"
+	"k8s.io/ingress/core/pkg/ingress/resolver"
 )
 
 const (
-	annotation = "ingress.kubernetes.io/port-in-redirect"
+	annotation = "ingress.kubernetes.io/use-port-in-redirects"
 )
 
 type portInRedirect struct {
+	backendResolver resolver.DefaultBackend
 }
 
 // NewParser creates a new port in redirect annotation parser
-func NewParser() parser.IngressAnnotation {
+func NewParser(db resolver.DefaultBackend) parser.IngressAnnotation {
-	return portInRedirect{}
+	return portInRedirect{db}
 }
 
 // Parse parses the annotations contained in the ingress
 // rule used to indicate if the redirects must
 func (a portInRedirect) Parse(ing *extensions.Ingress) (interface{}, error) {
-	return parser.GetBoolAnnotation(annotation, ing)
+	up, err := parser.GetBoolAnnotation(annotation, ing)
+	if err != nil {
+		return a.backendResolver.GetDefaultBackend().UsePortInRedirects, nil
+	}
+
+	return up, nil
 }

core/pkg/ingress/annotations/portinredirect/main_test.go

@@ -0,0 +1,120 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package portinredirect
+
+import (
+	"testing"
+
+	"k8s.io/kubernetes/pkg/api"
+	"k8s.io/kubernetes/pkg/apis/extensions"
+	"k8s.io/kubernetes/pkg/util/intstr"
+
+	"fmt"
+
+	"k8s.io/ingress/core/pkg/ingress/defaults"
+)
+
+func buildIngress() *extensions.Ingress {
+	defaultBackend := extensions.IngressBackend{
+		ServiceName: "default-backend",
+		ServicePort: intstr.FromInt(80),
+	}
+
+	return &extensions.Ingress{
+		ObjectMeta: api.ObjectMeta{
+			Name:      "foo",
+			Namespace: api.NamespaceDefault,
+		},
+		Spec: extensions.IngressSpec{
+			Backend: &extensions.IngressBackend{
+				ServiceName: "default-backend",
+				ServicePort: intstr.FromInt(80),
+			},
+			Rules: []extensions.IngressRule{
+				{
+					Host: "foo.bar.com",
+					IngressRuleValue: extensions.IngressRuleValue{
+						HTTP: &extensions.HTTPIngressRuleValue{
+							Paths: []extensions.HTTPIngressPath{
+								{
+									Path:    "/foo",
+									Backend: defaultBackend,
+								},
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+}
+
+type mockBackend struct {
+	usePortInRedirects bool
+}
+
+func (m mockBackend) GetDefaultBackend() defaults.Backend {
+	return defaults.Backend{UsePortInRedirects: m.usePortInRedirects}
+}
+
+func TestPortInRedirect(t *testing.T) {
+	tests := []struct {
+		title   string
+		usePort *bool
+		def     bool
+		exp     bool
+	}{
+		{"false - default false", newFalse(), false, false},
+		{"false - default true", newFalse(), true, false},
+		{"no annotation - default false", nil, false, false},
+		{"no annotation - default true", nil, true, true},
+		{"true - default true", newTrue(), true, true},
+	}
+
+	for _, test := range tests {
+		ing := buildIngress()
+
+		data := map[string]string{}
+		if test.usePort != nil {
+			data[annotation] = fmt.Sprintf("%v", *test.usePort)
+		}
+		ing.SetAnnotations(data)
+
+		i, err := NewParser(mockBackend{test.def}).Parse(ing)
+		if err != nil {
+			t.Errorf("unexpected error parsing a valid")
+		}
+		p, ok := i.(bool)
+		if !ok {
+			t.Errorf("expected a bool type")
+		}
+
+		if p != test.exp {
+			t.Errorf("%v: expected \"%v\" but \"%v\" was returned", test.title, test.exp, p)
+		}
+	}
+}
+
+func newTrue() *bool {
+	b := true
+	return &b
+}
+
+func newFalse() *bool {
+	b := false
+	return &b
+}

core/pkg/ingress/controller/annotations.go

@@ -28,6 +28,7 @@ import (
 	"k8s.io/ingress/core/pkg/ingress/annotations/healthcheck"
 	"k8s.io/ingress/core/pkg/ingress/annotations/ipwhitelist"
 	"k8s.io/ingress/core/pkg/ingress/annotations/parser"
+	"k8s.io/ingress/core/pkg/ingress/annotations/portinredirect"
 	"k8s.io/ingress/core/pkg/ingress/annotations/proxy"
 	"k8s.io/ingress/core/pkg/ingress/annotations/ratelimit"
 	"k8s.io/ingress/core/pkg/ingress/annotations/rewrite"
@@ -50,17 +51,18 @@ type annotationExtractor struct {
 func newAnnotationExtractor(cfg extractorConfig) annotationExtractor {
 	return annotationExtractor{
 		map[string]parser.IngressAnnotation{
-			"BasicDigestAuth": auth.NewParser(auth.AuthDirectory, cfg),
+			"BasicDigestAuth":    auth.NewParser(auth.AuthDirectory, cfg),
-			"ExternalAuth":    authreq.NewParser(),
+			"ExternalAuth":       authreq.NewParser(),
-			"CertificateAuth": authtls.NewParser(cfg),
+			"CertificateAuth":    authtls.NewParser(cfg),
-			"EnableCORS":      cors.NewParser(),
+			"EnableCORS":         cors.NewParser(),
-			"HealthCheck":     healthcheck.NewParser(cfg),
+			"HealthCheck":        healthcheck.NewParser(cfg),
-			"Whitelist":       ipwhitelist.NewParser(cfg),
+			"Whitelist":          ipwhitelist.NewParser(cfg),
-			"Proxy":           proxy.NewParser(cfg),
+			"UsePortInRedirects": portinredirect.NewParser(cfg),
-			"RateLimit":       ratelimit.NewParser(),
+			"Proxy":              proxy.NewParser(cfg),
-			"Redirect":        rewrite.NewParser(cfg),
+			"RateLimit":          ratelimit.NewParser(),
-			"SecureUpstream":  secureupstream.NewParser(),
+			"Redirect":           rewrite.NewParser(cfg),
-			"SSLPassthrough":  sslpassthrough.NewParser(),
+			"SecureUpstream":     secureupstream.NewParser(),
+			"SSLPassthrough":     sslpassthrough.NewParser(),
 		},
 	}
 }

core/pkg/ingress/controller/controller.go

@@ -223,10 +223,22 @@ func newIngressController(config *Configuration) *GenericController {
 	}
 
 	mapEventHandler := cache.ResourceEventHandlerFuncs{
+		AddFunc: func(obj interface{}) {
+			upCmap := obj.(*api.ConfigMap)
+			mapKey := fmt.Sprintf("%s/%s", upCmap.Namespace, upCmap.Name)
+			if mapKey == ic.cfg.ConfigMapName {
+				glog.V(2).Infof("adding configmap %v to backend", mapKey)
+				ic.cfg.Backend.SetConfig(upCmap)
+			}
+		},
 		UpdateFunc: func(old, cur interface{}) {
 			if !reflect.DeepEqual(old, cur) {
 				upCmap := cur.(*api.ConfigMap)
 				mapKey := fmt.Sprintf("%s/%s", upCmap.Namespace, upCmap.Name)
+				if mapKey == ic.cfg.ConfigMapName {
+					glog.V(2).Infof("updating configmap backend (%v)", mapKey)
+					ic.cfg.Backend.SetConfig(upCmap)
+				}
 				// updates to configuration configmaps can trigger an update
 				if mapKey == ic.cfg.ConfigMapName || mapKey == ic.cfg.TCPConfigMapName || mapKey == ic.cfg.UDPConfigMapName {
 					ic.recorder.Eventf(upCmap, api.EventTypeNormal, "UPDATE", fmt.Sprintf("ConfigMap %v", mapKey))
@@ -310,8 +322,14 @@ func (ic GenericController) GetSecret(name string) (*api.Secret, error) {
 }
 
 func (ic *GenericController) getConfigMap(ns, name string) (*api.ConfigMap, error) {
-	// TODO: check why ic.mapLister.Store.GetByKey(mapKey) is not stable (random content)
+	s, exists, err := ic.mapLister.Store.GetByKey(fmt.Sprintf("%v/%v", ns, name))
-	return ic.cfg.Client.ConfigMaps(ns).Get(name)
+	if err != nil {
+		return nil, err
+	}
+	if !exists {
+		return nil, fmt.Errorf("configmap %v was not found", name)
+	}
+	return s.(*api.ConfigMap), nil
 }
 
 // sync collects all the pieces required to assemble the configuration file and
@@ -329,20 +347,6 @@ func (ic *GenericController) sync(key interface{}) error {
 		return fmt.Errorf("deferring sync till endpoints controller has synced")
 	}
 
-	// by default no custom configuration
-	cfg := &api.ConfigMap{}
-
-	if ic.cfg.ConfigMapName != "" {
-		// search for custom configmap (defined in main args)
-		var err error
-		ns, name, _ := k8s.ParseNameNS(ic.cfg.ConfigMapName)
-		cfg, err = ic.getConfigMap(ns, name)
-		if err != nil {
-			// requeue
-			return fmt.Errorf("unexpected error searching configmap %v: %v", ic.cfg.ConfigMapName, err)
-		}
-	}
-
 	upstreams, servers := ic.getBackendServers()
 	var passUpstreams []*ingress.SSLPassthroughBackend
 	for _, server := range servers {
@@ -362,7 +366,7 @@ func (ic *GenericController) sync(key interface{}) error {
 		}
 	}
 
-	data, err := ic.cfg.Backend.OnUpdate(cfg, ingress.Configuration{
+	data, err := ic.cfg.Backend.OnUpdate(ingress.Configuration{
 		Backends:            upstreams,
 		Servers:             servers,
 		TCPEndpoints:        ic.getTCPServices(),

core/pkg/ingress/defaults/main.go

@@ -49,6 +49,10 @@ type Backend struct {
 	// Enables or disables the redirect (301) to the HTTPS port
 	SSLRedirect bool `json:"ssl-redirect"`
 
+	// Enables or disables the specification of port in redirects
+	// Default: false
+	UsePortInRedirects bool `json:"use-port-in-redirects"`
+
 	// Number of unsuccessful attempts to communicate with the server that should happen in the
 	// duration set by the fail_timeout parameter to consider the server unavailable
 	// http://nginx.org/en/docs/http/ngx_http_upstream_module.html#upstream

core/pkg/ingress/errors/errors.go

@@ -37,9 +37,9 @@ var (
 )
 
 // NewInvalidAnnotationContent returns a new InvalidContent error
-func NewInvalidAnnotationContent(name string) error {
+func NewInvalidAnnotationContent(name string, val interface{}) error {
 	return InvalidContent{
-		Name: fmt.Sprintf("the annotation %v does not contains a valid value", name),
+		Name: fmt.Sprintf("the annotation %v does not contains a valid value (%v)", name, val),
 	}
 }
 

core/pkg/ingress/errors/errors_test.go

@@ -38,7 +38,7 @@ func TestInvalidContent(t *testing.T) {
 	if IsInvalidContent(ErrMissingAnnotations) {
 		t.Error("expected false")
 	}
-	err := NewInvalidAnnotationContent("demo")
+	err := NewInvalidAnnotationContent("demo", "")
 	if !IsInvalidContent(err) {
 		t.Error("expected true")
 	}

core/pkg/ingress/types.go

@@ -68,7 +68,6 @@ type Controller interface {
 	// Notifications of type Add, Update and Delete:
 	// https://github.com/kubernetes/kubernetes/blob/master/pkg/client/cache/controller.go#L164
 	//
-	// ConfigMap content of --configmap
 	// Configuration returns the translation from Ingress rules containing
 	// information about all the upstreams (service endpoints ) "virtual"
 	// servers (FQDN) and all the locations inside each server. Each
@@ -79,7 +78,9 @@ type Controller interface {
 	//
 	// The returned configuration is then passed to test, and then to reload
 	// if there is no errors.
-	OnUpdate(*api.ConfigMap, Configuration) ([]byte, error)
+	OnUpdate(Configuration) ([]byte, error)
+	// ConfigMap content of --configmap
+	SetConfig(*api.ConfigMap)
 	// BackendDefaults returns the minimum settings required to configure the
 	// communication to endpoints
 	BackendDefaults() defaults.Backend
@@ -233,6 +234,9 @@ type Location struct {
 	// external authentication
 	// +optional
 	CertificateAuth resolver.AuthSSLCert `json:"certificateAuth,omitempty"`
+	// UsePortInRedirects indicates if redirects must specify the port
+	// +optional
+	UsePortInRedirects bool `json:"use-port-in-redirects"`
 }
 
 // SSLPassthroughBackend describes a SSL upstream server configured