Skip to content

Commit cc606c7

Browse files
committed
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]>
1 parent 4f86816 commit cc606c7

File tree

7 files changed

+33
-90
lines changed

7 files changed

+33
-90
lines changed

api/v1alpha1/spinapp_types.go

Lines changed: 3 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,10 @@ type SpinAppSpec struct {
8787
// +kubebuilder:validation:MinItems:=1
8888
Components []string `json:"components,omitempty"`
8989

90-
// WorkloadIdentity defines the workload identity configuration for cloud provider authentication.
90+
// ServiceAccountName is the name of the Kubernetes service account to use for the pod.
91+
// If not specified, the default service account will be used.
9192
// +optional
92-
WorkloadIdentity *WorkloadIdentity `json:"workloadIdentity,omitempty"`
93+
ServiceAccountName string `json:"serviceAccountName,omitempty"`
9394
}
9495

9596
// SpinAppStatus defines the observed state of SpinApp
@@ -291,17 +292,6 @@ type HTTPHealthProbeHeader struct {
291292
Value string `json:"value"`
292293
}
293294

294-
// WorkloadIdentity defines the configuration for cloud provider workload identity.
295-
type WorkloadIdentity struct {
296-
// ServiceAccountName is the name of the Kubernetes service account to use for workload identity.
297-
// +kubebuilder:validation:Required
298-
ServiceAccountName string `json:"serviceAccountName"`
299-
300-
// ProviderMetadata contains cloud provider-specific configuration for workload identity.
301-
// +kubebuilder:validation:Required
302-
ProviderMetadata map[string]string `json:"providerMetadata"`
303-
}
304-
305295
func init() {
306296
SchemeBuilder.Register(&SpinApp{}, &SpinAppList{})
307297
}

api/v1alpha1/zz_generated.deepcopy.go

Lines changed: 0 additions & 27 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

config/crd/bases/core.spinkube.dev_spinapps.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -532,6 +532,11 @@ spec:
532532
type: object
533533
type: array
534534
type: object
535+
serviceAccountName:
536+
description: |-
537+
ServiceAccountName is the name of the Kubernetes service account to use for the pod.
538+
If not specified, the default service account will be used.
539+
type: string
535540
serviceAnnotations:
536541
additionalProperties:
537542
type: string

go.sum

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0=
6868
github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
6969
github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc=
7070
github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
71-
github.com/grpc-ecosystem/grpc-gateway v1.16.0 h1:gmcG1KaJ57LophUzW0Hy8NmPhnMZb4M0+kPpLofRdBo=
7271
github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0 h1:bkypFPDjIYGfCYD5mRBvpqxfYX1YCS1PXdKYWi8FsN0=
7372
github.com/grpc-ecosystem/grpc-gateway/v2 v2.20.0/go.mod h1:P+Lt/0by1T8bfcF3z737NnSbmxQAppXMRziHUxPOC8k=
7473
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
214213
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
215214
gomodules.xyz/jsonpatch/v2 v2.4.0 h1:Ci3iUJyx9UeRx7CeFN8ARgGbkESwJK+KB9lLcWxY/Zw=
216215
gomodules.xyz/jsonpatch/v2 v2.4.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY=
217-
google.golang.org/genproto v0.0.0-20230822172742-b8732ec3820d h1:VBu5YqKPv6XiJ199exd8Br+Aetz+o08F+PLMnwJQHAY=
218216
google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157 h1:7whR9kGa5LUwFtpLm2ArCEejtnxlGeLbAyjFY8sGNFw=
219217
google.golang.org/genproto/googleapis/api v0.0.0-20240528184218-531527333157/go.mod h1:99sLkeliLXfdj2J75X3Ho+rrVCaJze0uwN7zDDkjPVU=
220218
google.golang.org/genproto/googleapis/rpc v0.0.0-20240701130421-f6361c86f094 h1:BwIjyKYGsK9dMCBOorzRri8MQwmi7mT9rGHsCEinZkA=

internal/controller/spinapp_controller.go

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -422,15 +422,6 @@ func constructDeployment(ctx context.Context, app *spinv1alpha1.SpinApp, config
422422
}
423423
maps.Copy(templateLabels, readyLabels)
424424

