Skip to content

Commit

Permalink
PWX-38064 Update ca.crt in the PX ServiceAccount Secret if updated (#…
Browse files Browse the repository at this point in the history 
…1606)

* update ca.crt in the secret if updated

Signed-off-by: shsun_pure <shsun@purestorage.com>

* address comments

* fix test, add setter for ca cert

---------

Signed-off-by: shsun_pure <shsun@purestorage.com>
Co-authored-by: shsun_pure <shsun@purestorage.com>
  • Loading branch information
@ssz1997
ssz1997 and shsun_pure committed Jul 29, 2024
1 parent c77e224 commit f8c8f04
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 37 deletions.
89 changes: 58 additions & 31 deletions drivers/storage/portworx/component/portworx_basic.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package component

import (
"bytes"
"context"
"fmt"
"os"
Expand All @@ -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"
Expand All @@ -46,6 +46,7 @@ const (

var (
defaultPxSaTokenExpirationSeconds = int64(12 * 60 * 60)
rootCaCrtPath = "/var/run/secrets/kubernetes.io/serviceaccount/ca.crt"
)

type portworxBasic struct {
Expand Down Expand Up @@ -540,23 +541,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,
Expand All @@ -565,8 +566,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 {
Expand All @@ -575,7 +575,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
Expand All @@ -584,15 +624,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
}

Expand All @@ -609,20 +645,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,
Expand Down Expand Up @@ -709,6 +731,11 @@ func getPxSaTokenExpirationSeconds(cluster *corev1.StorageCluster) (int64, error
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{})
Expand Down
58 changes: 52 additions & 6 deletions drivers/storage/portworx/components_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"bytes"
"context"
"fmt"
"io/fs"
"math/rand"
"net"
"os"
Expand Down Expand Up @@ -64,6 +65,7 @@ import (

const (
etcHostsFile = "/etc/hosts"
fakeRootCertPath = "/tmp/ca.crt"
tempEtcHostsMarker = "### px-operator unit-test"
)

Expand Down Expand Up @@ -100,6 +102,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()
Expand Down Expand Up @@ -153,6 +168,7 @@ func TestOrderOfComponents(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()
Expand Down Expand Up @@ -1076,12 +1092,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)
Expand All @@ -1105,6 +1115,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()
Expand Down

0 comments on commit f8c8f04

Please sign in to comment.