Fetch each secret only once, fix replacement of nested map values

README.md

@@ -1,17 +1,22 @@
 # go-cloudsecrets
 
-Go package to hydrate runtime secrets from Cloud providers
+Go package for hydrating config secrets from Cloud secret managers
 - [x] `"gcp"`, GCP Secret Manager
 - [ ] `"aws"`, AWS Secrets Manager
+- [ ] `""`, empty provider, which errors out on `$SECRET:` value
 
 ```go
 cloudsecrets.Hydrate(ctx, "gcp", &Config{})
 ```
 
-`Hydrate()` recursively walks a given config (struct pointer) and hydrates all string
-values matching `"$SECRET:"` prefix using a given Cloud secrets provider.
+`Hydrate()` recursively walks given config (a `struct` pointer) and replaces all string
+fields having `"$SECRET:"` prefix with a value fetched from a given Cloud secret provider.
 
-The secret values to be replaced must have a format of `"$SECRET:{name|path}"`.
+The value to be replaced must have a format of `"$SECRET:{name|path}"`.
+
+Secrets are de-duplicated and fetched only once.
+
+The `Hydrate()` function tries to replace as many fields as possible before returning error.
 
 ## Usage
 ```go
@@ -23,7 +28,7 @@ func main() {
 			Database: "postgres",
 			Host:     "localhost:5432",
 			Username: "sequence",
-			DPassword: "$SECRET:dbPassword", // to be hydrated
+			DPassword: "$SECRET:dbPassword", // will be hydrated (replaced with value of "dbPassword" secret)
 		},
 	}
 

collector.go

@@ -1,85 +1,57 @@
 package cloudsecrets
 
 import (
-	"errors"
-	"fmt"
 	"reflect"
+	"slices"
 	"strings"
 )
 
-type secretField struct {
-	value      reflect.Value
-	fieldPath  string
-	secretName string
-}
+// Returns de-duplicated secret keys found recursively in given v.
+func collectSecretKeys(v reflect.Value) []string {
+	c := collector{}
+	c.collectSecretFields(v)
+
+	slices.Sort(c)
+	dedup := slices.Compact(c)
 
-type collector struct {
-	fields []*secretField
-	hooks  []func()
-	err    error
+	return []string(dedup)
 }
 
-// Walks given reflect value recursively and collects any string fields with $SECRET: prefix.
-func (g *collector) collectSecretFields(v reflect.Value, path string) {
+type collector []string
+
+// Walk given reflect value recursively and collects any string fields matching $SECRET: prefix.
+func (c *collector) collectSecretFields(v reflect.Value) {
 	switch v.Kind() {
 	case reflect.Ptr:
 		if v.IsNil() {
 			return
 		}
-
-		// Dereference pointer
-		g.collectSecretFields(v.Elem(), path)
+		c.collectSecretFields(v.Elem())
 
 	case reflect.Struct:
 		for i := 0; i < v.NumField(); i++ {
 			field := v.Field(i)
-			g.collectSecretFields(field, fmt.Sprintf("%v.%v", path, v.Type().Field(i).Name))
+			c.collectSecretFields(field)
 		}
 
 	case reflect.Slice, reflect.Array:
 		for i := 0; i < v.Len(); i++ {
 			item := v.Index(i)
-			g.collectSecretFields(item, fmt.Sprintf("%v[%v]", path, i))
+			c.collectSecretFields(item)
 		}
 
 	case reflect.Map:
 		for _, key := range v.MapKeys() {
 			item := v.MapIndex(key)
-
-			if item.Kind() == reflect.Struct {
-				// If the value is a struct, create a pointer to the map value and modify via pointer
-				ptr := reflect.New(item.Type())
-				ptr.Elem().Set(item)
-
-				g.hooks = append(g.hooks, func() {
-					v.SetMapIndex(key, ptr.Elem())
-				})
-
-				g.collectSecretFields(ptr, fmt.Sprintf("%v[%v]", path, key))
-
-				// Set the modified struct back into the map
-
-			} else {
-				g.collectSecretFields(item, fmt.Sprintf("%v[%v]", path, key))
-			}
+			c.collectSecretFields(item)
 		}
 
 	case reflect.String:
 		secretName, found := strings.CutPrefix(v.String(), "$SECRET:")
 		if !found {
 			return
 		}
-
-		if !v.CanSet() {
-			g.err = errors.Join(g.err, fmt.Errorf("can't set field %v", path))
-			return
-		}
-
-		g.fields = append(g.fields, &secretField{
-			value:      v,
-			fieldPath:  path,
-			secretName: secretName,
-		})
+		*c = append(*c, secretName)
 
 	default:
 		return

collector_test.go

@@ -1,17 +1,17 @@
 package cloudsecrets
 
 import (
-	"fmt"
 	"reflect"
-	"sort"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
 )
 
-func TestCollectFields(t *testing.T) {
+func TestCollectSecretKeys(t *testing.T) {
 	tt := []struct {
 		Name  string
 		Input any
-		Out   []string // field paths
+		Out   []string // collected secret keys
 		Error bool
 	}{
 		{
@@ -71,7 +71,7 @@ func TestCollectFields(t *testing.T) {
 				},
 				JWTSecrets: []jwtSecret{"$SECRET:jwtSecret1", "$SECRET:jwtSecret2", "nope"},
 			},
-			Out: []string{"secretName", "jwtSecret1", "jwtSecret2"},
+			Out: []string{"jwtSecret1", "jwtSecret2", "secretName"},
 		},
 		{
 			Name: "Slice_of_secret_pointer_values",
@@ -82,7 +82,7 @@ func TestCollectFields(t *testing.T) {
 				},
 				JWTSecretsPtr: []*jwtSecret{ptr(jwtSecret("$SECRET:jwtSecret1")), ptr(jwtSecret("$SECRET:jwtSecret2")), ptr(jwtSecret("nope"))},
 			},
-			Out: []string{"secretName", "jwtSecret1", "jwtSecret2"},
+			Out: []string{"jwtSecret1", "jwtSecret2", "secretName"},
 		},
 		{
 			Name: "Map_with_values",
@@ -107,51 +107,31 @@ func TestCollectFields(t *testing.T) {
 			Out: []string{"secretProvider1", "secretProvider2", "secretProvider3"},
 		},
 		{
-			Name: "Unexported_field_should_fail_to_hydrate",
+			Name: "Duplicated_secret",
 			Input: &cfg{
-				unexported: dbConfig{ // unexported fields can't be updated via reflect pkg
+				DB: dbConfig{
 					User:     "db-user",
-					Password: "$SECRET:secretName", // match inside unexported field
+					Password: "$SECRET:duplicatedKey",
+				},
+				JWTSecrets: []jwtSecret{"$SECRET:duplicatedKey", "$SECRET:duplicatedKey"},
+				ProvidersPtr: map[string]*providerConfig{
+					"provider1": {Name: "provider1", Secret: "$SECRET:duplicatedKey"},
+					"provider2": {Name: "provider2", Secret: "$SECRET:duplicatedKey"},
+					"provider3": {Name: "provider3", Secret: "$SECRET:duplicatedKey"},
 				},
 			},
-			Out:   []string{},
-			Error: true, // expect error
+			Out: []string{"duplicatedKey"},
 		},
 	}
 
-	for i, tc := range tt {
-		i, tc := i, tc
+	for _, tc := range tt {
+		tc := tc
 		t.Run(tc.Name, func(t *testing.T) {
 			v := reflect.ValueOf(tc.Input)
 
-			c := &collector{}
-			c.collectSecretFields(v, fmt.Sprintf("tt[%v].input", i))
-
-			if tc.Error {
-				if c.err == nil {
-					t.Error("expected error, got nil")
-					return
-				}
-			} else {
-				if c.err != nil {
-					t.Errorf("unexpected error: %v", c.err)
-					return
-				}
-			}
-
-			if len(c.fields) != len(tc.Out) {
-				t.Errorf("expected %v secrets, got %v", len(tc.Out), len(c.fields))
-			}
-
-			fields := c.fields
-			sort.Slice(fields, func(i, j int) bool {
-				return fields[i].fieldPath <= fields[j].fieldPath
-			})
-
-			for i := 0; i < len(fields); i++ {
-				if fields[i].secretName != tc.Out[i] {
-					t.Errorf("collected field[%v].secretName=%v doesn't match tc.Out[%v]=%v", i, fields[i].secretName, i, tc.Out[i])
-				}
+			secretFields := collectSecretKeys(v)
+			if !cmp.Equal(secretFields, tc.Out) {
+				t.Errorf(cmp.Diff(tc.Out, secretFields))
 			}
 		})
 	}

gcp/gcp.go

@@ -21,22 +21,22 @@ type SecretsProvider struct {
 func NewSecretsProvider() (*SecretsProvider, error) {
 	gcpClient, err := secretmanager.NewClient(context.Background())
 	if err != nil {
-		return nil, fmt.Errorf("initializing GCP secret manager: %w", err)
+		return nil, fmt.Errorf("gcp: secretmanager client: %w", err)
 	}
 
 	var projectNumber string
 	if metadata.OnGCE() {
 		projectNumber, err = metadata.NumericProjectID()
 		if err != nil {
-			return nil, fmt.Errorf("getting project ID from metadata: %w", err)
+			return nil, fmt.Errorf("gcp: getting project ID from metadata: %w", err)
 		}
 	} else {
 		ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
 		defer cancel()
 
 		projectNumber, err = getProjectNumberFromGcloud(ctx)
 		if err != nil {
-			return nil, fmt.Errorf("getting project ID from gcloud: %w", err)
+			return nil, fmt.Errorf("gcp: getting project ID from gcloud: %w", err)
 		}
 	}
 
@@ -59,7 +59,7 @@ func (p SecretsProvider) FetchSecret(ctx context.Context, secretId string) (stri
 	// Access the secret version
 	result, err := p.client.AccessSecretVersion(reqCtx, req)
 	if err != nil {
-		return "", fmt.Errorf("fetching GCP secret %q: %w", secretId, err)
+		return "", fmt.Errorf("gcp: accessing secret: %w", err)
 	}
 
 	// Return the secret value
@@ -75,15 +75,15 @@ func getProjectNumberFromGcloud(ctx context.Context) (string, error) {
 	if projectId == "" {
 		out, err := exec.CommandContext(ctx, "gcloud", "config", "get-value", "project").Output()
 		if err != nil {
-			return "", fmt.Errorf("getting current gcloud project (try `gcloud auth application-default login'): %w", err)
+			return "", fmt.Errorf("gcp: getting current gcloud project (try `gcloud auth application-default login'): %w", err)
 		}
 		projectId = strings.TrimSpace(string(out))
 	}
 
 	// We need projectNumber (not projectName!) for GCP Secret Manager APIs.
 	out, err := exec.CommandContext(ctx, "gcloud", "projects", "describe", projectId, "--format=value(projectNumber)").Output()
 	if err != nil {
-		return "", fmt.Errorf("getting gcloud projectNumber from projectId %q: %w", projectId, err)
+		return "", fmt.Errorf("gcp: getting gcloud projectNumber from projectId %q: %w", projectId, err)
 	}
 	return strings.TrimSpace(string(out)), nil
 }

