Remove OIDC service account, when OIDC feature is disabled again (#7570)

* oidc feature disabled, remove serviceaccount Signed-off-by: pawarpranav83 <[email protected]> * directly create serviceAccount and place it in listers Signed-off-by: pawarpranav83 <[email protected]> * owner ref check, serviceaccount created by user is not deleted Signed-off-by: pawarpranav83 <[email protected]> * added test scenario when auth-oidc enabled then disabled Signed-off-by: pawarpranav83 <[email protected]> * handled errs Signed-off-by: pawarpranav83 <[email protected]> * Apply suggestions from code review Co-authored-by: Christoph Stäbler <[email protected]> --------- Signed-off-by: pawarpranav83 <[email protected]> Co-authored-by: Christoph Stäbler <[email protected]>
knative · Jan 22, 2024 · bb5313d · bb5313d
1 parent 44ff98b
commit bb5313d
Show file tree

Hide file tree

Showing 2 changed files with 98 additions and 0 deletions.
diff --git a/pkg/auth/serviceaccount.go b/pkg/auth/serviceaccount.go
@@ -101,6 +101,26 @@ func EnsureOIDCServiceAccountExistsForResource(ctx context.Context, serviceAccou
 	return nil
 }
 
+// DeleteOIDCServiceAccountIfExists makes sure the given resource does not have an OIDC service account.
+// If it does that service account is deleted.
+func DeleteOIDCServiceAccountIfExists(ctx context.Context, serviceAccountLister corev1listers.ServiceAccountLister, kubeclient kubernetes.Interface, gvk schema.GroupVersionKind, objectMeta metav1.ObjectMeta) error {
+	saName := GetOIDCServiceAccountNameForResource(gvk, objectMeta)
+	sa, err := serviceAccountLister.ServiceAccounts(objectMeta.Namespace).Get(saName)
+
+	if err == nil && metav1.IsControlledBy(&sa.ObjectMeta, &objectMeta) {
+		logging.FromContext(ctx).Debugf("OIDC Service account exists and has correct owner (%s/%s). Deleting OIDC service account", objectMeta.Name, objectMeta.Namespace)
+
+		err = kubeclient.CoreV1().ServiceAccounts(objectMeta.Namespace).Delete(ctx, sa.Name, metav1.DeleteOptions{})
+		if err != nil {
+			return fmt.Errorf("could not delete OIDC service account %s/%s for %s: %w", objectMeta.Name, objectMeta.Namespace, gvk.Kind, err)
+		}
+	} else if apierrs.IsNotFound(err) {
+		return nil
+	}
+
+	return err
+}
+
 type OIDCIdentityStatusMarker interface {
 	MarkOIDCIdentityCreatedSucceeded()
 	MarkOIDCIdentityCreatedSucceededWithReason(reason, messageFormat string, messageA ...interface{})
@@ -119,6 +139,9 @@ func SetupOIDCServiceAccount(ctx context.Context, flags feature.Flags, serviceAc
 		}
 		marker.MarkOIDCIdentityCreatedSucceeded()
 	} else {
+		if err := DeleteOIDCServiceAccountIfExists(ctx, serviceAccountLister, kubeclient, gvk, objectMeta); err != nil {
+			return err
+		}
 		setAuthStatus(nil)
 		marker.MarkOIDCIdentityCreatedSucceededWithReason(fmt.Sprintf("%s feature disabled", feature.OIDCAuthentication), "")
 	}

diff --git a/pkg/auth/serviceaccount_test.go b/pkg/auth/serviceaccount_test.go
@@ -157,6 +157,8 @@ func TestSetupOIDCServiceAccount(t *testing.T) {
 	listers := rttestingv1.NewListers(eventtypes)
 	trigger := rttestingv1.NewTrigger("my-trigger", "my-namespace", "my-broker")
 	expected := GetOIDCServiceAccountForResource(gvk, objectMeta)
+
+	// authentication-oidc feature enabled
 	err := SetupOIDCServiceAccount(ctx, feature.Flags{
 		feature.OIDCAuthentication: feature.Enabled,
 	}, listers.GetServiceAccountLister(), kubeclient.Get(ctx), gvk, objectMeta, &trigger.Status, func(as *duckv1.AuthStatus) {
@@ -183,6 +185,15 @@ func TestSetupOIDCServiceAccount(t *testing.T) {
 		t.Errorf("SetupOIDCServiceAccount didn't set TriggerConditionOIDCIdentityCreated Status")
 	}
 
+	// authentication-oidc feature disabled, after enabling so service account was created.
+	// expected serviceAccount was created when SetupOIDCServiceAccount was called with feature enabled.
+	listers = rttestingv1.NewListers([]runtime.Object{expected})
+
+	// Verifies that serviceAccount still exists.
+	sa, err := kubeclient.Get(ctx).CoreV1().ServiceAccounts(objectMeta.Namespace).Get(context.TODO(), expected.Name, metav1.GetOptions{})
+	if sa == nil || err != nil {
+		t.Errorf("ServiceAccount does not exist: %+v: %s", sa, err)
+	}
 	err = SetupOIDCServiceAccount(ctx, feature.Flags{
 		feature.OIDCAuthentication: feature.Disabled,
 	}, listers.GetServiceAccountLister(), kubeclient.Get(ctx), gvk, objectMeta, &trigger.Status, func(as *duckv1.AuthStatus) {
@@ -193,6 +204,12 @@ func TestSetupOIDCServiceAccount(t *testing.T) {
 		t.Errorf("SetupOIDCServiceAccount  failed: %s", err)
 	}
 
+	// Checks whether the created serviceAccount still exists or not.
+	sa, err = kubeclient.Get(ctx).CoreV1().ServiceAccounts(objectMeta.Namespace).Get(context.TODO(), expected.Name, metav1.GetOptions{})
+	if sa != nil || err == nil {
+		t.Errorf("DeleteOIDCServiceAccountIfExists failed to delete the serviceAccount: %+v", sa)
+	}
+
 	if trigger.Status.Auth != nil {
 		t.Errorf("SetupOIDCServiceAccount setAuthStatus  failed")
 	}
@@ -210,4 +227,62 @@ func TestSetupOIDCServiceAccount(t *testing.T) {
 	if !matched {
 		t.Errorf("SetupOIDCServiceAccount didn't set TriggerConditionOIDCIdentityCreated Status")
 	}
+
+	// authentication-oidc feature disabled.
+	listers = rttestingv1.NewListers(eventtypes)
+	err = SetupOIDCServiceAccount(ctx, feature.Flags{
+		feature.OIDCAuthentication: feature.Disabled,
+	}, listers.GetServiceAccountLister(), kubeclient.Get(ctx), gvk, objectMeta, &trigger.Status, func(as *duckv1.AuthStatus) {
+		trigger.Status.Auth = as
+	})
+
+	if err != nil {
+		t.Errorf("SetupOIDCServiceAccount  failed: %s", err)
+	}
+
+	if trigger.Status.Auth != nil {
+		t.Errorf("SetupOIDCServiceAccount setAuthStatus  failed")
+	}
+
+	// match OIDCIdentityCreated condition
+	matched = false
+	for _, condition := range trigger.Status.Conditions {
+		if condition.Type == eventingv1.TriggerConditionOIDCIdentityCreated {
+			if condition.Reason == "authentication-oidc feature disabled" {
+				matched = true
+			}
+		}
+	}
+
+	if !matched {
+		t.Errorf("SetupOIDCServiceAccount didn't set TriggerConditionOIDCIdentityCreated Status")
+	}
+}
+
+func TestDeleteOIDCServiceAccountIfExists(t *testing.T) {
+	ctx, _ := rectesting.SetupFakeContext(t)
+	gvk := eventingv1.SchemeGroupVersion.WithKind("Broker")
+	objectMeta := metav1.ObjectMeta{
+		Name:      "my-broker-unique",
+		Namespace: "my-namespace",
+		UID:       "my-uuid",
+	}
+
+	expected := GetOIDCServiceAccountForResource(gvk, objectMeta)
+	serviceAccount, err := kubeclient.Get(ctx).CoreV1().ServiceAccounts(objectMeta.Namespace).Create(ctx, expected, metav1.CreateOptions{})
+	if err != nil {
+		t.Errorf("could not create OIDC service account %s/%s for %s: %s", objectMeta.Name, objectMeta.Namespace, gvk.Kind, err)
+	}
+
+	listers := rttestingv1.NewListers([]runtime.Object{serviceAccount})
+
+	err = DeleteOIDCServiceAccountIfExists(ctx, listers.GetServiceAccountLister(), kubeclient.Get(ctx), gvk, objectMeta)
+	if err != nil {
+		t.Errorf("DeleteOIDCServiceAccountIfExists failed: %s", err)
+	}
+
+	sa, err := kubeclient.Get(ctx).CoreV1().ServiceAccounts(objectMeta.Namespace).Get(context.TODO(), expected.Name, metav1.GetOptions{})
+	if sa != nil || err == nil {
+		t.Errorf("DeleteOIDCServiceAccountIfExists failed to delete the serviceAccount: %+v", sa)
+	}
 }