From f825445b2d03a0d0ecc792cafebb133adaa4ec7c Mon Sep 17 00:00:00 2001 From: Yves Peter Date: Wed, 9 Aug 2017 21:49:52 +0200 Subject: [PATCH 1/3] Implement flag always-enable-tls --- core/pkg/ingress/controller/controller.go | 133 +++++++++--------- .../pkg/ingress/controller/controller_test.go | 72 ++++++++++ core/pkg/ingress/controller/launch.go | 4 + 3 files changed, 145 insertions(+), 64 deletions(-) create mode 100644 core/pkg/ingress/controller/controller_test.go diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index 5f794d839..670d3b51e 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -131,6 +131,7 @@ type Configuration struct { // optional UDPConfigMapName string DefaultSSLCertificate string + AlwaysEnableTLS bool DefaultHealthzURL string DefaultIngressClass string // optional @@ -1065,76 +1066,80 @@ func (ic *GenericController) createServers(data []interface{}, }, }, SSLPassthrough: sslpt} } - } - // configure default location and SSL - for _, ingIf := range data { - ing := ingIf.(*extensions.Ingress) - if !class.IsValid(ing, ic.cfg.IngressClass, ic.cfg.DefaultIngressClass) { - continue - } - - for _, rule := range ing.Spec.Rules { - host := rule.Host - if host == "" { - host = defServerName - } - - // only add a certificate if the server does not have one previously configured - if len(ing.Spec.TLS) == 0 || servers[host].SSLCertificate != "" { - continue - } - - tlsSecretName := "" - found := false - for _, tls := range ing.Spec.TLS { - if sets.NewString(tls.Hosts...).Has(host) { - tlsSecretName = tls.SecretName - found = true - break - } - } - - // the current ing.Spec.Rules[].Host doesn't have an entry at - // ing.Spec.TLS[].Hosts[] skipping to the next Rule - if !found { - continue - } - - if tlsSecretName == "" { - glog.V(3).Infof("host %v is listed on tls section but secretName is empty. Using default cert", host) - servers[host].SSLCertificate = defaultPemFileName - servers[host].SSLPemChecksum = defaultPemSHA - continue - } - - key := fmt.Sprintf("%v/%v", ing.Namespace, tlsSecretName) - bc, exists := ic.sslCertTracker.Get(key) - if !exists { - glog.Infof("ssl certificate \"%v\" does not exist in local store", key) - continue - } - - cert := bc.(*ingress.SSLCert) - err = cert.Certificate.VerifyHostname(host) - if err != nil { - glog.Warningf("ssl certificate %v does not contain a Common Name or Subject Alternative Name for host %v", key, host) - continue - } - - servers[host].SSLCertificate = cert.PemFileName - servers[host].SSLPemChecksum = cert.PemSHA - servers[host].SSLExpireTime = cert.ExpireTime - - if cert.ExpireTime.Before(time.Now().Add(240 * time.Hour)) { - glog.Warningf("ssl certificate for host %v is about to expire in 10 days", host) - } - } + ic.configureTLSforIng(ing, servers, defaultPemFileName, defaultPemSHA) } return servers } +func (ic *GenericController) configureTLSforIng(ing *extensions.Ingress, servers map[string]*ingress.Server, defaultPemFileName string, defaultPemSHA string) { + for _, rule := range ing.Spec.Rules { + host := rule.Host + if host == "" { + host = defServerName + } + + // only add a certificate if the server does not have one previously configured + if servers[host].SSLCertificate != "" { + continue + } + + tlsForHost := getTLSForHost(ing.Spec.TLS, host) + + if tlsForHost == nil && ic.cfg.AlwaysEnableTLS { + glog.V(3).Infof("no tls host defined for %v but EnforceTLS is true. Using default cert", host) + servers[host].SSLCertificate = defaultPemFileName + servers[host].SSLPemChecksum = defaultPemSHA + continue + } + + // the current ing.Spec.Rules[].Host doesn't have an entry at + // ing.Spec.TLS[].Hosts[] skipping to the next Rule + if tlsForHost == nil { + continue + } + + if tlsForHost.SecretName == "" { + glog.V(3).Infof("host %v is listed on tls section but secretName is empty. Using default cert", host) + servers[host].SSLCertificate = defaultPemFileName + servers[host].SSLPemChecksum = defaultPemSHA + continue + } + + key := fmt.Sprintf("%v/%v", ing.Namespace, tlsForHost.SecretName) + bc, exists := ic.sslCertTracker.Get(key) + if !exists { + glog.Infof("ssl certificate \"%v\" does not exist in local store", key) + continue + } + + cert := bc.(*ingress.SSLCert) + err := cert.Certificate.VerifyHostname(host) + if err != nil { + glog.Warningf("ssl certificate %v does not contain a Common Name or Subject Alternative Name for host %v", key, host) + continue + } + + servers[host].SSLCertificate = cert.PemFileName + servers[host].SSLPemChecksum = cert.PemSHA + servers[host].SSLExpireTime = cert.ExpireTime + + if cert.ExpireTime.Before(time.Now().Add(240 * time.Hour)) { + glog.Warningf("ssl certificate for host %v is about to expire in 10 days", host) + } + } +} + +func getTLSForHost(ingressTLS []extensions.IngressTLS, host string) *extensions.IngressTLS { + for _, tls := range ingressTLS { + if sets.NewString(tls.Hosts...).Has(host) { + return &tls + } + } + return nil +} + // getEndpoints returns a list of : for a given service/target port combination. func (ic *GenericController) getEndpoints( s *api.Service, diff --git a/core/pkg/ingress/controller/controller_test.go b/core/pkg/ingress/controller/controller_test.go new file mode 100644 index 000000000..edf4b6edf --- /dev/null +++ b/core/pkg/ingress/controller/controller_test.go @@ -0,0 +1,72 @@ +package controller + +import ( + "testing" + + extensions "k8s.io/api/extensions/v1beta1" + "k8s.io/ingress/core/pkg/ingress" +) + +func buildGenericControllerForCertTest() *GenericController { + return &GenericController{ + cfg: &Configuration{}, + } +} + +func buildTestIngressRuleForCertTest() extensions.Ingress { + return extensions.Ingress{ + Spec: extensions.IngressSpec{ + Rules: []extensions.IngressRule{ + extensions.IngressRule{ + Host: "hostWithTLS", + }, + extensions.IngressRule{ + Host: "hostWithoutTLS", + }, + }, + TLS: []extensions.IngressTLS{ + extensions.IngressTLS{ + Hosts: []string{"hostWithTLS"}, + }, + }, + }, + } +} + +func buildTestServersForCertTest() map[string]*ingress.Server { + return map[string]*ingress.Server{ + "hostWithTLS": &ingress.Server{ + Hostname: "hostWithTLS", + }, + "hostWithoutTLS": &ingress.Server{ + Hostname: "hostWithoutTLS", + }, + } +} + +func TestConfigureTLSforIng(t *testing.T) { + ic := buildGenericControllerForCertTest() + + testIng := buildTestIngressRuleForCertTest() + testServers := buildTestServersForCertTest() + + defaultPemFileName := "defaultPemFileName" + defaultPemSHA := "defaultPemSHA" + + ic.configureTLSforIng(&testIng, testServers, defaultPemFileName, defaultPemSHA) + if testServers["hostWithTLS"].SSLCertificate != defaultPemFileName { + t.Errorf("SSLCertificate set to %s instead of %s", testServers["hostWithTLS"].SSLCertificate, defaultPemFileName) + } + if testServers["hostWithoutTLS"].SSLCertificate != "" { + t.Errorf("SSLCertificate set to %s instead of being empty", testServers["hostWithoutTLS"].SSLCertificate) + } + + ic.cfg.AlwaysEnableTLS = true + ic.configureTLSforIng(&testIng, testServers, defaultPemFileName, defaultPemSHA) + if testServers["hostWithTLS"].SSLCertificate != defaultPemFileName { + t.Errorf("SSLCertificate set to %s instead of %s", testServers["hostWithTLS"].SSLCertificate, defaultPemFileName) + } + if testServers["hostWithoutTLS"].SSLCertificate != defaultPemFileName { + t.Errorf("SSLCertificate set to %s instead of %s", testServers["hostWithoutTLS"].SSLCertificate, defaultPemFileName) + } +} diff --git a/core/pkg/ingress/controller/launch.go b/core/pkg/ingress/controller/launch.go index c11ef4aec..fd0aa317e 100644 --- a/core/pkg/ingress/controller/launch.go +++ b/core/pkg/ingress/controller/launch.go @@ -78,6 +78,9 @@ func NewIngressController(backend ingress.Controller) *GenericController { 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`) + defAlwaysEnableTLS = flags.Bool("always-enable-tls", false, `Enable tls using the default-ssl-certificate even if no tls section is defined + for the ingress.`) + defHealthzURL = flags.String("health-check-path", "/healthz", `Defines the URL to be used as health check inside in the default server in NGINX.`) @@ -171,6 +174,7 @@ func NewIngressController(backend ingress.Controller) *GenericController { TCPConfigMapName: *tcpConfigMapName, UDPConfigMapName: *udpConfigMapName, DefaultSSLCertificate: *defSSLCertificate, + AlwaysEnableTLS: *defAlwaysEnableTLS, DefaultHealthzURL: *defHealthzURL, PublishService: *publishSvc, Backend: backend, From fdcd4e1cd3c9da5bb70f6993f4260d830535b5db Mon Sep 17 00:00:00 2001 From: Yves Peter Date: Fri, 11 Aug 2017 22:33:03 +0200 Subject: [PATCH 2/3] Added doc for flag always-enable-tls --- controllers/nginx/README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/controllers/nginx/README.md b/controllers/nginx/README.md index 4219988df..7abf01a18 100644 --- a/controllers/nginx/README.md +++ b/controllers/nginx/README.md @@ -50,6 +50,8 @@ Usage of : --configmap string Name of the ConfigMap that contains the custom configuration use --default-backend-service string Service used to serve a 404 page for the default backend. Takes the form namespace/name. The controller uses the first node port of this Service for the default backend. --default-ssl-certificate string Name of the secret that contains a SSL certificate to be used as default for a HTTPS catch-all server + --always-enable-tls bool Enable tls using the default-ssl-certificate even if no tls section is defined + for the ingress. --election-id string Election id to use for status update. (default "ingress-controller-leader") --force-namespace-isolation Force namespace isolation. This flag is required to avoid the reference of secrets or configmaps located in a different namespace than the specified in the flag --watch-namespace. --health-check-path string Defines the URL to be used as health check inside in the default server in NGINX. (default "/healthz") @@ -68,7 +70,7 @@ Usage of : The ports 80 and 443 are not allowed as external ports. This ports are reserved for the backend --udp-services-configmap string 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 number of the name of the port. - --update-status Indicates if the ingress controller should update the Ingress status IP/hostname. Default is true (default true) + --update-status bool Indicates if the ingress controller should update the Ingress status IP/hostname. Default is true (default true) -v, --v Level log level for V logs --vmodule moduleSpec comma-separated list of pattern=N settings for file-filtered logging --watch-namespace string Namespace to watch for Ingress. Default is to watch all namespaces From 4ea8bceb0935ebbc04db221027ee3de30dff1a1e Mon Sep 17 00:00:00 2001 From: Yves Peter Date: Fri, 11 Aug 2017 22:57:06 +0200 Subject: [PATCH 3/3] Fix typo in function name --- core/pkg/ingress/controller/controller.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/pkg/ingress/controller/controller.go b/core/pkg/ingress/controller/controller.go index 670d3b51e..9d17b60f3 100644 --- a/core/pkg/ingress/controller/controller.go +++ b/core/pkg/ingress/controller/controller.go @@ -342,7 +342,7 @@ func (ic GenericController) GetDefaultBackend() defaults.Backend { } // GetRecorder returns the event recorder -func (ic GenericController) GetRecoder() record.EventRecorder { +func (ic GenericController) GetRecorder() record.EventRecorder { return ic.recorder }