Skip to content

fix: conform dialer and volume-uploader pods to restricted pod security profile#3614

Open
Ankitsinghsisodya wants to merge 1 commit intoknative:mainfrom
Ankitsinghsisodya:issue-3517-restricted-pod-security
Open

fix: conform dialer and volume-uploader pods to restricted pod security profile#3614
Ankitsinghsisodya wants to merge 1 commit intoknative:mainfrom
Ankitsinghsisodya:issue-3517-restricted-pod-security

Conversation

@Ankitsinghsisodya
Copy link
Copy Markdown
Contributor

@Ankitsinghsisodya Ankitsinghsisodya commented Apr 16, 2026

Changes

  • defaultPodSecurityContext now sets RunAsNonRoot: true and SeccompProfile: RuntimeDefault unconditionally at pod level, so pods satisfy the restricted PSA profile on any cluster
  • On OpenShift the function previously returned nil, leaving the pod with no security context at all; it now returns a minimal context without RunAsUser/RunAsGroup/FSGroup (which OpenShift SCCs manage) so PSA admission passes
  • defaultSecurityContext restores Privileged: false explicitly for defence-in-depth, removes the version-gated seccomp check (RuntimeDefault is GA since k8s 1.25; this project tracks k8s 1.35), and drops the now-unused client parameter
  • SeccompProfile is set at both pod and container level intentionally — pod-level covers all containers by default; container-level ensures compliance even if the pod-level context is overridden downstream
  • RunAsGroup: 0 is retained on non-OpenShift to preserve Tekton buildpack task compatibility; this does not violate the restricted profile (which checks UID, not GID)
  • Unit tests cover both OpenShift and non-OpenShift paths and include a compliance test asserting all four restricted-profile requirements; end-to-end admission validation against a real restricted namespace is covered by make test-full

/kind bug

Fixes #3517

Release Note

```release-note
Pods created by func during deployment (in-cluster dialer, volume uploader) now
conform to the Kubernetes "restricted" pod security profile. Deployments no
longer fail or produce warnings on clusters or namespaces that enforce
pod-security.kubernetes.io/enforce: restricted.
```

Docs

```docs

```

Copilot AI review requested due to automatic review settings April 16, 2026 21:21
@knative-prow knative-prow bot added kind/bug Bugs size/L 🤖 PR changes 100-499 lines, ignoring generated files. needs-ok-to-test 🤖 Needs an org member to approve testing labels Apr 16, 2026
@knative-prow
Copy link
Copy Markdown

knative-prow bot commented Apr 16, 2026

Hi @Ankitsinghsisodya. Thanks for your PR.

I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@knative-prow
Copy link
Copy Markdown

knative-prow bot commented Apr 16, 2026

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Ankitsinghsisodya
Once this PR has been reviewed and has the lgtm label, please assign dprotaso for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow bot requested a review from jrangelramos April 16, 2026 21:21
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the Kubernetes security context helpers used by the in-cluster dialer and volume-uploader pods so they consistently meet the Kubernetes Pod Security Admission (PSA) restricted profile requirements, including on OpenShift.

Changes:

  • Make defaultPodSecurityContext always set runAsNonRoot=true and seccompProfile=RuntimeDefault, returning a minimal non-nil context on OpenShift.
  • Simplify defaultSecurityContext by always setting seccompProfile=RuntimeDefault and removing the now-unused live server-version/client dependency.
  • Add unit tests for OpenShift vs non-OpenShift behavior and a compliance test targeting restricted-profile-required fields.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
pkg/k8s/security_context.go Adjust pod/container security context defaults to satisfy restricted PSA and remove server-version gating.
pkg/k8s/security_context_test.go Add unit tests for OpenShift/non-OpenShift and restricted-profile compliance assertions.
pkg/k8s/persistent_volumes.go Update volume-uploader pod to call the new defaultSecurityContext() signature.
pkg/k8s/dialer.go Update dialer pod to call the new defaultSecurityContext() signature.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/k8s/security_context_test.go Outdated
…ty profile

The in-cluster dialer and volume-uploader pods lacked the security context
fields required by the Kubernetes "restricted" pod security profile, causing
pod admission failures (or warnings) on namespaces that enforce
pod-security.kubernetes.io/enforce: restricted.

Changes:
- defaultPodSecurityContext now sets RunAsNonRoot and SeccompProfile
  (RuntimeDefault) at pod level unconditionally, covering both OpenShift
  and vanilla Kubernetes clusters.
- On OpenShift the function previously returned nil, leaving the pod without
  any security context. It now returns a minimal context that omits
  RunAsUser/RunAsGroup/FSGroup (which OpenShift SCCs manage) and sets only
  the fields required by the restricted PSA profile.
- defaultSecurityContext no longer gates SeccompProfile on a live server-
  version check; RuntimeDefault has been GA since k8s 1.25 and the project
  tracks k8s client-go v0.35 (k8s 1.35). The now-unnecessary client
  parameter is removed.
- Privileged: false is set explicitly for defence-in-depth.
- SeccompProfile is set at both pod and container level intentionally; the
  pod-level covers all containers by default, the container-level ensures
  compliance even if the pod-level context is overridden downstream.
- RunAsGroup: 0 is retained on non-OpenShift to preserve Tekton buildpack
  task compatibility; this does not violate the restricted profile.
- Unit tests are added for both OpenShift and non-OpenShift paths including
  a compliance test that asserts all four restricted-profile requirements.
  End-to-end admission validation is covered by make test-full.

Fixes knative#3517
@Ankitsinghsisodya Ankitsinghsisodya force-pushed the issue-3517-restricted-pod-security branch from aedd85e to 4c2879a Compare April 16, 2026 21:31
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 16, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 56.37%. Comparing base (07bdeaf) to head (4c2879a).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3614      +/-   ##
==========================================
+ Coverage   56.26%   56.37%   +0.11%     
==========================================
  Files         180      180              
  Lines       20522    20556      +34     
==========================================
+ Hits        11546    11588      +42     
+ Misses       7774     7768       -6     
+ Partials     1202     1200       -2     
Flag Coverage Δ
e2e 36.20% <62.96%> (+<0.01%) ⬆️
e2e go 32.86% <85.00%> (+0.01%) ⬆️
e2e node 28.61% <85.00%> (+0.02%) ⬆️
e2e python 33.21% <85.00%> (+<0.01%) ⬆️
e2e quarkus 28.76% <85.00%> (+0.02%) ⬆️
e2e rust 28.12% <85.00%> (-0.04%) ⬇️
e2e springboot 26.64% <85.00%> (+0.02%) ⬆️
e2e typescript 28.72% <85.00%> (+0.02%) ⬆️
e2e-config-ci 18.08% <0.00%> (+<0.01%) ⬆️
integration 17.50% <85.00%> (+0.03%) ⬆️
unit macos-14 43.56% <90.00%> (+0.25%) ⬆️
unit macos-latest 43.56% <90.00%> (+0.25%) ⬆️
unit ubuntu-24.04-arm 43.73% <92.59%> (+0.22%) ⬆️
unit ubuntu-latest 44.43% <90.00%> (+0.25%) ⬆️
unit windows-latest 43.58% <90.00%> (+0.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

kind/bug Bugs needs-ok-to-test 🤖 Needs an org member to approve testing size/L 🤖 PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dialer pod does not conform to restricted pod security profile

2 participants