Skip to content

Commit 1ea14d1

Browse files
committed
feat: Enhance reloader with dynamic secret tracking and version change handling
- Introduced dynamic secret lease management in the Controller, allowing for tracking of secret metadata and restart times. - Added methods to handle KV version changes and dynamic secret TTL thresholds for workload restarts. - Implemented concurrency-safe access to workload tracking using sync.RWMutex. - Updated the Vault client initialization to support dynamic secret renewal and version retrieval. - Enhanced tests to cover scenarios for KV version changes, dynamic secret TTL thresholds, and mixed workload decisions. - Refactored secret retrieval logic to return detailed secret information, including dynamic lease data.
1 parent 3d0008a commit 1ea14d1

File tree

9 files changed

+1955
-139
lines changed

9 files changed

+1955
-139
lines changed

deploy/charts/vault-secrets-reloader/templates/rbac.yaml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,14 @@ rules:
2929
- "list"
3030
- "update"
3131
- "watch"
32+
- apiGroups:
33+
- ""
34+
resources:
35+
- pods
36+
verbs:
37+
- "get"
38+
- "list"
39+
- "watch"
3240
- apiGroups:
3341
- ""
3442
resources:

deploy/charts/vault-secrets-reloader/values.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ env: {}
9898
# VAULT_PATH: "kubernetes"
9999
# VAULT_CLIENT_TIMEOUT: "10s"
100100
# VAULT_IGNORE_MISSING_SECRETS: "false"
101+
# VAULT_DYNAMIC_SECRET_RESTART_THRESHOLD: "0.70"
101102

102103
# -- Extra volume definitions for Reloader deployment
103104
volumes: []

pkg/reloader/collector.go

Lines changed: 222 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,24 @@ import (
1919
"log/slog"
2020
"regexp"
2121
"slices"
22+
"strconv"
2223
"strings"
2324
"sync"
25+
"time"
2426

2527
"github.com/bank-vaults/secrets-webhook/pkg/common"
2628
corev1 "k8s.io/api/core/v1"
2729
)
2830

31+
var (
32+
vaultSecretRegex = regexp.MustCompile(`vault:([^#]+)#`)
33+
)
34+
2935
type workloadSecretsStore interface {
30-
Store(workload workload, secrets []string)
36+
Store(workload workload, secrets []secretMetadata)
3137
Delete(workload workload)
32-
GetWorkloadSecretsMap() map[workload][]string
33-
GetSecretWorkloadsMap() map[string][]workload
38+
GetWorkloadSecretsMap() map[workload][]secretMetadata
39+
GetSecretWorkloadsMap() map[secretMetadata][]workload
3440
}
3541

