Skip to content

OCPBUGS-77404: bugfix: mount /etc/containers so image controllers respect cluster-wide image registry configurations #439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
everettraven wants to merge 9 commits into
base: master
Choose a base branch
from
Open

OCPBUGS-77404: bugfix: mount /etc/containers so image controllers respect cluster-wide image registry configurations #439

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a7aa392
bugfix: mount /etc/containers so image controllers respect cluster-wi…
everettraven Jun 8, 2026
ff619ce
bugfix: configure registry authentication
everettraven Jun 8, 2026
b41e22c
assets: add hostaccess use permissions
everettraven Jun 8, 2026
78b69b8
testing: update tests with deployment changes
everettraven Jun 8, 2026
1ed86f9
fixup! remove seccompprofile from openshift-controller-manager deploy…
everettraven Jun 10, 2026
38f570c
fixup! add apigroup to rolebinding
everettraven Jun 10, 2026
1643cc1
fixup! inverted pull secret syncing
everettraven Jun 11, 2026
038c418
fixup! Add PSA privileged labels to ns
everettraven Jun 12, 2026
28a62c9
fixup! dockerconfigjson
everettraven Jun 12, 2026
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
22 changes: 18 additions & 4 deletions bindata/assets/openshift-controller-manager/deploy.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,12 @@ spec:
name: openshift-controller-manager
annotations:
target.workload.openshift.io/management: '{"effect": "PreferredDuringScheduling"}'
openshift.io/required-scc: restricted-v3
openshift.io/required-scc: hostaccess

@coderabbitai coderabbitai Bot Jun 8, 2026

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for existing custom SCC definitions in the repository
rg -n "kind:\s*SecurityContextConstraints" -A 20

Repository: openshift/cluster-openshift-controller-manager-operator

Length of output: 81

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="bindata/assets/openshift-controller-manager/deploy.yaml"

echo "== File size =="
wc -l "$FILE"

echo "== SCC annotation occurrences (context) =="
rg -n "openshift\.io/required-scc|required-scc" "$FILE" -n

echo "== runAsNonRoot occurrences =="
rg -n "runAsNonRoot" "$FILE" || true

echo "== resources.limits occurrences =="
rg -n "resources:|limits:" "$FILE"

echo "== Extract: container securityContext/resources/volumes around likely sections =="
# show around where the reviewer cited lines (roughly); adjust if file differs
sed -n '1,130p' "$FILE" | nl -ba | sed -n '1,130p'
sed -n '90,180p' "$FILE" | nl -ba | sed -n '1,120p'

echo "== hostPath volume /etc/containers mount check =="
rg -n "etc/containers|hostPath|readOnly" "$FILE"

echo "== Anti-affinity / topology-related scheduling constraints =="
rg -n "antiAffinity|requiredDuringSchedulingIgnoredDuringExecution|topologyKey|hostname" "$FILE" || true

Repository: openshift/cluster-openshift-controller-manager-operator

Length of output: 499

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Search for any custom SCC definitions/usage in repo =="
rg -n "SecurityContextConstraints" -S . || true
rg -n "hostaccess|openshift\.io/required-scc" -S . || true
rg -n "SecurityContextConstraint" -S . || true

Repository: openshift/cluster-openshift-controller-manager-operator

Length of output: 19745

🏁 Script executed:

#!/bin/bash
set -euo pipefail

FILE="bindata/assets/openshift-controller-manager/deploy.yaml"

echo "== Extract around SCC (lines 1-60) =="
awk 'NR<=60{print NR ":" $0}' "$FILE"

echo "== Extract around resources (55-95) =="
awk 'NR>=55 && NR<=95{print NR ":" $0}' "$FILE"

echo "== Extract around hostPath/volumes (95-140) =="
awk 'NR>=95 && NR<=140{print NR ":" $0}' "$FILE"

echo "== Extract last lines (141-200) =="
awk 'NR>=141 && NR<=200{print NR ":" $0}' "$FILE"

