PWX-38064 Update ca.crt in the PX ServiceAccount Secret if updated

drivers/storage/portworx/component/portworx_basic.go

@@ -1,6 +1,7 @@
 package component
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 	"os"
@@ -19,7 +20,6 @@
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/intstr"
 	"k8s.io/client-go/tools/record"
-	"k8s.io/kubernetes/pkg/apis/core"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	pxutil "github.com/libopenstorage/operator/drivers/storage/portworx/util"
@@ -46,6 +46,7 @@
 
 var (
 	defaultPxSaTokenExpirationSeconds = int64(12 * 60 * 60)
+	rootCaCrtPath                     = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
 )
 
 type portworxBasic struct {
@@ -557,23 +558,23 @@
 			return err
 		}
 	}
-	needRefresh, err := isTokenRefreshRequired(secret)
+	caCrtUpdated, err := updateCaCrtIfNeeded(secret)
 	if err != nil {
 		return err
 	}
-	if needRefresh {
-		if err := c.refreshTokenSecret(secret, cluster, ownerRef); err != nil {
-			return fmt.Errorf("failed to refresh the token secret for px container: %w", err)
+	tokenRefreshed, err := refreshTokenIfNeeded(secret, cluster)
+	if err != nil {
+		return err
+	}
+	if caCrtUpdated || tokenRefreshed {
+		if err := k8sutil.CreateOrUpdateSecret(c.k8sClient, secret, ownerRef); err != nil {
+			return err
 		}
 	}
 	return nil
 }
 
 func (c *portworxBasic) createTokenSecret(cluster *corev1.StorageCluster, ownerRef *metav1.OwnerReference) (*v1.Secret, error) {
-	rootCaCrt, err := os.ReadFile("/var/run/secrets/kubernetes.io/serviceaccount/ca.crt")
-	if err != nil && !os.IsNotExist(err) {
-		return nil, fmt.Errorf("error reading k8s cluster certificate located inside the pod at /var/run/secrets/kubernetes.io/serviceaccount/ca.crt: %w", err)
-	}
 	secret := &v1.Secret{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:            pxutil.PortworxServiceAccountTokenSecretName,
@@ -582,8 +583,7 @@
 		},
 		Type: v1.SecretTypeOpaque,
 		Data: map[string][]byte{
-			core.ServiceAccountRootCAKey:    rootCaCrt,
-			core.ServiceAccountNamespaceKey: []byte(cluster.Namespace),
+			v1.ServiceAccountNamespaceKey: []byte(cluster.Namespace),
 		},
 	}
 	if err := k8sutil.CreateOrUpdateSecret(c.k8sClient, secret, ownerRef); err != nil {
@@ -592,7 +592,47 @@
 	return secret, nil
 }
 
-func (c *portworxBasic) refreshTokenSecret(secret *v1.Secret, cluster *corev1.StorageCluster, ownerRef *metav1.OwnerReference) error {
+func updateCaCrtIfNeeded(secret *v1.Secret) (bool, error) {
+	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 %s: %w", rootCaCrtPath, err)
+	}
+	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
+	}
+	expirationTime, err := time.Parse(time.RFC3339, string(secret.Data[PxSaTokenRefreshTimeKey]))
+	if err != nil {
+		return false, fmt.Errorf("error parsing expiration time: %w", err)
+	}
+	if time.Now().UTC().After(expirationTime) {
+		return true, nil
+	}
+	return false, nil
+}
+
+func refreshToken(secret *v1.Secret, cluster *corev1.StorageCluster) error {
 	expirationSeconds, err := getPxSaTokenExpirationSeconds(cluster)
 	if err != nil {
 		return err
@@ -601,15 +641,11 @@
 	if err != nil {
 		return err
 	}
-	secret.Data[core.ServiceAccountTokenKey] = []byte(newToken.Status.Token)
+	secret.Data[v1.ServiceAccountTokenKey] = []byte(newToken.Status.Token)
 	// ServiceAccount token expiration time we defined might get overwritten by the maxExpirationSeconds defined by the k8s token RESTful server,
 	// so our token refresh machanism has to honor this server limit.
 	// https://github.com/kubernetes/kubernetes/blob/79fee524e65ddc7c1448d5d2554c6f91233cf98d/pkg/registry/core/serviceaccount/storage/token.go#L208
 	secret.Data[PxSaTokenRefreshTimeKey] = []byte(time.Now().UTC().Add(time.Duration(*newToken.Spec.ExpirationSeconds/2) * time.Second).Format(time.RFC3339))
-	err = k8sutil.CreateOrUpdateSecret(c.k8sClient, secret, ownerRef)
-	if err != nil {
-		return err
-	}
 	return nil
 }
 
@@ -626,20 +662,6 @@
 	return tokenResp, nil
 }
 
-func isTokenRefreshRequired(secret *v1.Secret) (bool, error) {
-	if len(secret.Data) == 0 || len(secret.Data[v1.ServiceAccountTokenKey]) == 0 {
-		return true, nil
-	}
-	expirationTime, err := time.Parse(time.RFC3339, string(secret.Data[PxSaTokenRefreshTimeKey]))
-	if err != nil {
-		return false, fmt.Errorf("error parsing expiration time: %w", err)
-	}
-	if time.Now().UTC().After(expirationTime) {
-		return true, nil
-	}
-	return false, nil
-}
-
 func (c *portworxBasic) createPortworxKVDBService(
 	cluster *corev1.StorageCluster,
 	ownerRef *metav1.OwnerReference,
@@ -728,6 +750,11 @@
 	return defaultPxSaTokenExpirationSeconds, nil
 }
 
