-
-
Notifications
You must be signed in to change notification settings - Fork 599
Add csi support #834
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
base: master
Are you sure you want to change the base?
Add csi support #834
Conversation
Signed-off-by: Zanis <[email protected]>
@ZanisO can you plz pull changes from main and update your PR? and also update the readme; we will start its review next week. |
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.
Nice work, thank you!
Unless you respond we will fork this and take it over the finish line, sorry for the late review.
var controllers []*controller.Controller | ||
for k := range kube.ResourceMap { | ||
if k == "secretproviderclasspodstatuses" { | ||
if !options.EnableCSIIntegration { |
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.
Do we want to invert the setting as to having it enabled if no option is supplied?
if options.DisableCSIIntegration
switch object := old.(type) { | ||
case *v1.Namespace: | ||
c.removeSelectedNamespaceFromCache(*object) | ||
return |
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.
Moving this return
to line 209 changes behavior
oldSHAData = util.GetSHAfromSecretProviderClassPodStatus(r.OldResource.(*csiv1.SecretProviderClassPodStatus).Status) | ||
config = util.GetSecretProviderClassPodStatusConfig(r.Resource.(*csiv1.SecretProviderClassPodStatus)) | ||
} else { | ||
logrus.Warnf("Invalid resource: Resource should be 'Secret' or 'Configmap' but found, %v", r.Resource) |
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 log message is no longer accurate.
} else if _, ok := r.Resource.(*v1.Secret); ok { | ||
oldSHAData = util.GetSHAfromSecret(r.OldResource.(*v1.Secret).Data) | ||
config = util.GetSecretConfig(r.Resource.(*v1.Secret)) | ||
} else if _, ok := r.Resource.(*csiv1.SecretProviderClassPodStatus); ok { |
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.
If you could turn this whole block into a type-switch I think that would look a lot cleaner
} | ||
} | ||
} else if mountType == constants.SecretProviderClassEnvVarPostfix { | ||
if volumes[i].CSI != nil && volumes[i].CSI.VolumeAttributes["secretProviderClass"] == volumeName { |
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.
Check if the secreetProviderClass
is required.
@ZanisO are you around? |
Hi,
I've attempted to implement ability to watch SecretProviderClassPodStatus, discussed in #440