diff --git a/controllers/nginx/pkg/cmd/controller/nginx.go b/controllers/nginx/pkg/cmd/controller/nginx.go index 1700ce555..9e2bb64d2 100644 --- a/controllers/nginx/pkg/cmd/controller/nginx.go +++ b/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 diff --git a/controllers/nginx/pkg/config/config.go b/controllers/nginx/pkg/config/config.go index d7ad8aff8..c3dc11331 100644 --- a/controllers/nginx/pkg/config/config.go +++ b/controllers/nginx/pkg/config/config.go @@ -270,6 +270,7 @@ func NewDefault() Configuration { CustomHTTPErrors: []int{}, WhitelistSourceRange: []string{}, SkipAccessLogURLs: []string{}, + UsePortInRedirects: false, }, } diff --git a/controllers/nginx/pkg/template/configmap.go b/controllers/nginx/pkg/template/configmap.go index 3617988ad..701d5a238 100644 --- a/controllers/nginx/pkg/template/configmap.go +++ b/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 { - delete(conf.Data, customHTTPErrors) + if val, ok := conf[customHTTPErrors]; ok { + 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 { - delete(conf.Data, skipAccessLogUrls) + if val, ok := conf[skipAccessLogUrls]; ok { + delete(conf, skipAccessLogUrls) skipUrls = strings.Split(val, ",") } - if val, ok := conf.Data[whitelistSourceRange]; ok { - delete(conf.Data, whitelistSourceRange) + if val, ok := conf[whitelistSourceRange]; ok { + 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) } diff --git a/controllers/nginx/pkg/template/configmap_test.go b/controllers/nginx/pkg/template/configmap_test.go index 028fb2168..2e4c43af2 100644 --- a/controllers/nginx/pkg/template/configmap_test.go +++ b/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{ - Data: map[string]string{ - "custom-http-errors": "300,400,demo", - "proxy-read-timeout": "1", - "proxy-send-timeout": "2", - "skip-access-log-urls": "/log,/demo,/test", - "use-proxy-protocol": "true", - "use-gzip": "true", - "enable-dynamic-tls-records": "false", - "gzip-types": "text/html", - }, + conf := map[string]string{ + "custom-http-errors": "300,400,demo", + "proxy-read-timeout": "1", + "proxy-send-timeout": "2", + "skip-access-log-urls": "/log,/demo,/test", + "use-proxy-protocol": "true", + "use-gzip": "true", + "enable-dynamic-tls-records": "false", + "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{ - Data: map[string]string{ - "whitelist-source-range": "1.1.1.1/32", - }, + to = ReadConfig(map[string]string{ + "whitelist-source-range": "1.1.1.1/32", }) if diff := pretty.Compare(to, def); diff != "" { diff --git a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl index 7e4979956..5f10d030b 100644 --- a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl +++ b/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 }}; diff --git a/core/pkg/ingress/annotations/parser/main.go b/core/pkg/ingress/annotations/parser/main.go index eb505ae74..bff6f2210 100644 --- a/core/pkg/ingress/annotations/parser/main.go +++ b/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 } diff --git a/core/pkg/ingress/annotations/portinredirect/main.go b/core/pkg/ingress/annotations/portinredirect/main.go index c76de0940..1b16e8dcd 100644 --- a/core/pkg/ingress/annotations/portinredirect/main.go +++ b/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 { - return portInRedirect{} +func NewParser(db resolver.DefaultBackend) parser.IngressAnnotation { + 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 } diff --git a/core/pkg/ingress/annotations/portinredirect/main_test.go b/core/pkg/ingress/annotations/portinredirect/main_test.go new file mode 100644 index 000000000..cb6403fd1 --- /dev/null +++ b/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 +} diff --git a/core/pkg/ingress/controller/annotations.go b/core/pkg/ingress/controller/annotations.go index 00f62f7f5..c1eb09fbd 100644 --- a/core/pkg/ingress/controller/annotations.go +++ 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), - "ExternalAuth": authreq.NewParser(), - "CertificateAuth": authtls.NewParser(cfg), - "EnableCORS": cors.NewParser(), - "HealthCheck": healthcheck.NewParser(cfg), - "Whitelist": ipwhitelist.NewParser(cfg), - "Proxy": proxy.NewParser(cfg), - "RateLimit": ratelimit.NewParser(), - "Redirect": rewrite.NewParser(cfg), - "SecureUpstream": secureupstream.NewParser(), - "SSLPassthrough": sslpassthrough.NewParser(), + "BasicDigestAuth": auth.NewParser(auth.AuthDirectory, cfg), + "ExternalAuth": authreq.NewParser(), + "CertificateAuth": authtls.NewParser(cfg), + "EnableCORS": cors.NewParser(), + "HealthCheck": healthcheck.NewParser(cfg), + "Whitelist": ipwhitelist.NewParser(cfg), + "UsePortInRedirects": portinredirect.NewParser(cfg), + "Proxy": proxy.NewParser(cfg), + "RateLimit": ratelimit.NewParser(), + "Redirect": rewrite.NewParser(cfg), + "SecureUpstream": secureupstream.NewParser(), + "SSLPassthrough": sslpassthrough.NewParser(), }, } } diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index b82fae771..29b2c30d3 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/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) - return ic.cfg.Client.ConfigMaps(ns).Get(name) + s, exists, err := ic.mapLister.Store.GetByKey(fmt.Sprintf("%v/%v", ns, 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(), diff --git a/core/pkg/ingress/defaults/main.go b/core/pkg/ingress/defaults/main.go index 12badd1b2..ba56bc7c9 100644 --- a/core/pkg/ingress/defaults/main.go +++ b/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 diff --git a/core/pkg/ingress/errors/errors.go b/core/pkg/ingress/errors/errors.go index 924753963..8db16c780 100644 --- a/core/pkg/ingress/errors/errors.go +++ b/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), } } diff --git a/core/pkg/ingress/errors/errors_test.go b/core/pkg/ingress/errors/errors_test.go index c0bd52406..957f10be3 100644 --- a/core/pkg/ingress/errors/errors_test.go +++ b/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") } diff --git a/core/pkg/ingress/types.go b/core/pkg/ingress/types.go index dc8080987..4891995e7 100644 --- a/core/pkg/ingress/types.go +++ b/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