defaultSSLCertificate via key vault URI (Azure#166)

* moving changes from aamgayle/crdwatchreconciler to new verified branch * unit tests for nginx_ingress_controller and parameter adjustments * tests and fix for app-routing-system namespace * keyVaultUri to pointer and associated changes * Generic BuildSPC function and namespace fixes * placeholder_pod refactor and additional nic kv uri e2e test * obj to nic bug fix, ing annotation change and unit test adjustment * changed nginx_secret_provider_class namespace to come from config * Added CEL validation for keyvault fields and properties size on DefaultSSLCertificate * addressing comments and added placeholder pod check test * Added e2e tests to confirm pulling secret from keyvault * error checking for spc get * removing unneeded upsert of nic * added secretproviderclass to scheme * added wait time to ensure spc is found after nic is created * addressing comments * removed waits for spc and service in e2e test * Reverted from zap to logr.Discard * Addressing comments * typo fix * Addressing comments * Added tests for buildDeployment and moved buildSPC tests to kv_util_test * ingressClassName error check test * string fix in test * Error() and ToPtr fix * UserError function and better error checking * changes for UserError * addressing comments * typo fix * test fixes * added logging for buildSPC error * Added logging * Error level logs * Addressing comments * moved logging for placeholder_pod to switch cases * default case for placeholder_pod --------- Co-authored-by: Oliver King <[email protected]>
aamgayle · Mar 27, 2024 · ef0b85a · ef0b85a
1 parent 49119e1
commit ef0b85a
Show file tree

Hide file tree

Showing 17 changed files with 1,539 additions and 291 deletions.
diff --git a/api/v1alpha1/nginxingresscontroller_types.go b/api/v1alpha1/nginxingresscontroller_types.go
@@ -61,10 +61,17 @@ type NginxIngressControllerSpec struct {
 	Scaling *Scaling `json:"scaling,omitempty"`
 }
 
+// DefaultSSLCertificate holds a secret in the form of a secret struct with name and namespace properties or a key vault uri
+// +kubebuilder:validation:MaxProperties=1
+// +kubebuilder:validation:XValidation:rule="(isURL(self.keyVaultURI) || !has(self.keyVaultURI))"
 type DefaultSSLCertificate struct {
 	// Secret is a struct that holds the name and namespace fields used for the default ssl secret
 	// +optional
 	Secret *Secret `json:"secret,omitempty"`
+
+	// Secret in the form of a Key Vault URI
+	// +optional
+	KeyVaultURI *string `json:"keyVaultURI"`
 }
 
 // Secret is a struct that holds a name and namespace to be used in DefaultSSLCertificate

diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/approuting.kubernetes.azure.com_nginxingresscontrollers.yaml b/config/crd/bases/approuting.kubernetes.azure.com_nginxingresscontrollers.yaml
@@ -65,7 +65,11 @@ spec:
                 description: DefaultSSLCertificate defines whether the NginxIngressController
                   should use a certain SSL certificate by default. If this field is
                   omitted, no default certificate will be used.
+                maxProperties: 1
                 properties:
+                  keyVaultURI:
+                    description: Secret in the form of a Key Vault URI
+                    type: string
                   secret:
                     description: Secret is a struct that holds the name and namespace
                       fields used for the default ssl secret
@@ -85,6 +89,8 @@ spec:
                     - namespace
                     type: object
                 type: object
+                x-kubernetes-validations:
+                - rule: (isURL(self.keyVaultURI) || !has(self.keyVaultURI))
               ingressClassName:
                 default: nginx.approuting.kubernetes.azure.com
                 description: IngressClassName is the name of the IngressClass that

diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go
@@ -184,6 +184,10 @@ func setupControllers(mgr ctrl.Manager, conf *config.Config, lgr logr.Logger, cl
 	if err := keyvault.NewIngressSecretProviderClassReconciler(mgr, conf, ingressManager); err != nil {
 		return fmt.Errorf("setting up ingress secret provider class reconciler: %w", err)
 	}
+	lgr.Info("setting up nginx keyvault secret provider class reconciler")
+	if err := keyvault.NewNginxSecretProviderClassReconciler(mgr, conf); err != nil {
+		return fmt.Errorf("setting up nginx secret provider class reconciler: %w", err)
+	}
 	lgr.Info("setting up keyvault placeholder pod controller")
 	if err := keyvault.NewPlaceholderPodController(mgr, conf, ingressManager); err != nil {
 		return fmt.Errorf("setting up placeholder pod controller: %w", err)

diff --git a/pkg/controller/keyvault/ingress_secret_provider_class.go b/pkg/controller/keyvault/ingress_secret_provider_class.go
@@ -5,25 +5,21 @@ package keyvault
 
 import (
 	"context"
-	"encoding/json"
+	"errors"
 	"fmt"
-	"net/url"
-	"strings"
 
+	"github.com/Azure/aks-app-routing-operator/pkg/config"
+	"github.com/Azure/aks-app-routing-operator/pkg/controller/controllername"
+	"github.com/Azure/aks-app-routing-operator/pkg/controller/metrics"
+	"github.com/Azure/aks-app-routing-operator/pkg/manifests"
+	"github.com/Azure/aks-app-routing-operator/pkg/util"
 	"github.com/go-logr/logr"
 	netv1 "k8s.io/api/networking/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	secv1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1"
-
-	"github.com/Azure/aks-app-routing-operator/pkg/config"
-	"github.com/Azure/aks-app-routing-operator/pkg/controller/controllername"
-	"github.com/Azure/aks-app-routing-operator/pkg/controller/metrics"
-	"github.com/Azure/aks-app-routing-operator/pkg/manifests"
-	"github.com/Azure/aks-app-routing-operator/pkg/util"
-	kvcsi "github.com/Azure/secrets-store-csi-driver-provider-azure/pkg/provider/types"
 )
 
 var (
@@ -103,28 +99,45 @@ func (i *IngressSecretProviderClassReconciler) Reconcile(ctx context.Context, re
 		},
 	}
 	logger = logger.WithValues("spc", spc.Name)
-	ok, err := i.buildSPC(ing, spc)
-	if err != nil {
-		logger.Info("failed to build secret provider class for ingress, user input invalid. sending warning event")
-		i.events.Eventf(ing, "Warning", "InvalidInput", "error while processing Keyvault reference: %s", err)
-		return result, nil
+
+	// Checking if we manage the ingress. All false cases without an error are assumed that we don't manage it
+	var isManaged bool
+	if isManaged, err = i.ingressManager.IsManaging(ing); err != nil {
+		logger.Error(err, fmt.Sprintf("failed while checking if ingress was managed with error: %s.", err.Error()))
+		return result, fmt.Errorf("determining if ingress is managed: %w", err)
 	}
-	if ok {
-		logger.Info("reconciling secret provider class for ingress")
-		err = util.Upsert(ctx, i.client, spc)
-		if err != nil {
-			i.events.Eventf(ing, "Warning", "FailedUpdateOrCreateSPC", "error while creating or updating SecretProviderClass needed to pull Keyvault reference: %s", err)
+
+	if isManaged {
+		var upsertSPC bool
+
+		if upsertSPC, err = buildSPC(ing, spc, i.config); err != nil {
+			var userErr userError
+			if errors.As(err, &userErr) {
+				logger.Info(fmt.Sprintf("failed to build secret provider class for ingress with error: %s. sending warning event", userErr.Error()))
+				i.events.Eventf(ing, "Warning", "InvalidInput", "error while processing Keyvault reference: %s", userErr.UserError())
+				return result, nil
+			}
+
+			logger.Error(err, fmt.Sprintf("failed to build secret provider class for ingress with error: %s.", err.Error()))
+			return result, err
+		}
+
+		if upsertSPC {
+			logger.Info("reconciling secret provider class for ingress")
+			if err = util.Upsert(ctx, i.client, spc); err != nil {
+				i.events.Eventf(ing, "Warning", "FailedUpdateOrCreateSPC", "error while creating or updating SecretProviderClass needed to pull Keyvault reference: %s", err.Error())
+				logger.Error(err, fmt.Sprintf("failed to upsert secret provider class for ingress with error: %s.", err.Error()))
+			}
+			return result, err
 		}
-		return result, err
 	}
 
 	logger.Info("cleaning unused managed spc for ingress")
 	logger.Info("getting secret provider class for ingress")
 
 	toCleanSPC := &secv1.SecretProviderClass{}
 
-	err = i.client.Get(ctx, client.ObjectKeyFromObject(spc), toCleanSPC)
-	if err != nil {
+	if err = i.client.Get(ctx, client.ObjectKeyFromObject(spc), toCleanSPC); err != nil {
 		return result, client.IgnoreNotFound(err)
 	}
 
@@ -136,85 +149,3 @@ func (i *IngressSecretProviderClassReconciler) Reconcile(ctx context.Context, re
 
 	return result, nil
 }
-
-func (i *IngressSecretProviderClassReconciler) buildSPC(ing *netv1.Ingress, spc *secv1.SecretProviderClass) (bool, error) {
-	if ing.Spec.IngressClassName == nil || ing.Annotations == nil {
-		return false, nil
-	}
-
-	managed, err := i.ingressManager.IsManaging(ing)
-	if err != nil {
-		return false, fmt.Errorf("determining if ingress is managed: %w", err)
-	}
-	if !managed {
-		return false, nil
-	}
-
-	certURI := ing.Annotations[tlsCertKvUriAnnotation]
-	if certURI == "" {
-		return false, nil
-	}
-
-	uri, err := url.Parse(certURI)
-	if err != nil {
-		return false, err
-	}
-	vaultName := strings.Split(uri.Host, ".")[0]
-	chunks := strings.Split(uri.Path, "/")
-	if len(chunks) < 3 {
-		return false, fmt.Errorf("invalid secret uri: %s", certURI)
-	}
-	secretName := chunks[2]
-	p := map[string]interface{}{
-		"objectName": secretName,
-		"objectType": "secret",
-	}
-	if len(chunks) > 3 {
-		p["objectVersion"] = chunks[3]
-	}
-
-	params, err := json.Marshal(p)
-	if err != nil {
-		return false, err
-	}
-	objects, err := json.Marshal(map[string]interface{}{"array": []string{string(params)}})
-	if err != nil {
-		return false, err
-	}
-
-	spc.Spec = secv1.SecretProviderClassSpec{
-		Provider: secv1.Provider("azure"),
-		SecretObjects: []*secv1.SecretObject{{
-			SecretName: certSecretName(ing.Name),
-			Type:       "kubernetes.io/tls",
-			Data: []*secv1.SecretObjectData{
-				{
-					ObjectName: secretName,
-					Key:        "tls.key",
-				},
-				{
-					ObjectName: secretName,
-					Key:        "tls.crt",
-				},
-			},
-		}},
-		// https://azure.github.io/secrets-store-csi-driver-provider-azure/docs/getting-started/usage/#create-your-own-secretproviderclass-object
-		Parameters: map[string]string{
-			"keyvaultName":           vaultName,
-			"useVMManagedIdentity":   "true",
-			"userAssignedIdentityID": i.config.MSIClientID,
-			"tenantId":               i.config.TenantID,
-			"objects":                string(objects),
-		},
-	}
-
-	if i.config.Cloud != "" {
-		spc.Spec.Parameters[kvcsi.CloudNameParameter] = i.config.Cloud
-	}
-
-	return true, nil
-}
-
-func certSecretName(ingressName string) string {
-	return fmt.Sprintf("keyvault-%s", ingressName)
-}