Skip to content

Commit 91894d0

Browse files
gigerdogigerdo
committed
Implement ids handler for secret values
1 parent 5b31272 commit 91894d0

File tree

2 files changed

+278
-7
lines changed

2 files changed

+278
-7
lines changed

‎internal/fleet/integration_policy/secrets.go‎

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,9 +71,25 @@ func HandleRespSecrets(ctx context.Context, resp *kbapi.PackagePolicy, private p
7171
}
7272

7373
handleVar := func(key string, mval map[string]any, vars map[string]any) {
74-
refID := mval["id"].(string)
75-
if original, ok := secrets[refID]; ok {
76-
vars[key] = original
74+
// Handle single secret reference: {"id": "refID", "isSecretRef": true}
75+
if refID, ok := mval["id"].(string); ok {
76+
if original, ok := secrets[refID]; ok {
77+
vars[key] = original
78+
}
79+
return
80+
}
81+
82+
// Handle list secret reference: {"ids": ["a", "b"], "isSecretRef": true}
83+
if refIDs, ok := mval["ids"].([]any); ok {
84+
resolvedValues := make([]any, 0, len(refIDs))
85+
for _, refIDInterface := range refIDs {
86+
if refID, ok := refIDInterface.(string); ok {
87+
if original, ok := secrets[refID]; ok {
88+
resolvedValues = append(resolvedValues, original)
89+
}
90+
}
91+
}
92+
vars[key] = resolvedValues
7793
}
7894
}
7995

@@ -136,8 +152,23 @@ func HandleReqRespSecrets(ctx context.Context, req kbapi.PackagePolicyRequest, r
136152
}
137153
}
138154

139-
refID := mval["id"].(string)
140-
secrets[refID] = original
155+
// Handle single secret reference: {"id": "refID", "isSecretRef": true}
156+
if refID, ok := mval["id"].(string); ok {
157+
secrets[refID] = original
158+
return
159+
}
160+
161+
// Handle list secret reference: {"ids": ["a", "b"], "isSecretRef": true}
162+
if refIDs, ok := mval["ids"].([]any); ok {
163+
// For list secrets, the original should be an array
164+
if originalArray, ok := original.([]any); ok {
165+
for i, refIDInterface := range refIDs {
166+
if refID, ok := refIDInterface.(string); ok && i < len(originalArray) {
167+
secrets[refID] = originalArray[i]
168+
}
169+
}
170+
}
171+
}
141172
}
142173
}
143174

‎internal/fleet/integration_policy/secrets_test.go‎

Lines changed: 242 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,26 @@ func TestHandleRespSecrets(t *testing.T) {
7979
input: Map{"k": Map{"type": "password", "value": Map{"isSecretRef": true, "id": "unknown-secret"}}},
8080
want: Map{"k": Map{"isSecretRef": true, "id": "unknown-secret"}},
8181
},
82+
{
83+
name: "converts list secret",
84+
input: Map{"k": Map{"isSecretRef": true, "ids": []any{"known-secret"}}},
85+
want: Map{"k": []any{"secret"}},
86+
},
87+
{
88+
name: "converts multiple list secrets",
89+
input: Map{"k": Map{"isSecretRef": true, "ids": []any{"known-secret", "known-secret"}}},
90+
want: Map{"k": []any{"secret", "secret"}},
91+
},
92+
{
93+
name: "converts mixed list secrets",
94+
input: Map{"k": Map{"isSecretRef": true, "ids": []any{"known-secret", "unknown-secret"}}},
95+
want: Map{"k": []any{"secret"}},
96+
},
97+
{
98+
name: "converts wrapped list secret",
99+
input: Map{"k": Map{"type": "array", "value": Map{"isSecretRef": true, "ids": []any{"known-secret"}}}},
100+
want: Map{"k": []any{"secret"}},
101+
},
82102
}
83103

84104
for _, tt := range tests {
@@ -184,6 +204,30 @@ func TestHandleReqRespSecrets(t *testing.T) {
184204
respInput: Map{"k": Map{"type": "password", "value": Map{"isSecretRef": true, "id": "unknown-secret"}}},
185205
want: Map{"k": Map{"isSecretRef": true, "id": "unknown-secret"}},
186206
},
207+
{
208+
name: "converts list secret",
209+
reqInput: Map{"k": []any{"secret1", "secret2"}},
210+
respInput: Map{"k": Map{"isSecretRef": true, "ids": []any{"ref1", "ref2"}}},
211+
want: Map{"k": []any{"secret1", "secret2"}},
212+
},
213+
{
214+
name: "converts wrapped list secret",
215+
reqInput: Map{"k": []any{"secret1", "secret2"}},
216+
respInput: Map{"k": Map{"type": "array", "value": Map{"isSecretRef": true, "ids": []any{"ref1", "ref2"}}}},
217+
want: Map{"k": []any{"secret1", "secret2"}},
218+
},
219+
{
220+
name: "converts partial list secret",
221+
reqInput: Map{"k": []any{"secret1"}},
222+
respInput: Map{"k": Map{"isSecretRef": true, "ids": []any{"ref1", "ref2"}}},
223+
want: Map{"k": []any{"secret1"}},
224+
},
225+
{
226+
name: "converts empty list secret",
227+
reqInput: Map{"k": []any{}},
228+
respInput: Map{"k": Map{"isSecretRef": true, "ids": []any{"ref1"}}},
229+
want: Map{"k": []any{}},
230+
},
187231
}
188232

