Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that pulumi config cp also copies the environments from the source stack #14847

Merged
merged 8 commits into from Dec 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -0,0 +1,4 @@
changes:
- type: fix
scope: cli/config
description: Fixes config copy command to also copy environments from the source stack
44 changes: 30 additions & 14 deletions pkg/cmd/pulumi/config.go
Expand Up @@ -161,7 +161,25 @@ func newConfigCopyCmd(stack *string) *cobra.Command {
destinationProjectStack)
}

return copyEntireConfigMap(currentStack, currentProjectStack, destinationStack, destinationProjectStack)
requiresSaving, err := copyEntireConfigMap(
currentStack,
currentProjectStack,
destinationStack,
destinationProjectStack)
if err != nil {
return err
}

// The use of `requiresSaving` here ensures that there was actually some config
// that needed saved, otherwise it's an unnecessary save call
if requiresSaving {
err := saveProjectStack(destinationStack, destinationProjectStack)
if err != nil {
return err
}
}

return nil
}),
}

Expand Down Expand Up @@ -224,13 +242,15 @@ func copySingleConfigKey(configKey string, path bool, currentStack backend.Stack
func copyEntireConfigMap(currentStack backend.Stack,
currentProjectStack *workspace.ProjectStack, destinationStack backend.Stack,
destinationProjectStack *workspace.ProjectStack,
) error {
) (bool, error) {
var decrypter config.Decrypter
currentConfig := currentProjectStack.Config
currentEnvironments := currentProjectStack.Environment

if currentConfig.HasSecureValue() {
dec, needsSave, decerr := getStackDecrypter(currentStack, currentProjectStack)
if decerr != nil {
return decerr
return false, decerr
}
contract.Assertf(!needsSave, "We're reading a secure value so the encryption information must be present already")
decrypter = dec
Expand All @@ -240,33 +260,29 @@ func copyEntireConfigMap(currentStack backend.Stack,

encrypter, _, cerr := getStackEncrypter(destinationStack, destinationProjectStack)
if cerr != nil {
return cerr
return false, cerr
}

newProjectConfig, err := currentConfig.Copy(decrypter, encrypter)
if err != nil {
return err
return false, err
}

var requiresSaving bool
for key, val := range newProjectConfig {
err = destinationProjectStack.Config.Set(key, val, false)
if err != nil {
return err
return false, err
}
requiresSaving = true
}

// The use of `requiresSaving` here ensures that there was actually some config
// that needed saved, otherwise it's an unnecessary save call
if requiresSaving {
err := saveProjectStack(destinationStack, destinationProjectStack)
if err != nil {
return err
}
if currentEnvironments != nil && len(currentEnvironments.Imports()) > 0 {
destinationProjectStack.Environment = currentEnvironments
requiresSaving = true
}

return nil
return requiresSaving, nil
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this change had a cascading effect on another place. Though I am not a huge fan of this approach I feel like this is simple and makes this function testable through unit testing.

I am also thinking if I should rename this method from copyEntireConfigMap to something like getMergdConfigMap?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@justinvp @pgavlin before merging this though, I want to draw attention to this comment in case you missed it. Would love to know your thoughts.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Returning requiresSaving and making it so callers have to call save doesn't seem the best, but I don't feel strongly about it and agree it's nice that it makes it easier to unit test. I'm fine with it.

I also don't feel strongly about the function name.

}

func newConfigGetCmd(stack *string) *cobra.Command {
Expand Down
115 changes: 101 additions & 14 deletions pkg/cmd/pulumi/config_test.go
Expand Up @@ -191,20 +191,8 @@ func TestOpenStackEnvUnsupportedBackend(t *testing.T) {
assert.Error(t, err)
}

func TestOpenStackEnv(t *testing.T) {
t.Parallel()

env := map[string]esc.Value{
"pulumiConfig": esc.NewValue(map[string]esc.Value{
"test:string": esc.NewValue("esc"),
}),
"environmentVariables": esc.NewValue(map[string]esc.Value{
"TEST_VAR": esc.NewSecret("hunter2"),
}),
"files": esc.NewValue(map[string]esc.Value{
"TEST_FILE": esc.NewSecret("sensitive"),
}),
}
func getMockStackWithEnv(t *testing.T, env map[string]esc.Value) *backend.MockStack {
t.Helper()

be := &backend.MockEnvironmentsBackend{
MockBackend: backend.MockBackend{
Expand All @@ -227,6 +215,26 @@ func TestOpenStackEnv(t *testing.T) {
BackendF: func() backend.Backend { return be },
}

return stack
}

func TestOpenStackEnv(t *testing.T) {
t.Parallel()

env := map[string]esc.Value{
"pulumiConfig": esc.NewValue(map[string]esc.Value{
"test:string": esc.NewValue("esc"),
}),
"environmentVariables": esc.NewValue(map[string]esc.Value{
"TEST_VAR": esc.NewSecret("hunter2"),
}),
"files": esc.NewValue(map[string]esc.Value{
"TEST_FILE": esc.NewSecret("sensitive"),
}),
}

stack := getMockStackWithEnv(t, env)

var projectStack workspace.ProjectStack
err := yaml.Unmarshal([]byte("environment:\n - test"), &projectStack)
require.NoError(t, err)
Expand All @@ -237,6 +245,85 @@ func TestOpenStackEnv(t *testing.T) {
assert.Equal(t, env, openEnv.Properties)
}

func TestCopyConfig(t *testing.T) {
t.Parallel()

env := map[string]esc.Value{
"pulumiConfig": esc.NewValue(map[string]esc.Value{
"test:string": esc.NewValue("esc"),
}),
"environmentVariables": esc.NewValue(map[string]esc.Value{
"TEST_VAR": esc.NewSecret("hunter2"),
}),
"files": esc.NewValue(map[string]esc.Value{
"TEST_FILE": esc.NewSecret("sensitive"),
}),
}

mockSecretsManager := &secrets.MockSecretsManager{
EncrypterF: func() (config.Encrypter, error) {
encrypter := &secrets.MockEncrypter{EncryptValueF: func() string { return "ciphertext" }}
return encrypter, nil
},
DecrypterF: func() (config.Decrypter, error) {
decrypter := &secrets.MockDecrypter{
DecryptValueF: func() string {
return "plaintext"
},
BulkDecryptF: func() map[string]string {
return map[string]string{
"idontknow": "whatiamdoing",
}
},
}

return decrypter, nil
},
}

getMockStack := func(name string) *backend.MockStack {
stack := getMockStackWithEnv(t, env)
stack.RefF = func() backend.StackReference {
return &backend.MockStackReference{
StringV: "org/project/" + name,
NameV: tokens.MustParseStackName(name),
ProjectV: "project",
FullyQualifiedNameV: tokens.QName("org/project/" + name),
}
}
stack.DefaultSecretManagerF = func(info *workspace.ProjectStack) (secrets.Manager, error) {
return mockSecretsManager, nil
}

return stack
}

sourceStack := getMockStack("mystack")

var sourceProjectStack workspace.ProjectStack
err := yaml.Unmarshal([]byte("environment:\n - test"), &sourceProjectStack)
require.NoError(t, err)

t.Run("TestCopyConfigIncludesEnvironments", func(t *testing.T) {
destinationStack := getMockStack("mystack2")

var destinationProjectStack workspace.ProjectStack
err := yaml.Unmarshal([]byte("environment:\n - test2"), &destinationProjectStack)
require.NoError(t, err)

requiresSaving, err := copyEntireConfigMap(
sourceStack, &sourceProjectStack, destinationStack, &destinationProjectStack)
require.NoError(t, err)
assert.True(t, requiresSaving, "expected config file changes requiring saving")

// Assert that only the source stack's environment
// remains in the destination stack.
envImports := destinationProjectStack.Environment.Imports()
assert.Contains(t, envImports, "test")
assert.NotContains(t, envImports, "test2")
})
}

func TestOpenStackEnvDiags(t *testing.T) {
t.Parallel()

Expand Down
14 changes: 13 additions & 1 deletion pkg/cmd/pulumi/stack_init.go
Expand Up @@ -200,7 +200,19 @@ func (cmd *stackInitCmd) Run(ctx context.Context, args []string) error {
}

// copy the config from the old to the new
return copyEntireConfigMap(copyStack, copyProjectStack, newStack, newProjectStack)
requiresSaving, err := copyEntireConfigMap(copyStack, copyProjectStack, newStack, newProjectStack)
if err != nil {
return err
}

// The use of `requiresSaving` here ensures that there was actually some config
// that needed saved, otherwise it's an unnecessary save call
if requiresSaving {
err := saveProjectStack(newStack, newProjectStack)
if err != nil {
return err
}
}
}

return nil
Expand Down
97 changes: 97 additions & 0 deletions pkg/secrets/mock.go
@@ -0,0 +1,97 @@
// Copyright 2016-2023, Pulumi Corporation.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package secrets

import (
"context"
"encoding/json"
"errors"

"github.com/pulumi/pulumi/sdk/v3/go/common/resource/config"
)

type MockSecretsManager struct {
TypeF func() string
StateF func() json.RawMessage
EncrypterF func() (config.Encrypter, error)
DecrypterF func() (config.Decrypter, error)
}

var _ Manager = &MockSecretsManager{}

func (msm *MockSecretsManager) Type() string {
if msm.TypeF != nil {
return msm.TypeF()
}

panic("not implemented")
}

func (msm *MockSecretsManager) State() json.RawMessage {
if msm.StateF != nil {
return msm.StateF()
}

panic("not implemented")
}

func (msm *MockSecretsManager) Encrypter() (config.Encrypter, error) {
if msm.EncrypterF != nil {
return msm.EncrypterF()
}

panic("not implemented")
}

func (msm *MockSecretsManager) Decrypter() (config.Decrypter, error) {
if msm.DecrypterF != nil {
return msm.DecrypterF()
}

panic("not implemented")
}

type MockEncrypter struct {
EncryptValueF func() string
}

func (me *MockEncrypter) EncryptValue(ctx context.Context, plaintext string) (string, error) {
if me.EncryptValueF != nil {
return me.EncryptValueF(), nil
}

return "", errors.New("mock value not provided")
}

type MockDecrypter struct {
DecryptValueF func() string
BulkDecryptF func() map[string]string
}

func (md *MockDecrypter) DecryptValue(ctx context.Context, ciphertext string) (string, error) {
if md.DecryptValueF != nil {
return md.DecryptValueF(), nil
}

return "", errors.New("mock value not provided")
}

func (md *MockDecrypter) BulkDecrypt(ctx context.Context, ciphertexts []string) (map[string]string, error) {
if md.BulkDecryptF != nil {
return md.BulkDecryptF(), nil
}

return nil, errors.New("mock value not provided")
}