refactor: replace WorkloadIdentity with ServiceAccountName

This change simplifies the SpinApp CRD by removing the complex WorkloadIdentity struct and replacing it with a simple ServiceAccountName field.

Signed-off-by: Jiaxiao (mossaka) Zhou <[email protected]>

Author: Mossaka

api/v1alpha1/spinapp_types.go

@@ -87,9 +87,10 @@ type SpinAppSpec struct {
 	// +kubebuilder:validation:MinItems:=1
 	Components []string `json:"components,omitempty"`
 
-	// WorkloadIdentity defines the workload identity configuration for cloud provider authentication.
+	// ServiceAccountName is the name of the Kubernetes service account to use for the pod.
+	// If not specified, the default service account will be used.
 	// +optional
-	WorkloadIdentity *WorkloadIdentity `json:"workloadIdentity,omitempty"`
+	ServiceAccountName string `json:"serviceAccountName,omitempty"`
 }
 
 // SpinAppStatus defines the observed state of SpinApp
@@ -291,17 +292,6 @@ type HTTPHealthProbeHeader struct {
 	Value string `json:"value"`
 }
 
-// WorkloadIdentity defines the configuration for cloud provider workload identity.
-type WorkloadIdentity struct {
-	// ServiceAccountName is the name of the Kubernetes service account to use for workload identity.
-	// +kubebuilder:validation:Required
-	ServiceAccountName string `json:"serviceAccountName"`
-
-	// ProviderMetadata contains cloud provider-specific configuration for workload identity.
-	// +kubebuilder:validation:Required
-	ProviderMetadata map[string]string `json:"providerMetadata"`
-}
-
 func init() {
 	SchemeBuilder.Register(&SpinApp{}, &SpinAppList{})
 }

api/v1alpha1/zz_generated.deepcopy.go

@@ -68,7 +68,6 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
 github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc=
 github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
-github.com/grpc-ecosystem/grpc-gateway v1.16.0 h1:gmcG1KaJ57LophUzW0Hy8NmPhnMZb4M0+kPpLofRdBo=
 github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0 h1:bkypFPDjIYGfCYD5mRBvpqxfYX1YCS1PXdKYWi8FsN0=
 github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0/go.mod h1:P+Lt/0by1T8bfcF3z737NnSbmxQAppXMRziHUxPOC8k=
 github.com/imdario/mergo v0.3.15 h1:M8XP7IuFNsqUx6VPK2P9OSmsYsI/YFaGil0uD21V3dM=
@@ -214,7 +213,6 @@ golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8T
 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/genproto v0.0.0-20230822172742-b8732ec3820d h1:VBu5YqKPv6XiJ199exd8Br+Aetz+o08F+PLMnwJQHAY=
 google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157 h1:7whR9kGa5LUwFtpLm2ArCEejtnxlGeLbAyjFY8sGNFw=
 google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157/go.mod h1:99sLkeliLXfdj2J75X3Ho+rrVCaJze0uwN7zDDkjPVU=
 google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 h1:BwIjyKYGsK9dMCBOorzRri8MQwmi7mT9rGHsCEinZkA=

go.sum

@@ -422,15 +422,6 @@ func constructDeployment(ctx context.Context, app *spinv1alpha1.SpinApp, config
 	}
 	maps.Copy(templateLabels, readyLabels)
 