189233
for _, tt := range tests {
@@ -236,13 +280,209 @@ func TestHandleReqRespSecrets(t *testing.T) {
236280
want = *(*wants.Inputs["input1"].Streams)["stream1"].Vars
237281
require.Equal(t, want, got)
238282

239-
if v, ok := (*req.Vars)["k"]; ok && v == "secret" {
283+
// Check private data expectations based on test case
284+
switch tt.name {
285+
case "converts secret", "converts wrapped secret":
240286
privateWants := privateData{"secrets": `{"known-secret":"secret"}`}
241287
require.Equal(t, privateWants, private)
242-
} else {
288+
case "converts list secret", "converts wrapped list secret":
289+
privateWants := privateData{"secrets": `{"ref1":"secret1","ref2":"secret2"}`}
290+
require.Equal(t, privateWants, private)
291+
case "converts partial list secret":
292+
privateWants := privateData{"secrets": `{"ref1":"secret1"}`}
293+
require.Equal(t, privateWants, private)
294+
case "converts empty list secret":
295+
privateWants := privateData{"secrets": `{}`}
296+
require.Equal(t, privateWants, private)
297+
default:
243298
privateWants := privateData{"secrets": `{}`}
244299
require.Equal(t, privateWants, private)
245300
}
246301
})
247302
}
248303
}
304+
305+
func TestEdgeCases(t *testing.T) {
306+
t.Parallel()
307+
308+
ctx := context.Background()
309+
310+
t.Run("handles deeply nested secret references", func(t *testing.T) {
311+
private := privateData{"secrets": `{"nested-ref":"nested-secret"}`}
312+
313+
resp := &kbapi.PackagePolicy{
314+
SecretReferences: &[]kbapi.PackagePolicySecretRef{{Id: "nested-ref"}},
315+
Vars: &map[string]any{
316+
"level1": map[string]any{
317+
"type": "object",
318+
"value": map[string]any{
319+
"level2": map[string]any{
320+
"isSecretRef": true,
321+
"id": "nested-ref",
322+
},
323+
},
324+
},
325+
},
326+
}
327+
328+
diags := integration_policy.HandleRespSecrets(ctx, resp, &private)
329+
require.Empty(t, diags)
330+
331+
expected := map[string]any{
332+
"level1": map[string]any{
333+
"level2": "nested-secret",
334+
},
335+
}
336+
require.Equal(t, expected, *resp.Vars)
337+
})
338+
339+
t.Run("handles multiple input streams", func(t *testing.T) {
340+
private := privateData{"secrets": `{"stream1-ref":"stream1-secret","stream2-ref":"stream2-secret"}`}
341+
342+
resp := &kbapi.PackagePolicy{
343+
SecretReferences: &[]kbapi.PackagePolicySecretRef{
344+
{Id: "stream1-ref"}, {Id: "stream2-ref"},
345+
},
346+
Inputs: map[string]kbapi.PackagePolicyInput{
347+
"input1": {
348+
Streams: &map[string]kbapi.PackagePolicyInputStream{
349+
"stream1": {
350+
Vars: &map[string]any{
351+
"secret1": map[string]any{"isSecretRef": true, "id": "stream1-ref"},
352+
},
353+
},
354+
"stream2": {
355+
Vars: &map[string]any{
356+
"secret2": map[string]any{"isSecretRef": true, "id": "stream2-ref"},
357+
},
358+
},
359+
},
360+
},
361+
},
362+
}
363+
364+
diags := integration_policy.HandleRespSecrets(ctx, resp, &private)
365+
require.Empty(t, diags)
366+
367+
streams := *resp.Inputs["input1"].Streams
368+
require.Equal(t, "stream1-secret", (*streams["stream1"].Vars)["secret1"])
369+
require.Equal(t, "stream2-secret", (*streams["stream2"].Vars)["secret2"])
370+
})
371+
372+
t.Run("handles invalid JSON in private data", func(t *testing.T) {
373+
private := privateData{"secrets": `{"invalid": json}`}
374+
375+
resp := &kbapi.PackagePolicy{
376+
SecretReferences: &[]kbapi.PackagePolicySecretRef{},
377+
Vars: &map[string]any{},
378+
}
379+
380+
diags := integration_policy.HandleRespSecrets(ctx, resp, &private)
381+
require.True(t, diags.HasError(), "Expected error diagnostics")
382+
})
383+
}
384+
385+
func TestMigrationScenarios(t *testing.T) {
386+
t.Parallel()
387+
388+
// Test pre-0.11.7 migration scenarios mentioned in HandleReqRespSecrets
389+
ctx := context.Background()
390+
391+
t.Run("handles importing existing secret refs", func(t *testing.T) {
392+
// Simulate importing a resource that already has secret refs in request
393+
req := kbapi.PackagePolicyRequest{
394+
Vars: &map[string]any{
395+
"existing_secret": map[string]any{
396+
"isSecretRef": true,
397+
"id": "existing-ref",
398+
},
399+
},
400+
}
401+
402+
resp := &kbapi.PackagePolicy{
403+
SecretReferences: &[]kbapi.PackagePolicySecretRef{{Id: "new-ref"}},
404+
Vars: &map[string]any{
405+
"existing_secret": map[string]any{
406+
"isSecretRef": true,
407+
"id": "new-ref",
408+
},
409+
},
410+
}
411+
412+
private := privateData{}
413+
diags := integration_policy.HandleReqRespSecrets(ctx, req, resp, &private)
414+
require.Empty(t, diags)
415+
416+
// Should preserve the original secret ref structure since original is also a secret ref
417+
expected := map[string]any{
418+
"existing_secret": map[string]any{
419+
"isSecretRef": true,
420+
"id": "existing-ref",
421+
},
422+
}
423+
require.Equal(t, expected, *resp.Vars)
424+
})
425+
426+
t.Run("handles migration from plain text to secret ref", func(t *testing.T) {
427+
// Simulate migration where plain text becomes secret ref
428+
req := kbapi.PackagePolicyRequest{
429+
Vars: &map[string]any{
430+
"password": "plain-text-password",
431+
},
432+
}
433+
434+
resp := &kbapi.PackagePolicy{
435+
SecretReferences: &[]kbapi.PackagePolicySecretRef{{Id: "password-ref"}},
436+
Vars: &map[string]any{
437+
"password": map[string]any{
438+
"isSecretRef": true,
439+
"id": "password-ref",
440+
},
441+
},
442+
}
443+
444+
private := privateData{}
445+
diags := integration_policy.HandleReqRespSecrets(ctx, req, resp, &private)
446+
require.Empty(t, diags)
447+
448+
// Should replace secret ref with original plain text
449+
expected := map[string]any{"password": "plain-text-password"}
450+
require.Equal(t, expected, *resp.Vars)
451+
452+
// Should save the mapping for future use
453+
expectedPrivate := `{"password-ref":"plain-text-password"}`
454+
require.Equal(t, expectedPrivate, private["secrets"])
455+
})
456+
457+
t.Run("handles list migration scenarios", func(t *testing.T) {
458+
req := kbapi.PackagePolicyRequest{
459+
Vars: &map[string]any{
460+
"hosts": []any{"host1.example.com", "host2.example.com", "host3.example.com"},
461+
},
462+
}
463+
464+
resp := &kbapi.PackagePolicy{
465+
SecretReferences: &[]kbapi.PackagePolicySecretRef{
466+
{Id: "host-ref-1"}, {Id: "host-ref-2"}, {Id: "host-ref-3"},
467+
},
468+
Vars: &map[string]any{
469+
"hosts": map[string]any{
470+
"isSecretRef": true,
471+
"ids": []any{"host-ref-1", "host-ref-2", "host-ref-3"},
472+
},
473+
},
474+
}
475+
476+
private := privateData{}
477+
diags := integration_policy.HandleReqRespSecrets(ctx, req, resp, &private)
478+
require.Empty(t, diags)
479+
480+
// Should replace list secret refs with original array
481+
expected := map[string]any{"hosts": []any{"host1.example.com", "host2.example.com", "host3.example.com"}}
482+
require.Equal(t, expected, *resp.Vars)
483+
484+
// Should save individual mappings
485+
expectedPrivate := `{"host-ref-1":"host1.example.com","host-ref-2":"host2.example.com","host-ref-3":"host3.example.com"}`
486+
require.Equal(t, expectedPrivate, private["secrets"])
487+
})
488+
}

0 commit comments

Comments
 (0)