diff --git a/nginx-controller/controller/controller.go b/nginx-controller/controller/controller.go index 142f5fb27f..5523cc9188 100644 --- a/nginx-controller/controller/controller.go +++ b/nginx-controller/controller/controller.go @@ -293,7 +293,7 @@ func (lbc *LoadBalancerController) syncEndp(key string) { continue } glog.V(3).Infof("Updating Endpoints for %v/%v", ing.Name, ing.Namespace) - name := ing.Namespace + "-" + ing.Name + name := ing.Namespace + "/" + ing.Name lbc.cnf.UpdateEndpoints(name, ingEx) } } @@ -473,12 +473,9 @@ func (lbc *LoadBalancerController) syncIng(key string) { return } - // defaut/some-ingress -> default-some-ingress - name := strings.Replace(key, "/", "-", -1) - if !ingExists { glog.V(2).Infof("Deleting Ingress: %v\n", key) - lbc.cnf.DeleteIngress(name) + lbc.cnf.DeleteIngress(key) } else { glog.V(2).Infof("Adding or Updating Ingress: %v\n", key) @@ -488,7 +485,7 @@ func (lbc *LoadBalancerController) syncIng(key string) { lbc.ingQueue.requeueAfter(key, err, 5*time.Second) return } - lbc.cnf.AddOrUpdateIngress(name, ingEx) + lbc.cnf.AddOrUpdateIngress(key, ingEx) } } diff --git a/nginx-controller/main.go b/nginx-controller/main.go index 172f1af97f..817199ccf1 100644 --- a/nginx-controller/main.go +++ b/nginx-controller/main.go @@ -29,6 +29,15 @@ var ( nginxConfigMaps = flag.String("nginx-configmaps", "", `Specifies a configmaps resource that can be used to customize NGINX configuration. The value must follow the following format: /`) + + enableMerging = flag.Bool("enable-merging", false, + `Enables merging of ingress rules in multiple ingress objects targeting the same host. + By default referencing the same host in multiple ingress objects will result in an error, + and only the first ingress object will be used by the ingress controller. + If this flag is enabled these rules will be merged using the server generated of + the oldest ingress object as a base and adding the locations and settings of + every ingress object in descending oder of their age. + This is similar to the behavoir of other nginx ingress controller.`) ) func main() { @@ -55,7 +64,13 @@ func main() { ngxc, _ := nginx.NewNginxController("/etc/nginx/", local) ngxc.Start() config := nginx.NewDefaultConfig() - cnf := nginx.NewConfigurator(ngxc, config) + var collisionHandler nginx.CollisionHandler + if *enableMerging { + collisionHandler = nginx.NewMergingCollisionHandler() + } else { + collisionHandler = nginx.NewDenyCollisionHandler() + } + cnf := nginx.NewConfigurator(ngxc, collisionHandler, config) lbc, _ := controller.NewLoadBalancerController(kubeClient, 30*time.Second, *watchNamespace, cnf, *nginxConfigMaps) lbc.Run() } diff --git a/nginx-controller/nginx/collision_handler.go b/nginx-controller/nginx/collision_handler.go new file mode 100644 index 0000000000..ea830b1806 --- /dev/null +++ b/nginx-controller/nginx/collision_handler.go @@ -0,0 +1,29 @@ +package nginx + +import "k8s.io/kubernetes/pkg/apis/extensions" + +// CollisionHandler rosolves collisions in the generated configuration, so the nginx configuration stays valid. +type CollisionHandler interface { + // AddConfigs is resolving definition collisions that would result in a invalid nginx configuration + AddConfigs(ing *extensions.Ingress, ingServers []Server) (changed []Server, err error) + + // RemoveConfigs frees server definitions, so they can be used again + RemoveConfigs(key string) (changed []Server, deleted []Server, err error) +} + +type cacheEntry struct { + Ingress extensions.Ingress + Servers []Server +} + +type cacheEntryList []cacheEntry + +func (list cacheEntryList) Len() int { + return len(list) +} +func (list cacheEntryList) Less(i, j int) bool { + return list[i].Ingress.CreationTimestamp.Before(list[j].Ingress.CreationTimestamp) +} +func (list cacheEntryList) Swap(i, j int) { + list[i], list[j] = list[j], list[i] +} diff --git a/nginx-controller/nginx/collision_handler_test.go b/nginx-controller/nginx/collision_handler_test.go new file mode 100644 index 0000000000..dce13b1042 --- /dev/null +++ b/nginx-controller/nginx/collision_handler_test.go @@ -0,0 +1,123 @@ +package nginx + +import ( + "time" + + "k8s.io/kubernetes/pkg/api" + "k8s.io/kubernetes/pkg/api/unversioned" + "k8s.io/kubernetes/pkg/apis/extensions" +) + +var ( + // + // Ingress 1 + // + ingress1Upstream1 = Upstream{ + Name: "default-ing1-one.example.com-svc1", + } + ingress1Location1 = Location{ + Path: "/1", + Upstream: ingress1Upstream1, + } + ingress1Server1 = Server{ + Name: "one.example.com", + Locations: []Location{ingress1Location1}, + Upstreams: []Upstream{ingress1Upstream1}, + } + // ingress1Config1 = IngressNginxConfig{ + // Upstreams: []Upstream{ingress1Upstream1}, + // Server: ingress1Server1, + // } + ingress1 = extensions.Ingress{ + ObjectMeta: api.ObjectMeta{ + Name: "ing1", + Namespace: "default", + CreationTimestamp: unversioned.NewTime(time.Now().Add(-2 * time.Hour)), + }, + } + + // + // Ingress 2 + // + ingress2Upstream1 = Upstream{ + Name: "default-ing2-one.example.com-svc1", + } + ingress2Upstream2 = Upstream{ + Name: "default-ing2-two.example.com-svc1", + } + ingress2Location1 = Location{ + Path: "/1", + Upstream: ingress2Upstream1, + } + ingress2Location2 = Location{ + Path: "/2", + Upstream: ingress2Upstream1, + } + ingress2Location3 = Location{ + Path: "/3", + Upstream: ingress2Upstream2, + } + ingress2Server1 = Server{ + Name: "one.example.com", + Upstreams: []Upstream{ingress2Upstream1}, + Locations: []Location{ingress2Location1, ingress2Location2}, + } + ingress2Server2 = Server{ + Name: "two.example.com", + Upstreams: []Upstream{ingress2Upstream2}, + Locations: []Location{ingress2Location3}, + } + // ingress2Config1 = IngressNginxConfig{ + // Upstreams: []Upstream{ingress2Upstream1}, + // Server: ingress2Server1, + // } + // ingress2Config2 = IngressNginxConfig{ + // Upstreams: []Upstream{ingress2Upstream2}, + // Server: ingress2Server2, + // } + ingress2 = extensions.Ingress{ + ObjectMeta: api.ObjectMeta{ + Name: "ing2", + Namespace: "default", + // this ingress is older than ing1, so it shoud be the base of the merge, + // which means that ing2 will override conflicting locations, but not server settings + CreationTimestamp: unversioned.NewTime(time.Now().Add(-4 * time.Hour)), + }, + } + + // + // Ingress 3 + // + ingress3Upstream1 = Upstream{ + Name: "default-ing3-one.example.com-svc1", + } + ingress3Location1 = Location{ + Path: "/1", + Upstream: ingress3Upstream1, + } + ingress3Server1 = Server{ + Name: "one.example.com", + Locations: []Location{ingress3Location1}, + // this server introduces additional settings, + // because the merging process is additive they will be + // used regardless of age + SSL: true, + SSLCertificate: "cert.pem", + SSLCertificateKey: "cert.pem", + HTTP2: true, + HSTS: true, + HSTSMaxAge: 2000, + HSTSIncludeSubdomains: true, + } + // ingress3Config1 = IngressNginxConfig{ + // Upstreams: []Upstream{ingress3Upstream1}, + // Server: ingress3Server1, + // } + ingress3 = extensions.Ingress{ + ObjectMeta: api.ObjectMeta{ + Name: "ing3", + Namespace: "default", + CreationTimestamp: unversioned.NewTime(time.Now().Add(-1 * time.Hour)), + }, + } +) diff --git a/nginx-controller/nginx/configurator.go b/nginx-controller/nginx/configurator.go index 377381110c..bae074c886 100644 --- a/nginx-controller/nginx/configurator.go +++ b/nginx-controller/nginx/configurator.go @@ -14,16 +14,18 @@ const emptyHost = "" // Configurator transforms an Ingress resource into NGINX Configuration type Configurator struct { - nginx *NginxController - config *Config - lock sync.Mutex + nginx *NginxController + config *Config + collisionHandler CollisionHandler + lock sync.Mutex } // NewConfigurator creates a new Configurator -func NewConfigurator(nginx *NginxController, config *Config) *Configurator { +func NewConfigurator(nginx *NginxController, collisionHandler CollisionHandler, config *Config) *Configurator { cnf := Configurator{ - nginx: nginx, - config: config, + nginx: nginx, + config: config, + collisionHandler: collisionHandler, } return &cnf @@ -39,8 +41,15 @@ func (cnf *Configurator) AddOrUpdateIngress(name string, ingEx *IngressEx) { defer cnf.lock.Unlock() pems := cnf.updateCertificates(ingEx) - nginxCfg := cnf.generateNginxCfg(ingEx, pems) - cnf.nginx.AddOrUpdateIngress(name, nginxCfg) + generatedServers := cnf.generateNginxCfg(ingEx, pems) + servers, err := cnf.collisionHandler.AddConfigs(ingEx.Ingress, generatedServers) + if err != nil { + glog.Errorf("Error when checking generated servers for collisions: %v", err) + return + } + for _, server := range servers { + cnf.nginx.AddOrUpdateConfig(server.Name, server) + } if err := cnf.nginx.Reload(); err != nil { glog.Errorf("Error when adding or updating ingress %q: %q", name, err) } @@ -79,21 +88,14 @@ func (cnf *Configurator) updateCertificates(ingEx *IngressEx) map[string]string return pems } -func (cnf *Configurator) generateNginxCfg(ingEx *IngressEx, pems map[string]string) IngressNginxConfig { - ingCfg := cnf.createConfig(ingEx) - upstreams := make(map[string]Upstream) +func (cnf *Configurator) generateNginxCfg(ingEx *IngressEx, pems map[string]string) []Server { + ingCfg := cnf.createConfig(ingEx) wsServices := getWebsocketServices(ingEx) rewrites := getRewrites(ingEx) sslServices := getSSLServices(ingEx) - if ingEx.Ingress.Spec.Backend != nil { - name := getNameForUpstream(ingEx.Ingress, emptyHost, ingEx.Ingress.Spec.Backend.ServiceName) - upstream := cnf.createUpstream(ingEx, name, ingEx.Ingress.Spec.Backend, ingEx.Ingress.Namespace) - upstreams[name] = upstream - } - var servers []Server for _, rule := range ingEx.Ingress.Spec.Rules { @@ -107,27 +109,8 @@ func (cnf *Configurator) generateNginxCfg(ingEx *IngressEx, pems map[string]stri glog.Warningf("Host field of ingress rule in %v/%v is empty", ingEx.Ingress.Namespace, ingEx.Ingress.Name) } - server := Server{ - Name: serverName, - HTTP2: ingCfg.HTTP2, - ProxyProtocol: ingCfg.ProxyProtocol, - HSTS: ingCfg.HSTS, - HSTSMaxAge: ingCfg.HSTSMaxAge, - HSTSIncludeSubdomains: ingCfg.HSTSIncludeSubdomains, - RealIPHeader: ingCfg.RealIPHeader, - SetRealIPFrom: ingCfg.SetRealIPFrom, - RealIPRecursive: ingCfg.RealIPRecursive, - ProxyHideHeaders: ingCfg.ProxyHideHeaders, - ProxyPassHeaders: ingCfg.ProxyPassHeaders, - } - - if pemFile, ok := pems[serverName]; ok { - server.SSL = true - server.SSLCertificate = pemFile - server.SSLCertificateKey = pemFile - } - var locations []Location + upstreams := map[string]Upstream{} rootLocation := false for _, path := range rule.HTTP.Paths { @@ -152,13 +135,41 @@ func (cnf *Configurator) generateNginxCfg(ingEx *IngressEx, pems map[string]stri locations = append(locations, loc) } - server.Locations = locations + server := Server{ + Name: serverName, + Locations: locations, + HTTP2: ingCfg.HTTP2, + ProxyProtocol: ingCfg.ProxyProtocol, + HSTS: ingCfg.HSTS, + HSTSMaxAge: ingCfg.HSTSMaxAge, + HSTSIncludeSubdomains: ingCfg.HSTSIncludeSubdomains, + RealIPHeader: ingCfg.RealIPHeader, + SetRealIPFrom: ingCfg.SetRealIPFrom, + RealIPRecursive: ingCfg.RealIPRecursive, + ProxyHideHeaders: ingCfg.ProxyHideHeaders, + ProxyPassHeaders: ingCfg.ProxyPassHeaders, + } + + if pemFile, ok := pems[serverName]; ok { + server.SSL = true + server.SSLCertificate = pemFile + server.SSLCertificateKey = pemFile + } + + // server.Locations = locations + // server.Upstreams = upstreamMapToSlice(upstreams) servers = append(servers, server) } if len(ingEx.Ingress.Spec.Rules) == 0 && ingEx.Ingress.Spec.Backend != nil { + upsName := getNameForUpstream(ingEx.Ingress, emptyHost, ingEx.Ingress.Spec.Backend.ServiceName) + upstream := cnf.createUpstream(ingEx, upsName, ingEx.Ingress.Spec.Backend, ingEx.Ingress.Namespace) + location := createLocation(pathOrDefault("/"), upstream, &ingCfg, wsServices[ingEx.Ingress.Spec.Backend.ServiceName], rewrites[ingEx.Ingress.Spec.Backend.ServiceName], sslServices[ingEx.Ingress.Spec.Backend.ServiceName]) + server := Server{ Name: emptyHost, + Upstreams: []Upstream{upstream}, + Locations: []Location{location}, HTTP2: ingCfg.HTTP2, ProxyProtocol: ingCfg.ProxyProtocol, HSTS: ingCfg.HSTS, @@ -177,18 +188,10 @@ func (cnf *Configurator) generateNginxCfg(ingEx *IngressEx, pems map[string]stri server.SSLCertificateKey = pemFile } - var locations []Location - - upsName := getNameForUpstream(ingEx.Ingress, emptyHost, ingEx.Ingress.Spec.Backend.ServiceName) - - loc := createLocation(pathOrDefault("/"), upstreams[upsName], &ingCfg, wsServices[ingEx.Ingress.Spec.Backend.ServiceName], rewrites[ingEx.Ingress.Spec.Backend.ServiceName], sslServices[ingEx.Ingress.Spec.Backend.ServiceName]) - locations = append(locations, loc) - - server.Locations = locations servers = append(servers, server) } - return IngressNginxConfig{Upstreams: upstreamMapToSlice(upstreams), Servers: servers} + return servers } func (cnf *Configurator) createConfig(ingEx *IngressEx) Config { @@ -397,7 +400,16 @@ func (cnf *Configurator) DeleteIngress(name string) { cnf.lock.Lock() defer cnf.lock.Unlock() - cnf.nginx.DeleteIngress(name) + changedServers, deletedServers, err := cnf.collisionHandler.RemoveConfigs(name) + if err != nil { + glog.Errorf("Error when removing ingress %q from collision handler: %v", name, err) + } + for _, server := range changedServers { + cnf.nginx.AddOrUpdateConfig(server.Name, server) + } + for _, server := range deletedServers { + cnf.nginx.DeleteConfig(server.Name) + } if err := cnf.nginx.Reload(); err != nil { glog.Errorf("Error when removing ingress %q: %q", name, err) } diff --git a/nginx-controller/nginx/configurator_test.go b/nginx-controller/nginx/configurator_test.go index ed1cf4f275..bfea5244da 100644 --- a/nginx-controller/nginx/configurator_test.go +++ b/nginx-controller/nginx/configurator_test.go @@ -1,8 +1,6 @@ package nginx -import ( - "testing" -) +import "testing" func TestPathOrDefaultReturnDefault(t *testing.T) { path := "" diff --git a/nginx-controller/nginx/deny_collision_handler.go b/nginx-controller/nginx/deny_collision_handler.go new file mode 100644 index 0000000000..d51f02957d --- /dev/null +++ b/nginx-controller/nginx/deny_collision_handler.go @@ -0,0 +1,116 @@ +package nginx + +import ( + "fmt" + "sort" + + "github.com/golang/glog" + "k8s.io/kubernetes/pkg/apis/extensions" +) + +// DenyCollisionHandler skips colliding server definitions and emmits an error message +type DenyCollisionHandler struct { + cache map[string]cacheEntry + hostIngressMapping map[string]string +} + +func NewDenyCollisionHandler() CollisionHandler { + return &DenyCollisionHandler{ + map[string]cacheEntry{}, + map[string]string{}, + } +} + +func (n *DenyCollisionHandler) AddConfigs(ingress *extensions.Ingress, servers []Server) ([]Server, error) { + ingressKey := fmt.Sprintf("%s/%s", ingress.GetNamespace(), ingress.GetName()) + n.cache[ingressKey] = cacheEntry{ + Ingress: *ingress, + Servers: servers, + } + serversWithoutConflict := []Server{} + for _, server := range servers { + if ingressLockingServerName, exists := n.hostIngressMapping[server.Name]; exists { + // there is already a config using this servername + if n.cache[ingressLockingServerName].Ingress.CreationTimestamp.Before(ingress.CreationTimestamp) { + // the ingress object currently holding the lock on this server name is older than the new one, + // so it is still holder of the log + glog.Errorf("Conflicting server with name '%s' in ingress %s/%s, a server with this name is already defined in ingress %s, ignored", server.Name, ingress.GetNamespace(), ingress.GetName(), ingressLockingServerName) + continue + } + } + serversWithoutConflict = append(serversWithoutConflict, server) + n.hostIngressMapping[server.Name] = ingressKey + } + + return serversWithoutConflict, nil +} + +func (n *DenyCollisionHandler) RemoveConfigs(ingressKey string) ([]Server, []Server, error) { + deletedCacheEntry, exists := n.cache[ingressKey] + if !exists { + return nil, nil, fmt.Errorf("Ingress '%s' cannot be removed, because it was not found in the mapping", ingressKey) + } + delete(n.cache, ingressKey) + + freedServerNames := n.getServerNamesBlockedByIngressKey(ingressKey) + changedServers := []Server{} + deletedServers := []Server{} + for _, serverName := range freedServerNames { + // server name is now free again + delete(n.hostIngressMapping, serverName) + cacheEntries := n.getCacheEntryServersByServerName(serverName) + for _, entry := range cacheEntries { + // there is another cacheEntry that wants to use this host, + // check this server for collisions and add it to the changed servers + serversWithoutConflict, err := n.AddConfigs(&entry.Ingress, entry.Servers) + if err != nil { + return nil, nil, err + } + changedServers = append(changedServers, serversWithoutConflict...) + } + + if len(cacheEntries) == 0 { + // this serverName is not in use anymore and can be deleted + for _, server := range deletedCacheEntry.Servers { + if server.Name == serverName { + deletedServers = append(deletedServers, server) + } + } + } + } + + return changedServers, deletedServers, nil +} + +func (n *DenyCollisionHandler) getServerNamesBlockedByIngressKey(ingressKey string) []string { + blocked := []string{} + for host, ingress := range n.hostIngressMapping { + if ingress == ingressKey { + blocked = append(blocked, host) + } + } + return blocked +} + +// returns a sorted list of cache entries including only servers with a matching server name +func (n *DenyCollisionHandler) getCacheEntryServersByServerName(serverName string) []*cacheEntry { + entryList := cacheEntryList{} + for _, cacheEntry := range n.cache { + entryList = append(entryList, cacheEntry) + } + sort.Sort(entryList) + + results := []*cacheEntry{} + for _, entry := range entryList { + servers := []Server{} + for _, server := range entry.Servers { + if server.Name == serverName { + servers = append(servers, server) + } + } + if len(servers) > 0 { + results = append(results, &cacheEntry{entry.Ingress, servers}) + } + } + return results +} diff --git a/nginx-controller/nginx/deny_collision_handler_test.go b/nginx-controller/nginx/deny_collision_handler_test.go new file mode 100644 index 0000000000..82bd1f6810 --- /dev/null +++ b/nginx-controller/nginx/deny_collision_handler_test.go @@ -0,0 +1,68 @@ +package nginx + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestDenyCollisionHandler(t *testing.T) { + ch := NewDenyCollisionHandler() + t.Run("Add first ingress", func(t *testing.T) { + assert := assert.New(t) + result, err := ch.AddConfigs(&ingress1, []Server{ingress1Server1}) + if assert.NoError(err) && assert.NotNil(result) && assert.Len(result, 1) { + assert.Equal(result[0], ingress1Server1) + } + }) + + t.Run("Add 2nd ingress", func(t *testing.T) { + assert := assert.New(t) + result, err := ch.AddConfigs(&ingress2, []Server{ingress2Server1, ingress2Server2}) + if assert.NoError(err) && assert.Len(result, 2, "Unexpected number of configs returned") { + // 1. servername + assert.Equal(ingress2Server1.Name, result[0].Name, "Server names do not match") + assert.Contains(result[0].Locations, ingress2Location1) + assert.Contains(result[0].Locations, ingress2Location2) + if assert.Len(result[0].Upstreams, 1, "Unexpected number of upstreams") { + assert.Contains(result[0].Upstreams, ingress2Upstream1) + } + + // 2. Server + assert.Equal(ingress2Server2.Name, result[1].Name, "Server names do not match") + assert.Contains(result[1].Locations, ingress2Location3) + if assert.Len(result[1].Upstreams, 1, "Unexpected number of upstreams") { + assert.Contains(result[1].Upstreams, ingress2Upstream2) + } + } + }) + + t.Run("Remove 2nd ingress", func(t *testing.T) { + assert := assert.New(t) + changed, deleted, err := ch.RemoveConfigs("default/ing2") + if assert.NoError(err) { + if assert.Len(changed, 1, "Unexpected number of changed servers") { + assert.Contains(changed, ingress1Server1) + } + if assert.Len(deleted, 1) { + assert.Contains(deleted, ingress2Server2) + } + } + }) + + t.Run("Add 3rd ingress", func(t *testing.T) { + assert := assert.New(t) + changed, err := ch.AddConfigs(&ingress3, []Server{ingress3Server1}) + if assert.NoError(err) { + assert.Len(changed, 0, "Unexpected number of changed servers") + } + }) + + t.Run("Try to remove 2nd ingress again", func(t *testing.T) { + assert := assert.New(t) + _, _, err := ch.RemoveConfigs("default/ing2") + if assert.Error(err) { + assert.Equal("Ingress 'default/ing2' cannot be removed, because it was not found in the mapping", err.Error()) + } + }) +} diff --git a/nginx-controller/nginx/ingress.tmpl b/nginx-controller/nginx/ingress.tmpl index 2b2fed503e..c9222a7e10 100644 --- a/nginx-controller/nginx/ingress.tmpl +++ b/nginx-controller/nginx/ingress.tmpl @@ -4,36 +4,36 @@ upstream {{$upstream.Name}} { server {{$server.Address}}:{{$server.Port}};{{end}} }{{end}} -{{range $server := .Servers}} server { - listen 80{{if $server.ProxyProtocol}} proxy_protocol{{end}}; - {{if $server.SSL}} - listen 443 ssl{{if $server.HTTP2}} http2{{end}}{{if $server.ProxyProtocol}} proxy_protocol{{end}}; - ssl_certificate {{$server.SSLCertificate}}; - ssl_certificate_key {{$server.SSLCertificateKey}}; + listen 80{{if .ProxyProtocol}} proxy_protocol{{end}}; + {{if .SSL}} + listen 443 ssl{{if .HTTP2}} http2{{end}}{{if .ProxyProtocol}} proxy_protocol{{end}}; + ssl_certificate {{.SSLCertificate}}; + ssl_certificate_key {{.SSLCertificateKey}}; {{end}} - {{range $setRealIPFrom := $server.SetRealIPFrom}} + {{range $setRealIPFrom := .SetRealIPFrom}} set_real_ip_from {{$setRealIPFrom}};{{end}} - {{if $server.RealIPHeader}}real_ip_header {{$server.RealIPHeader}};{{end}} - {{if $server.RealIPRecursive}}real_ip_recursive on;{{end}} + {{if .RealIPHeader}}real_ip_header {{.RealIPHeader}};{{end}} + {{if .RealIPRecursive}}real_ip_recursive on;{{end}} - {{if $server.Name}} - server_name {{$server.Name}}; + {{if .Name}} + server_name {{.Name}}; {{end}} - {{range $proxyHideHeader := $server.ProxyHideHeaders}} + {{range $proxyHideHeader := .ProxyHideHeaders}} proxy_hide_header {{$proxyHideHeader}};{{end}} - {{range $proxyPassHeader := $server.ProxyPassHeaders}} + {{range $proxyPassHeader := .ProxyPassHeaders}} proxy_pass_header {{$proxyPassHeader}};{{end}} - {{if $server.SSL}} + + {{if .SSL}} if ($scheme = http) { return 301 https://$host$request_uri; } - {{- if $server.HSTS}} + {{- if .HSTS}} proxy_hide_header Strict-Transport-Security; - add_header Strict-Transport-Security "max-age={{$server.HSTSMaxAge}}; {{if $server.HSTSIncludeSubdomains}}includeSubDomains; {{end}}preload" always;{{end}} + add_header Strict-Transport-Security "max-age={{.HSTSMaxAge}}; {{if .HSTSIncludeSubdomains}}includeSubDomains; {{end}}preload" always;{{end}} {{- end}} - {{range $location := $server.Locations}} + {{range $location := .Locations}} location {{$location.Path}} { proxy_http_version 1.1; {{if $location.Websocket}} @@ -66,4 +66,4 @@ server { proxy_pass http://{{$location.Upstream.Name}}{{$location.Rewrite}}; {{end}} }{{end}} -}{{end}} +} diff --git a/nginx-controller/nginx/merging_collision_handler.go b/nginx-controller/nginx/merging_collision_handler.go new file mode 100644 index 0000000000..e44b60c23c --- /dev/null +++ b/nginx-controller/nginx/merging_collision_handler.go @@ -0,0 +1,185 @@ +package nginx + +import ( + "fmt" + "sort" + + "k8s.io/kubernetes/pkg/apis/extensions" +) + +type MergingCollisionHandler struct { + cache map[string]cacheEntry + hostIngressMapping map[string]map[string]bool +} + +func NewMergingCollisionHandler() CollisionHandler { + return &MergingCollisionHandler{ + map[string]cacheEntry{}, + map[string]map[string]bool{}, + } +} + +func (m *MergingCollisionHandler) AddConfigs(ingress *extensions.Ingress, servers []Server) ([]Server, error) { + ingressKey := fmt.Sprintf("%s/%s", ingress.GetNamespace(), ingress.GetName()) + m.cache[ingressKey] = cacheEntry{ + Ingress: *ingress, + Servers: servers, + } + + hosts := []string{} + for _, server := range servers { + hosts = append(hosts, server.Name) + } + m.updateHostIngressMapping(ingressKey, hosts) + + result := []Server{} + for _, server := range servers { + result = append(result, *m.addOrUpdateServer(&server)) + } + return result, nil +} + +func (m *MergingCollisionHandler) RemoveConfigs(ingressKey string) ([]Server, []Server, error) { + deletedCacheEntry, exists := m.cache[ingressKey] + if !exists { + return nil, nil, fmt.Errorf("Ingress '%s' cannot be removed, because it was not found in the mapping", ingressKey) + } + delete(m.cache, ingressKey) + + changed := []Server{} + affectedHosts := []string{} + for _, server := range deletedCacheEntry.Servers { + affectedHosts = append(affectedHosts, server.Name) + } + + stillExistingHosts := map[string]bool{} + m.removeHostIngressMapping(ingressKey) + + for _, server := range deletedCacheEntry.Servers { + host := server.Name + servers := m.getOrderedServerList(host) + for _, server := range servers { + if server.Name == host { + stillExistingHosts[host] = true + } + } + + if len(servers) == 0 { + continue + } + baseServer := &servers[0] + if len(servers) > 1 { + for _, server := range servers { + baseServer = m.mergeServers(*baseServer, &server) + } + } + baseServer.Upstreams = m.getUpstreamsForServer(baseServer) + changed = append(changed, *baseServer) + } + + deleted := []Server{} + for _, server := range deletedCacheEntry.Servers { + if _, ok := stillExistingHosts[server.Name]; !ok { + deleted = append(deleted, server) + } + } + + return changed, deleted, nil +} + +func (m *MergingCollisionHandler) addOrUpdateServer(server *Server) *Server { + var baseServer Server + if len(m.hostIngressMapping[server.Name]) > 1 { + // the server must be composed of multiple ingress objects + servers := m.getOrderedServerList(server.Name) + for si, server := range servers { + if si == 0 { + baseServer = server + } else { + baseServer = *(m.mergeServers(baseServer, &server)) + } + } + } else { + // the server is not composed + baseServer = *server + } + baseServer.Upstreams = m.getUpstreamsForServer(&baseServer) + return &baseServer +} + +func (m *MergingCollisionHandler) getOrderedServerList(host string) []Server { + affectedCacheEntries := cacheEntryList{} + for ingressName := range m.hostIngressMapping[host] { + affectedCacheEntries = append(affectedCacheEntries, m.cache[ingressName]) + } + sort.Sort(affectedCacheEntries) + + results := []Server{} + for _, cacheEntry := range affectedCacheEntries { + for _, server := range cacheEntry.Servers { + if server.Name == host { + results = append(results, server) + } + } + } + return results +} + +func (m *MergingCollisionHandler) getUpstreamsForServer(server *Server) []Upstream { + tmp := map[string]Upstream{} + for _, location := range server.Locations { + tmp[location.Upstream.Name] = location.Upstream + } + + result := []Upstream{} + for _, upstream := range tmp { + result = append(result, upstream) + } + return result +} + +func (i *MergingCollisionHandler) mergeServers(base Server, merge *Server) *Server { + locationMap := map[string]Location{} + for _, location := range base.Locations { + locationMap[location.Path] = location + } + for _, location := range merge.Locations { + locationMap[location.Path] = location + } + + if merge.SSL { + base.SSL = true + base.SSLCertificate = merge.SSLCertificate + base.SSLCertificateKey = merge.SSLCertificateKey + } + if merge.HTTP2 { + base.HTTP2 = true + } + if merge.HSTS { + base.HSTS = true + base.HSTSMaxAge = merge.HSTSMaxAge + base.HSTSIncludeSubdomains = merge.HSTSIncludeSubdomains + } + + base.Locations = []Location{} + for _, location := range locationMap { + base.Locations = append(base.Locations, location) + } + return &base +} + +func (m *MergingCollisionHandler) removeHostIngressMapping(ingressKey string) { + for _, ingMap := range m.hostIngressMapping { + delete(ingMap, ingressKey) + } +} + +func (m *MergingCollisionHandler) updateHostIngressMapping(ingressKey string, hosts []string) { + m.removeHostIngressMapping(ingressKey) + for _, host := range hosts { + if _, ok := m.hostIngressMapping[host]; !ok { + m.hostIngressMapping[host] = map[string]bool{} + } + m.hostIngressMapping[host][ingressKey] = true + } +} diff --git a/nginx-controller/nginx/merging_collision_handler_test.go b/nginx-controller/nginx/merging_collision_handler_test.go new file mode 100644 index 0000000000..c4ee55c541 --- /dev/null +++ b/nginx-controller/nginx/merging_collision_handler_test.go @@ -0,0 +1,112 @@ +package nginx + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMergingCollisionHandler(t *testing.T) { + testServerHasDefaultSettings := func(assert *assert.Assertions, server *Server) { + assert.False(server.SSL, "SSL should not be active") + assert.Empty(server.SSLCertificate, "SSLCertificate should be empty") + assert.Empty(server.SSLCertificateKey, "SSLCertificateKey should be empty") + assert.False(server.HTTP2, "HTTP2 should not be active") + assert.False(server.HSTS, "HSTS should not be active") + assert.Empty(server.HSTSMaxAge, "HSTSMaxAge should be empty") + assert.False(server.HSTSIncludeSubdomains, "HSTSIncludeSubdomains should not be active") + } + + testServerHasSettingsFromIngress3Server1 := func(assert *assert.Assertions, server *Server) { + assert.True(server.SSL, "SSL should be active") + assert.Equal("cert.pem", server.SSLCertificate, "SSLCertificate is not set") + assert.Equal("cert.pem", server.SSLCertificateKey, "SSLCertificateKey is not set") + assert.True(server.HTTP2, "HTTP2 should be active") + assert.True(server.HSTS, "HSTS should be active") + assert.Equal(int64(2000), server.HSTSMaxAge, "HSTSMaxAge is not set") + assert.True(server.HSTSIncludeSubdomains, "HSTSIncludeSubdomains should be active") + } + + i := NewMergingCollisionHandler() + t.Run("First ingress", func(t *testing.T) { + assert := assert.New(t) + result, err := i.AddConfigs(&ingress1, []Server{ingress1Server1}) + if assert.NoError(err) && assert.NotNil(result) && assert.Len(result, 1) { + assert.Equal(result[0], ingress1Server1) + } + }) + + t.Run("Merge 2nd ingress", func(t *testing.T) { + assert := assert.New(t) + result, err := i.AddConfigs(&ingress2, []Server{ingress2Server1, ingress2Server2}) + if assert.NoError(err) && assert.Len(result, 2, "Unexpected number of configs returned") { + // 1. IngressNginxConfig + assert.Equal(ingress2Server1.Name, result[0].Name, "Server names do not match") + assert.Contains(result[0].Locations, ingress1Location1) + assert.Contains(result[0].Locations, ingress2Location2) + testServerHasDefaultSettings(assert, &result[0]) + if assert.Len(result[0].Upstreams, 2, "Unexpected number of upstreams") { + assert.Contains(result[0].Upstreams, ingress1Upstream1) + assert.Contains(result[0].Upstreams, ingress2Upstream1) + } + + // 2. IngressNginxConfig + assert.Equal(ingress2Server2.Name, result[1].Name, "Server names do not match") + assert.Contains(result[1].Locations, ingress2Location3) + testServerHasDefaultSettings(assert, &result[1]) + if assert.Len(result[1].Upstreams, 1, "Unexpected number of upstreams") { + assert.Contains(result[1].Upstreams, ingress2Upstream2) + } + } + }) + + t.Run("Merge 3rd ingress", func(t *testing.T) { + assert := assert.New(t) + result, err := i.AddConfigs(&ingress3, []Server{ingress3Server1}) + if assert.NoError(err) && assert.Len(result, 1, "Unexpected number of configs returned") { + // 1. IngressNginxConfig + // The order of locations is not fixed, so we must use contains + result := result[0] + assert.Equal(ingress3Server1.Name, result.Name, "Server names do not match") + testServerHasSettingsFromIngress3Server1(assert, &result) + if assert.Len(result.Locations, 2, "Unexpected number of locations") { + assert.Contains(result.Locations, ingress3Location1) + assert.Contains(result.Locations, ingress2Location2) + } + if assert.Len(result.Upstreams, 2, "Unexpected number of upstreams") { + assert.Contains(result.Upstreams, ingress3Upstream1) + assert.Contains(result.Upstreams, ingress2Upstream1) + } + } + }) + + t.Run("Remove 2nd ingress", func(t *testing.T) { + assert := assert.New(t) + changed, deleted, err := i.RemoveConfigs("default/ing2") + + if assert.NoError(err) { + if assert.Len(changed, 1, "Unexpected number of changed configs") { + // 1. IngressNginxConfig + assert.Equal(ingress1Server1.Name, changed[0].Name, "Server names do not match") + testServerHasSettingsFromIngress3Server1(assert, &changed[0]) + if assert.Len(changed[0].Locations, 1) { + assert.Equal(changed[0].Locations[0], ingress3Location1) + } + if assert.Len(changed[0].Upstreams, 1, "Unexpected number of upstreams") { + assert.Equal(changed[0].Upstreams[0], ingress3Upstream1) + } + } + if assert.Len(deleted, 1, "Unexpected number of deleted hosts") { + assert.Contains(deleted, ingress2Server2) + } + } + }) + + t.Run("Try to remove 2nd ingress again", func(t *testing.T) { + assert := assert.New(t) + _, _, err := i.RemoveConfigs("default/ing2") + if assert.Error(err) { + assert.Equal("Ingress 'default/ing2' cannot be removed, because it was not found in the mapping", err.Error()) + } + }) +} diff --git a/nginx-controller/nginx/nginx.go b/nginx-controller/nginx/nginx.go index e7be342f68..b03406315b 100644 --- a/nginx-controller/nginx/nginx.go +++ b/nginx-controller/nginx/nginx.go @@ -20,12 +20,6 @@ type NginxController struct { local bool } -// IngressNginxConfig describes an NGINX configuration -type IngressNginxConfig struct { - Upstreams []Upstream - Servers []Server -} - // Upstream describes an NGINX upstream type Upstream struct { Name string @@ -41,6 +35,7 @@ type UpstreamServer struct { // Server describes an NGINX server type Server struct { Name string + Upstreams []Upstream Locations []Location SSL bool SSLCertificate string @@ -115,9 +110,9 @@ func NewNginxController(nginxConfPath string, local bool) (*NginxController, err return &ngxc, nil } -// DeleteIngress deletes the configuration file, which corresponds for the +// DeleteConfig deletes the configuration file, which corresponds for the // specified ingress from NGINX conf directory -func (nginx *NginxController) DeleteIngress(name string) { +func (nginx *NginxController) DeleteConfig(name string) { filename := nginx.getIngressNginxConfigFileName(name) glog.V(3).Infof("deleting %v", filename) @@ -128,9 +123,9 @@ func (nginx *NginxController) DeleteIngress(name string) { } } -// AddOrUpdateIngress creates or updates a file with -// the specified configuration for the specified ingress -func (nginx *NginxController) AddOrUpdateIngress(name string, config IngressNginxConfig) { +// AddOrUpdateConfig creates or updates a file with +// the specified configuration +func (nginx *NginxController) AddOrUpdateConfig(name string, config Server) { glog.V(3).Infof("Updating NGINX configuration") filename := nginx.getIngressNginxConfigFileName(name) nginx.templateIt(config, filename) @@ -186,10 +181,13 @@ func (nginx *NginxController) AddOrUpdateCertAndKey(name string, cert string, ke } func (nginx *NginxController) getIngressNginxConfigFileName(name string) string { + if name == emptyHost { + name = "default" + } return path.Join(nginx.nginxConfdPath, name+".conf") } -func (nginx *NginxController) templateIt(config IngressNginxConfig, filename string) { +func (nginx *NginxController) templateIt(config Server, filename string) { tmpl, err := template.New("ingress.tmpl").ParseFiles("ingress.tmpl") if err != nil { glog.Fatal("Failed to parse template file")