Populate container.SecurityContext.RunAsUser when --user flag is used #1927
Conversation
knative-prow
bot
added
the
approved
![knative-prow[bot]](https://avatars.githubusercontent.com/in/183726?s=60&v=4){kind=small}
There was a problem hiding this comment.
@vyasgun: 0 warnings.
In response to this:
Description
Changes
- Setting container's RunAsUser field based on the
--userflag at the end so it does not reinitialize the security contextReference
Fixes #1926
Release Note
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
bot
added
the
size/S
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1927 +/- ##
==========================================
+ Coverage 74.58% 76.83% +2.25%
==========================================
Files 207 207
Lines 15563 12753 -2810
==========================================
- Hits 11607 9799 -1808
+ Misses 3167 2165 -1002
Partials 789 789 ☔ View full report in Codecov by Sentry. |
| if flags.Changed("user") { | ||
| err = UpdateUser(podSpec, p.User) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Is there a reason to change the order of evaluation here?
There was a problem hiding this comment.
IMO it's very likely SC was override by UpdateSecurityContext.
There was a problem hiding this comment.
The order of evaluation is causing the value of the user flag to get overwritten. I have created an issue for this: #1926
|
It would be nice to have a simple unit tests for the change. |
|
@vyasgun gentle ping. If you could check my comments, pls. I'd like to make sure both |
|
Thanks for the review, @dsimansk! Updated the PR with a small change which will test both flags together. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dsimansk, vyasgun The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/retest |
|
@vyasgun I still have a small hesitation how to fix the issue actually. Maybe the |
Description
Changes
--userflag at the end so it does not reinitialize the security contextReference
Fixes #1926
Release Note