Skip to content

Commit

Permalink
add managed-by label checks for spc, placeholder pods and ingress bac…
Browse files Browse the repository at this point in the history 
…kend (#111)
  • Loading branch information
@aamgayle
aamgayle authored Oct 26, 2023
1 parent 4d1b718 commit 4db42d5
Show file tree
Hide file tree
Showing 8 changed files with 468 additions and 99 deletions.
19 changes: 14 additions & 5 deletions pkg/controller/keyvault/ingress_secret_provider_class.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"net/url"
"strings"

"github.com/Azure/aks-app-routing-operator/pkg/controller/controllername"
"github.com/go-logr/logr"
netv1 "k8s.io/api/networking/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand All @@ -20,7 +19,9 @@ import (
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"
)
Expand Down Expand Up @@ -91,6 +92,7 @@ func (i *IngressSecretProviderClassReconciler) Reconcile(ctx context.Context, re
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("keyvault-%s", ing.Name),
Namespace: ing.Namespace,
Labels: manifests.GetTopLevelLabels(),
OwnerReferences: []metav1.OwnerReference{{
APIVersion: ing.APIVersion,
Controller: util.BoolPtr(true),
Expand All @@ -115,14 +117,21 @@ func (i *IngressSecretProviderClassReconciler) Reconcile(ctx context.Context, re

logger.Info("cleaning unused managed spc for ingress")
logger.Info("getting secret provider class for ingress")
err = i.client.Get(ctx, client.ObjectKeyFromObject(spc), spc)

toCleanSPC := &secv1.SecretProviderClass{}

err = i.client.Get(ctx, client.ObjectKeyFromObject(spc), toCleanSPC)
if err != nil {
return result, client.IgnoreNotFound(err)
}

logger.Info("removing secret provider class for ingress")
err = i.client.Delete(ctx, spc)
return result, 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)
}

return result, nil
}

func (i *IngressSecretProviderClassReconciler) buildSPC(ing *netv1.Ingress, spc *secv1.SecretProviderClass) (bool, error) {
Expand Down
186 changes: 147 additions & 39 deletions pkg/controller/keyvault/ingress_secret_provider_class_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import (
"net/url"
"testing"

"github.com/Azure/aks-app-routing-operator/pkg/controller/testutils"

"github.com/go-logr/logr"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand All @@ -26,18 +24,31 @@ import (

"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"
kvcsi "github.com/Azure/secrets-store-csi-driver-provider-azure/pkg/provider/types"
)

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",
var (
spcTestIngressClassName = "webapprouting.kubernetes.azure.com"
spcTestIngress = &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,
},
}
)

func TestIngressSecretProviderClassReconcilerIntegration(t *testing.T) {
// Create the ingress
ing := spcTestIngress.DeepCopy()

c := fake.NewClientBuilder().WithObjects(ing).Build()
require.NoError(t, secv1.AddToScheme(c.Scheme()))
Expand All @@ -47,7 +58,7 @@ func TestIngressSecretProviderClassReconcilerIntegration(t *testing.T) {
TenantID: "test-tenant-id",
MSIClientID: "test-msi-client-id",
},
ingressManager: NewIngressManager(map[string]struct{}{ingressClass: {}}),
ingressManager: NewIngressManager(map[string]struct{}{spcTestIngressClassName: {}}),
}

ctx := context.Background()
Expand All @@ -67,6 +78,7 @@ func TestIngressSecretProviderClassReconcilerIntegration(t *testing.T) {
spc := &secv1.SecretProviderClass{}
spc.Name = "keyvault-" + ing.Name
spc.Namespace = ing.Namespace
spc.Labels = manifests.GetTopLevelLabels()
require.NoError(t, c.Get(ctx, client.ObjectKeyFromObject(spc), spc))

expected := &secv1.SecretProviderClass{
Expand Down Expand Up @@ -138,12 +150,114 @@ func TestIngressSecretProviderClassReconcilerIntegration(t *testing.T) {
require.Greater(t, testutils.GetReconcileMetricCount(t, ingressSecretProviderControllerName, metrics.LabelSuccess), beforeRequestCount)
}

func TestIngressSecretProviderClassReconcilerIntegrationWithoutSPCLabels(t *testing.T) {
// Create the ingress
ing := spcTestIngress.DeepCopy()

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{}{spcTestIngressClassName: {}}),
}

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)

spc := &secv1.SecretProviderClass{
TypeMeta: metav1.TypeMeta{
APIVersion: "secrets-store.csi.x-k8s.io/v1",
Kind: "SecretProviderClass",
},
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("keyvault-%s", ing.Name),
Namespace: ing.Namespace,
Labels: manifests.GetTopLevelLabels(),
OwnerReferences: []metav1.OwnerReference{{
APIVersion: ing.APIVersion,
Controller: util.BoolPtr(true),
Kind: ing.Kind,
Name: ing.Name,
UID: ing.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, i.client.Update(ctx, spc))
assert.Equal(t, 0, len(spc.Labels))

// Remove the cert annotation from the ingress
ing.Annotations = map[string]string{}
require.NoError(t, i.client.Update(ctx, ing))

// Reconcile both changes
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 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": 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"},
},
}},
},
}
assert.Equal(t, expected.Spec, spc.Spec)

