diff --git a/docs/user-guide/annotations.md b/docs/user-guide/annotations.md index ed3314c3d..5a267018f 100644 --- a/docs/user-guide/annotations.md +++ b/docs/user-guide/annotations.md @@ -16,11 +16,11 @@ The following annotations are supported: |[nginx.ingress.kubernetes.io/auth-realm](#authentication)|string| |[nginx.ingress.kubernetes.io/auth-secret](#authentication)|string| |[nginx.ingress.kubernetes.io/auth-type](#authentication)|basic or digest| -|[nginx.ingress.kubernetes.io/auth-tls-secret](#certificate-authentication)|string| -|[nginx.ingress.kubernetes.io/auth-tls-verify-depth](#certificate-authentication)|number| -|[nginx.ingress.kubernetes.io/auth-tls-verify-client](#certificate-authentication)|string| -|[nginx.ingress.kubernetes.io/auth-tls-error-page](#certificate-authentication)|string| -|[nginx.ingress.kubernetes.io/auth-tls-pass-certificate-to-upstream](#certificate-authentication)|"true" or "false"| +|[nginx.ingress.kubernetes.io/auth-tls-secret](#client-certificate-authentication)|string| +|[nginx.ingress.kubernetes.io/auth-tls-verify-depth](#client-certificate-authentication)|number| +|[nginx.ingress.kubernetes.io/auth-tls-verify-client](#client-certificate-authentication)|string| +|[nginx.ingress.kubernetes.io/auth-tls-error-page](#client-certificate-authentication)|string| +|[nginx.ingress.kubernetes.io/auth-tls-pass-certificate-to-upstream](#client-certificate-authentication)|"true" or "false"| |[nginx.ingress.kubernetes.io/auth-url](#external-authentication)|string| |[nginx.ingress.kubernetes.io/base-url-scheme](#rewrite)|string| |[nginx.ingress.kubernetes.io/client-body-buffer-size](#client-body-buffer-size)|string| @@ -108,8 +108,8 @@ Indicates the [HTTP Authentication Type: Basic or Digest Access Authentication]( nginx.ingress.kubernetes.io/auth-secret: secretName ``` -The name of the secret that contains the usernames and passwords with access to the `path`s defined in the Ingress Rule. -The secret must be created in the same namespace as the Ingress rule. +The name of the Secret that contains the usernames and passwords which are granted access to the `path`s defined in the Ingress rules. +This annotation also accepts the alternative form "namespace/secretName", in which case the Secret lookup is performed in the referenced namespace instead of the Ingress namespace. ``` nginx.ingress.kubernetes.io/auth-realm: "realm string" @@ -161,7 +161,8 @@ The annotations are: nginx.ingress.kubernetes.io/auth-tls-secret: secretName ``` -The name of the secret that contains the full Certificate Authority chain `ca.crt` that is enabled to authenticate against this ingress. It's composed of namespace/secretName. +The name of the Secret that contains the full Certificate Authority chain `ca.crt` that is enabled to authenticate against this Ingress. +This annotation also accepts the alternative form "namespace/secretName", in which case the Secret lookup is performed in the referenced namespace instead of the Ingress namespace. ``` nginx.ingress.kubernetes.io/auth-tls-verify-depth diff --git a/internal/ingress/controller/store/store.go b/internal/ingress/controller/store/store.go index 5bb978dfd..91ecaa87c 100644 --- a/internal/ingress/controller/store/store.go +++ b/internal/ingress/controller/store/store.go @@ -538,12 +538,12 @@ func (s *k8sStore) updateSecretIngressMap(ing *extensions.Ingress) { "auth-tls-secret", } for _, ann := range secretAnnotations { - secrName, err := parser.GetStringAnnotation(ann, ing) + secrKey, err := objectRefAnnotationNsKey(ann, ing) if err != nil { + glog.Errorf("error reading secret reference in annotation %q: %s", ann, err) continue } - if secrName != "" { - secrKey := fmt.Sprintf("%v/%v", ing.Namespace, secrName) + if secrKey != "" { refSecrets = append(refSecrets, secrKey) } } @@ -552,6 +552,25 @@ func (s *k8sStore) updateSecretIngressMap(ing *extensions.Ingress) { s.secretIngressMap.Insert(key, refSecrets...) } +// objectRefAnnotationNsKey returns an object reference formatted as a +// 'namespace/name' key from the given annotation name. +func objectRefAnnotationNsKey(ann string, ing *extensions.Ingress) (string, error) { + annValue, err := parser.GetStringAnnotation(ann, ing) + if annValue == "" { + return "", err + } + + secrNs, secrName, err := cache.SplitMetaNamespaceKey(annValue) + if secrName == "" { + return "", err + } + + if secrNs == "" { + return fmt.Sprintf("%v/%v", ing.Namespace, secrName), nil + } + return annValue, nil +} + // 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) { diff --git a/internal/ingress/controller/store/store_test.go b/internal/ingress/controller/store/store_test.go index 3549197db..8a4a3d3b4 100644 --- a/internal/ingress/controller/store/store_test.go +++ b/internal/ingress/controller/store/store_test.go @@ -19,6 +19,7 @@ package store import ( "fmt" "os" + "sync" "sync/atomic" "testing" "time" @@ -31,8 +32,10 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/kubernetes" + "k8s.io/client-go/tools/cache" "k8s.io/ingress-nginx/internal/file" + "k8s.io/ingress-nginx/internal/ingress/annotations/parser" "k8s.io/ingress-nginx/test/e2e/framework" ) @@ -653,3 +656,88 @@ func newFS(t *testing.T) file.Filesystem { } return fs } + +// newStore creates a new mock object store for tests which do not require the +// use of Informers. +func newStore(t *testing.T) *k8sStore { + fs, err := file.NewFakeFS() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + return &k8sStore{ + listers: &Lister{ + // add more listers if needed + Ingress: IngressLister{cache.NewStore(cache.MetaNamespaceKeyFunc)}, + }, + sslStore: NewSSLCertTracker(), + filesystem: fs, + updateCh: channels.NewRingChannel(10), + mu: new(sync.Mutex), + secretIngressMap: NewObjectRefMap(), + } +} + +func TestUpdateSecretIngressMap(t *testing.T) { + s := newStore(t) + + ingTpl := &v1beta1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test", + Namespace: "testns", + }, + } + s.listers.Ingress.Add(ingTpl) + + t.Run("with TLS secret", func(t *testing.T) { + ing := ingTpl.DeepCopy() + ing.Spec = v1beta1.IngressSpec{ + TLS: []v1beta1.IngressTLS{{SecretName: "tls"}}, + } + s.listers.Ingress.Update(ing) + s.updateSecretIngressMap(ing) + + if l := s.secretIngressMap.Len(); !(l == 1 && s.secretIngressMap.Has("testns/tls")) { + t.Errorf("Expected \"testns/tls\" to be the only referenced Secret (got %d)", l) + } + }) + + t.Run("with annotation in simple name format", func(t *testing.T) { + ing := ingTpl.DeepCopy() + ing.ObjectMeta.SetAnnotations(map[string]string{ + parser.GetAnnotationWithPrefix("auth-secret"): "auth", + }) + s.listers.Ingress.Update(ing) + s.updateSecretIngressMap(ing) + + if l := s.secretIngressMap.Len(); !(l == 1 && s.secretIngressMap.Has("testns/auth")) { + t.Errorf("Expected \"testns/auth\" to be the only referenced Secret (got %d)", l) + } + }) + + t.Run("with annotation in namespace/name format", func(t *testing.T) { + ing := ingTpl.DeepCopy() + ing.ObjectMeta.SetAnnotations(map[string]string{ + parser.GetAnnotationWithPrefix("auth-secret"): "otherns/auth", + }) + s.listers.Ingress.Update(ing) + s.updateSecretIngressMap(ing) + + if l := s.secretIngressMap.Len(); !(l == 1 && s.secretIngressMap.Has("otherns/auth")) { + t.Errorf("Expected \"otherns/auth\" to be the only referenced Secret (got %d)", l) + } + }) + + t.Run("with annotation in invalid format", func(t *testing.T) { + ing := ingTpl.DeepCopy() + ing.ObjectMeta.SetAnnotations(map[string]string{ + parser.GetAnnotationWithPrefix("auth-secret"): "ns/name/garbage", + }) + s.listers.Ingress.Update(ing) + s.updateSecretIngressMap(ing) + + if l := s.secretIngressMap.Len(); l != 0 { + t.Errorf("Expected 0 referenced Secret (got %d)", l) + } + }) +}