From 088b67709371288c78decef4ca687ee0180ecfe9 Mon Sep 17 00:00:00 2001 From: Michael Pleshakov Date: Wed, 9 Aug 2017 14:14:41 +0100 Subject: [PATCH] Add watching for secret resource updates - Feature: The Ingress controller now watches for updates of Secrets with a TLS certificate and a key. If a Secret is updated and it is referenced by any deployed Ingress resource, the Ingress controller will update NGINX configuration. - Change: If a Secret referenced by one or more Ingress resources becomes invalid or gets removed, the configuration for those Ingress resources will be disabled until there is a valid Secret. --- nginx-controller/controller/controller.go | 161 ++++++++++++++++++++-- nginx-controller/controller/secret.go | 20 +++ nginx-controller/controller/utils.go | 4 + nginx-controller/nginx/configurator.go | 109 ++++++++++----- nginx-controller/nginx/nginx.go | 35 +++-- 5 files changed, 269 insertions(+), 60 deletions(-) create mode 100644 nginx-controller/controller/secret.go diff --git a/nginx-controller/controller/controller.go b/nginx-controller/controller/controller.go index 30e31dc429..3c2c7f2b64 100644 --- a/nginx-controller/controller/controller.go +++ b/nginx-controller/controller/controller.go @@ -52,10 +52,12 @@ type LoadBalancerController struct { svcController cache.Controller endpController cache.Controller cfgmController cache.Controller + secrController cache.Controller ingLister StoreToIngressLister svcLister cache.Store endpLister StoreToEndpointLister cfgmLister StoreToConfigMapLister + secrLister StoreToSecretLister syncQueue *taskQueue stopCh chan struct{} cnf *nginx.Configurator @@ -200,8 +202,49 @@ func NewLoadBalancerController(kubeClient kubernetes.Interface, resyncPeriod tim cache.NewListWatchFromClient(lbc.client.Core().RESTClient(), "endpoints", namespace, fields.Everything()), &api_v1.Endpoints{}, resyncPeriod, endpHandlers) + secrHandlers := cache.ResourceEventHandlerFuncs{ + DeleteFunc: func(obj interface{}) { + remSecr, isSecr := obj.(*api_v1.Secret) + if !isSecr { + deletedState, ok := obj.(cache.DeletedFinalStateUnknown) + if !ok { + glog.V(3).Infof("Error received unexpected object: %v", obj) + return + } + remSecr, ok = deletedState.Obj.(*api_v1.Secret) + if !ok { + glog.V(3).Infof("Error DeletedFinalStateUnknown contained non-Secret object: %v", deletedState.Obj) + return + } + } + if err := ValidateTLSSecret(remSecr); err != nil { + return + } + + glog.V(3).Infof("Removing Secret: %v", remSecr.Name) + lbc.syncQueue.enqueue(obj) + }, + UpdateFunc: func(old, cur interface{}) { + errOld := ValidateTLSSecret(old.(*api_v1.Secret)) + errCur := ValidateTLSSecret(cur.(*api_v1.Secret)) + if errOld != nil && errCur != nil { + return + } + + if !reflect.DeepEqual(old, cur) { + glog.V(3).Infof("Secret %v changed, syncing", + cur.(*api_v1.Secret).Name) + lbc.syncQueue.enqueue(cur) + } + }, + } + + lbc.secrLister.Store, lbc.secrController = cache.NewInformer( + cache.NewListWatchFromClient(lbc.client.Core().RESTClient(), "secrets", namespace, fields.Everything()), + &api_v1.Secret{}, resyncPeriod, secrHandlers) + if nginxConfigMaps != "" { - nginxConfigMapsNS, nginxConfigMapsName, err := parseNginxConfigMaps(nginxConfigMaps) + nginxConfigMapsNS, nginxConfigMapsName, err := parseNamespaceName(nginxConfigMaps) if err != nil { glog.Warning(err) } else { @@ -259,6 +302,7 @@ func (lbc *LoadBalancerController) Run() { go lbc.ingController.Run(lbc.stopCh) go lbc.svcController.Run(lbc.stopCh) go lbc.endpController.Run(lbc.stopCh) + go lbc.secrController.Run(lbc.stopCh) go lbc.syncQueue.run(time.Second, lbc.stopCh) if lbc.watchNginxConfigMaps { go lbc.cfgmController.Run(lbc.stopCh) @@ -296,8 +340,7 @@ func (lbc *LoadBalancerController) syncEndp(task Task) { continue } glog.V(3).Infof("Updating Endpoints for %v/%v", ing.Name, ing.Namespace) - name := ing.Namespace + "-" + ing.Name - lbc.cnf.UpdateEndpoints(name, ingEx) + lbc.cnf.UpdateEndpoints(ingEx) if err != nil { glog.Errorf("Error updating endpoints for %v/%v: %v", ing.Namespace, ing.Name, err) } @@ -507,7 +550,7 @@ func (lbc *LoadBalancerController) syncCfgm(task Task) { var ingExes []*nginx.IngressEx ings, _ := lbc.ingLister.List() - for i, _ := range ings.Items { + for i := range ings.Items { if !isNginxIngress(&ings.Items[i]) { continue } @@ -551,6 +594,8 @@ func (lbc *LoadBalancerController) sync(task Task) { case Endpoints: lbc.syncEndp(task) return + case Secret: + lbc.syncSecret(task) } } @@ -562,14 +607,11 @@ func (lbc *LoadBalancerController) syncIng(task Task) { return } - // defaut/some-ingress -> default-some-ingress - name := strings.Replace(key, "/", "-", -1) - if !ingExists { glog.V(2).Infof("Deleting Ingress: %v\n", key) - err := lbc.cnf.DeleteIngress(name) + err := lbc.cnf.DeleteIngress(key) if err != nil { - glog.Errorf("Error when deleting configuration for %v: %v", name, err) + glog.Errorf("Error when deleting configuration for %v: %v", key, err) } } else { glog.V(2).Infof("Adding or Updating Ingress: %v\n", key) @@ -581,7 +623,7 @@ func (lbc *LoadBalancerController) syncIng(task Task) { lbc.recorder.Eventf(ing, api_v1.EventTypeWarning, "Rejected", "%v was rejected: %v", key, err) return } - err = lbc.cnf.AddOrUpdateIngress(name, ingEx) + err = lbc.cnf.AddOrUpdateIngress(ingEx) if err != nil { lbc.recorder.Eventf(ing, api_v1.EventTypeWarning, "AddedOrUpdatedWithError", "Configuration for %v was added or updated, but not applied: %v", key, err) } else { @@ -590,6 +632,95 @@ func (lbc *LoadBalancerController) syncIng(task Task) { } } +func (lbc *LoadBalancerController) syncSecret(task Task) { + key := task.Key + obj, secrExists, err := lbc.secrLister.Store.GetByKey(key) + if err != nil { + lbc.syncQueue.requeue(task, err) + return + } + + _, name, err := parseNamespaceName(key) + if err != nil { + glog.Warningf("Secret key %v is invalid: %v", key, err) + return + } + + ings, err := lbc.findIngressesForSecret(name) + if err != nil { + glog.Warningf("Failed to find Ingress resources for Secret %v: %v", key, err) + lbc.syncQueue.requeueAfter(task, err, 5*time.Second) + } + + glog.V(2).Infof("Found %v Ingress resources with Secret %v", len(ings), key) + + if !secrExists { + glog.V(2).Infof("Deleting Secret: %v\n", key) + + if err := lbc.cnf.DeleteTLSSecret(key, ings); err != nil { + glog.Errorf("Error when deleting Secret: %v: %v", key, err) + } + + for _, ing := range ings { + lbc.syncQueue.enqueue(&ing) + lbc.recorder.Eventf(&ing, api_v1.EventTypeWarning, "Rejected", "%v/%v was rejected due to deleted Secret %v: %v", ing.Namespace, ing.Name, key) + } + } else { + glog.V(2).Infof("Updating Secret: %v\n", key) + + secret := obj.(*api_v1.Secret) + + if len(ings) > 0 { + err := ValidateTLSSecret(secret) + if err != nil { + glog.Errorf("Couldn't validate secret %v: %v", key, err) + if err := lbc.cnf.DeleteTLSSecret(key, ings); err != nil { + glog.Errorf("Error when deleting Secret: %v: %v", key, err) + } + for _, ing := range ings { + lbc.syncQueue.enqueue(&ing) + lbc.recorder.Eventf(&ing, api_v1.EventTypeWarning, "Rejected", "%v/%v was rejected due to invalid Secret %v: %v", ing.Namespace, ing.Name, key, err) + } + lbc.recorder.Eventf(secret, api_v1.EventTypeWarning, "Rejected", "%v was rejected: %v", key, err) + return + } + + if err := lbc.cnf.AddOrUpdateTLSSecret(secret, true); err != nil { + glog.Errorf("Error when updating Secret %v: %v", key, err) + lbc.recorder.Eventf(secret, api_v1.EventTypeWarning, "UpdatedWithError", "%v was updated, but not applied: %v", key, err) + for _, ing := range ings { + lbc.recorder.Eventf(&ing, api_v1.EventTypeWarning, "UpdatedWithError", "Configuration for %v/%v was updated, but not applied: %v", ing.Namespace, ing.Name, err) + } + } else { + lbc.recorder.Eventf(secret, api_v1.EventTypeNormal, "Updated", "%v was updated", key) + for _, ing := range ings { + lbc.recorder.Eventf(&ing, api_v1.EventTypeNormal, "Updated", "Configuration for %v/%v was updated", ing.Namespace, ing.Name) + } + } + } + } +} + +func (lbc *LoadBalancerController) findIngressesForSecret(secret string) ([]extensions.Ingress, error) { + res := []extensions.Ingress{} + ings, err := lbc.ingLister.List() + if err != nil { + return nil, fmt.Errorf("Couldn't get the list of Ingress resources: %v", err) + } + for _, ing := range ings.Items { + if !isNginxIngress(&ing) { + continue + } + for _, tls := range ing.Spec.TLS { + if tls.SecretName == secret { + res = append(res, ing) + } + } + } + + return res, nil +} + func (lbc *LoadBalancerController) enqueueIngressForService(svc *api_v1.Service) { ings := lbc.getIngressesForService(svc) for _, ing := range ings { @@ -636,6 +767,10 @@ func (lbc *LoadBalancerController) createIngress(ing *extensions.Ingress) (*ngin if err != nil { return nil, fmt.Errorf("Error retrieving secret %v for Ingress %v: %v", secretName, ing.Name, err) } + err = ValidateTLSSecret(secret) + if err != nil { + return nil, fmt.Errorf("Error validating secret %v for Ingress %v: %v", secretName, ing.Name, err) + } ingEx.Secrets[secretName] = secret } @@ -768,10 +903,10 @@ func (lbc *LoadBalancerController) getServiceForIngressBackend(backend *extensio return nil, fmt.Errorf("service %s doesn't exists", svcKey) } -func parseNginxConfigMaps(nginxConfigMaps string) (string, string, error) { - res := strings.Split(nginxConfigMaps, "/") +func parseNamespaceName(value string) (ns string, name string, err error) { + res := strings.Split(value, "/") if len(res) != 2 { - return "", "", fmt.Errorf("NGINX configmaps name must follow the format /, got: %v", nginxConfigMaps) + return "", "", fmt.Errorf("%v must follow the format /", value) } return res[0], res[1], nil } diff --git a/nginx-controller/controller/secret.go b/nginx-controller/controller/secret.go new file mode 100644 index 0000000000..5ab63f0134 --- /dev/null +++ b/nginx-controller/controller/secret.go @@ -0,0 +1,20 @@ +package controller + +import ( + "fmt" + + api_v1 "k8s.io/client-go/pkg/api/v1" +) + +// ValidateTLSSecret validates the secret. If it is valid, the function returns nil. +func ValidateTLSSecret(secret *api_v1.Secret) error { + if _, exists := secret.Data[api_v1.TLSCertKey]; !exists { + return fmt.Errorf("Secret doesn't have %v", api_v1.TLSCertKey) + } + + if _, exists := secret.Data[api_v1.TLSPrivateKeyKey]; !exists { + return fmt.Errorf("Secret doesn't have %v", api_v1.TLSPrivateKeyKey) + } + + return nil +} diff --git a/nginx-controller/controller/utils.go b/nginx-controller/controller/utils.go index b8eba9f8a1..fa7ef868da 100644 --- a/nginx-controller/controller/utils.go +++ b/nginx-controller/controller/utils.go @@ -118,6 +118,8 @@ const ( Endpoints // ConfigMap resource ConfigMap + // Secret resource + Secret ) // Task is an element of a taskQueue @@ -136,6 +138,8 @@ func NewTask(key string, obj interface{}) (Task, error) { k = Endpoints case *api_v1.ConfigMap: k = ConfigMap + case *api_v1.Secret: + k = Secret default: return Task{}, fmt.Errorf("Unknow type: %v", t) } diff --git a/nginx-controller/nginx/configurator.go b/nginx-controller/nginx/configurator.go index 2750e122e2..fa1770a22c 100644 --- a/nginx-controller/nginx/configurator.go +++ b/nginx-controller/nginx/configurator.go @@ -1,11 +1,13 @@ package nginx import ( + "bytes" "fmt" "strings" "github.com/golang/glog" "github.com/nginxinc/kubernetes-ingress/nginx-controller/nginx/plus" + meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1" api_v1 "k8s.io/client-go/pkg/api/v1" extensions "k8s.io/client-go/pkg/apis/extensions/v1beta1" ) @@ -34,19 +36,20 @@ func (cnf *Configurator) AddOrUpdateDHParam(content string) (string, error) { return cnf.nginx.AddOrUpdateDHParam(content) } -// AddOrUpdateIngress adds or updates NGINX configuration for an Ingress resource -func (cnf *Configurator) AddOrUpdateIngress(name string, ingEx *IngressEx) error { - cnf.addOrUpdateIngress(name, ingEx) +// AddOrUpdateIngress adds or updates NGINX configuration for the Ingress resource +func (cnf *Configurator) AddOrUpdateIngress(ingEx *IngressEx) error { + cnf.addOrUpdateIngress(ingEx) if err := cnf.nginx.Reload(); err != nil { - return fmt.Errorf("Error when adding or updating ingress %v: %v", name, err) + return fmt.Errorf("Error when adding or updating ingress %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } return nil } -func (cnf *Configurator) addOrUpdateIngress(name string, ingEx *IngressEx) { +func (cnf *Configurator) addOrUpdateIngress(ingEx *IngressEx) { pems := cnf.updateCertificates(ingEx) nginxCfg := cnf.generateNginxCfg(ingEx, pems) + name := objectMetaToFileName(&ingEx.Ingress.ObjectMeta) cnf.nginx.AddOrUpdateIngress(name, nginxCfg) } @@ -55,23 +58,8 @@ func (cnf *Configurator) updateCertificates(ingEx *IngressEx) map[string]string for _, tls := range ingEx.Ingress.Spec.TLS { secretName := tls.SecretName - secret, exist := ingEx.Secrets[secretName] - if !exist { - continue - } - cert, ok := secret.Data[api_v1.TLSCertKey] - if !ok { - glog.Warningf("Secret %v has no cert", secretName) - continue - } - key, ok := secret.Data[api_v1.TLSPrivateKeyKey] - if !ok { - glog.Warningf("Secret %v has no private key", secretName) - continue - } - name := ingEx.Ingress.Namespace + "-" + secretName - pemFileName := cnf.nginx.AddOrUpdateCertAndKey(name, string(cert), string(key)) + pemFileName := cnf.addOrUpdateTLSSecret(ingEx.Secrets[secretName]) for _, host := range tls.Hosts { pems[host] = pemFileName @@ -478,30 +466,77 @@ func upstreamMapToSlice(upstreams map[string]Upstream) []Upstream { return result } -// DeleteIngress deletes NGINX configuration for an Ingress resource -func (cnf *Configurator) DeleteIngress(name string) error { - cnf.nginx.DeleteIngress(name) +// AddOrUpdateTLSSecret creates or updates a file with the content of the TLS secret +func (cnf *Configurator) AddOrUpdateTLSSecret(secret *api_v1.Secret, reload bool) error { + cnf.addOrUpdateTLSSecret(secret) + + if !reload { + return nil + } + if err := cnf.nginx.Reload(); err != nil { - return fmt.Errorf("Error when removing ingress %v: %v", name, err) + return fmt.Errorf("Error when reloading NGINX when updating Secret: %v", err) } return nil } -// UpdateEndpoints updates endpoints in NGINX configuration for an Ingress resource -func (cnf *Configurator) UpdateEndpoints(name string, ingEx *IngressEx) error { +func (cnf *Configurator) addOrUpdateTLSSecret(secret *api_v1.Secret) string { + name := objectMetaToFileName(&secret.ObjectMeta) + data := generateCertAndKeyFileContent(secret) + return cnf.nginx.AddOrUpdatePemFile(name, data) +} + +func generateCertAndKeyFileContent(secret *api_v1.Secret) []byte { + var res bytes.Buffer + + res.Write(secret.Data[api_v1.TLSCertKey]) + res.WriteString("\n") + res.Write(secret.Data[api_v1.TLSPrivateKeyKey]) + + return res.Bytes() +} + +// DeleteTLSSecret deletes the file associated with the TLS secret and the configuration files for the Ingress resources. NGINX is reloaded only when len(ings) > 0 +func (cnf *Configurator) DeleteTLSSecret(key string, ings []extensions.Ingress) error { + for _, ing := range ings { + cnf.nginx.DeleteIngress(objectMetaToFileName(&ing.ObjectMeta)) + } + + cnf.nginx.DeletePemFile(keyToFileName(key)) + + if len(ings) > 0 { + if err := cnf.nginx.Reload(); err != nil { + return fmt.Errorf("Error when reloading NGINX when deleting Secret %v: %v", key, err) + } + } + + return nil +} + +// DeleteIngress deletes NGINX configuration for the Ingress resource +func (cnf *Configurator) DeleteIngress(key string) error { + cnf.nginx.DeleteIngress(keyToFileName(key)) + if err := cnf.nginx.Reload(); err != nil { + return fmt.Errorf("Error when removing ingress %v: %v", key, err) + } + return nil +} + +// UpdateEndpoints updates endpoints in NGINX configuration for the Ingress resource +func (cnf *Configurator) UpdateEndpoints(ingEx *IngressEx) error { + cnf.addOrUpdateIngress(ingEx) + if cnf.isPlus() { - cnf.addOrUpdateIngress(name, ingEx) - cnf.updatePlusEndpoints(name, ingEx) + cnf.updatePlusEndpoints(ingEx) } else { - cnf.addOrUpdateIngress(name, ingEx) if err := cnf.nginx.Reload(); err != nil { - return fmt.Errorf("Error reloading NGINX when updating endpoints for %v: %v", name, err) + return fmt.Errorf("Error reloading NGINX when updating endpoints for %v/%v: %v", ingEx.Ingress.Namespace, ingEx.Ingress.Name, err) } } return nil } -func (cnf *Configurator) updatePlusEndpoints(name string, ingEx *IngressEx) { +func (cnf *Configurator) updatePlusEndpoints(ingEx *IngressEx) { if ingEx.Ingress.Spec.Backend != nil { name := getNameForUpstream(ingEx.Ingress, emptyHost, ingEx.Ingress.Spec.Backend.ServiceName) endps, exists := ingEx.Endpoints[ingEx.Ingress.Spec.Backend.ServiceName+ingEx.Ingress.Spec.Backend.ServicePort.String()] @@ -546,7 +581,7 @@ func (cnf *Configurator) UpdateConfig(config *Config, ingExes []*IngressEx) erro cnf.nginx.UpdateMainConfigFile(mainCfg) for _, ingEx := range ingExes { - cnf.addOrUpdateIngress(ingEx.Ingress.Namespace+"-"+ingEx.Ingress.Name, ingEx) + cnf.addOrUpdateIngress(ingEx) } if err := cnf.nginx.Reload(); err != nil { @@ -559,3 +594,11 @@ func (cnf *Configurator) UpdateConfig(config *Config, ingExes []*IngressEx) erro func (cnf *Configurator) isPlus() bool { return cnf.nginxAPI != nil } + +func keyToFileName(key string) string { + return strings.Replace(key, "/", "-", -1) +} + +func objectMetaToFileName(meta *meta_v1.ObjectMeta) string { + return meta.Namespace + "-" + meta.Name +} diff --git a/nginx-controller/nginx/nginx.go b/nginx-controller/nginx/nginx.go index 1a30aa57a2..c8152a291a 100644 --- a/nginx-controller/nginx/nginx.go +++ b/nginx-controller/nginx/nginx.go @@ -169,10 +169,10 @@ func (nginx *NginxController) AddOrUpdateDHParam(dhparam string) (string, error) return fileName, nil } -// AddOrUpdateCertAndKey creates a .pem file wth the cert and the key with the +// AddOrUpdatePemFile creates a .pem file wth the cert and the key with the // specified name -func (nginx *NginxController) AddOrUpdateCertAndKey(name string, cert string, key string) string { - pemFileName := nginx.nginxCertsPath + "/" + name + ".pem" +func (nginx *NginxController) AddOrUpdatePemFile(name string, content []byte) string { + pemFileName := nginx.getPemFileName(name) if !nginx.local { pem, err := ioutil.TempFile(nginx.nginxCertsPath, name) @@ -180,17 +180,7 @@ func (nginx *NginxController) AddOrUpdateCertAndKey(name string, cert string, ke glog.Fatalf("Couldn't create a temp file for the pem file %v: %v", name, err) } - _, err = pem.WriteString(key) - if err != nil { - glog.Fatalf("Couldn't write to the temp pem file %v: %v", pem.Name(), err) - } - - _, err = pem.WriteString("\n") - if err != nil { - glog.Fatalf("Couldn't write to the temp pem file %v: %v", pem.Name(), err) - } - - _, err = pem.WriteString(cert) + _, err = pem.Write(content) if err != nil { glog.Fatalf("Couldn't write to the temp pem file %v: %v", pem.Name(), err) } @@ -209,10 +199,27 @@ func (nginx *NginxController) AddOrUpdateCertAndKey(name string, cert string, ke return pemFileName } +// DeletePemFile deletes the pem file +func (nginx *NginxController) DeletePemFile(name string) { + pemFileName := nginx.getPemFileName(name) + glog.V(3).Infof("deleting %v", pemFileName) + + if !nginx.local { + if err := os.Remove(pemFileName); err != nil { + glog.Warningf("Failed to delete %v: %v", pemFileName, err) + } + } + +} + func (nginx *NginxController) getIngressNginxConfigFileName(name string) string { return path.Join(nginx.nginxConfdPath, name+".conf") } +func (nginx *NginxController) getPemFileName(name string) string { + return path.Join(nginx.nginxCertsPath, name+".pem") +} + func (nginx *NginxController) templateIt(config IngressNginxConfig, filename string) { tmpl, err := template.New(nginx.nginxIngressTempatePath).ParseFiles(nginx.nginxIngressTempatePath) if err != nil {