425-
// Add Azure workload identity label if configured
426-
if app.Spec.WorkloadIdentity != nil {
427-
if val, ok := app.Spec.WorkloadIdentity.ProviderMetadata["azure"]; ok {
428-
if val == "true" {
429-
templateLabels["azure.workload.identity/use"] = "true"
430-
}
431-
}
432-
}
433-
434425
// TODO: Once we land admission webhooks write some validation for this e.g.
435426
// don't allow setting memory limit with cyclotron runtime.
436427
resources := corev1.ResourceRequirements{
@@ -453,6 +444,8 @@ func constructDeployment(ctx context.Context, app *spinv1alpha1.SpinApp, config
453444

454445
labels := constructAppLabels(app)
455446

447+
serviceAccountName := getServiceAccountName(ctx, app)
448+
456449
var container corev1.Container
457450
if config.RuntimeClassName != nil {
458451
container = corev1.Container{
@@ -518,7 +511,7 @@ func constructDeployment(ctx context.Context, app *spinv1alpha1.SpinApp, config
518511
},
519512
Spec: corev1.PodSpec{
520513
RuntimeClassName: config.RuntimeClassName,
521-
ServiceAccountName: getServiceAccountName(app),
514+
ServiceAccountName: serviceAccountName,
522515
Containers: []corev1.Container{container},
523516
ImagePullSecrets: app.Spec.ImagePullSecrets,
524517
Volumes: volumes,
@@ -541,12 +534,22 @@ func constructDeployment(ctx context.Context, app *spinv1alpha1.SpinApp, config
541534
}
542535

543536
// getServiceAccountName returns the service account name to use for the deployment.
544-
// If workload identity is configured, it returns the configured service account name.
537+
// If serviceAccountName is specified on the SpinApp, it returns that value.
545538
// Otherwise, it returns "default" which is the Kubernetes default.
546-
func getServiceAccountName(app *spinv1alpha1.SpinApp) string {
547-
if app.Spec.WorkloadIdentity != nil {
548-
return app.Spec.WorkloadIdentity.ServiceAccountName
539+
func getServiceAccountName(ctx context.Context, app *spinv1alpha1.SpinApp) string {
540+
log := logging.FromContext(ctx).WithValues("component", "getServiceAccountName")
541+
542+
log.Debug("Determining service account name",
543+
"app", app.Name,
544+
"namespace", app.Namespace,
545+
"serviceAccountNameInSpec", app.Spec.ServiceAccountName)
546+
547+
if app.Spec.ServiceAccountName != "" {
548+
log.Debug("Using service account from SpinApp", "serviceAccountName", app.Spec.ServiceAccountName)
549+
return app.Spec.ServiceAccountName
549550
}
551+
552+
log.Info("Using default service account", "serviceAccountName", "default")
550553
return "default"
551554
}
552555

internal/controller/spinapp_controller_test.go

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,7 @@ func TestReconcile_Integration_Deployment_SpinCAInjection(t *testing.T) {
631631
wg.Wait()
632632
}
633633

634-
func TestReconcile_Integration_WorkloadIdentity_Azure(t *testing.T) {
634+
func TestReconcile_Integration_Deployment_ServiceAccountName(t *testing.T) {
635635
t.Parallel()
636636

637637
envTest, mgr, _ := setupController(t)
@@ -646,7 +646,6 @@ func TestReconcile_Integration_WorkloadIdentity_Azure(t *testing.T) {
646646
wg.Done()
647647
}()
648648

649-
// Create an executor that creates a deployment
650649
executor := &spinv1alpha1.SpinAppExecutor{
651650
ObjectMeta: metav1.ObjectMeta{
652651
Name: "executor",
@@ -655,7 +654,7 @@ func TestReconcile_Integration_WorkloadIdentity_Azure(t *testing.T) {
655654
Spec: spinv1alpha1.SpinAppExecutorSpec{
656655
CreateDeployment: true,
657656
DeploymentConfig: &spinv1alpha1.ExecutorDeploymentConfig{
658-
RuntimeClassName: generics.Ptr("a-runtime-class"),
657+
RuntimeClassName: generics.Ptr("foobar"),
659658
},
660659
},
661660
}
@@ -668,34 +667,25 @@ func TestReconcile_Integration_WorkloadIdentity_Azure(t *testing.T) {
668667
Namespace: "default",
669668
},
670669
Spec: spinv1alpha1.SpinAppSpec{
671-
Executor: "executor",
672-
Image: "ghcr.io/radu-matei/perftest:v1",
673-
WorkloadIdentity: &spinv1alpha1.WorkloadIdentity{
674-
ServiceAccountName: "custom-sa",
675-
ProviderMetadata: map[string]string{
676-
"azure": "true",
677-
},
678-
},
670+
Executor: "executor",
671+
Image: "ghcr.io/radu-matei/perftest:v1",
672+
ServiceAccountName: "my-service-account",
679673
},
680674
}
681675

682676
require.NoError(t, envTest.k8sClient.Create(ctx, spinApp))
683677

684-
// Wait for the underlying deployment to exist
685678
var deployment appsv1.Deployment
686679
require.Eventually(t, func() bool {
687680
err := envTest.k8sClient.Get(ctx,
688681
types.NamespacedName{
689682
Namespace: "default",
690-
Name: "app"},
683+
Name: spinApp.Name},
691684
&deployment)
692685
return err == nil
693686
}, 3*time.Second, 100*time.Millisecond)
694687

695-
// Verify Azure workload identity label is set
696-
require.Equal(t, "true", deployment.Spec.Template.ObjectMeta.Labels["azure.workload.identity/use"])
697-
// Verify service account name is set
698-
require.Equal(t, "custom-sa", deployment.Spec.Template.Spec.ServiceAccountName)
688+
require.Equal(t, "my-service-account", deployment.Spec.Template.Spec.ServiceAccountName)
699689

700690
// Terminate the context to force the manager to shut down.
701691
cancelFunc()

internal/webhook/spinapp_validating.go

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,7 @@ func (v *SpinAppValidator) validateSpinApp(ctx context.Context, spinApp *spinv1a
6464
if err := validateAnnotations(spinApp.Spec, executor); err != nil {
6565
allErrs = append(allErrs, err)
6666
}
67-
if err := validateWorkloadIdentity(spinApp.Spec); err != nil {
68-
allErrs = append(allErrs, err)
69-
}
67+
7068
if len(allErrs) == 0 {
7169
return nil
7270
}
@@ -140,17 +138,3 @@ func validateAnnotations(spec spinv1alpha1.SpinAppSpec, executor *spinv1alpha1.S
140138

141139
return nil
142140
}
143-
144-
func validateWorkloadIdentity(spec spinv1alpha1.SpinAppSpec) *field.Error {
145-
if spec.WorkloadIdentity == nil {
146-
return nil
147-
}
148-
149-
if spec.WorkloadIdentity.ServiceAccountName == "" {
150-
return field.Required(
151-
field.NewPath("spec").Child("workloadIdentity").Child("serviceAccountName"),
152-
"serviceAccountName must be provided when workload identity is configured")
153-
}
154-
155-
return nil
156-
}

0 commit comments

Comments
 (0)