Skip to content

Commit 8d332b7

Browse files
dargudear-googledargudear-google
committed
Use requiresRepublish in staging manifest.
1 parent 3b9d617 commit 8d332b7

File tree

14 files changed

+109
-135
lines changed

14 files changed

+109
-135
lines changed

‎controllers/secretproviderclasspodstatus_controller.go‎

Lines changed: 40 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -295,58 +295,44 @@ func (r *SecretProviderClassPodStatusReconciler) Reconcile(ctx context.Context,
295295
errs = append(errs, fmt.Errorf("failed to validate secret object in spc %s/%s, err: %w", spc.Namespace, spc.Name, err))
296296
continue
297297
}
298-
exists, err := r.secretExists(ctx, secretName, req.Namespace)
299-
if err != nil {
300-
klog.ErrorS(err, "failed to check if secret exists", "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spc", klog.KObj(spc), "pod", klog.KObj(pod), "spcps", klog.KObj(spcPodStatus))
301-
// syncSecret.enabled is set to false by default in the helm chart for installing the driver in v0.0.23+
302-
// that would result in a forbidden error, so generate a warning that can be helpful for debugging
303-
if apierrors.IsForbidden(err) {
304-
klog.Warning(SyncSecretForbiddenWarning)
305-
}
306-
errs = append(errs, fmt.Errorf("failed to check if secret %s exists, err: %w", secretName, err))
307-
continue
308-
}
309298

310299
var funcs []func() (bool, error)
300+
secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type))
311301

