From ef482331228c41288b058e115030538ca0b44009 Mon Sep 17 00:00:00 2001 From: Daniel Bradley Date: Mon, 10 Feb 2025 15:41:13 +0000 Subject: [PATCH] Swap BulkDecrypt to return list (#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. --- ...decrypt-to-return-list-instead-of-map.yaml | 4 +++ pkg/cmd/pulumi/config/io_test.go | 12 +++---- pkg/engine/lifecycletest/pulumi_test.go | 2 +- pkg/resource/deploy/source_eval_test.go | 4 +-- pkg/resource/stack/deployment.go | 7 ++++- pkg/resource/stack/secrets.go | 31 +++++++++++++------ pkg/resource/stack/secrets_test.go | 29 +++++++++++++++-- pkg/secrets/mock.go | 4 +-- pkg/secrets/passphrase/manager.go | 2 +- pkg/secrets/service/manager.go | 10 +++--- sdk/go/common/resource/config/crypt.go | 25 ++++++++------- sdk/go/common/resource/config/value_test.go | 2 +- 12 files changed, 90 insertions(+), 42 deletions(-) create mode 100644 changelog/pending/20250207--pkg--change-bulkdecrypt-to-return-list-instead-of-map.yaml 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 new file mode 100644 index 000000000000..b4e52424c337 --- /dev/null +++ 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 index 41a0dde44871..78edcf01a261 100644 --- 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 index 891ca52bad44..f68481bba875 100644 --- 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 index 25b3c6557391..1c47fd164a86 100644 --- 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 index c5e64af7a620..0b75c28bbc74 100644 --- 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 index 9d6b2254d93d..7d6d447219f1 100644 --- 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 index ce5a1fbffbb5..b9f7c4475192 100644 --- 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 index 7a2d9f499a8a..d3a54b565502 100644 --- 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 index 370ee661da92..3f21b682bb4d 100644 --- 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 index 51c86a01021c..3090f2ea3058 100644 --- 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 index bf755ec03929..28af746e7032 100644 --- 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 index 063d47c87a4f..db356602d6c7 100644 --- 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) }