fix: get secret from cluster if not present in cache (#1571) (#1581)

pkg/k8scontext/context.go

@@ -107,7 +107,7 @@ func NewContext(kubeClient kubernetes.Interface, crdClient versioned.Interface,
 		informers:              &informerCollection,
 		ingressSecretsMap:      utils.NewThreadsafeMultimap(),
 		Caches:                 &cacheCollection,
-		CertificateSecretStore: NewSecretStore(),
+		CertificateSecretStore: NewSecretStore(kubeClient),
 		Work:                   make(chan events.Event, workBuffer),
 		CacheSynced:            make(chan interface{}),
 

pkg/k8scontext/handlers_test.go

@@ -12,7 +12,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
-	"github.com/onsi/ginkgo"
+	"github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"k8s.io/client-go/kubernetes"
 	testclient "k8s.io/client-go/kubernetes/fake"

pkg/k8scontext/ingress_handlers_test.go

@@ -9,7 +9,7 @@ import (
 	"context"
 	"time"
 
-	"github.com/onsi/ginkgo"
+	"github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

pkg/k8scontext/k8scontext_suite_test.go

@@ -3,6 +3,7 @@
 // Licensed under the MIT License. See License.txt in the project root for license information.
 // --------------------------------------------------------------------------------------------
 
+//go:build unittest
 // +build unittest
 
 package k8scontext
@@ -11,7 +12,7 @@ import (
 	"flag"
 	"testing"
 
-	"github.com/onsi/ginkgo"
+	"github.com/onsi/ginkgo/v2"
 	"github.com/onsi/gomega"
 	"k8s.io/klog/v2"
 )

pkg/k8scontext/k8scontext_test.go

@@ -12,7 +12,7 @@ import (
 
 	"github.com/getlantern/deepcopy"
 	"github.com/knative/pkg/apis/istio/v1alpha3"
-	"github.com/onsi/ginkgo"
+	"github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	v1 "k8s.io/api/core/v1"
 	networking "k8s.io/api/networking/v1"
@@ -73,7 +73,6 @@ var _ = ginkgo.Describe("K8scontext", func() {
 					}
 				}
 			case <-time.After(1 * time.Second):
-				break
 			}
 
 			if len(exists) == len(resourceList) {

pkg/k8scontext/secrets_handlers.go

@@ -18,7 +18,12 @@ import (
 
 // secret resource handlers
 func (h handlers) secretAdd(obj interface{}) {
-	sec := obj.(*v1.Secret)
+	sec, ok := obj.(*v1.Secret)
+	if !ok {
+		klog.Error("error decoding object, invalid type")
+		return
+	}
+
 	if _, exists := namespacesToIgnore[sec.Namespace]; exists {
 		return
 	}
@@ -42,7 +47,12 @@ func (h handlers) secretAdd(obj interface{}) {
 }
 
 func (h handlers) secretUpdate(oldObj, newObj interface{}) {
-	sec := newObj.(*v1.Secret)
+	sec, ok := newObj.(*v1.Secret)
+	if !ok {
+		klog.Error("error decoding object, invalid type")
+		return
+	}
+
 	if _, exists := namespacesToIgnore[sec.Namespace]; exists {
 		return
 	}
@@ -70,6 +80,11 @@ func (h handlers) secretUpdate(oldObj, newObj interface{}) {
 
 func (h handlers) secretDelete(obj interface{}) {
 	sec, ok := obj.(*v1.Secret)
+	if !ok {
+		klog.Error("error decoding object, invalid type")
+		return
+	}
+
 	if _, exists := namespacesToIgnore[sec.Namespace]; exists {
 		return
 	}

pkg/k8scontext/secrets_handlers_test.go

@@ -12,7 +12,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
-	"github.com/onsi/ginkgo"
+	"github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"k8s.io/client-go/kubernetes"
 	testclient "k8s.io/client-go/kubernetes/fake"

pkg/k8scontext/secretstore.go

@@ -7,21 +7,19 @@ package k8scontext
 
 import (
 	"bytes"
+	"context"
 	"io/ioutil"
 	"os"
 	"os/exec"
 	"sync"
 
 	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/tools/cache"
 
 	"github.com/Azure/application-gateway-kubernetes-ingress/pkg/controllererrors"
-)
-
-const (
-	recognizedSecretType = "kubernetes.io/tls"
-	tlsKey               = "tls.key"
-	tlsCrt               = "tls.crt"
+	"github.com/Azure/application-gateway-kubernetes-ingress/pkg/utils"
 )
 
 // SecretsKeeper is the interface definition for secret store
@@ -34,13 +32,15 @@ type SecretsKeeper interface {
 // SecretsStore maintains a cache of the deployment secrets.
 type SecretsStore struct {
 	conversionSync sync.Mutex
+	Client         kubernetes.Interface
 	Cache          cache.ThreadSafeStore
 }
 
 // NewSecretStore creates a new SecretsKeeper object
-func NewSecretStore() SecretsKeeper {
+func NewSecretStore(client kubernetes.Interface) SecretsKeeper {
 	return &SecretsStore{
-		Cache: cache.NewThreadSafeStore(cache.Indexers{}, cache.Indices{}),
+		Cache:  cache.NewThreadSafeStore(cache.Indexers{}, cache.Indices{}),
+		Client: client,
 	}
 }
 
@@ -51,9 +51,33 @@ func (s *SecretsStore) GetPfxCertificate(secretKey string) []byte {
 			return cert
 		}
 	}
+
+	if cert, err := s.GetFromCluster(secretKey); err == nil {
+		return cert
+	}
 	return nil
 }
 
+func (s *SecretsStore) GetFromCluster(secretKey string) ([]byte, error) {
+	secretNamespace, secretName, err := utils.ParseNamespacedName(secretKey)
+	if err != nil {
+		return nil, err
+	}
+
+	secret, err := s.Client.CoreV1().Secrets(secretNamespace).Get(context.Background(), secretName, metav1.GetOptions{})
+	if err != nil {
+		return nil, err
+	}
+
+	if err := s.ConvertSecret(secretKey, secret); err != nil {
+		return nil, err
+	}
+
+	certInterface, _ := s.Cache.Get(secretKey)
+	cert, _ := certInterface.([]byte)
+	return cert, nil
+}
+
 func (s *SecretsStore) delete(secretKey string) {
 	s.conversionSync.Lock()
 	defer s.conversionSync.Unlock()
@@ -67,14 +91,14 @@ func (s *SecretsStore) ConvertSecret(secretKey string, secret *v1.Secret) error
 	defer s.conversionSync.Unlock()
 
 	// check if this is a secret with the correct type
-	if secret.Type != recognizedSecretType {
+	if secret.Type != v1.SecretTypeTLS {
 		return controllererrors.NewErrorf(
 			controllererrors.ErrorUnknownSecretType,
 			"secret [%v] is not type kubernetes.io/tls", secretKey,
 		)
 	}
 
-	if len(secret.Data[tlsKey]) == 0 || len(secret.Data[tlsCrt]) == 0 {
+	if len(secret.Data[v1.TLSCertKey]) == 0 || len(secret.Data[v1.TLSPrivateKeyKey]) == 0 {
 		return controllererrors.NewErrorf(
 			controllererrors.ErrorMalformedSecret,
 			"secret [%v] is malformed, tls.key or tls.crt is not defined", secretKey,

pkg/k8scontext/secretstore_test.go

@@ -6,42 +6,58 @@
 package k8scontext
 
 import (
-	"github.com/onsi/ginkgo"
+	"github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/client-go/kubernetes"
+	testclient "k8s.io/client-go/kubernetes/fake"
 
 	"github.com/Azure/application-gateway-kubernetes-ingress/pkg/controllererrors"
 	"github.com/Azure/application-gateway-kubernetes-ingress/pkg/tests"
 )
 
 var _ = ginkgo.Describe("Testing K8sContext.SecretStore", func() {
-	secretsStore := NewSecretStore()
-	ginkgo.Context("Test ConvertSecret function", func() {
-		secret := v1.Secret{}
-		ginkgo.It("Should have returned an error - unrecognized type of secret", func() {
-			err := secretsStore.ConvertSecret("someKey", &secret)
-			Expect(err.(*controllererrors.Error).Code).To(Equal(controllererrors.ErrorUnknownSecretType))
-		})
-		ginkgo.It("", func() {
-			malformed := secret
-			malformed.Type = recognizedSecretType
-			err := secretsStore.ConvertSecret("someKey", &malformed)
-			Expect(err.(*controllererrors.Error).Code).To(Equal(controllererrors.ErrorMalformedSecret))
-		})
-		ginkgo.It("", func() {
-			malformed := secret
-			malformed.Type = recognizedSecretType
-			malformed.Data = make(map[string][]byte)
-			malformed.Data[tlsKey] = []byte("X")
-			malformed.Data[tlsCrt] = []byte("Y")
-			err := secretsStore.ConvertSecret("someKey", &malformed)
-			Expect(err.(*controllererrors.Error).Code).To(Equal(controllererrors.ErrorExportingWithOpenSSL))
-		})
-		ginkgo.It("", func() {
+	secretsStore := NewSecretStore(nil)
+
+	ginkgo.DescribeTable("when converting certificate to PFX",
+		func(secret *v1.Secret, expectedError controllererrors.ErrorCode) {
+			err := secretsStore.ConvertSecret("someKey", secret)
+			Expect(err.(*controllererrors.Error).Code).To(Equal(expectedError))
+		},
+		ginkgo.Entry("no type in secret", &v1.Secret{}, controllererrors.ErrorUnknownSecretType),
+		ginkgo.Entry("unrecognized type of secret", &v1.Secret{Type: v1.SecretTypeOpaque}, controllererrors.ErrorUnknownSecretType),
+		ginkgo.Entry("malformed data", &v1.Secret{Type: v1.SecretTypeTLS, Data: map[string][]byte{}}, controllererrors.ErrorMalformedSecret),
+		ginkgo.Entry("invalid data", &v1.Secret{Type: v1.SecretTypeTLS, Data: map[string][]byte{
+			v1.TLSCertKey:       []byte("X"),
+			v1.TLSPrivateKeyKey: []byte("X"),
+		}}, controllererrors.ErrorExportingWithOpenSSL),
+	)
+
+	ginkgo.When("certificate gets stored", func() {
+		ginkgo.It("should be retrivable with the secret key", func() {
 			err := secretsStore.ConvertSecret("someKey", tests.NewSecretTestFixture())
 			Expect(err).ToNot(HaveOccurred())
 			actual := secretsStore.GetPfxCertificate("someKey")
 			Expect(len(actual)).To(BeNumerically(">", 0))
 		})
 	})
+
+	ginkgo.When("certificate is no cached", func() {
+		ginkgo.It("should get it from the api-server", func() {
+			secret := tests.NewSecretTestFixture()
+			var client kubernetes.Interface = testclient.NewSimpleClientset(secret)
+			secretsStore := NewSecretStore(client)
+
+			actual := secretsStore.GetPfxCertificate(secret.Namespace + "/" + secret.Name)
+			Expect(len(actual)).To(BeNumerically(">", 0))
+		})
+
+		ginkgo.It("should return nil if secret does not exist", func() {
+			var client kubernetes.Interface = testclient.NewSimpleClientset()
+			secretsStore := NewSecretStore(client)
+
+			actual := secretsStore.GetPfxCertificate("someKey")
+			Expect(actual).To(BeNil())
+		})
+	})
 })

pkg/utils/utils.go

@@ -97,3 +97,11 @@ func RemoveDuplicateStrings(list []string) []string {
 
 	return result
 }
+
+func ParseNamespacedName(namespacedName string) (string, string, error) {
+	split := strings.Split(namespacedName, "/")
+	if len(split) != 2 {
+		return "", "", fmt.Errorf("invalid namespaced name %s", namespacedName)
+	}
+	return split[0], split[1], nil
+}