Skip to content
Merged
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
2 changes: 1 addition & 1 deletion internal/controller/operatorconfig/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
61 changes: 53 additions & 8 deletions internal/controller/workspace/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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"
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +344 to +352

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Custom images still fail the activity-probe contract.

Once Command is left unset, isWorkspaceActive() on Lines 558-563 still execs /bin/sh -c and assumes ls, grep, ps, and awk exist in the workspace image. Minimal/distroless images will now start, but every auto-pause probe will fail, so they never auto-pause and the controller will keep retrying.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@internal/controller/workspace/controller.go` around lines 344 - 350,
isWorkspaceActive currently relies on execing "/bin/sh -c" with a shell pipeline
(ls | grep | ps | awk) which fails in minimal/distroless custom images; update
isWorkspaceActive to avoid shell pipelines by invoking binaries directly as
command slices and parsing outputs in Go (e.g. run exec command
["ps","-o","pid,cmd"] and parse for the workspace process, and run
["ls","-A","/some/path"] when listing is needed) instead of using "/bin/sh -c";
additionally, treat missing binaries as non-fatal (if exec returns ENOENT,
assume active or use alternative marker) so probes do not falsely fail for
shell-less images — change references inside isWorkspaceActive and any helper
that builds the shell command to use direct command/args and Go-side parsing.


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,
Expand Down
139 changes: 139 additions & 0 deletions internal/controller/workspace/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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")
}
}
})
}
}
Loading