From aa520a1763aa113b1b8206b66eb537d0b158892d Mon Sep 17 00:00:00 2001 From: Mikhail Volkov Date: Thu, 7 May 2026 18:48:21 +0300 Subject: [PATCH 1/2] node publish secret ref added --- driver/nodeserver.go | 2 ++ manager/manager.go | 23 +++++++++++++++++++++++ metadata/metadata.go | 6 ++++++ 3 files changed, 31 insertions(+) diff --git a/driver/nodeserver.go b/driver/nodeserver.go index baed4baa..fc58ada2 100644 --- a/driver/nodeserver.go +++ b/driver/nodeserver.go @@ -90,6 +90,8 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis } } + ns.manager.StoreVolumeSecrets(req.GetVolumeId(), req.GetSecrets()) + if !ns.manager.IsVolumeReady(req.GetVolumeId()) { // Only wait for the volume to be ready if it is in a state of 'ready to request' // already. This allows implementors to defer actually requesting certificates diff --git a/manager/manager.go b/manager/manager.go index 48878036..9ce87f34 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -254,6 +254,7 @@ func NewManager(opts Options) (*Manager, error) { readyToRequest: opts.ReadyToRequest, managedVolumes: map[string]chan struct{}{}, + volumeSecrets: map[string]map[string]string{}, stopInformer: stopCh, maxRequestsPerVolume: opts.MaxRequestsPerVolume, @@ -353,6 +354,12 @@ type Manager struct { // volume managedVolumes map[string]chan struct{} + // volumeSecrets stores secrets from nodePublishSecretRef per volume in-memory. + // Secrets are not persisted to disk, so they must be re-registered on each + // NodePublishVolume call and kept alive here for background renewals. + volumeSecrets map[string]map[string]string + volumeSecretsLock sync.Mutex + // Used to stop the informer watching for updates stopInformer chan struct{} @@ -395,6 +402,9 @@ func (m *Manager) issue(ctx context.Context, volumeID string) error { if err != nil { return fmt.Errorf("reading metadata: %w", err) } + m.volumeSecretsLock.Lock() + meta.Secrets = m.volumeSecrets[volumeID] + m.volumeSecretsLock.Unlock() log.V(2).Info("Read metadata", "metadata", meta) // check if there is already a pending request in-flight for this volume. @@ -883,6 +893,19 @@ func (m *Manager) UnmanageVolume(volumeID string) { close(stopCh) delete(m.managedVolumes, volumeID) } + + m.volumeSecretsLock.Lock() + delete(m.volumeSecrets, volumeID) + m.volumeSecretsLock.Unlock() +} + +// StoreVolumeSecrets stores secrets from nodePublishSecretRef for the given volume. +// Must be called before ManageVolume/ManageVolumeImmediate so that secrets are +// available during initial issuance and background renewals. +func (m *Manager) StoreVolumeSecrets(volumeID string, secrets map[string]string) { + m.volumeSecretsLock.Lock() + defer m.volumeSecretsLock.Unlock() + m.volumeSecrets[volumeID] = secrets } func (m *Manager) IsVolumeReadyToRequest(volumeID string) (bool, string) { diff --git a/metadata/metadata.go b/metadata/metadata.go index 788db10c..722cf99a 100644 --- a/metadata/metadata.go +++ b/metadata/metadata.go @@ -41,6 +41,11 @@ type Metadata struct { // VolumeMountGroup is the filesystem group that the volume should be mounted as. VolumeMountGroup string `json:"volumeMountGroup,omitempty"` + + // Secrets contains data from the secret referenced by nodePublishSecretRef. + // Not persisted to disk — repopulated on every NodePublishVolume call and + // kept in-memory by the manager for renewals. + Secrets map[string]string `json:"-"` } // FromNodePublishVolumeRequest constructs a Metadata from a NodePublishVolumeRequest. @@ -51,5 +56,6 @@ func FromNodePublishVolumeRequest(request *csi.NodePublishVolumeRequest) Metadat TargetPath: request.GetTargetPath(), VolumeContext: request.GetVolumeContext(), VolumeMountGroup: request.GetVolumeCapability().GetMount().GetVolumeMountGroup(), + Secrets: request.GetSecrets(), } } From 696a943d355d9bf4170887df25d4b4836b283b2e Mon Sep 17 00:00:00 2001 From: Mikhail Volkov Date: Fri, 8 May 2026 09:22:54 +0300 Subject: [PATCH 2/2] add tests --- manager/manager_secrets_test.go | 180 +++++++++++++++++++++++++++++++ test/integration/secrets_test.go | 141 ++++++++++++++++++++++++ 2 files changed, 321 insertions(+) create mode 100644 manager/manager_secrets_test.go create mode 100644 test/integration/secrets_test.go diff --git a/manager/manager_secrets_test.go b/manager/manager_secrets_test.go new file mode 100644 index 00000000..b527b6d9 --- /dev/null +++ b/manager/manager_secrets_test.go @@ -0,0 +1,180 @@ +/* +Copyright 2021 The cert-manager Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package manager + +import ( + "crypto" + "crypto/x509" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + fakeclock "k8s.io/utils/clock/testing" + + "github.com/cert-manager/csi-lib/metadata" + "github.com/cert-manager/csi-lib/storage" + testutil "github.com/cert-manager/csi-lib/test/util" +) + +func TestManager_StoreVolumeSecrets(t *testing.T) { + opts := newDefaultTestOptions(t) + m, err := NewManager(opts) + require.NoError(t, err) + defer m.Stop() + + secrets := map[string]string{"pkcs12-password": "test-pass", "other-key": "other-value"} + m.StoreVolumeSecrets("vol-id", secrets) + + m.volumeSecretsLock.Lock() + got := m.volumeSecrets["vol-id"] + m.volumeSecretsLock.Unlock() + + assert.Equal(t, secrets, got) +} + +func TestManager_StoreVolumeSecrets_overwritesPreviousSecrets(t *testing.T) { + opts := newDefaultTestOptions(t) + m, err := NewManager(opts) + require.NoError(t, err) + defer m.Stop() + + m.StoreVolumeSecrets("vol-id", map[string]string{"key": "old-value"}) + m.StoreVolumeSecrets("vol-id", map[string]string{"key": "new-value"}) + + m.volumeSecretsLock.Lock() + got := m.volumeSecrets["vol-id"] + m.volumeSecretsLock.Unlock() + + assert.Equal(t, map[string]string{"key": "new-value"}, got) +} + +func TestManager_UnmanageVolume_cleansUpSecrets(t *testing.T) { + opts := newDefaultTestOptions(t) + m, err := NewManager(opts) + require.NoError(t, err) + + store := opts.MetadataReader.(storage.Interface) + meta := metadata.Metadata{VolumeID: "vol-id", TargetPath: "/fake/path"} + _, err = store.RegisterMetadata(meta) + require.NoError(t, err) + defer store.RemoveVolume(meta.VolumeID) + + m.StoreVolumeSecrets("vol-id", map[string]string{"pkcs12-password": "test-pass"}) + m.ManageVolume("vol-id") + m.UnmanageVolume("vol-id") + + m.volumeSecretsLock.Lock() + _, exists := m.volumeSecrets["vol-id"] + m.volumeSecretsLock.Unlock() + + assert.False(t, exists, "secrets should be cleaned up after UnmanageVolume") +} + +func TestManager_SecretsInjectedIntoCallbacks(t *testing.T) { + ctx := t.Context() + + var capturedSecrets map[string]string + + store := storage.NewMemoryFS() + clk := fakeclock.NewFakeClock(time.Now()) + + opts := defaultTestOptions(t, Options{ + MetadataReader: store, + Clock: clk, + WriteKeypair: func(meta metadata.Metadata, key crypto.PrivateKey, chain []byte, ca []byte) error { + capturedSecrets = meta.Secrets + store.WriteFiles(meta, map[string][]byte{"ca": ca, "cert": chain}) + nextIssuanceTime := clk.Now().Add(time.Hour) + meta.NextIssuanceTime = &nextIssuanceTime + return store.WriteMetadata(meta.VolumeID, meta) + }, + }) + + m, err := NewManager(opts) + require.NoError(t, err) + defer m.Stop() + + go testutil.IssueOneRequest(ctx, t, opts.Client, defaultTestNamespace, selfSignedExampleCertificate, []byte("ca bytes")) + + meta := metadata.Metadata{VolumeID: "vol-id", TargetPath: "/fake/path"} + _, err = store.RegisterMetadata(meta) + require.NoError(t, err) + defer func() { + store.RemoveVolume(meta.VolumeID) + m.UnmanageVolume(meta.VolumeID) + }() + + secrets := map[string]string{"pkcs12-password": "test-pass"} + m.StoreVolumeSecrets(meta.VolumeID, secrets) + + _, err = m.ManageVolumeImmediate(ctx, meta.VolumeID) + require.NoError(t, err) + + assert.Equal(t, secrets, capturedSecrets) +} + +func TestManager_SecretsNotInjectedAfterUnmanage(t *testing.T) { + ctx := t.Context() + + var writeKeypairCallCount int + + store := storage.NewMemoryFS() + clk := fakeclock.NewFakeClock(time.Now()) + + opts := defaultTestOptions(t, Options{ + MetadataReader: store, + Clock: clk, + WriteKeypair: func(meta metadata.Metadata, key crypto.PrivateKey, chain []byte, ca []byte) error { + writeKeypairCallCount++ + store.WriteFiles(meta, map[string][]byte{"ca": ca, "cert": chain}) + nextIssuanceTime := clk.Now().Add(time.Hour) + meta.NextIssuanceTime = &nextIssuanceTime + return store.WriteMetadata(meta.VolumeID, meta) + }, + GenerateRequest: func(meta metadata.Metadata) (*CertificateRequestBundle, error) { + return &CertificateRequestBundle{Namespace: defaultTestNamespace}, nil + }, + SignRequest: func(meta metadata.Metadata, key crypto.PrivateKey, req *x509.CertificateRequest) ([]byte, error) { + return []byte{}, nil + }, + }) + + m, err := NewManager(opts) + require.NoError(t, err) + defer m.Stop() + + go testutil.IssueOneRequest(ctx, t, opts.Client, defaultTestNamespace, selfSignedExampleCertificate, []byte("ca bytes")) + + meta := metadata.Metadata{VolumeID: "vol-id", TargetPath: "/fake/path"} + _, err = store.RegisterMetadata(meta) + require.NoError(t, err) + defer store.RemoveVolume(meta.VolumeID) + + m.StoreVolumeSecrets(meta.VolumeID, map[string]string{"pkcs12-password": "test-pass"}) + _, err = m.ManageVolumeImmediate(ctx, meta.VolumeID) + require.NoError(t, err) + + m.UnmanageVolume(meta.VolumeID) + + m.volumeSecretsLock.Lock() + _, exists := m.volumeSecrets[meta.VolumeID] + m.volumeSecretsLock.Unlock() + + assert.False(t, exists, "secrets should not exist after UnmanageVolume") + assert.Equal(t, 1, writeKeypairCallCount, "WriteKeypair should have been called exactly once") +} diff --git a/test/integration/secrets_test.go b/test/integration/secrets_test.go new file mode 100644 index 00000000..86099224 --- /dev/null +++ b/test/integration/secrets_test.go @@ -0,0 +1,141 @@ +/* +Copyright 2021 The cert-manager Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package integration + +import ( + "crypto" + "crypto/x509" + "os" + "testing" + "time" + + "github.com/container-storage-interface/spec/lib/go/csi" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + fakeclock "k8s.io/utils/clock/testing" + + "github.com/cert-manager/csi-lib/manager" + "github.com/cert-manager/csi-lib/metadata" + "github.com/cert-manager/csi-lib/storage" + testdriver "github.com/cert-manager/csi-lib/test/driver" + testutil "github.com/cert-manager/csi-lib/test/util" +) + +func TestNodePublishVolume_secretsAvailableInCallbacks(t *testing.T) { + ctx := t.Context() + + var capturedSecrets map[string]string + + store := storage.NewMemoryFS() + clock := fakeclock.NewFakeClock(time.Now()) + + opts, cl, stop := testdriver.Run(t, testdriver.Options{ + Store: store, + Clock: clock, + GeneratePrivateKey: func(_ metadata.Metadata) (crypto.PrivateKey, error) { + return nil, nil + }, + GenerateRequest: func(_ metadata.Metadata) (*manager.CertificateRequestBundle, error) { + return &manager.CertificateRequestBundle{Namespace: "certificaterequest-namespace"}, nil + }, + SignRequest: func(_ metadata.Metadata, _ crypto.PrivateKey, _ *x509.CertificateRequest) ([]byte, error) { + return []byte{}, nil + }, + WriteKeypair: func(meta metadata.Metadata, _ crypto.PrivateKey, chain []byte, ca []byte) error { + capturedSecrets = meta.Secrets + store.WriteFiles(meta, map[string][]byte{"ca": ca, "cert": chain}) + nextIssuanceTime := clock.Now().Add(time.Hour) + meta.NextIssuanceTime = &nextIssuanceTime + return store.WriteMetadata(meta.VolumeID, meta) + }, + }) + defer stop() + + go testutil.IssueOneRequest(ctx, t, opts.Client, "certificaterequest-namespace", selfSignedExampleCertificate, []byte("ca bytes")) + + tmpDir, err := os.MkdirTemp("", "*") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + + secrets := map[string]string{"pkcs12-password": "my-secret-password"} + + _, err = cl.NodePublishVolume(ctx, &csi.NodePublishVolumeRequest{ + VolumeId: "test-vol", + VolumeContext: map[string]string{ + "csi.storage.k8s.io/ephemeral": "true", + "csi.storage.k8s.io/pod.name": "the-pod-name", + "csi.storage.k8s.io/pod.namespace": "the-pod-namespace", + }, + Secrets: secrets, + TargetPath: tmpDir, + Readonly: true, + }) + require.NoError(t, err) + + assert.Equal(t, secrets, capturedSecrets) +} + +func TestNodePublishVolume_noSecretsWhenRefNotSpecified(t *testing.T) { + ctx := t.Context() + + var capturedSecrets map[string]string + + store := storage.NewMemoryFS() + clock := fakeclock.NewFakeClock(time.Now()) + + opts, cl, stop := testdriver.Run(t, testdriver.Options{ + Store: store, + Clock: clock, + GeneratePrivateKey: func(_ metadata.Metadata) (crypto.PrivateKey, error) { + return nil, nil + }, + GenerateRequest: func(_ metadata.Metadata) (*manager.CertificateRequestBundle, error) { + return &manager.CertificateRequestBundle{Namespace: "certificaterequest-namespace"}, nil + }, + SignRequest: func(_ metadata.Metadata, _ crypto.PrivateKey, _ *x509.CertificateRequest) ([]byte, error) { + return []byte{}, nil + }, + WriteKeypair: func(meta metadata.Metadata, _ crypto.PrivateKey, chain []byte, ca []byte) error { + capturedSecrets = meta.Secrets + store.WriteFiles(meta, map[string][]byte{"ca": ca, "cert": chain}) + nextIssuanceTime := clock.Now().Add(time.Hour) + meta.NextIssuanceTime = &nextIssuanceTime + return store.WriteMetadata(meta.VolumeID, meta) + }, + }) + defer stop() + + go testutil.IssueOneRequest(ctx, t, opts.Client, "certificaterequest-namespace", selfSignedExampleCertificate, []byte("ca bytes")) + + tmpDir, err := os.MkdirTemp("", "*") + require.NoError(t, err) + defer os.RemoveAll(tmpDir) + + _, err = cl.NodePublishVolume(ctx, &csi.NodePublishVolumeRequest{ + VolumeId: "test-vol", + VolumeContext: map[string]string{ + "csi.storage.k8s.io/ephemeral": "true", + "csi.storage.k8s.io/pod.name": "the-pod-name", + "csi.storage.k8s.io/pod.namespace": "the-pod-namespace", + }, + TargetPath: tmpDir, + Readonly: true, + }) + require.NoError(t, err) + + assert.Nil(t, capturedSecrets, "meta.Secrets should be nil when nodePublishSecretRef is not specified") +}