chore: env manager patcher and service discovery namespace should wor…

…k with bootstrap stack (#3966) Previously, we have two fixes: 1. #3956 which patches the S3 permissions to the environment manager role if it's needed. 2. #3949 which preserves service discovery endpoint namespace from a previously deployed env stack. Fix 1 performs an environment template version check to infer whether the necessary permissions are present. This version check fails to take into account the `"bootstrap"` version, which is present when the environment is only bootstrapped (by running `env init` and not having run `env deploy`) Fix 2 grabs the old parameter from a deployed environment stack. It fails to take into consideration that, while an env stack that's on `"bootstrap"` version is not considered a "deployed environment" in Copilot, it is a deployed stack in CFN. By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
aws · Aug 30, 2022 · a830133 · a830133
1 parent 9cdf694
commit a830133
Show file tree

Hide file tree

Showing 10 changed files with 91 additions and 25 deletions.
diff --git a/internal/pkg/cli/deploy/env.go b/internal/pkg/cli/deploy/env.go
@@ -32,7 +32,7 @@ type appResourcesGetter interface {
 
 type environmentDeployer interface {
 	UpdateAndRenderEnvironment(conf deploycfn.StackConfiguration, bucketARN string, opts ...cloudformation.StackOption) error
-	EnvironmentParameters(app, env string) ([]*awscfn.Parameter, error)
+	DeployedEnvironmentParameters(app, env string) ([]*awscfn.Parameter, error)
 	ForceUpdateOutputID(app, env string) (string, error)
 }
 
@@ -171,7 +171,7 @@ func (d *envDeployer) GenerateCloudFormationTemplate(in *DeployEnvironmentInput)
 	if err != nil {
 		return nil, err
 	}
-	oldParams, err := d.envDeployer.EnvironmentParameters(d.app.Name, d.env.Name)
+	oldParams, err := d.envDeployer.DeployedEnvironmentParameters(d.app.Name, d.env.Name)
 	if err != nil {
 		return nil, fmt.Errorf("describe environment stack parameters: %w", err)
 	}
@@ -200,7 +200,7 @@ func (d *envDeployer) DeployEnvironment(in *DeployEnvironmentInput) error {
 	if err != nil {
 		return err
 	}
-	oldParams, err := d.envDeployer.EnvironmentParameters(d.app.Name, d.env.Name)
+	oldParams, err := d.envDeployer.DeployedEnvironmentParameters(d.app.Name, d.env.Name)
 	if err != nil {
 		return fmt.Errorf("describe environment stack parameters: %w", err)
 	}

diff --git a/internal/pkg/cli/deploy/env_test.go b/internal/pkg/cli/deploy/env_test.go
@@ -169,7 +169,7 @@ func TestEnvDeployer_GenerateCloudFormationTemplate(t *testing.T) {
 				m.appCFN.EXPECT().GetAppResourcesByRegion(gomock.Any(), gomock.Any()).Return(&stack.AppRegionalResources{
 					S3Bucket: "mockS3Bucket",
 				}, nil)
-				m.envDeployer.EXPECT().EnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, errors.New("some error"))
+				m.envDeployer.EXPECT().DeployedEnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, errors.New("some error"))
 			},
 			wantedError: errors.New("describe environment stack parameters: some error"),
 		},
@@ -178,7 +178,7 @@ func TestEnvDeployer_GenerateCloudFormationTemplate(t *testing.T) {
 				m.appCFN.EXPECT().GetAppResourcesByRegion(gomock.Any(), gomock.Any()).Return(&stack.AppRegionalResources{
 					S3Bucket: "mockS3Bucket",
 				}, nil)
-				m.envDeployer.EXPECT().EnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil)
+				m.envDeployer.EXPECT().DeployedEnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil)
 				m.envDeployer.EXPECT().ForceUpdateOutputID(gomock.Any(), gomock.Any()).Return("", errors.New("some error"))
 			},
 			wantedError: errors.New("retrieve environment stack force update ID: some error"),
@@ -188,7 +188,7 @@ func TestEnvDeployer_GenerateCloudFormationTemplate(t *testing.T) {
 				m.appCFN.EXPECT().GetAppResourcesByRegion(gomock.Any(), gomock.Any()).Return(&stack.AppRegionalResources{
 					S3Bucket: "mockS3Bucket",
 				}, nil)
-				m.envDeployer.EXPECT().EnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil)
+				m.envDeployer.EXPECT().DeployedEnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil)
 				m.envDeployer.EXPECT().ForceUpdateOutputID(gomock.Any(), gomock.Any()).Return("", nil)
 				m.stackSerializer.EXPECT().Template().Return("", errors.New("some error"))
 			},