go.mod

@@ -17,6 +17,7 @@ require (
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
 	github.com/golang/protobuf v1.5.3 // indirect
+	github.com/google/go-cmp v0.6.0 // indirect
 	github.com/google/s2a-go v0.1.7 // indirect
 	github.com/googleapis/enterprise-certificate-proxy v0.3.2 // indirect
 	github.com/googleapis/gax-go/v2 v2.12.0 // indirect

go.work.sum

@@ -110,6 +110,7 @@ cloud.google.com/go/vpcaccess v1.7.4/go.mod h1:lA0KTvhtEOb/VOdnH/gwPuOzGgM+CWsmG
 cloud.google.com/go/webrisk v1.9.4/go.mod h1:w7m4Ib4C+OseSr2GL66m0zMBywdrVNTDKsdEsfMl7X0=
 cloud.google.com/go/websecurityscanner v1.6.4/go.mod h1:mUiyMQ+dGpPPRkHgknIZeCzSHJ45+fY4F52nZFDHm2o=
 cloud.google.com/go/workflows v1.12.3/go.mod h1:fmOUeeqEwPzIU81foMjTRQIdwQHADi/vEr1cx9R1m5g=
+github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
 github.com/census-instrumentation/opencensus-proto v0.4.1/go.mod h1:4T9NM4+4Vw91VeyqjLS6ao50K5bOcLKN6Q42XnYaRYw=
 github.com/cespare/xxhash/v2 v2.2.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs=
 github.com/cncf/udpa/go v0.0.0-20220112060539-c52dc94e7fbe/go.mod h1:6pvJx4me5XPnfI9Z40ddWsdw2W/uZgQLFXToKeRcDiI=
@@ -118,6 +119,7 @@ github.com/golang/glog v1.1.2/go.mod h1:zR+okUeTbrL6EL3xHUDxZuEtGv04p5shwip1+mL/
 github.com/google/go-pkcs11 v0.2.1-0.20230907215043-c6f79328ddf9/go.mod h1:6eQoGcuNJpa7jnd5pMGdkSaQpNDYvPlXWMcjXXThLlY=
 github.com/google/uuid v1.5.0 h1:1p67kYwdtXjb0gL0BPiP1Av9wiZPo5A8z2cWkTZ+eyU=
 github.com/google/uuid v1.5.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
+golang.org/x/exp v0.0.0-20190121172915-509febef88a4 h1:c2HOrn5iMezYjSlGPncknSEr/8x5LELb/ilJbXi9DEA=
 golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
 golang.org/x/term v0.16.0/go.mod h1:yn7UURbUtPyrVJPGPq404EukNFxcm/foM+bV/bfcDsY=
 golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=