Fix the comments, logs and delete unused configs

Author: dargudear-google

Makefile

@@ -532,15 +532,6 @@ manifests: $(CONTROLLER_GEN) $(KUSTOMIZE)  ## Generate manifests e.g. CRD, RBAC
 	@sed -i '1s/^/{{ if .Values.enableSecretRotation }}\n/gm; s/namespace: .*/namespace: {{ .Release.Namespace }}/gm; $$s/$$/\n{{ end }}/gm' manifest_staging/charts/secrets-store-csi-driver/templates/role-rotation_binding.yaml
 	@sed -i '/^roleRef:/i \ \ labels:\n{{ include \"sscd.labels\" . | indent 4 }}' manifest_staging/charts/secrets-store-csi-driver/templates/role-rotation_binding.yaml
 
-	# Generate token requests specific RBAC
-	$(CONTROLLER_GEN) rbac:roleName=secretprovidertokenrequest-role paths="./controllers/tokenrequest" output:dir=config/rbac-tokenrequest
-	$(KUSTOMIZE) build config/rbac-tokenrequest -o manifest_staging/deploy/rbac-secretprovidertokenrequest.yaml
-	cp config/rbac-tokenrequest/role.yaml manifest_staging/charts/secrets-store-csi-driver/templates/role-tokenrequest.yaml
-	cp config/rbac-tokenrequest/role_binding.yaml manifest_staging/charts/secrets-store-csi-driver/templates/role-tokenrequest_binding.yaml
-	@sed -i '1s/^/{{ if .Values.tokenRequests }}\n/gm; $$s/$$/\n{{ end }}/gm' manifest_staging/charts/secrets-store-csi-driver/templates/role-tokenrequest.yaml
-	@sed -i '/^rules:/i \ \ labels:\n{{ include \"sscd.labels\" . | indent 4 }}' manifest_staging/charts/secrets-store-csi-driver/templates/role-tokenrequest.yaml
-	@sed -i '1s/^/{{ if .Values.tokenRequests }}\n/gm; s/namespace: .*/namespace: {{ .Release.Namespace }}/gm; $$s/$$/\n{{ end }}/gm' manifest_staging/charts/secrets-store-csi-driver/templates/role-tokenrequest_binding.yaml
-	@sed -i '/^roleRef:/i \ \ labels:\n{{ include \"sscd.labels\" . | indent 4 }}' manifest_staging/charts/secrets-store-csi-driver/templates/role-tokenrequest_binding.yaml
 
 .PHONY: generate-protobuf
 generate-protobuf: $(PROTOC) $(PROTOC_GEN_GO) $(PROTOC_GEN_GO_GRPC) # generates protobuf

config/rbac-tokenrequest/kustomization.yaml

@@ -397,7 +397,7 @@ func (r *SecretProviderClassPodStatusReconciler) processIfBelongsToNode(objMeta
 }
 
 // createOrUpdateK8sSecret creates K8s secret with data from mounted files
