fix securityContext for Knative Service Pod (user-container and queue-proxy) - #14363

Merged
knative-prow[bot] merged 3 commits into
knative:mainfrom
KauzClay:ck-add-seccompprofile
Sep 14, 2023
Merged

fix securityContext for Knative Service Pod (user-container and queue-proxy)#14363
knative-prow[bot] merged 3 commits into
knative:mainfrom
KauzClay:ck-add-seccompprofile

Conversation

@KauzClay

@KauzClayKauzClay commented Sep 13, 2023•
edited by dprotaso
Loading

Copy link
Copy Markdown
Contributor

Fixes#14365

Related to:

Proposed Changes

  • Add seccompProfile type to queue proxy security context.
  • Add runAsNonRoot to secure-pod-defaults
  • I believe this PR added much of the security context sections. But when I install a Knative Service on a cluster that enforces Pod Security Standards, and I enabled secure-pod-defaults, I still get errors.
 status:
conditions:
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Created new replica set "tomato-00001-deployment-5db9b9f88d"
reason: NewReplicaSetCreated
status: "True"
type: Progressing
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Deployment does not have minimum availability.
reason: MinimumReplicasUnavailable
status: "False"
type: Available
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: 'admission webhook "pod-security-webhook.kubernetes.io" denied the request:
pods "tomato-00001-deployment-5db9b9f88d-w8gd4" is forbidden: violates PodSecurity
"restricted:latest": runAsNonRoot != true (pod or container "user-container"
must set securityContext.runAsNonRoot=true), seccompProfile (pod or container
"queue-proxy" must set securityContext.seccompProfile.type to "RuntimeDefault"
or "Localhost")'
reason: FailedCreate
status: "True"
type: ReplicaFailure

Release Note

Fix secure 'secure-pod-defaults' to work with restricted namespaces

@knative-prow

knative-prowBot commented Sep 13, 2023

Copy link
Copy Markdown

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@knative-prowknative-prowBot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 13, 2023
@knative-prowknative-prowBot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. area/API API objects and controllers labels Sep 13, 2023
@KauzClayKauzClay changed the title fix security Context for Knative Service Pod (user-container and queue-proxy)fix securityContext for Knative Service Pod (user-container and queue-proxy)Sep 13, 2023
@KauzClayKauzClay mentioned this pull request Sep 13, 2023
Closed
@dprotaso

dprotaso commented Sep 13, 2023

Copy link
Copy Markdown
Member

To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?

@knative-prowknative-prowBot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 13, 2023
@KauzClay

KauzClay commented Sep 13, 2023

Copy link
Copy Markdown
ContributorAuthor

To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?

yeah, that is what I'm seeing. This issue (#14365) has an example of the errors I see on my deployment

@codecov

codecovBot commented Sep 13, 2023•
edited
Loading

Copy link
Copy Markdown

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02% 🎉

Comparison is base (4dddf9f) 86.12% compared to head (2cf6abf) 86.15%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #14363 +/- ##
==========================================
+ Coverage 86.12% 86.15% +0.02% 
==========================================
Files 196 196 Lines 14787 14790 +3 ==========================================
+ Hits 12735 12742 +7 + Misses 1744 1743 -1 + Partials 308 305 -3
Files ChangedCoverage Δ
pkg/reconciler/revision/resources/queue.go98.42% <ø> (ø)
pkg/apis/serving/v1/revision_defaults.go97.48% <100.00%> (+0.04%)⬆️

... and 5 files with indirect coverage changes

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

@KauzClay
KauzClay marked this pull request as ready for review September 13, 2023 21:10
@knative-prowknative-prowBot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Sep 13, 2023
@dprotaso

dprotaso commented Sep 14, 2023

Copy link
Copy Markdown
Member

/lgtm
/approve
/cherry-pick release-1.11

@knative-prow-robot

knative-prow-robot commented Sep 14, 2023

Copy link
Copy Markdown
Contributor

@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.11 in a new PR and assign it to you.

Details

In response to this:

/lgtm
/approve
/cherry-pick release-1.11

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/test-infra repository.

@knative-prowknative-prowBot added the lgtm Indicates that a PR is ready to be merged. label Sep 14, 2023
@knative-prow

knative-prowBot commented Sep 14, 2023

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dprotaso, KauzClay

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

The pull request process is described 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-prowknative-prowBot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 14, 2023
@dprotaso

dprotaso commented Sep 14, 2023

Copy link
Copy Markdown
Member

/cherry-pick release-1.10

@knative-prow-robot

knative-prow-robot commented Sep 14, 2023

Copy link
Copy Markdown
Contributor

@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.10 in a new PR and assign it to you.

Details

In response to this:

/cherry-pick release-1.10

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/test-infra repository.

@knative-prow
knative-prowBot merged commit 2a732e5 into knative:mainSep 14, 2023
@knative-prow-robotknative-prow-robot mentioned this pull request Sep 14, 2023
Merged
@knative-prow-robot

knative-prow-robot commented Sep 14, 2023

Copy link
Copy Markdown
Contributor

@dprotaso: new pull request created: #14377

Details

In response to this:

/cherry-pick release-1.10

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/test-infra repository.

@knative-prow-robot

knative-prow-robot commented Sep 14, 2023

Copy link
Copy Markdown
Contributor

@dprotaso: new pull request created: #14378

Details

In response to this:

/lgtm
/approve
/cherry-pick release-1.11

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/test-infra repository.

@knative-prow-robotknative-prow-robot mentioned this pull request Sep 14, 2023
Merged
@BobyMCbobs

BobyMCbobs commented Sep 15, 2023

Copy link
Copy Markdown
Contributor

Woohoo! This is a good change

@dprotaso

dprotaso commented Sep 15, 2023

Copy link
Copy Markdown
Member

Surprisingly it means no one has used the secure-pod-defaults feature on a restricted profile :/

