From 91894d067b105bb2a6444d86dcf5d5a8e6ab7540 Mon Sep 17 00:00:00 2001 From: Dominik Giger Date: Wed, 17 Sep 2025 16:32:25 +0200 Subject: [PATCH 1/2] Implement ids handler for secret values --- internal/fleet/integration_policy/secrets.go | 41 ++- .../fleet/integration_policy/secrets_test.go | 244 +++++++++++++++++- 2 files changed, 278 insertions(+), 7 deletions(-) diff --git a/internal/fleet/integration_policy/secrets.go b/internal/fleet/integration_policy/secrets.go index 0a5ef36a8..af0e3c02d 100644 --- a/internal/fleet/integration_policy/secrets.go +++ b/internal/fleet/integration_policy/secrets.go @@ -71,9 +71,25 @@ func HandleRespSecrets(ctx context.Context, resp *kbapi.PackagePolicy, private p } handleVar := func(key string, mval map[string]any, vars map[string]any) { - refID := mval["id"].(string) - if original, ok := secrets[refID]; ok { - vars[key] = original + // Handle single secret reference: {"id": "refID", "isSecretRef": true} + if refID, ok := mval["id"].(string); ok { + if original, ok := secrets[refID]; ok { + vars[key] = original + } + return + } + + // Handle list secret reference: {"ids": ["a", "b"], "isSecretRef": true} + if refIDs, ok := mval["ids"].([]any); ok { + resolvedValues := make([]any, 0, len(refIDs)) + for _, refIDInterface := range refIDs { + if refID, ok := refIDInterface.(string); ok { + if original, ok := secrets[refID]; ok { + resolvedValues = append(resolvedValues, original) + } + } + } + vars[key] = resolvedValues } } @@ -136,8 +152,23 @@ func HandleReqRespSecrets(ctx context.Context, req kbapi.PackagePolicyRequest, r } } - refID := mval["id"].(string) - secrets[refID] = original + // Handle single secret reference: {"id": "refID", "isSecretRef": true} + if refID, ok := mval["id"].(string); ok { + secrets[refID] = original + return + } + + // Handle list secret reference: {"ids": ["a", "b"], "isSecretRef": true} + if refIDs, ok := mval["ids"].([]any); ok { + // For list secrets, the original should be an array + if originalArray, ok := original.([]any); ok { + for i, refIDInterface := range refIDs { + if refID, ok := refIDInterface.(string); ok && i < len(originalArray) { + secrets[refID] = originalArray[i] + } + } + } + } } } diff --git a/internal/fleet/integration_policy/secrets_test.go b/internal/fleet/integration_policy/secrets_test.go index c62348c00..37a167169 100644 --- a/internal/fleet/integration_policy/secrets_test.go +++ b/internal/fleet/integration_policy/secrets_test.go @@ -79,6 +79,26 @@ func TestHandleRespSecrets(t *testing.T) { input: Map{"k": Map{"type": "password", "value": Map{"isSecretRef": true, "id": "unknown-secret"}}}, want: Map{"k": Map{"isSecretRef": true, "id": "unknown-secret"}}, }, + { + name: "converts list secret", + input: Map{"k": Map{"isSecretRef": true, "ids": []any{"known-secret"}}}, + want: Map{"k": []any{"secret"}}, + }, + { + name: "converts multiple list secrets", + input: Map{"k": Map{"isSecretRef": true, "ids": []any{"known-secret", "known-secret"}}}, + want: Map{"k": []any{"secret", "secret"}}, + }, + { + name: "converts mixed list secrets", + input: Map{"k": Map{"isSecretRef": true, "ids": []any{"known-secret", "unknown-secret"}}}, + want: Map{"k": []any{"secret"}}, + }, + { + name: "converts wrapped list secret", + input: Map{"k": Map{"type": "array", "value": Map{"isSecretRef": true, "ids": []any{"known-secret"}}}}, + want: Map{"k": []any{"secret"}}, + }, } for _, tt := range tests { @@ -184,6 +204,30 @@ func TestHandleReqRespSecrets(t *testing.T) { respInput: Map{"k": Map{"type": "password", "value": Map{"isSecretRef": true, "id": "unknown-secret"}}}, want: Map{"k": Map{"isSecretRef": true, "id": "unknown-secret"}}, }, + { + name: "converts list secret", + reqInput: Map{"k": []any{"secret1", "secret2"}}, + respInput: Map{"k": Map{"isSecretRef": true, "ids": []any{"ref1", "ref2"}}}, + want: Map{"k": []any{"secret1", "secret2"}}, + }, + { + name: "converts wrapped list secret", + reqInput: Map{"k": []any{"secret1", "secret2"}}, + respInput: Map{"k": Map{"type": "array", "value": Map{"isSecretRef": true, "ids": []any{"ref1", "ref2"}}}}, + want: Map{"k": []any{"secret1", "secret2"}}, + }, + { + name: "converts partial list secret", + reqInput: Map{"k": []any{"secret1"}}, + respInput: Map{"k": Map{"isSecretRef": true, "ids": []any{"ref1", "ref2"}}}, + want: Map{"k": []any{"secret1"}}, + }, + { + name: "converts empty list secret", + reqInput: Map{"k": []any{}}, + respInput: Map{"k": Map{"isSecretRef": true, "ids": []any{"ref1"}}}, + want: Map{"k": []any{}}, + }, } for _, tt := range tests { @@ -236,13 +280,209 @@ func TestHandleReqRespSecrets(t *testing.T) { want = *(*wants.Inputs["input1"].Streams)["stream1"].Vars require.Equal(t, want, got) - if v, ok := (*req.Vars)["k"]; ok && v == "secret" { + // Check private data expectations based on test case + switch tt.name { + case "converts secret", "converts wrapped secret": privateWants := privateData{"secrets": `{"known-secret":"secret"}`} require.Equal(t, privateWants, private) - } else { + case "converts list secret", "converts wrapped list secret": + privateWants := privateData{"secrets": `{"ref1":"secret1","ref2":"secret2"}`} + require.Equal(t, privateWants, private) + case "converts partial list secret": + privateWants := privateData{"secrets": `{"ref1":"secret1"}`} + require.Equal(t, privateWants, private) + case "converts empty list secret": + privateWants := privateData{"secrets": `{}`} + require.Equal(t, privateWants, private) + default: privateWants := privateData{"secrets": `{}`} require.Equal(t, privateWants, private) } }) } } + +func TestEdgeCases(t *testing.T) { + t.Parallel() + + ctx := context.Background() + + t.Run("handles deeply nested secret references", func(t *testing.T) { + private := privateData{"secrets": `{"nested-ref":"nested-secret"}`} + + resp := &kbapi.PackagePolicy{ + SecretReferences: &[]kbapi.PackagePolicySecretRef{{Id: "nested-ref"}}, + Vars: &map[string]any{ + "level1": map[string]any{ + "type": "object", + "value": map[string]any{ + "level2": map[string]any{ + "isSecretRef": true, + "id": "nested-ref", + }, + }, + }, + }, + } + + diags := integration_policy.HandleRespSecrets(ctx, resp, &private) + require.Empty(t, diags) + + expected := map[string]any{ + "level1": map[string]any{ + "level2": "nested-secret", + }, + } + require.Equal(t, expected, *resp.Vars) + }) + + t.Run("handles multiple input streams", func(t *testing.T) { + private := privateData{"secrets": `{"stream1-ref":"stream1-secret","stream2-ref":"stream2-secret"}`} + + resp := &kbapi.PackagePolicy{ + SecretReferences: &[]kbapi.PackagePolicySecretRef{ + {Id: "stream1-ref"}, {Id: "stream2-ref"}, + }, + Inputs: map[string]kbapi.PackagePolicyInput{ + "input1": { + Streams: &map[string]kbapi.PackagePolicyInputStream{ + "stream1": { + Vars: &map[string]any{ + "secret1": map[string]any{"isSecretRef": true, "id": "stream1-ref"}, + }, + }, + "stream2": { + Vars: &map[string]any{ + "secret2": map[string]any{"isSecretRef": true, "id": "stream2-ref"}, + }, + }, + }, + }, + }, + } + + diags := integration_policy.HandleRespSecrets(ctx, resp, &private) + require.Empty(t, diags) + + streams := *resp.Inputs["input1"].Streams + require.Equal(t, "stream1-secret", (*streams["stream1"].Vars)["secret1"]) + require.Equal(t, "stream2-secret", (*streams["stream2"].Vars)["secret2"]) + }) + + t.Run("handles invalid JSON in private data", func(t *testing.T) { + private := privateData{"secrets": `{"invalid": json}`} + + resp := &kbapi.PackagePolicy{ + SecretReferences: &[]kbapi.PackagePolicySecretRef{}, + Vars: &map[string]any{}, + } + + diags := integration_policy.HandleRespSecrets(ctx, resp, &private) + require.True(t, diags.HasError(), "Expected error diagnostics") + }) +} + +func TestMigrationScenarios(t *testing.T) { + t.Parallel() + + // Test pre-0.11.7 migration scenarios mentioned in HandleReqRespSecrets + ctx := context.Background() + + t.Run("handles importing existing secret refs", func(t *testing.T) { + // Simulate importing a resource that already has secret refs in request + req := kbapi.PackagePolicyRequest{ + Vars: &map[string]any{ + "existing_secret": map[string]any{ + "isSecretRef": true, + "id": "existing-ref", + }, + }, + } + + resp := &kbapi.PackagePolicy{ + SecretReferences: &[]kbapi.PackagePolicySecretRef{{Id: "new-ref"}}, + Vars: &map[string]any{ + "existing_secret": map[string]any{ + "isSecretRef": true, + "id": "new-ref", + }, + }, + } + + private := privateData{} + diags := integration_policy.HandleReqRespSecrets(ctx, req, resp, &private) + require.Empty(t, diags) + + // Should preserve the original secret ref structure since original is also a secret ref + expected := map[string]any{ + "existing_secret": map[string]any{ + "isSecretRef": true, + "id": "existing-ref", + }, + } + require.Equal(t, expected, *resp.Vars) + }) + + t.Run("handles migration from plain text to secret ref", func(t *testing.T) { + // Simulate migration where plain text becomes secret ref + req := kbapi.PackagePolicyRequest{ + Vars: &map[string]any{ + "password": "plain-text-password", + }, + } + + resp := &kbapi.PackagePolicy{ + SecretReferences: &[]kbapi.PackagePolicySecretRef{{Id: "password-ref"}}, + Vars: &map[string]any{ + "password": map[string]any{ + "isSecretRef": true, + "id": "password-ref", + }, + }, + } + + private := privateData{} + diags := integration_policy.HandleReqRespSecrets(ctx, req, resp, &private) + require.Empty(t, diags) + + // Should replace secret ref with original plain text + expected := map[string]any{"password": "plain-text-password"} + require.Equal(t, expected, *resp.Vars) + + // Should save the mapping for future use + expectedPrivate := `{"password-ref":"plain-text-password"}` + require.Equal(t, expectedPrivate, private["secrets"]) + }) + + t.Run("handles list migration scenarios", func(t *testing.T) { + req := kbapi.PackagePolicyRequest{ + Vars: &map[string]any{ + "hosts": []any{"host1.example.com", "host2.example.com", "host3.example.com"}, + }, + } + + resp := &kbapi.PackagePolicy{ + SecretReferences: &[]kbapi.PackagePolicySecretRef{ + {Id: "host-ref-1"}, {Id: "host-ref-2"}, {Id: "host-ref-3"}, + }, + Vars: &map[string]any{ + "hosts": map[string]any{ + "isSecretRef": true, + "ids": []any{"host-ref-1", "host-ref-2", "host-ref-3"}, + }, + }, + } + + private := privateData{} + diags := integration_policy.HandleReqRespSecrets(ctx, req, resp, &private) + require.Empty(t, diags) + + // Should replace list secret refs with original array + expected := map[string]any{"hosts": []any{"host1.example.com", "host2.example.com", "host3.example.com"}} + require.Equal(t, expected, *resp.Vars) + + // Should save individual mappings + expectedPrivate := `{"host-ref-1":"host1.example.com","host-ref-2":"host2.example.com","host-ref-3":"host3.example.com"}` + require.Equal(t, expectedPrivate, private["secrets"]) + }) +} From de2ed8c32a7dca7da28616e178f66acd6e48e13f Mon Sep 17 00:00:00 2001 From: Dominik Giger Date: Thu, 18 Sep 2025 14:49:49 +0200 Subject: [PATCH 2/2] Ignore diff when using empty json object {}. --- internal/fleet/integration_policy/models.go | 17 +++++++- .../fleet/integration_policy/models_test.go | 39 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/internal/fleet/integration_policy/models.go b/internal/fleet/integration_policy/models.go index 0ffe6014e..a6169fee8 100644 --- a/internal/fleet/integration_policy/models.go +++ b/internal/fleet/integration_policy/models.go @@ -50,7 +50,7 @@ func (model *integrationPolicyModel) populateFromAPI(ctx context.Context, data * model.Enabled = types.BoolValue(data.Enabled) model.IntegrationName = types.StringValue(data.Package.Name) model.IntegrationVersion = types.StringValue(data.Package.Version) - model.VarsJson = utils.MapToNormalizedType(utils.Deref(data.Vars), path.Root("vars_json"), &diags) + model.VarsJson = normalizeVarsJson(model.VarsJson, utils.MapToNormalizedType(utils.Deref(data.Vars), path.Root("vars_json"), &diags)) model.populateInputFromAPI(ctx, data.Inputs, &diags) @@ -64,7 +64,7 @@ func (model *integrationPolicyModel) populateInputFromAPI(ctx context.Context, i InputID: types.StringValue(meta.Key), Enabled: types.BoolPointerValue(inputData.Enabled), StreamsJson: utils.MapToNormalizedType(utils.Deref(inputData.Streams), meta.Path.AtName("streams_json"), diags), - VarsJson: utils.MapToNormalizedType(utils.Deref(inputData.Vars), meta.Path.AtName("vars_json"), diags), + VarsJson: normalizeVarsJson(model.VarsJson, utils.MapToNormalizedType(utils.Deref(inputData.Vars), meta.Path.AtName("vars_json"), diags)), } }) if newInputs == nil { @@ -147,3 +147,16 @@ func sortInputs(incoming []integrationPolicyInputModel, existing []integrationPo return iIdx < jIdx }) } + +// normalizeVarsJson handles the case where both nil (from API) and "{}" (from config) +// represent an empty JSON object. If the existing value is "{}" and the incoming +// value is null (from API returning nil), preserve the "{}" to avoid unwanted diffs. +func normalizeVarsJson(existing jsontypes.Normalized, incoming jsontypes.Normalized) jsontypes.Normalized { + if incoming.IsNull() && !existing.IsNull() && !existing.IsUnknown() { + existingValue := existing.ValueString() + if existingValue == "{}" { + return jsontypes.NewNormalizedValue("{}") + } + } + return incoming +} diff --git a/internal/fleet/integration_policy/models_test.go b/internal/fleet/integration_policy/models_test.go index b8362f11f..26b4ca324 100644 --- a/internal/fleet/integration_policy/models_test.go +++ b/internal/fleet/integration_policy/models_test.go @@ -3,6 +3,7 @@ package integration_policy import ( "testing" + "github.com/hashicorp/terraform-plugin-framework-jsontypes/jsontypes" "github.com/hashicorp/terraform-plugin-framework/types" "github.com/stretchr/testify/require" ) @@ -62,3 +63,41 @@ func Test_SortInputs(t *testing.T) { require.Equal(t, want, incoming) }) } + +func TestNormalizeVarsJson(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + existing jsontypes.Normalized + api jsontypes.Normalized + want jsontypes.Normalized + }{ + { + name: "plan defines empty object, but api returns null -> preserve empty object", + existing: jsontypes.NewNormalizedValue("{}"), + api: jsontypes.NewNormalizedNull(), + want: jsontypes.NewNormalizedValue("{}"), + }, + { + name: "plan does not define value, api returns null -> preserve null", + existing: jsontypes.NewNormalizedUnknown(), + api: jsontypes.NewNormalizedNull(), + want: jsontypes.NewNormalizedNull(), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := normalizeVarsJson(tt.existing, tt.api) + + // Compare the states and values + require.Equal(t, tt.want.IsNull(), result.IsNull(), "IsNull() should match") + require.Equal(t, tt.want.IsUnknown(), result.IsUnknown(), "IsUnknown() should match") + + if !tt.want.IsNull() && !tt.want.IsUnknown() { + require.Equal(t, tt.want.ValueString(), result.ValueString(), "ValueString() should match") + } + }) + } +}