From 5c7d33316a6a11af8ad2715ee6349fd1a866f383 Mon Sep 17 00:00:00 2001
From: shsun_pure <shsun@purestorage.com>
Date: Fri, 12 Jul 2024 22:37:55 +0000
Subject: [PATCH] address comments

---
 .../portworx/component/portworx_basic.go      | 33 ++++++++++++-------
 drivers/storage/portworx/components_test.go   | 11 ++++---
 2 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/storage/portworx/component/portworx_basic.go b/drivers/storage/portworx/component/portworx_basic.go
index b1110ea14..95272a247 100644
--- a/drivers/storage/portworx/component/portworx_basic.go
+++ b/drivers/storage/portworx/component/portworx_basic.go
@@ -46,6 +46,8 @@ const (
 
 var (
 	defaultPxSaTokenExpirationSeconds = int64(12 * 60 * 60)
+	// Accessible for testing purpose
+	RootCaCrtPath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
 )
 
 type portworxBasic struct {
@@ -561,16 +563,11 @@ func (c *portworxBasic) createAndMaintainPxSaTokenSecret(cluster *corev1.Storage
 	if err != nil {
 		return err
 	}
-	needRefreshToken, err := isTokenRefreshRequired(secret)
+	tokenRefreshed, err := refreshTokenIfNeeded(secret, cluster)
 	if err != nil {
 		return err
 	}
-	if needRefreshToken {
-		if err := c.refreshToken(secret, cluster); err != nil {
-			return fmt.Errorf("failed to refresh the token secret for px container: %w", err)
-		}
-	}
-	if caCrtUpdated || needRefreshToken {
+	if caCrtUpdated || tokenRefreshed {
 		if err := k8sutil.CreateOrUpdateSecret(c.k8sClient, secret, ownerRef); err != nil {
 			return err
 		}
@@ -597,17 +594,31 @@ func (c *portworxBasic) createTokenSecret(cluster *corev1.StorageCluster, ownerR
 }
 
 func updateCaCrtIfNeeded(secret *v1.Secret) (bool, error) {
-	rootCaCrt, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/ca.crt")
+	rootCaCrt, err := os.ReadFile(RootCaCrtPath)
 	if err != nil && !os.IsNotExist(err) {
-		return false, fmt.Errorf("error reading k8s cluster certificate located inside the pod at /var/run/secrets/kubernetes.io/serviceaccount/ca.crt: %w", err)
+		return false, fmt.Errorf("error reading k8s cluster certificate located inside the pod at %s: %w", RootCaCrtPath, err)
 	}
-	if len(secret.Data) == 0 || len(secret.Data[v1.ServiceAccountRootCAKey]) == 0 || !bytes.Equal(secret.Data[v1.ServiceAccountRootCAKey], rootCaCrt) {
+	if len(secret.Data) == 0 || !bytes.Equal(secret.Data[v1.ServiceAccountRootCAKey], rootCaCrt) {
 		secret.Data[v1.ServiceAccountRootCAKey] = rootCaCrt
 		return true, nil
 	}
 	return false, nil
 }
 
+func refreshTokenIfNeeded(secret *v1.Secret, cluster *corev1.StorageCluster) (bool, error) {
+	needRefreshToken, err := isTokenRefreshRequired(secret)
+	if err != nil {
+		return false, err
+	}
+	if needRefreshToken {
+		if err := refreshToken(secret, cluster); err != nil {
+			return false, fmt.Errorf("failed to refresh the token secret for px container: %w", err)
+		}
+		return true, nil
+	}
+	return false, nil
+}
+
 func isTokenRefreshRequired(secret *v1.Secret) (bool, error) {
 	if len(secret.Data) == 0 || len(secret.Data[v1.ServiceAccountTokenKey]) == 0 {
 		return true, nil
@@ -622,7 +633,7 @@ func isTokenRefreshRequired(secret *v1.Secret) (bool, error) {
 	return false, nil
 }
 
-func (c *portworxBasic) refreshToken(secret *v1.Secret, cluster *corev1.StorageCluster) error {
+func refreshToken(secret *v1.Secret, cluster *corev1.StorageCluster) error {
 	expirationSeconds, err := getPxSaTokenExpirationSeconds(cluster)
 	if err != nil {
 		return err
diff --git a/drivers/storage/portworx/components_test.go b/drivers/storage/portworx/components_test.go
index 0ffb5bda3..bc7032f7f 100644
--- a/drivers/storage/portworx/components_test.go
+++ b/drivers/storage/portworx/components_test.go
@@ -1274,11 +1274,12 @@ func TestServiceAccountTokenRefreshOnExpire(t *testing.T) {
 }
 
 func TestUpdateServiceAccountTokenSecretCaCrt(t *testing.T) {
-	caCrtDir := "/var/run/secrets/kubernetes.io/serviceaccount"
-	caCrtPath := "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
-	err := os.MkdirAll(caCrtDir, fs.ModePerm)
+	// Set root CA certificate path to a safe place
+	component.RootCaCrtPath = "/tmp/ca.crt"
+	rootCaCrtDir := "/tmp"
+	err := os.MkdirAll(rootCaCrtDir, fs.ModePerm)
 	require.NoError(t, err)
-	file, err := os.Create(caCrtPath)
+	file, err := os.Create(component.RootCaCrtPath)
 	require.NoError(t, err)
 	file.Close()
 
@@ -1304,7 +1305,7 @@ func TestUpdateServiceAccountTokenSecretCaCrt(t *testing.T) {
 	require.NoError(t, err)
 	oldCaCrt := saTokenSecret.Data[v1.ServiceAccountRootCAKey]
 
-	err = os.WriteFile(caCrtPath, []byte("test"), 0644)
+	err = os.WriteFile(component.RootCaCrtPath, []byte("test"), 0644)
 	require.NoError(t, err)
 
 	err = driver.PreInstall(cluster)