@@ -199,7 +199,7 @@ func TestEnvDeployer_GenerateCloudFormationTemplate(t *testing.T) {
 				m.appCFN.EXPECT().GetAppResourcesByRegion(gomock.Any(), gomock.Any()).Return(&stack.AppRegionalResources{
 					S3Bucket: "mockS3Bucket",
 				}, nil)
-				m.envDeployer.EXPECT().EnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil)
+				m.envDeployer.EXPECT().DeployedEnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil)
 				m.envDeployer.EXPECT().ForceUpdateOutputID(gomock.Any(), gomock.Any()).Return("", nil)
 				m.stackSerializer.EXPECT().Template().Return("", nil)
 				m.stackSerializer.EXPECT().SerializedParameters().Return("", errors.New("some error"))
@@ -211,7 +211,7 @@ func TestEnvDeployer_GenerateCloudFormationTemplate(t *testing.T) {
 				m.appCFN.EXPECT().GetAppResourcesByRegion(mockApp, mockEnvRegion).Return(&stack.AppRegionalResources{
 					S3Bucket: "mockS3Bucket",
 				}, nil)
-				m.envDeployer.EXPECT().EnvironmentParameters(mockAppName, mockEnvName).Return(nil, nil)
+				m.envDeployer.EXPECT().DeployedEnvironmentParameters(mockAppName, mockEnvName).Return(nil, nil)
 				m.envDeployer.EXPECT().ForceUpdateOutputID(gomock.Any(), gomock.Any()).Return("", nil)
 				m.stackSerializer.EXPECT().Template().Return("aloo", nil)
 				m.stackSerializer.EXPECT().SerializedParameters().Return("gobi", nil)
@@ -307,7 +307,7 @@ func TestEnvDeployer_DeployEnvironment(t *testing.T) {
 				m.appCFN.EXPECT().GetAppResourcesByRegion(mockApp, mockEnvRegion).Return(&stack.AppRegionalResources{
 					S3Bucket: "mockS3Bucket",
 				}, nil)
-				m.envDeployer.EXPECT().EnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil)
+				m.envDeployer.EXPECT().DeployedEnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil)
 				m.envDeployer.EXPECT().ForceUpdateOutputID(gomock.Any(), gomock.Any()).Return("", nil)
 				m.prefixListGetter.EXPECT().CloudFrontManagedPrefixListID().Return("mockPrefixListID", nil).Times(0)
 				m.envDeployer.EXPECT().UpdateAndRenderEnvironment(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
@@ -319,7 +319,7 @@ func TestEnvDeployer_DeployEnvironment(t *testing.T) {
 				m.appCFN.EXPECT().GetAppResourcesByRegion(gomock.Any(), gomock.Any()).Return(&stack.AppRegionalResources{
 					S3Bucket: "mockS3Bucket",
 				}, nil)
-				m.envDeployer.EXPECT().EnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, errors.New("some error"))
+				m.envDeployer.EXPECT().DeployedEnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, errors.New("some error"))
 			},
 			wantedError: errors.New("describe environment stack parameters: some error"),
 		},
@@ -328,7 +328,7 @@ func TestEnvDeployer_DeployEnvironment(t *testing.T) {
 				m.appCFN.EXPECT().GetAppResourcesByRegion(gomock.Any(), gomock.Any()).Return(&stack.AppRegionalResources{
 					S3Bucket: "mockS3Bucket",
 				}, nil)
-				m.envDeployer.EXPECT().EnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil)
+				m.envDeployer.EXPECT().DeployedEnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil)
 				m.envDeployer.EXPECT().ForceUpdateOutputID(gomock.Any(), gomock.Any()).Return("", errors.New("some error"))
 			},
 			wantedError: errors.New("retrieve environment stack force update ID: some error"),
@@ -339,7 +339,7 @@ func TestEnvDeployer_DeployEnvironment(t *testing.T) {
 					S3Bucket: "mockS3Bucket",
 				}, nil)
 				m.prefixListGetter.EXPECT().CloudFrontManagedPrefixListID().Return("mockPrefixListID", nil).Times(0)
-				m.envDeployer.EXPECT().EnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil)
+				m.envDeployer.EXPECT().DeployedEnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil)
 				m.envDeployer.EXPECT().ForceUpdateOutputID(gomock.Any(), gomock.Any()).Return("", nil)
 				m.envDeployer.EXPECT().UpdateAndRenderEnvironment(gomock.Any(), gomock.Any(), gomock.Any()).Return(errors.New("some error"))
 			},
@@ -351,7 +351,7 @@ func TestEnvDeployer_DeployEnvironment(t *testing.T) {
 					S3Bucket: "mockS3Bucket",
 				}, nil)
 				m.prefixListGetter.EXPECT().CloudFrontManagedPrefixListID().Return("mockPrefixListID", nil).Times(0)
