chore: replace auto-upgrade of environments with compatibility checkers (#3698)

Previously, Copilot automatically upgrades an environment during workload's `deploy` commands if the environment is not on the latest template version.

With this PR, it stops doing that, and instead will check if the workload's manifest is attempting to use a feature that is not yet supported by the environment's template; if yes, then Copilot errors out and asks user to upgrade their environment.

Similar logic applies to the "pipeline + `svc package`" workflow. 

Note: this PR can be merged only after #3688 .

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.

Author: Lou1415926

internal/pkg/cli/job_deploy.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ssm"
+	"github.com/aws/copilot-cli/internal/pkg/describe"
 
 	"github.com/aws/copilot-cli/internal/pkg/deploy/cloudformation/stack"
 	"github.com/aws/copilot-cli/internal/pkg/exec"
@@ -30,14 +31,14 @@ import (
 type deployJobOpts struct {
 	deployWkldVars
 
-	store           store
-	ws              wsWlDirReader
-	unmarshal       func(in []byte) (manifest.WorkloadManifest, error)
-	newInterpolator func(app, env string) interpolator
-	cmd             runner
-	sessProvider    *sessions.Provider
-	envUpgradeCmd   actionCommand
-	newJobDeployer  func() (workloadDeployer, error)
+	store                store
+	ws                   wsWlDirReader
+	unmarshal            func(in []byte) (manifest.WorkloadManifest, error)
+	newInterpolator      func(app, env string) interpolator
+	cmd                  runner
+	sessProvider         *sessions.Provider
+	newJobDeployer       func() (workloadDeployer, error)
+	envFeaturesDescriber versionCompatibilityChecker
 
 	sel wsSelector
 
@@ -138,9 +139,6 @@ func (o *deployJobOpts) Execute() error {
 			return err
 		}
 	}
-	if err := o.envUpgradeCmd.Execute(); err != nil {
-		return fmt.Errorf(`execute "env upgrade --app %s --name %s": %v`, o.appName, o.envName, err)
-	}
 	mft, err := workloadManifest(&workloadManifestInput{
 		name:         o.name,
 		appName:      o.appName,
@@ -153,6 +151,9 @@ func (o *deployJobOpts) Execute() error {
 		return err
 	}
 	o.appliedManifest = mft
+	if err := validateManifestCompatibilityWithEnv(mft, o.envName, o.envFeaturesDescriber); err != nil {
+		return err
+	}
 	deployer, err := o.newJobDeployer()
 	if err != nil {
 		return err
@@ -217,22 +218,22 @@ func (o *deployJobOpts) configureClients() error {
 		return fmt.Errorf("create default session: %w", err)
 	}
 
-	cmd, err := newEnvUpgradeOpts(envUpgradeVars{
-		appName: o.appName,
-		name:    env.Name,
-	})
-	if err != nil {
-		return fmt.Errorf("new env upgrade command: %v", err)
-	}
-	o.envUpgradeCmd = cmd
-
 	// client to retrieve caller identity.
 	caller, err := identity.New(defaultSess).Get()
 	if err != nil {
 		return fmt.Errorf("get identity: %w", err)
 	}
 	o.rootUserARN = caller.RootUserARN
 
+	envDescriber, err := describe.NewEnvDescriber(describe.NewEnvDescriberConfig{
+		App:         o.appName,
+		Env:         o.envName,
+		ConfigStore: o.store,
+	})
+	if err != nil {
+		return err
+	}
+	o.envFeaturesDescriber = envDescriber
 	return nil
 }
 

internal/pkg/cli/job_deploy_test.go

@@ -199,35 +199,55 @@ func TestJobDeployOpts_Execute(t *testing.T) {
 
 		wantedError error
 	}{
-		"error if failed to upgrade environment": {
-			mock: func(m *deployMocks) {
-				m.mockEnvUpgrader.EXPECT().Execute().Return(mockError)
-			},
-
-			wantedError: fmt.Errorf(`execute "env upgrade --app phonetool --name prod-iad": some error`),
-		},
 		"error out if fail to read workload manifest": {
 			mock: func(m *deployMocks) {
-				m.mockEnvUpgrader.EXPECT().Execute().Return(nil)
 				m.mockWsReader.EXPECT().ReadWorkloadManifest(mockJobName).Return(nil, mockError)
 			},
 
 			wantedError: fmt.Errorf("read manifest file for upload: some error"),
 		},
 		"error out if fail to interpolate workload manifest": {
 			mock: func(m *deployMocks) {
-				m.mockEnvUpgrader.EXPECT().Execute().Return(nil)
 				m.mockWsReader.EXPECT().ReadWorkloadManifest(mockJobName).Return([]byte(""), nil)
 				m.mockInterpolator.EXPECT().Interpolate("").Return("", mockError)
 			},
 
 			wantedError: fmt.Errorf("interpolate environment variables for upload manifest: some error"),
 		},
+		"error if fail to get a list of available features from the environment": {
+			mock: func(m *deployMocks) {
+				m.mockWsReader.EXPECT().ReadWorkloadManifest(mockJobName).Return([]byte(""), nil)
+				m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
+				m.mockEnvFeaturesDescriber.EXPECT().AvailableFeatures().Return(nil, mockError)
+			},
+
+			wantedError: fmt.Errorf("get available features of the prod-iad environment stack: some error"),
+		},
+		"error if some required features are not available in the environment": {
+			mock: func(m *deployMocks) {
+				m.mockWsReader.EXPECT().ReadWorkloadManifest(mockJobName).Return([]byte(""), nil)
+				m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
+				m.mockMft = &mockWorkloadMft{
+					mockRequiredEnvironmentFeatures: func() []string {
+						return []string{"mockFeature1", "mockFeature3"}
+					},
+				}
+				m.mockEnvFeaturesDescriber.EXPECT().AvailableFeatures().Return([]string{"mockFeature1", "mockFeature2"}, nil)
+				m.mockEnvFeaturesDescriber.EXPECT().Version().Return("v1.mock", nil)
+			},
+			wantedError: fmt.Errorf(`environment "prod-iad" is not on a version that supports the "mockFeature3" feature`),
+		},
 		"error if failed to upload artifacts": {
 			mock: func(m *deployMocks) {
-				m.mockEnvUpgrader.EXPECT().Execute().Return(nil)
 				m.mockWsReader.EXPECT().ReadWorkloadManifest(mockJobName).Return([]byte(""), nil)
 				m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
+				m.mockMft = &mockWorkloadMft{
+					mockRequiredEnvironmentFeatures: func() []string {
+						return []string{"mockFeature1"}
+					},
+				}
+				m.mockEnvFeaturesDescriber.EXPECT().AvailableFeatures().Return([]string{"mockFeature1", "mockFeature2"}, nil)
+				m.mockEnvFeaturesDescriber.EXPECT().Version().Times(0)
 				m.mockDeployer.EXPECT().IsServiceAvailableInRegion("").Return(false, nil)
 				m.mockDeployer.EXPECT().UploadArtifacts().Return(nil, mockError)
 			},
@@ -236,9 +256,15 @@ func TestJobDeployOpts_Execute(t *testing.T) {
 		},
 		"error if failed to deploy service": {
 			mock: func(m *deployMocks) {
-				m.mockEnvUpgrader.EXPECT().Execute().Return(nil)
 				m.mockWsReader.EXPECT().ReadWorkloadManifest(mockJobName).Return([]byte(""), nil)
 				m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
+				m.mockMft = &mockWorkloadMft{
+					mockRequiredEnvironmentFeatures: func() []string {
+						return []string{"mockFeature1"}
+					},
+				}
+				m.mockEnvFeaturesDescriber.EXPECT().AvailableFeatures().Return([]string{"mockFeature1", "mockFeature2"}, nil)
+				m.mockEnvFeaturesDescriber.EXPECT().Version().Times(0)
 				m.mockDeployer.EXPECT().UploadArtifacts().Return(&deploy.UploadArtifactsOutput{}, nil)
 				m.mockDeployer.EXPECT().DeployWorkload(gomock.Any()).Return(nil, mockError)
 				m.mockDeployer.EXPECT().IsServiceAvailableInRegion("").Return(false, nil)
@@ -255,10 +281,10 @@ func TestJobDeployOpts_Execute(t *testing.T) {
 			defer ctrl.Finish()
 
 			m := &deployMocks{
-				mockDeployer:     mocks.NewMockworkloadDeployer(ctrl),
-				mockEnvUpgrader:  mocks.NewMockactionCommand(ctrl),
-				mockInterpolator: mocks.NewMockinterpolator(ctrl),
-				mockWsReader:     mocks.NewMockwsWlDirReader(ctrl),
+				mockDeployer:             mocks.NewMockworkloadDeployer(ctrl),
+				mockInterpolator:         mocks.NewMockinterpolator(ctrl),
+				mockWsReader:             mocks.NewMockwsWlDirReader(ctrl),
+				mockEnvFeaturesDescriber: mocks.NewMockversionCompatibilityChecker(ctrl),
 			}
 			tc.mock(m)
 
@@ -278,9 +304,9 @@ func TestJobDeployOpts_Execute(t *testing.T) {
 					return m.mockInterpolator
 				},
 				unmarshal: func(b []byte) (manifest.WorkloadManifest, error) {
-					return &mockWorkloadMft{}, nil
+					return m.mockMft, nil
 				},
-				envUpgradeCmd: m.mockEnvUpgrader,
+				envFeaturesDescriber: m.mockEnvFeaturesDescriber,
 
 				targetApp: &config.Application{},
 				targetEnv: &config.Environment{},

internal/pkg/cli/job_package.go

@@ -140,6 +140,11 @@ func (o *packageJobOpts) Execute() error {
 	return o.packageCmd.Execute()
 }
 
+// RecommendActions suggests recommended actions before the packaged template is used for deployment.
+func (o *packageJobOpts) RecommendActions() error {
+	return o.packageCmd.RecommendActions()
+}
+
 func (o *packageJobOpts) askJobName() error {
 	if o.name != "" {
 		return nil

internal/pkg/cli/job_package_test.go

@@ -262,3 +262,39 @@ func TestPackageJobOpts_Execute(t *testing.T) {
 		})
 	}
 }
+
+func TestPackageJobOpts_RecommendActions(t *testing.T) {
+	testCases := map[string]struct {
+		mockDependencies func(*gomock.Controller, *packageJobOpts)
+
+		wantedErr error
+	}{
+		"reuse svc package RecommandActions": {
+			mockDependencies: func(ctrl *gomock.Controller, opts *packageJobOpts) {
+				mockCmd := mocks.NewMockactionCommand(ctrl)
+				mockCmd.EXPECT().RecommendActions().Return(nil)
+				opts.packageCmd = mockCmd
+			},
+			wantedErr: nil,
+		},
+	}
+
+	for name, tc := range testCases {
+		t.Run(name, func(t *testing.T) {
+			// GIVEN
+			ctrl := gomock.NewController(t)
+			defer ctrl.Finish()
+
+			opts := &packageJobOpts{
+				packageCmd: mocks.NewMockactionCommand(ctrl),
+			}
+			tc.mockDependencies(ctrl, opts)
+
+			// WHEN
+			err := opts.RecommendActions()
+
+			// THEN
+			require.Equal(t, tc.wantedErr, err)
+		})
+	}
+}

internal/pkg/cli/svc_deploy.go

@@ -50,7 +50,6 @@ type deploySvcOpts struct {
 	unmarshal            func([]byte) (manifest.WorkloadManifest, error)
 	newInterpolator      func(app, env string) interpolator
 	cmd                  runner
-	envUpgradeCmd        actionCommand
 	sessProvider         *sessions.Provider
 	newSvcDeployer       func() (workloadDeployer, error)
 	envFeaturesDescriber versionCompatibilityChecker
@@ -82,6 +81,7 @@ func newSvcDeployOpts(vars deployWkldVars) (*deploySvcOpts, error) {
 
 	store := config.NewSSMStore(identity.New(defaultSession), ssm.New(defaultSession), aws.StringValue(defaultSession.Config.Region))
 	prompter := prompt.New()
+
 	opts := &deploySvcOpts{
 		deployWkldVars: vars,
 
@@ -171,9 +171,6 @@ func (o *deploySvcOpts) Execute() error {
 			return err
 		}
 	}
-	if err := o.envUpgradeCmd.Execute(); err != nil {
-		return fmt.Errorf(`execute "env upgrade --app %s --name %s": %v`, o.appName, o.envName, err)
-	}
 	mft, err := workloadManifest(&workloadManifestInput{
 		name:         o.name,
 		appName:      o.appName,
@@ -186,6 +183,9 @@ func (o *deploySvcOpts) Execute() error {
 		return err
 	}
 	o.appliedManifest = mft
+	if err := validateManifestCompatibilityWithEnv(mft, o.envName, o.envFeaturesDescriber); err != nil {
+		return err
+	}
 	deployer, err := o.newSvcDeployer()
 	if err != nil {
 		return err
@@ -315,15 +315,6 @@ func (o *deploySvcOpts) configureClients() error {
 	}
 	o.svcType = svc.Type
 
-	cmd, err := newEnvUpgradeOpts(envUpgradeVars{
-		appName: o.appName,
-		name:    env.Name,
-	})
-	if err != nil {
-		return fmt.Errorf("new env upgrade command: %v", err)
-	}
-	o.envUpgradeCmd = cmd
-
 	// client to retrieve an application's resources created with CloudFormation.
 	defaultSess, err := o.sessProvider.Default()
 	if err != nil {
@@ -337,6 +328,15 @@ func (o *deploySvcOpts) configureClients() error {
 	}
 	o.rootUserARN = caller.RootUserARN
 
+	envDescriber, err := describe.NewEnvDescriber(describe.NewEnvDescriberConfig{
+		App:         o.appName,
+		Env:         o.envName,
+		ConfigStore: o.store,
+	})
+	if err != nil {
+		return err
+	}
+	o.envFeaturesDescriber = envDescriber
 	return nil
 }
 
@@ -349,7 +349,7 @@ type workloadManifestInput struct {
 	unmarshal    func([]byte) (manifest.WorkloadManifest, error)
 }
 
-func workloadManifest(in *workloadManifestInput) (interface{}, error) {
+func workloadManifest(in *workloadManifestInput) (manifest.WorkloadManifest, error) {
 	raw, err := in.ws.ReadWorkloadManifest(in.name)
 	if err != nil {
 		return nil, fmt.Errorf("read manifest file for %s: %w", in.name, err)
@@ -372,7 +372,7 @@ func workloadManifest(in *workloadManifestInput) (interface{}, error) {
 	return envMft, nil
 }
 
-func isManifestCompatibleWithEnvironment(mft manifest.WorkloadManifest, envName string, env versionCompatibilityChecker) error {
+func validateManifestCompatibilityWithEnv(mft manifest.WorkloadManifest, envName string, env versionCompatibilityChecker) error {
 	availableFeatures, err := env.AvailableFeatures()
 	if err != nil {
 		return fmt.Errorf("get available features of the %s environment stack: %w", envName, err)
@@ -388,14 +388,16 @@ func isManifestCompatibleWithEnvironment(mft manifest.WorkloadManifest, envName
 		if _, ok := available[f]; !ok {
 			logMsg := fmt.Sprintf(`Your manifest configuration requires your environment %q to have the feature %q available.`, envName, template.FriendlyEnvFeatureName(f))
 			if v := template.LeastVersionForFeature(f); v != "" {
-				logMsg += fmt.Sprintf(`The least environment version that supports the feature is %s.`, v)
+				logMsg += fmt.Sprintf(` The least environment version that supports the feature is %s.`, v)
 			}
 			if currVersion, err := env.Version(); err == nil {
-				logMsg += fmt.Sprintf(" Your environment is on %s.\n", currVersion)
+				logMsg += fmt.Sprintf(" Your environment is on %s.", currVersion)
 			}
-			logMsg += fmt.Sprintf(`Please upgrade your environment by running %s.`, color.HighlightCode(fmt.Sprintf("copilot env deploy --name %s", envName)))
 			log.Errorln(logMsg)
-			return fmt.Errorf("environment %q is not on a version that supports the %q feature", envName, template.FriendlyEnvFeatureName(f))
+			return &errManifestIncompatibleWithEnvironment{
+				missingFeature: f,
+				envName:        envName,
+			}
 		}
 	}
 	return nil
@@ -467,6 +469,21 @@ func (o *deploySvcOpts) getTargetApp() (*config.Application, error) {
 	return o.targetApp, nil
 }
 
+type errManifestIncompatibleWithEnvironment struct {
+	missingFeature string
+	envName        string
+}
+
+func (e *errManifestIncompatibleWithEnvironment) Error() string {
+	return fmt.Sprintf("environment %q is not on a version that supports the %q feature", e.envName, template.FriendlyEnvFeatureName(e.missingFeature))
+}
+
+// RecommendActions returns recommended actions to be taken after the error.
+// Implements main.actionRecommender interface.
+func (e *errManifestIncompatibleWithEnvironment) RecommendActions() string {
+	return fmt.Sprintf("You can upgrade your environment template by running %s.\n", color.HighlightCode(fmt.Sprintf("copilot env deploy --name %s", e.envName)))
+}
+
 // buildSvcDeployCmd builds the `svc deploy` subcommand.
 func buildSvcDeployCmd() *cobra.Command {
 	vars := deployWkldVars{}