3642
type workload struct {
@@ -39,18 +45,31 @@ type workload struct {
3945
kind string
4046
}
4147

48+
type secretMetadata struct {
49+
Path string
50+
IsKV bool
51+
KVVersion int
52+
IsDynamic bool
53+
DynamicLeaseTTL int // in seconds
54+
}
55+
56+
type workloadTrackingInfo struct {
57+
LastRestartTime time.Time
58+
ShortestDynamicTTL int // in seconds, 0 if no dynamic secrets
59+
}
60+
4261
type workloadSecrets struct {
4362
sync.RWMutex
44-
workloadSecretsMap map[workload][]string
63+
workloadSecretsMap map[workload][]secretMetadata
4564
}
4665

4766
func newWorkloadSecrets() workloadSecretsStore {
4867
return &workloadSecrets{
49-
workloadSecretsMap: make(map[workload][]string),
68+
workloadSecretsMap: make(map[workload][]secretMetadata),
5069
}
5170
}
5271

53-
func (w *workloadSecrets) Store(workload workload, secrets []string) {
72+
func (w *workloadSecrets) Store(workload workload, secrets []secretMetadata) {
5473
w.Lock()
5574
defer w.Unlock()
5675
w.workloadSecretsMap[workload] = secrets
@@ -62,17 +81,17 @@ func (w *workloadSecrets) Delete(workload workload) {
6281
delete(w.workloadSecretsMap, workload)
6382
}
6483

65-
func (w *workloadSecrets) GetWorkloadSecretsMap() map[workload][]string {
84+
func (w *workloadSecrets) GetWorkloadSecretsMap() map[workload][]secretMetadata {
6685
return w.workloadSecretsMap
6786
}
6887

69-
func (w *workloadSecrets) GetSecretWorkloadsMap() map[string][]workload {
88+
func (w *workloadSecrets) GetSecretWorkloadsMap() map[secretMetadata][]workload {
7089
w.Lock()
7190
defer w.Unlock()
72-
secretWorkloads := make(map[string][]workload)
73-
for workload, secretPaths := range w.workloadSecretsMap {
74-
for _, secretPath := range secretPaths {
75-
secretWorkloads[secretPath] = append(secretWorkloads[secretPath], workload)
91+
secretWorkloads := make(map[secretMetadata][]workload)
92+
for workload, secretsMetadata := range w.workloadSecretsMap {
93+
for _, secretMetadata := range secretsMetadata {
94+
secretWorkloads[secretMetadata] = append(secretWorkloads[secretMetadata], workload)
7695
}
7796
}
7897
return secretWorkloads
@@ -86,15 +105,128 @@ func (c *Controller) collectWorkloadSecrets(workload workload, template corev1.P
86105

87106
if len(vaultSecretPaths) == 0 {
88107
collectorLogger.Debug("No Vault secret paths found in container env vars")
108+
109+
// Clear stale deployment state to prevent restarts based on old metadata
110+
c.trackingMutex.Lock()
111+
delete(c.workloadTracking, workload)
112+
c.trackingMutex.Unlock()
113+
89114
return
90115
}
91116
collectorLogger.Debug(fmt.Sprintf("Vault secret paths found: %v", vaultSecretPaths))
92117

118+
// Query Vault for metadata about each secret
119+
err := c.initVaultClientFn()
120+
if err != nil {
121+
collectorLogger.Error(fmt.Sprintf("Failed to initialize Vault client: %v", err))
122+
return
123+
}
124+
125+
// Build a lookup of already-tracked dynamic secrets to avoid re-reading them
126+
c.trackingMutex.RLock()
127+
existingMetadata, hasExisting := c.workloadSecrets.GetWorkloadSecretsMap()[workload]
128+
c.trackingMutex.RUnlock()
129+
130+
dynamicMetadataByPath := make(map[string]secretMetadata)
131+
if hasExisting {
132+
for _, metadata := range existingMetadata {
133+
if metadata.IsDynamic {
134+
dynamicMetadataByPath[metadata.Path] = metadata
135+
}
136+
}
137+
}
138+
139+
secretsMetadata := make([]secretMetadata, 0, len(vaultSecretPaths))
140+
for _, secretPath := range vaultSecretPaths {
141+
if metadata, exists := dynamicMetadataByPath[secretPath]; exists {
142+
secretsMetadata = append(secretsMetadata, metadata)
143+
collectorLogger.Debug(fmt.Sprintf("Secret %s is dynamic, using tracked TTL: %d seconds", secretPath, metadata.DynamicLeaseTTL))
144+
continue
145+
}
146+
147+
secretInfo, err := c.getSecretInfoFn(c.getVaultLogicalFn(), secretPath)
148+
if err != nil {
149+
collectorLogger.Error(fmt.Sprintf("Failed to get secret info for %s: %v", secretPath, err))
150+
continue
151+
}
152+
153+
metadata := secretMetadata{
154+
Path: secretPath,
155+
}
156+
157+
if secretInfo.IsKV {
158+
metadata.IsKV = true
159+
metadata.KVVersion = secretInfo.Version
160+
collectorLogger.Debug(fmt.Sprintf("Secret %s is KV v2, version: %d", secretPath, secretInfo.Version))
161+
} else if secretInfo.IsDynamic {
162+
metadata.IsDynamic = true
163+
metadata.DynamicLeaseTTL = secretInfo.LeaseInfo.LeaseDuration
164+
collectorLogger.Debug(fmt.Sprintf("Secret %s is dynamic, TTL: %d seconds", secretPath, secretInfo.LeaseInfo.LeaseDuration))
165+
}
166+
167+
secretsMetadata = append(secretsMetadata, metadata)
168+
}
169+
93170
// Add workload and secrets to workloadSecrets map
94-
c.workloadSecrets.Store(workload, vaultSecretPaths)
171+
c.workloadSecrets.Store(workload, secretsMetadata)
95172
collectorLogger.Info(fmt.Sprintf("Collected secrets from %s %s/%s", workload.kind, workload.namespace, workload.name))
96173
}
97174

175+
func (c *Controller) trackWorkloadRestartTime(workload workload, pods []corev1.Pod) {
176+
c.trackingMutex.Lock()
177+
defer c.trackingMutex.Unlock()
178+
179+
// Get deployment secrets to calculate shortest dynamic TTL
180+
workloadSecretsMeta, exists := c.workloadSecrets.GetWorkloadSecretsMap()[workload]
181+
if !exists {
182+
return
183+
}
184+
185+
shortestTTL := 0
186+
for _, secret := range workloadSecretsMeta {
187+
if secret.IsDynamic {
188+
if shortestTTL == 0 || secret.DynamicLeaseTTL < shortestTTL {
189+
shortestTTL = secret.DynamicLeaseTTL
190+
}
191+
}
192+
}
193+
194+
// Find the oldest running pod to determine deployment's effective start time
195+
var oldestStartTime time.Time
196+
for _, pod := range pods {
197+
if pod.Status.StartTime == nil {
198+
continue // Pod hasn't started yet
199+
}
200+
201+
if pod.Status.Phase != corev1.PodRunning {
202+
continue // Only consider running pods
203+
}
204+
205+
if pod.ObjectMeta.DeletionTimestamp != nil {
206+
continue // Skip pods that are terminating
207+
}
208+
209+
if oldestStartTime.IsZero() || pod.Status.StartTime.Time.Before(oldestStartTime) {
210+
oldestStartTime = pod.Status.StartTime.Time
211+
}
212+
}
213+
214+
// Only track if we found at least one running pod
215+
if !oldestStartTime.IsZero() {
216+
if existing, exists := c.workloadTracking[workload]; exists {
217+
// Always update to reflect current pod state (oldest time may be earlier now,
218+
// and TTL may have changed)
219+
existing.LastRestartTime = oldestStartTime
220+
existing.ShortestDynamicTTL = shortestTTL
221+
} else {
222+
c.workloadTracking[workload] = &workloadTrackingInfo{
223+
LastRestartTime: oldestStartTime,
224+
ShortestDynamicTTL: shortestTTL,
225+
}
226+
}
227+
}
228+
}
229+
98230
func collectSecrets(template corev1.PodTemplateSpec) []string {
99231
containers := []corev1.Container{}
100232
containers = append(containers, template.Spec.Containers...)
@@ -114,12 +246,17 @@ func collectSecretsFromContainerEnvVars(containers []corev1.Container) []string
114246
// iterate through all environment variables and extract secrets
115247
for _, container := range containers {
116248
for _, env := range container.Env {
117-
// Skip if env var does not contain a vault secret or is a secret with pinned version
118-
if isValidPrefix(env.Value) && unversionedSecretValue(env.Value) {
119-
secret := regexp.MustCompile(`vault:(.*?)#`).FindStringSubmatch(env.Value)[1]
120-
if secret != "" {
121-
vaultSecretPaths = append(vaultSecretPaths, secret)
249+
// Skip if env var does not contain a vault secret
250+
if !isValidVaultSubstring(env.Value) {
251+
continue
252+
}
253+
254+
segments := extractVaultSecretSegments(env.Value)
255+
for _, segment := range segments {
256+
if segment.IsVersioned {
257+
continue
122258
}
259+
vaultSecretPaths = append(vaultSecretPaths, segment.Path)
123260
}
124261
}
125262
}
@@ -133,8 +270,12 @@ func collectSecretsFromAnnotations(annotations map[string]string) []string {
133270
secretPaths := annotations[common.VaultFromPathAnnotation]
134271
if secretPaths != "" {
135272
for _, secretPath := range strings.Split(secretPaths, ",") {
136-
if unversionedAnnotationSecretValue(secretPath) {
137-
vaultSecretPaths = append(vaultSecretPaths, secretPath)
273+
segments := extractVaultSecretSegments(secretPath)
274+
for _, segment := range segments {
275+
if segment.IsVersioned {
276+
continue
277+
}
278+
vaultSecretPaths = append(vaultSecretPaths, segment.Path)
138279
}
139280
}
140281
}
@@ -144,8 +285,12 @@ func collectSecretsFromAnnotations(annotations map[string]string) []string {
144285
deprecatedSecretPaths := annotations[common.VaultEnvFromPathAnnotationDeprecated]
145286
if deprecatedSecretPaths != "" {
146287
for _, secretPath := range strings.Split(deprecatedSecretPaths, ",") {
147-
if unversionedAnnotationSecretValue(secretPath) {
148-
vaultSecretPaths = append(vaultSecretPaths, secretPath)
288+
segments := extractVaultSecretSegments(secretPath)
289+
for _, segment := range segments {
290+
if segment.IsVersioned {
291+
continue
292+
}
293+
vaultSecretPaths = append(vaultSecretPaths, segment.Path)
149294
}
150295
}
151296
}
@@ -155,14 +300,63 @@ func collectSecretsFromAnnotations(annotations map[string]string) []string {
155300
}
156301

157302
// implementation based on bank-vaults/secrets-webhook/pkg/provider/vault/provider.go
158-
func isValidPrefix(value string) bool {
159-
return strings.HasPrefix(value, "vault:") || strings.HasPrefix(value, ">>vault:")
303+
func isValidVaultSubstring(value string) bool {
304+
return vaultSecretRegex.MatchString(value)
305+
}
306+
307+
type vaultSecretSegment struct {
308+
Path string
309+
IsVersioned bool
310+
}
311+
312+
func extractVaultSecretSegments(value string) []vaultSecretSegment {
313+
segments := []vaultSecretSegment{}
314+
searchIndex := 0
315+
for {
316+
start := strings.Index(value[searchIndex:], "vault:")
317+
if start == -1 {
318+
break
319+
}
320+
start += searchIndex + len("vault:")
321+
segmentEnd := len(value)
322+
if next := strings.Index(value[start:], "vault:"); next != -1 {
323+
segmentEnd = start + next
324+
}
325+
segment := value[start:segmentEnd]
326+
firstHash := strings.Index(segment, "#")
327+
if firstHash == -1 {
328+
searchIndex = start
329+
continue
330+
}
331+
path := segment[:firstHash]
332+
if path == "" {
333+
searchIndex = start
334+
continue
335+
}
336+
remainder := segment[firstHash+1:]
337+
isVersioned := false
338+
if remainder != "" {
339+
parts := strings.Split(remainder, "#")
340+
if len(parts) >= 2 {
341+
last := parts[len(parts)-1]
342+
if last != "" && isAllDigits(last) {
343+
isVersioned = true
344+
}
345+
}
346+
}
347+
segments = append(segments, vaultSecretSegment{Path: path, IsVersioned: isVersioned})
348+
searchIndex = start
349+
}
350+
351+
return segments
160352
}
161353

162-
// implementation based on bank-vaults/internal/pkg/injector/vault/injector.go
163-
func unversionedSecretValue(value string) bool {
164-
split := strings.SplitN(value, "#", 3)
165-
return len(split) == 2
354+
func isAllDigits(value string) bool {
355+
if value == "" {
356+
return false
357+
}
358+
_, err := strconv.ParseUint(value, 10, 64)
359+
return err == nil
166360
}
167361

168362
func unversionedAnnotationSecretValue(value string) bool {

0 commit comments

Comments
 (0)