chore(cli): better error msg if workload deploy/package to an undeplo…

…yed env

internal/pkg/cli/job_deploy_test.go

@@ -218,6 +218,7 @@ func TestJobDeployOpts_Execute(t *testing.T) {
  mock: func(m *deployMocks) {
  m.mockWsReader.EXPECT().ReadWorkloadManifest(mockJobName).Return([]byte(""), nil)
  m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
+ m.mockEnvFeaturesDescriber.EXPECT().Version().Return("v1.mock", nil)
  m.mockEnvFeaturesDescriber.EXPECT().AvailableFeatures().Return(nil, mockError)
  },
 
@@ -232,8 +233,8 @@ func TestJobDeployOpts_Execute(t *testing.T) {
  return []string{"mockFeature1", "mockFeature3"}
  },
  }
- m.mockEnvFeaturesDescriber.EXPECT().AvailableFeatures().Return([]string{"mockFeature1", "mockFeature2"}, nil)
  m.mockEnvFeaturesDescriber.EXPECT().Version().Return("v1.mock", nil)
+ m.mockEnvFeaturesDescriber.EXPECT().AvailableFeatures().Return([]string{"mockFeature1", "mockFeature2"}, nil)
  },
  wantedError: fmt.Errorf(`environment "prod-iad" is on version "v1.mock" which does not support the "mockFeature3" feature`),
  },
@@ -246,8 +247,8 @@ func TestJobDeployOpts_Execute(t *testing.T) {
  return []string{"mockFeature1"}
  },
  }
+ m.mockEnvFeaturesDescriber.EXPECT().Version().Return("v1.mock", nil)
  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)
  },
@@ -263,8 +264,8 @@ func TestJobDeployOpts_Execute(t *testing.T) {
  return []string{"mockFeature1"}
  },
  }
+ m.mockEnvFeaturesDescriber.EXPECT().Version().Return("v1.mock", nil)
  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)

internal/pkg/cli/svc_deploy.go

