From c0aca1833ab6ec21f05b3d05013bc6e1e1daba0e Mon Sep 17 00:00:00 2001 From: Tang Le Date: Tue, 24 Jan 2017 16:19:28 +0800 Subject: [PATCH 01/11] Fix rate limit issue when more than 2 servers enabled in ingress Signed-off-by: Tang Le --- controllers/nginx/pkg/template/template.go | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/controllers/nginx/pkg/template/template.go b/controllers/nginx/pkg/template/template.go index 18373bda0f..cc21dccacd 100644 --- a/controllers/nginx/pkg/template/template.go +++ b/controllers/nginx/pkg/template/template.go @@ -26,6 +26,8 @@ import ( "strings" text_template "text/template" + "k8s.io/kubernetes/pkg/util/sets" + "github.com/golang/glog" "k8s.io/ingress/controllers/nginx/pkg/config" @@ -328,11 +330,11 @@ func buildProxyPass(b interface{}, loc interface{}) string { // rate limiting of request. Each Ingress rule could have up to two zones, one // for connection limit by IP address and other for limiting request per second func buildRateLimitZones(input interface{}) []string { - zones := []string{} + zones := sets.String{} servers, ok := input.([]*ingress.Server) if !ok { - return zones + return zones.List() } for _, server := range servers { @@ -342,7 +344,9 @@ func buildRateLimitZones(input interface{}) []string { zone := fmt.Sprintf("limit_conn_zone $binary_remote_addr zone=%v:%vm;", loc.RateLimit.Connections.Name, loc.RateLimit.Connections.SharedSize) - zones = append(zones, zone) + if !zones.Has(zone) { + zones.Insert(zone) + } } if loc.RateLimit.RPS.Limit > 0 { @@ -350,12 +354,14 @@ func buildRateLimitZones(input interface{}) []string { loc.RateLimit.RPS.Name, loc.RateLimit.RPS.SharedSize, loc.RateLimit.RPS.Limit) - zones = append(zones, zone) + if !zones.Has(zone) { + zones.Insert(zone) + } } } } - return zones + return zones.List() } // buildRateLimit produces an array of limit_req to be used inside the Path of From 725f45b996e839f05d1a0f6e608a15fde3297d67 Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Tue, 24 Jan 2017 16:03:56 -0200 Subject: [PATCH 02/11] Corrects the behaviour of default-ssl-certificate --- .../nginx/rootfs/etc/nginx/template/nginx.tmpl | 2 +- core/pkg/ingress/controller/controller.go | 17 +++++++++++++++-- core/pkg/ingress/controller/launch.go | 4 ++++ 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl index 5f10d030bc..4f05e9faf5 100644 --- a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl +++ b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl @@ -203,7 +203,7 @@ http { server_name {{ $server.Hostname }}; listen [::]:80{{ if $cfg.UseProxyProtocol }} proxy_protocol{{ end }}{{ if eq $index 0 }} ipv6only=off{{end}}{{ if eq $server.Hostname "_"}} default_server reuseport backlog={{ $backlogSize }}{{end}}; {{/* Listen on 442 because port 443 is used in the stream section */}} - {{ if not (empty $server.SSLCertificate) }}listen 442 {{ if $cfg.UseProxyProtocol }}proxy_protocol{{ end }} ssl {{ if $cfg.UseHTTP2 }}http2{{ end }}; + {{ if not (empty $server.SSLCertificate) }}listen 442 {{ if $cfg.UseProxyProtocol }}proxy_protocol{{ end }} {{ if eq $server.Hostname "_"}} default_server reuseport backlog={{ $backlogSize }}{{end}} ssl {{ if $cfg.UseHTTP2 }}http2{{ end }}; {{/* comment PEM sha is required to detect changes in the generated configuration and force a reload */}} # PEM sha: {{ $server.SSLPemChecksum }} ssl_certificate {{ $server.SSLCertificate }}; diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index 29b2c30d3f..c937d3a099 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -807,9 +807,21 @@ func (ic *GenericController) createServers(data []interface{}, upstreams map[str dun := ic.getDefaultUpstream().Name + // This adds the Default Certificate to Default Backend and also for vhosts missing the secret + var defaultPemFileName, defaultPemSHA string + defaultCertificate, err := ic.getPemCertificate(ic.cfg.DefaultSSLCertificate) + if err != nil { + glog.Fatalf("Unable to get default SSL Certificate %v", ic.cfg.DefaultSSLCertificate) + } else { + defaultPemFileName = defaultCertificate.PemFileName + defaultPemSHA = defaultCertificate.PemSHA + } + // default server servers[defServerName] = &ingress.Server{ - Hostname: defServerName, + Hostname: defServerName, + SSLCertificate: defaultPemFileName, + SSLPemChecksum: defaultPemSHA, Locations: []*ingress.Location{ { Path: rootLocation, @@ -879,7 +891,8 @@ func (ic *GenericController) createServers(data []interface{}, upstreams map[str servers[host].SSLPemChecksum = cert.PemSHA } } else { - glog.Warningf("secret %v does not exists", key) + servers[host].SSLCertificate = defaultPemFileName + servers[host].SSLPemChecksum = defaultPemSHA } } diff --git a/core/pkg/ingress/controller/launch.go b/core/pkg/ingress/controller/launch.go index a1f325d4fb..fd85fbc7f4 100644 --- a/core/pkg/ingress/controller/launch.go +++ b/core/pkg/ingress/controller/launch.go @@ -99,6 +99,10 @@ func NewIngressController(backend ingress.Controller) *GenericController { glog.Fatalf("Please specify --default-backend-service") } + if *defSSLCertificate == "" { + glog.Fatalf("Please specify --default-ssl-certificate") + } + kubeClient, err := createApiserverClient(*apiserverHost, *kubeConfigFile) if err != nil { handleFatalInitError(err) From 07ff57854b8b742bc8c6c83e496263be804b5a0d Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Tue, 24 Jan 2017 23:37:39 -0200 Subject: [PATCH 03/11] Removes the need of configuring a default ssl certificate --- core/pkg/ingress/controller/controller.go | 15 ++++++++++++--- core/pkg/ingress/controller/launch.go | 14 +++++--------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index c937d3a099..97156c38b9 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -46,6 +46,7 @@ import ( "k8s.io/ingress/core/pkg/ingress/resolver" "k8s.io/ingress/core/pkg/ingress/status" "k8s.io/ingress/core/pkg/k8s" + ssl "k8s.io/ingress/core/pkg/net/ssl" local_strings "k8s.io/ingress/core/pkg/strings" "k8s.io/ingress/core/pkg/task" ) @@ -810,8 +811,17 @@ func (ic *GenericController) createServers(data []interface{}, upstreams map[str // This adds the Default Certificate to Default Backend and also for vhosts missing the secret var defaultPemFileName, defaultPemSHA string defaultCertificate, err := ic.getPemCertificate(ic.cfg.DefaultSSLCertificate) + // If no default Certificate was supplied, tries to generate a new dumb one if err != nil { - glog.Fatalf("Unable to get default SSL Certificate %v", ic.cfg.DefaultSSLCertificate) + var cert *ingress.SSLCert + defCert, defKey := ssl.GetFakeSSLCert() + cert, err = ssl.AddOrUpdateCertAndKey("system-snake-oil-certificate", defCert, defKey, []byte{}) + if err != nil { + glog.Fatalf("Error generating self signed certificate: %v", err) + } else { + defaultPemFileName = cert.PemFileName + defaultPemSHA = cert.PemSHA + } } else { defaultPemFileName = defaultCertificate.PemFileName defaultPemSHA = defaultCertificate.PemSHA @@ -891,8 +901,7 @@ func (ic *GenericController) createServers(data []interface{}, upstreams map[str servers[host].SSLPemChecksum = cert.PemSHA } } else { - servers[host].SSLCertificate = defaultPemFileName - servers[host].SSLPemChecksum = defaultPemSHA + glog.Warningf("secret %v does not exists", key) } } diff --git a/core/pkg/ingress/controller/launch.go b/core/pkg/ingress/controller/launch.go index fd85fbc7f4..a91eb8ced6 100644 --- a/core/pkg/ingress/controller/launch.go +++ b/core/pkg/ingress/controller/launch.go @@ -54,14 +54,14 @@ func NewIngressController(backend ingress.Controller) *GenericController { tcpConfigMapName = flags.String("tcp-services-configmap", "", `Name of the ConfigMap that contains the definition of the TCP services to expose. The key in the map indicates the external port to be used. The value is the name of the - service with the format namespace/serviceName and the port of the service could be a + service with the format namespace/serviceName and the port of the service could be a number of the name of the port. The ports 80 and 443 are not allowed as external ports. This ports are reserved for the backend`) udpConfigMapName = flags.String("udp-services-configmap", "", `Name of the ConfigMap that contains the definition of the UDP services to expose. The key in the map indicates the external port to be used. The value is the name of the - service with the format namespace/serviceName and the port of the service could be a + service with the format namespace/serviceName and the port of the service could be a number of the name of the port.`) resyncPeriod = flags.Duration("sync-period", 60*time.Second, @@ -74,13 +74,13 @@ func NewIngressController(backend ingress.Controller) *GenericController { profiling = flags.Bool("profiling", true, `Enable profiling via web interface host:port/debug/pprof/`) - defSSLCertificate = flags.String("default-ssl-certificate", "", `Name of the secret + defSSLCertificate = flags.String("default-ssl-certificate", "", `Name of the secret that contains a SSL certificate to be used as default for a HTTPS catch-all server`) - defHealthzURL = flags.String("health-check-path", "/healthz", `Defines + defHealthzURL = flags.String("health-check-path", "/healthz", `Defines the URL to be used as health check inside in the default server in NGINX.`) - updateStatus = flags.Bool("update-status", true, `Indicates if the + updateStatus = flags.Bool("update-status", true, `Indicates if the ingress controller should update the Ingress status IP/hostname. Default is true`) ) @@ -99,10 +99,6 @@ func NewIngressController(backend ingress.Controller) *GenericController { glog.Fatalf("Please specify --default-backend-service") } - if *defSSLCertificate == "" { - glog.Fatalf("Please specify --default-ssl-certificate") - } - kubeClient, err := createApiserverClient(*apiserverHost, *kubeConfigFile) if err != nil { handleFatalInitError(err) From f5706d1d74242c4a13076c2bb8b683967ade811c Mon Sep 17 00:00:00 2001 From: chentao1596 Date: Tue, 24 Jan 2017 11:08:46 +0800 Subject: [PATCH 04/11] prefect unit test cases for core.pkg.ingress.controller.util --- core/pkg/ingress/controller/util_test.go | 111 +++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/core/pkg/ingress/controller/util_test.go b/core/pkg/ingress/controller/util_test.go index ba19ea8304..3707145334 100644 --- a/core/pkg/ingress/controller/util_test.go +++ b/core/pkg/ingress/controller/util_test.go @@ -19,10 +19,25 @@ package controller import ( "testing" + "k8s.io/ingress/core/pkg/ingress" + "k8s.io/ingress/core/pkg/ingress/annotations/auth" + "k8s.io/ingress/core/pkg/ingress/annotations/authreq" + "k8s.io/ingress/core/pkg/ingress/annotations/ipwhitelist" + "k8s.io/ingress/core/pkg/ingress/annotations/proxy" + "k8s.io/ingress/core/pkg/ingress/annotations/ratelimit" + "k8s.io/ingress/core/pkg/ingress/annotations/rewrite" + "k8s.io/ingress/core/pkg/ingress/resolver" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/extensions" + "reflect" ) +type fakeError struct{} + +func (fe *fakeError) Error() string { + return "fakeError" +} + func TestIsValidClass(t *testing.T) { ing := &extensions.Ingress{ ObjectMeta: api.ObjectMeta{ @@ -48,3 +63,99 @@ func TestIsValidClass(t *testing.T) { t.Errorf("Expected invalid class but %v returned", b) } } + +func TestIsHostValid(t *testing.T) { + fkCert := &ingress.SSLCert{ + CAFileName: "foo", + PemFileName: "foo.cr", + PemSHA: "perha", + CN: []string{ + "*.cluster.local", "default.local", + }, + } + + fooTests := []struct { + cr *ingress.SSLCert + host string + er bool + }{ + {nil, "foo1.cluster.local", false}, + {fkCert, "foo1.cluster.local", true}, + {fkCert, "default.local", true}, + {fkCert, "foo2.cluster.local.t", false}, + {fkCert, "", false}, + } + + for _, foo := range fooTests { + r := isHostValid(foo.host, foo.cr) + if r != foo.er { + t.Errorf("Returned %v but expected %v for foo=%v", r, foo.er, foo) + } + } +} + +func TestMatchHostnames(t *testing.T) { + fooTests := []struct { + pattern string + host string + er bool + }{ + {"*.cluster.local.", "foo1.cluster.local.", true}, + {"foo1.cluster.local.", "foo2.cluster.local.", false}, + {"cluster.local.", "foo1.cluster.local.", false}, + {".", "foo1.cluster.local.", false}, + {"cluster.local.", ".", false}, + } + + for _, foo := range fooTests { + r := matchHostnames(foo.pattern, foo.host) + if r != foo.er { + t.Errorf("Returned %v but expected %v for foo=%v", r, foo.er, foo) + } + } +} + +func TestMergeLocationAnnotations(t *testing.T) { + // initial parameters + loc := ingress.Location{} + annotations := map[string]interface{}{ + "Path": "/checkpath", + "IsDefBackend": true, + "Backend": "foo_backend", + "BasicDigestAuth": auth.BasicDigest{}, + DeniedKeyName: &fakeError{}, + "EnableCORS": true, + "ExternalAuth": authreq.External{}, + "RateLimit": ratelimit.RateLimit{}, + "Redirect": rewrite.Redirect{}, + "Whitelist": ipwhitelist.SourceRange{}, + "Proxy": proxy.Configuration{}, + "CertificateAuth": resolver.AuthSSLCert{}, + "UsePortInRedirects": true, + } + + // create test table + type fooMergeLocationAnnotationsStruct struct { + fName string + er interface{} + } + fooTests := []fooMergeLocationAnnotationsStruct{} + for name, value := range annotations { + fva := fooMergeLocationAnnotationsStruct{name, value} + fooTests = append(fooTests, fva) + } + + // execute test + mergeLocationAnnotations(&loc, annotations) + + // check result + for _, foo := range fooTests { + fv := reflect.ValueOf(loc).FieldByName(foo.fName).Interface() + if !reflect.DeepEqual(fv, foo.er) { + t.Errorf("Returned %v but expected %v for the field %s", fv, foo.er, foo.fName) + } + } + if _, ok := annotations[DeniedKeyName]; ok { + t.Errorf("%s should be removed after mergeLocationAnnotations", DeniedKeyName) + } +} From 0245868808b37888321a8f9297ea466a5801ee37 Mon Sep 17 00:00:00 2001 From: chentao1596 Date: Wed, 25 Jan 2017 10:17:45 +0800 Subject: [PATCH 05/11] prefect unit test cases for core.pkg.ingress.controller.annotations --- .../ingress/controller/annotations_test.go | 89 +++++++++++++++++++ 1 file changed, 89 insertions(+) diff --git a/core/pkg/ingress/controller/annotations_test.go b/core/pkg/ingress/controller/annotations_test.go index 6878302883..f577920db9 100644 --- a/core/pkg/ingress/controller/annotations_test.go +++ b/core/pkg/ingress/controller/annotations_test.go @@ -27,6 +27,13 @@ import ( "k8s.io/ingress/core/pkg/ingress/resolver" ) +const ( + annotation_secureUpstream = "ingress.kubernetes.io/secure-backends" + annotation_upsMaxFails = "ingress.kubernetes.io/upstream-max-fails" + annotation_upsFailTimeout = "ingress.kubernetes.io/upstream-fail-timeout" + annotation_passthrough = "ingress.kubernetes.io/ssl-passthrough" +) + type mockCfg struct { } @@ -90,3 +97,85 @@ func buildIngress() *extensions.Ingress { }, } } + +func TestSecureUpstream(t *testing.T) { + ec := newAnnotationExtractor(mockCfg{}) + ing := buildIngress() + + fooAnns := []struct { + annotations map[string]string + er bool + }{ + {map[string]string{annotation_secureUpstream: "true"}, true}, + {map[string]string{annotation_secureUpstream: "false"}, false}, + {map[string]string{annotation_secureUpstream + "_no": "true"}, false}, + {map[string]string{}, false}, + {nil, false}, + } + + for _, foo := range fooAnns { + ing.SetAnnotations(foo.annotations) + r := ec.SecureUpstream(ing) + if r != foo.er { + t.Errorf("Returned %v but expected %v", r, foo.er) + } + } +} + +func TestHealthCheck(t *testing.T) { + ec := newAnnotationExtractor(mockCfg{}) + ing := buildIngress() + + fooAnns := []struct { + annotations map[string]string + eumf int + euft int + }{ + {map[string]string{annotation_upsMaxFails: "3", annotation_upsFailTimeout: "10"}, 3, 10}, + {map[string]string{annotation_upsMaxFails: "3"}, 3, 0}, + {map[string]string{annotation_upsFailTimeout: "10"}, 0, 10}, + {map[string]string{}, 0, 0}, + {nil, 0, 0}, + } + + for _, foo := range fooAnns { + ing.SetAnnotations(foo.annotations) + r := ec.HealthCheck(ing) + if r == nil { + t.Errorf("Returned nil but expected a healthcheck.Upstream") + continue + } + + if r.FailTimeout != foo.euft { + t.Errorf("Returned %d but expected %d for FailTimeout", r.FailTimeout, foo.euft) + } + + if r.MaxFails != foo.eumf { + t.Errorf("Returned %d but expected %d for MaxFails", r.MaxFails, foo.eumf) + } + } +} + +func TestSSLPassthrough(t *testing.T) { + ec := newAnnotationExtractor(mockCfg{}) + ing := buildIngress() + + fooAnns := []struct { + annotations map[string]string + er bool + }{ + {map[string]string{annotation_passthrough: "true"}, true}, + {map[string]string{annotation_passthrough: "false"}, false}, + {map[string]string{annotation_passthrough + "_no": "true"}, false}, + {map[string]string{}, false}, + {nil, false}, + } + + for _, foo := range fooAnns { + ing.SetAnnotations(foo.annotations) + r := ec.SSLPassthrough(ing) + if r != foo.er { + t.Errorf("Returned %v but expected %v", r, foo.er) + } + } +} From 93c712867ab83524a15ca9427f20cb83a8c8fa5c Mon Sep 17 00:00:00 2001 From: chentao1596 Date: Wed, 25 Jan 2017 10:59:46 +0800 Subject: [PATCH 06/11] add unit test cases for core.pkg.ingress.sort_ingress --- core/pkg/ingress/sort_ingress_test.go | 379 ++++++++++++++++++++++++++ 1 file changed, 379 insertions(+) create mode 100644 core/pkg/ingress/sort_ingress_test.go diff --git a/core/pkg/ingress/sort_ingress_test.go b/core/pkg/ingress/sort_ingress_test.go new file mode 100644 index 0000000000..52ceb7029b --- /dev/null +++ b/core/pkg/ingress/sort_ingress_test.go @@ -0,0 +1,379 @@ +/* +Copyright 2017 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 ingress + +import ( + "testing" + + "k8s.io/kubernetes/pkg/api" +) + +func buildBackendByNameServers() BackendByNameServers { + return []*Backend{ + { + Name: "foo1", + Secure: true, + Endpoints: []Endpoint{}, + }, + { + Name: "foo2", + Secure: false, + Endpoints: []Endpoint{}, + }, + { + Name: "foo3", + Secure: true, + Endpoints: []Endpoint{}, + }, + } +} + +func TestBackendByNameServersLen(t *testing.T) { + fooTests := []struct { + backends BackendByNameServers + el int + }{ + {[]*Backend{}, 0}, + {buildBackendByNameServers(), 3}, + {nil, 0}, + } + + for _, fooTest := range fooTests { + r := fooTest.backends.Len() + if r != fooTest.el { + t.Errorf("returned %v but expected %v for the len of BackendByNameServers: %v", r, fooTest.el, fooTest.backends) + } + } +} + +func TestBackendByNameServersSwap(t *testing.T) { + fooTests := []struct { + backends BackendByNameServers + i int + j int + }{ + {buildBackendByNameServers(), 0, 1}, + {buildBackendByNameServers(), 2, 1}, + } + + for _, fooTest := range fooTests { + fooi := fooTest.backends[fooTest.i] + fooj := fooTest.backends[fooTest.j] + fooTest.backends.Swap(fooTest.i, fooTest.j) + if fooi.Name != fooTest.backends[fooTest.j].Name || fooj.Name != fooTest.backends[fooTest.i].Name { + t.Errorf("failed to swap for ByNameServers, foo: %v", fooTest) + } + } +} + +func TestBackendByNameServersLess(t *testing.T) { + fooTests := []struct { + backends BackendByNameServers + i int + j int + er bool + }{ + // order by name + {buildBackendByNameServers(), 0, 2, true}, + {buildBackendByNameServers(), 1, 0, false}, + } + + for _, fooTest := range fooTests { + r := fooTest.backends.Less(fooTest.i, fooTest.j) + if r != fooTest.er { + t.Errorf("returned %v but expected %v for the foo: %v", r, fooTest.er, fooTest) + } + } +} + +func buildEndpointByAddrPort() EndpointByAddrPort { + return []Endpoint{ + { + Address: "127.0.0.1", + Port: "8080", + MaxFails: 3, + FailTimeout: 10, + }, + { + Address: "127.0.0.1", + Port: "8081", + MaxFails: 3, + FailTimeout: 10, + }, + { + Address: "127.0.0.1", + Port: "8082", + MaxFails: 3, + FailTimeout: 10, + }, + { + Address: "127.0.0.2", + Port: "8082", + MaxFails: 3, + FailTimeout: 10, + }, + } +} + +func TestEndpointByAddrPortLen(t *testing.T) { + fooTests := []struct { + endpoints EndpointByAddrPort + el int + }{ + {[]Endpoint{}, 0}, + {buildEndpointByAddrPort(), 4}, + {nil, 0}, + } + + for _, fooTest := range fooTests { + r := fooTest.endpoints.Len() + if r != fooTest.el { + t.Errorf("returned %v but expected %v for the len of EndpointByAddrPort: %v", r, fooTest.el, fooTest.endpoints) + } + } +} + +func TestEndpointByAddrPortSwap(t *testing.T) { + fooTests := []struct { + endpoints EndpointByAddrPort + i int + j int + }{ + {buildEndpointByAddrPort(), 0, 1}, + {buildEndpointByAddrPort(), 2, 1}, + } + + for _, fooTest := range fooTests { + fooi := fooTest.endpoints[fooTest.i] + fooj := fooTest.endpoints[fooTest.j] + fooTest.endpoints.Swap(fooTest.i, fooTest.j) + if fooi.Port != fooTest.endpoints[fooTest.j].Port || + fooi.Address != fooTest.endpoints[fooTest.j].Address || + fooj.Port != fooTest.endpoints[fooTest.i].Port || + fooj.Address != fooTest.endpoints[fooTest.i].Address { + t.Errorf("failed to swap for EndpointByAddrPort, foo: %v", fooTest) + } + } +} + +func TestEndpointByAddrPortLess(t *testing.T) { + fooTests := []struct { + endpoints EndpointByAddrPort + i int + j int + er bool + }{ + // 1) order by name + // 2) order by port(if the name is the same one) + {buildEndpointByAddrPort(), 0, 1, true}, + {buildEndpointByAddrPort(), 2, 1, false}, + {buildEndpointByAddrPort(), 2, 3, true}, + } + + for _, fooTest := range fooTests { + r := fooTest.endpoints.Less(fooTest.i, fooTest.j) + if r != fooTest.er { + t.Errorf("returned %v but expected %v for the foo: %v", r, fooTest.er, fooTest) + } + } +} + +func buildServerByName() ServerByName { + return []*Server{ + { + Hostname: "foo1", + SSLPassthrough: true, + SSLCertificate: "foo1_cert", + SSLPemChecksum: "foo1_pem", + Locations: []*Location{}, + }, + { + Hostname: "foo2", + SSLPassthrough: true, + SSLCertificate: "foo2_cert", + SSLPemChecksum: "foo2_pem", + Locations: []*Location{}, + }, + { + Hostname: "foo3", + SSLPassthrough: false, + SSLCertificate: "foo3_cert", + SSLPemChecksum: "foo3_pem", + Locations: []*Location{}, + }, + { + Hostname: "_", + SSLPassthrough: false, + SSLCertificate: "foo4_cert", + SSLPemChecksum: "foo4_pem", + Locations: []*Location{}, + }, + } +} + +func TestServerByNameLen(t *testing.T) { + fooTests := []struct { + servers ServerByName + el int + }{ + {[]*Server{}, 0}, + {buildServerByName(), 4}, + {nil, 0}, + } + + for _, fooTest := range fooTests { + r := fooTest.servers.Len() + if r != fooTest.el { + t.Errorf("returned %v but expected %v for the len of ServerByName: %v", r, fooTest.el, fooTest.servers) + } + } +} + +func TestServerByNameSwap(t *testing.T) { + fooTests := []struct { + servers ServerByName + i int + j int + }{ + {buildServerByName(), 0, 1}, + {buildServerByName(), 2, 1}, + } + + for _, fooTest := range fooTests { + fooi := fooTest.servers[fooTest.i] + fooj := fooTest.servers[fooTest.j] + fooTest.servers.Swap(fooTest.i, fooTest.j) + if fooi.Hostname != fooTest.servers[fooTest.j].Hostname || + fooj.Hostname != fooTest.servers[fooTest.i].Hostname { + t.Errorf("failed to swap for ServerByName, foo: %v", fooTest) + } + } +} + +func TestServerByNameLess(t *testing.T) { + fooTests := []struct { + servers ServerByName + i int + j int + er bool + }{ + {buildServerByName(), 0, 1, true}, + {buildServerByName(), 2, 1, false}, + {buildServerByName(), 2, 3, false}, + } + + for _, fooTest := range fooTests { + r := fooTest.servers.Less(fooTest.i, fooTest.j) + if r != fooTest.er { + t.Errorf("returned %v but expected %v for the foo: %v", r, fooTest.er, fooTest) + } + } +} + +func buildLocationByPath() LocationByPath { + return []*Location{ + { + Path: "a", + IsDefBackend: true, + Backend: "a_back", + }, + { + Path: "b", + IsDefBackend: true, + Backend: "b_back", + }, + { + Path: "c", + IsDefBackend: true, + Backend: "c_back", + }, + } +} + +func TestLocationByPath(t *testing.T) { + fooTests := []struct { + locations LocationByPath + el int + }{ + {[]*Location{}, 0}, + {buildLocationByPath(), 3}, + {nil, 0}, + } + + for _, fooTest := range fooTests { + r := fooTest.locations.Len() + if r != fooTest.el { + t.Errorf("returned %v but expected %v for the len of LocationByPath: %v", r, fooTest.el, fooTest.locations) + } + } +} + +func TestLocationByPathSwap(t *testing.T) { + fooTests := []struct { + locations LocationByPath + i int + j int + }{ + {buildLocationByPath(), 0, 1}, + {buildLocationByPath(), 2, 1}, + } + + for _, fooTest := range fooTests { + fooi := fooTest.locations[fooTest.i] + fooj := fooTest.locations[fooTest.j] + fooTest.locations.Swap(fooTest.i, fooTest.j) + if fooi.Path != fooTest.locations[fooTest.j].Path || + fooj.Path != fooTest.locations[fooTest.i].Path { + t.Errorf("failed to swap for LocationByPath, foo: %v", fooTest) + } + } +} + +func TestLocationByPathLess(t *testing.T) { + fooTests := []struct { + locations LocationByPath + i int + j int + er bool + }{ + // sorts location by path in descending order + {buildLocationByPath(), 0, 1, false}, + {buildLocationByPath(), 2, 1, true}, + } + + for _, fooTest := range fooTests { + r := fooTest.locations.Less(fooTest.i, fooTest.j) + if r != fooTest.er { + t.Errorf("returned %v but expected %v for the foo: %v", r, fooTest.er, fooTest) + } + } +} + +func TestGetObjectKindForSSLCert(t *testing.T) { + fk := &SSLCert{ + ObjectMeta: api.ObjectMeta{}, + CAFileName: "ca_file", + PemFileName: "pemfile", + PemSHA: "pem_sha", + CN: []string{}, + } + + r := fk.GetObjectKind() + if r == nil { + t.Errorf("Returned nil but expected a valid ObjectKind") + } +} From 96df5b3d55f6bccded24226c08cac96c1f0297be Mon Sep 17 00:00:00 2001 From: Justin Ryan Date: Wed, 25 Jan 2017 09:52:50 -0500 Subject: [PATCH 07/11] Clarify usage of Ingress backend.servicePort --- controllers/gce/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/gce/README.md b/controllers/gce/README.md index 77ed0e1536..aa1d6bd8d2 100644 --- a/controllers/gce/README.md +++ b/controllers/gce/README.md @@ -51,7 +51,7 @@ __Lines 5-7__: Ingress Spec has all the information needed to configure a GCE L7 __Lines 8-9__: Each http rule contains the following information: A host (eg: foo.bar.com, defaults to `*` in this example), a list of paths (eg: `/hostless`) each of which has an associated backend (`test:80`). Both the `host` and `path` must match the content of an incoming request before the L7 directs traffic to the `backend`. -__Lines 10-12__: A `backend` is a service:port combination. It selects a group of pods capable of servicing traffic sent to the path specified in the parent rule. +__Lines 10-12__: A `backend` is a service:port combination. It selects a group of pods capable of servicing traffic sent to the path specified in the parent rule. The `port` is the desired `spec.ports[*].port` from the Service Spec -- Note, though, that the L7 actually directs traffic to the corresponding `NodePort`. __Global Prameters__: For the sake of simplicity the example Ingress has no global parameters. However, one can specify a default backend (see examples below) in the absence of which requests that don't match a path in the spec are sent to the default backend of glbc. Though glbc doesn't support HTTPS yet, security configs would also be global. From 08eda50ebb6c72cedbd2e4ac9bda58258e9d4235 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Wed, 25 Jan 2017 15:16:31 -0300 Subject: [PATCH 08/11] Update nginx to 1.11.9 --- controllers/nginx/rootfs/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/nginx/rootfs/Dockerfile b/controllers/nginx/rootfs/Dockerfile index 10c833e371..74ee4f4934 100644 --- a/controllers/nginx/rootfs/Dockerfile +++ b/controllers/nginx/rootfs/Dockerfile @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -FROM gcr.io/google_containers/nginx-slim:0.12 +FROM gcr.io/google_containers/nginx-slim:0.13 RUN DEBIAN_FRONTEND=noninteractive apt-get update && apt-get install -y \ diffutils \ From bc810d8eef382e8f9bceb0dfcc780e945ad1d708 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Wed, 25 Jan 2017 23:08:40 -0300 Subject: [PATCH 09/11] Fix TLS does not get updated when changed --- core/pkg/ingress/controller/controller.go | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index 29b2c30d3f..650fcc2a5d 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -26,6 +26,7 @@ import ( "time" "github.com/golang/glog" + "github.com/kylelemons/godebug/pretty" "k8s.io/kubernetes/pkg/api" "k8s.io/kubernetes/pkg/apis/extensions" @@ -178,6 +179,7 @@ func newIngressController(config *Configuration) *GenericController { ic.syncQueue.Enqueue(obj) }, UpdateFunc: func(old, cur interface{}) { + oldIng := old.(*extensions.Ingress) curIng := cur.(*extensions.Ingress) if !IsValidClass(curIng, config.IngressClass) { return @@ -186,6 +188,24 @@ func newIngressController(config *Configuration) *GenericController { if !reflect.DeepEqual(old, cur) { upIng := cur.(*extensions.Ingress) ic.recorder.Eventf(upIng, api.EventTypeNormal, "UPDATE", fmt.Sprintf("Ingress %s/%s", upIng.Namespace, upIng.Name)) + // the referenced secret is different? + if diff := pretty.Compare(curIng.Spec.TLS, oldIng.Spec.TLS); diff != "" { + for _, secretName := range curIng.Spec.TLS { + secKey := fmt.Sprintf("%v/%v", curIng.Namespace, secretName.SecretName) + go func() { + glog.Infof("TLS section in ingress %v/%v changed (secret is now %v)", upIng.Namespace, upIng.Name, secKey) + // we need to wait until the ingress store is updated + time.Sleep(10 * time.Second) + key, err := ic.GetSecret(secKey) + if err != nil { + glog.Errorf("unexpected error: %v", err) + } + if key != nil { + ic.secretQueue.Enqueue(key) + } + }() + } + } ic.syncQueue.Enqueue(cur) } }, From ec67f83305f188dd9753a0d9f731314de973f9ab Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 26 Jan 2017 00:10:33 -0300 Subject: [PATCH 10/11] Refactoring sysctlFSFileMax helper --- controllers/nginx/pkg/cmd/controller/utils.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/controllers/nginx/pkg/cmd/controller/utils.go b/controllers/nginx/pkg/cmd/controller/utils.go index c6e1979f94..3a3b048234 100644 --- a/controllers/nginx/pkg/cmd/controller/utils.go +++ b/controllers/nginx/pkg/cmd/controller/utils.go @@ -20,6 +20,7 @@ import ( "io/ioutil" "os" "os/exec" + "syscall" "k8s.io/kubernetes/pkg/util/sysctl" @@ -42,14 +43,14 @@ func sysctlSomaxconn() int { // sysctlFSFileMax returns the value of fs.file-max, i.e. // maximum number of open file descriptors func sysctlFSFileMax() int { - maxConns, err := sysctl.New().GetSysctl("fs/file-max") + var rLimit syscall.Rlimit + err := syscall.Getrlimit(syscall.RLIMIT_NOFILE, &rLimit) if err != nil { - glog.Errorf("unexpected error reading system maximum number of open file descriptors (fs.file-max): %v", err) + glog.Errorf("unexpected error reading system maximum number of open file descriptors (RLIMIT_NOFILE): %v", err) // returning 0 means don't render the value return 0 } - - return maxConns + return int(rLimit.Max) } func diff(b1, b2 []byte) ([]byte, error) { From 9c73ca40d0939fe48ccc32fb8aad3f80405df6f6 Mon Sep 17 00:00:00 2001 From: Ricardo Pchevuzinske Katz Date: Thu, 26 Jan 2017 14:54:34 -0200 Subject: [PATCH 11/11] Changes the behaviour of default ssl cert --- .../rootfs/etc/nginx/template/nginx.tmpl | 2 +- core/pkg/ingress/controller/controller.go | 27 +++++++++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl index 5f10d030bc..4f05e9faf5 100644 --- a/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl +++ b/controllers/nginx/rootfs/etc/nginx/template/nginx.tmpl @@ -203,7 +203,7 @@ http { server_name {{ $server.Hostname }}; listen [::]:80{{ if $cfg.UseProxyProtocol }} proxy_protocol{{ end }}{{ if eq $index 0 }} ipv6only=off{{end}}{{ if eq $server.Hostname "_"}} default_server reuseport backlog={{ $backlogSize }}{{end}}; {{/* Listen on 442 because port 443 is used in the stream section */}} - {{ if not (empty $server.SSLCertificate) }}listen 442 {{ if $cfg.UseProxyProtocol }}proxy_protocol{{ end }} ssl {{ if $cfg.UseHTTP2 }}http2{{ end }}; + {{ if not (empty $server.SSLCertificate) }}listen 442 {{ if $cfg.UseProxyProtocol }}proxy_protocol{{ end }} {{ if eq $server.Hostname "_"}} default_server reuseport backlog={{ $backlogSize }}{{end}} ssl {{ if $cfg.UseHTTP2 }}http2{{ end }}; {{/* comment PEM sha is required to detect changes in the generated configuration and force a reload */}} # PEM sha: {{ $server.SSLPemChecksum }} ssl_certificate {{ $server.SSLCertificate }}; diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index 650fcc2a5d..3a54776a56 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -47,6 +47,7 @@ import ( "k8s.io/ingress/core/pkg/ingress/resolver" "k8s.io/ingress/core/pkg/ingress/status" "k8s.io/ingress/core/pkg/k8s" + ssl "k8s.io/ingress/core/pkg/net/ssl" local_strings "k8s.io/ingress/core/pkg/strings" "k8s.io/ingress/core/pkg/task" ) @@ -827,9 +828,30 @@ func (ic *GenericController) createServers(data []interface{}, upstreams map[str dun := ic.getDefaultUpstream().Name + // This adds the Default Certificate to Default Backend and also for vhosts missing the secret + var defaultPemFileName, defaultPemSHA string + defaultCertificate, err := ic.getPemCertificate(ic.cfg.DefaultSSLCertificate) + // If no default Certificate was supplied, tries to generate a new dumb one + if err != nil { + var cert *ingress.SSLCert + defCert, defKey := ssl.GetFakeSSLCert() + cert, err = ssl.AddOrUpdateCertAndKey("system-snake-oil-certificate", defCert, defKey, []byte{}) + if err != nil { + glog.Fatalf("Error generating self signed certificate: %v", err) + } else { + defaultPemFileName = cert.PemFileName + defaultPemSHA = cert.PemSHA + } + } else { + defaultPemFileName = defaultCertificate.PemFileName + defaultPemSHA = defaultCertificate.PemSHA + } + // default server servers[defServerName] = &ingress.Server{ - Hostname: defServerName, + Hostname: defServerName, + SSLCertificate: defaultPemFileName, + SSLPemChecksum: defaultPemSHA, Locations: []*ingress.Location{ { Path: rootLocation, @@ -899,7 +921,8 @@ func (ic *GenericController) createServers(data []interface{}, upstreams map[str servers[host].SSLPemChecksum = cert.PemSHA } } else { - glog.Warningf("secret %v does not exists", key) + servers[host].SSLCertificate = defaultPemFileName + servers[host].SSLPemChecksum = defaultPemSHA } }