Skip to content

OCPBUGS-86895: Ensure packageserver pod seccompProfile is always set #1341

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
tmshort wants to merge 1 commit into
base: main
Choose a base branch
from
+51 −0
Open

OCPBUGS-86895: Ensure packageserver pod seccompProfile is always set #1341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 15 additions & 0 deletions pkg/package-server-manager/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,21 @@ func ensureCSVHighAvailability(image string, csv *olmv1alpha1.ClusterServiceVers
modified = true
}

// Ensure pod-level seccompProfile is always set. The openshift-operator-lifecycle-manager
// namespace enforces restricted:latest PodSecurity, which requires seccompProfile. The CSV
// template includes this field, but it may be absent if the cluster CSV was created by an
// older version or if SCC/PSA admission ordering masks the missing field on most clusters.
if deployment.Template.Spec.SecurityContext == nil {
deployment.Template.Spec.SecurityContext = &corev1.PodSecurityContext{}
}
if deployment.Template.Spec.SecurityContext.SeccompProfile == nil ||
deployment.Template.Spec.SecurityContext.SeccompProfile.Type != corev1.SeccompProfileTypeRuntimeDefault {

@hongkailiu hongkailiu Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

(probably because of my limited knowledge on SecurityContext) I do not see why we enforce corev1.SeccompProfileTypeRuntimeDefault.
But it looks like a natural choice out of the 3 candidates.

operator-framework-olm/vendor/k8s.io/api/core/v1/types.go

Lines 5006 to 5014 in 5bebe93

const (
// SeccompProfileTypeUnconfined indicates no seccomp profile is applied (A.K.A. unconfined).
SeccompProfileTypeUnconfined SeccompProfileType = "Unconfined"
// SeccompProfileTypeRuntimeDefault represents the default container runtime seccomp profile.
SeccompProfileTypeRuntimeDefault SeccompProfileType = "RuntimeDefault"
// SeccompProfileTypeLocalhost indicates a profile defined in a file on the node should be used.
// The file's location relative to <kubelet-root-dir>/seccomp.
SeccompProfileTypeLocalhost SeccompProfileType = "Localhost"
)

@tmshort tmshort Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think that's basically the reason; we don't want to rely on Localhost or Unconfined

@hongkailiu hongkailiu Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:resolved. Thanks for the answers.

@vwalek vwalek Aug 6, 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.

The "RuntimeDefault" is usually added when the Object is processed by the controller. By default, any pod running under restricted-v2 should have the seccompProfile set to RuntimeDefault. However, it is better to explicitly define it in the deployment as we always want to have the default value.
Just adding my thoughts about the issue.

@tmshort tmshort Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is the intent to explicitly define the setting.

deployment.Template.Spec.SecurityContext.SeccompProfile = &corev1.SeccompProfile{
Type: corev1.SeccompProfileTypeRuntimeDefault,
}
modified = true
}

if modified {
csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs[0].Spec = *deployment
}
Expand Down
36 changes: 36 additions & 0 deletions pkg/package-server-manager/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,21 @@ func withAffinity(affinity *corev1.Affinity) func(*olmv1alpha1.ClusterServiceVer
csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs[0].Spec.Template.Spec.Affinity = affinity
}
}

func withoutSeccompProfile() testCSVOption {
return func(csv *olmv1alpha1.ClusterServiceVersion) {
podSpec := &csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs[0].Spec.Template.Spec
if podSpec.SecurityContext != nil {
podSpec.SecurityContext.SeccompProfile = nil
}
}
}

func withNilPodSecurityContext() testCSVOption {
return func(csv *olmv1alpha1.ClusterServiceVersion) {
csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs[0].Spec.Template.Spec.SecurityContext = nil
}
}
func withRollingUpdateStrategy(strategy *appsv1.RollingUpdateDeployment) func(*olmv1alpha1.ClusterServiceVersion) {
return func(csv *olmv1alpha1.ClusterServiceVersion) {
csv.Spec.InstallStrategy.StrategySpec.DeploymentSpecs[0].Spec.Strategy.RollingUpdate = strategy
Expand Down Expand Up @@ -255,6 +270,27 @@ func TestEnsureCSV(t *testing.T) {
inputCSV: newTestCSV(withReplicas(singleReplicas), withRollingUpdateStrategy(emptyRollout), withAffinity(&corev1.Affinity{})),
expectedCSV: newTestCSV(withReplicas(singleReplicas), withRollingUpdateStrategy(emptyRollout), withAffinity(&corev1.Affinity{})),
},
{
name: "Modified/HighlyAvailable/MissingSeccompProfile",
want: wanted{true, nil},
highlyAvailable: true,
inputCSV: newTestCSV(withReplicas(defaultReplicas), withRollingUpdateStrategy(defaultRollout), withAffinity(defaultAffinity), withoutSeccompProfile()),
expectedCSV: newTestCSV(withReplicas(defaultReplicas), withRollingUpdateStrategy(defaultRollout), withAffinity(defaultAffinity)),
},
{
name: "Modified/SingleReplica/MissingSeccompProfile",
want: wanted{true, nil},
highlyAvailable: false,
inputCSV: newTestCSV(withReplicas(singleReplicas), withRollingUpdateStrategy(emptyRollout), withAffinity(&corev1.Affinity{}), withoutSeccompProfile()),
expectedCSV: newTestCSV(withReplicas(singleReplicas), withRollingUpdateStrategy(emptyRollout), withAffinity(&corev1.Affinity{})),
},
{
name: "Modified/HighlyAvailable/NilPodSecurityContext",
want: wanted{true, nil},
highlyAvailable: true,
inputCSV: newTestCSV(withReplicas(defaultReplicas), withRollingUpdateStrategy(defaultRollout), withAffinity(defaultAffinity), withNilPodSecurityContext()),
expectedCSV: newTestCSV(withReplicas(defaultReplicas), withRollingUpdateStrategy(defaultRollout), withAffinity(defaultAffinity)),
},

@hongkailiu hongkailiu Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we want to have a case of "Modified/SingleReplica/NilPodSecurityContext"?

My guess is the intension is that HA is more important here and for SingleReplica we do not need as much coverage. A case of "Modified/SingleReplica/MissingSeccompProfile" seems enough.

@tmshort tmshort Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It might be something to consider in the future.

@hongkailiu hongkailiu Aug 6, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

:resolved.

}

for _, tc := range tt {
Expand Down