@@ -13,6 +13,7 @@ import (
  "github.com/aws/aws-sdk-go/service/ssm"
  "github.com/aws/copilot-cli/internal/pkg/aws/identity"
  "github.com/aws/copilot-cli/internal/pkg/aws/tags"
+ "github.com/aws/copilot-cli/internal/pkg/deploy"
  "github.com/aws/copilot-cli/internal/pkg/deploy/cloudformation/stack"
  "github.com/aws/copilot-cli/internal/pkg/template"
  "github.com/spf13/afero"
@@ -395,6 +396,13 @@ func workloadManifest(in *workloadManifestInput) (manifest.DynamicWorkload, erro
 }
 
 func validateWorkloadManifestCompatibilityWithEnv(ws wsEnvironmentsLister, env versionCompatibilityChecker, mft manifest.DynamicWorkload, envName string) error {
+ currVersion, err := env.Version()
+ if err != nil {
+ return fmt.Errorf("get environment %q version: %w", envName, err)
+ }
+ if currVersion == deploy.EnvTemplateVersionBootstrap {
+ return fmt.Errorf(`cannot deploy a service to an undeployed environment. Please run "copilot env deploy --name %s" to deploy the environment first`, envName)
+ }
  availableFeatures, err := env.AvailableFeatures()
  if err != nil {
  return fmt.Errorf("get available features of the %s environment stack: %w", envName, err)
@@ -412,10 +420,7 @@ func validateWorkloadManifestCompatibilityWithEnv(ws wsEnvironmentsLister, env v
  if v := template.LeastVersionForFeature(f); v != "" {
  logMsg += fmt.Sprintf(` The least environment version that supports the feature is %s.`, v)
  }
- currVersion, err := env.Version()
- if err == nil {
- logMsg += fmt.Sprintf(" Your environment is on %s.", currVersion)
- }
+ logMsg += fmt.Sprintf(" Your environment is on %s.", currVersion)
  log.Errorln(logMsg)
  return &errFeatureIncompatibleWithEnvironment{
  ws: ws,

internal/pkg/cli/svc_deploy_test.go

@@ -9,12 +9,13 @@ import (
  "testing"
 
  "github.com/aws/aws-sdk-go/aws/session"
+ "github.com/aws/copilot-cli/internal/pkg/deploy"
  "github.com/aws/copilot-cli/internal/pkg/manifest"
  "github.com/aws/copilot-cli/internal/pkg/template"
  "github.com/golang/mock/gomock"
  "github.com/stretchr/testify/require"
 
- "github.com/aws/copilot-cli/internal/pkg/cli/deploy"
+ clideploy "github.com/aws/copilot-cli/internal/pkg/cli/deploy"
  "github.com/aws/copilot-cli/internal/pkg/cli/mocks"
  "github.com/aws/copilot-cli/internal/pkg/config"
 )
@@ -162,6 +163,7 @@ func TestSvcDeployOpts_Execute(t *testing.T) {
  mock: func(m *deployMocks) {
  m.mockWsReader.EXPECT().ReadWorkloadManifest(mockSvcName).Return([]byte(""), nil)
  m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
+ m.mockEnvFeaturesDescriber.EXPECT().Version().Return("v1.mock", nil)
  m.mockEnvFeaturesDescriber.EXPECT().AvailableFeatures().Return(nil, mockError)
  },
 
@@ -176,8 +178,8 @@ func TestSvcDeployOpts_Execute(t *testing.T) {
  return []string{"mockFeature1", "mockFeature3"}
  },
  }
- m.mockEnvFeaturesDescriber.EXPECT().AvailableFeatures().Return([]string{"mockFeature1", "mockFeature2"}, nil)
  m.mockEnvFeaturesDescriber.EXPECT().Version().Return("v1.mock", nil)
+ m.mockEnvFeaturesDescriber.EXPECT().AvailableFeatures().Return([]string{"mockFeature1", "mockFeature2"}, nil)
  },
 
  wantedError: fmt.Errorf(`environment "prod-iad" is on version "v1.mock" which does not support the "mockFeature3" feature`),
@@ -191,8 +193,8 @@ func TestSvcDeployOpts_Execute(t *testing.T) {
  return []string{"mockFeature1"}
  },
  }
+ m.mockEnvFeaturesDescriber.EXPECT().Version().Return("v1.mock", nil)
  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)
  },
@@ -208,9 +210,9 @@ func TestSvcDeployOpts_Execute(t *testing.T) {
  return []string{"mockFeature1"}
  },
  }
+ m.mockEnvFeaturesDescriber.EXPECT().Version().Return("v1.mock", nil)
  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().UploadArtifacts().Return(&clideploy.UploadArtifactsOutput{}, nil)
  m.mockDeployer.EXPECT().DeployWorkload(gomock.Any()).Return(nil, mockError)
  m.mockDeployer.EXPECT().IsServiceAvailableInRegion("").Return(false, nil)
  },
@@ -226,9 +228,9 @@ func TestSvcDeployOpts_Execute(t *testing.T) {
  return []string{"mockFeature1"}
  },
  }
+ m.mockEnvFeaturesDescriber.EXPECT().Version().Return("v1.mock", nil)
  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().UploadArtifacts().Return(&clideploy.UploadArtifactsOutput{}, nil)
  m.mockDeployer.EXPECT().DeployWorkload(gomock.Any()).Return(nil, nil)
  m.mockDeployer.EXPECT().IsServiceAvailableInRegion("").Return(false, nil)
  },
