Skip to content

Commit

Permalink
fix(cli): --app flag can now be overridden by users (#1399)
Browse files Browse the repository at this point in the history 
Resolves #1329

Removes GlobalOpts struct and the viper lib dependency from the CLI.

Previously, if we ran "env init --app other" the overridden value
"other" was ignored and only the workspace's app was used.
This is an issue with viper as was found in #1329. Instead, with this
change we get rid of Global Flags and force commands themselves to write
whether they need an --app flag or not.

_By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
  • Loading branch information
@efekarakus
efekarakus authored Sep 21, 2020
1 parent b5f1c92 commit d7ce60e
Show file tree
Hide file tree
Showing 61 changed files with 1,049 additions and 1,345 deletions.
6 changes: 1 addition & 5 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,15 @@ require (
github.com/karrick/godirwalk v1.15.6 // indirect
github.com/lnquy/cron v1.0.1
github.com/mattn/go-colorable v0.1.6 // indirect
github.com/mitchellh/mapstructure v1.3.0 // indirect
github.com/moby/buildkit v0.7.2
github.com/onsi/ginkgo v1.14.1
github.com/onsi/gomega v1.10.2
github.com/pelletier/go-toml v1.8.0 // indirect
github.com/robfig/cron/v3 v3.0.1
github.com/sirupsen/logrus v1.6.0 // indirect
github.com/smartystreets/goconvey v1.6.4 // indirect
github.com/spf13/afero v1.4.0
github.com/spf13/cast v1.3.1 // indirect
github.com/spf13/cobra v1.0.0
github.com/spf13/jwalterweatherman v1.1.0 // indirect
github.com/spf13/pflag v1.0.5
github.com/spf13/viper v1.7.1
github.com/stretchr/testify v1.6.1
github.com/xlab/treeprint v1.0.0
golang.org/x/crypto v0.0.0-20200510223506-06a226fb4e37 // indirect
Expand Down
141 changes: 1 addition & 140 deletions go.sum
View file

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions internal/pkg/cli/app.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ Applications are a collection of services and environments.`,
Applications are a collection of services and environments.`,
}

cmd.AddCommand(BuildAppInitCommand())
cmd.AddCommand(BuildAppListCommand())
cmd.AddCommand(BuildAppShowCmd())
cmd.AddCommand(BuildAppDeleteCommand())
cmd.AddCommand(buildAppInitCommand())
cmd.AddCommand(buildAppListCommand())
cmd.AddCommand(buildAppShowCmd())
cmd.AddCommand(buildAppDeleteCommand())

cmd.SetUsageTemplate(template.Usage)
cmd.Annotations = map[string]string{
Expand Down
55 changes: 28 additions & 27 deletions internal/pkg/cli/app_delete.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ var (
)

type deleteAppVars struct {
name string
skipConfirmation bool
envProfiles map[string]string
*GlobalOpts
}

type deleteAppOpts struct {
Expand All @@ -55,6 +55,7 @@ type deleteAppOpts struct {
ws wsFileDeleter
sessProvider sessionProvider
cfn deployer
prompt prompter
s3 func(session *session.Session) bucketEmptier
executor func(svcName string) (executor, error)
askExecutor func(envName, envProfile string) (askExecutor, error)
Expand Down Expand Up @@ -85,14 +86,15 @@ func newDeleteAppOpts(vars deleteAppVars) (*deleteAppOpts, error) {
ws: ws,
sessProvider: provider,
cfn: cloudformation.New(defaultSession),
prompt: prompt.New(),
s3: func(session *session.Session) bucketEmptier {
return s3.New(session)
},
executor: func(svcName string) (executor, error) {
opts, err := newDeleteSvcOpts(deleteSvcVars{
SkipConfirmation: true, // always skip sub-confirmations
GlobalOpts: NewGlobalOpts(),
Name: svcName,
skipConfirmation: true, // always skip sub-confirmations
name: svcName,
appName: vars.name,
})
if err != nil {
return nil, err
Expand All @@ -101,10 +103,10 @@ func newDeleteAppOpts(vars deleteAppVars) (*deleteAppOpts, error) {
},
askExecutor: func(envName, envProfile string) (askExecutor, error) {
opts, err := newDeleteEnvOpts(deleteEnvVars{
SkipConfirmation: true,
GlobalOpts: NewGlobalOpts(),
EnvName: envName,
EnvProfile: envProfile,
skipConfirmation: true,
appName: vars.name,
name: envName,
profile: envProfile,
})
if err != nil {
return nil, err
Expand All @@ -113,9 +115,9 @@ func newDeleteAppOpts(vars deleteAppVars) (*deleteAppOpts, error) {
},
deletePipelineRunner: func() (deletePipelineRunner, error) {
opts, err := newDeletePipelineOpts(deletePipelineVars{
GlobalOpts: NewGlobalOpts(),
SkipConfirmation: true,
DeleteSecret: true,
appName: vars.name,
skipConfirmation: true,
shouldDeleteSecret: true,
})
if err != nil {
return nil, err
Expand All @@ -127,7 +129,7 @@ func newDeleteAppOpts(vars deleteAppVars) (*deleteAppOpts, error) {

// Validate returns an error if the user's input is invalid.
func (o *deleteAppOpts) Validate() error {
if o.AppName() == "" {
if o.name == "" {
return errNoAppInWorkspace
}
return nil
Expand All @@ -140,7 +142,7 @@ func (o *deleteAppOpts) Ask() error {
}

manualConfirm, err := o.prompt.Confirm(
fmt.Sprintf(fmtDeleteAppConfirmPrompt, o.AppName()),
fmt.Sprintf(fmtDeleteAppConfirmPrompt, o.name),
deleteAppConfirmHelp,
prompt.WithTrueDefault())
if err != nil {
Expand Down Expand Up @@ -192,9 +194,9 @@ func (o *deleteAppOpts) Execute() error {
}

func (o *deleteAppOpts) deleteSvcs() error {
svcs, err := o.store.ListServices(o.AppName())
svcs, err := o.store.ListServices(o.name)
if err != nil {
return fmt.Errorf("list services for application %s: %w", o.AppName(), err)
return fmt.Errorf("list services for application %s: %w", o.name, err)
}

for _, svc := range svcs {
Expand All @@ -210,9 +212,9 @@ func (o *deleteAppOpts) deleteSvcs() error {
}

func (o *deleteAppOpts) deleteEnvs() error {
envs, err := o.store.ListEnvironments(o.AppName())
envs, err := o.store.ListEnvironments(o.name)
if err != nil {
return fmt.Errorf("list environments for application %s: %w", o.AppName(), err)
return fmt.Errorf("list environments for application %s: %w", o.name, err)
}

for _, env := range envs {
Expand All @@ -236,9 +238,9 @@ func (o *deleteAppOpts) deleteEnvs() error {
}

func (o *deleteAppOpts) emptyS3Bucket() error {
app, err := o.store.GetApplication(o.AppName())
app, err := o.store.GetApplication(o.name)
if err != nil {
return fmt.Errorf("get application %s: %w", o.AppName(), err)
return fmt.Errorf("get application %s: %w", o.name, err)
}
appResources, err := o.cfn.GetRegionalAppResources(app)
if err != nil {
Expand Down Expand Up @@ -272,7 +274,7 @@ func (o *deleteAppOpts) deletePipeline() error {

func (o *deleteAppOpts) deleteAppResources() error {
o.spinner.Start(deleteAppResourcesStartMsg)
if err := o.cfn.DeleteApp(o.AppName()); err != nil {
if err := o.cfn.DeleteApp(o.name); err != nil {
o.spinner.Stop(log.Serror("Error deleting application resources."))
return fmt.Errorf("delete app resources: %w", err)
}
Expand All @@ -282,9 +284,9 @@ func (o *deleteAppOpts) deleteAppResources() error {

func (o *deleteAppOpts) deleteAppConfigs() error {
o.spinner.Start(deleteAppConfigStartMsg)
if err := o.store.DeleteApplication(o.AppName()); err != nil {
if err := o.store.DeleteApplication(o.name); err != nil {
o.spinner.Stop(log.Serror("Error deleting application configuration."))
return fmt.Errorf("delete application %s configuration: %w", o.AppName(), err)
return fmt.Errorf("delete application %s configuration: %w", o.name, err)
}
o.spinner.Stop(log.Ssuccess(deleteAppConfigStopMsg))
return nil
Expand All @@ -300,11 +302,9 @@ func (o *deleteAppOpts) deleteWs() error {
return nil
}

// BuildAppDeleteCommand builds the `app delete` subcommand.
func BuildAppDeleteCommand() *cobra.Command {
vars := deleteAppVars{
GlobalOpts: NewGlobalOpts(),
}
// buildAppDeleteCommand builds the `app delete` subcommand.
func buildAppDeleteCommand() *cobra.Command {
vars := deleteAppVars{}
cmd := &cobra.Command{
Use: "delete",
Short: "Delete all resources associated with the application.",
Expand All @@ -327,6 +327,7 @@ func BuildAppDeleteCommand() *cobra.Command {
}),
}

cmd.Flags().StringVarP(&vars.name, nameFlag, nameFlagShort, tryReadingAppName(), appFlagDescription)
cmd.Flags().BoolVar(&vars.skipConfirmation, yesFlag, false, yesFlagDescription)
cmd.Flags().StringToStringVar(&vars.envProfiles, envProfilesFlag, nil, envProfilesFlagDescription)
return cmd
Expand Down
14 changes: 4 additions & 10 deletions internal/pkg/cli/app_delete_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ func TestDeleteAppOpts_Validate(t *testing.T) {
t.Run(name, func(t *testing.T) {
opts := deleteAppOpts{
deleteAppVars: deleteAppVars{
GlobalOpts: &GlobalOpts{
appName: test.name,
},
name: test.name,
},
}

Expand Down Expand Up @@ -114,12 +112,10 @@ func TestDeleteAppOpts_Ask(t *testing.T) {
test.setupMocks(ctrl)
opts := deleteAppOpts{
deleteAppVars: deleteAppVars{
GlobalOpts: &GlobalOpts{
appName: mockAppName,
prompt: mockPrompter,
},
name: mockAppName,
skipConfirmation: test.skipConfirmation,
},
prompt: mockPrompter,
}

got := opts.Ask()
Expand Down Expand Up @@ -303,9 +299,7 @@ func TestDeleteAppOpts_Execute(t *testing.T) {

opts := deleteAppOpts{
deleteAppVars: deleteAppVars{
GlobalOpts: &GlobalOpts{
appName: mockAppName,
},
name: mockAppName,
},
spinner: mockSpinner,
store: mockStore,
Expand Down
Loading

0 comments on commit d7ce60e

Please sign in to comment.