312-
if !exists {
313-
secretType := secretutil.GetSecretType(strings.TrimSpace(secretObj.Type))
314-
315-
var datamap map[string][]byte
316-
if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files); err != nil {
317-
r.generateEvent(pod, corev1.EventTypeWarning, secretCreationFailedReason, fmt.Sprintf("failed to get data in spc %s/%s for secret %s, err: %+v", req.Namespace, spcName, secretName, err))
318-
klog.ErrorS(err, "failed to get data in spc for secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus))
319-
errs = append(errs, fmt.Errorf("failed to get data in spc %s/%s for secret %s, err: %w", req.Namespace, spcName, secretName, err))
320-
continue
321-
}
302+
var datamap map[string][]byte
303+
if datamap, err = secretutil.GetSecretData(secretObj.Data, secretType, files); err != nil {
304+
r.generateEvent(pod, corev1.EventTypeWarning, secretCreationFailedReason, fmt.Sprintf("failed to get data in spc %s/%s for secret %s, err: %+v", req.Namespace, spcName, secretName, err))
305+
klog.ErrorS(err, "failed to get data in spc for secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus))
306+
errs = append(errs, fmt.Errorf("failed to get data in spc %s/%s for secret %s, err: %w", req.Namespace, spcName, secretName, err))
307+
continue
308+
}
322309

323-
labelsMap := make(map[string]string)
324-
if secretObj.Labels != nil {
325-
labelsMap = secretObj.Labels
326-
}
327-
annotationsMap := make(map[string]string)
328-
if secretObj.Annotations != nil {
329-
annotationsMap = secretObj.Annotations
330-
}
331-
// Set secrets-store.csi.k8s.io/managed=true label on the secret that's created and managed
332-
// by the secrets-store-csi-driver. This label will be used to perform a filtered list watch
333-
// only on secrets created and managed by the driver
334-
labelsMap[SecretManagedLabel] = "true"
335-
336-
createFn := func() (bool, error) {
337-
if err := r.createK8sSecret(ctx, secretName, req.Namespace, datamap, labelsMap, annotationsMap, secretType); err != nil {
338-
klog.ErrorS(err, "failed to create Kubernetes secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus))
339-
// syncSecret.enabled is set to false by default in the helm chart for installing the driver in v0.0.23+
340-
// that would result in a forbidden error, so generate a warning that can be helpful for debugging
341-
if apierrors.IsForbidden(err) {
342-
klog.Warning(SyncSecretForbiddenWarning)
343-
}
344-
return false, nil
310+
labelsMap := make(map[string]string)
311+
if secretObj.Labels != nil {
312+
labelsMap = secretObj.Labels
313+
}
314+
annotationsMap := make(map[string]string)
315+
if secretObj.Annotations != nil {
316+
annotationsMap = secretObj.Annotations
317+
}
318+
// Set secrets-store.csi.k8s.io/managed=true label on the secret that's created and managed
319+
// by the secrets-store-csi-driver. This label will be used to perform a filtered list watch
320+
// only on secrets created and managed by the driver
321+
labelsMap[SecretManagedLabel] = "true"
322+
323+
createFn := func() (bool, error) {
324+
if err := r.createOrUpdateK8sSecret(ctx, secretName, req.Namespace, datamap, labelsMap, annotationsMap, secretType); err != nil {
325+
klog.ErrorS(err, "failed to create Kubernetes secret", "spc", klog.KObj(spc), "pod", klog.KObj(pod), "secret", klog.ObjectRef{Namespace: req.Namespace, Name: secretName}, "spcps", klog.KObj(spcPodStatus))
326+
// syncSecret.enabled is set to false by default in the helm chart for installing the driver in v0.0.23+
327+
// that would result in a forbidden error, so generate a warning that can be helpful for debugging
328+
if apierrors.IsForbidden(err) {
329+
klog.Warning(SyncSecretForbiddenWarning)
345330
}
346-
return true, nil
331+
return false, nil
347332
}
348-
funcs = append(funcs, createFn)
333+
return true, nil
349334
}
335+
funcs = append(funcs, createFn)
350336

351337
for _, f := range funcs {
352338
if err := wait.ExponentialBackoff(wait.Backoff{
@@ -410,9 +396,9 @@ func (r *SecretProviderClassPodStatusReconciler) processIfBelongsToNode(objMeta
410396
return true
411397
}
412398

413-
// createK8sSecret creates K8s secret with data from mounted files
399+
// createOrUpdateK8sSecret creates K8s secret with data from mounted files
414400
// If a secret with the same name already exists in the namespace of the pod, the error is nil.
415-
func (r *SecretProviderClassPodStatusReconciler) createK8sSecret(ctx context.Context, name, namespace string, datamap map[string][]byte, labelsmap map[string]string, annotationsmap map[string]string, secretType corev1.SecretType) error {
401+
func (r *SecretProviderClassPodStatusReconciler) createOrUpdateK8sSecret(ctx context.Context, name, namespace string, datamap map[string][]byte, labelsmap map[string]string, annotationsmap map[string]string, secretType corev1.SecretType) error {
416402
secret := &corev1.Secret{
417403
ObjectMeta: metav1.ObjectMeta{
418404
Namespace: namespace,
@@ -430,6 +416,13 @@ func (r *SecretProviderClassPodStatusReconciler) createK8sSecret(ctx context.Con
430416
return nil
431417
}
432418
if apierrors.IsAlreadyExists(err) {
419+
klog.InfoS("Kubernetes secret is already created", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
420+
err := r.writer.Update(ctx, secret)
421+
if err != nil {
422+
klog.Errorf("Unable to update kubernetes secret", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
423+
return err
424+
}
425+
klog.InfoS("successfully updated Kubernetes secret", "secret", klog.ObjectRef{Namespace: namespace, Name: name})
433426
return nil
434427
}
435428
return err
@@ -477,23 +470,6 @@ func (r *SecretProviderClassPodStatusReconciler) patchSecretWithOwnerRef(ctx con
477470
return nil
478471
}
479472

480-
// secretExists checks if the secret with name and namespace already exists
481-
func (r *SecretProviderClassPodStatusReconciler) secretExists(ctx context.Context, name, namespace string) (bool, error) {
482-
o := &corev1.Secret{}
483-
secretKey := types.NamespacedName{
484-
Namespace: namespace,
485-
Name: name,
486-
}
487-
err := r.Client.Get(ctx, secretKey, o)
488-
if err == nil {
489-
return true, nil
490-
}
491-
if apierrors.IsNotFound(err) {
492-
return false, nil
493-
}
494-
return false, err
495-
}
496-
497473
// generateEvent generates an event
498474
func (r *SecretProviderClassPodStatusReconciler) generateEvent(obj apiruntime.Object, eventType, reason, message string) {
499475
if obj != nil {

‎controllers/secretproviderclasspodstatus_controller_test.go‎

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -121,31 +121,6 @@ func newReconciler(client client.Client, scheme *runtime.Scheme, nodeID string)
121121
}
122122
}
123123

124-
func TestSecretExists(t *testing.T) {
125-
g := NewWithT(t)
126-
127-
scheme, err := setupScheme()
128-
g.Expect(err).NotTo(HaveOccurred())
129-
130-
labels := map[string]string{"environment": "test"}
131-
annotations := map[string]string{"kubed.appscode.com/sync": "app=test"}
132-
133-
initObjects := []client.Object{
134-
newSecret("my-secret", "default", labels, annotations),
135-
}
136-
137-
client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(initObjects...).Build()
138-
reconciler := newReconciler(client, scheme, "node1")
139-
140-
exists, err := reconciler.secretExists(context.TODO(), "my-secret", "default")
141-
g.Expect(exists).To(Equal(true))
142-
g.Expect(err).NotTo(HaveOccurred())
143-
144-
exists, err = reconciler.secretExists(context.TODO(), "my-secret2", "default")
145-
g.Expect(exists).To(Equal(false))
146-
g.Expect(err).NotTo(HaveOccurred())
147-
}
148-
149124
func TestPatchSecretWithOwnerRef(t *testing.T) {
150125
g := NewWithT(t)
151126

@@ -183,7 +158,7 @@ func TestPatchSecretWithOwnerRef(t *testing.T) {
183158
g.Expect(secret.GetOwnerReferences()).To(HaveLen(1))
184159
}
185160

186-
func TestCreateK8sSecret(t *testing.T) {
161+
func TestCreateOrUpdateK8sSecret(t *testing.T) {
187162
g := NewWithT(t)
188163

189164
scheme, err := setupScheme()
@@ -198,11 +173,11 @@ func TestCreateK8sSecret(t *testing.T) {
198173
client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(initObjects...).Build()
199174
reconciler := newReconciler(client, scheme, "node1")
200175

201-
// secret already exists
202-
err = reconciler.createK8sSecret(context.TODO(), "my-secret", "default", nil, labels, annotations, corev1.SecretTypeOpaque)
176+
// secret already exists, just update it.
177+
err = reconciler.createOrUpdateK8sSecret(context.TODO(), "my-secret", "default", nil, labels, annotations, corev1.SecretTypeOpaque)
203178
g.Expect(err).NotTo(HaveOccurred())
204179

205-
err = reconciler.createK8sSecret(context.TODO(), "my-secret2", "default", nil, labels, annotations, corev1.SecretTypeOpaque)
180+
err = reconciler.createOrUpdateK8sSecret(context.TODO(), "my-secret2", "default", nil, labels, annotations, corev1.SecretTypeOpaque)
206181
g.Expect(err).NotTo(HaveOccurred())
207182
secret := &corev1.Secret{}
208183
err = client.Get(context.TODO(), types.NamespacedName{Name: "my-secret2", Namespace: "default"}, secret)

‎deploy/csidriver.yaml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,3 @@ spec:
77
attachRequired: false
88
volumeLifecycleModes:
99
- Ephemeral
10-
requiresRepublish: true

‎manifest_staging/charts/secrets-store-csi-driver/templates/csidriver.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ spec:
1111
volumeLifecycleModes:
1212
- Ephemeral
1313
{{- if and (semverCompare ">=1.20-0" .Capabilities.KubeVersion.Version) .Values.tokenRequests }}
14+
requiresRepublish: true
1415
tokenRequests:
1516
{{- toYaml .Values.tokenRequests | nindent 2 }}
1617
{{- end }}

‎manifest_staging/deploy/csidriver.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ spec:
77
attachRequired: false
88
volumeLifecycleModes:
99
- Ephemeral
10+
requiresRepublish: true

‎pkg/secrets-store/nodeserver.go‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -93,15 +93,6 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
9393
ns.reporter.ReportNodePublishCtMetric(ctx, providerName)
9494
}()
9595

96-
if ns.rotationConfig.enabled {
97-
// Node server retries nodepublish volume continuously, so to rotate secret after every `nextRotationTime`,
98-
// nodeserver should skip secret mount till the next `nextRotationTime`
99-
if ns.rotationConfig.nextRotationTime.After(startTime) {
100-
return &csi.NodePublishVolumeResponse{}, nil
101-
}
102-
ns.rotationConfig.nextRotationTime = ns.rotationConfig.nextRotationTime.Add(ns.rotationConfig.interval)
103-
}
104-
10596
// Check arguments
10697
if req.GetVolumeCapability() == nil {
10798
return nil, status.Error(codes.InvalidArgument, "Volume capability missing in request")
@@ -128,6 +119,20 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
128119
podNamespace = attrib[CSIPodNamespace]
129120
podUID = attrib[CSIPodUID]
130121

122+
klog.InfoS("Checking object", "targetPath", targetPath, "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName})
123+
124+
if ns.rotationConfig.enabled {
125+
lastModificationTime, err := ns.getLastUpdateTime(targetPath)
126+
if err != nil {
127+
klog.Infof("could not find last modification time for %s, error: %v\n", targetPath, err)
128+
} else if startTime.Before(lastModificationTime.Add(ns.rotationConfig.interval)) {
129+
// if next rotation is not yet, then skip the mount operation
130+
return &csi.NodePublishVolumeResponse{}, nil
131+
}
132+
}
133+
134+
klog.InfoS("Processing object", "targetPath", targetPath, "pod", klog.ObjectRef{Namespace: podNamespace, Name: podName})
135+
131136
mounted, err = ns.ensureMountPoint(targetPath)
132137
if err != nil {
133138
// kubelet will not create the CSI NodePublishVolume target directory in 1.20+, in accordance with the CSI specification.
@@ -198,7 +203,6 @@ func (ns *nodeServer) NodePublishVolume(ctx context.Context, req *csi.NodePublis
198203
if parameters[CSIPodServiceAccountTokens] == "" {
199204
// Inject pod service account token into volume attributes
200205
klog.Error("csi.storage.k8s.io/serviceAccount.tokens is not populated, set RequiresRepublish")
201-
202206
}
203207

204208
// ensure it's read-only

‎pkg/secrets-store/nodeserver_test.go‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -297,9 +297,8 @@ func TestNodePublishVolume(t *testing.T) {
297297
},
298298
},
299299
rotationConfig: &RotationConfig{
300-
enabled: false,
301-
nextRotationTime: time.Now(),
302-
interval: time.Minute,
300+
enabled: false,
301+
interval: time.Minute,
303302
},
304303
},
305304
{
@@ -332,9 +331,8 @@ func TestNodePublishVolume(t *testing.T) {
332331
},
333332
},
334333
rotationConfig: &RotationConfig{
335-
enabled: true,
336-
nextRotationTime: time.Now().Add(-3 * time.Minute), // so that rotation period is passed and secret will be mounted.
337-
interval: time.Minute,
334+
enabled: true,
335+
interval: time.Minute,
338336
},
339337
},
340338
{
@@ -364,9 +362,8 @@ func TestNodePublishVolume(t *testing.T) {
364362
},
365363
},
366364
rotationConfig: &RotationConfig{
367-
enabled: true,
368-
nextRotationTime: time.Now().Add(2 * time.Minute),
369-
interval: time.Minute,
365+
enabled: true,
366+
interval: time.Minute,
370367
},
371368
},
372369
}
@@ -410,8 +407,8 @@ func TestNodePublishVolume(t *testing.T) {
410407
t.Fatalf("expected err to be nil, got: %v", err)
411408
}
412409
expectedMounts := 1
413-
if ns.rotationConfig.enabled && ns.rotationConfig.nextRotationTime.After(time.Now()) {
414-
// If rotation time is not reached, there should not be any mounts.
410+
if ns.rotationConfig.enabled {
411+
// If rotation time is not enabled, there should not be any mounts.
415412
expectedMounts = 0
416413
}
417414
if len(mnts) != expectedMounts {

‎pkg/secrets-store/secrets-store.go‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,8 @@ type SecretsStore struct {
4040

4141
// RotationConfig stores the informarmation required to rotate the secrets.
4242
type RotationConfig struct {
43-
enabled bool
44-
interval time.Duration
45-
nextRotationTime time.Time
43+
enabled bool
44+
interval time.Duration
4645
}
4746

4847
func NewSecretsStoreDriver(driverName, nodeID, endpoint string,
@@ -92,9 +91,8 @@ func newNodeServer(nodeID string,
9291

9392
func NewRotationConfig(enabled bool, interval time.Duration) *RotationConfig {
9493
return &RotationConfig{
95-
enabled: enabled,
96-
interval: interval,
97-
nextRotationTime: time.Now(),
94+
enabled: enabled,
95+
interval: interval,
9896
}
9997
}
10098

‎pkg/secrets-store/utils.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"fmt"
2222
"os"
2323
"strings"
24+
"time"
2425

2526
secretsstorev1 "sigs.k8s.io/secrets-store-csi-driver/apis/v1"
2627
"sigs.k8s.io/secrets-store-csi-driver/pkg/util/runtimeutil"
@@ -75,6 +76,14 @@ func (ns *nodeServer) ensureMountPoint(target string) (bool, error) {
7576
return false, nil
7677
}
7778

79+
func (ns *nodeServer) getLastUpdateTime(target string) (time.Time, error) {
80+
info, err := os.Stat(target)
81+
if err != nil {
82+
return time.Time{}, err
83+
}
84+
return info.ModTime(), nil
85+
}
86+
7887
// getSecretProviderItem returns the secretproviderclass object by name and namespace
7988
func getSecretProviderItem(ctx context.Context, c client.Client, name, namespace string) (*secretsstorev1.SecretProviderClass, error) {
8089
spc := &secretsstorev1.SecretProviderClass{}

‎test/bats/aws.bats‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ teardown_file() {
8181
[[ "${result//$'\r'}" == "BeforeRotation" ]]
8282

8383
aws ssm put-parameter --name $PM_ROTATION_TEST_NAME --value AfterRotation --type SecureString --overwrite --region $REGION
84-
sleep 40
84+
sleep 180
8585
result=$(kubectl --namespace $NAMESPACE exec $POD_NAME -- cat /mnt/secrets-store/$PM_ROTATION_TEST_NAME)
8686
[[ "${result//$'\r'}" == "AfterRotation" ]]
8787
}
@@ -91,7 +91,7 @@ teardown_file() {
9191
[[ "${result//$'\r'}" == "BeforeRotation" ]]
9292

9393
aws secretsmanager put-secret-value --secret-id $SM_ROT_TEST_NAME --secret-string AfterRotation --region $REGION
94-
sleep 40
94+
sleep 180
9595
result=$(kubectl --namespace $NAMESPACE exec $POD_NAME -- cat /mnt/secrets-store/$SM_ROT_TEST_NAME)
9696
[[ "${result//$'\r'}" == "AfterRotation" ]]
9797
}

0 commit comments

Comments
 (0)