Merge pull request #309 from jsecchiero/main

feat: add syncOnResourceChange opt-in to sync the target secret on VaultSecret changes

Author: raffaelespazzoli

api/v1alpha1/vaultsecret_types.go

@@ -38,6 +38,12 @@ type VaultSecretSpec struct {
 	// +kubebuilder:validation:Required
 	// +kubebuilder:default=90
 	RefreshThreshold int `json:"refreshThreshold,omitempty"`
+	// SyncOnResourceChange if set to true, the operator will immediately resync the secret from Vault
+	// whenever the VaultSecret spec or metadata changes, bypassing the time-based refresh gate.
+	// By default this is false, meaning changes to the resource will only take effect at the next scheduled refresh.
+	// +kubebuilder:validation:Optional
+	// +kubebuilder:default=false
+	SyncOnResourceChange bool `json:"syncOnResourceChange,omitempty"`
 	// VaultSecretDefinitions are the secrets in Vault.
 	// +kubebuilder:validation:Required
 	VaultSecretDefinitions []VaultSecretDefinition `json:"vaultSecretDefinitions,omitempty"`
@@ -60,6 +66,11 @@ type VaultSecretStatus struct {
 	//NextVaultSecretUpdate the next time when this secret will be synced with Vault. If nil, it will not be refreshed.
 	NextVaultSecretUpdate *metav1.Time `json:"nextVaultSecretUpdate,omitempty"`
 
+	//SyncedResourceVersion is a combination of the VaultSecret's generation and metadata hash.
+	//Is enabled by SyncOnResourceChange: true
+	// +kubebuilder:validation:Optional
+	SyncedResourceVersion string `json:"syncedResourceVersion,omitempty"`
+
 	//VaultSecretDefinitionsStatus information used to determine if the secret should be rereconciled
 	VaultSecretDefinitionsStatus []VaultSecretDefinitionStatus `json:"vaultSecretDefinitionsStatus,omitempty" patchStrategy:"merge" patchMergeKey:"type"`
 }

config/crd/bases/redhatcop.redhat.io_vaultsecrets.yaml

@@ -81,6 +81,13 @@ spec:
                   This is particularly useful for controlling when dynamic secrets should be refreshed before the lease duration is exceeded.
                   The default is 90, meaning the secret would refresh after 90% of the time has passed from the vault secret's lease duration.
                 type: integer
+              syncOnResourceChange:
+                default: false
+                description: |-
+                  SyncOnResourceChange if set to true, the operator will immediately resync the secret from Vault
+                  whenever the VaultSecret spec or metadata changes, bypassing the time-based refresh gate.
+                  By default this is false, meaning changes to the resource will only take effect at the next scheduled refresh.
+                type: boolean
               vaultSecretDefinitions:
                 description: VaultSecretDefinitions are the secrets in Vault.
                 items:
@@ -290,6 +297,11 @@ spec:
                   will be synced with Vault. If nil, it will not be refreshed.
                 format: date-time
                 type: string
+              syncedResourceVersion:
+                description: |-
+                  SyncedResourceVersion is a combination of the VaultSecret's generation and metadata hash.
+                  Is is enabled by SyncOnResourceChange: true
+                type: string
               vaultSecretDefinitionsStatus:
                 description: VaultSecretDefinitionsStatus information used to determine
                   if the secret should be rereconciled

controllers/vaultsecret_controller.go

@@ -259,6 +259,20 @@ func toNamespacedName(obj metav1.Object) string {
 
 func (r *VaultSecretReconciler) shouldSync(ctx context.Context, instance *redhatcopv1alpha1.VaultSecret) (bool, error) {
 
+	// if syncOnResourceChange is enabled and the VaultSecret spec or metadata has changed since the last sync,
+	// always sync immediately.
+	if instance.Spec.SyncOnResourceChange {
+		currentResourceVersion := vaultsecretutils.GetResourceVersion(instance.ObjectMeta)
+		if instance.Status.SyncedResourceVersion != currentResourceVersion {
+			r.Log.V(1).Info("VaultSecret resource version changed, forcing sync",
+				"namespacedName", toNamespacedName(instance),
+				"syncedResourceVersion", instance.Status.SyncedResourceVersion,
+				"currentResourceVersion", currentResourceVersion)
+			return true, nil
+		}
+	}
+
+	// check if the k8s secret is valid (exists, owned, data not tampered with)
 	secretNamespacedName := &types.NamespacedName{
 		Name:      instance.Spec.TemplatizedK8sSecret.Name,
 		Namespace: instance.Namespace,
@@ -374,6 +388,7 @@ func (r *VaultSecretReconciler) manageSyncLogic(ctx context.Context, instance *r
 
 	now := metav1.NewTime(time.Now())
 	instance.Status.LastVaultSecretUpdate = &now
+	instance.Status.SyncedResourceVersion = vaultsecretutils.GetResourceVersion(instance.ObjectMeta)
 	instance.Status.VaultSecretDefinitionsStatus = definitionsStatus
 
 	return nil

controllers/vaultsecretutils/hash.go

@@ -3,7 +3,10 @@ package vaultsecretutils
 import (
 	"crypto/sha256"
 	"encoding/hex"
+	"fmt"
 	"sort"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 func hashHashes(l [32]byte, r [32]byte) [32]byte {
@@ -34,3 +37,17 @@ func HashData(data map[string][]byte) string {
 
 	return hex.EncodeToString(rootSha[:])
 }
+
+// HashMeta returns a SHA-256 hash of the object's labels and annotations.
+func HashMeta(meta metav1.ObjectMeta) string {
+	h := sha256.New()
+	h.Write([]byte(fmt.Sprintf("%v", meta.Labels)))
+	h.Write([]byte(fmt.Sprintf("%v", meta.Annotations)))
+	return hex.EncodeToString(h.Sum(nil))
+}
+
+// GetResourceVersion returns a string combining the object's generation and a hash of its metadata.
+// This is used to detect spec or metadata changes that should trigger an immediate resync.
+func GetResourceVersion(meta metav1.ObjectMeta) string {
+	return fmt.Sprintf("%d-%s", meta.GetGeneration(), HashMeta(meta))
+}

controllers/vaultsecretutils/hash_test.go

@@ -3,6 +3,8 @@ package vaultsecretutils
 import (
 	"strings"
 	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
 const (
@@ -101,3 +103,141 @@ func TestNil(t *testing.T) {
 		t.Errorf("Unexpected Hash, got: %v, want: %v.", hash, emptyOrNilHash)
 	}
 }
+
+func TestHashMeta(t *testing.T) {
+	meta := metav1.ObjectMeta{
+		Labels:      map[string]string{"app": "test"},
+		Annotations: map[string]string{"note": "value"},
+	}
+
+	hash := HashMeta(meta)
+
+	if hash == "" {
+		t.Error("HashMeta returned empty string")
+	}
+
+	// same input should produce same hash
+	hash2 := HashMeta(meta)
+	if hash != hash2 {
+		t.Errorf("HashMeta not deterministic, got: %v and %v", hash, hash2)
+	}
+}
+
+func TestHashMetaDifferentLabels(t *testing.T) {
+	meta1 := metav1.ObjectMeta{
+		Labels: map[string]string{"app": "test"},
+	}
+	meta2 := metav1.ObjectMeta{
+		Labels: map[string]string{"app": "other"},
+	}
+
+	if HashMeta(meta1) == HashMeta(meta2) {
+		t.Error("HashMeta should differ when labels differ")
+	}
+}
+
+func TestHashMetaDifferentAnnotations(t *testing.T) {
+	meta1 := metav1.ObjectMeta{
+		Annotations: map[string]string{"key": "value1"},
+	}
+	meta2 := metav1.ObjectMeta{
+		Annotations: map[string]string{"key": "value2"},
+	}
+
+	if HashMeta(meta1) == HashMeta(meta2) {
+		t.Error("HashMeta should differ when annotations differ")
+	}
+}
+
+func TestHashMetaNilMaps(t *testing.T) {
+	meta := metav1.ObjectMeta{}
+
+	hash := HashMeta(meta)
+
+	if hash == "" {
+		t.Error("HashMeta returned empty string for nil labels/annotations")
+	}
+}
+
+func TestHashMetaEmptyMaps(t *testing.T) {
+	meta1 := metav1.ObjectMeta{
+		Labels:      map[string]string{},
+		Annotations: map[string]string{},
+	}
+	meta2 := metav1.ObjectMeta{}
+
+	// empty maps and nil maps should produce the same hash since fmt.Sprintf
+	// renders both as "map[]"
+	if HashMeta(meta1) != HashMeta(meta2) {
+		t.Errorf("HashMeta should be equal for empty and nil maps, got: %v and %v", HashMeta(meta1), HashMeta(meta2))
+	}
+}
+
+func TestGetResourceVersion(t *testing.T) {
+	meta := metav1.ObjectMeta{
+		Generation: 1,
+		Labels:     map[string]string{"app": "test"},
+	}
+
+	rv := GetResourceVersion(meta)
+
+	if rv == "" {
+		t.Error("GetResourceVersion returned empty string")
+	}
+
+	// should start with the generation number
+	if rv[:2] != "1-" {
+		t.Errorf("GetResourceVersion should start with generation, got: %v", rv)
+	}
+}
+
+func TestGetResourceVersionChangesOnGeneration(t *testing.T) {
+	meta1 := metav1.ObjectMeta{
+		Generation: 1,
+		Labels:     map[string]string{"app": "test"},
+	}
+	meta2 := metav1.ObjectMeta{
+		Generation: 2,
+		Labels:     map[string]string{"app": "test"},
+	}
+
+	rv1 := GetResourceVersion(meta1)
+	rv2 := GetResourceVersion(meta2)
+
+	if rv1 == rv2 {
+		t.Error("GetResourceVersion should differ when generation differs")
+	}
+}
+
+func TestGetResourceVersionChangesOnMetadata(t *testing.T) {
+	meta1 := metav1.ObjectMeta{
+		Generation: 1,
+		Labels:     map[string]string{"app": "test"},
+	}
+	meta2 := metav1.ObjectMeta{
+		Generation: 1,
+		Labels:     map[string]string{"app": "changed"},
+	}
+
+	rv1 := GetResourceVersion(meta1)
+	rv2 := GetResourceVersion(meta2)
+
+	if rv1 == rv2 {
+		t.Error("GetResourceVersion should differ when labels differ")
+	}
+}
+
+func TestGetResourceVersionStableWhenUnchanged(t *testing.T) {
+	meta := metav1.ObjectMeta{
+		Generation:  3,
+		Labels:      map[string]string{"app": "test"},
+		Annotations: map[string]string{"note": "value"},
+	}
+
+	rv1 := GetResourceVersion(meta)
+	rv2 := GetResourceVersion(meta)
+
+	if rv1 != rv2 {
+		t.Errorf("GetResourceVersion should be stable, got: %v and %v", rv1, rv2)
+	}
+}

docs/secret-management.md

@@ -72,6 +72,7 @@ Any manual data change or deletion of the K8s Secret owned by a VaultSecret CR w
 
 - `refreshPeriod` the pull interval for syncing Vault secrets with the K8s Secret. This settings takes precedence over any lease duration returned by vault, effectively controlling when exactly all vault secrets defined in the vaultSecretDefinitions should re-sync.
 - `refreshThreshold` this is will instruct the operator to refresh the K8s Secret when a percentage of the lease duration has elapsed, if no `refreshPeriod` is specified. This is particularly useful for controlling when dynamic secrets should be refreshed before the lease duration is exceeded. The default is 90, meaning the secret would refresh after 90% of the time has passed from the vault secret's lease duration. When multiple vaultSecretDefinitions are defined, the smallest lease duration will be used when performing the scheduling for the next refresh.
+- `syncOnResourceChange` if set to `true`, the operator will immediately resync the secret from Vault whenever the VaultSecret spec or metadata (labels/annotations) changes, bypassing the time-based refresh gate. By default this is `false`, meaning changes to the VaultSecret resource will only take effect at the next scheduled refresh (controlled by `refreshPeriod` or `refreshThreshold`). This is useful when you want spec changes like updating `output.stringData` templates or `vaultSecretDefinitions` to be reflected in the K8s Secret right away without waiting for the next refresh cycle.
 - `vaultSecretDefinitions` is an array of Vault Secret References. Every `vaultSecretDefinition` has...
   - [authentication](#the-authentication-section) section.
   - `name` a unique name for the Vault secret to reference when templating, since many Vault secrets may have the same name.
@@ -94,6 +95,7 @@ metadata:
   name: randomsecret
 spec:
   refreshPeriod: 1m0s
+  syncOnResourceChange: true
   vaultSecretDefinitions:
     - authentication:
         path: kubernetes