diff --git a/nginx-controller/controller/controller.go b/nginx-controller/controller/controller.go index bf4e94da8f..140ebf683b 100644 --- a/nginx-controller/controller/controller.go +++ b/nginx-controller/controller/controller.go @@ -757,13 +757,13 @@ func (lbc *LoadBalancerController) syncSecret(task Task) { return } - _, name, err := ParseNamespaceName(key) + namespace, name, err := ParseNamespaceName(key) if err != nil { glog.Warningf("Secret key %v is invalid: %v", key, err) return } - ings, err := lbc.findIngressesForSecret(name) + ings, err := lbc.findIngressesForSecret(namespace, 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) @@ -780,7 +780,7 @@ func (lbc *LoadBalancerController) syncSecret(task Task) { 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) + lbc.recorder.Eventf(&ing, api_v1.EventTypeWarning, "Rejected", "%v/%v was rejected due to deleted Secret %v", ing.Namespace, ing.Name, key) } if key == lbc.defaultServerSecret { @@ -839,7 +839,7 @@ func (lbc *LoadBalancerController) syncSecret(task Task) { } } -func (lbc *LoadBalancerController) findIngressesForSecret(secret string) ([]extensions.Ingress, error) { +func (lbc *LoadBalancerController) findIngressesForSecret(secretNamespace string, secretName string) ([]extensions.Ingress, error) { res := []extensions.Ingress{} ings, err := lbc.ingLister.List() if err != nil { @@ -848,6 +848,10 @@ func (lbc *LoadBalancerController) findIngressesForSecret(secret string) ([]exte items: for _, ing := range ings.Items { + if ing.Namespace != secretNamespace { + continue + } + if !lbc.isNginxIngress(&ing) { continue } @@ -855,14 +859,14 @@ items: continue } for _, tls := range ing.Spec.TLS { - if tls.SecretName == secret { + if tls.SecretName == secretName { res = append(res, ing) continue items } } if lbc.nginxPlus { if jwtKey, exists := ing.Annotations[nginx.JWTKeyAnnotation]; exists { - if jwtKey == secret { + if jwtKey == secretName { res = append(res, ing) } } diff --git a/nginx-controller/controller/controller_test.go b/nginx-controller/controller/controller_test.go index c41ada87f0..6454a83c10 100644 --- a/nginx-controller/controller/controller_test.go +++ b/nginx-controller/controller/controller_test.go @@ -2,6 +2,7 @@ package controller import ( "fmt" + "net/http" "reflect" "testing" "time" @@ -827,3 +828,163 @@ func TestGetServicePortForIngressPort(t *testing.T) { t.Errorf("Mismatched strings should not return port: %+v", svcPort) } } + +func TestFindIngressesForSecret(t *testing.T) { + testCases := []struct { + secret v1.Secret + ingress extensions.Ingress + expectedToFind bool + desc string + }{ + { + secret: v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "my-tls-secret", + Namespace: "namespace-1", + }, + }, + ingress: extensions.Ingress{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "my-ingress", + Namespace: "namespace-1", + }, + Spec: extensions.IngressSpec{ + TLS: []extensions.IngressTLS{ + extensions.IngressTLS{ + SecretName: "my-tls-secret", + }, + }, + }, + }, + expectedToFind: true, + desc: "an Ingress references a TLS Secret that exists in the Ingress namespace", + }, + { + secret: v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "my-tls-secret", + Namespace: "namespace-1", + }, + }, + ingress: extensions.Ingress{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "my-ingress", + Namespace: "namespace-2", + }, + Spec: extensions.IngressSpec{ + TLS: []extensions.IngressTLS{ + extensions.IngressTLS{ + SecretName: "my-tls-secret", + }, + }, + }, + }, + expectedToFind: false, + desc: "an Ingress references a TLS Secret that exists in a different namespace", + }, + { + secret: v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "my-jwk-secret", + Namespace: "namespace-1", + }, + }, + ingress: extensions.Ingress{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "my-ingress", + Namespace: "namespace-1", + Annotations: map[string]string{ + nginx.JWTKeyAnnotation: "my-jwk-secret", + }, + }, + }, + expectedToFind: true, + desc: "an Ingress references a JWK Secret that exists in the Ingress namespace", + }, + { + secret: v1.Secret{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "my-jwk-secret", + Namespace: "namespace-1", + }, + }, + ingress: extensions.Ingress{ + ObjectMeta: meta_v1.ObjectMeta{ + Name: "my-ingress", + Namespace: "namespace-2", + Annotations: map[string]string{ + nginx.JWTKeyAnnotation: "my-jwk-secret", + }, + }, + }, + expectedToFind: false, + desc: "an Ingress references a JWK secret that exists in a different namespace", + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + fakeClient := fake.NewSimpleClientset() + + templateExecutor, err := nginx.NewTemplateExecutor("../nginx/templates/nginx-plus.tmpl", "../nginx/templates/nginx-plus.ingress.tmpl", true) + if err != nil { + t.Errorf("templateExecuter could not start: %v", err) + } + ngxc, err := nginx.NewNginxController("/etc/nginx", true, true, "../nginx/templates/nginx-plus.tmpl", "../nginx/templates/nginx-plus.ingress.tmpl") + if err != nil { + t.Errorf("NGINX Controller could not start: %v", err) + } + apiCtrl, err := plus.NewNginxAPIController(&http.Client{}, "", true) + if err != nil { + t.Errorf("NGINX API Controller could not start: %v", err) + } + + cnf := nginx.NewConfigurator(ngxc, &nginx.Config{}, apiCtrl, templateExecutor) + lbc := LoadBalancerController{ + client: fakeClient, + ingressClass: "nginx", + cnf: cnf, + nginxPlus: true, + } + + lbc.ingLister.Store, _ = cache.NewInformer( + cache.NewListWatchFromClient(lbc.client.ExtensionsV1beta1().RESTClient(), "ingresses", "default", fields.Everything()), + &extensions.Ingress{}, time.Duration(1), nil) + + lbc.secrLister.Store, lbc.secrController = cache.NewInformer( + cache.NewListWatchFromClient(lbc.client.Core().RESTClient(), "secrets", "default", fields.Everything()), + &v1.Secret{}, time.Duration(1), nil) + + ngxIngress := &nginx.IngressEx{ + Ingress: &test.ingress, + TLSSecrets: map[string]*v1.Secret{ + test.secret.ObjectMeta.Name: &test.secret, + }, + } + + err = cnf.AddOrUpdateIngress(ngxIngress) + if err != nil { + t.Errorf("Ingress was not added: %v", err) + } + + lbc.ingLister.Add(&test.ingress) + lbc.secrLister.Add(&test.secret) + + ingresses, err := lbc.findIngressesForSecret(test.secret.ObjectMeta.Namespace, test.secret.ObjectMeta.Name) + if err != nil { + t.Errorf("Couldn't list Ingress resource: %v", err) + } + + if len(ingresses) > 0 { + if !test.expectedToFind { + t.Errorf("Expected no ingresses. Got: %v/%v", ingresses[0].Namespace, ingresses[0].Name) + } + if ingresses[0].Name != test.ingress.ObjectMeta.Name || ingresses[0].Namespace != test.ingress.ObjectMeta.Namespace { + t.Errorf("Expected: %v/%v. Got: %v/%v.", test.ingress.ObjectMeta.Namespace, test.ingress.ObjectMeta.Name, ingresses[0].Namespace, ingresses[0].Name) + } + } else if test.expectedToFind { + t.Errorf("Expected %v/%v. Got: No ingresses. %v", test.ingress.ObjectMeta.Namespace, test.ingress.ObjectMeta.Name, err) + } + }) + } +}