@@ -292,14 +294,28 @@ type checkEnvironmentCompatibilityMocks struct {
 }
 
 func Test_isManifestCompatibleWithEnvironment(t *testing.T) {
+ mockError := errors.New("some error")
  testCases := map[string]struct {
  setupMock func(m *checkEnvironmentCompatibilityMocks)
  mockManifest *mockWorkloadMft
  wantedError error
  }{
+ "error getting environment version": {
+ setupMock: func(m *checkEnvironmentCompatibilityMocks) {
+ m.versionFeatureGetter.EXPECT().Version().Return("", mockError)
+ },
+ wantedError: errors.New("get environment \"mockEnv\" version: some error"),
+ },
+ "error if env is not deployed": {
+ setupMock: func(m *checkEnvironmentCompatibilityMocks) {
+ m.versionFeatureGetter.EXPECT().Version().Return(deploy.EnvTemplateVersionBootstrap, nil)
+ },
+ wantedError: errors.New("cannot deploy a service to an undeployed environment. Please run \"copilot env deploy --name mockEnv\" to deploy the environment first"),
+ },
  "error getting environment available features": {
  setupMock: func(m *checkEnvironmentCompatibilityMocks) {
- m.versionFeatureGetter.EXPECT().AvailableFeatures().Return(nil, errors.New("some error"))
+ m.versionFeatureGetter.EXPECT().Version().Return("mockVersion", nil)
+ m.versionFeatureGetter.EXPECT().AvailableFeatures().Return(nil, mockError)
  m.requiredEnvironmentFeaturesFunc = func() []string {
  return nil
  }
@@ -308,8 +324,8 @@ func Test_isManifestCompatibleWithEnvironment(t *testing.T) {
  },
  "not compatible": {
  setupMock: func(m *checkEnvironmentCompatibilityMocks) {
- m.versionFeatureGetter.EXPECT().AvailableFeatures().Return([]string{template.ALBFeatureName}, nil)
  m.versionFeatureGetter.EXPECT().Version().Return("mockVersion", nil)
+ m.versionFeatureGetter.EXPECT().AvailableFeatures().Return([]string{template.ALBFeatureName}, nil)
  m.requiredEnvironmentFeaturesFunc = func() []string {
  return []string{template.InternalALBFeatureName}
  }
@@ -318,6 +334,7 @@ func Test_isManifestCompatibleWithEnvironment(t *testing.T) {
  },
  "compatible": {
  setupMock: func(m *checkEnvironmentCompatibilityMocks) {
+ m.versionFeatureGetter.EXPECT().Version().Return("mockVersion", nil)
  m.versionFeatureGetter.EXPECT().AvailableFeatures().Return([]string{template.ALBFeatureName, template.InternalALBFeatureName}, nil)
  m.requiredEnvironmentFeaturesFunc = func() []string {
  return []string{template.InternalALBFeatureName}

internal/pkg/cli/svc_package.go

@@ -308,6 +308,9 @@ func (o *packageSvcOpts) getStackGenerator(env *config.Environment) (workloadSta
  return nil, err
  }
  o.appliedDynamicMft = mft
+ if err := validateWorkloadManifestCompatibilityWithEnv(o.ws, o.envFeaturesDescriber, o.appliedDynamicMft, o.envName); err != nil {
+ return nil, err
+ }
  return o.newStackGenerator(o)
 }
 
@@ -411,9 +414,9 @@ func (o *packageSvcOpts) writeAndClose(wc io.WriteCloser, dat string) error {
  return wc.Close()
 }
 
-// RecommendActions suggests recommended actions before the packaged template is used for deployment.
+// RecommendActions is a no-op.
 func (o *packageSvcOpts) RecommendActions() error {
- return validateWorkloadManifestCompatibilityWithEnv(o.ws, o.envFeaturesDescriber, o.appliedDynamicMft, o.envName)
+ return nil
 }
 
 func contains(s string, items []string) bool {

internal/pkg/cli/svc_package_test.go

@@ -5,7 +5,6 @@ package cli
 
 import (
  "bytes"
- "errors"
  "io"
  "testing"
 
@@ -214,6 +213,13 @@ count: 1`
  }, nil)
  m.interpolator.EXPECT().Interpolate(lbwsMft).Return(lbwsMft, nil)
  m.generator.EXPECT().AddonsTemplate().Return("", nil)
+ m.envFeaturesDescriber.EXPECT().Version().Return("v1.mock", nil)
+ m.mft = &mockWorkloadMft{
+ mockRequiredEnvironmentFeatures: func() []string {
+ return []string{}
+ },
+ }
+ m.envFeaturesDescriber.EXPECT().AvailableFeatures().Return([]string{}, nil)
  },
  wantedStack: "mystack",
  wantedParams: "myparams",
@@ -230,6 +236,13 @@ count: 1`
  m.ws.EXPECT().ReadWorkloadManifest("api").Return([]byte(rdwsMft), nil)
  m.interpolator.EXPECT().Interpolate(rdwsMft).Return(rdwsMft, nil)
  m.generator.EXPECT().AddonsTemplate().Return("", nil)
+ m.envFeaturesDescriber.EXPECT().Version().Return("v1.mock", nil)
+ m.mft = &mockWorkloadMft{
+ mockRequiredEnvironmentFeatures: func() []string {
+ return []string{}
+ },
+ }
+ m.envFeaturesDescriber.EXPECT().AvailableFeatures().Return([]string{}, nil)
  m.generator.EXPECT().GenerateCloudFormationTemplate(&deploy.GenerateCloudFormationTemplateInput{
  StackRuntimeConfiguration: deploy.StackRuntimeConfiguration{
  RootUserARN: mockARN,
@@ -280,9 +293,8 @@ count: 1`
  return m.generator, nil
  },
  envFeaturesDescriber: m.envFeaturesDescriber,
-
- targetApp: &config.Application{},
- targetEnv: &config.Environment{},
+ targetApp: &config.Application{},
+ targetEnv: &config.Environment{},
  }
  // tc.mockDependencies(ctrl, opts)
 
@@ -297,57 +309,3 @@ count: 1`
  })
  }
 }
-
-func TestPackageSvcOpts_RecommendedActions(t *testing.T) {
- testCases := map[string]struct {
- setupMocks func(m *svcPackageExecuteMock)
- wantedError error
- }{
- "no recommended action when manifest is compatible with env": {
- setupMocks: func(m *svcPackageExecuteMock) {
- m.mft = &mockWorkloadMft{
- mockRequiredEnvironmentFeatures: func() []string {
- return []string{"mockFeature1"}
- },
- }
- m.envFeaturesDescriber.EXPECT().AvailableFeatures().Return([]string{"mockFeature1", "mockFeature2"}, nil)
- },
- },
- "error out when manifest is incompatible with env": {
- setupMocks: func(m *svcPackageExecuteMock) {
- m.mft = &mockWorkloadMft{
- mockRequiredEnvironmentFeatures: func() []string {
- return []string{"mockFeature1", "mockFeature3"}
- },
- }
- m.envFeaturesDescriber.EXPECT().AvailableFeatures().Return([]string{"mockFeature1", "mockFeature2"}, nil)
- m.envFeaturesDescriber.EXPECT().Version().Return("v1.mock", nil)
- },
- wantedError: errors.New("environment \"mockEnv\" is on version \"v1.mock\" which does not support the \"mockFeature3\" feature"),
- },
- }
- for name, tc := range testCases {
- t.Run(name, func(t *testing.T) {
- ctrl := gomock.NewController(t)
- defer ctrl.Finish()
- m := &svcPackageExecuteMock{
- envFeaturesDescriber: mocks.NewMockversionCompatibilityChecker(ctrl),
- }
- tc.setupMocks(m)
- opts := &packageSvcOpts{
- packageSvcVars: packageSvcVars{
- name: "mockSvc",
- envName: "mockEnv",
- },
- envFeaturesDescriber: m.envFeaturesDescriber,
- appliedDynamicMft: m.mft,
- }
- got := opts.RecommendActions()
- if tc.wantedError != nil {
- require.EqualError(t, got, tc.wantedError.Error())
- } else {
- require.NoError(t, got)
- }
- })
- }
-}