diff --git a/internal/ingress/controller/store/backend_ssl.go b/internal/ingress/controller/store/backend_ssl.go index 57ce68cf54..e593f564fc 100644 --- a/internal/ingress/controller/store/backend_ssl.go +++ b/internal/ingress/controller/store/backend_ssl.go @@ -35,9 +35,9 @@ import ( // syncSecret synchronizes the content of a TLS Secret (certificate(s), secret // key) with the filesystem. The resulting files can be used by NGINX. -func (s k8sStore) syncSecret(key string) { - s.mu.Lock() - defer s.mu.Unlock() +func (s *k8sStore) syncSecret(key string) { + s.syncSecretMu.Lock() + defer s.syncSecretMu.Unlock() klog.V(3).Infof("Syncing Secret %q", key) @@ -74,7 +74,7 @@ func (s k8sStore) syncSecret(key string) { // getPemCertificate receives a secret, and creates a ingress.SSLCert as return. // It parses the secret and verifies if it's a keypair, or a 'ca.crt' secret only. -func (s k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error) { +func (s *k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error) { secret, err := s.listers.Secret.ByKey(secretName) if err != nil { return nil, err @@ -143,7 +143,7 @@ func (s k8sStore) getPemCertificate(secretName string) (*ingress.SSLCert, error) return sslCert, nil } -func (s k8sStore) checkSSLChainIssues() { +func (s *k8sStore) checkSSLChainIssues() { for _, item := range s.ListLocalSSLCerts() { secrKey := k8s.MetaNamespaceKey(item) secret, err := s.GetLocalSSLCert(secrKey) diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index ce67ef3c4b..db2066cf1f 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -211,8 +211,11 @@ type k8sStore struct { // updateCh updateCh *channels.RingChannel - // mu protects against simultaneous invocations of syncSecret - mu *sync.Mutex + // syncSecretMu protects against simultaneous invocations of syncSecret + syncSecretMu *sync.Mutex + + // backendConfigMu protects against simultaneous read/write of backendConfig + backendConfigMu *sync.RWMutex defaultSSLCertificate string @@ -239,7 +242,8 @@ func New(checkOCSP bool, filesystem: fs, updateCh: updateCh, backendConfig: ngx_config.NewDefault(), - mu: &sync.Mutex{}, + syncSecretMu: &sync.Mutex{}, + backendConfigMu: &sync.RWMutex{}, secretIngressMap: NewObjectRefMap(), defaultSSLCertificate: defaultSSLCertificate, isDynamicCertificatesEnabled: isDynamicCertificatesEnabled, @@ -363,6 +367,9 @@ func New(checkOCSP bool, recorder.Eventf(curIng, corev1.EventTypeNormal, "DELETE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) } else if validCur && !reflect.DeepEqual(old, cur) { recorder.Eventf(curIng, corev1.EventTypeNormal, "UPDATE", fmt.Sprintf("Ingress %s/%s", curIng.Namespace, curIng.Name)) + } else { + klog.Infof("ignoring ingress %v based on annotation %v", curIng.Name, class.IngressKey) + return } store.syncIngress(curIng) @@ -682,7 +689,7 @@ func objectRefAnnotationNsKey(ann string, ing *extensions.Ingress) (string, erro // syncSecrets synchronizes data from all Secrets referenced by the given // Ingress with the local store and file system. -func (s k8sStore) syncSecrets(ing *extensions.Ingress) { +func (s *k8sStore) syncSecrets(ing *extensions.Ingress) { key := k8s.MetaNamespaceKey(ing) for _, secrKey := range s.secretIngressMap.ReferencedBy(key) { s.syncSecret(secrKey) @@ -690,12 +697,12 @@ func (s k8sStore) syncSecrets(ing *extensions.Ingress) { } // GetSecret returns the Secret matching key. -func (s k8sStore) GetSecret(key string) (*corev1.Secret, error) { +func (s *k8sStore) GetSecret(key string) (*corev1.Secret, error) { return s.listers.Secret.ByKey(key) } // ListLocalSSLCerts returns the list of local SSLCerts -func (s k8sStore) ListLocalSSLCerts() []*ingress.SSLCert { +func (s *k8sStore) ListLocalSSLCerts() []*ingress.SSLCert { var certs []*ingress.SSLCert for _, item := range s.sslStore.List() { if s, ok := item.(*ingress.SSLCert); ok { @@ -707,12 +714,12 @@ func (s k8sStore) ListLocalSSLCerts() []*ingress.SSLCert { } // GetService returns the Service matching key. -func (s k8sStore) GetService(key string) (*corev1.Service, error) { +func (s *k8sStore) GetService(key string) (*corev1.Service, error) { return s.listers.Service.ByKey(key) } // getIngress returns the Ingress matching key. -func (s k8sStore) getIngress(key string) (*extensions.Ingress, error) { +func (s *k8sStore) getIngress(key string) (*extensions.Ingress, error) { ing, err := s.listers.IngressWithAnnotation.ByKey(key) if err != nil { return nil, err @@ -722,7 +729,7 @@ func (s k8sStore) getIngress(key string) (*extensions.Ingress, error) { } // ListIngresses returns the list of Ingresses -func (s k8sStore) ListIngresses() []*ingress.Ingress { +func (s *k8sStore) ListIngresses() []*ingress.Ingress { // filter ingress rules ingresses := make([]*ingress.Ingress, 0) for _, item := range s.listers.IngressWithAnnotation.List() { @@ -734,22 +741,22 @@ func (s k8sStore) ListIngresses() []*ingress.Ingress { } // GetLocalSSLCert returns the local copy of a SSLCert -func (s k8sStore) GetLocalSSLCert(key string) (*ingress.SSLCert, error) { +func (s *k8sStore) GetLocalSSLCert(key string) (*ingress.SSLCert, error) { return s.sslStore.ByKey(key) } // GetConfigMap returns the ConfigMap matching key. -func (s k8sStore) GetConfigMap(key string) (*corev1.ConfigMap, error) { +func (s *k8sStore) GetConfigMap(key string) (*corev1.ConfigMap, error) { return s.listers.ConfigMap.ByKey(key) } // GetServiceEndpoints returns the Endpoints of a Service matching key. -func (s k8sStore) GetServiceEndpoints(key string) (*corev1.Endpoints, error) { +func (s *k8sStore) GetServiceEndpoints(key string) (*corev1.Endpoints, error) { return s.listers.Endpoint.ByKey(key) } // GetAuthCertificate is used by the auth-tls annotations to get a cert from a secret -func (s k8sStore) GetAuthCertificate(name string) (*resolver.AuthSSLCert, error) { +func (s *k8sStore) GetAuthCertificate(name string) (*resolver.AuthSSLCert, error) { if _, err := s.GetLocalSSLCert(name); err != nil { s.syncSecret(name) } @@ -766,7 +773,7 @@ func (s k8sStore) GetAuthCertificate(name string) (*resolver.AuthSSLCert, error) }, nil } -func (s k8sStore) writeSSLSessionTicketKey(cmap *corev1.ConfigMap, fileName string) { +func (s *k8sStore) writeSSLSessionTicketKey(cmap *corev1.ConfigMap, fileName string) { ticketString := ngx_template.ReadConfig(cmap.Data).SSLSessionTicketKey s.backendConfig.SSLSessionTicketKey = "" @@ -795,22 +802,28 @@ func (s k8sStore) writeSSLSessionTicketKey(cmap *corev1.ConfigMap, fileName stri } // GetDefaultBackend returns the default backend -func (s k8sStore) GetDefaultBackend() defaults.Backend { - return s.backendConfig.Backend +func (s *k8sStore) GetDefaultBackend() defaults.Backend { + return s.GetBackendConfiguration().Backend } -func (s k8sStore) GetBackendConfiguration() ngx_config.Configuration { +func (s *k8sStore) GetBackendConfiguration() ngx_config.Configuration { + s.backendConfigMu.RLock() + defer s.backendConfigMu.RUnlock() + return s.backendConfig } func (s *k8sStore) setConfig(cmap *corev1.ConfigMap) { + s.backendConfigMu.Lock() + defer s.backendConfigMu.Unlock() + s.backendConfig = ngx_template.ReadConfig(cmap.Data) s.writeSSLSessionTicketKey(cmap, "/etc/nginx/tickets.key") } // Run initiates the synchronization of the informers and the initial // synchronization of the secrets. -func (s k8sStore) Run(stopCh chan struct{}) { +func (s *k8sStore) Run(stopCh chan struct{}) { // start informers s.informers.Run(stopCh) @@ -820,7 +833,7 @@ func (s k8sStore) Run(stopCh chan struct{}) { } // ListControllerPods returns a list of ingress-nginx controller Pods -func (s k8sStore) ListControllerPods() []*corev1.Pod { +func (s *k8sStore) ListControllerPods() []*corev1.Pod { var pods []*corev1.Pod for _, i := range s.listers.Pod.List() { diff --git a/internal/ingress/controller/store/store_test.go b/internal/ingress/controller/store/store_test.go index 3f7519bd06..7530f66a4c 100644 --- a/internal/ingress/controller/store/store_test.go +++ b/internal/ingress/controller/store/store_test.go @@ -265,6 +265,118 @@ func TestStore(t *testing.T) { } }) + t.Run("should not receive updates for ingress with invalid class", func(t *testing.T) { + ns := createNamespace(clientSet, t) + defer deleteNamespace(ns, clientSet, t) + cm := createConfigMap(clientSet, ns, t) + defer deleteConfigMap(cm, ns, clientSet, t) + + stopCh := make(chan struct{}) + updateCh := channels.NewRingChannel(1024) + + var add uint64 + var upd uint64 + var del uint64 + + go func(ch *channels.RingChannel) { + for { + evt, ok := <-ch.Out() + if !ok { + return + } + + e := evt.(Event) + if e.Obj == nil { + continue + } + if _, ok := e.Obj.(*extensions.Ingress); !ok { + continue + } + + switch e.Type { + case CreateEvent: + atomic.AddUint64(&add, 1) + case UpdateEvent: + atomic.AddUint64(&upd, 1) + case DeleteEvent: + atomic.AddUint64(&del, 1) + } + } + }(updateCh) + + fs := newFS(t) + storer := New(true, + ns, + fmt.Sprintf("%v/config", ns), + fmt.Sprintf("%v/tcp", ns), + fmt.Sprintf("%v/udp", ns), + "", + 10*time.Minute, + clientSet, + fs, + updateCh, + false, + pod) + + storer.Run(stopCh) + + // create an invalid ingress (different class) + invalidIngress := ensureIngress(&extensions.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "custom-class", + SelfLink: fmt.Sprintf("/apis/extensions/v1beta1/namespaces/%s/ingresses/custom-class", ns), + Namespace: ns, + Annotations: map[string]string{ + "kubernetes.io/ingress.class": "something", + }, + }, + Spec: extensions.IngressSpec{ + Rules: []extensions.IngressRule{ + { + Host: "dummy", + IngressRuleValue: extensions.IngressRuleValue{ + HTTP: &extensions.HTTPIngressRuleValue{ + Paths: []extensions.HTTPIngressPath{ + { + Path: "/", + Backend: extensions.IngressBackend{ + ServiceName: "http-svc", + ServicePort: intstr.FromInt(80), + }, + }, + }, + }, + }, + }, + }, + }, + }, clientSet, t) + err := framework.WaitForIngressInNamespace(clientSet, ns, invalidIngress.Name) + if err != nil { + t.Errorf("error waiting for ingress: %v", err) + } + time.Sleep(1 * time.Second) + + invalidIngressUpdated := invalidIngress.DeepCopy() + invalidIngressUpdated.Spec.Rules[0].Host = "update-dummy" + _ = ensureIngress(invalidIngressUpdated, clientSet, t) + if err != nil { + t.Errorf("error creating ingress: %v", err) + } + // Secret takes a bit to update + time.Sleep(3 * time.Second) + + if atomic.LoadUint64(&add) != 0 { + t.Errorf("expected 0 event of type Create but %v occurred", add) + } + if atomic.LoadUint64(&upd) != 0 { + t.Errorf("expected 0 event of type Update but %v occurred", upd) + } + if atomic.LoadUint64(&del) != 0 { + t.Errorf("expected 0 event of type Delete but %v occurred", del) + } + }) + t.Run("should not receive events from secret not referenced from ingress", func(t *testing.T) { ns := createNamespace(clientSet, t) defer deleteNamespace(ns, clientSet, t) @@ -753,7 +865,8 @@ func newStore(t *testing.T) *k8sStore { sslStore: NewSSLCertTracker(), filesystem: fs, updateCh: channels.NewRingChannel(10), - mu: new(sync.Mutex), + syncSecretMu: new(sync.Mutex), + backendConfigMu: new(sync.RWMutex), secretIngressMap: NewObjectRefMap(), pod: pod, }