-// If a secret with the same name already exists in the namespace of the pod, the error is nil.
+// If a secret with the same name already exists in the namespace of the pod, it will update that existing secret.
 func (r *SecretProviderClassPodStatusReconciler) createOrUpdateK8sSecret(ctx context.Context, name, namespace string, datamap map[string][]byte, labelsmap map[string]string, annotationsmap map[string]string, secretType corev1.SecretType) error {
 	secret := &corev1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
@@ -415,17 +415,18 @@ func (r *SecretProviderClassPodStatusReconciler) createOrUpdateK8sSecret(ctx con
 		klog.InfoS("successfully created Kubernetes secret", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
 		return nil
 	}
-	if apierrors.IsAlreadyExists(err) {
-		klog.InfoS("Kubernetes secret is already created", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
-		err := r.writer.Update(ctx, secret)
-		if err != nil {
-			klog.ErrorS(err, "Unable to update kubernetes secret", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
-			return err
-		}
-		klog.InfoS("successfully updated Kubernetes secret", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
-		return nil
+	if !apierrors.IsAlreadyExists(err) {
+		return err
 	}
-	return err
+
+	klog.InfoS("Kubernetes secret is already created", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
+	err = r.writer.Update(ctx, secret)
+	if err != nil {
+		klog.ErrorS(err, "Unable to update kubernetes secret", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
+		return err
+	}
+	klog.InfoS("successfully updated Kubernetes secret", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
+	return nil
 }
 
 // patchSecretWithOwnerRef patches the secret owner reference with the spc pod status

config/rbac-tokenrequest/role.yaml

@@ -14,4 +14,5 @@ spec:
   requiresRepublish: true
   tokenRequests:
     {{- toYaml .Values.tokenRequests | nindent 2 }}
+  requiresRepublish: {{ .Values.requiresRepublish }}
   {{- end }}

config/rbac-tokenrequest/role_binding.yaml

@@ -239,6 +239,10 @@ tokenRequests: []
 # - audience: aud1
 # - audience: aud2
 
+# To set the requiresRepublish which can be used to refresh the mounted secret periodically
+# refer to https://kubernetes-csi.github.io/docs/token-requests.html for more details.
+# Supported only for Kubernetes v1.20+
+requiresRepublish: true
 # -- Labels to apply to all resources
 commonLabels: {}
 # team_name: dev

controllers/secretproviderclasspodstatus_controller.go

@@ -80,8 +80,9 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	defer func() {
 		if err != nil {
 			// if there is an error at any stage during node publish volume and if the path
-			// has already been mounted, unmount the target path so the next time kubelet calls
+			// has already been mounted if the rotation is disabled, unmount the target path so the next time kubelet calls
 			// again for mount, entire node publish volume is retried
+			// If the rotation is enabled the target path won't be unmounted just the next call will try to mount the content.
 			if targetPath != "" && mounted && !isRemountRequest {
 				klog.InfoS("unmounting target path as node publish volume failed", "targetPath", targetPath, "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName})
 				if unmountErr := ns.mounter.Unmount(targetPath); unmountErr != nil {
@@ -120,16 +121,6 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	podNamespace = attrib[CSIPodNamespace]
 	podUID = attrib[CSIPodUID]
 
-	if ns.rotationConfig.enabled {
-		lastModificationTime, err := ns.getLastUpdateTime(targetPath)
-		if err != nil {
-			klog.InfoS("could not find last modification time for targetpath", targetPath, "error", err)
-		} else if startTime.Before(lastModificationTime.Add(ns.rotationConfig.rotationPollInterval)) {
-			// if next rotation is not yet due, then skip the mount operation
-			return &csi.NodePublishVolumeResponse{}, nil
-		}
-	}
-
 	mounted, err = ns.ensureMountPoint(targetPath)
 	if err != nil {
 		// kubelet will not create the CSI NodePublishVolume target directory in 1.20+, in accordance with the CSI specification.
@@ -152,6 +143,16 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 		return &csi.NodePublishVolumeResponse{}, nil
 	}
 
+	if rotationEnabled {
+		lastModificationTime, err := ns.getLastUpdateTime(targetPath)
+		if err != nil {
+			klog.InfoS("could not find last modification time for targetpath", targetPath, "error", err)
+		} else if startTime.Before(lastModificationTime.Add(ns.rotationConfig.rotationPollInterval)) {
+			// if next rotation is not yet due, then skip the mount operation
+			return &csi.NodePublishVolumeResponse{}, nil
+		}
+	}
+
 	klog.V(2).InfoS("node publish volume", "target", targetPath, "volumeId", volumeID, "mount flags", mountFlags)
 
 	if isMockProvider(providerName) {
@@ -200,7 +201,7 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
 	// and send it to the provider in the parameters.
 	if parameters[CSIPodServiceAccountTokens] == "" {
 		// Inject pod service account token into volume attributes
-		klog.ErrorS(err, "csi.storage.k8s.io/serviceAccount.tokens is not populated, set RequiresRepublish")
+		klog.Info(err, "csi.storage.k8s.io/serviceAccount.tokens is not populated")
 	}
 
 	// ensure it's read-only