Accept secret references in both formats: inline and path (#5852)

* Rename function for consistency

* Add unit test

* Implement fallback for inputs secrets replacement

* Better naming

* Add comment

* Rename for broader use

* WIP

* Remove unnecessary error returns

* Make sure to overwrite replaced inline secret keys

* Remove TODO

* Add assertions for output secrets

* Remove inaccurate comment

* Adding CHANGELOG fragment

* Update secret unit tests

* [Debugging] Don't zero out secret references

* Add output secret to references

* Undo debugging change

Author: ycombinator

changelog/fragments/1762435732-secrets-fallback.yaml

@@ -0,0 +1,36 @@
+# Kind can be one of:
+# - breaking-change: a change to previously-documented behavior
+# - deprecation: functionality that is being removed in a later release
+# - bug-fix: fixes a problem in a previous version
+# - enhancement: extends functionality but does not break or fix existing behavior
+# - feature: new functionality
+# - known-issue: problems that we are aware of in a given version
+# - security: impacts on the security of a product or a user’s deployment.
+# - upgrade: important information for someone upgrading from a prior version
+# - other: does not fit into any of the other categories
+kind: enhancement
+
+# Change summary; a 80ish characters long description of the change.
+summary: Accept secret references in policies in either inline or path formats
+
+# Long description; in case the summary is not enough to describe the change
+# this field accommodate a description without length limits.
+# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment.
+description: |
+  Elastic Agent policies can contain secret references in one of two formats: inline or path.
+  With the inline format, the reference looks like this: `<path>: $co.elastic.secret{<secret ref>}`. 
+  With the path format, the reference looks like this: `secrets.<path>.id:<secret ref>`.
+  This change ensures that Fleet Server accepts secret references in policies in either format.
+
+# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc.
+component: fleet-server
+
+# PR URL; optional; the PR number that added the changeset.
+# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added.
+# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number.
+# Please provide it if you are adding a fragment for a different PR.
+#pr: https://github.com/owner/repo/1234
+
+# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of).
+# If not present is automatically filled by the tooling with the issue linked to the PR number.
+#issue: https://github.com/owner/repo/1234

internal/pkg/api/handleCheckin.go

@@ -879,14 +879,17 @@ func processPolicy(ctx context.Context, zlog zerolog.Logger, bulker bulk.Bulk, a
 		return nil, ErrNoPolicyOutput
 	}
 
+	// Get secret values so secret references in outputs and inputs can be replaced
+	// with their corresponding values.
+	secretValues, err := secret.GetSecretValues(ctx, pp.Policy.Data.SecretReferences, bulker)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get secret values: %w", err)
+	}
+
 	data := model.ClonePolicyData(pp.Policy.Data)
