diff --git a/.golangci.yml b/.golangci.yml index e5b21b0..389b586 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -36,6 +36,9 @@ linters: - dupl - lll path: internal/* + - linters: + - goconst + path: (pkg/|internal/).*/*_test.go paths: - third_party$ - builtin$ diff --git a/api/v1alpha1/marimonotebook_types.go b/api/v1alpha1/marimonotebook_types.go index 3774f02..76b8517 100644 --- a/api/v1alpha1/marimonotebook_types.go +++ b/api/v1alpha1/marimonotebook_types.go @@ -87,6 +87,12 @@ type MarimoNotebookSpec struct { // +optional Image string `json:"image,omitempty"` + // User id for containers in the marimo pod + // +kubebuilder:default:=1000 + // +kubebuilder:validation:Minimum=0 + // +optional + RunAsUser *int64 `json:"runAsUser,omitempty"` + // Port for marimo server // +kubebuilder:default:=2718 // +kubebuilder:validation:Minimum=1 diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 261db1d..0d2acd2 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -108,6 +108,11 @@ func (in *MarimoNotebookList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *MarimoNotebookSpec) DeepCopyInto(out *MarimoNotebookSpec) { *out = *in + if in.RunAsUser != nil { + in, out := &in.RunAsUser, &out.RunAsUser + *out = new(int64) + **out = **in + } if in.Content != nil { in, out := &in.Content, &out.Content *out = new(string) diff --git a/config/crd/bases/marimo.io_marimos.yaml b/config/crd/bases/marimo.io_marimos.yaml index b2627e6..4d81b82 100644 --- a/config/crd/bases/marimo.io_marimos.yaml +++ b/config/crd/bases/marimo.io_marimos.yaml @@ -303,6 +303,12 @@ spec: description: Requests specifies minimum resources required type: object type: object + runAsUser: + default: 1000 + description: User id for containers in the marimo pod + format: int64 + minimum: 0 + type: integer sidecars: description: |- Sidecars are additional containers that run alongside marimo diff --git a/deploy/install.yaml b/deploy/install.yaml index 799168c..2a518b3 100644 --- a/deploy/install.yaml +++ b/deploy/install.yaml @@ -311,6 +311,12 @@ spec: description: Requests specifies minimum resources required type: object type: object + runAsUser: + default: 1000 + description: User id for containers in the marimo pod + format: int64 + minimum: 0 + type: integer sidecars: description: |- Sidecars are additional containers that run alongside marimo diff --git a/pkg/resources/pod.go b/pkg/resources/pod.go index 24e9f41..a8f0b98 100644 --- a/pkg/resources/pod.go +++ b/pkg/resources/pod.go @@ -29,6 +29,12 @@ func BuildPod(notebook *marimov1alpha1.MarimoNotebook) *corev1.Pod { var volumeMounts []corev1.VolumeMount var volumes []corev1.Volume + runAsUser := notebook.Spec.RunAsUser + if runAsUser == nil { + defaultUID := int64(1000) + runAsUser = &defaultUID + } + // Use PVC if storage is configured, otherwise emptyDir if notebook.Spec.Storage != nil { volumes = []corev1.Volume{ @@ -82,6 +88,7 @@ func BuildPod(notebook *marimov1alpha1.MarimoNotebook) *corev1.Pod { {Name: PVCVolumeName, MountPath: NotebookDir}, {Name: ConfigMapVolumeName, MountPath: "/content", ReadOnly: true}, }, + SecurityContext: withRunAsUser(nil, runAsUser), }, } } else if notebook.Spec.Source != "" { @@ -99,6 +106,7 @@ func BuildPod(notebook *marimov1alpha1.MarimoNotebook) *corev1.Pod { VolumeMounts: []corev1.VolumeMount{ {Name: PVCVolumeName, MountPath: NotebookDir}, }, + SecurityContext: withRunAsUser(nil, runAsUser), }, } } @@ -114,6 +122,7 @@ func BuildPod(notebook *marimov1alpha1.MarimoNotebook) *corev1.Pod { VolumeMounts: []corev1.VolumeMount{ {Name: "venv", MountPath: "/opt/venv"}, }, + SecurityContext: withRunAsUser(nil, runAsUser), }) // Add venv volume (emptyDir, shared between init and main container) @@ -201,7 +210,7 @@ func BuildPod(notebook *marimov1alpha1.MarimoNotebook) *corev1.Pod { // Expand mounts to sidecars and merge with explicit sidecars // (do this first so we can check for FUSE sidecars) - allSidecars := expandMounts(notebook.Spec.Mounts) + allSidecars := expandMounts(notebook.Spec.Mounts, runAsUser) allSidecars = append(allSidecars, notebook.Spec.Sidecars...) // Check if any sidecar uses FUSE (privileged) - if so, marimo container needs @@ -261,8 +270,9 @@ func BuildPod(notebook *marimov1alpha1.MarimoNotebook) *corev1.Pod { Protocol: corev1.ProtocolTCP, }, }, - VolumeMounts: marimoVolumeMounts, - Resources: buildResourceRequirements(notebook.Spec.Resources), + VolumeMounts: marimoVolumeMounts, + Resources: buildResourceRequirements(notebook.Spec.Resources), + SecurityContext: withRunAsUser(nil, runAsUser), }, } @@ -426,12 +436,12 @@ func parseCWMountURI(uri string) (bucket, subpath, mountPoint string) { // // Note: sshfs:// and rsync:// mounts are handled by the kubectl-marimo plugin, // not the operator. The plugin adds explicit sidecar specs to the CRD. -func expandMounts(mounts []string) []marimov1alpha1.SidecarSpec { +func expandMounts(mounts []string, runAsUser *int64) []marimov1alpha1.SidecarSpec { var sidecars []marimov1alpha1.SidecarSpec for i, mount := range mounts { if strings.HasPrefix(mount, "cw://") { - if sidecar := buildCWSidecar(mount, i); sidecar != nil { + if sidecar := buildCWSidecar(mount, i, runAsUser); sidecar != nil { sidecars = append(sidecars, *sidecar) } } @@ -449,7 +459,7 @@ const CWCredentialsSecret = "cw-credentials" // URI format: cw://bucket[/path][:mount] // Credentials from cw-credentials secret (auto-created by kubectl-marimo plugin). // Endpoint from S3_ENDPOINT env var (default: https://cwobject.com). -func buildCWSidecar(uri string, index int) *marimov1alpha1.SidecarSpec { +func buildCWSidecar(uri string, index int, runAsUser *int64) *marimov1alpha1.SidecarSpec { bucket, subpath, customMount := parseCWMountURI(uri) if bucket == "" { return nil @@ -509,9 +519,12 @@ func buildCWSidecar(uri string, index int) *marimov1alpha1.SidecarSpec { }, }, // FUSE requires privileged access to /dev/fuse - SecurityContext: &corev1.SecurityContext{ - Privileged: ptrBool(true), - }, + SecurityContext: withRunAsUser( + &corev1.SecurityContext{ + Privileged: ptrBool(true), + }, + runAsUser, + ), } } @@ -520,7 +533,17 @@ func ptrBool(b bool) *bool { return &b } -// ptrString returns a pointer to a string value. -func ptrString(s string) *string { - return &s +// withRunAsUser returns a SecurityContext with RunAsUser set. +// Returns nil if both arguments are nil. +// If existing is non-nil it is shallow-copied so the original is not modified. +func withRunAsUser(existing *corev1.SecurityContext, runAsUser *int64) *corev1.SecurityContext { + if existing == nil && runAsUser == nil { + return nil + } + if existing == nil { + return &corev1.SecurityContext{RunAsUser: runAsUser} + } + sc := *existing + sc.RunAsUser = runAsUser + return &sc } diff --git a/pkg/resources/pod_test.go b/pkg/resources/pod_test.go index 43a851d..aafd95a 100644 --- a/pkg/resources/pod_test.go +++ b/pkg/resources/pod_test.go @@ -20,6 +20,8 @@ const ( testSSHPubkeyName = "ssh-pubkey" ) +func ptrString(s string) *string { return &s } + func TestBuildPod_BasicConfig(t *testing.T) { notebook := &marimov1alpha1.MarimoNotebook{ ObjectMeta: metav1.ObjectMeta{ @@ -1069,13 +1071,71 @@ func TestBuildPod_EnvVarsEmpty(t *testing.T) { } } +func TestBuildPod_RunAsUser(t *testing.T) { + var runAsUser int64 = 1234 + notebook := &marimov1alpha1.MarimoNotebook{ + ObjectMeta: metav1.ObjectMeta{ + Name: testNotebookName, + Namespace: testNamespace, + }, + Spec: marimov1alpha1.MarimoNotebookSpec{ + Image: "ghcr.io/marimo-team/marimo:latest", + Port: 2718, + Source: "https://github.com/marimo-team/marimo.git", + RunAsUser: &runAsUser, + }, + } + + pod := BuildPod(notebook) + + // Check marimo main container has RunAsUser set + var marimoContainer *corev1.Container + for i := range pod.Spec.Containers { + if pod.Spec.Containers[i].Name == testMarimoContainer { + marimoContainer = &pod.Spec.Containers[i] + break + } + } + if marimoContainer == nil { + t.Fatal("marimo container not found") + } + if marimoContainer.SecurityContext == nil || marimoContainer.SecurityContext.RunAsUser == nil { + t.Fatal("marimo container SecurityContext.RunAsUser not set") + } + if *marimoContainer.SecurityContext.RunAsUser != runAsUser { + t.Errorf("marimo container RunAsUser: expected %d, got %d", runAsUser, *marimoContainer.SecurityContext.RunAsUser) + } + + // Check git-clone init container has RunAsUser set + var gitCloneContainer *corev1.Container + for i := range pod.Spec.InitContainers { + if pod.Spec.InitContainers[i].Name == "git-clone" { + gitCloneContainer = &pod.Spec.InitContainers[i] + break + } + } + if gitCloneContainer == nil { + t.Fatal("git-clone init container not found") + } + if gitCloneContainer.SecurityContext == nil || gitCloneContainer.SecurityContext.RunAsUser == nil { + t.Fatal("git-clone container SecurityContext.RunAsUser not set") + } + if *gitCloneContainer.SecurityContext.RunAsUser != runAsUser { + t.Errorf( + "git-clone container RunAsUser: expected %d, got %d", + runAsUser, + *gitCloneContainer.SecurityContext.RunAsUser, + ) + } +} + func TestExpandMounts_SSHFSIgnored(t *testing.T) { // sshfs:// mounts are handled by the plugin, not the operator mounts := []string{ "sshfs:///home/marimo/notebooks", } - sidecars := expandMounts(mounts) + sidecars := expandMounts(mounts, nil) // sshfs:// should be ignored (plugin handles it) if len(sidecars) != 0 { @@ -1089,7 +1149,7 @@ func TestExpandMounts_UnsupportedScheme(t *testing.T) { "nfs://server/path", // Not supported yet } - sidecars := expandMounts(mounts) + sidecars := expandMounts(mounts, nil) // Should return empty - unsupported schemes are ignored if len(sidecars) != 0 { @@ -1103,7 +1163,7 @@ func TestExpandMounts_RsyncIgnored(t *testing.T) { "rsync://./local/data", } - sidecars := expandMounts(mounts) + sidecars := expandMounts(mounts, nil) // rsync:// should be ignored (plugin handles it) if len(sidecars) != 0 { @@ -1119,7 +1179,7 @@ func TestExpandMounts_MixedSchemes(t *testing.T) { "cw://bucket/path3", } - sidecars := expandMounts(mounts) + sidecars := expandMounts(mounts, nil) // Only cw:// should produce sidecar if len(sidecars) != 1 { @@ -1203,7 +1263,7 @@ func TestParseCWMountURI(t *testing.T) { func TestExpandMounts_CW(t *testing.T) { mounts := []string{"cw://mybucket/data"} - sidecars := expandMounts(mounts) + sidecars := expandMounts(mounts, nil) if len(sidecars) != 1 { t.Fatalf("expected 1 sidecar, got %d", len(sidecars)) @@ -1236,7 +1296,7 @@ func TestExpandMounts_CW(t *testing.T) { func TestExpandMounts_CWCustomMountPoint(t *testing.T) { mounts := []string{"cw://mybucket/data:/mnt/s3"} - sidecars := expandMounts(mounts) + sidecars := expandMounts(mounts, nil) if len(sidecars) != 1 { t.Fatalf("expected 1 sidecar, got %d", len(sidecars)) @@ -1255,7 +1315,7 @@ func TestExpandMounts_CWCustomMountPoint(t *testing.T) { func TestExpandMounts_CWBucketOnly(t *testing.T) { mounts := []string{"cw://mybucket"} - sidecars := expandMounts(mounts) + sidecars := expandMounts(mounts, nil) if len(sidecars) != 1 { t.Fatalf("expected 1 sidecar, got %d", len(sidecars)) @@ -1275,7 +1335,7 @@ func TestExpandMounts_CWBucketOnly(t *testing.T) { func TestExpandMounts_CWSecretReference(t *testing.T) { mounts := []string{"cw://mybucket/data"} - sidecars := expandMounts(mounts) + sidecars := expandMounts(mounts, nil) if len(sidecars) != 1 { t.Fatalf("expected 1 sidecar, got %d", len(sidecars))