krsna-m pushed a commit to krsna-m/serving that referenced this pull request Nov 6, 2023
@KauzClay@krsna-m
fix securityContext for Knative Service Pod (user-container and queue…
0b65385
…-proxy) (knative#14363)
* add seccompProfile to queue container security context
* run as non root by default
* update tests to expect new default run as nonroot
wmfgerrit pushed a commit to wikimedia/operations-docker-images-production-images that referenced this pull request Feb 27, 2025
@elukey
knative-serving: backportknative/serving#14363
a6725fc
The pull request is needed to force Knative to set runAsNonRoot
when secure-pod-defaults is true. The option forces knative to
create the user pods with all the safest security options to pass
the PSS restriction policy, but at the moment it is lacking
runAsNonRoot forced to true.
Note: I haven't backported the whole patch since part of it overlapped
with a previous one for queue.go, where seccomp's defaults were
set. The same code was committed in different pull requests, so I just
removed the bit already there to allow patch to avoid error/warnings.
Bug: T369493
Change-Id: Iceb1ac2d83f298ef2a834e24a8fdc8a6f1df4a28
wmfgerrit pushed a commit to wikimedia/operations-docker-images-production-images that referenced this pull request Feb 27, 2025
@elukeyGerrit Code Review
Merge "knative-serving: backportknative/serving#14363"
9c8d863
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kauanakauanaAwaiting requested review from kauana
@krsna-mkrsna-mAwaiting requested review from krsna-m
@jsanin-vmwjsanin-vmwAwaiting requested review from jsanin-vmw
@nak3nak3Awaiting requested review from nak3

Assignees

@dprotasodprotaso

Labels

approvedIndicates a PR has been approved by an approver from all required OWNERS files.area/APIAPI objects and controllerslgtmIndicates that a PR is ready to be merged.size/SDenotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

secure-pod-defaults does not work when enforcing restricted Pod Security Standards

4 participants

@KauzClay@dprotaso@knative-prow-robot@BobyMCbobs
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
fix securityContext for Knative Service Pod (user-container and queue-proxy) - #14363
Merged
knative-prow[bot] merged 3 commits into
knative:mainfrom 
KauzClay:ck-add-seccompprofile
Sep 14, 2023
Merged
fix securityContext for Knative Service Pod (user-container and queue-proxy)#14363
knative-prow[bot] merged 3 commits into
knative:mainfrom 
KauzClay:ck-add-seccompprofile
Conversation
@KauzClay
@KauzClayKauzClay
commented
Sep 13, 2023•

edited by dprotaso

Loading

Copy link


Copy Markdown

Contributor
Fixes#14365
Related to:
Proposed Changes
  • Add seccompProfile type to queue proxy security context.
  • Add runAsNonRoot to secure-pod-defaults
  • I believe this PR added much of the security context sections. But when I install a Knative Service on a cluster that enforces Pod Security Standards, and I enabled secure-pod-defaults, I still get errors.
 status:
conditions:
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Created new replica set "tomato-00001-deployment-5db9b9f88d"
reason: NewReplicaSetCreated
status: "True"
type: Progressing
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Deployment does not have minimum availability.
reason: MinimumReplicasUnavailable
status: "False"
type: Available
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: 'admission webhook "pod-security-webhook.kubernetes.io" denied the request:
pods "tomato-00001-deployment-5db9b9f88d-w8gd4" is forbidden: violates PodSecurity
"restricted:latest": runAsNonRoot != true (pod or container "user-container"
must set securityContext.runAsNonRoot=true), seccompProfile (pod or container
"queue-proxy" must set securityContext.seccompProfile.type to "RuntimeDefault"
or "Localhost")'
reason: FailedCreate
status: "True"
type: ReplicaFailure

Release Note
Fix secure 'secure-pod-defaults' to work with restricted namespaces

@knative-prow
knative-prowBot
commented
Sep 13, 2023

Copy link


Copy Markdown

Skipping CI for Draft Pull Request.

If you want CI signal for your change, please convert it to an actual PR.

You can still manually trigger a test run with /test all
@knative-prowknative-prowBot
added
the 
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Sep 13, 2023
@knative-prowknative-prowBot
added

size/XS
Denotes a PR that changes 0-9 lines, ignoring generated files.
area/API
API objects and controllers
labels
Sep 13, 2023
@KauzClayKauzClay
changed the title
fix security Context for Knative Service Pod (user-container and queue-proxy)fix securityContext for Knative Service Pod (user-container and queue-proxy)Sep 13, 2023
@KauzClayKauzClay
mentioned this pull request
Sep 13, 2023
 Closed

@dprotaso
dprotaso
commented
Sep 13, 2023

Copy link


Copy Markdown

Member
To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?
@knative-prowknative-prowBot
added

size/S
Denotes a PR that changes 10-29 lines, ignoring generated files.
and removed

size/XS
Denotes a PR that changes 0-9 lines, ignoring generated files.
labels
Sep 13, 2023
@KauzClay
KauzClay
commented
Sep 13, 2023

Copy link


Copy Markdown

ContributorAuthor
To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?
yeah, that is what I'm seeing. This issue (#14365) has an example of the errors I see on my deployment
@codecov
codecovBot
commented
Sep 13, 2023•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Patch coverage: 100.00% and project coverage change: +0.02% 🎉
Comparison is base (4dddf9f) 86.12% compared to head (2cf6abf) 86.15%.

Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@## main #14363 +/- ##
==========================================
+ Coverage 86.12% 86.15% +0.02% 
==========================================
Files 196 196 Lines 14787 14790 +3 ==========================================
+ Hits 12735 12742 +7 + Misses 1744 1743 -1 + Partials 308 305 -3 
Files ChangedCoverage Δ
pkg/reconciler/revision/resources/queue.go98.42% <ø> (ø)
pkg/apis/serving/v1/revision_defaults.go97.48% <100.00%> (+0.04%)⬆️
... and 5 files with indirect coverage changes
☔ View full report in Codecov by Sentry.

📢 Have feedback on the report? Share it here.
@KauzClay
KauzClay
marked this pull request as ready for review
September 13, 2023 21:10
@knative-prowknative-prowBot
removed
the 
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Sep 13, 2023
@dprotaso
dprotaso
commented
Sep 14, 2023

Copy link


Copy Markdown

Member
/lgtm

/approve

/cherry-pick release-1.11
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.11 in a new PR and assign it to you.
Details
In response to this:
/lgtm

/approve

/cherry-pick release-1.11
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/test-infra repository.
@knative-prowknative-prowBot
added
the 
lgtm
Indicates that a PR is ready to be merged.
label
Sep 14, 2023
@knative-prow
knative-prowBot
commented
Sep 14, 2023

Copy link


Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: dprotaso, KauzClay
The full list of commands accepted by this bot can be found here.
The pull request process is described 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-prowknative-prowBot
added
the 
approved
Indicates a PR has been approved by an approver from all required OWNERS files.
label
Sep 14, 2023
@dprotaso
dprotaso
commented
Sep 14, 2023

Copy link


Copy Markdown

Member
/cherry-pick release-1.10
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.10 in a new PR and assign it to you.
Details
In response to this:
/cherry-pick release-1.10
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/test-infra repository.
@knative-prow
knative-prowBot
merged commit 2a732e5
into
knative:mainSep 14, 2023
@knative-prow-robotknative-prow-robot
mentioned this pull request
Sep 14, 2023
 Merged

@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: new pull request created: #14377
Details
In response to this:
/cherry-pick release-1.10
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/test-infra repository.
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: new pull request created: #14378
Details
In response to this:
/lgtm

/approve

/cherry-pick release-1.11
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/test-infra repository.
@knative-prow-robotknative-prow-robot
mentioned this pull request
Sep 14, 2023
 Merged

@BobyMCbobs
BobyMCbobs
commented
Sep 15, 2023

Copy link


Copy Markdown

Contributor
Woohoo! This is a good change
@dprotaso
dprotaso
commented
Sep 15, 2023

Copy link


Copy Markdown

Member
Surprisingly it means no one has used the secure-pod-defaults feature on a restricted profile :/
krsna-m pushed a commit
to krsna-m/serving
that referenced
this pull request
Nov 6, 2023
@KauzClay@krsna-m
fix securityContext for Knative Service Pod (user-container and queue…
0b65385
…-proxy) (knative#14363)
* add seccompProfile to queue container security context
* run as non root by default
* update tests to expect new default run as nonroot
wmfgerrit pushed a commit
to wikimedia/operations-docker-images-production-images
that referenced
this pull request
Feb 27, 2025
@elukey
knative-serving: backportknative/serving#14363
a6725fc
The pull request is needed to force Knative to set runAsNonRoot
when secure-pod-defaults is true. The option forces knative to
create the user pods with all the safest security options to pass
the PSS restriction policy, but at the moment it is lacking
runAsNonRoot forced to true.
Note: I haven't backported the whole patch since part of it overlapped
with a previous one for queue.go, where seccomp's defaults were
set. The same code was committed in different pull requests, so I just
removed the bit already there to allow patch to avoid error/warnings.
Bug: T369493
Change-Id: Iceb1ac2d83f298ef2a834e24a8fdc8a6f1df4a28
wmfgerrit pushed a commit
to wikimedia/operations-docker-images-production-images
that referenced
this pull request
Feb 27, 2025
@elukeyGerrit Code Review
Merge "knative-serving: backportknative/serving#14363"
9c8d863
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@kauanakauanaAwaiting requested review from kauana
@krsna-mkrsna-mAwaiting requested review from krsna-m
@jsanin-vmwjsanin-vmwAwaiting requested review from jsanin-vmw
@nak3nak3Awaiting requested review from nak3

Assignees

@dprotasodprotaso

Labels

approvedIndicates a PR has been approved by an approver from all required OWNERS files.area/APIAPI objects and controllerslgtmIndicates that a PR is ready to be merged.size/SDenotes a PR that changes 10-29 lines, ignoring generated files.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.
secure-pod-defaults does not work when enforcing restricted Pod Security Standards


4 participants

@KauzClay@dprotaso@knative-prow-robot@BobyMCbobs

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix securityContext for Knative Service Pod (user-container and queue-proxy) - #14363
Merged
knative-prow[bot] merged 3 commits into
knative:mainfrom 
KauzClay:ck-add-seccompprofile
Sep 14, 2023
Merged
fix securityContext for Knative Service Pod (user-container and queue-proxy)#14363
knative-prow[bot] merged 3 commits into
knative:mainfrom 
KauzClay:ck-add-seccompprofile
Conversation
@KauzClay
@KauzClayKauzClay
commented
Sep 13, 2023•

edited by dprotaso

Loading

Copy link


Copy Markdown

Contributor
Fixes#14365
Related to:
Proposed Changes
  • Add seccompProfile type to queue proxy security context.
  • Add runAsNonRoot to secure-pod-defaults
  • I believe this PR added much of the security context sections. But when I install a Knative Service on a cluster that enforces Pod Security Standards, and I enabled secure-pod-defaults, I still get errors.
 status:
conditions:
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Created new replica set "tomato-00001-deployment-5db9b9f88d"
reason: NewReplicaSetCreated
status: "True"
type: Progressing
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Deployment does not have minimum availability.
reason: MinimumReplicasUnavailable
status: "False"
type: Available
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: 'admission webhook "pod-security-webhook.kubernetes.io" denied the request:
pods "tomato-00001-deployment-5db9b9f88d-w8gd4" is forbidden: violates PodSecurity
"restricted:latest": runAsNonRoot != true (pod or container "user-container"
must set securityContext.runAsNonRoot=true), seccompProfile (pod or container
"queue-proxy" must set securityContext.seccompProfile.type to "RuntimeDefault"
or "Localhost")'
reason: FailedCreate
status: "True"
type: ReplicaFailure

Release Note
Fix secure 'secure-pod-defaults' to work with restricted namespaces

@knative-prow
knative-prowBot
commented
Sep 13, 2023

Copy link


Copy Markdown

Skipping CI for Draft Pull Request.

If you want CI signal for your change, please convert it to an actual PR.

You can still manually trigger a test run with /test all
@knative-prowknative-prowBot
added
the 
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Sep 13, 2023
@knative-prowknative-prowBot
added

size/XS
Denotes a PR that changes 0-9 lines, ignoring generated files.
area/API
API objects and controllers
labels
Sep 13, 2023
@KauzClayKauzClay
changed the title
fix security Context for Knative Service Pod (user-container and queue-proxy)fix securityContext for Knative Service Pod (user-container and queue-proxy)Sep 13, 2023
@KauzClayKauzClay
mentioned this pull request
Sep 13, 2023
 Closed

@dprotaso
dprotaso
commented
Sep 13, 2023

Copy link


Copy Markdown

Member
To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?
@knative-prowknative-prowBot
added

size/S
Denotes a PR that changes 10-29 lines, ignoring generated files.
and removed

size/XS
Denotes a PR that changes 0-9 lines, ignoring generated files.
labels
Sep 13, 2023
@KauzClay
KauzClay
commented
Sep 13, 2023

Copy link


Copy Markdown

ContributorAuthor
To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?
yeah, that is what I'm seeing. This issue (#14365) has an example of the errors I see on my deployment
@codecov
codecovBot
commented
Sep 13, 2023•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Patch coverage: 100.00% and project coverage change: +0.02% 🎉
Comparison is base (4dddf9f) 86.12% compared to head (2cf6abf) 86.15%.

Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@## main #14363 +/- ##
==========================================
+ Coverage 86.12% 86.15% +0.02% 
==========================================
Files 196 196 Lines 14787 14790 +3 ==========================================
+ Hits 12735 12742 +7 + Misses 1744 1743 -1 + Partials 308 305 -3 
Files ChangedCoverage Δ
pkg/reconciler/revision/resources/queue.go98.42% <ø> (ø)
pkg/apis/serving/v1/revision_defaults.go97.48% <100.00%> (+0.04%)⬆️
... and 5 files with indirect coverage changes
☔ View full report in Codecov by Sentry.

📢 Have feedback on the report? Share it here.
@KauzClay
KauzClay
marked this pull request as ready for review
September 13, 2023 21:10
@knative-prowknative-prowBot
removed
the 
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Sep 13, 2023
@dprotaso
dprotaso
commented
Sep 14, 2023

Copy link


Copy Markdown

Member
/lgtm

/approve

/cherry-pick release-1.11
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.11 in a new PR and assign it to you.
Details
In response to this:
/lgtm

/approve

/cherry-pick release-1.11
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/test-infra repository.
@knative-prowknative-prowBot
added
the 
lgtm
Indicates that a PR is ready to be merged.
label
Sep 14, 2023
@knative-prow
knative-prowBot
commented
Sep 14, 2023

Copy link


Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: dprotaso, KauzClay
The full list of commands accepted by this bot can be found here.
The pull request process is described 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-prowknative-prowBot
added
the 
approved
Indicates a PR has been approved by an approver from all required OWNERS files.
label
Sep 14, 2023
@dprotaso
dprotaso
commented
Sep 14, 2023

Copy link


Copy Markdown

Member
/cherry-pick release-1.10
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.10 in a new PR and assign it to you.
Details
In response to this:
/cherry-pick release-1.10
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/test-infra repository.
@knative-prow
knative-prowBot
merged commit 2a732e5
into
knative:mainSep 14, 2023
@knative-prow-robotknative-prow-robot
mentioned this pull request
Sep 14, 2023
 Merged

@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: new pull request created: #14377
Details
In response to this:
/cherry-pick release-1.10
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/test-infra repository.
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: new pull request created: #14378
Details
In response to this:
/lgtm

/approve

/cherry-pick release-1.11
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/test-infra repository.
@knative-prow-robotknative-prow-robot
mentioned this pull request
Sep 14, 2023
 Merged

@BobyMCbobs
BobyMCbobs
commented
Sep 15, 2023

Copy link


Copy Markdown

Contributor
Woohoo! This is a good change
@dprotaso
dprotaso
commented
Sep 15, 2023

Copy link


Copy Markdown

Member
Surprisingly it means no one has used the secure-pod-defaults feature on a restricted profile :/
krsna-m pushed a commit
to krsna-m/serving
that referenced
this pull request
Nov 6, 2023
@KauzClay@krsna-m
fix securityContext for Knative Service Pod (user-container and queue…
0b65385
…-proxy) (knative#14363)
* add seccompProfile to queue container security context
* run as non root by default
* update tests to expect new default run as nonroot
wmfgerrit pushed a commit
to wikimedia/operations-docker-images-production-images
that referenced
this pull request
Feb 27, 2025
@elukey
knative-serving: backportknative/serving#14363
a6725fc
The pull request is needed to force Knative to set runAsNonRoot
when secure-pod-defaults is true. The option forces knative to
create the user pods with all the safest security options to pass
the PSS restriction policy, but at the moment it is lacking
runAsNonRoot forced to true.
Note: I haven't backported the whole patch since part of it overlapped
with a previous one for queue.go, where seccomp's defaults were
set. The same code was committed in different pull requests, so I just
removed the bit already there to allow patch to avoid error/warnings.
Bug: T369493
Change-Id: Iceb1ac2d83f298ef2a834e24a8fdc8a6f1df4a28
wmfgerrit pushed a commit
to wikimedia/operations-docker-images-production-images
that referenced
this pull request
Feb 27, 2025
@elukeyGerrit Code Review
Merge "knative-serving: backportknative/serving#14363"
9c8d863
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@kauanakauanaAwaiting requested review from kauana
@krsna-mkrsna-mAwaiting requested review from krsna-m
@jsanin-vmwjsanin-vmwAwaiting requested review from jsanin-vmw
@nak3nak3Awaiting requested review from nak3

Assignees

@dprotasodprotaso

Labels

approvedIndicates a PR has been approved by an approver from all required OWNERS files.area/APIAPI objects and controllerslgtmIndicates that a PR is ready to be merged.size/SDenotes a PR that changes 10-29 lines, ignoring generated files.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.
secure-pod-defaults does not work when enforcing restricted Pod Security Standards


4 participants

@KauzClay@dprotaso@knative-prow-robot@BobyMCbobs

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix securityContext for Knative Service Pod (user-container and queue-proxy) - #14363
Merged
knative-prow[bot] merged 3 commits into
knative:mainfrom 
KauzClay:ck-add-seccompprofile
Sep 14, 2023
Merged
fix securityContext for Knative Service Pod (user-container and queue-proxy)#14363
knative-prow[bot] merged 3 commits into
knative:mainfrom 
KauzClay:ck-add-seccompprofile
Conversation
@KauzClay
@KauzClayKauzClay
commented
Sep 13, 2023•

edited by dprotaso

Loading

Copy link


Copy Markdown

Contributor
Fixes#14365
Related to:
Proposed Changes
  • Add seccompProfile type to queue proxy security context.
  • Add runAsNonRoot to secure-pod-defaults
  • I believe this PR added much of the security context sections. But when I install a Knative Service on a cluster that enforces Pod Security Standards, and I enabled secure-pod-defaults, I still get errors.
 status:
conditions:
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Created new replica set "tomato-00001-deployment-5db9b9f88d"
reason: NewReplicaSetCreated
status: "True"
type: Progressing
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Deployment does not have minimum availability.
reason: MinimumReplicasUnavailable
status: "False"
type: Available
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: 'admission webhook "pod-security-webhook.kubernetes.io" denied the request:
pods "tomato-00001-deployment-5db9b9f88d-w8gd4" is forbidden: violates PodSecurity
"restricted:latest": runAsNonRoot != true (pod or container "user-container"
must set securityContext.runAsNonRoot=true), seccompProfile (pod or container
"queue-proxy" must set securityContext.seccompProfile.type to "RuntimeDefault"
or "Localhost")'
reason: FailedCreate
status: "True"
type: ReplicaFailure

Release Note
Fix secure 'secure-pod-defaults' to work with restricted namespaces

@knative-prow
knative-prowBot
commented
Sep 13, 2023

Copy link


Copy Markdown

Skipping CI for Draft Pull Request.

If you want CI signal for your change, please convert it to an actual PR.

You can still manually trigger a test run with /test all
@knative-prowknative-prowBot
added
the 
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Sep 13, 2023
@knative-prowknative-prowBot
added

size/XS
Denotes a PR that changes 0-9 lines, ignoring generated files.
area/API
API objects and controllers
labels
Sep 13, 2023
@KauzClayKauzClay
changed the title
fix security Context for Knative Service Pod (user-container and queue-proxy)fix securityContext for Knative Service Pod (user-container and queue-proxy)Sep 13, 2023
@KauzClayKauzClay
mentioned this pull request
Sep 13, 2023
 Closed

@dprotaso
dprotaso
commented
Sep 13, 2023

Copy link


Copy Markdown

Member
To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?
@knative-prowknative-prowBot
added

size/S
Denotes a PR that changes 10-29 lines, ignoring generated files.
and removed

size/XS
Denotes a PR that changes 0-9 lines, ignoring generated files.
labels
Sep 13, 2023
@KauzClay
KauzClay
commented
Sep 13, 2023

Copy link


Copy Markdown

ContributorAuthor
To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?
yeah, that is what I'm seeing. This issue (#14365) has an example of the errors I see on my deployment
@codecov
codecovBot
commented
Sep 13, 2023•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Patch coverage: 100.00% and project coverage change: +0.02% 🎉
Comparison is base (4dddf9f) 86.12% compared to head (2cf6abf) 86.15%.

Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@## main #14363 +/- ##
==========================================
+ Coverage 86.12% 86.15% +0.02% 
==========================================
Files 196 196 Lines 14787 14790 +3 ==========================================
+ Hits 12735 12742 +7 + Misses 1744 1743 -1 + Partials 308 305 -3 
Files ChangedCoverage Δ
pkg/reconciler/revision/resources/queue.go98.42% <ø> (ø)
pkg/apis/serving/v1/revision_defaults.go97.48% <100.00%> (+0.04%)⬆️
... and 5 files with indirect coverage changes
☔ View full report in Codecov by Sentry.

📢 Have feedback on the report? Share it here.
@KauzClay
KauzClay
marked this pull request as ready for review
September 13, 2023 21:10
@knative-prowknative-prowBot
removed
the 
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Sep 13, 2023
@dprotaso
dprotaso
commented
Sep 14, 2023

Copy link


Copy Markdown

Member
/lgtm

/approve

/cherry-pick release-1.11
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.11 in a new PR and assign it to you.
Details
In response to this:
/lgtm

/approve

/cherry-pick release-1.11
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/test-infra repository.
@knative-prowknative-prowBot
added
the 
lgtm
Indicates that a PR is ready to be merged.
label
Sep 14, 2023
@knative-prow
knative-prowBot
commented
Sep 14, 2023

Copy link


Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: dprotaso, KauzClay
The full list of commands accepted by this bot can be found here.
The pull request process is described 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-prowknative-prowBot
added
the 
approved
Indicates a PR has been approved by an approver from all required OWNERS files.
label
Sep 14, 2023
@dprotaso
dprotaso
commented
Sep 14, 2023

Copy link


Copy Markdown

Member
/cherry-pick release-1.10
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.10 in a new PR and assign it to you.
Details
In response to this:
/cherry-pick release-1.10
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/test-infra repository.
@knative-prow
knative-prowBot
merged commit 2a732e5
into
knative:mainSep 14, 2023
@knative-prow-robotknative-prow-robot
mentioned this pull request
Sep 14, 2023
 Merged

@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: new pull request created: #14377
Details
In response to this:
/cherry-pick release-1.10
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/test-infra repository.
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: new pull request created: #14378
Details
In response to this:
/lgtm

/approve

/cherry-pick release-1.11
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/test-infra repository.
@knative-prow-robotknative-prow-robot
mentioned this pull request
Sep 14, 2023
 Merged

@BobyMCbobs
BobyMCbobs
commented
Sep 15, 2023

Copy link


Copy Markdown

Contributor
Woohoo! This is a good change
@dprotaso
dprotaso
commented
Sep 15, 2023

Copy link


Copy Markdown

Member
Surprisingly it means no one has used the secure-pod-defaults feature on a restricted profile :/
krsna-m pushed a commit
to krsna-m/serving
that referenced
this pull request
Nov 6, 2023
@KauzClay@krsna-m
fix securityContext for Knative Service Pod (user-container and queue…
0b65385
…-proxy) (knative#14363)
* add seccompProfile to queue container security context
* run as non root by default
* update tests to expect new default run as nonroot
wmfgerrit pushed a commit
to wikimedia/operations-docker-images-production-images
that referenced
this pull request
Feb 27, 2025
@elukey
knative-serving: backportknative/serving#14363
a6725fc
The pull request is needed to force Knative to set runAsNonRoot
when secure-pod-defaults is true. The option forces knative to
create the user pods with all the safest security options to pass
the PSS restriction policy, but at the moment it is lacking
runAsNonRoot forced to true.
Note: I haven't backported the whole patch since part of it overlapped
with a previous one for queue.go, where seccomp's defaults were
set. The same code was committed in different pull requests, so I just
removed the bit already there to allow patch to avoid error/warnings.
Bug: T369493
Change-Id: Iceb1ac2d83f298ef2a834e24a8fdc8a6f1df4a28
wmfgerrit pushed a commit
to wikimedia/operations-docker-images-production-images
that referenced
this pull request
Feb 27, 2025
@elukeyGerrit Code Review
Merge "knative-serving: backportknative/serving#14363"
9c8d863
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@kauanakauanaAwaiting requested review from kauana
@krsna-mkrsna-mAwaiting requested review from krsna-m
@jsanin-vmwjsanin-vmwAwaiting requested review from jsanin-vmw
@nak3nak3Awaiting requested review from nak3

Assignees

@dprotasodprotaso

Labels

approvedIndicates a PR has been approved by an approver from all required OWNERS files.area/APIAPI objects and controllerslgtmIndicates that a PR is ready to be merged.size/SDenotes a PR that changes 10-29 lines, ignoring generated files.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.
secure-pod-defaults does not work when enforcing restricted Pod Security Standards


4 participants

@KauzClay@dprotaso@knative-prow-robot@BobyMCbobs

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
fix securityContext for Knative Service Pod (user-container and queue-proxy) - #14363
Merged
knative-prow[bot] merged 3 commits into
knative:mainfrom 
KauzClay:ck-add-seccompprofile
Sep 14, 2023
Merged
fix securityContext for Knative Service Pod (user-container and queue-proxy)#14363
knative-prow[bot] merged 3 commits into
knative:mainfrom 
KauzClay:ck-add-seccompprofile
Conversation
@KauzClay
@KauzClayKauzClay
commented
Sep 13, 2023•

edited by dprotaso

Loading

Copy link


Copy Markdown

Contributor
Fixes#14365
Related to:
Proposed Changes
  • Add seccompProfile type to queue proxy security context.
  • Add runAsNonRoot to secure-pod-defaults
  • I believe this PR added much of the security context sections. But when I install a Knative Service on a cluster that enforces Pod Security Standards, and I enabled secure-pod-defaults, I still get errors.
 status:
conditions:
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Created new replica set "tomato-00001-deployment-5db9b9f88d"
reason: NewReplicaSetCreated
status: "True"
type: Progressing
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Deployment does not have minimum availability.
reason: MinimumReplicasUnavailable
status: "False"
type: Available
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: 'admission webhook "pod-security-webhook.kubernetes.io" denied the request:
pods "tomato-00001-deployment-5db9b9f88d-w8gd4" is forbidden: violates PodSecurity
"restricted:latest": runAsNonRoot != true (pod or container "user-container"
must set securityContext.runAsNonRoot=true), seccompProfile (pod or container
"queue-proxy" must set securityContext.seccompProfile.type to "RuntimeDefault"
or "Localhost")'
reason: FailedCreate
status: "True"
type: ReplicaFailure

Release Note
Fix secure 'secure-pod-defaults' to work with restricted namespaces

@knative-prow
knative-prowBot
commented
Sep 13, 2023

Copy link


Copy Markdown

Skipping CI for Draft Pull Request.

If you want CI signal for your change, please convert it to an actual PR.

You can still manually trigger a test run with /test all
@knative-prowknative-prowBot
added
the 
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Sep 13, 2023
@knative-prowknative-prowBot
added

size/XS
Denotes a PR that changes 0-9 lines, ignoring generated files.
area/API
API objects and controllers
labels
Sep 13, 2023
@KauzClayKauzClay
changed the title
fix security Context for Knative Service Pod (user-container and queue-proxy)fix securityContext for Knative Service Pod (user-container and queue-proxy)Sep 13, 2023
@KauzClayKauzClay
mentioned this pull request
Sep 13, 2023
 Closed

@dprotaso
dprotaso
commented
Sep 13, 2023

Copy link


Copy Markdown

Member
To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?
@knative-prowknative-prowBot
added

size/S
Denotes a PR that changes 10-29 lines, ignoring generated files.
and removed

size/XS
Denotes a PR that changes 0-9 lines, ignoring generated files.
labels
Sep 13, 2023
@KauzClay
KauzClay
commented
Sep 13, 2023

Copy link


Copy Markdown

ContributorAuthor
To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?
yeah, that is what I'm seeing. This issue (#14365) has an example of the errors I see on my deployment
@codecov
codecovBot
commented
Sep 13, 2023•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Patch coverage: 100.00% and project coverage change: +0.02% 🎉
Comparison is base (4dddf9f) 86.12% compared to head (2cf6abf) 86.15%.

Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@## main #14363 +/- ##
==========================================
+ Coverage 86.12% 86.15% +0.02% 
==========================================
Files 196 196 Lines 14787 14790 +3 ==========================================
+ Hits 12735 12742 +7 + Misses 1744 1743 -1 + Partials 308 305 -3 
Files ChangedCoverage Δ
pkg/reconciler/revision/resources/queue.go98.42% <ø> (ø)
pkg/apis/serving/v1/revision_defaults.go97.48% <100.00%> (+0.04%)⬆️
... and 5 files with indirect coverage changes
☔ View full report in Codecov by Sentry.

📢 Have feedback on the report? Share it here.
@KauzClay
KauzClay
marked this pull request as ready for review
September 13, 2023 21:10
@knative-prowknative-prowBot
removed
the 
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Sep 13, 2023
@dprotaso
dprotaso
commented
Sep 14, 2023

Copy link


Copy Markdown

Member
/lgtm

/approve

/cherry-pick release-1.11
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.11 in a new PR and assign it to you.
Details
In response to this:
/lgtm

/approve

/cherry-pick release-1.11
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/test-infra repository.
@knative-prowknative-prowBot
added
the 
lgtm
Indicates that a PR is ready to be merged.
label
Sep 14, 2023
@knative-prow
knative-prowBot
commented
Sep 14, 2023

Copy link


Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: dprotaso, KauzClay
The full list of commands accepted by this bot can be found here.
The pull request process is described 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-prowknative-prowBot
added
the 
approved
Indicates a PR has been approved by an approver from all required OWNERS files.
label
Sep 14, 2023
@dprotaso
dprotaso
commented
Sep 14, 2023

Copy link


Copy Markdown

Member
/cherry-pick release-1.10
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.10 in a new PR and assign it to you.
Details
In response to this:
/cherry-pick release-1.10
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/test-infra repository.
@knative-prow
knative-prowBot
merged commit 2a732e5
into
knative:mainSep 14, 2023
@knative-prow-robotknative-prow-robot
mentioned this pull request
Sep 14, 2023
 Merged

@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: new pull request created: #14377
Details
In response to this:
/cherry-pick release-1.10
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/test-infra repository.
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: new pull request created: #14378
Details
In response to this:
/lgtm

/approve

/cherry-pick release-1.11
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/test-infra repository.
@knative-prow-robotknative-prow-robot
mentioned this pull request
Sep 14, 2023
 Merged

@BobyMCbobs
BobyMCbobs
commented
Sep 15, 2023

Copy link


Copy Markdown

Contributor
Woohoo! This is a good change
@dprotaso
dprotaso
commented
Sep 15, 2023

Copy link


Copy Markdown

Member
Surprisingly it means no one has used the secure-pod-defaults feature on a restricted profile :/
krsna-m pushed a commit
to krsna-m/serving
that referenced
this pull request
Nov 6, 2023
@KauzClay@krsna-m
fix securityContext for Knative Service Pod (user-container and queue…
0b65385
…-proxy) (knative#14363)
* add seccompProfile to queue container security context
* run as non root by default
* update tests to expect new default run as nonroot
wmfgerrit pushed a commit
to wikimedia/operations-docker-images-production-images
that referenced
this pull request
Feb 27, 2025
@elukey
knative-serving: backportknative/serving#14363
a6725fc
The pull request is needed to force Knative to set runAsNonRoot
when secure-pod-defaults is true. The option forces knative to
create the user pods with all the safest security options to pass
the PSS restriction policy, but at the moment it is lacking
runAsNonRoot forced to true.
Note: I haven't backported the whole patch since part of it overlapped
with a previous one for queue.go, where seccomp's defaults were
set. The same code was committed in different pull requests, so I just
removed the bit already there to allow patch to avoid error/warnings.
Bug: T369493
Change-Id: Iceb1ac2d83f298ef2a834e24a8fdc8a6f1df4a28
wmfgerrit pushed a commit
to wikimedia/operations-docker-images-production-images
that referenced
this pull request
Feb 27, 2025
@elukeyGerrit Code Review
Merge "knative-serving: backportknative/serving#14363"
9c8d863
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@kauanakauanaAwaiting requested review from kauana
@krsna-mkrsna-mAwaiting requested review from krsna-m
@jsanin-vmwjsanin-vmwAwaiting requested review from jsanin-vmw
@nak3nak3Awaiting requested review from nak3

Assignees

@dprotasodprotaso

Labels

approvedIndicates a PR has been approved by an approver from all required OWNERS files.area/APIAPI objects and controllerslgtmIndicates that a PR is ready to be merged.size/SDenotes a PR that changes 10-29 lines, ignoring generated files.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.
secure-pod-defaults does not work when enforcing restricted Pod Security Standards


4 participants

@KauzClay@dprotaso@knative-prow-robot@BobyMCbobs

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix securityContext for Knative Service Pod (user-container and queue-proxy) - #14363
Merged
knative-prow[bot] merged 3 commits into
knative:mainfrom 
KauzClay:ck-add-seccompprofile
Sep 14, 2023
Merged
fix securityContext for Knative Service Pod (user-container and queue-proxy)#14363
knative-prow[bot] merged 3 commits into
knative:mainfrom 
KauzClay:ck-add-seccompprofile
Conversation
@KauzClay
@KauzClayKauzClay
commented
Sep 13, 2023•

edited by dprotaso

Loading

Copy link


Copy Markdown

Contributor
Fixes#14365
Related to:
Proposed Changes
  • Add seccompProfile type to queue proxy security context.
  • Add runAsNonRoot to secure-pod-defaults
  • I believe this PR added much of the security context sections. But when I install a Knative Service on a cluster that enforces Pod Security Standards, and I enabled secure-pod-defaults, I still get errors.
 status:
conditions:
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Created new replica set "tomato-00001-deployment-5db9b9f88d"
reason: NewReplicaSetCreated
status: "True"
type: Progressing
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Deployment does not have minimum availability.
reason: MinimumReplicasUnavailable
status: "False"
type: Available
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: 'admission webhook "pod-security-webhook.kubernetes.io" denied the request:
pods "tomato-00001-deployment-5db9b9f88d-w8gd4" is forbidden: violates PodSecurity
"restricted:latest": runAsNonRoot != true (pod or container "user-container"
must set securityContext.runAsNonRoot=true), seccompProfile (pod or container
"queue-proxy" must set securityContext.seccompProfile.type to "RuntimeDefault"
or "Localhost")'
reason: FailedCreate
status: "True"
type: ReplicaFailure

Release Note
Fix secure 'secure-pod-defaults' to work with restricted namespaces

@knative-prow
knative-prowBot
commented
Sep 13, 2023

Copy link


Copy Markdown

Skipping CI for Draft Pull Request.

If you want CI signal for your change, please convert it to an actual PR.

You can still manually trigger a test run with /test all
@knative-prowknative-prowBot
added
the 
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Sep 13, 2023
@knative-prowknative-prowBot
added

size/XS
Denotes a PR that changes 0-9 lines, ignoring generated files.
area/API
API objects and controllers
labels
Sep 13, 2023
@KauzClayKauzClay
changed the title
fix security Context for Knative Service Pod (user-container and queue-proxy)fix securityContext for Knative Service Pod (user-container and queue-proxy)Sep 13, 2023
@KauzClayKauzClay
mentioned this pull request
Sep 13, 2023
 Closed

@dprotaso
dprotaso
commented
Sep 13, 2023

Copy link


Copy Markdown

Member
To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?
@knative-prowknative-prowBot
added

size/S
Denotes a PR that changes 10-29 lines, ignoring generated files.
and removed

size/XS
Denotes a PR that changes 0-9 lines, ignoring generated files.
labels
Sep 13, 2023
@KauzClay
KauzClay
commented
Sep 13, 2023

Copy link


Copy Markdown

ContributorAuthor
To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?
yeah, that is what I'm seeing. This issue (#14365) has an example of the errors I see on my deployment
@codecov
codecovBot
commented
Sep 13, 2023•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Patch coverage: 100.00% and project coverage change: +0.02% 🎉
Comparison is base (4dddf9f) 86.12% compared to head (2cf6abf) 86.15%.

Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@## main #14363 +/- ##
==========================================
+ Coverage 86.12% 86.15% +0.02% 
==========================================
Files 196 196 Lines 14787 14790 +3 ==========================================
+ Hits 12735 12742 +7 + Misses 1744 1743 -1 + Partials 308 305 -3 
Files ChangedCoverage Δ
pkg/reconciler/revision/resources/queue.go98.42% <ø> (ø)
pkg/apis/serving/v1/revision_defaults.go97.48% <100.00%> (+0.04%)⬆️
... and 5 files with indirect coverage changes
☔ View full report in Codecov by Sentry.

📢 Have feedback on the report? Share it here.
@KauzClay
KauzClay
marked this pull request as ready for review
September 13, 2023 21:10
@knative-prowknative-prowBot
removed
the 
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Sep 13, 2023
@dprotaso
dprotaso
commented
Sep 14, 2023

Copy link


Copy Markdown

Member
/lgtm

/approve

/cherry-pick release-1.11
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.11 in a new PR and assign it to you.
Details
In response to this:
/lgtm

/approve

/cherry-pick release-1.11
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/test-infra repository.
@knative-prowknative-prowBot
added
the 
lgtm
Indicates that a PR is ready to be merged.
label
Sep 14, 2023
@knative-prow
knative-prowBot
commented
Sep 14, 2023

Copy link


Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: dprotaso, KauzClay
The full list of commands accepted by this bot can be found here.
The pull request process is described 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-prowknative-prowBot
added
the 
approved
Indicates a PR has been approved by an approver from all required OWNERS files.
label
Sep 14, 2023
@dprotaso
dprotaso
commented
Sep 14, 2023

Copy link


Copy Markdown

Member
/cherry-pick release-1.10
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.10 in a new PR and assign it to you.
Details
In response to this:
/cherry-pick release-1.10
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/test-infra repository.
@knative-prow
knative-prowBot
merged commit 2a732e5
into
knative:mainSep 14, 2023
@knative-prow-robotknative-prow-robot
mentioned this pull request
Sep 14, 2023
 Merged

@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: new pull request created: #14377
Details
In response to this:
/cherry-pick release-1.10
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/test-infra repository.
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: new pull request created: #14378
Details
In response to this:
/lgtm

/approve

/cherry-pick release-1.11
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/test-infra repository.
@knative-prow-robotknative-prow-robot
mentioned this pull request
Sep 14, 2023
 Merged

@BobyMCbobs
BobyMCbobs
commented
Sep 15, 2023

Copy link


Copy Markdown

Contributor
Woohoo! This is a good change
@dprotaso
dprotaso
commented
Sep 15, 2023

Copy link


Copy Markdown

Member
Surprisingly it means no one has used the secure-pod-defaults feature on a restricted profile :/
krsna-m pushed a commit
to krsna-m/serving
that referenced
this pull request
Nov 6, 2023
@KauzClay@krsna-m
fix securityContext for Knative Service Pod (user-container and queue…
0b65385
…-proxy) (knative#14363)
* add seccompProfile to queue container security context
* run as non root by default
* update tests to expect new default run as nonroot
wmfgerrit pushed a commit
to wikimedia/operations-docker-images-production-images
that referenced
this pull request
Feb 27, 2025
@elukey
knative-serving: backportknative/serving#14363
a6725fc
The pull request is needed to force Knative to set runAsNonRoot
when secure-pod-defaults is true. The option forces knative to
create the user pods with all the safest security options to pass
the PSS restriction policy, but at the moment it is lacking
runAsNonRoot forced to true.
Note: I haven't backported the whole patch since part of it overlapped
with a previous one for queue.go, where seccomp's defaults were
set. The same code was committed in different pull requests, so I just
removed the bit already there to allow patch to avoid error/warnings.
Bug: T369493
Change-Id: Iceb1ac2d83f298ef2a834e24a8fdc8a6f1df4a28
wmfgerrit pushed a commit
to wikimedia/operations-docker-images-production-images
that referenced
this pull request
Feb 27, 2025
@elukeyGerrit Code Review
Merge "knative-serving: backportknative/serving#14363"
9c8d863
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@kauanakauanaAwaiting requested review from kauana
@krsna-mkrsna-mAwaiting requested review from krsna-m
@jsanin-vmwjsanin-vmwAwaiting requested review from jsanin-vmw
@nak3nak3Awaiting requested review from nak3

Assignees

@dprotasodprotaso

Labels

approvedIndicates a PR has been approved by an approver from all required OWNERS files.area/APIAPI objects and controllerslgtmIndicates that a PR is ready to be merged.size/SDenotes a PR that changes 10-29 lines, ignoring generated files.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.
secure-pod-defaults does not work when enforcing restricted Pod Security Standards


4 participants

@KauzClay@dprotaso@knative-prow-robot@BobyMCbobs

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
fix securityContext for Knative Service Pod (user-container and queue-proxy) - #14363
Merged
knative-prow[bot] merged 3 commits into
knative:mainfrom 
KauzClay:ck-add-seccompprofile
Sep 14, 2023
Merged
fix securityContext for Knative Service Pod (user-container and queue-proxy)#14363
knative-prow[bot] merged 3 commits into
knative:mainfrom 
KauzClay:ck-add-seccompprofile
Conversation
@KauzClay
@KauzClayKauzClay
commented
Sep 13, 2023•

edited by dprotaso

Loading

Copy link


Copy Markdown

Contributor
Fixes#14365
Related to:
Proposed Changes
  • Add seccompProfile type to queue proxy security context.
  • Add runAsNonRoot to secure-pod-defaults
  • I believe this PR added much of the security context sections. But when I install a Knative Service on a cluster that enforces Pod Security Standards, and I enabled secure-pod-defaults, I still get errors.
 status:
conditions:
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Created new replica set "tomato-00001-deployment-5db9b9f88d"
reason: NewReplicaSetCreated
status: "True"
type: Progressing
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Deployment does not have minimum availability.
reason: MinimumReplicasUnavailable
status: "False"
type: Available
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: 'admission webhook "pod-security-webhook.kubernetes.io" denied the request:
pods "tomato-00001-deployment-5db9b9f88d-w8gd4" is forbidden: violates PodSecurity
"restricted:latest": runAsNonRoot != true (pod or container "user-container"
must set securityContext.runAsNonRoot=true), seccompProfile (pod or container
"queue-proxy" must set securityContext.seccompProfile.type to "RuntimeDefault"
or "Localhost")'
reason: FailedCreate
status: "True"
type: ReplicaFailure

Release Note
Fix secure 'secure-pod-defaults' to work with restricted namespaces

@knative-prow
knative-prowBot
commented
Sep 13, 2023

Copy link


Copy Markdown

Skipping CI for Draft Pull Request.

If you want CI signal for your change, please convert it to an actual PR.

You can still manually trigger a test run with /test all
@knative-prowknative-prowBot
added
the 
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Sep 13, 2023
@knative-prowknative-prowBot
added

size/XS
Denotes a PR that changes 0-9 lines, ignoring generated files.
area/API
API objects and controllers
labels
Sep 13, 2023
@KauzClayKauzClay
changed the title
fix security Context for Knative Service Pod (user-container and queue-proxy)fix securityContext for Knative Service Pod (user-container and queue-proxy)Sep 13, 2023
@KauzClayKauzClay
mentioned this pull request
Sep 13, 2023
 Closed

@dprotaso
dprotaso
commented
Sep 13, 2023

Copy link


Copy Markdown

Member
To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?
@knative-prowknative-prowBot
added

size/S
Denotes a PR that changes 10-29 lines, ignoring generated files.
and removed

size/XS
Denotes a PR that changes 0-9 lines, ignoring generated files.
labels
Sep 13, 2023
@KauzClay
KauzClay
commented
Sep 13, 2023

Copy link


Copy Markdown

ContributorAuthor
To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?
yeah, that is what I'm seeing. This issue (#14365) has an example of the errors I see on my deployment
@codecov
codecovBot
commented
Sep 13, 2023•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Patch coverage: 100.00% and project coverage change: +0.02% 🎉
Comparison is base (4dddf9f) 86.12% compared to head (2cf6abf) 86.15%.

Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@## main #14363 +/- ##
==========================================
+ Coverage 86.12% 86.15% +0.02% 
==========================================
Files 196 196 Lines 14787 14790 +3 ==========================================
+ Hits 12735 12742 +7 + Misses 1744 1743 -1 + Partials 308 305 -3 
Files ChangedCoverage Δ
pkg/reconciler/revision/resources/queue.go98.42% <ø> (ø)
pkg/apis/serving/v1/revision_defaults.go97.48% <100.00%> (+0.04%)⬆️
... and 5 files with indirect coverage changes
☔ View full report in Codecov by Sentry.

📢 Have feedback on the report? Share it here.
@KauzClay
KauzClay
marked this pull request as ready for review
September 13, 2023 21:10
@knative-prowknative-prowBot
removed
the 
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Sep 13, 2023
@dprotaso
dprotaso
commented
Sep 14, 2023

Copy link


Copy Markdown

Member
/lgtm

/approve

/cherry-pick release-1.11
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.11 in a new PR and assign it to you.
Details
In response to this:
/lgtm

/approve

/cherry-pick release-1.11
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/test-infra repository.
@knative-prowknative-prowBot
added
the 
lgtm
Indicates that a PR is ready to be merged.
label
Sep 14, 2023
@knative-prow
knative-prowBot
commented
Sep 14, 2023

Copy link


Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: dprotaso, KauzClay
The full list of commands accepted by this bot can be found here.
The pull request process is described 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-prowknative-prowBot
added
the 
approved
Indicates a PR has been approved by an approver from all required OWNERS files.
label
Sep 14, 2023
@dprotaso
dprotaso
commented
Sep 14, 2023

Copy link


Copy Markdown

Member
/cherry-pick release-1.10
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.10 in a new PR and assign it to you.
Details
In response to this:
/cherry-pick release-1.10
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/test-infra repository.
@knative-prow
knative-prowBot
merged commit 2a732e5
into
knative:mainSep 14, 2023
@knative-prow-robotknative-prow-robot
mentioned this pull request
Sep 14, 2023
 Merged

@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: new pull request created: #14377
Details
In response to this:
/cherry-pick release-1.10
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/test-infra repository.
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: new pull request created: #14378
Details
In response to this:
/lgtm

/approve

/cherry-pick release-1.11
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/test-infra repository.
@knative-prow-robotknative-prow-robot
mentioned this pull request
Sep 14, 2023
 Merged

@BobyMCbobs
BobyMCbobs
commented
Sep 15, 2023

Copy link


Copy Markdown

Contributor
Woohoo! This is a good change
@dprotaso
dprotaso
commented
Sep 15, 2023

Copy link


Copy Markdown

Member
Surprisingly it means no one has used the secure-pod-defaults feature on a restricted profile :/
krsna-m pushed a commit
to krsna-m/serving
that referenced
this pull request
Nov 6, 2023
@KauzClay@krsna-m
fix securityContext for Knative Service Pod (user-container and queue…
0b65385
…-proxy) (knative#14363)
* add seccompProfile to queue container security context
* run as non root by default
* update tests to expect new default run as nonroot
wmfgerrit pushed a commit
to wikimedia/operations-docker-images-production-images
that referenced
this pull request
Feb 27, 2025
@elukey
knative-serving: backportknative/serving#14363
a6725fc
The pull request is needed to force Knative to set runAsNonRoot
when secure-pod-defaults is true. The option forces knative to
create the user pods with all the safest security options to pass
the PSS restriction policy, but at the moment it is lacking
runAsNonRoot forced to true.
Note: I haven't backported the whole patch since part of it overlapped
with a previous one for queue.go, where seccomp's defaults were
set. The same code was committed in different pull requests, so I just
removed the bit already there to allow patch to avoid error/warnings.
Bug: T369493
Change-Id: Iceb1ac2d83f298ef2a834e24a8fdc8a6f1df4a28
wmfgerrit pushed a commit
to wikimedia/operations-docker-images-production-images
that referenced
this pull request
Feb 27, 2025
@elukeyGerrit Code Review
Merge "knative-serving: backportknative/serving#14363"
9c8d863
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@kauanakauanaAwaiting requested review from kauana
@krsna-mkrsna-mAwaiting requested review from krsna-m
@jsanin-vmwjsanin-vmwAwaiting requested review from jsanin-vmw
@nak3nak3Awaiting requested review from nak3

Assignees

@dprotasodprotaso

Labels

approvedIndicates a PR has been approved by an approver from all required OWNERS files.area/APIAPI objects and controllerslgtmIndicates that a PR is ready to be merged.size/SDenotes a PR that changes 10-29 lines, ignoring generated files.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.
secure-pod-defaults does not work when enforcing restricted Pod Security Standards


4 participants

@KauzClay@dprotaso@knative-prow-robot@BobyMCbobs

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
fix securityContext for Knative Service Pod (user-container and queue-proxy) - #14363
Merged
knative-prow[bot] merged 3 commits into
knative:mainfrom 
KauzClay:ck-add-seccompprofile
Sep 14, 2023
Merged
fix securityContext for Knative Service Pod (user-container and queue-proxy)#14363
knative-prow[bot] merged 3 commits into
knative:mainfrom 
KauzClay:ck-add-seccompprofile
Conversation
@KauzClay
@KauzClayKauzClay
commented
Sep 13, 2023•

edited by dprotaso

Loading

Copy link


Copy Markdown

Contributor
Fixes#14365
Related to:
Proposed Changes
  • Add seccompProfile type to queue proxy security context.
  • Add runAsNonRoot to secure-pod-defaults
  • I believe this PR added much of the security context sections. But when I install a Knative Service on a cluster that enforces Pod Security Standards, and I enabled secure-pod-defaults, I still get errors.
 status:
conditions:
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Created new replica set "tomato-00001-deployment-5db9b9f88d"
reason: NewReplicaSetCreated
status: "True"
type: Progressing
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: Deployment does not have minimum availability.
reason: MinimumReplicasUnavailable
status: "False"
type: Available
- lastTransitionTime: "2023-09-13T15:13:42Z"
lastUpdateTime: "2023-09-13T15:13:42Z"
message: 'admission webhook "pod-security-webhook.kubernetes.io" denied the request:
pods "tomato-00001-deployment-5db9b9f88d-w8gd4" is forbidden: violates PodSecurity
"restricted:latest": runAsNonRoot != true (pod or container "user-container"
must set securityContext.runAsNonRoot=true), seccompProfile (pod or container
"queue-proxy" must set securityContext.seccompProfile.type to "RuntimeDefault"
or "Localhost")'
reason: FailedCreate
status: "True"
type: ReplicaFailure

Release Note
Fix secure 'secure-pod-defaults' to work with restricted namespaces

@knative-prow
knative-prowBot
commented
Sep 13, 2023

Copy link


Copy Markdown

Skipping CI for Draft Pull Request.

If you want CI signal for your change, please convert it to an actual PR.

You can still manually trigger a test run with /test all
@knative-prowknative-prowBot
added
the 
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Sep 13, 2023
@knative-prowknative-prowBot
added

size/XS
Denotes a PR that changes 0-9 lines, ignoring generated files.
area/API
API objects and controllers
labels
Sep 13, 2023
@KauzClayKauzClay
changed the title
fix security Context for Knative Service Pod (user-container and queue-proxy)fix securityContext for Knative Service Pod (user-container and queue-proxy)Sep 13, 2023
@KauzClayKauzClay
mentioned this pull request
Sep 13, 2023
 Closed

@dprotaso
dprotaso
commented
Sep 13, 2023

Copy link


Copy Markdown

Member
To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?
@knative-prowknative-prowBot
added

size/S
Denotes a PR that changes 10-29 lines, ignoring generated files.
and removed

size/XS
Denotes a PR that changes 0-9 lines, ignoring generated files.
labels
Sep 13, 2023
@KauzClay
KauzClay
commented
Sep 13, 2023

Copy link


Copy Markdown

ContributorAuthor
To confirm - Does the changes in this PR imply that SecurePodDefaults doesn't work? Since the queue proxy and RevisionSpec are missing those explicit settings?
yeah, that is what I'm seeing. This issue (#14365) has an example of the errors I see on my deployment
@codecov
codecovBot
commented
Sep 13, 2023•

edited

Loading

Copy link


Copy Markdown

Codecov Report
Patch coverage: 100.00% and project coverage change: +0.02% 🎉
Comparison is base (4dddf9f) 86.12% compared to head (2cf6abf) 86.15%.

Report is 1 commits behind head on main.
Additional details and impacted files
@@ Coverage Diff @@## main #14363 +/- ##
==========================================
+ Coverage 86.12% 86.15% +0.02% 
==========================================
Files 196 196 Lines 14787 14790 +3 ==========================================
+ Hits 12735 12742 +7 + Misses 1744 1743 -1 + Partials 308 305 -3 
Files ChangedCoverage Δ
pkg/reconciler/revision/resources/queue.go98.42% <ø> (ø)
pkg/apis/serving/v1/revision_defaults.go97.48% <100.00%> (+0.04%)⬆️
... and 5 files with indirect coverage changes
☔ View full report in Codecov by Sentry.

📢 Have feedback on the report? Share it here.
@KauzClay
KauzClay
marked this pull request as ready for review
September 13, 2023 21:10
@knative-prowknative-prowBot
removed
the 
do-not-merge/work-in-progress
Indicates that a PR should not merge because it is a work in progress.
label
Sep 13, 2023
@dprotaso
dprotaso
commented
Sep 14, 2023

Copy link


Copy Markdown

Member
/lgtm

/approve

/cherry-pick release-1.11
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.11 in a new PR and assign it to you.
Details
In response to this:
/lgtm

/approve

/cherry-pick release-1.11
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/test-infra repository.
@knative-prowknative-prowBot
added
the 
lgtm
Indicates that a PR is ready to be merged.
label
Sep 14, 2023
@knative-prow
knative-prowBot
commented
Sep 14, 2023

Copy link


Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: dprotaso, KauzClay
The full list of commands accepted by this bot can be found here.
The pull request process is described 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-prowknative-prowBot
added
the 
approved
Indicates a PR has been approved by an approver from all required OWNERS files.
label
Sep 14, 2023
@dprotaso
dprotaso
commented
Sep 14, 2023

Copy link


Copy Markdown

Member
/cherry-pick release-1.10
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: once the present PR merges, I will cherry-pick it on top of release-1.10 in a new PR and assign it to you.
Details
In response to this:
/cherry-pick release-1.10
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/test-infra repository.
@knative-prow
knative-prowBot
merged commit 2a732e5
into
knative:mainSep 14, 2023
@knative-prow-robotknative-prow-robot
mentioned this pull request
Sep 14, 2023
 Merged

@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: new pull request created: #14377
Details
In response to this:
/cherry-pick release-1.10
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/test-infra repository.
@knative-prow-robot
knative-prow-robot
commented
Sep 14, 2023

Copy link


Copy Markdown

Contributor
@dprotaso: new pull request created: #14378
Details
In response to this:
/lgtm

/approve

/cherry-pick release-1.11
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/test-infra repository.
@knative-prow-robotknative-prow-robot
mentioned this pull request
Sep 14, 2023
 Merged

@BobyMCbobs
BobyMCbobs
commented
Sep 15, 2023

Copy link


Copy Markdown

Contributor
Woohoo! This is a good change
@dprotaso
dprotaso
commented
Sep 15, 2023

Copy link


Copy Markdown

Member
Surprisingly it means no one has used the secure-pod-defaults feature on a restricted profile :/
krsna-m pushed a commit
to krsna-m/serving
that referenced
this pull request
Nov 6, 2023
@KauzClay@krsna-m
fix securityContext for Knative Service Pod (user-container and queue…
0b65385
…-proxy) (knative#14363)
* add seccompProfile to queue container security context
* run as non root by default
* update tests to expect new default run as nonroot
wmfgerrit pushed a commit
to wikimedia/operations-docker-images-production-images
that referenced
this pull request
Feb 27, 2025
@elukey
knative-serving: backportknative/serving#14363
a6725fc
The pull request is needed to force Knative to set runAsNonRoot
when secure-pod-defaults is true. The option forces knative to
create the user pods with all the safest security options to pass
the PSS restriction policy, but at the moment it is lacking
runAsNonRoot forced to true.
Note: I haven't backported the whole patch since part of it overlapped
with a previous one for queue.go, where seccomp's defaults were
set. The same code was committed in different pull requests, so I just
removed the bit already there to allow patch to avoid error/warnings.
Bug: T369493
Change-Id: Iceb1ac2d83f298ef2a834e24a8fdc8a6f1df4a28
wmfgerrit pushed a commit
to wikimedia/operations-docker-images-production-images
that referenced
this pull request
Feb 27, 2025
@elukeyGerrit Code Review
Merge "knative-serving: backportknative/serving#14363"
9c8d863
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment

Reviewers

@kauanakauanaAwaiting requested review from kauana
@krsna-mkrsna-mAwaiting requested review from krsna-m
@jsanin-vmwjsanin-vmwAwaiting requested review from jsanin-vmw
@nak3nak3Awaiting requested review from nak3

Assignees

@dprotasodprotaso

Labels

approvedIndicates a PR has been approved by an approver from all required OWNERS files.area/APIAPI objects and controllerslgtmIndicates that a PR is ready to be merged.size/SDenotes a PR that changes 10-29 lines, ignoring generated files.

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.
secure-pod-defaults does not work when enforcing restricted Pod Security Standards


4 participants

@KauzClay@dprotaso@knative-prow-robot@BobyMCbobs