diff --git a/internal/controller/operatorconfig/controller.go b/internal/controller/operatorconfig/controller.go index c3bb92e5..63207236 100644 --- a/internal/controller/operatorconfig/controller.go +++ b/internal/controller/operatorconfig/controller.go @@ -904,7 +904,7 @@ func (r *OperatorConfigReconciler) deployWorkspaceInfra(ctx context.Context, con // Re-create the SCC in privileged mode scc = r.buildWorkspaceSCCPrivileged() if err := r.createOrUpdate(ctx, scc, config); err != nil { - return fmt.Errorf("failed to create/update workspace SCC (privileged): %w", err) + return fmt.Errorf("failed to create/update workspace SCC (no user namespaces): %w", err) } } diff --git a/internal/controller/workspace/controller.go b/internal/controller/workspace/controller.go index 215971fb..84d0116f 100644 --- a/internal/controller/workspace/controller.go +++ b/internal/controller/workspace/controller.go @@ -230,22 +230,18 @@ func (r *Reconciler) buildPod(ws *automotivev1alpha1.Workspace, operatorConfig * } // Determine if the cluster supports user namespaces. - // With user namespaces: hostUsers=false, drop ALL caps + add specific ones, procMount=Unmasked + // With user namespaces: drop ALL caps + add specific ones, procMount=Unmasked // Without (OCP < 4.19): privileged=true for nested podman/buildah userNamespaces := operatorConfig != nil && operatorConfig.Status.UserNamespacesSupported var secCtx *corev1.SecurityContext if userNamespaces { - caps := []corev1.Capability{"SETUID", "SETGID", "DAC_OVERRIDE", "CHOWN", "FOWNER"} - if image == configuredImage { - caps = append(caps, "SYS_ADMIN") - } secCtx = &corev1.SecurityContext{ AllowPrivilegeEscalation: ptr.To(true), ProcMount: ptr.To(corev1.UnmaskedProcMount), Capabilities: &corev1.Capabilities{ Drop: []corev1.Capability{"ALL"}, - Add: caps, + Add: []corev1.Capability{"SETUID", "SETGID", "SYS_ADMIN", "DAC_OVERRIDE", "CHOWN", "FOWNER"}, }, } } else if image == configuredImage { @@ -321,17 +317,66 @@ func (r *Reconciler) buildPod(ws *automotivev1alpha1.Workspace, operatorConfig * }) } + // Init container: sets up PVC directories, SSH keys, and Jumpstarter config + // using the toolchain image (which has ssh-keygen, etc.). Runs before the main + // container regardless of image, so arbitrary images get a prepared workspace. + initMounts := []corev1.VolumeMount{ + {Name: "workspace", MountPath: "/workspace"}, + } + if ws.Spec.ClientConfigSecretRef != "" { + initMounts = append(initMounts, corev1.VolumeMount{ + Name: "jumpstarter-client", MountPath: "/jumpstarter", ReadOnly: true, + }) + } + initScript := `set -e +mkdir -p /workspace/src /workspace/cache /workspace/.cache /workspace/.ssh \ + /workspace/.config /workspace/.local/share/containers \ + /workspace/.pkg-overlay/{usr,etc,var-lib,opt}-{upper,work} +[ -f /workspace/.ssh/id_ed25519 ] || ssh-keygen -t ed25519 -f /workspace/.ssh/id_ed25519 -N '' -q +if [ -f /jumpstarter/client.yaml ]; then + mkdir -p /workspace/.config/jumpstarter/clients + cp /jumpstarter/client.yaml /workspace/.config/jumpstarter/clients/workspace.yaml + HOME=/workspace jmp config client use workspace || true +fi +chown -R 1000:1000 /workspace/src /workspace/cache /workspace/.cache /workspace/.ssh \ + /workspace/.config /workspace/.local` + + // For the toolchain image, run the entrypoint (handles user creation, overlayfs, + // subuid/subgid setup). For custom images, don't override Command so the + // image's own ENTRYPOINT/CMD runs — the init container already prepared the PVC. + var command []string + var workingDir string + if image == configuredImage { + command = []string{"/usr/local/bin/workspace-entrypoint.sh"} + workingDir = "/workspace" + } + podSpec := corev1.PodSpec{ ServiceAccountName: workspaceServiceAccountName, SecurityContext: &corev1.PodSecurityContext{ RunAsUser: ptr.To[int64](0), }, + InitContainers: []corev1.Container{ + { + Name: "workspace-init", + Image: configuredImage, + Command: []string{"/bin/sh", "-c", initScript}, + VolumeMounts: initMounts, + SecurityContext: &corev1.SecurityContext{ + AllowPrivilegeEscalation: ptr.To(false), + Capabilities: &corev1.Capabilities{ + Drop: []corev1.Capability{"ALL"}, + Add: []corev1.Capability{"CHOWN", "DAC_OVERRIDE", "FOWNER"}, + }, + }, + }, + }, Containers: []corev1.Container{ { Name: containerName, Image: image, - Command: []string{"/usr/local/bin/workspace-entrypoint.sh"}, - WorkingDir: "/workspace", + Command: command, + WorkingDir: workingDir, Env: env, Resources: resourcesOrDefaults(ws.Spec.Resources), SecurityContext: secCtx, diff --git a/internal/controller/workspace/controller_test.go b/internal/controller/workspace/controller_test.go index 1f0bca26..679aa93f 100644 --- a/internal/controller/workspace/controller_test.go +++ b/internal/controller/workspace/controller_test.go @@ -13,6 +13,7 @@ import ( "k8s.io/apimachinery/pkg/types" utilruntime "k8s.io/apimachinery/pkg/util/runtime" clientgoscheme "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" @@ -411,3 +412,141 @@ func TestBuildPod_PreservesWorkspaceConfig(t *testing.T) { t.Error("expected tmpfs-build volume") } } + +func TestBuildPod_SecurityContext(t *testing.T) { + toolchainImage := automotivev1alpha1.DefaultToolchainImage + customImage := "quay.io/example/custom:latest" + + makeWorkspace := func(image string) *automotivev1alpha1.Workspace { + return &automotivev1alpha1.Workspace{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-ws", + Namespace: "default", + }, + Spec: automotivev1alpha1.WorkspaceSpec{ + Architecture: "amd64", + Image: image, + }, + Status: automotivev1alpha1.WorkspaceStatus{ + PVCName: "test-ws-workspace", + }, + } + } + + tests := []struct { + name string + image string + userNamespaces bool + wantPrivileged bool + wantSysAdmin bool + wantProcMount *corev1.ProcMountType + }{ + { + name: "no userns + toolchain image → privileged", + image: "", // defaults to toolchain + userNamespaces: false, + wantPrivileged: true, + wantSysAdmin: false, // privileged implies all caps + }, + { + name: "no userns + custom image → not privileged, no SYS_ADMIN", + image: customImage, + userNamespaces: false, + wantPrivileged: false, + wantSysAdmin: false, + }, + { + name: "userns + toolchain image → not privileged, has SYS_ADMIN", + image: "", // defaults to toolchain + userNamespaces: true, + wantPrivileged: false, + wantSysAdmin: true, + wantProcMount: ptr.To(corev1.UnmaskedProcMount), + }, + { + name: "userns + custom image → not privileged, has SYS_ADMIN (scoped to userns)", + image: customImage, + userNamespaces: true, + wantPrivileged: false, + wantSysAdmin: true, + wantProcMount: ptr.To(corev1.UnmaskedProcMount), + }, + { + name: "userns + explicit toolchain image → not privileged, has SYS_ADMIN", + image: toolchainImage, + userNamespaces: true, + wantPrivileged: false, + wantSysAdmin: true, + wantProcMount: ptr.To(corev1.UnmaskedProcMount), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ws := makeWorkspace(tt.image) + + var operatorConfig *automotivev1alpha1.OperatorConfig + if tt.userNamespaces { + operatorConfig = &automotivev1alpha1.OperatorConfig{ + Status: automotivev1alpha1.OperatorConfigStatus{ + UserNamespacesSupported: true, + }, + } + } + + r := &Reconciler{Scheme: newTestScheme()} + pod := r.buildPod(ws, operatorConfig) + + secCtx := pod.Spec.Containers[0].SecurityContext + if secCtx == nil { + t.Fatal("expected SecurityContext to be set") + } + + // Check Privileged + isPrivileged := secCtx.Privileged != nil && *secCtx.Privileged + if isPrivileged != tt.wantPrivileged { + t.Errorf("Privileged = %v, want %v", isPrivileged, tt.wantPrivileged) + } + + // Check SYS_ADMIN capability + hasSysAdmin := false + if secCtx.Capabilities != nil { + for _, cap := range secCtx.Capabilities.Add { + if cap == "SYS_ADMIN" { + hasSysAdmin = true + } + } + } + if hasSysAdmin != tt.wantSysAdmin { + t.Errorf("SYS_ADMIN capability = %v, want %v", hasSysAdmin, tt.wantSysAdmin) + } + + // Check ProcMount + if tt.wantProcMount != nil { + if secCtx.ProcMount == nil { + t.Errorf("ProcMount = nil, want %v", *tt.wantProcMount) + } else if *secCtx.ProcMount != *tt.wantProcMount { + t.Errorf("ProcMount = %v, want %v", *secCtx.ProcMount, *tt.wantProcMount) + } + } else if secCtx.ProcMount != nil { + t.Errorf("ProcMount = %v, want nil", *secCtx.ProcMount) + } + + // Non-privileged containers must drop ALL caps + if !tt.wantPrivileged { + if secCtx.Capabilities == nil { + t.Fatal("expected Capabilities to be set for non-privileged container") + } + dropsAll := false + for _, cap := range secCtx.Capabilities.Drop { + if cap == "ALL" { + dropsAll = true + } + } + if !dropsAll { + t.Error("non-privileged container must drop ALL capabilities") + } + } + }) + } +}