diff --git a/api/v1alpha1/nginxingresscontroller_types.go b/api/v1alpha1/nginxingresscontroller_types.go index 69e056e1..0347d7f6 100644 --- 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 index a02a0e20..69bc189a 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -17,6 +17,11 @@ func (in *DefaultSSLCertificate) DeepCopyInto(out *DefaultSSLCertificate) { *out = new(Secret) **out = **in } + if in.KeyVaultURI != nil { + in, out := &in.KeyVaultURI, &out.KeyVaultURI + *out = new(string) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DefaultSSLCertificate. diff --git a/config/crd/bases/approuting.kubernetes.azure.com_nginxingresscontrollers.yaml b/config/crd/bases/approuting.kubernetes.azure.com_nginxingresscontrollers.yaml index 4c0c6d79..1216d0cf 100644 --- 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 index ef88a203..546afdec 100644 --- 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 index f3f27b0d..3edb82bc 100644 --- a/pkg/controller/keyvault/ingress_secret_provider_class.go +++ b/pkg/controller/keyvault/ingress_secret_provider_class.go @@ -5,11 +5,14 @@ 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" @@ -17,13 +20,6 @@ import ( 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,19 +99,37 @@ 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") @@ -123,8 +137,7 @@ func (i *IngressSecretProviderClassReconciler) Reconcile(ctx context.Context, re 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) -} diff --git a/pkg/controller/keyvault/ingress_secret_provider_class_test.go b/pkg/controller/keyvault/ingress_secret_provider_class_test.go index 210ab191..1c0656d1 100644 --- a/pkg/controller/keyvault/ingress_secret_provider_class_test.go +++ b/pkg/controller/keyvault/ingress_secret_provider_class_test.go @@ -6,7 +6,6 @@ package keyvault import ( "context" "fmt" - "net/url" "testing" "github.com/go-logr/logr" @@ -27,7 +26,6 @@ import ( "github.com/Azure/aks-app-routing-operator/pkg/controller/testutils" "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 ( @@ -44,6 +42,7 @@ var ( IngressClassName: &spcTestIngressClassName, }, } + spcTestDefaultConf = buildTestSpcConfig("test-msi", "test-tenant", "test-cloud") ) func TestIngressSecretProviderClassReconcilerIntegration(t *testing.T) { @@ -62,7 +61,9 @@ func TestIngressSecretProviderClassReconcilerIntegration(t *testing.T) { if *ing.Spec.IngressClassName == spcTestIngressClassName { return true, nil } - + if *ing.Spec.IngressClassName == "error" { + return false, fmt.Errorf("ingressClassNameError") + } return false, nil }), } @@ -117,6 +118,28 @@ func TestIngressSecretProviderClassReconcilerIntegration(t *testing.T) { require.Equal(t, testutils.GetErrMetricCount(t, ingressSecretProviderControllerName), beforeErrCount) require.Greater(t, testutils.GetReconcileMetricCount(t, ingressSecretProviderControllerName, metrics.LabelSuccess), beforeRequestCount) + // Error check for isManaged function + ingClassName := ing.Spec.IngressClassName + errorName := "error" + ing.Spec.IngressClassName = &errorName + + require.NoError(t, i.client.Update(ctx, ing)) + beforeErrCount = testutils.GetErrMetricCount(t, ingressSecretProviderControllerName) + beforeRequestCount = testutils.GetReconcileMetricCount(t, ingressSecretProviderControllerName, metrics.LabelSuccess) + _, err = i.Reconcile(ctx, req) + require.Errorf(t, err, fmt.Sprintf("determining if ingress is managed: %s", "ingressClassNameError")) + require.Greater(t, testutils.GetErrMetricCount(t, ingressSecretProviderControllerName), beforeErrCount) + require.Equal(t, testutils.GetReconcileMetricCount(t, ingressSecretProviderControllerName, metrics.LabelSuccess), beforeRequestCount) + + ing.Spec.IngressClassName = ingClassName + require.NoError(t, i.client.Update(ctx, ing)) + beforeErrCount = testutils.GetErrMetricCount(t, ingressSecretProviderControllerName) + beforeRequestCount = testutils.GetReconcileMetricCount(t, ingressSecretProviderControllerName, metrics.LabelSuccess) + _, err = i.Reconcile(ctx, req) + require.NoError(t, err) + require.Equal(t, testutils.GetErrMetricCount(t, ingressSecretProviderControllerName), beforeErrCount) + require.Greater(t, testutils.GetReconcileMetricCount(t, ingressSecretProviderControllerName, metrics.LabelSuccess), beforeRequestCount) + // Remove the cert's version from the ingress ing.Annotations = map[string]string{ "kubernetes.azure.com/tls-cert-keyvault-uri": "https://testvault.vault.azure.net/certificates/testcert", @@ -315,163 +338,12 @@ func TestIngressSecretProviderClassReconcilerInvalidURL(t *testing.T) { assert.Greater(t, afterRequestCount, beforeRequestCount) } -func TestIngressSecretProviderClassReconcilerBuildSPCInvalidURLs(t *testing.T) { - i := &IngressSecretProviderClassReconciler{ - ingressManager: NewIngressManagerFromFn(func(ing *netv1.Ingress) (bool, error) { - if *ing.Spec.IngressClassName == spcTestIngressClassName { - return true, nil - } - - return false, nil - }), +func buildTestSpcConfig(msiClient, tenantID, cloud string) *config.Config { + spcTestConf := config.Config{ + MSIClientID: msiClient, + TenantID: tenantID, + Cloud: cloud, } - invalidURLIng := &netv1.Ingress{ - Spec: netv1.IngressSpec{ - IngressClassName: &spcTestIngressClassName, - }, - } - - t.Run("missing ingress class", func(t *testing.T) { - ing := invalidURLIng.DeepCopy() - ing.Spec.IngressClassName = nil - ing.Annotations = map[string]string{"kubernetes.azure.com/tls-cert-keyvault-uri": "inv@lid URL"} - - ok, err := i.buildSPC(ing, &secv1.SecretProviderClass{}) - assert.False(t, ok) - require.NoError(t, err) - }) - - t.Run("incorrect ingress class", func(t *testing.T) { - ing := invalidURLIng.DeepCopy() - incorrect := "some-other-ingress-class" - ing.Spec.IngressClassName = &incorrect - ing.Annotations = map[string]string{"kubernetes.azure.com/tls-cert-keyvault-uri": "inv@lid URL"} - - ok, err := i.buildSPC(ing, &secv1.SecretProviderClass{}) - assert.False(t, ok) - require.NoError(t, err) - }) - - t.Run("nil annotations", func(t *testing.T) { - ing := invalidURLIng.DeepCopy() - - ok, err := i.buildSPC(ing, &secv1.SecretProviderClass{}) - assert.False(t, ok) - require.NoError(t, err) - }) - - t.Run("empty url", func(t *testing.T) { - ing := invalidURLIng.DeepCopy() - ing.Annotations = map[string]string{"kubernetes.azure.com/tls-cert-keyvault-uri": ""} - - ok, err := i.buildSPC(ing, &secv1.SecretProviderClass{}) - assert.False(t, ok) - require.NoError(t, err) - }) - - t.Run("url with control character", func(t *testing.T) { - ing := invalidURLIng.DeepCopy() - cc := string([]byte{0x7f}) - ing.Annotations = map[string]string{"kubernetes.azure.com/tls-cert-keyvault-uri": cc} - - ok, err := i.buildSPC(ing, &secv1.SecretProviderClass{}) - assert.False(t, ok) - _, expectedErr := url.Parse(cc) // the exact error depends on operating system - require.EqualError(t, err, fmt.Sprintf("%s", expectedErr)) - }) - - t.Run("url with one path segment", func(t *testing.T) { - ing := invalidURLIng.DeepCopy() - ing.Annotations = map[string]string{"kubernetes.azure.com/tls-cert-keyvault-uri": "http://test.com/foo"} - - ok, err := i.buildSPC(ing, &secv1.SecretProviderClass{}) - assert.False(t, ok) - require.EqualError(t, err, "invalid secret uri: http://test.com/foo") - }) -} - -func TestIngressSecretProviderClassReconcilerBuildSPCCloud(t *testing.T) { - cases := []struct { - name, configCloud, spcCloud string - expected bool - }{ - { - name: "empty config cloud", - configCloud: "", - expected: false, - }, - { - name: "public cloud", - configCloud: "AzurePublicCloud", - spcCloud: "AzurePublicCloud", - expected: true, - }, - { - name: "sov cloud", - configCloud: "AzureUSGovernmentCloud", - spcCloud: "AzureUSGovernmentCloud", - expected: true, - }, - } - - for _, c := range cases { - t.Run(c.name, func(t *testing.T) { - i := &IngressSecretProviderClassReconciler{ - config: &config.Config{ - Cloud: c.configCloud, - }, - ingressManager: NewIngressManagerFromFn(func(ing *netv1.Ingress) (bool, error) { - if *ing.Spec.IngressClassName == spcTestIngressClassName { - return true, nil - } - - return false, nil - }), - } - - ing := spcTestIngress.DeepCopy() - ing.Annotations = map[string]string{ - "kubernetes.azure.com/tls-cert-keyvault-uri": "https://test.vault.azure.net/secrets/test-secret", - } - - spc := &secv1.SecretProviderClass{} - ok, err := i.buildSPC(ing, spc) - require.NoError(t, err, "building SPC should not error") - require.True(t, ok, "SPC should be built") - - spcCloud, ok := spc.Spec.Parameters[kvcsi.CloudNameParameter] - require.Equal(t, c.expected, ok, "SPC cloud annotation unexpected") - require.Equal(t, c.spcCloud, spcCloud, "SPC cloud annotation doesn't match") - }) - } -} - -func TestIngressSecretProviderClassReconcilerBuildSPCFailedIsManaging(t *testing.T) { - i := &IngressSecretProviderClassReconciler{ - ingressManager: NewIngressManagerFromFn(func(ing *netv1.Ingress) (bool, error) { - return false, fmt.Errorf("failed to get ingress") - }), - } - - ing := &netv1.Ingress{ - ObjectMeta: metav1.ObjectMeta{ - Name: "test-ingress", - Annotations: map[string]string{}, - }, - Spec: netv1.IngressSpec{ - IngressClassName: &spcTestIngressClassName, - }, - } - spc := &secv1.SecretProviderClass{} - - ok, err := i.buildSPC(ing, spc) - require.False(t, ok) - require.Error(t, err) - require.ErrorContains(t, err, "determining if ingress is managed") -} - -func TestCertSecretName(t *testing.T) { - require.Equal(t, "keyvault-ingressname", certSecretName("ingressname")) - require.Equal(t, "keyvault-anotheringressname", certSecretName("anotheringressname")) + return &spcTestConf } diff --git a/pkg/controller/keyvault/kv_util.go b/pkg/controller/keyvault/kv_util.go new file mode 100644 index 00000000..48125bbb --- /dev/null +++ b/pkg/controller/keyvault/kv_util.go @@ -0,0 +1,139 @@ +package keyvault + +import ( + "encoding/json" + "fmt" + "net/url" + "strings" + + "github.com/Azure/aks-app-routing-operator/api/v1alpha1" + "github.com/Azure/aks-app-routing-operator/pkg/config" + kvcsi "github.com/Azure/secrets-store-csi-driver-provider-azure/pkg/provider/types" + v1 "k8s.io/api/networking/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + secv1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1" +) + +var nginxNamePrefix = "keyvault-nginx-" + +func buildSPC(obj client.Object, spc *secv1.SecretProviderClass, config *config.Config) (bool, error) { + var certURI, specSecretName string + + switch t := obj.(type) { + case *v1alpha1.NginxIngressController: + if t.Spec.DefaultSSLCertificate == nil || t.Spec.DefaultSSLCertificate.KeyVaultURI == nil { + return false, nil + } + certURI = *t.Spec.DefaultSSLCertificate.KeyVaultURI + specSecretName = DefaultNginxCertName(t) + case *v1.Ingress: + if t.Annotations == nil { + return false, nil + } + + certURI = t.Annotations["kubernetes.azure.com/tls-cert-keyvault-uri"] + specSecretName = certSecretName(t.Name) + default: + return false, fmt.Errorf("incorrect object type: %s", t) + } + + if certURI == "" { + return false, nil + } + + uri, err := url.Parse(certURI) + if err != nil { + return false, newUserError(err, fmt.Sprintf("unable to parse certificate uri: %s", certURI)) + } + vaultName := strings.Split(uri.Host, ".")[0] + chunks := strings.Split(uri.Path, "/") + + if len(chunks) < 3 { + return false, newUserError(fmt.Errorf("uri Path contains too few segments: has: %d requires greater than: %d uri path: %s", len(chunks), 3, uri.Path), fmt.Sprintf("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: specSecretName, + 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": config.MSIClientID, + "tenantId": config.TenantID, + "objects": string(objects), + }, + } + + if config.Cloud != "" { + spc.Spec.Parameters[kvcsi.CloudNameParameter] = config.Cloud + } + + return true, nil +} + +// DefaultNginxCertName returns a default name for the nginx certificate name using the IngressClassName from the spec. +// Truncates characters in the IngressClassName passed the max secret length (255) if the IngressClassName and the default namespace are over the limit +func DefaultNginxCertName(nic *v1alpha1.NginxIngressController) string { + secretMaxSize := 255 + certName := nginxNamePrefix + nic.Name + + if len(certName) > secretMaxSize { + return certName[0:secretMaxSize] + } + + return certName +} + +func certSecretName(ingressName string) string { + return fmt.Sprintf("keyvault-%s", ingressName) +} + +type userError struct { + err error + userMessage string +} + +// for internal use +func (b userError) Error() string { + return b.err.Error() +} + +// for user facing messages +func (b userError) UserError() string { + return b.userMessage +} + +func newUserError(err error, msg string) userError { + return userError{err, msg} +} diff --git a/pkg/controller/keyvault/kv_util_test.go b/pkg/controller/keyvault/kv_util_test.go new file mode 100644 index 00000000..3ee1d023 --- /dev/null +++ b/pkg/controller/keyvault/kv_util_test.go @@ -0,0 +1,218 @@ +package keyvault + +import ( + "errors" + "fmt" + "github.com/Azure/aks-app-routing-operator/api/v1alpha1" + kvcsi "github.com/Azure/secrets-store-csi-driver-provider-azure/pkg/provider/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + netv1 "k8s.io/api/networking/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "net/url" + "sigs.k8s.io/controller-runtime/pkg/client" + secv1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1" + "testing" +) + +var ( + buildSpcTestIngress = &netv1.Ingress{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ingress", + Namespace: "default", + Annotations: map[string]string{ + "kubernetes.azure.com/tls-cert-keyvault-uri": "https://testvault.vault.azure.net/certificates/testcert/f8982febc6894c0697b884f946fb1a34", + }, + }, + Spec: netv1.IngressSpec{ + IngressClassName: &spcTestIngressClassName, + }, + } + + buildSpcTestNginxIngress = &v1alpha1.NginxIngressController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-nic", + Namespace: "test-namespace", + }, + Spec: v1alpha1.NginxIngressControllerSpec{ + IngressClassName: spcTestNginxIngressClassName, + DefaultSSLCertificate: &v1alpha1.DefaultSSLCertificate{ + Secret: nil, + KeyVaultURI: &spcTestKeyVaultURI}, + }, + } + + testName = "testName" + maxSizeString = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaabbbbbbbbbbbbbbb" +) + +func TestDefaultNginxCertName(t *testing.T) { + + testStr := DefaultNginxCertName(&v1alpha1.NginxIngressController{ + ObjectMeta: metav1.ObjectMeta{ + Name: testName, + }, + }) + require.Equal(t, testStr, nginxNamePrefix+testName) + + testStr = DefaultNginxCertName(&v1alpha1.NginxIngressController{ + ObjectMeta: metav1.ObjectMeta{ + Name: maxSizeString, + }, + }) + + require.NotContains(t, testStr, "b") + require.Contains(t, testStr, nginxNamePrefix) +} + +func TestCertSecretName(t *testing.T) { + require.Equal(t, "keyvault-ingressname", certSecretName("ingressname")) + require.Equal(t, "keyvault-anotheringressname", certSecretName("anotheringressname")) +} + +func TestIngressSecretProviderClassReconcilerbuildSPCCloud(t *testing.T) { + cases := []struct { + name, configCloud, spcCloud string + expected bool + }{ + { + name: "empty config cloud", + configCloud: "", + expected: false, + }, + { + name: "public cloud", + configCloud: "AzurePublicCloud", + spcCloud: "AzurePublicCloud", + expected: true, + }, + { + name: "sov cloud", + configCloud: "AzureUSGovernmentCloud", + spcCloud: "AzureUSGovernmentCloud", + expected: true, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + ing := buildSpcTestIngress.DeepCopy() + ing.Annotations = map[string]string{ + "kubernetes.azure.com/tls-cert-keyvault-uri": "https://test.vault.azure.net/secrets/test-secret", + } + + spc := &secv1.SecretProviderClass{} + ok, err := buildSPC(ing, spc, buildTestSpcConfig("test-msi", "test-tenant", c.configCloud)) + require.NoError(t, err, "building SPC should not error") + require.True(t, ok, "SPC should be built") + + spcCloud, ok := spc.Spec.Parameters[kvcsi.CloudNameParameter] + require.Equal(t, c.expected, ok, "SPC cloud annotation unexpected") + require.Equal(t, c.spcCloud, spcCloud, "SPC cloud annotation doesn't match") + }) + } +} + +func TestNginxSecretProviderClassReconcilerbuildSPCCloud(t *testing.T) { + cases := []struct { + name, configCloud, spcCloud string + expected bool + }{ + { + name: "empty config cloud", + configCloud: "", + expected: false, + }, + { + name: "public cloud", + configCloud: "AzurePublicCloud", + spcCloud: "AzurePublicCloud", + expected: true, + }, + { + name: "sov cloud", + configCloud: "AzureUSGovernmentCloud", + spcCloud: "AzureUSGovernmentCloud", + expected: true, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + nic := buildSpcTestNginxIngress.DeepCopy() + testSecretUri := "https://test.vault.azure.net/secrets/test-secret" + nic.Spec.DefaultSSLCertificate.KeyVaultURI = &testSecretUri + + spc := &secv1.SecretProviderClass{} + ok, err := buildSPC(nic, spc, buildTestSpcConfig("test-msi", "test-tenant", c.configCloud)) + require.NoError(t, err, "building SPC should not error") + require.True(t, ok, "SPC should be built") + + spcCloud, ok := spc.Spec.Parameters[kvcsi.CloudNameParameter] + require.Equal(t, c.expected, ok, "SPC cloud annotation unexpected") + require.Equal(t, c.spcCloud, spcCloud, "SPC cloud annotation doesn't match") + }) + } +} + +func TestIngressSecretProviderClassReconcilerBuildSPCInvalidURLs(t *testing.T) { + invalidURLIng := &netv1.Ingress{ + Spec: netv1.IngressSpec{ + IngressClassName: &spcTestIngressClassName, + }, + } + + t.Run("nil annotations", func(t *testing.T) { + ing := invalidURLIng.DeepCopy() + + ok, err := buildSPC(ing, &secv1.SecretProviderClass{}, spcTestDefaultConf) + assert.False(t, ok) + require.NoError(t, err) + }) + + t.Run("empty url", func(t *testing.T) { + ing := invalidURLIng.DeepCopy() + ing.Annotations = map[string]string{"kubernetes.azure.com/tls-cert-keyvault-uri": ""} + + ok, err := buildSPC(ing, &secv1.SecretProviderClass{}, spcTestDefaultConf) + assert.False(t, ok) + require.NoError(t, err) + }) + + t.Run("url with control character", func(t *testing.T) { + ing := invalidURLIng.DeepCopy() + cc := string([]byte{0x7f}) + ing.Annotations = map[string]string{"kubernetes.azure.com/tls-cert-keyvault-uri": cc} + + ok, err := buildSPC(ing, &secv1.SecretProviderClass{}, spcTestDefaultConf) + assert.False(t, ok) + _, expectedErr := url.Parse(cc) // the exact error depends on operating system + require.EqualError(t, err, fmt.Sprintf("%s", expectedErr)) + }) + + t.Run("url with one path segment", func(t *testing.T) { + ing := invalidURLIng.DeepCopy() + ing.Annotations = map[string]string{"kubernetes.azure.com/tls-cert-keyvault-uri": "http://test.com/foo"} + + ok, err := buildSPC(ing, &secv1.SecretProviderClass{}, spcTestDefaultConf) + assert.False(t, ok) + require.EqualError(t, err, "uri Path contains too few segments: has: 2 requires greater than: 3 uri path: /foo") + }) +} + +func TestBuildSPCWithWrongObject(t *testing.T) { + var obj client.Object + + ok, err := buildSPC(obj, &secv1.SecretProviderClass{}, spcTestDefaultConf) + assert.False(t, ok) + require.EqualError(t, err, fmt.Sprintf("incorrect object type: %s", obj)) +} + +func TestUserErrors(t *testing.T) { + testMsg := "test error message" + testError := newUserError(errors.New("test"), testMsg) + var userErr userError + + assert.True(t, testError.UserError() == testMsg) + assert.True(t, errors.As(testError, &userErr)) +} diff --git a/pkg/controller/keyvault/nginx_secret_provider_class.go b/pkg/controller/keyvault/nginx_secret_provider_class.go new file mode 100644 index 00000000..466718fe --- /dev/null +++ b/pkg/controller/keyvault/nginx_secret_provider_class.go @@ -0,0 +1,146 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package keyvault + +import ( + "context" + "errors" + "fmt" + approutingv1alpha1 "github.com/Azure/aks-app-routing-operator/api/v1alpha1" + "github.com/go-logr/logr" + 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" +) + +var ( + nginxSecretProviderControllerName = controllername.New("keyvault", "nginx", "secret", "provider") +) + +// NginxSecretProviderClassReconciler manages a SecretProviderClass for each nginx ingress controller that +// has a Keyvault URI in its DefaultSSLCertificate field. The SPC is used to mirror the Keyvault values into +// a k8s secret so that it can be used by the CRD controller. +type NginxSecretProviderClassReconciler struct { + client client.Client + events record.EventRecorder + config *config.Config +} + +func NewNginxSecretProviderClassReconciler(manager ctrl.Manager, conf *config.Config) error { + metrics.InitControllerMetrics(nginxSecretProviderControllerName) + if conf.DisableKeyvault { + return nil + } + return nginxSecretProviderControllerName.AddToController( + ctrl. + NewControllerManagedBy(manager). + For(&approutingv1alpha1.NginxIngressController{}), manager.GetLogger(), + ).Complete(&NginxSecretProviderClassReconciler{ + client: manager.GetClient(), + events: manager.GetEventRecorderFor("aks-app-routing-operator"), + config: conf, + }) +} + +func (i *NginxSecretProviderClassReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { + var err error + result := ctrl.Result{} + + // do metrics + defer func() { + //placing this call inside a closure allows for result and err to be bound after Reconcile executes + //this makes sure they have the proper value + //just calling defer metrics.HandleControllerReconcileMetrics(controllerName, result, err) would bind + //the values of result and err to their zero values, since they were just instantiated + metrics.HandleControllerReconcileMetrics(nginxSecretProviderControllerName, result, err) + }() + + logger, err := logr.FromContext(ctx) + if err != nil { + return result, err + } + logger = nginxSecretProviderControllerName.AddToLogger(logger).WithValues("name", req.Name, "namespace", req.Namespace) + + logger.Info("getting Nginx Ingress") + nic := &approutingv1alpha1.NginxIngressController{} + err = i.client.Get(ctx, req.NamespacedName, nic) + if err != nil { + return result, client.IgnoreNotFound(err) + } + logger = logger.WithValues("name", nic.Name, "generation", nic.Generation) + + spc := &secv1.SecretProviderClass{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "secrets-store.csi.x-k8s.io/v1", + Kind: "SecretProviderClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultNginxCertName(nic), + Namespace: i.config.NS, + Labels: manifests.GetTopLevelLabels(), + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: nic.APIVersion, + Controller: util.ToPtr(true), + Kind: nic.Kind, + Name: nic.Name, + UID: nic.UID, + }}, + }, + } + logger = logger.WithValues("spc", spc.Name) + logger.Info("building spc and upserting if managed with labels") + upsertSPC, err := buildSPC(nic, spc, i.config) + + if err != nil { + var userErr userError + if errors.As(err, &userErr) { + logger.Info(fmt.Sprintf("failed to build secret provider class for nginx ingress controller with error: %s. sending warning event", userErr.Error())) + i.events.Eventf(nic, "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 nginx ingress controller with error: %s.", err.Error())) + return result, err + } + + if upsertSPC { + logger.Info("reconciling secret provider class for ingress") + err = util.Upsert(ctx, i.client, spc) + if err != nil { + i.events.Eventf(nic, "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 nginx ingress class with error: %s.", err.Error())) + } + return result, err + } else { + logger.Info("spc is either not managed or key vault uri was removed") + } + + logger.Info("cleaning unused spc if ingress is managed") + logger.Info("getting secret provider class for ingress") + + toCleanSPC := &secv1.SecretProviderClass{} + + err = i.client.Get(ctx, client.ObjectKeyFromObject(spc), toCleanSPC) + if err != nil { + return result, client.IgnoreNotFound(err) + } + + if manifests.HasTopLevelLabels(toCleanSPC.Labels) { + logger.Info("removing secret provider class for ingress") + err = i.client.Delete(ctx, toCleanSPC) + return result, client.IgnoreNotFound(err) + } else { + logger.Info("spc was not managed so spc was not removed") + } + + return result, nil +} diff --git a/pkg/controller/keyvault/nginx_secret_provider_class_test.go b/pkg/controller/keyvault/nginx_secret_provider_class_test.go new file mode 100644 index 00000000..864c8848 --- /dev/null +++ b/pkg/controller/keyvault/nginx_secret_provider_class_test.go @@ -0,0 +1,302 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package keyvault + +import ( + "context" + "github.com/Azure/aks-app-routing-operator/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" + "testing" + + "github.com/go-logr/logr" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/tools/record" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + 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/metrics" + "github.com/Azure/aks-app-routing-operator/pkg/controller/testutils" + "github.com/Azure/aks-app-routing-operator/pkg/manifests" + "github.com/Azure/aks-app-routing-operator/pkg/util" +) + +var ( + spcTestKeyVaultURI = "https://testvault.vault.azure.net/certificates/testcert/f8982febc6894c0697b884f946fb1a34" + spcTestNginxIngressClassName = "webapprouting.kubernetes.azure.com" + spcTestNginxIngress = &v1alpha1.NginxIngressController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-nic", + Namespace: "test-namespace", + }, + Spec: v1alpha1.NginxIngressControllerSpec{ + IngressClassName: spcTestNginxIngressClassName, + DefaultSSLCertificate: &v1alpha1.DefaultSSLCertificate{ + Secret: nil, + KeyVaultURI: &spcTestKeyVaultURI}, + }, + } +) + +func TestNginxSecretProviderClassReconcilerIntegration(t *testing.T) { + // Create the nic + ctx := context.Background() + ctx = logr.NewContext(ctx, logr.Discard()) + nic := spcTestNginxIngress.DeepCopy() + + scheme := runtime.NewScheme() + require.NoError(t, v1alpha1.AddToScheme(scheme)) + require.NoError(t, secv1.AddToScheme(scheme)) + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(nic).Build() + + recorder := record.NewFakeRecorder(10) + n := &NginxSecretProviderClassReconciler{ + client: c, + config: &config.Config{ + TenantID: "test-tenant-id", + MSIClientID: "test-msi-client-id", + }, + events: recorder, + } + + // Create the secret provider class + req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: nic.Namespace, Name: nic.Name}} + beforeErrCount := testutils.GetErrMetricCount(t, nginxSecretProviderControllerName) + beforeRequestCount := testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess) + _, err := n.Reconcile(ctx, req) + require.NoError(t, err) + + require.Equal(t, testutils.GetErrMetricCount(t, nginxSecretProviderControllerName), beforeErrCount) + require.Greater(t, testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess), beforeRequestCount) + + // Prove it exists + spc := &secv1.SecretProviderClass{} + spc.Name = DefaultNginxCertName(nic) + spc.Namespace = n.config.NS + require.NoError(t, c.Get(ctx, client.ObjectKeyFromObject(spc), spc)) + + expected := &secv1.SecretProviderClass{ + Spec: secv1.SecretProviderClassSpec{ + Provider: "azure", + Parameters: map[string]string{ + "keyvaultName": "testvault", + "objects": "{\"array\":[\"{\\\"objectName\\\":\\\"testcert\\\",\\\"objectType\\\":\\\"secret\\\",\\\"objectVersion\\\":\\\"f8982febc6894c0697b884f946fb1a34\\\"}\"]}", + "tenantId": n.config.TenantID, + "useVMManagedIdentity": "true", + "userAssignedIdentityID": n.config.MSIClientID, + }, + SecretObjects: []*secv1.SecretObject{{ + SecretName: spc.Name, + Type: "kubernetes.io/tls", + Data: []*secv1.SecretObjectData{ + {ObjectName: "testcert", Key: "tls.key"}, + {ObjectName: "testcert", Key: "tls.crt"}, + }, + }}, + }, + } + assert.Equal(t, expected.Spec, spc.Spec) + + // Check for idempotence + beforeErrCount = testutils.GetErrMetricCount(t, nginxSecretProviderControllerName) + beforeRequestCount = testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess) + _, err = n.Reconcile(ctx, req) + require.NoError(t, err) + require.Equal(t, testutils.GetErrMetricCount(t, nginxSecretProviderControllerName), beforeErrCount) + require.Greater(t, testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess), beforeRequestCount) + + // Remove the cert's version from the nic + testKvUri := "https://testvault.vault.azure.net/certificates/testcert" + nic.Spec.DefaultSSLCertificate.KeyVaultURI = &testKvUri + require.NoError(t, n.client.Update(ctx, nic)) + beforeErrCount = testutils.GetErrMetricCount(t, nginxSecretProviderControllerName) + beforeRequestCount = testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess) + _, err = n.Reconcile(ctx, req) + require.NoError(t, err) + require.Equal(t, testutils.GetErrMetricCount(t, nginxSecretProviderControllerName), beforeErrCount) + require.Greater(t, testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess), beforeRequestCount) + + // Prove the objectVersion property was removed + require.NoError(t, c.Get(ctx, client.ObjectKeyFromObject(spc), spc)) + expected.Spec.Parameters["objects"] = "{\"array\":[\"{\\\"objectName\\\":\\\"testcert\\\",\\\"objectType\\\":\\\"secret\\\"}\"]}" + assert.Equal(t, expected.Spec, spc.Spec) + + // Remove the cert annotation from the nic + nic.Spec.DefaultSSLCertificate.KeyVaultURI = nil + require.NoError(t, n.client.Update(ctx, nic)) + beforeErrCount = testutils.GetErrMetricCount(t, nginxSecretProviderControllerName) + beforeRequestCount = testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess) + _, err = n.Reconcile(ctx, req) + require.NoError(t, err) + require.Equal(t, testutils.GetErrMetricCount(t, nginxSecretProviderControllerName), beforeErrCount) + require.Greater(t, testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess), beforeRequestCount) + + // Prove secret class was removed + require.True(t, errors.IsNotFound(c.Get(ctx, client.ObjectKeyFromObject(spc), spc))) + + // Check for idempotence + beforeErrCount = testutils.GetErrMetricCount(t, nginxSecretProviderControllerName) + beforeRequestCount = testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess) + _, err = n.Reconcile(ctx, req) + require.NoError(t, err) + require.Equal(t, testutils.GetErrMetricCount(t, nginxSecretProviderControllerName), beforeErrCount) + require.Greater(t, testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess), beforeRequestCount) +} + +func TestNginxSecretProviderClassReconcilerIntegrationWithoutSPCLabels(t *testing.T) { + // Create the nic + ctx := context.Background() + ctx = logr.NewContext(ctx, logr.Discard()) + nic := spcTestNginxIngress.DeepCopy() + + scheme := runtime.NewScheme() + require.NoError(t, v1alpha1.AddToScheme(scheme)) + require.NoError(t, secv1.AddToScheme(scheme)) + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(nic).Build() + + recorder := record.NewFakeRecorder(10) + n := &NginxSecretProviderClassReconciler{ + client: c, + config: &config.Config{ + TenantID: "test-tenant-id", + MSIClientID: "test-msi-client-id", + }, + events: recorder, + } + + // Create the secret provider class + req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: nic.Namespace, Name: nic.Name}} + beforeErrCount := testutils.GetErrMetricCount(t, nginxSecretProviderControllerName) + beforeRequestCount := testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess) + _, err := n.Reconcile(ctx, req) + require.NoError(t, err) + + require.Equal(t, testutils.GetErrMetricCount(t, nginxSecretProviderControllerName), beforeErrCount) + require.Greater(t, testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess), beforeRequestCount) + + spc := &secv1.SecretProviderClass{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "secrets-store.csi.x-k8s.io/v1", + Kind: "SecretProviderClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultNginxCertName(nic), + Namespace: n.config.NS, + Labels: manifests.GetTopLevelLabels(), + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: nic.APIVersion, + Controller: util.ToPtr(true), + Kind: nic.Kind, + Name: nic.Name, + UID: nic.UID, + }}, + }, + } + + // Get secret provider class + require.False(t, errors.IsNotFound(c.Get(ctx, client.ObjectKeyFromObject(spc), spc))) + assert.Equal(t, len(manifests.GetTopLevelLabels()), len(spc.Labels)) + + // Remove the labels + spc.Labels = map[string]string{} + require.NoError(t, n.client.Update(ctx, spc)) + assert.Equal(t, 0, len(spc.Labels)) + + // Remove the cert uri from the nic + nic.Spec.DefaultSSLCertificate.KeyVaultURI = nil + require.NoError(t, n.client.Update(ctx, nic)) + + // Reconcile both changes + beforeErrCount = testutils.GetErrMetricCount(t, nginxSecretProviderControllerName) + beforeRequestCount = testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess) + _, err = n.Reconcile(ctx, req) + require.NoError(t, err) + require.Equal(t, testutils.GetErrMetricCount(t, nginxSecretProviderControllerName), beforeErrCount) + require.Greater(t, testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess), beforeRequestCount) + + // Prove secret class was not removed + require.False(t, errors.IsNotFound(c.Get(ctx, client.ObjectKeyFromObject(spc), spc))) + assert.Equal(t, 0, len(spc.Labels)) + require.NoError(t, c.Get(ctx, client.ObjectKeyFromObject(spc), spc)) + + // Check secret provider class Spec after Reconcile + expected := &secv1.SecretProviderClass{ + Spec: secv1.SecretProviderClassSpec{ + Provider: "azure", + Parameters: map[string]string{ + "keyvaultName": "testvault", + "objects": "{\"array\":[\"{\\\"objectName\\\":\\\"testcert\\\",\\\"objectType\\\":\\\"secret\\\",\\\"objectVersion\\\":\\\"f8982febc6894c0697b884f946fb1a34\\\"}\"]}", + "tenantId": n.config.TenantID, + "useVMManagedIdentity": "true", + "userAssignedIdentityID": n.config.MSIClientID, + }, + SecretObjects: []*secv1.SecretObject{{ + SecretName: spc.Name, + Type: "kubernetes.io/tls", + Data: []*secv1.SecretObjectData{ + {ObjectName: "testcert", Key: "tls.key"}, + {ObjectName: "testcert", Key: "tls.crt"}, + }, + }}, + }, + } + assert.Equal(t, expected.Spec, spc.Spec) + + // Check for idempotence + beforeErrCount = testutils.GetErrMetricCount(t, nginxSecretProviderControllerName) + beforeRequestCount = testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess) + _, err = n.Reconcile(ctx, req) + require.NoError(t, err) + require.Equal(t, testutils.GetErrMetricCount(t, nginxSecretProviderControllerName), beforeErrCount) + require.Greater(t, testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelSuccess), beforeRequestCount) +} + +func TestNginxSecretProviderClassReconcilerInvalidURL(t *testing.T) { + ctx := context.Background() + ctx = logr.NewContext(ctx, logr.Discard()) + // Create the nic + invalidUri := "inv@lid URL" + nic := spcTestNginxIngress.DeepCopy() + nic.Spec.DefaultSSLCertificate.KeyVaultURI = &invalidUri + + scheme := runtime.NewScheme() + require.NoError(t, v1alpha1.AddToScheme(scheme)) + require.NoError(t, secv1.AddToScheme(scheme)) + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(nic).Build() + recorder := record.NewFakeRecorder(10) + n := &NginxSecretProviderClassReconciler{ + client: c, + config: &config.Config{ + TenantID: "test-tenant-id", + MSIClientID: "test-msi-client-id", + }, + events: recorder, + } + + metrics.InitControllerMetrics(nginxSecretProviderControllerName) + + // get the before value of the error metrics + beforeErrCount := testutils.GetErrMetricCount(t, nginxSecretProviderControllerName) + beforeRequestCount := testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelError) + + req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: nic.Namespace, Name: nic.Name}} + _, err := n.Reconcile(ctx, req) + require.NoError(t, err) + + assert.Equal(t, "Warning InvalidInput error while processing Keyvault reference: invalid secret uri: inv@lid URL", <-recorder.Events) + //even though no error was returned, we should expect the error count to be incremented + afterErrCount := testutils.GetErrMetricCount(t, nginxSecretProviderControllerName) + afterRequestCount := testutils.GetReconcileMetricCount(t, nginxSecretProviderControllerName, metrics.LabelError) + + assert.Greater(t, afterErrCount, beforeErrCount) + assert.Greater(t, afterRequestCount, beforeRequestCount) +} diff --git a/pkg/controller/keyvault/placeholder_pod.go b/pkg/controller/keyvault/placeholder_pod.go index c2967a3c..0001ca87 100644 --- a/pkg/controller/keyvault/placeholder_pod.go +++ b/pkg/controller/keyvault/placeholder_pod.go @@ -6,9 +6,7 @@ package keyvault import ( "context" "fmt" - "path" - "strconv" - + "github.com/Azure/aks-app-routing-operator/api/v1alpha1" "github.com/go-logr/logr" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" @@ -17,9 +15,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/client-go/tools/record" + "path" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" secv1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1" + "strconv" "github.com/Azure/aks-app-routing-operator/pkg/config" "github.com/Azure/aks-app-routing-operator/pkg/controller/controllername" @@ -108,27 +108,65 @@ func (p *PlaceholderPodController) Reconcile(ctx context.Context, req ctrl.Reque } logger = logger.WithValues("deployment", dep.Name) - ing := &netv1.Ingress{} - ing.Name = util.FindOwnerKind(spc.OwnerReferences, "Ingress") - ing.Namespace = req.Namespace - logger = logger.WithValues("ingress", ing.Name) - if ing.Name != "" { - logger.Info("getting owner ingress") - if err = p.client.Get(ctx, client.ObjectKeyFromObject(ing), ing); err != nil { + return p.reconcileObjectDeployment(dep, spc, req, ctx, logger) +} + +func (p *PlaceholderPodController) placeholderPodCleanCheck(obj client.Object) (bool, error) { + switch t := obj.(type) { + case *v1alpha1.NginxIngressController: + if t.Spec.DefaultSSLCertificate == nil || t.Spec.DefaultSSLCertificate.KeyVaultURI == nil { + return true, nil + } + case *netv1.Ingress: + managed, err := p.ingressManager.IsManaging(t) + if err != nil { + return false, fmt.Errorf("determining if ingress is managed: %w", err) + } + if t.Name == "" || t.Spec.IngressClassName == nil || !managed { + return true, nil + } + } + + return false, nil +} + +func (p *PlaceholderPodController) reconcileObjectDeployment(dep *appsv1.Deployment, spc *secv1.SecretProviderClass, req ctrl.Request, ctx context.Context, logger logr.Logger) (ctrl.Result, error) { + var ( + err error + obj client.Object + ) + + result := ctrl.Result{} + + switch { + case util.FindOwnerKind(spc.OwnerReferences, "NginxIngressController") != "": + obj = &v1alpha1.NginxIngressController{} + obj.SetName(util.FindOwnerKind(spc.OwnerReferences, "NginxIngressController")) + logger.Info(fmt.Sprint("getting owner NginxIngressController")) + case util.FindOwnerKind(spc.OwnerReferences, "Ingress") != "": + obj = &netv1.Ingress{} + obj.SetName(util.FindOwnerKind(spc.OwnerReferences, "Ingress")) + obj.SetNamespace(req.Namespace) + logger.Info(fmt.Sprint("getting owner Ingress")) + default: + logger.Info("owner type not found") + return result, nil + } + + if obj.GetName() != "" { + if err = p.client.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil { return result, client.IgnoreNotFound(err) } } - managed, err := p.ingressManager.IsManaging(ing) + cleanPod, err := p.placeholderPodCleanCheck(obj) if err != nil { - return result, fmt.Errorf("determining if ingress is managed: %w", err) + return result, err } - if ing.Name == "" || ing.Spec.IngressClassName == nil || !managed { + if cleanPod { logger.Info("cleaning unused placeholder pod deployment") - logger.Info("getting placeholder deployment") - toCleanDeployment := &appsv1.Deployment{} if err = p.client.Get(ctx, client.ObjectKeyFromObject(dep), toCleanDeployment); err != nil { return result, client.IgnoreNotFound(err) @@ -139,17 +177,19 @@ func (p *PlaceholderPodController) Reconcile(ctx context.Context, req ctrl.Reque return result, client.IgnoreNotFound(err) } } + // Manage a deployment resource logger.Info("reconciling placeholder deployment for secret provider class") - if err = p.buildDeployment(ctx, dep, spc, ing); err != nil { + if err = p.buildDeployment(ctx, dep, spc, obj); err != nil { err = fmt.Errorf("building deployment: %w", err) - p.events.Eventf(ing, "Warning", "FailedUpdateOrCreatePlaceholderPodDeployment", "error while building placeholder pod Deployment needed to pull Keyvault reference: %s", err.Error()) + p.events.Eventf(obj, "Warning", "FailedUpdateOrCreatePlaceholderPodDeployment", "error while building placeholder pod Deployment needed to pull Keyvault reference: %s", err.Error()) logger.Error(err, "failed to build placeholder deployment") return result, err } if err = util.Upsert(ctx, p.client, dep); err != nil { - p.events.Eventf(ing, "Warning", "FailedUpdateOrCreatePlaceholderPodDeployment", "error while creating or updating placeholder pod Deployment needed to pull Keyvault reference: %s", err.Error()) + p.events.Eventf(obj, "Warning", "FailedUpdateOrCreatePlaceholderPodDeployment", "error while creating or updating placeholder pod Deployment needed to pull Keyvault reference: %s", err.Error()) + logger.Error(err, "failed to upsert placeholder deployment") return result, err } @@ -167,7 +207,7 @@ func (p *PlaceholderPodController) getCurrentDeployment(ctx context.Context, nam return dep, nil } -func (p *PlaceholderPodController) buildDeployment(ctx context.Context, dep *appsv1.Deployment, spc *secv1.SecretProviderClass, ing *netv1.Ingress) error { +func (p *PlaceholderPodController) buildDeployment(ctx context.Context, dep *appsv1.Deployment, spc *secv1.SecretProviderClass, obj client.Object) error { old, err := p.getCurrentDeployment(ctx, client.ObjectKeyFromObject(dep)) if err != nil { return fmt.Errorf("getting current deployment: %w", err) @@ -179,6 +219,16 @@ func (p *PlaceholderPodController) buildDeployment(ctx context.Context, dep *app labels = old.Spec.Selector.MatchLabels } + var ingAnnotation string + switch obj.(type) { + case *v1alpha1.NginxIngressController: + ingAnnotation = "kubernetes.azure.com/nginx-ingress-controller-owner" + case *netv1.Ingress: + ingAnnotation = "kubernetes.azure.com/ingress-owner" + default: + return fmt.Errorf("failed to build deployment: object type not ingress or nginxingresscontroller") + } + dep.Spec = appsv1.DeploymentSpec{ Replicas: util.Int32Ptr(1), RevisionHistoryLimit: util.Int32Ptr(2), @@ -189,7 +239,7 @@ func (p *PlaceholderPodController) buildDeployment(ctx context.Context, dep *app Annotations: map[string]string{ "kubernetes.azure.com/observed-generation": strconv.FormatInt(spc.Generation, 10), "kubernetes.azure.com/purpose": "hold CSI mount to enable keyvault-to-k8s secret mirroring", - "kubernetes.azure.com/ingress-owner": ing.Name, + ingAnnotation: obj.GetName(), "openservicemesh.io/sidecar-injection": "disabled", }, }, diff --git a/pkg/controller/keyvault/placeholder_pod_test.go b/pkg/controller/keyvault/placeholder_pod_test.go index 048a9d07..90246517 100644 --- a/pkg/controller/keyvault/placeholder_pod_test.go +++ b/pkg/controller/keyvault/placeholder_pod_test.go @@ -5,6 +5,10 @@ package keyvault import ( "context" + "fmt" + "github.com/Azure/aks-app-routing-operator/api/v1alpha1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/client-go/tools/record" "testing" "k8s.io/apimachinery/pkg/api/errors" @@ -43,7 +47,19 @@ var ( IngressClassName: &placeholderTestIngClassName, }, } - + placeholderTestUri = "https://testvault.vault.azure.net/certificates/testcert/f8982febc6894c0697b884f946fb1a34" + placeholderTestNginxIngress = &v1alpha1.NginxIngressController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-nic", + }, + TypeMeta: metav1.TypeMeta{Kind: "NginxIngressController"}, + Spec: v1alpha1.NginxIngressControllerSpec{ + IngressClassName: spcTestNginxIngressClassName, + DefaultSSLCertificate: &v1alpha1.DefaultSSLCertificate{ + KeyVaultURI: &placeholderTestUri, + }, + }, + } placeholderSpc = &secv1.SecretProviderClass{ ObjectMeta: metav1.ObjectMeta{ Name: "test-spc", @@ -55,9 +71,19 @@ var ( }}, }, } + placeholderDeployment = &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-deployment", + Namespace: "default", + }, + TypeMeta: metav1.TypeMeta{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + } ) -func TestPlaceholderPodControllerIntegration(t *testing.T) { +func TestPlaceholderPodControllerIntegrationWithIng(t *testing.T) { ing := placeholderTestIng.DeepCopy() spc := placeholderSpc.DeepCopy() spc.Labels = manifests.GetTopLevelLabels() @@ -213,6 +239,157 @@ func TestPlaceholderPodControllerIntegration(t *testing.T) { assert.Equal(t, labels, updatedPlaceholder.Spec.Selector.MatchLabels, "selector labels should have been retained") } +func TestPlaceholderPodControllerIntegrationWithNic(t *testing.T) { + ctx := context.Background() + ctx = logr.NewContext(ctx, logr.Discard()) + nic := placeholderTestNginxIngress.DeepCopy() + spc := getDefaultNginxSpc(nic) + spc.SetOwnerReferences(manifests.GetOwnerRefs(nic, true)) + + scheme := runtime.NewScheme() + require.NoError(t, v1alpha1.AddToScheme(scheme)) + require.NoError(t, secv1.AddToScheme(scheme)) + require.NoError(t, appsv1.AddToScheme(scheme)) + + c := fake.NewClientBuilder().WithScheme(scheme).Build() + recorder := record.NewFakeRecorder(10) + require.NoError(t, c.Create(ctx, nic)) + require.NoError(t, c.Create(ctx, spc)) + + p := &PlaceholderPodController{ + client: c, + config: &config.Config{Registry: "test-registry"}, + ingressManager: nil, + events: recorder, + } + + // Create placeholder pod deployment + req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: spc.Namespace, Name: spc.Name}} + beforeErrCount := testutils.GetErrMetricCount(t, placeholderPodControllerName) + beforeReconcileCount := testutils.GetReconcileMetricCount(t, placeholderPodControllerName, metrics.LabelSuccess) + _, err := p.Reconcile(ctx, req) + require.NoError(t, err) + require.Equal(t, testutils.GetErrMetricCount(t, placeholderPodControllerName), beforeErrCount) + require.Greater(t, testutils.GetReconcileMetricCount(t, placeholderPodControllerName, metrics.LabelSuccess), beforeReconcileCount) + + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: spc.Name, + Namespace: spc.Namespace, + }, + } + require.NoError(t, c.Get(ctx, client.ObjectKeyFromObject(dep), dep)) + + replicas := int32(1) + historyLimit := int32(2) + + expectedLabels := map[string]string{"app": spc.Name} + expected := appsv1.DeploymentSpec{ + Replicas: &replicas, + RevisionHistoryLimit: &historyLimit, + Selector: &metav1.LabelSelector{MatchLabels: expectedLabels}, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: expectedLabels, + Annotations: map[string]string{ + "kubernetes.azure.com/observed-generation": "123", + "kubernetes.azure.com/purpose": "hold CSI mount to enable keyvault-to-k8s secret mirroring", + "kubernetes.azure.com/nginx-ingress-controller-owner": nic.Name, + "openservicemesh.io/sidecar-injection": "disabled", + }, + }, + Spec: *manifests.WithPreferSystemNodes(&corev1.PodSpec{ + AutomountServiceAccountToken: util.ToPtr(false), + Containers: []corev1.Container{{ + Name: "placeholder", + Image: "test-registry/oss/kubernetes/pause:3.6-hotfix.20220114", + VolumeMounts: []corev1.VolumeMount{{ + Name: "secrets", + MountPath: "/mnt/secrets", + ReadOnly: true, + }}, + Resources: corev1.ResourceRequirements{ + Limits: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("20m"), + corev1.ResourceMemory: resource.MustParse("24Mi"), + }, + }, + }}, + Volumes: []corev1.Volume{{ + Name: "secrets", + VolumeSource: corev1.VolumeSource{ + CSI: &corev1.CSIVolumeSource{ + Driver: "secrets-store.csi.k8s.io", + ReadOnly: util.ToPtr(true), + VolumeAttributes: map[string]string{"secretProviderClass": spc.Name}, + }, + }, + }}, + }), + }, + } + fmt.Printf("%v", dep) + assert.Equal(t, expected, dep.Spec) + + // Prove idempotence + beforeErrCount = testutils.GetErrMetricCount(t, placeholderPodControllerName) + beforeReconcileCount = testutils.GetReconcileMetricCount(t, placeholderPodControllerName, metrics.LabelSuccess) + _, err = p.Reconcile(ctx, req) + require.NoError(t, err) + require.Equal(t, testutils.GetErrMetricCount(t, placeholderPodControllerName), beforeErrCount) + require.Greater(t, testutils.GetReconcileMetricCount(t, placeholderPodControllerName, metrics.LabelSuccess), beforeReconcileCount) + + // Update the secret class generation + spc.Generation = 234 + expected.Template.Annotations["kubernetes.azure.com/observed-generation"] = "234" + require.NoError(t, c.Update(ctx, spc)) + + beforeErrCount = testutils.GetErrMetricCount(t, placeholderPodControllerName) + beforeReconcileCount = testutils.GetReconcileMetricCount(t, placeholderPodControllerName, metrics.LabelSuccess) + _, err = p.Reconcile(ctx, req) + require.NoError(t, err) + require.Equal(t, testutils.GetErrMetricCount(t, placeholderPodControllerName), beforeErrCount) + require.Greater(t, testutils.GetReconcileMetricCount(t, placeholderPodControllerName, metrics.LabelSuccess), beforeReconcileCount) + + // Prove the generation annotation was updated + require.NoError(t, c.Get(ctx, client.ObjectKeyFromObject(dep), dep)) + assert.Equal(t, expected, dep.Spec) + + // Remove Key Vault URI + nic.Spec.DefaultSSLCertificate.KeyVaultURI = nil + require.NoError(t, c.Update(ctx, nic)) + + beforeErrCount = testutils.GetErrMetricCount(t, placeholderPodControllerName) + beforeReconcileCount = testutils.GetReconcileMetricCount(t, placeholderPodControllerName, metrics.LabelSuccess) + _, err = p.Reconcile(ctx, req) + require.NoError(t, err) + require.Equal(t, testutils.GetErrMetricCount(t, placeholderPodControllerName), beforeErrCount) + require.Greater(t, testutils.GetReconcileMetricCount(t, placeholderPodControllerName, metrics.LabelSuccess), beforeReconcileCount) + + // Prove the deployment was deleted + require.True(t, errors.IsNotFound(c.Get(ctx, client.ObjectKeyFromObject(dep), dep))) + + // Prove idempotence + require.True(t, errors.IsNotFound(c.Get(ctx, client.ObjectKeyFromObject(dep), dep))) + + // Prove that placeholder deployment retains immutable fields during updates + oldPlaceholder := &appsv1.Deployment{} + labels := map[string]string{"foo": "bar", "fizz": "buzz"} + oldPlaceholder.Spec.Selector = &metav1.LabelSelector{MatchLabels: labels} + oldPlaceholder.Name = "immutable-test" + require.NoError(t, c.Create(ctx, oldPlaceholder), "failed to create old placeholder deployment") + beforeErrCount = testutils.GetErrMetricCount(t, placeholderPodControllerName) + beforeReconcileCount = testutils.GetReconcileMetricCount(t, placeholderPodControllerName, metrics.LabelSuccess) + _, err = p.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(oldPlaceholder)}) + require.NoError(t, err) + require.Equal(t, testutils.GetErrMetricCount(t, placeholderPodControllerName), beforeErrCount) + require.Greater(t, testutils.GetReconcileMetricCount(t, placeholderPodControllerName, metrics.LabelSuccess), beforeReconcileCount) + + updatedPlaceholder := &appsv1.Deployment{} + require.NoError(t, c.Get(ctx, client.ObjectKeyFromObject(oldPlaceholder), updatedPlaceholder), "failed to get updated placeholder deployment") + assert.Equal(t, labels, updatedPlaceholder.Spec.Selector.MatchLabels, "selector labels should have been retained") +} + func TestPlaceholderPodControllerNoManagedByLabels(t *testing.T) { ing := placeholderTestIng.DeepCopy() spc := placeholderSpc.DeepCopy() @@ -357,9 +534,13 @@ func TestNewPlaceholderPodController(t *testing.T) { err = NewPlaceholderPodController(m, conf, ingressManager) require.NoError(t, err) + + // test nil ingress manager for nginx ingress controller + err = NewPlaceholderPodController(m, conf, nil) + require.NoError(t, err) } -func TestGetCurrentDeployment(t *testing.T) { +func TestGetCurrentDeploymentWithIng(t *testing.T) { dep := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ Name: "test-deployment", @@ -379,3 +560,143 @@ func TestGetCurrentDeployment(t *testing.T) { require.NoError(t, err) require.Nil(t, dep) } + +func TestPlaceholderPodCleanCheck(t *testing.T) { + ctx := context.Background() + ctx = logr.NewContext(ctx, logr.Discard()) + scheme := runtime.NewScheme() + require.NoError(t, v1alpha1.AddToScheme(scheme)) + require.NoError(t, netv1.AddToScheme(scheme)) + require.NoError(t, appsv1.AddToScheme(scheme)) + + nic := placeholderTestNginxIngress.DeepCopy() + ing := placeholderTestIng.DeepCopy() + ing.TypeMeta.Kind = "Ingress" + unmanagedIngClassName := "unmanagedClassName" + errorIngClassName := "errorClassName" + + c := fake.NewClientBuilder().WithScheme(scheme).Build() + require.NoError(t, c.Create(ctx, nic)) + require.NoError(t, c.Create(ctx, ing)) + p := &PlaceholderPodController{ + client: c, + config: &config.Config{Registry: "test-registry"}, + ingressManager: NewIngressManagerFromFn(func(ing *netv1.Ingress) (bool, error) { + if ing.Spec.IngressClassName != nil { + if *ing.Spec.IngressClassName == unmanagedIngClassName { + return false, nil + } + + if *ing.Spec.IngressClassName == errorIngClassName { + return false, fmt.Errorf("an error has occured checking if ingress is managed") + } + } + return true, nil + }), + } + + // Default scenarios for ingress and nginxingresscontroller. Does not clean when spc required fields are there + cleanPod, err := p.placeholderPodCleanCheck(nic) + require.NoError(t, err) + require.Equal(t, false, cleanPod) + + cleanPod, err = p.placeholderPodCleanCheck(ing) + require.NoError(t, err) + require.Equal(t, false, cleanPod) + + // Clean Placeholder Pod scenarios + + // nic without key vault uri + nic.Spec.DefaultSSLCertificate.KeyVaultURI = nil + cleanPod, err = p.placeholderPodCleanCheck(nic) + require.NoError(t, err) + require.Equal(t, true, cleanPod) + + // nic without DefaultSSLCertificate + nic.Spec.DefaultSSLCertificate = nil + cleanPod, err = p.placeholderPodCleanCheck(nic) + require.NoError(t, err) + require.Equal(t, true, cleanPod) + + // ingress without IngressClassName + ing.Spec.IngressClassName = nil + cleanPod, err = p.placeholderPodCleanCheck(ing) + require.NoError(t, err) + require.Equal(t, true, cleanPod) + + ing.Spec.IngressClassName = placeholderTestIng.Spec.IngressClassName //returning value to IngressClassName to test individual fields triggering clean + + // ingress with empty Name field + ing.Name = "" + cleanPod, err = p.placeholderPodCleanCheck(ing) + require.NoError(t, err) + require.Equal(t, true, cleanPod) + + // unmanaged ingress + ing.Spec.IngressClassName = &unmanagedIngClassName + cleanPod, err = p.placeholderPodCleanCheck(ing) + require.NoError(t, err) + require.Equal(t, true, cleanPod) + + // ingress that hits an error while checking if it's managed + ing.Spec.IngressClassName = &errorIngClassName + cleanPod, err = p.placeholderPodCleanCheck(ing) + require.Error(t, err, "determining if ingress is managed: an error has occured checking if ingress is managed") + require.Equal(t, false, cleanPod) +} + +func TestBuildDeployment(t *testing.T) { + ctx := context.Background() + ctx = logr.NewContext(ctx, logr.Discard()) + scheme := runtime.NewScheme() + require.NoError(t, v1alpha1.AddToScheme(scheme)) + require.NoError(t, netv1.AddToScheme(scheme)) + require.NoError(t, appsv1.AddToScheme(scheme)) + + spc := placeholderSpc.DeepCopy() + + c := fake.NewClientBuilder().WithScheme(scheme).Build() + p := &PlaceholderPodController{ + client: c, + config: &config.Config{Registry: "test-registry"}, + ingressManager: NewIngressManagerFromFn(func(ing *netv1.Ingress) (bool, error) { + return true, nil + }), + } + + dep := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: spc.Name, + Namespace: spc.Namespace, + }, + } + + var emptyObj client.Object + + err := p.buildDeployment(ctx, dep, spc, emptyObj) + require.EqualError(t, err, "failed to build deployment: object type not ingress or nginxingresscontroller") +} + +func getDefaultNginxSpc(nic *v1alpha1.NginxIngressController) *secv1.SecretProviderClass { + spc := &secv1.SecretProviderClass{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "secrets-store.csi.x-k8s.io/v1", + Kind: "SecretProviderClass", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: DefaultNginxCertName(nic), + Namespace: config.DefaultNs, + Labels: manifests.GetTopLevelLabels(), + Generation: 123, + OwnerReferences: []metav1.OwnerReference{{ + APIVersion: nic.APIVersion, + Controller: util.ToPtr(true), + Kind: "NginxIngressController", + Name: nic.Name, + UID: nic.UID, + }}, + }, + } + + return spc +} diff --git a/pkg/controller/nginxingress/nginx_ingress_controller.go b/pkg/controller/nginxingress/nginx_ingress_controller.go index f7df3f9e..651d5d18 100644 --- a/pkg/controller/nginxingress/nginx_ingress_controller.go +++ b/pkg/controller/nginxingress/nginx_ingress_controller.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "github.com/Azure/aks-app-routing-operator/pkg/controller/keyvault" "net/url" "time" @@ -562,9 +563,13 @@ func ToNginxIngressConfig(nic *approutingv1alpha1.NginxIngressController, defaul TargetCPUUtilizationPercentage: getTargetCPUUtilizationPercentage(nic), } - if nic.Spec.DefaultSSLCertificate != nil && - nic.Spec.DefaultSSLCertificate.Secret.Name != "" && nic.Spec.DefaultSSLCertificate.Secret.Namespace != "" { - nginxIng.DefaultSSLCertificate = nic.Spec.DefaultSSLCertificate.Secret.Namespace + "/" + nic.Spec.DefaultSSLCertificate.Secret.Name + if cert := nic.Spec.DefaultSSLCertificate; cert != nil { + if cert.Secret != nil && cert.Secret.Name != "" && cert.Secret.Namespace != "" { + nginxIng.DefaultSSLCertificate = cert.Secret.Namespace + "/" + cert.Secret.Name + } + if cert != nil && cert.KeyVaultURI != nil { + nginxIng.DefaultSSLCertificate = config.DefaultNs + "/" + keyvault.DefaultNginxCertName(nic) + } } return nginxIng diff --git a/pkg/controller/nginxingress/nginx_ingress_controller_test.go b/pkg/controller/nginxingress/nginx_ingress_controller_test.go index 25c080a3..2ec14893 100644 --- a/pkg/controller/nginxingress/nginx_ingress_controller_test.go +++ b/pkg/controller/nginxingress/nginx_ingress_controller_test.go @@ -73,6 +73,86 @@ func TestReconcileResources(t *testing.T) { } }) + t.Run("valid resources with defaultSSLCertificate Secret", func(t *testing.T) { + cl := fake.NewFakeClient() + events := record.NewFakeRecorder(10) + n := &nginxIngressControllerReconciler{ + conf: &config.Config{ + NS: "default", + }, + client: cl, + events: events, + } + + nic := &approutingv1alpha1.NginxIngressController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nic", + }, + Spec: approutingv1alpha1.NginxIngressControllerSpec{ + IngressClassName: "ingressClassName", + ControllerNamePrefix: "prefix", + DefaultSSLCertificate: &approutingv1alpha1.DefaultSSLCertificate{Secret: &approutingv1alpha1.Secret{Name: "test-name", Namespace: "test-namespace"}}, + }, + } + res := n.ManagedResources(nic) + + managed, err := n.ReconcileResource(context.Background(), nic, res) + require.NoError(t, err) + require.True(t, len(managed) == len(res.Objects())-1, "expected all resources to be returned as managed except the namespace") + + // prove objects were created + for _, obj := range res.Objects() { + require.NoError(t, cl.Get(context.Background(), client.ObjectKeyFromObject(obj), obj)) + } + + // no events + select { + case <-events.Events: + require.Fail(t, "expected no events") + default: + } + }) + + t.Run("valid resources with defaultSSLCertificate key vault URI", func(t *testing.T) { + cl := fake.NewFakeClient() + events := record.NewFakeRecorder(10) + n := &nginxIngressControllerReconciler{ + conf: &config.Config{ + NS: "default", + }, + client: cl, + events: events, + } + kvUri := "https://testvault.vault.azure.net/certificates/testcert/f8982febc6894c0697b884f946fb1a34" + nic := &approutingv1alpha1.NginxIngressController{ + ObjectMeta: metav1.ObjectMeta{ + Name: "nic", + }, + Spec: approutingv1alpha1.NginxIngressControllerSpec{ + IngressClassName: "ingressClassName", + ControllerNamePrefix: "prefix", + DefaultSSLCertificate: &approutingv1alpha1.DefaultSSLCertificate{KeyVaultURI: &kvUri}, + }, + } + res := n.ManagedResources(nic) + + managed, err := n.ReconcileResource(context.Background(), nic, res) + require.NoError(t, err) + require.True(t, len(managed) == len(res.Objects())-1, "expected all resources to be returned as managed except the namespace") + + // prove objects were created + for _, obj := range res.Objects() { + require.NoError(t, cl.Get(context.Background(), client.ObjectKeyFromObject(obj), obj)) + } + + // no events + select { + case <-events.Events: + require.Fail(t, "expected no events") + default: + } + }) + t.Run("invalid resources", func(t *testing.T) { cl := fake.NewFakeClient() events := record.NewFakeRecorder(10) diff --git a/testing/e2e/go.mod b/testing/e2e/go.mod index 41102966..976e2331 100644 --- a/testing/e2e/go.mod +++ b/testing/e2e/go.mod @@ -30,6 +30,7 @@ require ( k8s.io/apimachinery v0.28.1 k8s.io/client-go v0.28.1 sigs.k8s.io/controller-runtime v0.16.1 + sigs.k8s.io/secrets-store-csi-driver v1.3.4 ) require ( @@ -41,25 +42,32 @@ require ( github.com/Azure/go-autorest/autorest/to v0.4.0 // indirect github.com/Azure/go-autorest/logger v0.2.1 // indirect github.com/Azure/go-autorest/tracing v0.6.0 // indirect + github.com/Azure/secrets-store-csi-driver-provider-azure v1.4.1 // indirect github.com/AzureAD/microsoft-authentication-library-for-go v1.0.0 // indirect + github.com/beorn7/perks v1.0.1 // indirect + github.com/cespare/xxhash/v2 v2.2.0 // indirect github.com/cjlapao/common-go v0.0.39 // indirect github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc // indirect github.com/emicklei/go-restful/v3 v3.11.0 // indirect github.com/evanphx/json-patch/v5 v5.6.0 // indirect + github.com/fsnotify/fsnotify v1.6.0 // indirect github.com/go-logr/stdr v1.2.2 // indirect github.com/go-openapi/jsonpointer v0.20.0 // indirect github.com/go-openapi/jsonreference v0.20.2 // indirect github.com/go-openapi/swag v0.22.4 // indirect github.com/gogo/protobuf v1.3.2 // indirect + github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/google/gnostic-models v0.6.8 // indirect github.com/google/go-cmp v0.5.9 // indirect github.com/google/gofuzz v1.2.0 // indirect + github.com/imdario/mergo v0.3.16 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/josharian/intern v1.0.0 // indirect github.com/json-iterator/go v1.1.12 // indirect github.com/kylelemons/godebug v1.1.0 // indirect github.com/mailru/easyjson v0.7.7 // indirect + github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect github.com/microsoft/kiota-abstractions-go v1.2.2 // indirect github.com/microsoft/kiota-authentication-azure-go v1.0.0 // indirect github.com/microsoft/kiota-http-go v1.1.0 // indirect @@ -74,7 +82,9 @@ require ( github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 // indirect github.com/pkg/errors v0.9.1 // indirect github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 // indirect + github.com/prometheus/client_model v0.4.0 // indirect github.com/prometheus/common v0.44.0 // indirect + github.com/prometheus/procfs v0.11.1 // indirect github.com/sethvargo/go-envconfig v0.8.0 // indirect github.com/spf13/pflag v1.0.5 // indirect github.com/std-uritemplate/std-uritemplate/go v0.0.42 // indirect @@ -89,11 +99,14 @@ require ( golang.org/x/term v0.15.0 // indirect golang.org/x/text v0.14.0 // indirect golang.org/x/time v0.3.0 // indirect + gomodules.xyz/jsonpatch/v2 v2.4.0 // indirect google.golang.org/appengine v1.6.7 // indirect google.golang.org/protobuf v1.31.0 // indirect gopkg.in/inf.v0 v0.9.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect + k8s.io/apiextensions-apiserver v0.28.1 // indirect + k8s.io/component-base v0.28.1 // indirect k8s.io/klog/v2 v2.100.1 // indirect k8s.io/kube-openapi v0.0.0-20230816210353-14e408962443 // indirect k8s.io/utils v0.0.0-20230726121419-3b25d923346b // indirect diff --git a/testing/e2e/go.sum b/testing/e2e/go.sum index cf95c76c..04ab9afd 100644 --- a/testing/e2e/go.sum +++ b/testing/e2e/go.sum @@ -1,3 +1,4 @@ +github.com/Azure/azure-sdk-for-go v66.0.0+incompatible h1:bmmC38SlE8/E81nNADlgmVGurPWMHDX2YNXVQMrBpEE= github.com/Azure/azure-sdk-for-go/sdk/azcore v1.7.2 h1:t5+QXLCK9SVi0PPdaY0PrFvYUo24KwA0QwxnaHRSVd4= github.com/Azure/azure-sdk-for-go/sdk/azcore v1.7.2/go.mod h1:bjGvMhVMb+EEm3VRNQawDMUyMMjo+S5ewNjflkep/0Q= github.com/Azure/azure-sdk-for-go/sdk/azidentity v1.3.0 h1:vcYCAze6p19qBW7MhZybIsqD8sMV8js0NyQM8JDnVtg= @@ -44,12 +45,16 @@ github.com/Azure/go-autorest/logger v0.2.1 h1:IG7i4p/mDa2Ce4TRyAO8IHnVhAVF3RFU+Z github.com/Azure/go-autorest/logger v0.2.1/go.mod h1:T9E3cAhj2VqvPOtCYAvby9aBXkZmbF5NWuPV8+WeEW8= github.com/Azure/go-autorest/tracing v0.6.0 h1:TYi4+3m5t6K48TGI9AUdb+IzbnSxvnvUMfuitfgcfuo= github.com/Azure/go-autorest/tracing v0.6.0/go.mod h1:+vhtPC754Xsa23ID7GlGsrdKBpUA79WCAKPPZVC2DeU= +github.com/Azure/secrets-store-csi-driver-provider-azure v1.4.1 h1:24/mZ06Uzu8EkdgJj8zcy5C3Ipsg1doM62hx0WPscNU= +github.com/Azure/secrets-store-csi-driver-provider-azure v1.4.1/go.mod h1:xUXKV8vOut59vIrFhyEY+4PgiK2LXkP10BtI+2y8VXM= github.com/AzureAD/microsoft-authentication-library-for-go v1.0.0 h1:OBhqkivkhkMqLPymWEppkm7vgPQY2XsHoEkaMQ0AdZY= github.com/AzureAD/microsoft-authentication-library-for-go v1.0.0/go.mod h1:kgDmCTgBzIEPFElEF+FK0SdjAor06dRq2Go927dnQ6o= github.com/avast/retry-go v3.0.0+incompatible h1:4SOWQ7Qs+oroOTQOYnAHqelpCO0biHSxpiH9JdtuBj0= github.com/avast/retry-go v3.0.0+incompatible/go.mod h1:XtSnn+n/sHqQIpZ10K1qAevBhOOCWBLXXy3hyiqqBrY= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= +github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= github.com/cespare/xxhash/v2 v2.2.0 h1:DC2CZ1Ep5Y4k3ZQ899DldepgrayRUGE6BBZ/cd9Cj44= +github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/cjlapao/common-go v0.0.39 h1:bAAUrj2B9v0kMzbAOhzjSmiyDy+rd56r2sy7oEiQLlA= github.com/cjlapao/common-go v0.0.39/go.mod h1:M3dzazLjTjEtZJbbxoA5ZDiGCiHmpwqW9l4UWaddwOA= github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= @@ -65,6 +70,7 @@ github.com/evanphx/json-patch v5.6.0+incompatible h1:jBYDEEiFBPxA0v50tFdvOzQQTCv github.com/evanphx/json-patch/v5 v5.6.0 h1:b91NhWfaz02IuVxO9faSllyAtNXHMPkC5J8sJCLunww= github.com/evanphx/json-patch/v5 v5.6.0/go.mod h1:G79N1coSVB93tBe7j6PhzjmR3/2VvlbKOFpnXhI9Bw4= github.com/fsnotify/fsnotify v1.6.0 h1:n+5WquG0fcWoWp6xPWfHdbskMCQaFnG6PfBrh1Ky4HY= +github.com/fsnotify/fsnotify v1.6.0/go.mod h1:sl3t1tCWJFWoRz9R8WJCbQihKKwmorjAbSClcnxKAGw= github.com/go-logr/logr v1.2.0/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.2.2/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbVz/1A= github.com/go-logr/logr v1.2.4 h1:g01GSCwiDw2xSZfjJ2/T9M+S6pFdcNtFYsp+Y43HYDQ= @@ -86,6 +92,9 @@ github.com/gogo/protobuf v1.3.2/go.mod h1:P1XiOD3dCwIKUDQYPy72D8LYyHL2YPYrpS2s69 github.com/golang-jwt/jwt/v4 v4.0.0/go.mod h1:/xlHOz8bRuivTWchD4jCa+NbatV+wEUSzwAxVc6locg= github.com/golang-jwt/jwt/v4 v4.5.0 h1:7cYmW1XlMY7h7ii7UhUyChSgS5wUJEnm9uZVTGqOWzg= github.com/golang-jwt/jwt/v4 v4.5.0/go.mod h1:m21LjoU+eqJr34lmDMbreY2eSTRJ1cv77w39/MY0Ch0= +github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da h1:oI5xCqsCo564l8iNU+DwB5epxmsaqB+rhGL0m5jtYqE= +github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da/go.mod h1:cIg4eruTrX1D+g88fzRXU5OdNfaM+9IcxsU14FzY7Hc= +github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= @@ -102,6 +111,7 @@ github.com/google/pprof v0.0.0-20210720184732-4bb14d4b1be1 h1:K6RDEckDVWvDI9JAJY github.com/google/uuid v1.3.1 h1:KjJaJ9iWZ3jOFZIf1Lqf4laDRCasjl0BCmnEGxkdLb4= github.com/google/uuid v1.3.1/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= github.com/imdario/mergo v0.3.16 h1:wwQJbIsHYGMUyLSPrEq1CT16AhnhNJQ51+4fdHUnCl4= +github.com/imdario/mergo v0.3.16/go.mod h1:WBLT9ZmE3lPoWsEzCh9LPo3TiwVN+ZKEjmz+hD27ysY= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/jessevdk/go-flags v1.4.0/go.mod h1:4FA24M0QyGHXBuZZK/XkWh8h0e1EYbRYJSGM75WSRxI= @@ -123,6 +133,7 @@ github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+ github.com/mailru/easyjson v0.7.7 h1:UGYAvKxe3sBsEDzO8ZeWOSlIQfWFlxbzLZe7hwFURr0= github.com/mailru/easyjson v0.7.7/go.mod h1:xzfreul335JAWq5oZzymOObrkdz5UnU4kGfJJLY9Nlc= github.com/matttproud/golang_protobuf_extensions v1.0.4 h1:mmDVorXM7PCGKw94cs5zkfA9PSy5pEvNWRP0ET0TIVo= +github.com/matttproud/golang_protobuf_extensions v1.0.4/go.mod h1:BSXmuO+STAnVfrANrmjBb36TMTDstsz7MSK+HVaYKv4= github.com/microsoft/kiota-abstractions-go v1.2.2 h1:2i/zHAxjbONp7Q5+qN3NEF1zuf5YWMKCLxbkVqPm6uQ= github.com/microsoft/kiota-abstractions-go v1.2.2/go.mod h1:mWUfljJB88owshlQl6LhnqlEHmWyutTPK7A/L7AGnNw= github.com/microsoft/kiota-authentication-azure-go v1.0.0 h1:29FNZZ/4nnCOwFcGWlB/sxPvWz487HA2bXH8jR5k2Rk= @@ -149,6 +160,7 @@ github.com/modern-go/reflect2 v1.0.2/go.mod h1:yWuevngMOJpCy52FWWMvUC8ws7m/LJsjY github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 h1:C3w9PqII01/Oq1c1nUAm88MOHcQC9l5mIlSMApZMrHA= github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822/go.mod h1:+n7T8mK8HuQTcFwEeznm/DIxMOiR9yIdICNftLE1DvQ= github.com/mwitkow/go-conntrack v0.0.0-20190716064945-2f068394615f h1:KUppIJq7/+SVif2QVs3tOP0zanoHgBEVAwHxUSIzRqU= +github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= github.com/onsi/ginkgo/v2 v2.11.0 h1:WgqUCUt/lT6yXoQ8Wef0fsNn5cAuMK7+KT9UFRz2tcU= github.com/onsi/gomega v1.27.10 h1:naR28SdDFlqrG6kScpT8VWpu1xWY5nJRCF3XaYyBjhI= github.com/pkg/browser v0.0.0-20210911075715-681adbf594b8 h1:KoWmjvw+nsYOo29YJK9vDA65RGE3NrOnUtO7a+RF9HU= @@ -162,9 +174,11 @@ github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH github.com/prometheus/client_golang v1.16.0 h1:yk/hx9hDbrGHovbci4BY+pRMfSuuat626eFsHb7tmT8= github.com/prometheus/client_golang v1.16.0/go.mod h1:Zsulrv/L9oM40tJ7T815tM89lFEugiJ9HzIqaAx4LKc= github.com/prometheus/client_model v0.4.0 h1:5lQXD3cAg1OXBf4Wq03gTrXHeaV0TQvGfUooCfx1yqY= +github.com/prometheus/client_model v0.4.0/go.mod h1:oMQmHW1/JoDwqLtg57MGgP/Fb1CJEYF2imWWhWtMkYU= github.com/prometheus/common v0.44.0 h1:+5BrQJwiBB9xsMygAB3TNvpQKOwlkc25LbISbrdOOfY= github.com/prometheus/common v0.44.0/go.mod h1:ofAIvZbQ1e/nugmZGz4/qCb9Ap1VoSTIO7x0VV9VvuY= github.com/prometheus/procfs v0.11.1 h1:xRC8Iq1yyca5ypa9n1EZnWZkt7dwcoRPQwX/5gwaUuI= +github.com/prometheus/procfs v0.11.1/go.mod h1:eesXgaPo1q7lBpVMoMy0ZOFTth9hBn4W/y0/p/ScXhY= github.com/rogpeppe/go-internal v1.10.0 h1:TMyTOH3F/DB16zRVcYyreMH6GnZZrwQVAoYjRBZyWFQ= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/sethvargo/go-envconfig v0.8.0 h1:AcmdAewSFAc7pQ1Ghz+vhZkilUtxX559QlDuLLiSkdI= @@ -196,6 +210,7 @@ go.opentelemetry.io/otel/metric v1.18.0 h1:JwVzw94UYmbx3ej++CwLUQZxEODDj/pOuTCvz go.opentelemetry.io/otel/metric v1.18.0/go.mod h1:nNSpsVDjWGfb7chbRLUNW+PBNdcSTHD4Uu5pfFMOI0k= go.opentelemetry.io/otel/trace v1.18.0 h1:NY+czwbHbmndxojTEKiSMHkG2ClNH2PwmcHrdo0JY10= go.opentelemetry.io/otel/trace v1.18.0/go.mod h1:T2+SGJGuYZY3bjj5rgh/hN7KIrlpWC5nS8Mjvzckz+0= +go.uber.org/goleak v1.2.1 h1:NBol2c7O1ZokfZ0LEU9K6Whx/KnwvepVetCUhtKja4A= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/zap v1.25.0 h1:4Hvk6GtkucQ790dqmj7l1eEnRdKm3k3ZUrUMS2d5+5c= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= @@ -224,6 +239,7 @@ golang.org/x/net v0.14.0 h1:BONx9s002vGdD9umnlX1Po8vOZmrgH34qlHcD1MfK14= golang.org/x/net v0.14.0/go.mod h1:PpSgVXXLK0OxS0F31C1/tv6XNguvCrnXIDrFMspZIUI= golang.org/x/oauth2 v0.11.0 h1:vPL4xzxBM4niKCW6g9whtaWVXTJf1U5e4aZxxFx/gbU= golang.org/x/oauth2 v0.11.0/go.mod h1:LdF7O/8bLR/qWK9DrpXmbHLTouvRHK0SgJl0GmDBchk= +golang.org/x/sync v0.0.0-20181221193216-37e7f081c4d4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -239,6 +255,7 @@ golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20210616045830-e2b7044e8c71/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220908164124-27713097b956/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= @@ -268,6 +285,7 @@ golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8T golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gomodules.xyz/jsonpatch/v2 v2.4.0 h1:Ci3iUJyx9UeRx7CeFN8ARgGbkESwJK+KB9lLcWxY/Zw= +gomodules.xyz/jsonpatch/v2 v2.4.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY= google.golang.org/appengine v1.6.7 h1:FZR1q0exgwxzPzp/aF+VccGrSfxfPpkBqjIIEq3ru6c= google.golang.org/appengine v1.6.7/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= @@ -288,10 +306,13 @@ gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= k8s.io/api v0.28.1 h1:i+0O8k2NPBCPYaMB+uCkseEbawEt/eFaiRqUx8aB108= k8s.io/api v0.28.1/go.mod h1:uBYwID+66wiL28Kn2tBjBYQdEU0Xk0z5qF8bIBqk/Dg= k8s.io/apiextensions-apiserver v0.28.1 h1:l2ThkBRjrWpw4f24uq0Da2HaEgqJZ7pcgiEUTKSmQZw= +k8s.io/apiextensions-apiserver v0.28.1/go.mod h1:sVvrI+P4vxh2YBBcm8n2ThjNyzU4BQGilCQ/JAY5kGs= k8s.io/apimachinery v0.28.1 h1:EJD40og3GizBSV3mkIoXQBsws32okPOy+MkRyzh6nPY= k8s.io/apimachinery v0.28.1/go.mod h1:X0xh/chESs2hP9koe+SdIAcXWcQ+RM5hy0ZynB+yEvw= k8s.io/client-go v0.28.1 h1:pRhMzB8HyLfVwpngWKE8hDcXRqifh1ga2Z/PU9SXVK8= k8s.io/client-go v0.28.1/go.mod h1:pEZA3FqOsVkCc07pFVzK076R+P/eXqsgx5zuuRWukNE= +k8s.io/component-base v0.28.1 h1:LA4AujMlK2mr0tZbQDZkjWbdhTV5bRyEyAFe0TJxlWg= +k8s.io/component-base v0.28.1/go.mod h1:jI11OyhbX21Qtbav7JkhehyBsIRfnO8oEgoAR12ArIU= k8s.io/klog/v2 v2.100.1 h1:7WCHKK6K8fNhTqfBhISHQ97KrnJNFZMcQvKp7gP/tmg= k8s.io/klog/v2 v2.100.1/go.mod h1:y1WjHnz7Dj687irZUWR/WLkLc5N1YHtjLdmgWjndZn0= k8s.io/kube-openapi v0.0.0-20230816210353-14e408962443 h1:CAIciCnJnSOQxPd0xvpV6JU3D4AJvnYbImPpFpO9Hnw= @@ -302,6 +323,8 @@ sigs.k8s.io/controller-runtime v0.16.1 h1:+15lzrmHsE0s2kNl0Dl8cTchI5Cs8qofo5PGcP sigs.k8s.io/controller-runtime v0.16.1/go.mod h1:vpMu3LpI5sYWtujJOa2uPK61nB5rbwlN7BAB8aSLvGU= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd h1:EDPBXCAspyGV4jQlpZSudPeMmr1bNJefnuqLsRAsHZo= sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd/go.mod h1:B8JuhiUyNFVKdsE8h686QcCxMaH6HrOAZj4vswFpcB0= +sigs.k8s.io/secrets-store-csi-driver v1.3.4 h1:rCMOb2I4lJaN6sw0CjT6YHA8ts2yscWAOBGu0EaCIWk= +sigs.k8s.io/secrets-store-csi-driver v1.3.4/go.mod h1:jh6wML45aTbxT2YZtU4khzSm8JYxwVrQbhsum+WR6j8= sigs.k8s.io/structured-merge-diff/v4 v4.3.0 h1:UZbZAZfX0wV2zr7YZorDz6GXROfDFj6LvqCRm4VUVKk= sigs.k8s.io/structured-merge-diff/v4 v4.3.0/go.mod h1:N8hJocpFajUSSeSJ9bOZ77VzejKZaXsTtZo4/u7Io08= sigs.k8s.io/yaml v1.3.0 h1:a2VclLzOGrwOHDiV8EfBGhvjHvP46CtW5j6POvhYGGo= diff --git a/testing/e2e/suites/nginxIngressController.go b/testing/e2e/suites/nginxIngressController.go index 64aa20ec..5e37d406 100644 --- a/testing/e2e/suites/nginxIngressController.go +++ b/testing/e2e/suites/nginxIngressController.go @@ -4,8 +4,11 @@ import ( "context" "errors" "fmt" + "github.com/Azure/aks-app-routing-operator/pkg/controller/keyvault" "time" + secv1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1" + "github.com/Azure/aks-app-routing-operator/api/v1alpha1" "github.com/Azure/aks-app-routing-operator/pkg/util" "github.com/Azure/aks-app-routing-operator/testing/e2e/infra" @@ -41,6 +44,7 @@ func init() { appsv1.AddToScheme(scheme) policyv1.AddToScheme(scheme) rbacv1.AddToScheme(scheme) + secv1.AddToScheme(scheme) } func nicTests(in infra.Provisioned) []test { @@ -113,6 +117,32 @@ func nicTests(in infra.Provisioned) []test { return fmt.Errorf("able to create NginxIngressController despite missing Secret field'%s'", testNIC.Spec.ControllerNamePrefix) } + validUri := "https://testvault.vault.azure.net/certificates/testcert/f8982febc6894c0697b884f946fb1a34" + + invalidUri := "Invalid.URI" + testNIC = manifests.NewNginxIngressController("nginx-ingress-controller", "nginxingressclass") + testNIC.Spec.DefaultSSLCertificate = &v1alpha1.DefaultSSLCertificate{ + KeyVaultURI: &invalidUri, + } + lgr.Info("creating NginxIngressController with invalid keyvault uri field") + if err := c.Create(ctx, testNIC); err == nil { + return fmt.Errorf("able to create NginxIngressController despite invalid key vault uri'%s'", testNIC.Spec.ControllerNamePrefix) + } + + testNIC = manifests.NewNginxIngressController("nginx-ingress-controller", "nginxingressclass") + testNIC.Spec.DefaultSSLCertificate = &v1alpha1.DefaultSSLCertificate{ + KeyVaultURI: &validUri, + Secret: &v1alpha1.Secret{ + Name: "validname", + Namespace: "validnamespace", + }, + } + lgr.Info("creating NginxIngressController with both keyvault uri field and secret field") + if err := c.Create(ctx, testNIC); err == nil { + return fmt.Errorf("able to create NginxIngressController despite having both keyvaulturi and secret fields'%s'", testNIC.Spec.ControllerNamePrefix) + } + + // scaling profile rejectTests := []struct { name string nic *v1alpha1.NginxIngressController @@ -262,6 +292,102 @@ func nicTests(in infra.Provisioned) []test { return err } + lgr.Info("finished testing") + return nil + }, + }, + { + name: "testing DefaultSSLCertificate validity", + cfgs: builderFromInfra(in). + withOsm(in, false, true). + withVersions(manifests.OperatorVersionLatest). + withZones([]manifests.DnsZoneCount{manifests.DnsZoneCountNone}, []manifests.DnsZoneCount{manifests.DnsZoneCountNone}). + build(), + run: func(ctx context.Context, config *rest.Config, operator manifests.OperatorConfig) error { + lgr := logger.FromContext(ctx) + lgr.Info("starting test") + + c, err := client.New(config, client.Options{ + Scheme: scheme, + }) + if err != nil { + return fmt.Errorf("creating client: %w") + } + + // get keyvault uri + kvuri := in.Cert.GetId() + + // create defaultSSLCert + defaultSSLCert := v1alpha1.DefaultSSLCertificate{ + KeyVaultURI: &kvuri, + } + + // create nic and upsert + testNIC := manifests.NewNginxIngressController("nginx-ingress-controller", "nginxingressclass") + testNIC.Spec.DefaultSSLCertificate = &defaultSSLCert + if err := upsert(ctx, c, testNIC); err != nil { + return fmt.Errorf("upserting NIC: %w", err) + } + + var service *v1alpha1.ManagedObjectReference + var nic v1alpha1.NginxIngressController + lgr.Info("waiting for NIC to be available") + if err = wait.PollImmediate(1*time.Second, 1*time.Minute, func() (bool, error) { + lgr.Info("checking if NIC is available") + if err := c.Get(ctx, client.ObjectKeyFromObject(testNIC), &nic); err != nil { + return false, fmt.Errorf("get nic: %w", err) + } + + for _, cond := range nic.Status.Conditions { + if cond.Type == v1alpha1.ConditionTypeAvailable { + lgr.Info("found nic") + if len(nic.Status.ManagedResourceRefs) == 0 { + lgr.Info("nic has no ManagedResourceRefs") + return false, nil + } + return true, nil + } + } + lgr.Info("nic not available") + return false, nil + }); err != nil { + return fmt.Errorf("waiting for test NIC to be available: %w", err) + } + + lgr.Info("checking if associated SPC is created") + spc := &secv1.SecretProviderClass{ + ObjectMeta: metav1.ObjectMeta{ + Name: keyvault.DefaultNginxCertName(testNIC), + Namespace: "app-routing-system", + }, + } + cleanSPC := &secv1.SecretProviderClass{} + + if err := c.Get(ctx, client.ObjectKeyFromObject(spc), cleanSPC); err != nil { + lgr.Info("spc not found") + return err + } + lgr.Info("found spc") + + lgr.Info("checking for service in managed resource refs") + for _, ref := range nic.Status.ManagedResourceRefs { + if ref.Kind == "Service" { + lgr.Info("found service") + service = &ref + } + } + + if service == nil { + return fmt.Errorf("no service available in resource refs") + } + + if err := clientServerTest(ctx, config, operator, nil, in, func(ingress *netv1.Ingress, service *corev1.Service, z zoner) error { + ingress.Spec.IngressClassName = to.Ptr(testNIC.Spec.IngressClassName) + return nil + }, to.Ptr(service.Name)); err != nil { + return err + } + lgr.Info("finished testing") return nil },