Skip to content

NO-ISSUE: Enabled readonlyRootFilesystem by default #3614

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

Merged
merged 10 commits into from
Aug 12, 2025
Merged

NO-ISSUE: Enabled readonlyRootFilesystem by default #3614

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
c04a283
Enabled readonlyRootFilesystem by default
dusk125 Jul 24, 2025
cc6fe51
Enable in reconciler as well
dusk125 Jul 24, 2025
b6a8643
Enable in chart templates
dusk125 Jul 30, 2025
0dc6645
Fix typo
dusk125 Aug 4, 2025
a23e1f4
Ensure volumes and volumeMount objects exists
dusk125 Aug 5, 2025
a70ccda
Fixing default rorfs for legacy mode and fix tests
dusk125 Aug 5, 2025
e2dd97b
Only enable rorfs when running as non-root user
dusk125 Aug 7, 2025
660e0bb
Update tests
dusk125 Aug 8, 2025
bc09d7c
Fix errors
dusk125 Aug 11, 2025
f21c444
Update unit tests
dusk125 Aug 12, 2025
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
9 changes: 5 additions & 4 deletions deploy/chart/templates/0000_50_olm_07-olm-operator.deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ spec:
seccompProfile:
type: RuntimeDefault
serviceAccountName: olm-operator-serviceaccount
{{- if or .Values.olm.tlsSecret .Values.olm.clientCASecret }}
volumes:
{{- end }}
{{- if .Values.olm.tlsSecret }}
- name: srv-cert
secret:
Expand All @@ -35,15 +33,16 @@ spec:
secret:
secretName: {{ .Values.olm.clientCASecret }}
{{- end }}
- name: tmpfs
emptyDir: {}
containers:
- name: olm-operator
securityContext:
allowPrivilegeEscalation: false
readOnlyRootFilesystem: true
capabilities:
drop: [ "ALL" ]
{{- if or .Values.olm.tlsSecret .Values.olm.clientCASecret }}
volumeMounts:
{{- end }}
{{- if .Values.olm.tlsSecret }}
- name: srv-cert
mountPath: "/srv-cert"
Expand All @@ -54,6 +53,8 @@ spec:
mountPath: "/profile-collector-cert"
readOnly: true
{{- end }}
- name: tmpfs
mountPath: /tmp
command:
- /bin/olm
args:
Expand Down
9 changes: 5 additions & 4 deletions deploy/chart/templates/0000_50_olm_08-catalog-operator.deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ spec:
seccompProfile:
type: RuntimeDefault
serviceAccountName: olm-operator-serviceaccount
{{- if or .Values.catalog.tlsSecret .Values.catalog.clientCASecret }}
volumes:
{{- end }}
{{- if .Values.catalog.tlsSecret }}
- name: srv-cert
secret:
Expand All @@ -35,15 +33,16 @@ spec:
secret:
secretName: {{ .Values.catalog.clientCASecret }}
{{- end }}
- name: tmpfs
emptyDir: {}
containers:
- name: catalog-operator
securityContext:
allowPrivilegeEscalation: false
readOnlyRootFilesystem: true
capabilities:
drop: [ "ALL" ]
{{- if or .Values.catalog.tlsSecret .Values.catalog.clientCASecret }}
volumeMounts:
{{- end }}
{{- if .Values.catalog.tlsSecret }}
- name: srv-cert
mountPath: "/srv-cert"
Expand All @@ -54,6 +53,8 @@ spec:
mountPath: "/profile-collector-cert"
readOnly: true
{{- end }}
- name: tmpfs
mountPath: /tmp
command:
- /bin/catalog
args:
Expand Down
1 change: 1 addition & 0 deletions deploy/chart/templates/_packageserver.deployment-spec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ spec:
- name: packageserver
securityContext:
allowPrivilegeEscalation: false
readOnlyRootFilesystem: true
capabilities:
drop: [ "ALL" ]
command:
Expand Down
3 changes: 3 additions & 0 deletions pkg/controller/bundle/bundle_unpacker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ func (c *ConfigMapUnpacker) job(cmRef *corev1.ObjectReference, bundlePath string
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand All @@ -180,6 +181,7 @@ func (c *ConfigMapUnpacker) job(cmRef *corev1.ObjectReference, bundlePath string
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand Down Expand Up @@ -209,6 +211,7 @@ func (c *ConfigMapUnpacker) job(cmRef *corev1.ObjectReference, bundlePath string
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand Down
18 changes: 18 additions & 0 deletions pkg/controller/bundle/bundle_unpacker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,6 +308,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand All @@ -334,6 +335,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand Down Expand Up @@ -363,6 +365,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand Down Expand Up @@ -524,6 +527,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand All @@ -550,6 +554,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand Down Expand Up @@ -579,6 +584,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand Down Expand Up @@ -780,6 +786,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand All @@ -806,6 +813,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand Down Expand Up @@ -835,6 +843,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand Down Expand Up @@ -1031,6 +1040,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand All @@ -1057,6 +1067,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand Down Expand Up @@ -1086,6 +1097,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand Down Expand Up @@ -1252,6 +1264,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand All @@ -1278,6 +1291,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand Down Expand Up @@ -1307,6 +1321,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand Down Expand Up @@ -1486,6 +1501,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand All @@ -1512,6 +1528,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand Down Expand Up @@ -1541,6 +1558,7 @@ func TestConfigMapUnpacker(t *testing.T) {
},
SecurityContext: &corev1.SecurityContext{
AllowPrivilegeEscalation: ptr.To(bool(false)),
ReadOnlyRootFilesystem: ptr.To(true),
Capabilities: &corev1.Capabilities{
Drop: []corev1.Capability{"ALL"},
},
Expand Down
27 changes: 17 additions & 10 deletions pkg/controller/registry/reconciler/reconciler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,9 @@ func Pod(source *operatorsv1alpha1.CatalogSource, name, opmImg, utilImage, img s
Args: []string{"/bin/copy-content", fmt.Sprintf("%s/copy-content", utilitiesPath)},
VolumeMounts: []corev1.VolumeMount{utilitiesVolumeMount},
TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError,
SecurityContext: &corev1.SecurityContext{
ReadOnlyRootFilesystem: ptr.To(true),
},
}, corev1.Container{
Name: "extract-content",
Image: img,
Expand All @@ -301,8 +304,12 @@ func Pod(source *operatorsv1alpha1.CatalogSource, name, opmImg, utilImage, img s
Args: extractArgs,
VolumeMounts: []corev1.VolumeMount{utilitiesVolumeMount, contentVolumeMount},
TerminationMessagePolicy: corev1.TerminationMessageFallbackToLogsOnError,
SecurityContext: &corev1.SecurityContext{
ReadOnlyRootFilesystem: ptr.To(true),
},
})

pod.Spec.Containers[0].SecurityContext.ReadOnlyRootFilesystem = ptr.To(true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to confirm, if grpcPodConfig.ExtractContent == nil, then pod.Spec.Containers[0].SecurityContext.ReadOnlyRootFilesystem will be set to false. Is that the expected behavior? Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Correct. If we're using extractContent, then we are pulling out the catalog+cache and running it in a catalogsource using an in-cluster opm version.
If we're not using extractContent, then we're "just running an arbitrary pod", which is expected to speak GRPC on port 50051.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the confirmation! I have one more question—if we don’t use extractContent, the command running in the Pod is /bin/opm serve /configs --cache-dir=/tmp/cache, which requires readOnlyRootFilesystem: false. However, I believe mounting /tmp as a volume should also work if we set readOnlyRootFilesystem: true. So, my question is: why not set readOnlyRootFilesystem: true when extractContent is not used? Or am I missing something? Thanks!

pod.Spec.Containers[0].Image = opmImg
pod.Spec.Containers[0].Command = []string{"/bin/opm"}
pod.Spec.Containers[0].ImagePullPolicy = image.InferImagePullPolicy(opmImg)
Expand Down Expand Up @@ -356,6 +363,16 @@ func Pod(source *operatorsv1alpha1.CatalogSource, name, opmImg, utilImage, img s
}

func addSecurityContext(pod *corev1.Pod, runAsUser int64) {
pod.Spec.SecurityContext = &corev1.PodSecurityContext{
SeccompProfile: &corev1.SeccompProfile{
Type: corev1.SeccompProfileTypeRuntimeDefault,
},
}
if runAsUser > 0 {
pod.Spec.SecurityContext.RunAsUser = &runAsUser
pod.Spec.SecurityContext.RunAsNonRoot = ptr.To(true)
}

for i := range pod.Spec.InitContainers {
if pod.Spec.InitContainers[i].SecurityContext == nil {
pod.Spec.InitContainers[i].SecurityContext = &corev1.SecurityContext{}
Expand All @@ -374,16 +391,6 @@ func addSecurityContext(pod *corev1.Pod, runAsUser int64) {
Drop: []corev1.Capability{"ALL"},
}
}

pod.Spec.SecurityContext = &corev1.PodSecurityContext{
SeccompProfile: &corev1.SeccompProfile{
Type: corev1.SeccompProfileTypeRuntimeDefault,
},
}
if runAsUser > 0 {
pod.Spec.SecurityContext.RunAsUser = &runAsUser
pod.Spec.SecurityContext.RunAsNonRoot = ptr.To(true)
}
}

// getDefaultPodContextConfig returns Restricted if the defaultNamespace has the 'pod-security.kubernetes.io/enforce' label set to 'restricted',
Expand Down
Loading