Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions internal/pkg/cli/env_deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type deployEnvVars struct {
forceNewUpdate bool
disableRollback bool
showDiff bool
skipDiffPrompt bool
}

type deployEnvOpts struct {
Expand Down Expand Up @@ -227,6 +228,9 @@ func (o *deployEnvOpts) showDiffAndConfirmDeployment(deployer envDeployer, input
return false, fmt.Errorf("generate diff for environment %q: %w", o.name, err)
}
}
if o.skipDiffPrompt {
return true, nil
}
contd, err := o.prompt.Confirm(continueDeploymentPrompt, "")
if err != nil {
return false, fmt.Errorf("ask whether to continue with the deployment: %w", err)
Expand Down Expand Up @@ -330,5 +334,6 @@ Deploy an environment named "test".
cmd.Flags().BoolVar(&vars.forceNewUpdate, forceFlag, false, forceEnvDeployFlagDescription)
cmd.Flags().BoolVar(&vars.disableRollback, noRollbackFlag, false, noRollbackFlagDescription)
cmd.Flags().BoolVar(&vars.showDiff, diffFlag, false, diffFlagDescription)
cmd.Flags().BoolVar(&vars.skipDiffPrompt, diffAutoApproveFlag, false, diffAutoApproveFlagDescription)
return cmd
}
23 changes: 21 additions & 2 deletions internal/pkg/cli/env_deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ type deployEnvExecuteMocks struct {
func TestDeployEnvOpts_Execute(t *testing.T) {
testCases := map[string]struct {
inShowDiff bool
inSkipDiffPrompt bool
unmarshalManifest func(in []byte) (*manifest.Environment, error)
setUpMocks func(m *deployEnvExecuteMocks)
wantedDiff string
Expand Down Expand Up @@ -314,6 +315,23 @@ func TestDeployEnvOpts_Execute(t *testing.T) {
m.deployer.EXPECT().DeployEnvironment(gomock.Any()).Times(1)
},
},
"skip prompt and deploy immediately after diff": {
inShowDiff: true,
inSkipDiffPrompt: true,
setUpMocks: func(m *deployEnvExecuteMocks) {
m.ws.EXPECT().ReadEnvironmentManifest(gomock.Any()).Return([]byte("name: mockEnv\ntype: Environment\n"), nil)
m.interpolator.EXPECT().Interpolate(gomock.Any()).Return("name: mockEnv\ntype: Environment\n", nil)
m.identity.EXPECT().Get().Return(identity.Caller{
RootUserARN: "mockRootUserARN",
}, nil)
m.deployer.EXPECT().Validate(gomock.Any()).Return(nil)
m.deployer.EXPECT().UploadArtifacts().Return(&deploy.UploadEnvArtifactsOutput{}, nil)
m.deployer.EXPECT().GenerateCloudFormationTemplate(gomock.Any()).Return(&deploy.GenerateCloudFormationTemplateOutput{}, nil)
m.deployer.EXPECT().DeployDiff(gomock.Any()).Return("", nil)
m.prompter.EXPECT().Confirm(gomock.Eq(continueDeploymentPrompt), gomock.Any(), gomock.Any()).Times(0)
m.deployer.EXPECT().DeployEnvironment(gomock.Any()).Times(1)
},
},
"fail to deploy the environment": {
setUpMocks: func(m *deployEnvExecuteMocks) {
m.ws.EXPECT().ReadEnvironmentManifest(gomock.Any()).Return([]byte("name: mockEnv\ntype: Environment\n"), nil)
Expand Down Expand Up @@ -374,8 +392,9 @@ func TestDeployEnvOpts_Execute(t *testing.T) {
tc.setUpMocks(m)
opts := deployEnvOpts{
deployEnvVars: deployEnvVars{
name: "mockEnv",
showDiff: tc.inShowDiff,
name: "mockEnv",
showDiff: tc.inShowDiff,
skipDiffPrompt: tc.inSkipDiffPrompt,
},
ws: m.ws,
identity: m.identity,
Expand Down
4 changes: 3 additions & 1 deletion internal/pkg/cli/flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const (
uploadAssetsFlag = "upload-assets"
deployFlag = "deploy"
diffFlag = "diff"
diffAutoApproveFlag = "diff-yes"
sourcesFlag = "sources"

// Flags for operational commands.
Expand Down Expand Up @@ -253,7 +254,8 @@ const (
yesFlagDescription = "Skips confirmation prompt."
resourceTagsFlagDescription = `Optional. Labels with a key and value separated by commas.
Allows you to categorize resources.`
diffFlagDescription = "Compares the generated CloudFormation template to the deployed stack."
diffFlagDescription = "Compares the generated CloudFormation template to the deployed stack."
diffAutoApproveFlagDescription = "Skip interactive approval of diff before deploying."

// Deployment.
deployTestFlagDescription = `Deploy your service or job to a "test" environment.`
Expand Down
7 changes: 6 additions & 1 deletion internal/pkg/cli/svc_deploy.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type deployWkldVars struct {
forceNewUpdate bool // NOTE: this variable is not applicable for a job workload currently.
disableRollback bool
showDiff bool
skipDiffPrompt bool

// To facilitate unit tests.
clientConfigured bool
Expand Down Expand Up @@ -261,7 +262,10 @@ func (o *deploySvcOpts) Execute() error {
return err
}
}
contd, err := o.prompt.Confirm(continueDeploymentPrompt, "")
contd, err := o.skipDiffPrompt, nil
if !o.skipDiffPrompt {
contd, err = o.prompt.Confirm(continueDeploymentPrompt, "")
}
Comment on lines +265 to +268
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it's okay to write ⬇️ here? Because if we got --diff-yes that means noDeploy should render to false 🤔

if o.skipDiffPrompt {
 return true, nil
}

Copy link
Copy Markdown
Contributor Author

@efekarakus efekarakus May 3, 2023

Choose a reason for hiding this comment

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

oh I wasn't sure because l.277 in the same method seems to do the actual deployment, so if I were to return it'd skip the deployment altogether I think 💭

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

oh lol that's right

if err != nil {
return fmt.Errorf("ask whether to continue with the deployment: %w", err)
}
Expand Down Expand Up @@ -642,5 +646,6 @@ func buildSvcDeployCmd() *cobra.Command {
cmd.Flags().BoolVar(&vars.forceNewUpdate, forceFlag, false, forceFlagDescription)
cmd.Flags().BoolVar(&vars.disableRollback, noRollbackFlag, false, noRollbackFlagDescription)
cmd.Flags().BoolVar(&vars.showDiff, diffFlag, false, diffFlagDescription)
cmd.Flags().BoolVar(&vars.skipDiffPrompt, diffAutoApproveFlag, false, diffAutoApproveFlagDescription)
return cmd
}
40 changes: 32 additions & 8 deletions internal/pkg/cli/svc_deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,11 @@ func TestSvcDeployOpts_Execute(t *testing.T) {
)
mockError := errors.New("some error")
testCases := map[string]struct {
inShowDiff bool
mock func(m *deployMocks)
wantedDiff string
wantedError error
inShowDiff bool
inSkipDiffPrompt bool
mock func(m *deployMocks)
wantedDiff string
wantedError error
}{
"error out if fail to read workload manifest": {
mock: func(m *deployMocks) {
Expand Down Expand Up @@ -347,6 +348,28 @@ func TestSvcDeployOpts_Execute(t *testing.T) {
m.mockDeployer.EXPECT().DeployWorkload(gomock.Any()).Times(1)
},
},
"skip prompt and deploy immediately after diff": {
inShowDiff: true,
inSkipDiffPrompt: true,
mock: func(m *deployMocks) {
m.mockWsReader.EXPECT().ReadWorkloadManifest(mockSvcName).Return([]byte(""), nil)
m.mockInterpolator.EXPECT().Interpolate("").Return("", nil)
m.mockMft = &mockWorkloadMft{
mockRequiredEnvironmentFeatures: func() []string {
return []string{"mockFeature1"}
},
}
m.mockEnvFeaturesDescriber.EXPECT().Version().Return("v1.mock", nil)
m.mockEnvFeaturesDescriber.EXPECT().AvailableFeatures().Return([]string{"mockFeature1", "mockFeature2"}, nil)
m.mockDeployer.EXPECT().IsServiceAvailableInRegion("").Return(false, nil)
m.mockDeployer.EXPECT().UploadArtifacts().Return(&clideploy.UploadArtifactsOutput{}, nil)
m.mockDeployer.EXPECT().GenerateCloudFormationTemplate(gomock.Any()).Return(&clideploy.GenerateCloudFormationTemplateOutput{}, nil)
m.mockDeployer.EXPECT().DeployDiff(gomock.Any()).Return("mock diff", nil)
m.mockDiffWriter = &strings.Builder{}
m.mockPrompter.EXPECT().Confirm(gomock.Eq("Continue with the deployment?"), gomock.Any(), gomock.Any()).Times(0)
m.mockDeployer.EXPECT().DeployWorkload(gomock.Any()).Times(1)
},
},
"error if failed to deploy service": {
mock: func(m *deployMocks) {
m.mockWsReader.EXPECT().ReadWorkloadManifest(mockSvcName).Return([]byte(""), nil)
Expand Down Expand Up @@ -400,10 +423,11 @@ func TestSvcDeployOpts_Execute(t *testing.T) {

opts := deploySvcOpts{
deployWkldVars: deployWkldVars{
appName: mockAppName,
name: mockSvcName,
envName: mockEnvName,
showDiff: tc.inShowDiff,
appName: mockAppName,
name: mockSvcName,
envName: mockEnvName,
showDiff: tc.inShowDiff,
skipDiffPrompt: tc.inSkipDiffPrompt,

clientConfigured: true,
},
Expand Down
4 changes: 4 additions & 0 deletions internal/pkg/deploy/cloudformation/stack/static_site_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,10 @@ func TestStaticSite_Parameters(t *testing.T) {
}

func TestStaticSite_SerializedParameters(t *testing.T) {
t.Cleanup(func() {
fs = realEmbedFS
})
fs = templatetest.Stub{}
c, _ := NewStaticSite(&StaticSiteConfig{
EnvManifest: &manifest.Environment{
Workload: manifest.Workload{
Expand Down