// 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)
}

func TestIngressSecretProviderClassReconcilerInvalidURL(t *testing.T) {
ing := &netv1.Ingress{}
ing.Name = "test-ingress"
ing.Namespace = "default"
ingressClass := "webapprouting.kubernetes.azure.com"
ing.Spec.IngressClassName = &ingressClass
// Create the ingress
ing := spcTestIngress.DeepCopy()
ing.Annotations = map[string]string{
"kubernetes.azure.com/tls-cert-keyvault-uri": "inv@lid URL",
}
Expand All @@ -158,7 +272,7 @@ func TestIngressSecretProviderClassReconcilerInvalidURL(t *testing.T) {
MSIClientID: "test-msi-client-id",
},
events: recorder,
ingressManager: NewIngressManager(map[string]struct{}{ingressClass: {}}),
ingressManager: NewIngressManager(map[string]struct{}{spcTestIngressClassName: {}}),
}

ctx := context.Background()
Expand All @@ -184,17 +298,18 @@ func TestIngressSecretProviderClassReconcilerInvalidURL(t *testing.T) {
}

func TestIngressSecretProviderClassReconcilerBuildSPCInvalidURLs(t *testing.T) {
ingressClass := "webapprouting.kubernetes.azure.com"

i := &IngressSecretProviderClassReconciler{
ingressManager: NewIngressManager(map[string]struct{}{ingressClass: {}}),
ingressManager: NewIngressManager(map[string]struct{}{spcTestIngressClassName: {}}),
}

ing := &netv1.Ingress{}
ing.Spec.IngressClassName = &ingressClass
invalidURLIng := &netv1.Ingress{
Spec: netv1.IngressSpec{
IngressClassName: &spcTestIngressClassName,
},
}

t.Run("missing ingress class", func(t *testing.T) {
ing := ing.DeepCopy()
ing := invalidURLIng.DeepCopy()
ing.Spec.IngressClassName = nil
ing.Annotations = map[string]string{"kubernetes.azure.com/tls-cert-keyvault-uri": "inv@lid URL"}

Expand All @@ -204,7 +319,7 @@ func TestIngressSecretProviderClassReconcilerBuildSPCInvalidURLs(t *testing.T) {
})

t.Run("incorrect ingress class", func(t *testing.T) {
ing := ing.DeepCopy()
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"}
Expand All @@ -215,15 +330,15 @@ func TestIngressSecretProviderClassReconcilerBuildSPCInvalidURLs(t *testing.T) {
})

t.Run("nil annotations", func(t *testing.T) {
ing := ing.DeepCopy()
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 := ing.DeepCopy()
ing := invalidURLIng.DeepCopy()
ing.Annotations = map[string]string{"kubernetes.azure.com/tls-cert-keyvault-uri": ""}

ok, err := i.buildSPC(ing, &secv1.SecretProviderClass{})
Expand All @@ -232,7 +347,7 @@ func TestIngressSecretProviderClassReconcilerBuildSPCInvalidURLs(t *testing.T) {
})

t.Run("url with control character", func(t *testing.T) {
ing := ing.DeepCopy()
ing := invalidURLIng.DeepCopy()
cc := string([]byte{0x7f})
ing.Annotations = map[string]string{"kubernetes.azure.com/tls-cert-keyvault-uri": cc}

Expand All @@ -243,7 +358,7 @@ func TestIngressSecretProviderClassReconcilerBuildSPCInvalidURLs(t *testing.T) {
})

t.Run("url with one path segment", func(t *testing.T) {
ing := ing.DeepCopy()
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{})
Expand Down Expand Up @@ -278,23 +393,16 @@ func TestIngressSecretProviderClassReconcilerBuildSPCCloud(t *testing.T) {

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
ingressClass := "webapprouting.kubernetes.azure.com"
i := &IngressSecretProviderClassReconciler{
config: &config.Config{
Cloud: c.configCloud,
},
ingressManager: NewIngressManager(map[string]struct{}{ingressClass: {}}),
ingressManager: NewIngressManager(map[string]struct{}{spcTestIngressClassName: {}}),
}

ing := &netv1.Ingress{
ObjectMeta: metav1.ObjectMeta{
Annotations: map[string]string{
"kubernetes.azure.com/tls-cert-keyvault-uri": "https://test.vault.azure.net/secrets/test-secret",
},
},
Spec: netv1.IngressSpec{
IngressClassName: &ingressClass,
},
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{}
Expand Down
Loading

0 comments on commit 4db42d5

Please sign in to comment.