Skip to content

Conversation

ZanisO
Copy link

@ZanisO ZanisO commented Feb 7, 2025

Hi,

I've attempted to implement ability to watch SecretProviderClassPodStatus, discussed in #440

@rasheedamir rasheedamir requested a review from msafwankarim July 22, 2025 16:35
@rasheedamir
Copy link
Member

@ZanisO can you plz pull changes from main and update your PR? and also update the readme; we will start its review next week.

Copy link
Contributor

@Felix-Stakater Felix-Stakater left a 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 {
Copy link
Contributor

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
Copy link
Contributor

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)
Copy link
Contributor

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 {
Copy link
Contributor

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 {
Copy link
Contributor

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.

@rasheedamir
Copy link
Member

@ZanisO are you around?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants