Skip to content

Add default container user, remove hard-coded UID/GID injection#917

Open
wnevis-cmyk wants to merge 4 commits into
cybozu-go:mainfrom
wnevis-cmyk:wnevis/openshift-uidFixes
Open

Add default container user, remove hard-coded UID/GID injection#917
wnevis-cmyk wants to merge 4 commits into
cybozu-go:mainfrom
wnevis-cmyk:wnevis/openshift-uidFixes

Conversation

@wnevis-cmyk

@wnevis-cmyk wnevis-cmyk commented May 22, 2026

Copy link
Copy Markdown

Summary

This change makes MOCO compatible with restrictive Pod Security admission, in particular OpenShift's restricted-v2 SCC, by:

  1. Ensuring every container image MOCO ships declares a non-root USER so the operator does not need to inject UIDs.
  2. Allowing operators to opt out of MOCO's default injection of runAsUser / runAsGroup (on managed containers) and fsGroup (on managed pods) via a new command-line flag, so OpenShift can assign its project-scoped UID / GID / fsGroup ranges.

Restrictive SCCs assign project-scoped UID / GID / fsGroup ranges and reject pods that pin those fields to a fixed value. By default MOCO still injects its historical UID/GID/fsGroup values, so existing installations are unaffected; OpenShift users enable the new flag to let the platform assign these values instead.

Non-rootness is preserved by ensuring every container image MOCO ships already declares a non-root USER in its Dockerfile. The mysql, mysqld_exporter, and moco-agent images already do this; this PR adds the missing USER 10000:10000 to fluent-bit.