-	for policyName, policyOutput := range data.Outputs {
+	for _, policyOutput := range data.Outputs {
 		// NOTE: Not sure if output secret keys collected here include new entries, but they are collected for completeness
-		ks, err := secret.ProcessOutputSecret(ctx, policyOutput, bulker) // makes a bulk request to get secret values
-		if err != nil {
-			return nil, fmt.Errorf("failed to process output secrets %q: %w",
-				policyName, err)
-		}
+		ks := secret.ProcessOutputSecret(policyOutput, secretValues)
 		pp.SecretKeys = append(pp.SecretKeys, ks...)
 	}
 	// Iterate through the policy outputs and prepare them

internal/pkg/policy/parsed_policy.go

@@ -63,15 +63,19 @@ func NewParsedPolicy(ctx context.Context, bulker bulk.Bulk, p model.Policy) (*Pa
 		return nil, err
 	}
 
+	// Get secret values so secret references in outputs and inputs can be replaced
+	// with their corresponding values.
+	secretValues, err := secret.GetSecretValues(ctx, p.Data.SecretReferences, bulker)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get secret values: %w", err)
+	}
+
 	policyOutputs, err := constructPolicyOutputs(p.Data.Outputs, roles)
 	if err != nil {
 		return nil, err
 	}
 	for name, policyOutput := range p.Data.Outputs {
-		ks, err := secret.ProcessOutputSecret(ctx, policyOutput, bulker)
-		if err != nil {
-			return nil, err
-		}
+		ks := secret.ProcessOutputSecret(policyOutput, secretValues)
 		for _, key := range ks {
 			secretKeys = append(secretKeys, "outputs."+name+"."+key)
 		}
@@ -80,12 +84,12 @@ func NewParsedPolicy(ctx context.Context, bulker bulk.Bulk, p model.Policy) (*Pa
 	if err != nil {
 		return nil, err
 	}
-	policyInputs, keys, err := secret.GetPolicyInputsWithSecrets(ctx, p.Data, bulker)
-	if err != nil {
-		return nil, err
-	}
+	policyInputs, keys := secret.ProcessInputsSecrets(p.Data, secretValues)
 	secretKeys = append(secretKeys, keys...)
 
+	// Done replacing secrets.
+	p.Data.SecretReferences = nil
+
 	// We are cool and the gang
 	pp := &ParsedPolicy{
 		Policy:  p,

internal/pkg/policy/parsed_policy_test.go

@@ -15,6 +15,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/elastic/fleet-server/v7/internal/pkg/model"
+	ftesting "github.com/elastic/fleet-server/v7/internal/pkg/testing"
 )
 
 //go:embed testdata/test_policy.json
@@ -29,6 +30,9 @@ var logstashOutputPolicy string
 //go:embed testdata/remote_es_policy.json
 var testPolicyRemoteES string
 
+//go:embed testdata/policy_with_secrets_mixed.json
+var policyWithSecretsMixed string
+
 func TestNewParsedPolicy(t *testing.T) {
 	// Run two formatting of the same payload to validate that the sha2 remains the same
 	testcases := []struct {
@@ -102,3 +106,36 @@ func TestNewParsedPolicyRemoteES(t *testing.T) {
 	// Validate that default was found
 	require.Equal(t, "remote", pp.Default.Name)
 }
+
+// TestParsedPolicyMixedSecretsReplacement tests that secrets specified in a policy
+// using either the `secrets.<path-to-key>.<key>.id:<secret ref>` format or the
+// `<path>: $co.elastic.secret{<secret ref>}` format are both replaced correctly.
+func TestParsedPolicyMixedSecretsReplacement(t *testing.T) {
+	// Load the model into the policy object
+	var m model.Policy
+	var d model.PolicyData
+	err := json.Unmarshal([]byte(policyWithSecretsMixed), &d)
+	require.NoError(t, err)
+
+	m.Data = &d
+
+	bulker := ftesting.NewMockBulk()
+	pp, err := NewParsedPolicy(context.TODO(), bulker, m)
+	require.NoError(t, err)
+
+	// Validate that secrets were identified
+	require.Len(t, pp.SecretKeys, 4)
+	require.Contains(t, pp.SecretKeys, "outputs.fs-output.type")
+	require.Contains(t, pp.SecretKeys, "outputs.fs-output.ssl.key")
+	require.Contains(t, pp.SecretKeys, "inputs.0.streams.0.auth.basic.password")
+	require.Contains(t, pp.SecretKeys, "inputs.0.streams.1.auth.basic.password")
+
+	// Validate that secret references were replaced
+	firstInputStreams := pp.Inputs[0]["streams"].([]any)
+	firstInputFirstStream := firstInputStreams[0].(map[string]any)
+	firstInputSecondStream := firstInputStreams[1].(map[string]any)
+	require.Equal(t, "0Mx2UZoBTAyw4gQKSaao_value", firstInputFirstStream["auth.basic.password"])
+	require.Equal(t, "0Mx2UZoBTAyw4gQKSaao_value", firstInputSecondStream["auth.basic.password"])
+	require.Equal(t, "abcdef123_value", pp.Policy.Data.Outputs["fs-output"]["type"])
+	require.Equal(t, "w8yELZoBTAyw4gQK9KZ7_value", pp.Policy.Data.Outputs["fs-output"]["ssl"].(map[string]interface{})["key"])
+}