From d02a29292eec3bf421de5db0568a13fe727a9fc8 Mon Sep 17 00:00:00 2001 From: Colin Simmons Date: Tue, 5 Mar 2019 15:21:24 +0000 Subject: [PATCH] Make IAAS flag required for all commands Also improved test coverage in commands package Signed-off-by: Irbe Krumina --- README.md | 4 +- commands/commands_test.go | 88 +++++++++++++++++++++---- commands/deploy.go | 22 +++---- commands/deploy/deploy_args.go | 4 ++ commands/deploy/deploy_args_test.go | 11 ++++ commands/destroy.go | 29 ++++---- commands/destroy/destroy_args.go | 7 ++ commands/destroy/destroy_args_test.go | 84 +++++++++++++++++++++++ commands/info.go | 38 +++++++---- commands/info/info_args.go | 12 +++- commands/info/info_args_test.go | 87 ++++++++++++++++++++++++ commands/maintain.go | 37 +++++++---- commands/maintain/maintain_args.go | 10 ++- commands/maintain/maintain_args_test.go | 84 +++++++++++++++++++++++ 14 files changed, 451 insertions(+), 66 deletions(-) create mode 100644 commands/destroy/destroy_args_test.go create mode 100644 commands/info/info_args_test.go create mode 100644 commands/maintain/maintain_args_test.go diff --git a/README.md b/README.md index 451230b68..b7aa8b328 100644 --- a/README.md +++ b/README.md @@ -101,11 +101,11 @@ Download the [latest release](https://github.com/EngineerBetter/control-tower/re #### Choosing an IAAS -The default IAAS for Control-Tower is AWS. To choose a different IAAS use the `--iaas` flag. For every IAAS provider apart from AWS this flag is required for all commands. +Control-Tower can deploy to AWS or GCP. To choose an IAAS use the `--iaas` flag. This is required for all commands. Supported IAAS values: AWS, GCP -- `--iaas value` (optional) IAAS, can be AWS or GCP (default: "AWS") [$IAAS] +- `--iaas value` (required) IAAS, can be AWS or GCP [$IAAS] ### Deploy diff --git a/commands/commands_test.go b/commands/commands_test.go index 4ebdab0f7..f7626b3e8 100644 --- a/commands/commands_test.go +++ b/commands/commands_test.go @@ -45,9 +45,20 @@ var _ = Describe("commands", func() { }) }) + Context("When the IAAS is not specified", func() { + It("Should show a meaningful error", func() { + command := exec.Command(cliPath, "deploy", "abc") + session, err := Start(command, GinkgoWriter, GinkgoWriter) + Expect(err).ToNot(HaveOccurred()) + Eventually(session).Should(Exit(1)) + // Say takes a regexp so `[` and `]` need to be escaped + Expect(session.Err).To(Say("Error validating args on deploy: \\[failed to validate Deploy flags: \\[--iaas flag not set\\]\\]")) + }) + }) + Context("When no name is passed in", func() { It("should display correct usage", func() { - command := exec.Command(cliPath, "deploy") + command := exec.Command(cliPath, "deploy", "--iaas", "AWS") session, err := Start(command, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) Eventually(session).Should(Exit(1)) @@ -57,7 +68,7 @@ var _ = Describe("commands", func() { Context("When there is a key but no cert", func() { It("Should show a meaningful error", func() { - command := exec.Command(cliPath, "deploy", "abc", "--domain", "abc.engineerbetter.com", "--tls-key", "-- BEGIN RSA PRIVATE KEY --") + command := exec.Command(cliPath, "deploy", "abc", "--domain", "abc.engineerbetter.com", "--tls-key", "-- BEGIN RSA PRIVATE KEY --", "--iaas", "AWS") session, err := Start(command, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) Eventually(session).Should(Exit(1)) @@ -67,7 +78,7 @@ var _ = Describe("commands", func() { Context("When there is a cert but no key", func() { It("Should show a meaningful error", func() { - command := exec.Command(cliPath, "deploy", "abc", "--domain", "abc.engineerbetter.com", "--tls-cert", "-- BEGIN CERTIFICATE --") + command := exec.Command(cliPath, "deploy", "abc", "--domain", "abc.engineerbetter.com", "--tls-cert", "-- BEGIN CERTIFICATE --", "--iaas", "AWS") session, err := Start(command, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) Eventually(session).Should(Exit(1)) @@ -77,7 +88,7 @@ var _ = Describe("commands", func() { Context("When there is a cert and key but no domain", func() { It("Should show a meaningful error", func() { - command := exec.Command(cliPath, "deploy", "abc", "--tls-key", "-- BEGIN RSA PRIVATE KEY --", "--tls-cert", "-- BEGIN RSA PRIVATE KEY --") + command := exec.Command(cliPath, "deploy", "abc", "--tls-key", "-- BEGIN RSA PRIVATE KEY --", "--tls-cert", "-- BEGIN RSA PRIVATE KEY --", "--iaas", "AWS") session, err := Start(command, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) Eventually(session).Should(Exit(1)) @@ -87,7 +98,7 @@ var _ = Describe("commands", func() { Context("When an invalid worker count is provided", func() { It("Should show a meaningful error", func() { - command := exec.Command(cliPath, "deploy", "abc", "--workers", "0") + command := exec.Command(cliPath, "deploy", "abc", "--workers", "0", "--iaas", "AWS") session, err := Start(command, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) Eventually(session).Should(Exit(1)) @@ -97,7 +108,7 @@ var _ = Describe("commands", func() { Context("When an invalid worker size is provided", func() { It("Should show a meaningful error", func() { - command := exec.Command(cliPath, "deploy", "abc", "--worker-size", "small") + command := exec.Command(cliPath, "deploy", "abc", "--worker-size", "small", "--iaas", "AWS") session, err := Start(command, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) Eventually(session).Should(Exit(1)) @@ -107,7 +118,7 @@ var _ = Describe("commands", func() { Context("When an invalid web size is provided", func() { It("Should show a meaningful error", func() { - command := exec.Command(cliPath, "deploy", "abc", "--web-size", "tiny") + command := exec.Command(cliPath, "deploy", "abc", "--web-size", "tiny", "--iaas", "AWS") session, err := Start(command, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) Eventually(session).Should(Exit(1)) @@ -117,7 +128,7 @@ var _ = Describe("commands", func() { Context("When an invalid db size is provided", func() { It("Should show a meaningful error", func() { - command := exec.Command(cliPath, "deploy", "abc", "--db-size", "huge") + command := exec.Command(cliPath, "deploy", "abc", "--db-size", "huge", "--iaas", "AWS") session, err := Start(command, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) Eventually(session).Should(Exit(1)) @@ -129,7 +140,7 @@ var _ = Describe("commands", func() { Describe("destroy", func() { Context("When using --help", func() { It("should display usage details", func() { - command := exec.Command(cliPath, "destroy", "--help") + command := exec.Command(cliPath, "destroy", "--help", "--iaas", "AWS") session, err := Start(command, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred(), "Error running CLI: "+cliPath) Eventually(session).Should(Exit(0)) @@ -137,9 +148,20 @@ var _ = Describe("commands", func() { }) }) + Context("When the IAAS is not specified", func() { + It("Should show a meaningful error", func() { + command := exec.Command(cliPath, "destroy", "abc") + session, err := Start(command, GinkgoWriter, GinkgoWriter) + Expect(err).ToNot(HaveOccurred()) + Eventually(session).Should(Exit(1)) + // Say takes a regexp so `[` and `]` need to be escaped + Expect(session.Err).To(Say("Error validating args on destroy: \\[failed to validate Destroy flags: \\[--iaas flag not set\\]\\]")) + }) + }) + Context("When no name is passed in", func() { It("should display correct usage", func() { - command := exec.Command(cliPath, "destroy") + command := exec.Command(cliPath, "destroy", "--iaas", "AWS") session, err := Start(command, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) Eventually(session).Should(Exit(1)) @@ -159,9 +181,20 @@ var _ = Describe("commands", func() { }) }) + Context("When the IAAS is not specified", func() { + It("Should show a meaningful error", func() { + command := exec.Command(cliPath, "info", "abc") + session, err := Start(command, GinkgoWriter, GinkgoWriter) + Expect(err).ToNot(HaveOccurred()) + Eventually(session).Should(Exit(1)) + // Say takes a regexp so `[` and `]` need to be escaped + Expect(session.Err).To(Say("Error validating args on info: \\[failed to validate Info flags: \\[--iaas flag not set\\]\\]")) + }) + }) + Context("When no name is passed in", func() { It("should display correct usage", func() { - command := exec.Command(cliPath, "info") + command := exec.Command(cliPath, "info", "--iaas", "AWS") session, err := Start(command, GinkgoWriter, GinkgoWriter) Expect(err).ToNot(HaveOccurred()) Eventually(session).Should(Exit(1)) @@ -169,4 +202,37 @@ var _ = Describe("commands", func() { }) }) }) + + Describe("maintain", func() { + Context("When using --help", func() { + It("should display usage details", func() { + command := exec.Command(cliPath, "maintain", "--help", "--iaas", "AWS") + session, err := Start(command, GinkgoWriter, GinkgoWriter) + Expect(err).ToNot(HaveOccurred(), "Error running CLI: "+cliPath) + Eventually(session).Should(Exit(0)) + Expect(session.Out).To(Say("control-tower maintain - Handles maintenance operations in control-tower")) + }) + }) + + Context("When the IAAS is not specified", func() { + It("Should show a meaningful error", func() { + command := exec.Command(cliPath, "maintain", "abc") + session, err := Start(command, GinkgoWriter, GinkgoWriter) + Expect(err).ToNot(HaveOccurred()) + Eventually(session).Should(Exit(1)) + // Say takes a regexp so `[` and `]` need to be escaped + Expect(session.Err).To(Say("Error validating args on maintain: \\[failed to validate Maintain flags: \\[--iaas flag not set\\]\\]")) + }) + }) + + Context("When no name is passed in", func() { + It("should display correct usage", func() { + command := exec.Command(cliPath, "maintain", "--iaas", "AWS") + session, err := Start(command, GinkgoWriter, GinkgoWriter) + Expect(err).ToNot(HaveOccurred()) + Eventually(session).Should(Exit(1)) + Expect(session.Err).To(Say("Usage is `control-tower maintain `")) + }) + }) + }) }) diff --git a/commands/deploy.go b/commands/deploy.go index c8f460e64..a6c17a50e 100644 --- a/commands/deploy.go +++ b/commands/deploy.go @@ -82,9 +82,8 @@ var deployFlags = []cli.Flag{ }, cli.StringFlag{ Name: "iaas", - Usage: "(optional) IAAS, can be AWS or GCP", + Usage: "(required) IAAS, can be AWS or GCP", EnvVar: "IAAS", - Value: "AWS", Destination: &initialDeployArgs.IAAS, }, cli.BoolFlag{ @@ -189,12 +188,7 @@ func deployAction(c *cli.Context, deployArgs deploy.Args, provider iaas.Provider version := c.App.Version - deployArgs, err := validateDeployArgs(c, deployArgs) - if err != nil { - return err - } - - deployArgs, err = setZoneAndRegion(provider.Region(), deployArgs) + deployArgs, err := setZoneAndRegion(provider.Region(), deployArgs) if err != nil { return err } @@ -220,11 +214,11 @@ func deployAction(c *cli.Context, deployArgs deploy.Args, provider iaas.Provider func validateDeployArgs(c *cli.Context, deployArgs deploy.Args) (deploy.Args, error) { err := deployArgs.MarkSetFlags(c) if err != nil { - return deployArgs, err + return deployArgs, fmt.Errorf("failed to mark set Deploy flags: [%v]", err) } if err = deployArgs.Validate(); err != nil { - return deployArgs, err + return deployArgs, fmt.Errorf("failed to validate Deploy flags: [%v]", err) } return deployArgs, nil @@ -421,9 +415,13 @@ var deployCmd = cli.Command{ ArgsUsage: "", Flags: deployFlags, Action: func(c *cli.Context) error { - iaasName, err := iaas.Assosiate(initialDeployArgs.IAAS) + deployArgs, err := validateDeployArgs(c, initialDeployArgs) + if err != nil { + return fmt.Errorf("Error validating args on deploy: [%v]", err) + } + iaasName, err := iaas.Assosiate(deployArgs.IAAS) if err != nil { - return err + return fmt.Errorf("Error mapping to supported IAASes on deploy: [%v]", err) } provider, err := iaas.New(iaasName, initialDeployArgs.Region) if err != nil { diff --git a/commands/deploy/deploy_args.go b/commands/deploy/deploy_args.go index 5bf4b7b74..1b797af26 100644 --- a/commands/deploy/deploy_args.go +++ b/commands/deploy/deploy_args.go @@ -142,6 +142,10 @@ func (a *Args) ModifyGithub(GithubAuthClientID, GithubAuthClientSecret string, G // Validate validates that flag interdependencies func (a Args) Validate() error { + if !a.IAASIsSet { + return fmt.Errorf("--iaas flag not set") + } + if err := a.validateCertFields(); err != nil { return err } diff --git a/commands/deploy/deploy_args_test.go b/commands/deploy/deploy_args_test.go index 508ab2fc6..66747589e 100644 --- a/commands/deploy/deploy_args_test.go +++ b/commands/deploy/deploy_args_test.go @@ -18,6 +18,7 @@ func TestDeployArgs_Validate(t *testing.T) { GithubAuthClientID: "", GithubAuthClientSecret: "", IAAS: "AWS", + IAASIsSet: true, SelfUpdate: false, TLSCert: "", TLSKey: "", @@ -61,6 +62,16 @@ func TestDeployArgs_Validate(t *testing.T) { wantErr: true, expectedErr: "--tls-cert requires --tls-key to also be provided", }, + { + name: "IAAS not set", + modification: func() Args { + args := defaultFields + args.IAASIsSet = false + return args + }, + wantErr: true, + expectedErr: "--iaas flag not set", + }, { name: "TLSKey cannot be set without TLSCert", modification: func() Args { diff --git a/commands/destroy.go b/commands/destroy.go index 5b9ca6db3..df91b592a 100644 --- a/commands/destroy.go +++ b/commands/destroy.go @@ -30,9 +30,8 @@ var destroyFlags = []cli.Flag{ }, cli.StringFlag{ Name: "iaas", - Usage: "(optional) IAAS, can be AWS or GCP", + Usage: "(required) IAAS, can be AWS or GCP", EnvVar: "IAAS", - Value: "AWS", Destination: &initialDestroyArgs.IAAS, }, cli.StringFlag{ @@ -63,21 +62,23 @@ func destroyAction(c *cli.Context, destroyArgs destroy.Args, provider iaas.Provi version := c.App.Version - destroyArgs, err := markSetFlags(c, destroyArgs) - if err != nil { - return err - } client, err := buildDestroyClient(name, version, destroyArgs, provider) if err != nil { return err } return client.Destroy() } -func markSetFlags(c *cli.Context, destroyArgs destroy.Args) (destroy.Args, error) { + +func validateDestroyArgs(c *cli.Context, destroyArgs destroy.Args) (destroy.Args, error) { err := destroyArgs.MarkSetFlags(c) if err != nil { - return destroyArgs, err + return destroyArgs, fmt.Errorf("failed to mark set Destroy flags: [%v]", err) + } + + if err = destroyArgs.Validate(); err != nil { + return destroyArgs, fmt.Errorf("failed to validate Destroy flags: [%v]", err) } + return destroyArgs, nil } @@ -121,14 +122,18 @@ var destroyCmd = cli.Command{ ArgsUsage: "", Flags: destroyFlags, Action: func(c *cli.Context) error { - iaasName, err := iaas.Assosiate(initialDestroyArgs.IAAS) + destroyArgs, err := validateDestroyArgs(c, initialDestroyArgs) if err != nil { - return err + return fmt.Errorf("Error validating args on destroy: [%v]", err) + } + iaasName, err := iaas.Assosiate(destroyArgs.IAAS) + if err != nil { + return fmt.Errorf("Error mapping to supported IAASes on destroy: [%v]", err) } - provider, err := iaas.New(iaasName, initialDestroyArgs.Region) + provider, err := iaas.New(iaasName, destroyArgs.Region) if err != nil { return fmt.Errorf("Error creating IAAS provider on destroy: [%v]", err) } - return destroyAction(c, initialDestroyArgs, provider) + return destroyAction(c, destroyArgs, provider) }, } diff --git a/commands/destroy/destroy_args.go b/commands/destroy/destroy_args.go index 359e3e0fe..2fba3c499 100644 --- a/commands/destroy/destroy_args.go +++ b/commands/destroy/destroy_args.go @@ -35,6 +35,13 @@ func (a *Args) MarkSetFlags(c FlagSetChecker) error { return nil } +func (a *Args) Validate() error { + if !a.IAASIsSet { + return fmt.Errorf("--iaas flag not set") + } + return nil +} + // FlagSetChecker allows us to find out if flags were set, adn what the names of all flags are type FlagSetChecker interface { IsSet(name string) bool diff --git a/commands/destroy/destroy_args_test.go b/commands/destroy/destroy_args_test.go new file mode 100644 index 000000000..4079f929e --- /dev/null +++ b/commands/destroy/destroy_args_test.go @@ -0,0 +1,84 @@ +package destroy_test + +import ( + "strings" + "testing" + + . "github.com/EngineerBetter/control-tower/commands/destroy" +) + +func TestDestroyArgs_Validate(t *testing.T) { + defaultFields := Args{ + Region: "eu-west-1", + IAAS: "AWS", + IAASIsSet: true, + } + tests := []struct { + name string + modification func() Args + outcomeCheck func(Args) bool + wantErr bool + expectedErr string + }{ + { + name: "Default args", + modification: func() Args { + return defaultFields + }, + wantErr: false, + }, + { + name: "IAAS not set", + modification: func() Args { + args := defaultFields + args.IAASIsSet = false + return args + }, + wantErr: true, + expectedErr: "--iaas flag not set", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + args := tt.modification() + err := args.Validate() + if (err != nil) != tt.wantErr || (err != nil && tt.wantErr && !strings.Contains(err.Error(), tt.expectedErr)) { + if err != nil { + t.Errorf("DeployArgs.Validate() %v test failed.\nFailed with error = %v,\nExpected error = %v,\nShould fail %v\nWith args: %#v", tt.name, err.Error(), tt.expectedErr, tt.wantErr, args) + } else { + t.Errorf("DeployArgs.Validate() %v test failed.\nShould fail %v\nWith args: %#v", tt.name, tt.wantErr, args) + } + } + if tt.outcomeCheck != nil { + if tt.outcomeCheck(args) { + t.Errorf("DeployArgs.Validate() %v test failed.\nShould fail %v\nWith args: %#v", tt.name, tt.wantErr, args) + } + } + }) + } +} + +type FakeFlagSetChecker struct { + names []string + specifiedFlags []string +} + +func NewFakeFlagSetChecker(names, specifiedFlags []string) FakeFlagSetChecker { + return FakeFlagSetChecker{ + names: names, + specifiedFlags: specifiedFlags, + } +} + +func (f *FakeFlagSetChecker) IsSet(desired string) bool { + for _, flag := range f.specifiedFlags { + if desired == flag { + return true + } + } + return false +} + +func (f *FakeFlagSetChecker) FlagNames() (names []string) { + return names +} diff --git a/commands/info.go b/commands/info.go index 56c318310..0d34220b0 100644 --- a/commands/info.go +++ b/commands/info.go @@ -4,6 +4,8 @@ import ( "encoding/json" "errors" "fmt" + "os" + "github.com/EngineerBetter/control-tower/bosh" "github.com/EngineerBetter/control-tower/certs" "github.com/EngineerBetter/control-tower/commands/info" @@ -14,7 +16,6 @@ import ( "github.com/EngineerBetter/control-tower/terraform" "github.com/EngineerBetter/control-tower/util" "gopkg.in/urfave/cli.v1" - "os" ) var initialInfoArgs info.Args @@ -44,9 +45,8 @@ var infoFlags = []cli.Flag{ }, cli.StringFlag{ Name: "iaas", - Usage: "(optional) IAAS, can be AWS or GCP", + Usage: "(required) IAAS, can be AWS or GCP", EnvVar: "IAAS", - Value: "AWS", Destination: &initialInfoArgs.IAAS, }, cli.StringFlag{ @@ -65,11 +65,6 @@ func infoAction(c *cli.Context, infoArgs info.Args, provider iaas.Provider) erro version := c.App.Version - err := infoArgs.MarkSetFlags(c) - if err != nil { - return err - } - client, err := buildInfoClient(name, version, infoArgs, provider) if err != nil { return err @@ -95,8 +90,19 @@ func infoAction(c *cli.Context, infoArgs info.Args, provider iaas.Provider) erro _, err := fmt.Fprint(os.Stdout, i) return err } - //this will never run - return nil +} + +func validateInfoArgs(c *cli.Context, infoArgs info.Args) (info.Args, error) { + err := infoArgs.MarkSetFlags(c) + if err != nil { + return infoArgs, fmt.Errorf("failed to mark set Info flags: [%v]", err) + } + + if err = infoArgs.Validate(); err != nil { + return infoArgs, fmt.Errorf("failed to validate Info flags: [%v]", err) + } + + return infoArgs, nil } func buildInfoClient(name, version string, infoArgs info.Args, provider iaas.Provider) (*concourse.Client, error) { @@ -139,14 +145,18 @@ var infoCmd = cli.Command{ ArgsUsage: "", Flags: infoFlags, Action: func(c *cli.Context) error { - iaasName, err := iaas.Assosiate(initialInfoArgs.IAAS) + infoArgs, err := validateInfoArgs(c, initialInfoArgs) if err != nil { - return err + return fmt.Errorf("Error validating args on info: [%v]", err) + } + iaasName, err := iaas.Assosiate(infoArgs.IAAS) + if err != nil { + return fmt.Errorf("Error mapping to supported IAASes on info: [%v]", err) } - provider, err := iaas.New(iaasName, initialInfoArgs.Region) + provider, err := iaas.New(iaasName, infoArgs.Region) if err != nil { return fmt.Errorf("Error creating IAAS provider on info: [%v]", err) } - return infoAction(c, initialInfoArgs, provider) + return infoAction(c, infoArgs, provider) }, } diff --git a/commands/info/info_args.go b/commands/info/info_args.go index 46e5b3f60..d3d5e27ec 100644 --- a/commands/info/info_args.go +++ b/commands/info/info_args.go @@ -15,6 +15,7 @@ type Args struct { Namespace string NamespaceIsSet bool IAAS string + IAASIsSet bool CertExpiry bool } @@ -27,7 +28,9 @@ func (a *Args) MarkSetFlags(c FlagSetChecker) error { a.RegionIsSet = true case "namespace": a.NamespaceIsSet = true - case "iaas", "json", "env", "cert-expiry": + case "iaas": + a.IAASIsSet = true + case "json", "env", "cert-expiry": //do nothing default: return fmt.Errorf("flag %q is not supported by info flags", f) @@ -37,6 +40,13 @@ func (a *Args) MarkSetFlags(c FlagSetChecker) error { return nil } +func (a *Args) Validate() error { + if !a.IAASIsSet { + return fmt.Errorf("--iaas flag not set") + } + return nil +} + // FlagSetChecker allows us to find out if flags were set, adn what the names of all flags are type FlagSetChecker interface { IsSet(name string) bool diff --git a/commands/info/info_args_test.go b/commands/info/info_args_test.go new file mode 100644 index 000000000..f5157471d --- /dev/null +++ b/commands/info/info_args_test.go @@ -0,0 +1,87 @@ +package info_test + +import ( + "strings" + "testing" + + . "github.com/EngineerBetter/control-tower/commands/info" +) + +func TestInfoArgs_Validate(t *testing.T) { + defaultFields := Args{ + Region: "eu-west-1", + JSON: false, + Env: false, + IAAS: "AWS", + IAASIsSet: true, + CertExpiry: false, + } + tests := []struct { + name string + modification func() Args + outcomeCheck func(Args) bool + wantErr bool + expectedErr string + }{ + { + name: "Default args", + modification: func() Args { + return defaultFields + }, + wantErr: false, + }, + { + name: "IAAS not set", + modification: func() Args { + args := defaultFields + args.IAASIsSet = false + return args + }, + wantErr: true, + expectedErr: "--iaas flag not set", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + args := tt.modification() + err := args.Validate() + if (err != nil) != tt.wantErr || (err != nil && tt.wantErr && !strings.Contains(err.Error(), tt.expectedErr)) { + if err != nil { + t.Errorf("DeployArgs.Validate() %v test failed.\nFailed with error = %v,\nExpected error = %v,\nShould fail %v\nWith args: %#v", tt.name, err.Error(), tt.expectedErr, tt.wantErr, args) + } else { + t.Errorf("DeployArgs.Validate() %v test failed.\nShould fail %v\nWith args: %#v", tt.name, tt.wantErr, args) + } + } + if tt.outcomeCheck != nil { + if tt.outcomeCheck(args) { + t.Errorf("DeployArgs.Validate() %v test failed.\nShould fail %v\nWith args: %#v", tt.name, tt.wantErr, args) + } + } + }) + } +} + +type FakeFlagSetChecker struct { + names []string + specifiedFlags []string +} + +func NewFakeFlagSetChecker(names, specifiedFlags []string) FakeFlagSetChecker { + return FakeFlagSetChecker{ + names: names, + specifiedFlags: specifiedFlags, + } +} + +func (f *FakeFlagSetChecker) IsSet(desired string) bool { + for _, flag := range f.specifiedFlags { + if desired == flag { + return true + } + } + return false +} + +func (f *FakeFlagSetChecker) FlagNames() (names []string) { + return names +} diff --git a/commands/maintain.go b/commands/maintain.go index eb96719c6..cfcd7dafe 100644 --- a/commands/maintain.go +++ b/commands/maintain.go @@ -3,9 +3,10 @@ package commands import ( "errors" "fmt" - "github.com/EngineerBetter/control-tower/commands/maintain" "os" + "github.com/EngineerBetter/control-tower/commands/maintain" + "github.com/EngineerBetter/control-tower/bosh" "github.com/EngineerBetter/control-tower/certs" "github.com/EngineerBetter/control-tower/concourse" @@ -34,9 +35,8 @@ var maintainFlags = []cli.Flag{ }, cli.StringFlag{ Name: "iaas", - Usage: "(optional) IAAS, can be AWS or GCP", + Usage: "(required) IAAS, can be AWS or GCP", EnvVar: "IAAS", - Value: "AWS", Destination: &initialMaintainArgs.IAAS, }, cli.StringFlag{ @@ -61,11 +61,6 @@ func maintainAction(c *cli.Context, maintainArgs maintain.Args, provider iaas.Pr version := c.App.Version - err := maintainArgs.MarkSetFlags(c) - if err != nil { - return err - } - client, err := buildMaintainClient(name, version, maintainArgs, provider) if err != nil { return err @@ -78,6 +73,19 @@ func maintainAction(c *cli.Context, maintainArgs maintain.Args, provider iaas.Pr return nil } +func validateMaintainArgs(c *cli.Context, maintainArgs maintain.Args) (maintain.Args, error) { + err := maintainArgs.MarkSetFlags(c) + if err != nil { + return maintainArgs, fmt.Errorf("failed to mark set Maintain flags: [%v]", err) + } + + if err = maintainArgs.Validate(); err != nil { + return maintainArgs, fmt.Errorf("failed to validate Maintain flags: [%v]", err) + } + + return maintainArgs, nil +} + func buildMaintainClient(name, version string, maintainArgs maintain.Args, provider iaas.Provider) (*concourse.Client, error) { terraformClient, err := terraform.New(provider.IAAS(), terraform.DownloadTerraform()) if err != nil { @@ -118,15 +126,18 @@ var maintainCmd = cli.Command{ ArgsUsage: "", Flags: maintainFlags, Action: func(c *cli.Context) error { - iaasName, err := iaas.Assosiate(initialMaintainArgs.IAAS) + maintainArgs, err := validateMaintainArgs(c, initialMaintainArgs) + if err != nil { + return fmt.Errorf("Error validating args on maintain: [%v]", err) + } + iaasName, err := iaas.Assosiate(maintainArgs.IAAS) if err != nil { - return err + return fmt.Errorf("Error mapping to supported IAASes on maintain: [%v]", err) } - provider, err := iaas.New(iaasName, initialMaintainArgs.Region) + provider, err := iaas.New(iaasName, maintainArgs.Region) if err != nil { return fmt.Errorf("Error creating IAAS provider on maintain: [%v]", err) } - - return maintainAction(c, initialMaintainArgs, provider) + return maintainAction(c, maintainArgs, provider) }, } diff --git a/commands/maintain/maintain_args.go b/commands/maintain/maintain_args.go index e1f9657f0..af5dbbea3 100644 --- a/commands/maintain/maintain_args.go +++ b/commands/maintain/maintain_args.go @@ -15,6 +15,7 @@ type Args struct { Namespace string NamespaceIsSet bool IAAS string + IAASIsSet bool Stage int StageIsSet bool } @@ -33,7 +34,7 @@ func (a *Args) MarkSetFlags(c FlagSetChecker) error { case "stage": a.StageIsSet = true case "iaas": - //do nothing + a.IAASIsSet = true default: return fmt.Errorf("flag %q is not supported by maintain flags", f) } @@ -42,6 +43,13 @@ func (a *Args) MarkSetFlags(c FlagSetChecker) error { return nil } +func (a *Args) Validate() error { + if !a.IAASIsSet { + return fmt.Errorf("--iaas flag not set") + } + return nil +} + // FlagSetChecker allows us to find out if flags were set, adn what the names of all flags are type FlagSetChecker interface { IsSet(name string) bool diff --git a/commands/maintain/maintain_args_test.go b/commands/maintain/maintain_args_test.go new file mode 100644 index 000000000..2277a9d4c --- /dev/null +++ b/commands/maintain/maintain_args_test.go @@ -0,0 +1,84 @@ +package maintain_test + +import ( + "strings" + "testing" + + . "github.com/EngineerBetter/control-tower/commands/maintain" +) + +func TestMaintainArgs_Validate(t *testing.T) { + defaultFields := Args{ + Region: "eu-west-1", + IAAS: "AWS", + IAASIsSet: true, + } + tests := []struct { + name string + modification func() Args + outcomeCheck func(Args) bool + wantErr bool + expectedErr string + }{ + { + name: "Default args", + modification: func() Args { + return defaultFields + }, + wantErr: false, + }, + { + name: "IAAS not set", + modification: func() Args { + args := defaultFields + args.IAASIsSet = false + return args + }, + wantErr: true, + expectedErr: "--iaas flag not set", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + args := tt.modification() + err := args.Validate() + if (err != nil) != tt.wantErr || (err != nil && tt.wantErr && !strings.Contains(err.Error(), tt.expectedErr)) { + if err != nil { + t.Errorf("DeployArgs.Validate() %v test failed.\nFailed with error = %v,\nExpected error = %v,\nShould fail %v\nWith args: %#v", tt.name, err.Error(), tt.expectedErr, tt.wantErr, args) + } else { + t.Errorf("DeployArgs.Validate() %v test failed.\nShould fail %v\nWith args: %#v", tt.name, tt.wantErr, args) + } + } + if tt.outcomeCheck != nil { + if tt.outcomeCheck(args) { + t.Errorf("DeployArgs.Validate() %v test failed.\nShould fail %v\nWith args: %#v", tt.name, tt.wantErr, args) + } + } + }) + } +} + +type FakeFlagSetChecker struct { + names []string + specifiedFlags []string +} + +func NewFakeFlagSetChecker(names, specifiedFlags []string) FakeFlagSetChecker { + return FakeFlagSetChecker{ + names: names, + specifiedFlags: specifiedFlags, + } +} + +func (f *FakeFlagSetChecker) IsSet(desired string) bool { + for _, flag := range f.specifiedFlags { + if desired == flag { + return true + } + } + return false +} + +func (f *FakeFlagSetChecker) FlagNames() (names []string) { + return names +}