-				m.envDeployer.EXPECT().EnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil)
+				m.envDeployer.EXPECT().DeployedEnvironmentParameters(gomock.Any(), gomock.Any()).Return(nil, nil)
 				m.envDeployer.EXPECT().ForceUpdateOutputID(gomock.Any(), gomock.Any()).Return("", nil)
 				m.envDeployer.EXPECT().UpdateAndRenderEnvironment(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil)
 			},

diff --git a/internal/pkg/cli/deploy/mocks/mock_env.go b/internal/pkg/cli/deploy/mocks/mock_env.go
diff --git a/internal/pkg/cli/deploy/patch/env.go b/internal/pkg/cli/deploy/patch/env.go
@@ -12,6 +12,7 @@ import (
 	"github.com/aws/copilot-cli/internal/pkg/aws/cloudformation"
 	"github.com/aws/copilot-cli/internal/pkg/aws/s3"
 	"github.com/aws/copilot-cli/internal/pkg/config"
+	"github.com/aws/copilot-cli/internal/pkg/deploy"
 	"github.com/aws/copilot-cli/internal/pkg/term/log"
 	"golang.org/x/mod/semver"
 	"gopkg.in/yaml.v3"
@@ -122,6 +123,10 @@ func isManagerRoleAllowedToUpload(body string) (bool, error) {
 	if err := yaml.Unmarshal([]byte(body), &tpl); err != nil {
 		return false, fmt.Errorf("unmarshal environment template to detect Metadata.Version: %v", err)
 	}
+	if tpl.Metadata.Version == deploy.EnvTemplateVersionBootstrap {
+		// "bootstrap" version is introduced after v1.9.0. The environment manager roles must have had the permissions.
+		return true, nil
+	}
 	if !semver.IsValid(tpl.Metadata.Version) { // The template doesn't contain a version.
 		return false, nil
 	}

diff --git a/internal/pkg/deploy/cloudformation/cloudformation.go b/internal/pkg/deploy/cloudformation/cloudformation.go
@@ -79,6 +79,7 @@ type cfnClient interface {
 	ErrorEvents(stackName string) ([]cloudformation.StackEvent, error)
 	Outputs(stack *cloudformation.Stack) (map[string]string, error)
 	StackResources(name string) ([]*cloudformation.StackResource, error)
+	Metadata(opts cloudformation.MetadataOpts) (string, error)
 
 	// Methods vended by the aws sdk struct.
 	DescribeStackEvents(*sdkcloudformation.DescribeStackEventsInput) (*sdkcloudformation.DescribeStackEventsOutput, error)

diff --git a/internal/pkg/deploy/cloudformation/cloudformation_integration_test.go b/internal/pkg/deploy/cloudformation/cloudformation_integration_test.go
@@ -501,7 +501,7 @@ func Test_Environment_Deployment_Integration(t *testing.T) {
 		}
 
 		// Deploy the environment and wait for it to be complete.
-		oldParams, err := deployer.EnvironmentParameters(environmentToDeploy.App.Name, environmentToDeploy.Name)
+		oldParams, err := deployer.DeployedEnvironmentParameters(environmentToDeploy.App.Name, environmentToDeploy.Name)
 		require.NoError(t, err)
 		lastForceUpdateID, err := deployer.ForceUpdateOutputID(environmentToDeploy.App.Name, environmentToDeploy.Name)
 		require.NoError(t, err)

diff --git a/internal/pkg/deploy/cloudformation/env.go b/internal/pkg/deploy/cloudformation/env.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws/arn"
 	"github.com/aws/copilot-cli/internal/pkg/template"
+	"gopkg.in/yaml.v3"
 
 	"github.com/aws/aws-sdk-go/aws"
 	awscfn "github.com/aws/aws-sdk-go/service/cloudformation"
@@ -116,8 +117,15 @@ func (cf CloudFormation) ForceUpdateOutputID(app, env string) (string, error) {
 	return "", nil
 }
 
-// EnvironmentParameters returns the environment stack's parameters.
-func (cf CloudFormation) EnvironmentParameters(appName, envName string) ([]*awscfn.Parameter, error) {
+// DeployedEnvironmentParameters returns the environment stack's parameters.
+func (cf CloudFormation) DeployedEnvironmentParameters(appName, envName string) ([]*awscfn.Parameter, error) {
+	isInitial, err := cf.isInitialDeployment(appName, envName)
+	if err != nil {
+		return nil, err
+	}
+	if isInitial {
+		return nil, nil
+	}
 	out, err := cf.cachedStack(stack.NameForEnv(appName, envName))
 	if err != nil {
 		return nil, err
@@ -188,3 +196,18 @@ func (cf CloudFormation) cachedStack(stackName string) (*cloudformation.StackDes
 	cf.cachedDeployedStack = stackDescr
 	return cf.cachedDeployedStack, nil
 }
+
+// isInitialDeployment returns whether this is the first deployment of the environment stack.
+func (cf CloudFormation) isInitialDeployment(appName, envName string) (bool, error) {
+	raw, err := cf.cfnClient.Metadata(cloudformation.MetadataWithStackName(stack.NameForEnv(appName, envName)))
+	if err != nil {
+		return false, fmt.Errorf("get metadata of stack %q: %w", stack.NameForEnv(appName, envName), err)
+	}
+	metadata := struct {
+		Version string `yaml:"Version"`
+	}{}
+	if err := yaml.Unmarshal([]byte(raw), &metadata); err != nil {
+		return false, fmt.Errorf("unmarshal Metadata property to read Version: %w", err)
+	}
+	return metadata.Version == deploy.EnvTemplateVersionBootstrap, nil
+}
diff --git a/internal/pkg/deploy/cloudformation/env_test.go b/internal/pkg/deploy/cloudformation/env_test.go
@@ -48,7 +48,7 @@ func TestCloudFormation_EnvironmentTemplate(t *testing.T) {
 	}
 }
 
-func TestCloudFormation_EnvironmentParameters(t *testing.T) {
+func TestCloudFormation_DeployedEnvironmentParameters(t *testing.T) {
 	testCases := map[string]struct {
 		inAppName string
 		inEnvName string
@@ -57,11 +57,31 @@ func TestCloudFormation_EnvironmentParameters(t *testing.T) {
 		wantedParams []*awscfn.Parameter
 		wantedErr    error
 	}{
+		"error retrieving metadata": {
+			inAppName: "phonetool",
+			inEnvName: "test",
+			inClient: func(ctrl *gomock.Controller) *mocks.MockcfnClient {
+				m := mocks.NewMockcfnClient(ctrl)
+				m.EXPECT().Metadata(gomock.Any()).Return("", errors.New("some error"))
+				return m
+			},
+			wantedErr: errors.New("get metadata of stack \"phonetool-test\": some error"),
+		},
+		"returns nil if the version is bootstrap": {
+			inAppName: "phonetool",
+			inEnvName: "test",
+			inClient: func(ctrl *gomock.Controller) *mocks.MockcfnClient {
+				m := mocks.NewMockcfnClient(ctrl)
+				m.EXPECT().Metadata(gomock.Any()).Return(`Version: bootstrap`, nil)
+				return m
+			},
+		},
 		"should return stack parameters from a stack description": {
 			inAppName: "phonetool",
 			inEnvName: "test",
 			inClient: func(ctrl *gomock.Controller) *mocks.MockcfnClient {
 				m := mocks.NewMockcfnClient(ctrl)
+				m.EXPECT().Metadata(gomock.Any()).Return(`Version: `, nil)
 				m.EXPECT().Describe("phonetool-test").Return(&cloudformation.StackDescription{
 					Parameters: []*awscfn.Parameter{
 						{
@@ -85,6 +105,7 @@ func TestCloudFormation_EnvironmentParameters(t *testing.T) {
 			inEnvName: "test",
 			inClient: func(ctrl *gomock.Controller) *mocks.MockcfnClient {
 				m := mocks.NewMockcfnClient(ctrl)
+				m.EXPECT().Metadata(gomock.Any()).Return(`Version: v1.21.0`, nil)
 				m.EXPECT().Describe(gomock.Any()).Return(nil, errors.New("some error"))
 				return m
 			},
@@ -102,7 +123,7 @@ func TestCloudFormation_EnvironmentParameters(t *testing.T) {
 			}
 
 			// WHEN
-			actual, err := cf.EnvironmentParameters(tc.inAppName, tc.inEnvName)
+			actual, err := cf.DeployedEnvironmentParameters(tc.inAppName, tc.inEnvName)
 			if tc.wantedErr != nil {
 				require.EqualError(t, err, tc.wantedErr.Error())
 			} else {

diff --git a/internal/pkg/deploy/cloudformation/mocks/mock_cloudformation.go b/internal/pkg/deploy/cloudformation/mocks/mock_cloudformation.go
diff --git a/internal/pkg/deploy/env.go b/internal/pkg/deploy/env.go
@@ -14,7 +14,8 @@ const (
 	// LegacyEnvTemplateVersion is the version associated with the environment template before we started versioning.
 	LegacyEnvTemplateVersion = "v0.0.0"
 	// LatestEnvTemplateVersion is the latest version number available for environment templates.
-	LatestEnvTemplateVersion = "v1.12.2"
+	LatestEnvTemplateVersion    = "v1.12.2"
+	EnvTemplateVersionBootstrap = "bootstrap"
 )
 
 // CreateEnvironmentInput holds the fields required to deploy an environment.