diff --git a/internal/pkg/cli/job_deploy_test.go b/internal/pkg/cli/job_deploy_test.go index 6f539946d8c..73163acb784 100644 --- a/internal/pkg/cli/job_deploy_test.go +++ b/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) diff --git a/internal/pkg/cli/svc_deploy.go b/internal/pkg/cli/svc_deploy.go index 95ed6e96666..bcf5a090b47 100644 --- a/internal/pkg/cli/svc_deploy.go +++ b/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, diff --git a/internal/pkg/cli/svc_deploy_test.go b/internal/pkg/cli/svc_deploy_test.go index a8a7f137e5e..4a5d7191efb 100644 --- a/internal/pkg/cli/svc_deploy_test.go +++ b/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} diff --git a/internal/pkg/cli/svc_package.go b/internal/pkg/cli/svc_package.go index f800b4f85be..82225c11e4c 100644 --- a/internal/pkg/cli/svc_package.go +++ b/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 { diff --git a/internal/pkg/cli/svc_package_test.go b/internal/pkg/cli/svc_package_test.go index 3e7040dfb02..ee146fe3219 100644 --- a/internal/pkg/cli/svc_package_test.go +++ b/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) - } - }) - } -}