-
Notifications
You must be signed in to change notification settings - Fork 91
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
MAISTRA-551 Run sidecar with a dynamically-determined runAsUser ID #27
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a few comments/questions
@@ -687,6 +712,19 @@ func (wh *Webhook) inject(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionRespons | |||
return &reviewResponse | |||
} | |||
|
|||
func getProxyUID(pod corev1.Pod) (*uint64, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about return -1 in cases of errors? That would eliminate the need of dealing with pointers above
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that was my first instinct, too... but IMHO a pointer to represent optionality is much better than using magic values. Not sure what the golang position on this is.
@@ -715,7 +717,8 @@ func intoObject(sidecarTemplate string, meshconfig *meshconfig.MeshConfig, in ru | |||
podSpec, | |||
metadata, | |||
meshconfig.DefaultConfig, | |||
meshconfig) | |||
meshconfig, | |||
DefaultSidecarProxyUID) | |||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it safe to use the default value here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This preserves the old behavior. The thing is, we can't put the UID into the configmap, because the uid that should be used differs from one namespace to the next.
Fix istio repo location
* Recursive directory watcher * Fix initial timer value * Lint * Add copyright * Review comments * Add the codecov setup from istio repo (maistra#32) * Add the codecov setup from istio repo * Add the codecov setup from istio repo * Extend CODEOWNERS (maistra#37) * Add local codecov check script (maistra#23) * Add local codecov check script * Add codecov diff check script to check coverage drop * Add codecov diff check script to check coverage drop * Update common files. (maistra#39) * Add build infra for proto to Go structs (maistra#34) * Add build infra for proto to Go structs * Add deepcopy for gogo types used in IstioControlPlane proto * Bypass deepcopy for DynamicAny, not used * WIP * Disable deepcopy until clean solution can be found * Minor updates * add resources or certmanager. (maistra#45) * Add Environment WG leads to CODEOWNERS (maistra#46) * Refactoring utils (maistra#40) * Refactoring utils * Lint * Copyrights * Minor cleanup * Overview of component package group (maistra#27) * Config and profile samples (maistra#38) * Config and profile samples * Update profiles and sample * Additional fields * Default profile comes from compiled in * Listen also for create and delete events
* update Envoy sha (maistra#25) * update Envoy sha * update sha again * Bump base image (maistra#27) * Update proxy sha Co-authored-by: Yangmin Zhu <ymzhu@google.com> Co-authored-by: Eric Van Norman <ericvn@us.ibm.com>
No description provided.