-	// Add Azure workload identity label if configured
-	if app.Spec.WorkloadIdentity != nil {
-		if val, ok := app.Spec.WorkloadIdentity.ProviderMetadata["azure"]; ok {
-			if val == "true" {
-				templateLabels["azure.workload.identity/use"] = "true"
-			}
-		}
-	}
-
 	// TODO: Once we land admission webhooks write some validation for this e.g.
 	// don't allow setting memory limit with cyclotron runtime.
 	resources := corev1.ResourceRequirements{
@@ -453,6 +444,8 @@ func constructDeployment(ctx context.Context, app *spinv1alpha1.SpinApp, config
 
 	labels := constructAppLabels(app)
 
+	serviceAccountName := getServiceAccountName(ctx, app)
+
 	var container corev1.Container
 	if config.RuntimeClassName != nil {
 		container = corev1.Container{
@@ -518,7 +511,7 @@ func constructDeployment(ctx context.Context, app *spinv1alpha1.SpinApp, config
 				},
 				Spec: corev1.PodSpec{
 					RuntimeClassName:   config.RuntimeClassName,
-					ServiceAccountName: getServiceAccountName(app),
+					ServiceAccountName: serviceAccountName,
 					Containers:         []corev1.Container{container},
 					ImagePullSecrets:   app.Spec.ImagePullSecrets,
 					Volumes:            volumes,
@@ -541,12 +534,22 @@ func constructDeployment(ctx context.Context, app *spinv1alpha1.SpinApp, config
 }
 
 // getServiceAccountName returns the service account name to use for the deployment.
-// If workload identity is configured, it returns the configured service account name.
+// If serviceAccountName is specified on the SpinApp, it returns that value.
 // Otherwise, it returns "default" which is the Kubernetes default.
-func getServiceAccountName(app *spinv1alpha1.SpinApp) string {
-	if app.Spec.WorkloadIdentity != nil {
-		return app.Spec.WorkloadIdentity.ServiceAccountName
+func getServiceAccountName(ctx context.Context, app *spinv1alpha1.SpinApp) string {
+	log := logging.FromContext(ctx).WithValues("component", "getServiceAccountName")
+
+	log.Debug("Determining service account name",
+		"app", app.Name,
+		"namespace", app.Namespace,
+		"serviceAccountNameInSpec", app.Spec.ServiceAccountName)
+
+	if app.Spec.ServiceAccountName != "" {
+		log.Debug("Using service account from SpinApp", "serviceAccountName", app.Spec.ServiceAccountName)
+		return app.Spec.ServiceAccountName
 	}
+
+	log.Info("Using default service account", "serviceAccountName", "default")
 	return "default"
 }
 

internal/controller/spinapp_controller.go

@@ -631,7 +631,7 @@ func TestReconcile_Integration_Deployment_SpinCAInjection(t *testing.T) {
 	wg.Wait()
 }
 
-func TestReconcile_Integration_WorkloadIdentity_Azure(t *testing.T) {
+func TestReconcile_Integration_Deployment_ServiceAccountName(t *testing.T) {
 	t.Parallel()
 
 	envTest, mgr, _ := setupController(t)
@@ -646,7 +646,6 @@ func TestReconcile_Integration_WorkloadIdentity_Azure(t *testing.T) {
 		wg.Done()
 	}()
 
-	// Create an executor that creates a deployment
 	executor := &spinv1alpha1.SpinAppExecutor{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      "executor",
@@ -655,7 +654,7 @@ func TestReconcile_Integration_WorkloadIdentity_Azure(t *testing.T) {
 		Spec: spinv1alpha1.SpinAppExecutorSpec{
 			CreateDeployment: true,
 			DeploymentConfig: &spinv1alpha1.ExecutorDeploymentConfig{
-				RuntimeClassName: generics.Ptr("a-runtime-class"),
+				RuntimeClassName: generics.Ptr("foobar"),
 			},
 		},
 	}
@@ -668,34 +667,25 @@ func TestReconcile_Integration_WorkloadIdentity_Azure(t *testing.T) {
 			Namespace: "default",
 		},
 		Spec: spinv1alpha1.SpinAppSpec{
-			Executor: "executor",
-			Image:    "ghcr.io/radu-matei/perftest:v1",
-			WorkloadIdentity: &spinv1alpha1.WorkloadIdentity{
-				ServiceAccountName: "custom-sa",
-				ProviderMetadata: map[string]string{
-					"azure": "true",
-				},
-			},
+			Executor:           "executor",
+			Image:              "ghcr.io/radu-matei/perftest:v1",
+			ServiceAccountName: "my-service-account",
 		},
 	}
 
 	require.NoError(t, envTest.k8sClient.Create(ctx, spinApp))
 
-	// Wait for the underlying deployment to exist
 	var deployment appsv1.Deployment
 	require.Eventually(t, func() bool {
 		err := envTest.k8sClient.Get(ctx,
 			types.NamespacedName{
 				Namespace: "default",
-				Name:      "app"},
+				Name:      spinApp.Name},
 			&deployment)
 		return err == nil
 	}, 3*time.Second, 100*time.Millisecond)
 
-	// Verify Azure workload identity label is set
-	require.Equal(t, "true", deployment.Spec.Template.ObjectMeta.Labels["azure.workload.identity/use"])
-	// Verify service account name is set
-	require.Equal(t, "custom-sa", deployment.Spec.Template.Spec.ServiceAccountName)
+	require.Equal(t, "my-service-account", deployment.Spec.Template.Spec.ServiceAccountName)
 
 	// Terminate the context to force the manager to shut down.
 	cancelFunc()

internal/controller/spinapp_controller_test.go

@@ -64,9 +64,7 @@ func (v *SpinAppValidator) validateSpinApp(ctx context.Context, spinApp *spinv1a
 	if err := validateAnnotations(spinApp.Spec, executor); err != nil {
 		allErrs = append(allErrs, err)
 	}
-	if err := validateWorkloadIdentity(spinApp.Spec); err != nil {
-		allErrs = append(allErrs, err)
-	}
+
 	if len(allErrs) == 0 {
 		return nil
 	}
@@ -140,17 +138,3 @@ func validateAnnotations(spec spinv1alpha1.SpinAppSpec, executor *spinv1alpha1.S
 
 	return nil
 }
-
-func validateWorkloadIdentity(spec spinv1alpha1.SpinAppSpec) *field.Error {
-	if spec.WorkloadIdentity == nil {
-		return nil
-	}
-
-	if spec.WorkloadIdentity.ServiceAccountName == "" {
-		return field.Required(
-			field.NewPath("spec").Child("workloadIdentity").Child("serviceAccountName"),
-			"serviceAccountName must be provided when workload identity is configured")
-	}
-
-	return nil
-}