fix(collector): default fsGroup when volumeClaimTemplates are present

[ozzywalsh]

Summary

• Collectors using persistent storage (volumeClaimTemplates) fail with permission denied on PVC mounts when running as a non-root user without fsGroup set in podSecurityContext
• The mutating webhook now defaults podSecurityContext.fsGroup to 65532 when VCTs are present and no fsGroup is configured
• An explicitly set fsGroup is never overwritten

Test plan

• Unit test: TestCollectorDefaultingWebhook/StatefulSet_with_volumeClaimTemplates_gets_default_fsGroup — verifies webhook injects fsGroup: 65532
• Unit test: TestCollectorDefaultingWebhook/StatefulSet_with_volumeClaimTemplates_preserves_explicit_fsGroup — verifies user-set value is not overwritten
• Manual CRC verification: deployed fixed operator to OpenShift CRC, created StatefulSet collector with VCTs + file_storage sending queue + anyuid SCC — no fsGroup in CR. Webhook injected fsGroup: 65532, pod started successfully with no permission errors. Same config without the fix crashes with open /var/otelcol/storage/exporter_otlp_blackhole_traces: permission denied

[swiatekm]

Is there any reasoning behind picking 65532 for the GID?

[iblancasa]

Is there any reasoning behind picking 65532 for the GID?

I think there is a convention for distroless container images to do that for non-root users.

[swiatekm]

If we want to make the pvcs writable by default, then we should probably add an e2e test that verifies this.

[frzifus]

What will happen when I mount a volume to persist my sending queue with the current version fsgroup: 0 and run an operator upgrade and the default fsgroup will change?

Shouldnt the collector in that case have permission issues?

[frzifus]

I wonder if that should be marked as breaking. Depending on the current configuration.

Please add some e2e tests.

[github-actions]

E2E Test Results

 38 files  271 suites   2h 22m 23s ⏱️
115 tests 114 ✅ 1 💤 0 ❌
293 runs  291 ✅ 2 💤 0 ❌

Results for commit bc372e4.

♻️ This comment has been updated with latest results.

[ozzywalsh]

@frzifus @swiatekm
How would we like to e2e test this?

I can for example add an assertion to this e2e test to check the produced StatefulSet has the podSecurityContext.fsGroup set. Although that doesn't offer much over what's already in the webhook test.

A test that actually attempts to write to the container would be nice. But I'm not sure if that's feasible on kind. From my investigation; it seems like kinds storage provisioner doesn't replicate the same permissions one would see in a production environment. See below for example. I'm open to correction if this is wrong.

# Deploy on kind against main branch (no fsGroup fix).
# The initContainer runs as the same user as the collector (65532)
# and tries to write to the PVC. On kind's hostpath provisioner
# (0777 volumes) this will succeed — proving that kind masks the
# real-world bug and an e2e write test is not viable in CI.
#
# On production storage (NFS, Ceph, etc.) with root:root 0755
# volumes, the same manifest would fail without fsGroup.
#
# Usage:
#   kubectl apply -f hack/test-fsgroup-kind.yaml
#   kubectl get pods -n fsgroup-test -w
#
apiVersion: v1
kind: Namespace
metadata:
  name: fsgroup-test
---
apiVersion: opentelemetry.io/v1beta1
kind: OpenTelemetryCollector
metadata:
  name: fsgroup-test
  namespace: fsgroup-test
spec:
  mode: statefulset
  replicas: 1
  initContainers:
    - name: write-test
      image: mirror.gcr.io/library/busybox:latest
      securityContext:
        runAsUser: 65532
        runAsGroup: 65532
      command:
        - sh
        - -c
        - |
          ls -la /data
          echo "testing write as $(id)" &&
          touch /data/write-test &&
          echo "SUCCESS: write to PVC succeeded (kind masks the bug)" &&
          rm -f /data/write-test
      volumeMounts:
        - name: storage
          mountPath: /data
  volumeMounts:
    - name: storage
      mountPath: /data
  volumeClaimTemplates:
    - metadata:
        name: storage
      spec:
        accessModes: ["ReadWriteOnce"]
        resources:
          requests:
            storage: 64Mi
  config:
    receivers:
      otlp:
        protocols:
          grpc:
            endpoint: 0.0.0.0:4317
    exporters:
      debug: {}
    service:
      pipelines:
        traces:
          receivers: [otlp]
          exporters: [debug]

$ kubectl -n fsgroup-test logs statefulsets/fsgroup-test-collector -c write-test 
total 0
drwxrwxrwx    1 root     root             0 May 28 12:41 .
drwxr-xr-x    1 root     root            82 May 28 12:55 ..
testing write as uid=65532 gid=65532 groups=65532
SUCCESS: write to PVC succeeded (kind masks the bug)                                                                                                                                             

[swiatekm]

I'm really not sure we should be doing this. Largely, it should be on the user to ensure their volumes have the right permissions. Maybe there's a point in favor of making this work out of the box for the default collector image we ship, but even then I'd find this default surprising. That it's a breaking change is another argument against.

Thoughts @open-telemetry/operator-approvers ?