Make IAAS flag required for all commands

Also improved test coverage in commands package

Signed-off-by: Irbe Krumina <[email protected]>

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
 

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,17 +140,28 @@ 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))
 				Expect(session.Out).To(Say("control-tower destroy - Destroys a Concourse"))
 			})
 		})
 
+		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,14 +181,58 @@ 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))
 				Expect(session.Err).To(Say("Usage is `control-tower info <name>`"))
 			})
 		})
 	})
+
+	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 <name>`"))
+			})
+		})
+	})
 })

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: "<name>",
 	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 {

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
 	}

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 {

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: "<name>",
 	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)
 	},
 }

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