Users who need a specific UID/GID/fsGroup can still set it explicitly via PodTemplate.Spec.Containers[*].SecurityContext or PodTemplate.Spec.SecurityContext on the MySQLCluster resource (cf. #777, which made per-container SecurityContext overridable).

Related issues

Changes

  • containers/fluent-bit/Dockerfile — add USER 10000:10000 so the fluent-bit sidecar runs as non-root without operator-injected UIDs.
  • cmd/moco-controller/cmd/root.go — add a new --disable-default-security-context flag (default false).
  • cmd/moco-controller/cmd/run.go — wire the flag into MySQLClusterReconciler.
  • controllers/mysqlcluster_controller.go
    • Add a DisableDefaultSecurityContext field to MySQLClusterReconciler.
    • When the flag is disabled (default), behavior is unchanged: managed containers get RunAsUser/RunAsGroup and managed pods (StatefulSet, backup Job, restore Job) get FSGroup defaulted to constants.ContainerUID/ContainerGID. FSGroupChangePolicy=OnRootMismatch is always retained.
    • When the flag is enabled, MOCO no longer injects RunAsUser/RunAsGroup/FSGroup, leaving those fields for the platform (e.g. OpenShift SCC) to assign.
  • controllers/mysql_container.goupdateContainerWithSecurityContext is now a reconciler method that honors the flag, and a defaultPodSecurityContext() helper centralizes the pod-level defaulting.
  • docs/moco-controller.md — document the new flag.

Out of scope

The original draft of this PR also dropped blockOwnerDeletion from the PVC owner reference to work around the OpenShift OwnerReferencesPermissionEnforcement admission check (#389). Per maintainer feedback, that behavior is preserved here and the underlying issue will be addressed separately (likely via a finalizer-based approach). For now, OpenShift/OKD users can apply the RBAC workaround described in #389 (granting update on mysqlclusters/finalizers to the statefulset-controller service account).

Compatibility

No CRD or API changes. With the flag at its default (false), generated pod and container SecurityContext are identical to before this PR, so existing deployments are unaffected. On OpenShift, setting --disable-default-security-context lets the SCC admit the pod and assign project-scoped UID and fsGroup values; non-rootness is still guaranteed by the images' USER directives.

Helm users can enable the flag via extraArgs: ["--disable-default-security-context"].

Signed-off-by: Will Nevis <wnevis@coreweave.com>
@wnevis-cmyk wnevis-cmyk force-pushed the wnevis/openshift-uidFixes branch from cee24c7 to af98303 Compare May 22, 2026 22:01
Stop defaulting PodSecurityContext.FSGroup to constants.ContainerGID for
the StatefulSet, backup Job, and restore Job. Restrictive admission
controllers (e.g. OpenShift restricted-v2 SCC) assign a project-scoped
fsGroup, and a hard-coded default can prevent admission.

FSGroupChangePolicy=OnRootMismatch is retained.

Signed-off-by: Will Nevis <wnevis@coreweave.com>
OpenShift forbids setting blockOwnerDeletion on an owner reference unless
the caller has permission to set finalizers on the owner kind. The MOCO
controller does not have (and should not require) /finalizers permission
on MySQLCluster from the PVC owner-reference path, so on OpenShift the
PVC creation fails with:

  persistentvolumeclaims is forbidden: cannot set blockOwnerDeletion if
  an ownerReference refers to a resource you can't set finalizers on

The PVC retains the owner reference (with Controller=true) so garbage
collection still cascades when the MySQLCluster is deleted; only the
blockOwnerDeletion flag is dropped.

Refs: cybozu-go#389
Signed-off-by: Will Nevis <wnevis@coreweave.com>
@wnevis-cmyk

Copy link
Copy Markdown
Author

I should mention that I did test this on a local CRC cluster and it appears to be working fine.
@yamatcha or @lrf141, could use a set of eyes on this, but I think it's in pretty good shape.

@lrf141

lrf141 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

@wnevis-cmyk
Thank you for the correction request. We will discuss it within the team.

@yamatcha

yamatcha commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

@wnevis-cmyk
Thank you for the PR, and for the detailed description and test results. We've reviewed the three changes and would like to handle them as follows.

  1. USER instruction on the fluent-bit image
    Looks good — we'll accept this as-is.

  2. Removing the default securityContext (runAsUser/runAsGroup and fsGroup)
    We'd rather not drop these defaults unconditionally, since it changes the behavior of existing installations. Could you instead make it opt-in via a command-line flag on moco-controller (e.g. --disable-default-security-context) that defaults to the current behavior? OpenShift users can then enable it, while existing clusters are unaffected.

  3. Dropping blockOwnerDeletion on the PVC owner reference
    We'd like to preserve the behavior blockOwnerDeletion gives us, but achieve it a different way (e.g. a finalizer-based approach) on our side, so we'd prefer to keep this change out of this PR. We'll handle that fix separately. For now, could you use the workaround described in Cannot set blockOwnerDeletion #389?

Since some of these are breaking changes, we'd like to proceed carefully.

wnevis-cmyk added a commit to wnevis-cmyk/moco that referenced this pull request Jun 8, 2026
Address review feedback on PR cybozu-go#917:
- Gate default runAsUser/runAsGroup and fsGroup injection behind a new
  --disable-default-security-context flag (defaults to existing behavior)
  so existing installs are unaffected and OpenShift users can opt in.
- Restore WithBlockOwnerDeletion(true) on the PVC owner reference; the
  finalizer-based fix for cybozu-go#389 will be handled separately.
- Keep the fluent-bit USER 10000:10000 directive.
Address review feedback on PR cybozu-go#917:
- Gate default runAsUser/runAsGroup and fsGroup injection behind a new
  --disable-default-security-context flag (defaults to existing behavior)
  so existing installs are unaffected and OpenShift users can opt in.
- Restore WithBlockOwnerDeletion(true) on the PVC owner reference; the
  finalizer-based fix for cybozu-go#389 will be handled separately.
- Keep the fluent-bit USER 10000:10000 directive.

Signed-off-by: Will Nevis <wnevis@coreweave.com>
@wnevis-cmyk wnevis-cmyk force-pushed the wnevis/openshift-uidFixes branch from e9ffd28 to 2baf81e Compare June 8, 2026 20:18
@wnevis-cmyk

wnevis-cmyk commented Jun 8, 2026

Copy link
Copy Markdown
Author

@wnevis-cmyk Thank you for the PR, and for the detailed description and test results. We've reviewed the three changes and would like to handle them as follows.

  1. USER instruction on the fluent-bit image
    Looks good — we'll accept this as-is.
  2. Removing the default securityContext (runAsUser/runAsGroup and fsGroup)
    We'd rather not drop these defaults unconditionally, since it changes the behavior of existing installations. Could you instead make it opt-in via a command-line flag on moco-controller (e.g. --disable-default-security-context) that defaults to the current behavior? OpenShift users can then enable it, while existing clusters are unaffected.
  3. Dropping blockOwnerDeletion on the PVC owner reference
    We'd like to preserve the behavior blockOwnerDeletion gives us, but achieve it a different way (e.g. a finalizer-based approach) on our side, so we'd prefer to keep this change out of this PR. We'll handle that fix separately. For now, could you use the workaround described in Cannot set blockOwnerDeletion #389?

Since some of these are breaking changes, we'd like to proceed carefully.

@yamatcha I've gone ahead and implemented that flag-based approach for default security context and removed the blockOwnerDeletion bit. It should be good to go -- thanks for the help on this.

Also, yes, that RBAC role binding for the finalizers is a decent enough workaround in the meantime.

Worth mentioning that I did do another local test on my CRC cluster and it seems to be working with the workaround from #389

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants