Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add podServiceAccountName to SpinApp CRD #228

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions api/v1alpha1/spinapp_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,9 @@ type SpinAppSpec struct {

// Resources defines the resource requirements for this app.
Resources Resources `json:"resources,omitempty"`

// ServiceAccountName for the underlying Pod.
PodServiceAccountName string `json:"podServiceAccountName,omitempty"`
}

// SpinAppStatus defines the observed state of SpinApp
Expand Down
3 changes: 3 additions & 0 deletions config/crd/bases/core.spinoperator.dev_spinapps.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,9 @@ spec:
description: PodAnnotations defines annotations to be applied to the
underlying pods.
type: object
podServiceAccountName:
description: ServiceAccountName for the underlying Pod.
type: string
replicas:
description: Number of replicas to run.
format: int32
Expand Down
15 changes: 15 additions & 0 deletions internal/controller/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,3 +241,18 @@ func registerAndGetScheme() *runtime.Scheme {

return scheme
}

func spinAppWithPodServiceAccountName(name string) *spinv1alpha1.SpinApp {
return &spinv1alpha1.SpinApp{
ObjectMeta: metav1.ObjectMeta{
Name: "my-app",
Namespace: "default",
},
Spec: spinv1alpha1.SpinAppSpec{
Executor: "containerd-shim-spin",
Image: "fakereg.dev/noapp:latest",
Replicas: 1,
PodServiceAccountName: name,
},
}
}
3 changes: 2 additions & 1 deletion internal/controller/spinapp_controller.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any tips how I could implement validation of the serviceAccountName in the controller in order to prevent the SpinApp from being created, if the user provides a non-existing serviceAccountName?

I would add a validation step in constructDeployment that ensures the service account exists before adding the service account name to the deployment spec. If the serviceaccount name was provided but doesn't exists, we should return an error / update the spinapp status.

Original file line number Diff line number Diff line change
Expand Up @@ -414,7 +414,8 @@ func constructDeployment(ctx context.Context, app *spinv1alpha1.SpinApp, config
Annotations: templateAnnotations,
},
Spec: corev1.PodSpec{
RuntimeClassName: &config.RuntimeClassName,
RuntimeClassName: &config.RuntimeClassName,
ServiceAccountName: app.Spec.PodServiceAccountName,
Containers: []corev1.Container{
{
Name: app.Name,
Expand Down
15 changes: 15 additions & 0 deletions internal/controller/spinapp_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -411,3 +411,18 @@ func TestConstructDeployment_MinimalApp(t *testing.T) {
require.Equal(t, app.Spec.Image, deployment.Spec.Template.Spec.Containers[0].Image)
require.Equal(t, ptr("bananarama"), deployment.Spec.Template.Spec.RuntimeClassName)
}

func TestConstructDeployment_AppWithPodServiceAccountName(t *testing.T) {
t.Parallel()

app := spinAppWithPodServiceAccountName("test")

cfg := &spinv1alpha1.ExecutorDeploymentConfig{
RuntimeClassName: "bananarama",
}
deployment, err := constructDeployment(context.Background(), app, cfg, "", nil)
require.NoError(t, err)
require.NotNil(t, deployment)

require.Equal(t, deployment.Spec.Template.Spec.ServiceAccountName, "test")
}
Loading