Skip to content

Commit

Permalink
Unit test updates and refactors
Browse files Browse the repository at this point in the history
  • Loading branch information
@aamgayle
aamgayle committed Oct 19, 2023
1 parent ecdce88 commit b232c42
Show file tree
Hide file tree
Showing 7 changed files with 195 additions and 111 deletions.
4 changes: 2 additions & 2 deletions pkg/controller/keyvault/ingress_secret_provider_class.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ func (i *IngressSecretProviderClassReconciler) Reconcile(ctx context.Context, re
return result, client.IgnoreNotFound(err)
}

if len(spc.Labels) != 0 && manifests.HasTopLevelLabels(spc.Labels) {
if len(spc.Labels) != 0 && manifests.HasRequiredLabels(spc.Labels, manifests.GetTopLevelLabels()) {
logger.Info("removing secret provider class for ingress")
err = i.client.Delete(ctx, spc)
return result, client.IgnoreNotFound(err)
Expand All @@ -136,7 +136,7 @@ func (i *IngressSecretProviderClassReconciler) buildSPC(ing *netv1.Ingress, spc
return false, nil
}

if len(ing.Labels) == 0 || !(manifests.HasTopLevelLabels(ing.Labels)) {
if len(ing.Labels) == 0 || !(manifests.HasRequiredLabels(ing.Labels, manifests.GetTopLevelLabels())) {
return false, nil
}

Expand Down
112 changes: 28 additions & 84 deletions pkg/controller/keyvault/ingress_secret_provider_class_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"context"
"fmt"
"github.com/Azure/aks-app-routing-operator/pkg/manifests"
"github.com/Azure/aks-app-routing-operator/pkg/util"
"net/url"
"testing"

Expand All @@ -30,7 +31,7 @@ import (
kvcsi "github.com/Azure/secrets-store-csi-driver-provider-azure/pkg/provider/types"
)

func TestSecretProviderClassReconcilerLabelChecking(t *testing.T) {
func TestIngressSecretProviderClassReconcilerIntegration(t *testing.T) {
ing := &netv1.Ingress{}
ing.Name = "test-ingress"
ing.Namespace = "default"
Expand All @@ -39,6 +40,7 @@ func TestSecretProviderClassReconcilerLabelChecking(t *testing.T) {
ing.Annotations = map[string]string{
"kubernetes.azure.com/tls-cert-keyvault-uri": "https://testvault.vault.azure.net/certificates/testcert/f8982febc6894c0697b884f946fb1a34",
}
ing.Labels = manifests.GetTopLevelLabels()

c := fake.NewClientBuilder().WithObjects(ing).Build()
require.NoError(t, secv1.AddToScheme(c.Scheme()))
Expand Down Expand Up @@ -68,6 +70,7 @@ func TestSecretProviderClassReconcilerLabelChecking(t *testing.T) {
spc := &secv1.SecretProviderClass{}
spc.Name = "keyvault-" + ing.Name
spc.Namespace = ing.Namespace
spc.Labels = ing.Labels
require.NoError(t, c.Get(ctx, client.ObjectKeyFromObject(spc), spc))

expected := &secv1.SecretProviderClass{
Expand Down Expand Up @@ -111,97 +114,36 @@ func TestSecretProviderClassReconcilerLabelChecking(t *testing.T) {
require.NoError(t, err)
require.Equal(t, testutils.GetErrMetricCount(t, ingressSecretProviderControllerName), beforeErrCount)
require.Greater(t, testutils.GetReconcileMetricCount(t, ingressSecretProviderControllerName, metrics.LabelSuccess), beforeRequestCount)
}

func TestIngressSecretProviderClassReconcilerIntegration(t *testing.T) {
ing := &netv1.Ingress{}
ing.Name = "test-ingress"
ing.Namespace = "default"
ingressClass := "webapprouting.kubernetes.azure.com"
ing.Spec.IngressClassName = &ingressClass
ing.Annotations = map[string]string{
"kubernetes.azure.com/tls-cert-keyvault-uri": "https://testvault.vault.azure.net/certificates/testcert/f8982febc6894c0697b884f946fb1a34",
}

c := fake.NewClientBuilder().WithObjects(ing).Build()
require.NoError(t, secv1.AddToScheme(c.Scheme()))
i := &IngressSecretProviderClassReconciler{
client: c,
config: &config.Config{
TenantID: "test-tenant-id",
MSIClientID: "test-msi-client-id",
},
ingressManager: NewIngressManager(map[string]struct{}{ingressClass: {}}),
}

ctx := context.Background()
ctx = logr.NewContext(ctx, logr.Discard())

// Create the secret provider class
req := ctrl.Request{NamespacedName: types.NamespacedName{Namespace: ing.Namespace, Name: ing.Name}}
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)

// Prove it exists
spc := &secv1.SecretProviderClass{}
spc.Name = "keyvault-" + ing.Name
spc.Namespace = ing.Namespace
// Prove the objectVersion property was removed
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": i.config.TenantID,
"useVMManagedIdentity": "true",
"userAssignedIdentityID": i.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"},
},
}},
},
}
expected.Spec.Parameters["objects"] = "{\"array\":[\"{\\\"objectName\\\":\\\"testcert\\\",\\\"objectType\\\":\\\"secret\\\"}\"]}"
assert.Equal(t, expected.Spec, spc.Spec)

// Check for idempotence
// Remove the cert annotation from the ingress
ing.Annotations = map[string]string{}
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",
}
require.NoError(t, i.client.Update(ctx, ing))
// Prove secret class was removed
require.True(t, errors.IsNotFound(c.Get(ctx, client.ObjectKeyFromObject(spc), spc)))

// Check for idempotence
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)

// 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 ingress
ing.Annotations = map[string]string{}
fakeLabels := map[string]string{"fake1": "label1", "fake2": "label2", "fake3": "label3"}
// Check for top level labels with additional labels
ing.Labels = util.MergeMaps(manifests.GetTopLevelLabels(), fakeLabels)
require.NoError(t, i.client.Update(ctx, ing))
beforeErrCount = testutils.GetErrMetricCount(t, ingressSecretProviderControllerName)
beforeRequestCount = testutils.GetReconcileMetricCount(t, ingressSecretProviderControllerName, metrics.LabelSuccess)
Expand All @@ -210,16 +152,16 @@ func TestIngressSecretProviderClassReconcilerIntegration(t *testing.T) {
require.Equal(t, testutils.GetErrMetricCount(t, ingressSecretProviderControllerName), beforeErrCount)
require.Greater(t, testutils.GetReconcileMetricCount(t, ingressSecretProviderControllerName, metrics.LabelSuccess), beforeRequestCount)

// Prove secret class was removed
require.True(t, errors.IsNotFound(c.Get(ctx, client.ObjectKeyFromObject(spc), spc)))

// Check for idempotence
// Check for labels without top level labels
ing.Labels = fakeLabels
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)

}