echo "== Field presence checks =="
for pat in \
  "runAsNonRoot" \
  "allowPrivilegeEscalation" \
  "readOnlyRootFilesystem" \
  "capabilities:" \
  "automountServiceAccountToken" \
  "limits:" \
  "livenessProbe" \
  "readinessProbe" \
  "hostPID" \
  "hostNetwork" \
  "hostIPC" \
  "privileged" \
  "antiAffinity" \
  "topologyKey"; do
  echo "-- $pat"
  rg -n "$pat" "$FILE" || true
done

echo "== Search for custom SCC manifests outside vendor =="
rg -n --glob '!vendor/**' "kind:\s*SecurityContextConstraints" -S . || true

Repository: openshift/cluster-openshift-controller-manager-operator

Length of output: 6288

Major security hardening gaps in controller-manager OpenShift Deployment

        openshift.io/required-scc: hostaccess
  • hostaccess is less restricted than restricted-v3 and doesn’t align with the guideline requiring a restricted/custom-scoped SCC—either justify the need or wire a custom SCC.
  • Missing runAsNonRoot: true at pod/container securityContext (defense-in-depth).
  • Missing resources.limits (only resources.requests are set).
  • automountServiceAccountToken is not explicitly set to false.
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@bindata/assets/openshift-controller-manager/deploy.yaml` at line 36, The
Deployment currently sets openshift.io/required-scc: hostaccess which is too
permissive—either replace it with the restricted-v3 SCC or wire a purpose-built
custom SCC and document why elevated host access is required; additionally, in
the podSpec/container securityContext ensure runAsNonRoot: true is set,
explicitly set automountServiceAccountToken: false at the podSpec level, and add
resources.limits for the controller container to complement existing
resources.requests so limits are enforced.

Source: Coding guidelines

labels:
app: openshift-controller-manager-a
controller-manager: "true"
spec:
hostUsers: false
securityContext:
seccompProfile:
type: RuntimeDefault
priorityClassName: system-node-critical
serviceAccountName: openshift-controller-manager-sa
containers:
Expand Down Expand Up @@ -69,6 +66,8 @@ spec:
valueFrom:
fieldRef:
fieldPath: metadata.name
- name: REGISTRY_AUTH_FILE
value: /var/run/secrets/image-auth/auth.json
livenessProbe:
initialDelaySeconds: 30
httpGet:
Expand All @@ -92,6 +91,11 @@ spec:
name: proxy-ca-bundles
- mountPath: /tmp
name: tmp
- mountPath: /etc/containers
name: etc-containers
readOnly: true
- mountPath: /var/run/secrets/image-auth
name: image-auth
volumes:
- name: config
configMap:
Expand All @@ -110,6 +114,16 @@ spec:
path: tls-ca-bundle.pem
- emptyDir: {}
name: tmp
- hostPath:

@everettraven everettraven Jun 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Open Question: Does this automatically handle pull secrets for private registries as well?

@everettraven everettraven Jun 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It doesn't, but I've opened openshift/openshift-controller-manager#447 to help with this.

path: /etc/containers
type: Directory
name: etc-containers
- name: image-auth
secret:
secretName: pull-secret
items:
- key: .dockerconfigjson
path: auth.json
nodeSelector:
node-role.kubernetes.io/master: ""
tolerations:
Expand Down
3 changes: 3 additions & 0 deletions bindata/assets/openshift-controller-manager/ns.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ metadata:
labels:
openshift.io/cluster-monitoring: "true"
openshift.io/run-level: "" # specify no run-level turns it off on install and upgrades
pod-security.kubernetes.io/enforce: privileged
pod-security.kubernetes.io/audit: privileged
pod-security.kubernetes.io/warn: privileged
15 changes: 15 additions & 0 deletions bindata/assets/openshift-controller-manager/scc-role.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# needed to support host-mounted image registry configurations.
kind: Role
apiVersion: rbac.authorization.k8s.io/v1
metadata:
name: system:openshift-controller-manager:hostaccess-role
namespace: openshift-controller-manager
rules:
- apiGroups:
- security.openshift.io
resources:
- securitycontextconstraints
resourceNames:
- hostaccess
verbs:
- use
14 changes: 14 additions & 0 deletions bindata/assets/openshift-controller-manager/scc-rolebinding.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# needed to support host-mounted image registry configurations.
apiVersion: rbac.authorization.k8s.io/v1
kind: RoleBinding
metadata:
namespace: openshift-controller-manager
name: system:openshift-controller-manager:hostaccess-rolebinding
roleRef:
apiGroup: rbac.authorization.k8s.io
kind: Role
name: system:openshift-controller-manager:hostaccess-role
Comment thread
coderabbitai[bot] marked this conversation as resolved.
subjects:
- kind: ServiceAccount
namespace: openshift-controller-manager
name: openshift-controller-manager-sa
11 changes: 11 additions & 0 deletions pkg/operator/starter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,14 @@ func RunOperator(ctx context.Context, controllerConfig *controllercmd.Controller
controllerConfig.EventRecorder,
)

err = resourceSyncer.SyncSecret(
resourcesynccontroller.ResourceLocation{Namespace: util.TargetNamespace, Name: "pull-secret"},
resourcesynccontroller.ResourceLocation{Namespace: "openshift-config", Name: "pull-secret"},
)
if err != nil {
return fmt.Errorf("configuring global pull-secret syncing: %w", err)
}

if !cache.WaitForCacheSync(ctx.Done(), configInformers.Config().V1().ClusterVersions().Informer().HasSynced) {
klog.Errorf("timed out waiting for configInformers ClusterVersions")
return fmt.Errorf("timed out waiting for configInformers ClusterVersions")
Expand Down Expand Up @@ -243,6 +251,9 @@ func RunOperator(ctx context.Context, controllerConfig *controllercmd.Controller
"assets/openshift-controller-manager/networkpolicy-default-deny.yaml",
"assets/openshift-controller-manager/route-controller-manager-networkpolicy-allow.yaml",
"assets/openshift-controller-manager/route-controller-manager-networkpolicy-default-deny.yaml",

"assets/openshift-controller-manager/scc-role.yaml",
"assets/openshift-controller-manager/scc-rolebinding.yaml",
},
resourceapply.NewKubeClientHolder(kubeClient),
opClient,
Expand Down
55 changes: 38 additions & 17 deletions pkg/operator/sync_openshiftcontrollermanager_v311_00_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import (
)

func TestExpectedConfigMap(t *testing.T) {

objects := []runtime.Object{
&corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "serving-cert", Namespace: "openshift-controller-manager"}},
&corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: "etcd-client", Namespace: "kube-system"}},
Expand All @@ -70,7 +69,8 @@ func TestExpectedConfigMap(t *testing.T) {
LeaderElection: configv1.LeaderElection{
Name: "openshift-master-controllers",
},
Controllers: []string{"*",
Controllers: []string{
"*",
"-openshift.io/build",
"-openshift.io/build-config-change",
"-openshift.io/builder-rolebindings",
Expand Down Expand Up @@ -124,7 +124,6 @@ func TestExpectedConfigMap(t *testing.T) {
}

func TestControllerDisabling(t *testing.T) {

testCases := []struct {
name string
versionLister configlisterv1.ClusterVersionLister
Expand All @@ -145,9 +144,11 @@ func TestControllerDisabling(t *testing.T) {
configv1.ClusterVersionCapabilityImageRegistry,
},
result: map[string][]string{
"controllers": {"*",
"controllers": {
"*",
"-openshift.io/default-rolebindings",
}},
},
},
},
{
name: "BuildCapDisabled",
Expand All @@ -156,13 +157,15 @@ func TestControllerDisabling(t *testing.T) {
},
enabledCapabilities: []v1.ClusterVersionCapability{},
result: map[string][]string{
"controllers": {"*",
"controllers": {
"*",
"-openshift.io/build",
"-openshift.io/build-config-change",
"-openshift.io/builder-rolebindings",
"-openshift.io/builder-serviceaccount",
"-openshift.io/default-rolebindings",
}},
},
},
},
{
name: "DeploymentConfigCapDisabled",
Expand All @@ -171,13 +174,15 @@ func TestControllerDisabling(t *testing.T) {
},
enabledCapabilities: []v1.ClusterVersionCapability{},
result: map[string][]string{
"controllers": {"*",
"controllers": {
"*",
"-openshift.io/default-rolebindings",
"-openshift.io/deployer",
"-openshift.io/deployer-rolebindings",
"-openshift.io/deployer-serviceaccount",
"-openshift.io/deploymentconfig",
}},
},
},
},
{
name: "ImageRegistryCapDisabled",
Expand All @@ -186,11 +191,13 @@ func TestControllerDisabling(t *testing.T) {
},
enabledCapabilities: []v1.ClusterVersionCapability{},
result: map[string][]string{
"controllers": {"*",
"controllers": {
"*",
"-openshift.io/default-rolebindings",
"-openshift.io/image-puller-rolebindings",
"-openshift.io/serviceaccount-pull-secrets",
}},
},
},
},
{
name: "CapabilitiesDisabled",
Expand All @@ -201,7 +208,8 @@ func TestControllerDisabling(t *testing.T) {
},
enabledCapabilities: []v1.ClusterVersionCapability{},
result: map[string][]string{
"controllers": {"*",
"controllers": {
"*",
"-openshift.io/build",
"-openshift.io/build-config-change",
"-openshift.io/builder-rolebindings",
Expand All @@ -213,16 +221,19 @@ func TestControllerDisabling(t *testing.T) {
"-openshift.io/deploymentconfig",
"-openshift.io/image-puller-rolebindings",
"-openshift.io/serviceaccount-pull-secrets",
}},
},
},
},
{
name: "CapabilitiesDisabledButUnknown",
knownCapabilities: []v1.ClusterVersionCapability{},
enabledCapabilities: []v1.ClusterVersionCapability{},
result: map[string][]string{
"controllers": {"*",
"controllers": {
"*",
"-openshift.io/default-rolebindings",
}},
},
},
},
}

Expand Down Expand Up @@ -623,7 +634,6 @@ type conditionTestCase struct {
func testControllerManagerCondition(t *testing.T, conditionType string, testCases []conditionTestCase) {
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {

if len(tc.version) > 0 {
os.Setenv("RELEASE_VERSION", tc.version)
} else {
Expand Down Expand Up @@ -739,6 +749,10 @@ func TestDeploymentWithProxy(t *testing.T) {
},
},
},
{
Name: "REGISTRY_AUTH_FILE",
Value: "/var/run/secrets/image-auth/auth.json",
},
{
Name: "HTTPS_PROXY",
Value: "https://my-proxy",
Expand Down Expand Up @@ -771,6 +785,10 @@ func TestDeploymentWithProxy(t *testing.T) {
Name: "POD_NAME",
Value: "my-pod",
},
{
Name: "REGISTRY_AUTH_FILE",
Value: "/var/run/secrets/image-auth/auth.json",
},
}
}

Expand Down Expand Up @@ -798,6 +816,10 @@ func TestDeploymentWithProxy(t *testing.T) {
Name: "POD_NAME",
Value: "my-pod",
},
{
Name: "REGISTRY_AUTH_FILE",
Value: "/var/run/secrets/image-auth/auth.json",
},
// HTTPS_PROXY is added as it isn't in the template, but it's present in the proxy config.
{
Name: "HTTPS_PROXY",
Expand Down Expand Up @@ -874,7 +896,6 @@ func TestDeploymentWithProxy(t *testing.T) {
proxyLister,
specAnnotations,
)

if err != nil {
t.Fatalf("unexpected error: %s", err.Error())
}
Expand Down