+// Set the path of k8s cluster root certificate for the purpose of testing
+func SetRootCertPath(path string) {
+	rootCaCrtPath = path
+}
+
 // RegisterPortworxBasicComponent registers the Portworx Basic component
 func RegisterPortworxBasicComponent() {
 	Register(PortworxBasicComponentName, &portworxBasic{})

drivers/storage/portworx/components_test.go

@@ -4,7 +4,9 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io/fs"
 	"math/rand"
+	"os"
 	"strconv"
 	"strings"
 	"testing"
@@ -58,6 +60,8 @@ import (
 	testutil "github.com/libopenstorage/operator/pkg/util/test"
 )
 
+const fakeRootCertPath = "/tmp/ca.crt"
+
 // For some reason simpleClientset doesn't work with createToken, erroring out with serviceaccounts "" not found.
 // Thus wrap the simpleClientset with MockCoreOps.
 func setUpMockCoreOps(mockCtrl *gomock.Controller, clientset *fakek8sclient.Clientset) *mockcore.MockOps {
@@ -91,6 +95,19 @@ func setUpMockCoreOps(mockCtrl *gomock.Controller, clientset *fakek8sclient.Clie
 	return mockCoreOps
 }
 
+// Set root CA certificate path to a safe place
+func setUpFakeRootCert(t *testing.T) {
+	component.SetRootCertPath(fakeRootCertPath)
+	rootCaCrtDir := "/tmp"
+	err := os.MkdirAll(rootCaCrtDir, fs.ModePerm)
+	require.NoError(t, err)
+	file, err := os.Create(fakeRootCertPath)
+	require.NoError(t, err)
+	file.Close()
+	err = os.WriteFile(fakeRootCertPath, []byte("test"), 0644)
+	require.NoError(t, err)
+}
+
 func TestOrderOfComponents(t *testing.T) {
 	reregisterComponents()
 	component.RegisterDisruptionBudgetComponent()
@@ -241,6 +258,7 @@ func TestBasicComponentsInstallWithPreTLSPx(t *testing.T) {
 func TestBasicComponentsInstall(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	setUpMockCoreOps(mockCtrl, fakek8sclient.NewSimpleClientset())
+	setUpFakeRootCert(t)
 	logrus.SetLevel(logrus.TraceLevel)
 	reregisterComponents()
 	k8sClient := testutil.FakeK8sClient()
@@ -1248,12 +1266,6 @@ func TestServiceAccountTokenRefreshOnExpire(t *testing.T) {
 			Name:      "px-cluster",
 			Namespace: "kube-test",
 		},
-		Spec: corev1.StorageClusterSpec{
-			Stork: &corev1.StorkSpec{
-				Enabled: true,
-			},
-			Image: "portworx/image:2.10.1",
-		},
 	}
 	err = driver.PreInstall(cluster)
 	require.NoError(t, err)
@@ -1277,6 +1289,42 @@ func TestServiceAccountTokenRefreshOnExpire(t *testing.T) {
 	assert.True(t, updatedExpirationTime.Sub(fakeExpirationTime) > 6*time.Hour)
 }
 
+func TestUpdateServiceAccountTokenSecretCaCrt(t *testing.T) {
+	mockCtrl := gomock.NewController(t)
+	setUpMockCoreOps(mockCtrl, fakek8sclient.NewSimpleClientset())
+	setUpFakeRootCert(t)
+	reregisterComponents()
+	k8sClient := testutil.FakeK8sClient()
+	driver := portworx{}
+	err := driver.Init(k8sClient, runtime.NewScheme(), record.NewFakeRecorder(0))
+	require.NoError(t, err)
+
+	cluster := &corev1.StorageCluster{
+		ObjectMeta: metav1.ObjectMeta{
+			Name:      "px-cluster",
+			Namespace: "kube-test",
+		},
+	}
+	err = driver.PreInstall(cluster)
+	require.NoError(t, err)
+
+	saTokenSecret := &v1.Secret{}
+	err = testutil.Get(k8sClient, saTokenSecret, pxutil.PortworxServiceAccountTokenSecretName, cluster.Namespace)
+	require.NoError(t, err)
+	oldCaCrt := saTokenSecret.Data[v1.ServiceAccountRootCAKey]
+
+	err = os.WriteFile(fakeRootCertPath, []byte("newtest"), 0644)
+	require.NoError(t, err)
+
+	err = driver.PreInstall(cluster)
+	require.NoError(t, err)
+	err = testutil.Get(k8sClient, saTokenSecret, pxutil.PortworxServiceAccountTokenSecretName, cluster.Namespace)
+	require.NoError(t, err)
+	newCaCrt := saTokenSecret.Data[v1.ServiceAccountRootCAKey]
+	require.NotEqual(t, oldCaCrt, newCaCrt)
+	require.Equal(t, string(newCaCrt), "newtest")
+}
+
 func TestDefaultStorageClassesWithStork(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	versionClient := fakek8sclient.NewSimpleClientset()