Fix review comments to

1. Undo changes to the charts and deploy directories
2. Add boilerplate to pkg/constants/constants.go
3. Changed logging to entire VolumeContext to just the FSGroup
4. Updated the way empty string is checked
5. changed Infof to InfoS
6. Removed un-necessary/redundant logs

Author: mytreya-rh

charts/secrets-store-csi-driver/crds/secrets-store.csi.x-k8s.io_secretproviderclasspodstatuses.yaml

@@ -41,8 +41,6 @@ spec:
             description: SecretProviderClassPodStatusStatus defines the observed state
               of SecretProviderClassPodStatus
             properties:
-              fsGroup:
-                type: string
               mounted:
                 type: boolean
               objects:

deploy/secrets-store.csi.x-k8s.io_secretproviderclasspodstatuses.yaml

@@ -41,8 +41,6 @@ spec:
             description: SecretProviderClassPodStatusStatus defines the observed state
               of SecretProviderClassPodStatus
             properties:
-              fsGroup:
-                type: string
               mounted:
                 type: boolean
               objects:

pkg/constants/constants.go

@@ -1,3 +1,18 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
 package constants
 
 const (

pkg/rotation/reconciler.go

@@ -401,16 +401,16 @@ func (r *Reconciler) reconcile(ctx context.Context, spcps *secretsstorev1.Secret
 		return fmt.Errorf("failed to lookup provider client: %q", providerName)
 	}
 	gid := constants.NoGID
-	if spcps.Status.FSGroup != "" {
+	if len(spcps.Status.FSGroup) > 0 {
 		gid, err = strconv.ParseInt(spcps.Status.FSGroup, 10, 64)
 		if err != nil {
 			errorReason = internalerrors.FailedToParseFSGroup
-			errStr := fmt.Sprintf("failed to rotate objects for pod %s/%s, err: %v, invalid FSGroup:%s", spcps.Namespace, spcps.Status.PodName, err, spcps.Status.FSGroup)
+			errStr := fmt.Sprintf("failed to rotate objects for pod %s/%s, invalid FSGroup:%s, err: %w ", spcps.Namespace, spcps.Status.PodName, spcps.Status.FSGroup, err)
 			r.generateEvent(pod, corev1.EventTypeWarning, mountRotationFailedReason, errStr)
 			return fmt.Errorf("%s", errStr)
 		}
 	}
-	klog.V(5).Infof("Reconciling pod %s/%s with fsGroup: %v\n", spcps.Namespace, spcps.Status.PodName, gid)
+	klog.V(5).InfoS("updating the secret content", "pod", klog.ObjectRef{Namespace: spcps.Namespace, Name: spcps.Status.PodName}, "FSGroup", gid)
 	newObjectVersions, errorReason, err := secretsstore.MountContent(ctx, providerClient, string(paramsJSON), string(secretsJSON), spcps.Status.TargetPath, string(permissionJSON), oldObjectVersions, gid)
 	if err != nil {
 		r.generateEvent(pod, corev1.EventTypeWarning, mountRotationFailedReason, fmt.Sprintf("provider mount err: %+v", err))

pkg/secrets-store/nodeserver.go

@@ -142,21 +142,21 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 		return &csi.NodePublishVolumeResponse{}, nil
 	}
 
-	klog.V(2).InfoS("node publish volume", "target", targetPath, "volumeId", volumeID, "mount flags", mountFlags)
+	klog.V(2).InfoS("node publish volume", "target", targetPath, "volumeId", volumeID, "mount flags", mountFlags, "volumeCapabilities", req.VolumeCapability.String())
 
-	klog.V(5).InfoS("volume mounted with", "volumeContext", req.VolumeContext, "volumeCapabilities", req.VolumeCapability.String())
 	gid := constants.NoGID // Group ID to Chown the volume contents to
-	if req.VolumeCapability != nil && req.VolumeCapability.GetMount() != nil && req.VolumeCapability.GetMount().GetVolumeMountGroup() != "" {
-		fsGroupStr := req.VolumeCapability.GetMount().GetVolumeMountGroup()
+	mountVol := req.VolumeCapability.GetMount()
+	if len(mountVol.GetVolumeMountGroup()) > 0 {
+		fsGroupStr := mountVol.GetVolumeMountGroup()
 		klog.V(5).Info("fsGroupStr: %v\n", fsGroupStr)
 		gid, err = strconv.ParseInt(fsGroupStr, 10, 64)
 		klog.V(5).Info("converted gid: %v\n", gid)
 		if err != nil {
-			klog.ErrorS(err, "failed to mount secrets store object content", "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName}, "invalid FSGroup: ", fsGroupStr)
+			klog.ErrorS(err, "failed to mount secrets store object content", "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName}, "FSGroup: ", fsGroupStr)
 			return nil, status.Error(codes.InvalidArgument, "Error parsing FSGroup")
 		}
 	} else {
-		klog.V(5).Info("mount group not set")
+		klog.V(5).InfoS("mount group not set", "targetPath", targetPath, "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName})
 	}
 
 	if isMockProvider(providerName) {

pkg/secrets-store/utils.go

@@ -102,11 +102,10 @@ func createOrUpdateSecretProviderClassPodStatus(ctx context.Context, c client.Cl
 	}
 	o = spcpsutil.OrderSecretProviderClassObjectByID(o)
 	fsGroup := ""
-	klog.V(5).Infof("gid: %v for pod: %v/%v", gid, namespace, podname)
 	if gid != constants.NoGID {
 		fsGroup = strconv.FormatInt(gid, 10)
 	}
-	klog.V(5).Infof("gid string: %v for pod: %v/%v", fsGroup, namespace, podname)
+	klog.V(5).InfoS("setting gid in spcps", "pod", klog.ObjectRef{Namespace: namespace, Name: podname}, "gid", gid, "gidStr", fsGroup)
 	spcPodStatus := &secretsstorev1.SecretProviderClassPodStatus{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:      spcpsName,
@@ -133,7 +132,6 @@ func createOrUpdateSecretProviderClassPodStatus(ctx context.Context, c client.Cl
 			UID:        types.UID(podUID),
 		},
 	})
-	klog.V(5).InfoS("creating", "spcPodStatus", spcPodStatus)
 	if err = c.Create(ctx, spcPodStatus); err == nil || !apierrors.IsAlreadyExists(err) {
 		return err
 	}

test/e2eprovider/server/server.go

@@ -167,7 +167,7 @@ func (s *Server) Mount(ctx context.Context, req *v1alpha1.MountRequest) (*v1alph
 		}
 
 		klog.InfoS("Secret Object with", "name", mockSecretsStoreObject.ObjectName, "permission", mockSecretsStoreObject.FilePermission)
-		if mockSecretsStoreObject.FilePermission != "" {
+		if len(mockSecretsStoreObject.FilePermission) > 0 {
 			mode, err := strconv.ParseUint(mockSecretsStoreObject.FilePermission, 8, 32)
 			if err != nil || mode > 511 {
 				return nil, fmt.Errorf("invalid filePermission: %s, error: %w for file: %s", mockSecretsStoreObject.FilePermission, err, mockSecretsStoreObject.ObjectName)