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 @@ import (
 	"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,8 @@ const (
 
 var (
 	defaultPxSaTokenExpirationSeconds = int64(12 * 60 * 60)
+	// Accessible for testing purpose
+	RootCaCrtPath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
 )
 
 type portworxBasic struct {
@@ -557,23 +559,23 @@ func (c *portworxBasic) createAndMaintainPxSaTokenSecret(cluster *corev1.Storage
 			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 +584,7 @@ func (c *portworxBasic) createTokenSecret(cluster *corev1.StorageCluster, ownerR
 		},
 		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 +593,47 @@ func (c *portworxBasic) createTokenSecret(cluster *corev1.StorageCluster, ownerR
 	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 +642,11 @@ func (c *portworxBasic) refreshTokenSecret(secret *v1.Secret, cluster *corev1.St
 	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 +663,6 @@ func generatePxSaToken(cluster *corev1.StorageCluster, expirationSeconds int64)
 	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,

drivers/storage/portworx/components_test.go

@@ -4,7 +4,9 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io/fs"
 	"math/rand"
+	"os"
 	"strconv"
 	"strings"
 	"testing"
@@ -1248,12 +1250,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 +1273,50 @@ func TestServiceAccountTokenRefreshOnExpire(t *testing.T) {
 	assert.True(t, updatedExpirationTime.Sub(fakeExpirationTime) > 6*time.Hour)
 }
 
+func TestUpdateServiceAccountTokenSecretCaCrt(t *testing.T) {
+	// 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(component.RootCaCrtPath)
+	require.NoError(t, err)
+	file.Close()
+
+	mockCtrl := gomock.NewController(t)
+	setUpMockCoreOps(mockCtrl, fakek8sclient.NewSimpleClientset())
+	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(component.RootCaCrtPath, []byte("test"), 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), "test")
+}
+
 func TestDefaultStorageClassesWithStork(t *testing.T) {
 	mockCtrl := gomock.NewController(t)
 	versionClient := fakek8sclient.NewSimpleClientset()