Serviceaccount token synchronization #139
Conversation
galal-hussein
commented
Nov 6, 2024
•
galal-hussein
commented
- Virtual Kubelet: Pod Volumes + Mounts #115
Signed-off-by: galal-hussein <[email protected]>
Signed-off-by: galal-hussein <[email protected]>
galal-hussein
force-pushed
the
service_account_token_sync
branch
from
5f4fd2e to
f728c73
Compare
Signed-off-by: galal-hussein <[email protected]>
| if volume.ConfigMap != nil { | ||
| if err := p.syncConfigmap(ctx, podNamespace, volume.ConfigMap.Name); err != nil { | ||
| if volume.ConfigMap.Optional != nil { | ||
| optional = *volume.ConfigMap.Optional | ||
| } | ||
| if err := p.syncConfigmap(ctx, podNamespace, volume.ConfigMap.Name, optional); err != nil { | ||
| return fmt.Errorf("unable to sync configmap volume %s: %w", volume.Name, err) | ||
| } | ||
| volume.ConfigMap.Name = p.Translater.TranslateName(podNamespace, volume.ConfigMap.Name) | ||
| } else if volume.Secret != nil { | ||
| if err := p.syncSecret(ctx, podNamespace, volume.Secret.SecretName); err != nil { | ||
| if volume.Secret.Optional != nil { | ||
| optional = *volume.Secret.Optional | ||
| } | ||
| if err := p.syncSecret(ctx, podNamespace, volume.Secret.SecretName, optional); err != nil { |
There was a problem hiding this comment.
Imho the code will look more readable changing the structure to have less branches, and passing the struct to the syncXXX funcs. Something like:
if volume.ConfigMap != nil {
if err := p.syncConfigmap(ctx, podNamespace, *volume.ConfigMap); err != nil {
return fmt.Errorf("unable to sync configmap volume %s: %w", volume.Name, err)
}
volume.ConfigMap.Name = p.Translater.TranslateName(podNamespace, volume.ConfigMap.Name)
continue
}
if volume.Secret != nil {
// same
}What do you think?
There was a problem hiding this comment.
that makes sense, fixing and testing now
There was a problem hiding this comment.
hmmm, I think why Michael didnt want to do that in the first place, simply because there are 2 types of volumes , there is simple configmap and projection configmap, which are two different types, thats why he only sent the names and not the whole object, that said Michael wanted to make the syncer more generic anyway, so I think we can merge for now and for future iteration we can conver the whole syncer to a more generic functions to accept any type of object
There was a problem hiding this comment.
Yes, I've just realized that. :/
Kinda annoying but I agree, thanks anyway! 👍
* Serviceaccount token sync Signed-off-by: galal-hussein <[email protected]> * fixes Signed-off-by: galal-hussein <[email protected]> * fixing typo Signed-off-by: galal-hussein <[email protected]> --------- Signed-off-by: galal-hussein <[email protected]>
