fix issue https://github.com/kubernetes/kompose/issues/2054#2065
fix issue https://github.com/kubernetes/kompose/issues/2054#2065k8s-ci-robot merged 2 commits intokubernetes:mainfrom
Conversation
cdrage
left a comment
There was a problem hiding this comment.
Just need a simple test added to test the functionality, and this LGTM!
| if *securityContext != (api.SecurityContext{}) { | ||
| podSpec.Containers[0].SecurityContext = securityContext | ||
| //podSpec.Containers[0].SecurityContext = securityContext | ||
| for i := range podSpec.Containers { |
There was a problem hiding this comment.
Are you able to instead add a comment above saying something like...
"Search through all the containers, find the one that matches the container name and set the security context appropriately"?
And remove the commented out code.
This implementation LGTM (sucks we have to loop through all the containers... but that's a nitpick), but otherwise this is good!
There was a problem hiding this comment.
Changed as requested.
cdrage
left a comment
There was a problem hiding this comment.
SO SORRY for the late review!
But this works, looks good and I want this merged in!
|
/approve |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cdrage, hurzelpurzel The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind bug
What this PR does / why we need it:
Set SecurityContext to the Container by its name
Which issue(s) this PR fixes:
Fixes #2054
Special notes for your reviewer:
Please be kind. I'm a frequent contributor