Swap BulkDecrypt to return list (pulumi#18486)

In preparation for introducing a `BulkEncrypt([]string) []string` where inputs and outputs are correlated based on order, it will also be good to use the same pattern of access for the existing decrypt methods. This does not yet change the pattern of how these values are returned by the service for bulk decryption - only how the library functions operate. Most critical changes: - mapDecrypter.BulkDecrypt (pkg/resource/stack/secrets.go) - added missing test coverage here. - serviceCrypter.BulkDecrypt (pkg/secrets/service/manager.go) - already covered by existing tests. This changes the interface within sdk/go/common/resource/config but this does not appear to be implemented outside of pulumi/pkg based on GitHub search.
dytalabs-io · Feb 10, 2025 · ef48233 · ef48233
1 parent 44d3ab2
commit ef48233
Show file tree

Hide file tree

Showing 12 changed files with 90 additions and 42 deletions.
diff --git a/changelog/pending/20250207--pkg--change-bulkdecrypt-to-return-list-instead-of-map.yaml b/changelog/pending/20250207--pkg--change-bulkdecrypt-to-return-list-instead-of-map.yaml
@@ -0,0 +1,4 @@
+changes:
+- type: chore
+  scope: pkg
+  description: Change BulkDecrypt to return list instead of map
diff --git a/pkg/cmd/pulumi/config/io_test.go b/pkg/cmd/pulumi/config/io_test.go
@@ -302,9 +302,9 @@ func TestStackEnvConfig(t *testing.T) {
 				DecryptValueF: func() string {
 					return "plaintext"
 				},
-				BulkDecryptF: func() map[string]string {
-					return map[string]string{
-						"idontknow": "whatiamdoing",
+				BulkDecryptF: func() []string {
+					return []string{
+						"whatiamdoing",
 					}
 				},
 			}
@@ -387,9 +387,9 @@ func TestCopyConfig(t *testing.T) {
 				DecryptValueF: func() string {
 					return "plaintext"
 				},
-				BulkDecryptF: func() map[string]string {
-					return map[string]string{
-						"idontknow": "whatiamdoing",
+				BulkDecryptF: func() []string {
+					return []string{
+						"whatiamdoing",
 					}
 				},
 			}

diff --git a/pkg/engine/lifecycletest/pulumi_test.go b/pkg/engine/lifecycletest/pulumi_test.go
@@ -372,7 +372,7 @@ func (b brokenDecrypter) DecryptValue(_ context.Context, _ string) (string, erro
 	return "", errors.New(b.ErrorMessage)
 }
 
-func (b brokenDecrypter) BulkDecrypt(_ context.Context, _ []string) (map[string]string, error) {
+func (b brokenDecrypter) BulkDecrypt(_ context.Context, _ []string) ([]string, error) {
 	return nil, errors.New(b.ErrorMessage)
 }
 

diff --git a/pkg/resource/deploy/source_eval_test.go b/pkg/resource/deploy/source_eval_test.go
@@ -1777,7 +1777,7 @@ type decrypterMock struct {
 	DecryptValueF func(
 		ctx context.Context, ciphertext string) (string, error)
 	BulkDecryptF func(
-		ctx context.Context, ciphertexts []string) (map[string]string, error)
+		ctx context.Context, ciphertexts []string) ([]string, error)
 }
 
 var _ config.Decrypter = (*decrypterMock)(nil)
@@ -1789,7 +1789,7 @@ func (d *decrypterMock) DecryptValue(ctx context.Context, ciphertext string) (st
 	panic("unimplemented")
 }
 
-func (d *decrypterMock) BulkDecrypt(ctx context.Context, ciphertexts []string) (map[string]string, error) {
+func (d *decrypterMock) BulkDecrypt(ctx context.Context, ciphertexts []string) ([]string, error) {
 	if d.BulkDecryptF != nil {
 		return d.BulkDecryptF(ctx, ciphertexts)
 	}

diff --git a/pkg/resource/stack/deployment.go b/pkg/resource/stack/deployment.go
@@ -276,10 +276,15 @@ func DeserializeDeploymentV3(
 		}
 
 		// Decrypt the collected secrets and create a decrypter that will use the result as a cache.
-		cache, err := d.BulkDecrypt(ctx, ciphertexts)
+		decrypted, err := d.BulkDecrypt(ctx, ciphertexts)
 		if err != nil {
 			return nil, err
 		}
+		contract.Assertf(len(decrypted) == len(ciphertexts), "decrypted secrets count does not match ciphertexts count")
+		cache := make(map[string]string)
+		for i, ciphertext := range ciphertexts {
+			cache[ciphertext] = decrypted[i]
+		}
 		dec = newMapDecrypter(d, cache)
 
 		e, err := secretsManager.Encrypter()

diff --git a/pkg/resource/stack/secrets.go b/pkg/resource/stack/secrets.go
@@ -25,6 +25,7 @@ import (
 	"github.com/pulumi/pulumi/pkg/v3/secrets/service"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource"
 	"github.com/pulumi/pulumi/sdk/v3/go/common/resource/config"
+	"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
 )
 
 // DefaultSecretsProvider is the default SecretsProvider to use when deserializing deployments.
@@ -151,7 +152,7 @@ func (c *cachingCrypter) DecryptValue(ctx context.Context, ciphertext string) (s
 	return c.decrypter.DecryptValue(ctx, ciphertext)
 }
 
-func (c *cachingCrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) (map[string]string, error) {
+func (c *cachingCrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) ([]string, error) {
 	return c.decrypter.BulkDecrypt(ctx, ciphertexts)
 }
 
@@ -215,24 +216,28 @@ func (c *mapDecrypter) DecryptValue(ctx context.Context, ciphertext string) (str
 	return plaintext, nil
 }
 
-func (c *mapDecrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) (map[string]string, error) {
+func (c *mapDecrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) ([]string, error) {
 	// Loop and find the entries that are already cached, then BulkDecrypt the rest
-	secretMap := map[string]string{}
+	decryptedResult := make([]string, len(ciphertexts))
 	var toDecrypt []string
 	if c.cache == nil {
 		// Don't bother searching for the cached subset if the cache is nil
 		toDecrypt = ciphertexts
 	} else {
-		toDecrypt = make([]string, 0)
-		for _, ct := range ciphertexts {
+		toDecrypt = make([]string, 0, len(ciphertexts))
+		for i, ct := range ciphertexts {
 			if plaintext, ok := c.cache[ct]; ok {
-				secretMap[ct] = plaintext
+				decryptedResult[i] = plaintext
 			} else {
 				toDecrypt = append(toDecrypt, ct)
 			}
 		}
 	}
 
+	if len(toDecrypt) == 0 {
+		return decryptedResult, nil
+	}
+
 	// try and bulk decrypt the rest
 	decrypted, err := c.decrypter.BulkDecrypt(ctx, toDecrypt)
 	if err != nil {
@@ -243,11 +248,17 @@ func (c *mapDecrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) (m
 	if c.cache == nil {
 		c.cache = make(map[string]string)
 	}
-
-	for ct, pt := range decrypted {
-		secretMap[ct] = pt
+	for i, ct := range toDecrypt {
+		pt := decrypted[i]
 		c.cache[ct] = pt
 	}
 
-	return secretMap, nil
+	// Re-populate results
+	for i, ct := range ciphertexts {
+		plaintext, ok := c.cache[ct]
+		contract.Assertf(ok, "decrypted value not found in cache after bulk request")
+		decryptedResult[i] = plaintext
+	}
+
+	return decryptedResult, nil
 }
diff --git a/pkg/resource/stack/secrets_test.go b/pkg/resource/stack/secrets_test.go
@@ -69,7 +69,7 @@ func (t *testSecretsManager) DecryptValue(
 
 func (t *testSecretsManager) BulkDecrypt(
 	ctx context.Context, ciphertexts []string,
-) (map[string]string, error) {
+) ([]string, error) {
 	return config.DefaultBulkDecrypt(ctx, t, ciphertexts)
 }
 
@@ -206,6 +206,31 @@ func TestCachingCrypter(t *testing.T) {
 	assert.Equal(t, barSer, barSer2)
 }
 
+func TestBulkDecrypt(t *testing.T) {
+	t.Parallel()
+
+	ctx := context.Background()
+	sm := &testSecretsManager{}
+	decrypter, err := sm.Decrypter()
+	assert.NoError(t, err)
+	csm := newMapDecrypter(decrypter, map[string]string{})
+
+	decrypted, err := csm.BulkDecrypt(ctx, []string{"1:foo", "2:bar", "3:baz"})
+	assert.NoError(t, err)
+	assert.Equal(t, []string{"foo", "bar", "baz"}, decrypted)
+	assert.Equal(t, 3, sm.decryptCalls)
+
+	decryptedReordered, err := csm.BulkDecrypt(ctx, []string{"2:bar", "1:foo", "3:baz"}) // Re-ordered
+	assert.NoError(t, err)
+	assert.Equal(t, []string{"bar", "foo", "baz"}, decryptedReordered)
+	assert.Equal(t, 3, sm.decryptCalls) // No additional calls made
+
+	decrypted2, err := csm.BulkDecrypt(ctx, []string{"2:bar", "1:foo", "4:qux", "3:baz"}) // Add a new value
+	assert.NoError(t, err)
+	assert.Equal(t, []string{"bar", "foo", "qux", "baz"}, decrypted2)
+	assert.Equal(t, 4, sm.decryptCalls) // Only 1 additional call made
+}
+
 type mapTestSecretsProvider struct {
 	m *mapTestSecretsManager
 }
@@ -258,7 +283,7 @@ func (t *mapTestDecrypter) DecryptValue(
 
 func (t *mapTestDecrypter) BulkDecrypt(
 	ctx context.Context, ciphertexts []string,
-) (map[string]string, error) {
+) ([]string, error) {
 	t.bulkDecryptCalls++
 	return config.DefaultBulkDecrypt(ctx, t.d, ciphertexts)
 }

diff --git a/pkg/secrets/mock.go b/pkg/secrets/mock.go
@@ -78,7 +78,7 @@ func (me *MockEncrypter) EncryptValue(ctx context.Context, plaintext string) (st
 
 type MockDecrypter struct {
 	DecryptValueF func() string
-	BulkDecryptF  func() map[string]string
+	BulkDecryptF  func() []string
 }
 
 func (md *MockDecrypter) DecryptValue(ctx context.Context, ciphertext string) (string, error) {
@@ -89,7 +89,7 @@ func (md *MockDecrypter) DecryptValue(ctx context.Context, ciphertext string) (s
 	return "", errors.New("mock value not provided")
 }
 
-func (md *MockDecrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) (map[string]string, error) {
+func (md *MockDecrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) ([]string, error) {
 	if md.BulkDecryptF != nil {
 		return md.BulkDecryptF(), nil
 	}

diff --git a/pkg/secrets/passphrase/manager.go b/pkg/secrets/passphrase/manager.go
@@ -427,7 +427,7 @@ func (ec *errorCrypter) DecryptValue(ctx context.Context, _ string) (string, err
 		"correct passphrase or set PULUMI_CONFIG_PASSPHRASE_FILE to a file containing the passphrase")
 }
 
-func (ec *errorCrypter) BulkDecrypt(ctx context.Context, _ []string) (map[string]string, error) {
+func (ec *errorCrypter) BulkDecrypt(ctx context.Context, _ []string) ([]string, error) {
 	return nil, errors.New("failed to decrypt: incorrect passphrase, please set PULUMI_CONFIG_PASSPHRASE to the " +
 		"correct passphrase or set PULUMI_CONFIG_PASSPHRASE_FILE to a file containing the passphrase")
 }
diff --git a/pkg/secrets/service/manager.go b/pkg/secrets/service/manager.go
@@ -66,7 +66,7 @@ func (c *serviceCrypter) DecryptValue(ctx context.Context, cipherstring string)
 	return string(plaintext), nil
 }
 
-func (c *serviceCrypter) BulkDecrypt(ctx context.Context, secrets []string) (map[string]string, error) {
+func (c *serviceCrypter) BulkDecrypt(ctx context.Context, secrets []string) ([]string, error) {
 	secretsToDecrypt := slice.Prealloc[[]byte](len(secrets))
 	for _, val := range secrets {
 		ciphertext, err := base64.StdEncoding.DecodeString(val)
@@ -81,9 +81,11 @@ func (c *serviceCrypter) BulkDecrypt(ctx context.Context, secrets []string) (map
 		return nil, err
 	}
 
-	decryptedSecrets := make(map[string]string)
-	for name, val := range decryptedList {
-		decryptedSecrets[name] = string(val)
+	decryptedSecrets := make([]string, len(secrets))
+	for i, ciphertext := range secrets {
+		decrypted, ok := decryptedList[ciphertext]
+		contract.Assertf(ok, "decrypted value not found in bulk response")
+		decryptedSecrets[i] = string(decrypted)
 	}
 
 	return decryptedSecrets, nil

diff --git a/sdk/go/common/resource/config/crypt.go b/sdk/go/common/resource/config/crypt.go
@@ -39,7 +39,8 @@ type Decrypter interface {
 	DecryptValue(ctx context.Context, ciphertext string) (string, error)
 
 	// BulkDecrypt supports bulk decryption of secrets.
-	BulkDecrypt(ctx context.Context, ciphertexts []string) (map[string]string, error)
+	// Returns a list of decrypted values in the same order as the input ciphertexts.
+	BulkDecrypt(ctx context.Context, ciphertexts []string) ([]string, error)
 }
 
 // Crypter can both encrypt and decrypt values.
@@ -60,7 +61,7 @@ func (nopCrypter) DecryptValue(ctx context.Context, ciphertext string) (string,
 	return ciphertext, nil
 }
 
-func (nopCrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) (map[string]string, error) {
+func (nopCrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) ([]string, error) {
 	return DefaultBulkDecrypt(ctx, NopDecrypter, ciphertexts)
 }
 
@@ -88,7 +89,7 @@ func (b blindingCrypter) EncryptValue(ctx context.Context, plaintext string) (st
 	return "[secret]", nil
 }
 
-func (b blindingCrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) (map[string]string, error) {
+func (b blindingCrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) ([]string, error) {
 	return DefaultBulkDecrypt(ctx, b, ciphertexts)
 }
 
@@ -107,7 +108,7 @@ func (p panicCrypter) DecryptValue(ctx context.Context, _ string) (string, error
 	panic("attempt to decrypt value")
 }
 
-func (p panicCrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) (map[string]string, error) {
+func (p panicCrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) ([]string, error) {
 	panic("attempt to bulk decrypt values")
 }
 
@@ -177,7 +178,7 @@ func (s symmetricCrypter) DecryptValue(ctx context.Context, value string) (strin
 	return string(msg), err
 }
 
-func (s symmetricCrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) (map[string]string, error) {
+func (s symmetricCrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) ([]string, error) {
 	return DefaultBulkDecrypt(ctx, s, ciphertexts)
 }
 
@@ -219,7 +220,7 @@ func (c prefixCrypter) EncryptValue(ctx context.Context, plaintext string) (stri
 	return c.prefix + plaintext, nil
 }
 
-func (c prefixCrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) (map[string]string, error) {
+func (c prefixCrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) ([]string, error) {
 	return DefaultBulkDecrypt(ctx, c, ciphertexts)
 }
 
@@ -228,20 +229,20 @@ func (c prefixCrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) (m
 // their BulkDecrypt method in cases where they can't do more efficient than just individual decryptions.
 func DefaultBulkDecrypt(ctx context.Context,
 	decrypter Decrypter, ciphertexts []string,
-) (map[string]string, error) {
+) ([]string, error) {
 	if len(ciphertexts) == 0 {
 		return nil, nil
 	}
 
-	secretMap := map[string]string{}
-	for _, ct := range ciphertexts {
+	decrypted := make([]string, len(ciphertexts))
+	for i, ct := range ciphertexts {
 		pt, err := decrypter.DecryptValue(ctx, ct)
 		if err != nil {
 			return nil, err
 		}
-		secretMap[ct] = pt
+		decrypted[i] = pt
 	}
-	return secretMap, nil
+	return decrypted, nil
 }
 
 type base64Crypter struct{}
@@ -261,6 +262,6 @@ func (c *base64Crypter) DecryptValue(ctx context.Context, s string) (string, err
 	return string(b), nil
 }
 
-func (c *base64Crypter) BulkDecrypt(ctx context.Context, ciphertexts []string) (map[string]string, error) {
+func (c *base64Crypter) BulkDecrypt(ctx context.Context, ciphertexts []string) ([]string, error) {
 	return DefaultBulkDecrypt(ctx, c, ciphertexts)
 }
diff --git a/sdk/go/common/resource/config/value_test.go b/sdk/go/common/resource/config/value_test.go
@@ -217,7 +217,7 @@ func (d passThroughDecrypter) DecryptValue(
 
 func (d passThroughDecrypter) BulkDecrypt(
 	ctx context.Context, ciphertexts []string,
-) (map[string]string, error) {
+) ([]string, error) {
 	return DefaultBulkDecrypt(ctx, d, ciphertexts)
 }