func TestIngressSecretProviderClassReconcilerInvalidURL(t *testing.T) {
Expand All @@ -231,6 +173,7 @@ func TestIngressSecretProviderClassReconcilerInvalidURL(t *testing.T) {
ing.Annotations = map[string]string{
"kubernetes.azure.com/tls-cert-keyvault-uri": "inv@lid URL",
}
ing.Labels = manifests.GetTopLevelLabels()

c := fake.NewClientBuilder().WithObjects(ing).Build()
require.NoError(t, secv1.AddToScheme(c.Scheme()))
Expand Down Expand Up @@ -276,6 +219,7 @@ func TestIngressSecretProviderClassReconcilerBuildSPCInvalidURLs(t *testing.T) {

ing := &netv1.Ingress{}
ing.Spec.IngressClassName = &ingressClass
ing.Labels = manifests.GetTopLevelLabels()

t.Run("missing ingress class", func(t *testing.T) {
ing := ing.DeepCopy()
Expand Down Expand Up @@ -375,6 +319,7 @@ func TestIngressSecretProviderClassReconcilerBuildSPCCloud(t *testing.T) {
Annotations: map[string]string{
"kubernetes.azure.com/tls-cert-keyvault-uri": "https://test.vault.azure.net/secrets/test-secret",
},
Labels: manifests.GetTopLevelLabels(),
},
Spec: netv1.IngressSpec{
IngressClassName: &ingressClass,
Expand All @@ -397,9 +342,12 @@ func TestIngressSecretProviderClassReconcilerBuildSPCLabelChecking(t *testing.T)
ingressClass := "webapprouting.kubernetes.azure.com"

i := &IngressSecretProviderClassReconciler{
config: &config.Config{
TenantID: "test-tenant-id",
MSIClientID: "test-msi-client-id",
},
ingressManager: NewIngressManager(map[string]struct{}{ingressClass: {}}),
}

ing := &netv1.Ingress{}
ing.Spec.IngressClassName = &ingressClass
ing.Annotations = map[string]string{"kubernetes.azure.com/tls-cert-keyvault-uri": "https://testvault.vault.azure.net/certificates/testcert"}
Expand Down Expand Up @@ -434,11 +382,7 @@ func TestIngressSecretProviderClassReconcilerBuildSPCLabelChecking(t *testing.T)
t.Run("top level labels with extra labels", func(t *testing.T) {
ing := ing.DeepCopy()
extraLabels := map[string]string{"fake": "label", "fake2": "label2", "fake3": "label3"}
ing.Labels = extraLabels
for key, label := range manifests.GetTopLevelLabels() {
ing.Labels[key] = label
}

ing.Labels = util.MergeMaps(manifests.GetTopLevelLabels(), extraLabels)
ing.Name = "test-ingress"

ok, err := i.buildSPC(ing, &secv1.SecretProviderClass{})
Expand Down
7 changes: 4 additions & 3 deletions pkg/controller/keyvault/placeholder_pod.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ func (p *PlaceholderPodController) Reconcile(ctx context.Context, req ctrl.Reque
ObjectMeta: metav1.ObjectMeta{
Name: spc.Name,
Namespace: spc.Namespace,
Labels: manifests.GetTopLevelLabels(),
Labels: spc.Labels,
OwnerReferences: []metav1.OwnerReference{{
APIVersion: spc.APIVersion,
Controller: util.BoolPtr(true),
Expand All @@ -111,6 +111,7 @@ func (p *PlaceholderPodController) Reconcile(ctx context.Context, req ctrl.Reque
}

managed := p.ingressManager.IsManaging(ing)

if ing.Name == "" || ing.Spec.IngressClassName == nil || !managed {
logger.Info("cleaning unused placeholder pod deployment")

Expand All @@ -119,7 +120,7 @@ func (p *PlaceholderPodController) Reconcile(ctx context.Context, req ctrl.Reque
return result, client.IgnoreNotFound(err)

}
if len(dep.Labels) != 0 && manifests.HasTopLevelLabels(dep.Labels) {
if len(dep.Labels) != 0 && manifests.HasRequiredLabels(dep.Labels, manifests.GetTopLevelLabels()) {
logger.Info("deleting placeholder deployment")
err = p.client.Delete(ctx, dep)
return result, client.IgnoreNotFound(err)
Expand All @@ -136,7 +137,7 @@ func (p *PlaceholderPodController) Reconcile(ctx context.Context, req ctrl.Reque
}

func (p *PlaceholderPodController) buildDeployment(dep *appsv1.Deployment, spc *secv1.SecretProviderClass, ing *netv1.Ingress) {
labels := map[string]string{"app": spc.Name}
labels := util.MergeMaps(map[string]string{"app": spc.Name}, spc.Labels)
dep.Spec = appsv1.DeploymentSpec{
Replicas: util.Int32Ptr(1),
RevisionHistoryLimit: util.Int32Ptr(2),
Expand Down
Loading

0 comments on commit b232c42

Please sign in to comment.