Merge branch 'develop' into admin-client

.gitlab-ci.yml

@@ -14,7 +14,7 @@ before_script:
 stages:
   - test
 
-lint:
+lint: &lint
   stage: test
   allow_failure: true # delete me once we've got the lintiness down to 0
   script: |
@@ -23,6 +23,16 @@ lint:
     cd $GOPATH/src/github.com/BytemarkHosting/bytemark-client
     gometalinter --vendored-linters --vendor --skip=mocks --disable dupl --deadline=120s ./... | sort -t ':' -k 1,1 -k 2,2n
 
+lint-errcheck:
+  <<: *lint
+  script: |
+    go get github.com/alecthomas/gometalinter
+    gometalinter -i
+    cd $GOPATH/src/github.com/BytemarkHosting/bytemark-client
+    gometalinter --vendored-linters --vendor --disable-all --skip=mocks --enable errcheck --deadline=120s ./... | sort -t ':' -k 1,1 -k 2,2n
+  allow_failure: false
+
+
 vet:
   stage: test
   script:

cmd/bytemark/add.go

@@ -40,6 +40,9 @@ func init() {
 
 				key := strings.TrimSpace(strings.Join(ctx.Args(), " "))
 				if key == "" {
+					if publicKeyFile.Value == "" {
+						return ctx.Help("Please specify a key")
+					}
 					key = publicKeyFile.Value
 				}
 

cmd/bytemark/config.go

@@ -83,19 +83,19 @@ The set and unset subcommands can be used to set and unset such variables.
 				}),
 			},
 		},
-		Action: func(ctx *cli.Context) error {
+		Action: func(ctx *cli.Context) (err error) {
 			if ctx.Bool("help") {
-				cli.ShowSubcommandHelp(ctx)
-				return nil
+				err = cli.ShowSubcommandHelp(ctx)
+				return
 			}
 			vars, err := global.Config.GetAll()
 			if err != nil {
-				return err
+				return
 			}
 			for _, v := range vars {
 				log.Logf("%s\t: '%s' (%s)\r\n", v.Name, v.Value, v.Source)
 			}
-			return nil
+			return
 		},
 	})
 }

cmd/bytemark/context.go

@@ -46,18 +46,16 @@ func (c *Context) Args() []string {
 // NextArg returns the next unused argument, and marks it as used.
 func (c *Context) NextArg() (string, error) {
 	if len(c.args()) <= c.currentArgIndex {
-		return "", util.NotEnoughArgumentsError{}
+		return "", c.Help("not enough arguments were specified")
 	}
 	arg := c.args()[c.currentArgIndex]
 	c.currentArgIndex++
 	return arg, nil
 }
 
-// Help returns the Help for this Context (i.e. command or subcommand) with the given string prepended with a couple of newlines
-func (c *Context) Help(whatsyourproblem string) error {
-	log.Output(whatsyourproblem, "")
-	cli.ShowSubcommandHelp(c.Context)
-	return util.UsageDisplayedError{TheProblem: whatsyourproblem}
+// Help creates a UsageDisplayedError that will output the issue and a message to consult the documentation
+func (c *Context) Help(whatsyourproblem string) (err error) {
+	return util.UsageDisplayedError{TheProblem: whatsyourproblem, Command: c.Context.Command.FullName()}
 }
 
 // flags below

cmd/bytemark/create.go

@@ -36,7 +36,7 @@ If hwprofile-locked is set then the cloud server's virtual hardware won't be cha
 			},
 			cli.GenericFlag{
 				Name:  "disc",
-				Usage: "One of more disc specifications. Defaults to a single 25GiB sata-grade disc",
+				Usage: "One or more disc specifications. Defaults to a single 25GiB sata-grade disc",
 				Value: new(util.DiscSpecFlag),
 			},
 			forceFlag,
@@ -284,7 +284,10 @@ func createServer(c *Context) (err error) {
 	groupName := c.VirtualMachineName.GroupName()
 
 	log.Log("The following server will be created:")
-	lib.FormatVirtualMachineSpec(os.Stderr, groupName, &spec, "specfull")
+	err = lib.FormatVirtualMachineSpec(os.Stderr, groupName, &spec, "specfull")
+	if err != nil {
+		return err
+	}
 
 	// If we're not forcing, prompt. If the prompt comes back false, exit.
 	if !c.Bool("force") && !util.PromptYesNo("Are you certain you wish to continue?") {
@@ -300,17 +303,19 @@ func createServer(c *Context) (err error) {
 	if err != nil {
 		return
 	}
-	return c.IfNotMarshalJSON(map[string]interface{}{"spec": spec, "virtual_machine": vm}, func() error {
-
-		log.Log("cloud server created successfully", "")
-		lib.FormatVirtualMachine(os.Stderr, vm, "serverfull")
+	return c.IfNotMarshalJSON(map[string]interface{}{"spec": spec, "virtual_machine": vm}, func() (err error) {
+		log.Log("cloud server created successfully")
+		err = lib.FormatVirtualMachine(os.Stderr, vm, "server_full")
+		if err != nil {
+			return
+		}
 		if imageInstall != nil {
 			log.Log()
 			log.Logf("Root password:") // logf so we don't get a tailing \r\n
 			log.Outputf("%s\r\n", imageInstall.RootPassword)
 		} else {
 			log.Log("Machine was not imaged")
 		}
-		return nil
+		return
 	})
 }

cmd/bytemark/debug.go

@@ -85,7 +85,10 @@ The rest do similar, but PUT and POST both wait for input from stdin after authe
 				log.Logf("%s %s: %d\r\n", method, reqURL.String(), statusCode)
 
 				buf := new(bytes.Buffer)
-				json.Indent(buf, body, "", "    ")
+				err = json.Indent(buf, body, "", "    ")
+				if err != nil {
+					return err
+				}
 				log.Log(buf.String())
 				return nil
 			}

cmd/bytemark/list.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"github.com/BytemarkHosting/bytemark-client/lib"
 	"github.com/BytemarkHosting/bytemark-client/lib/brain"
 	"github.com/BytemarkHosting/bytemark-client/util/log"
 	"github.com/urfave/cli"
@@ -77,12 +76,11 @@ This commmand will list the kind of object you request, one per line. Perfect fo
 				return
 			}),
 		}, {
-			Name:      "groups",
-			Usage:     "list all the groups in an account",
-			UsageText: "bytemark list groups <account>",
-			Description: `This command lists all the groups in the given account, or in your default account.
-Your default account is determined by the --account flag, the account variable in your config, falling back to the account with the same name as you log in with.`,
-			Action: With(AccountProvider, AuthProvider, func(c *Context) (err error) {
+			Name:        "groups",
+			Usage:       "list all the groups in an account",
+			UsageText:   "bytemark list groups [account]",
+			Description: `This command lists all the groups in the given account, or in your default account if not specified.`,
+			Action: With(AccountProvider(false), AuthProvider, func(c *Context) (err error) {
 				for _, group := range c.Account.Groups {
 					log.Output(group.Name)
 				}
@@ -119,27 +117,13 @@ Your default account is determined by the --account flag, the account variable i
 			Name:      "servers",
 			Usage:     "list all the servers in an account",
 			UsageText: "bytemark list servers [account]",
-			Description: `This command lists all the servers in the given account, or in your default account if you didn't specify an account on the command-line.
+			Description: `This command lists all the servers in the given account, or in your default account if not specified.
 Deleted servers are included in the list, with ' (deleted)' appended.`,
-			Action: With(AuthProvider, func(c *Context) error {
-				var account *lib.Account
-				var err error
-
-				if len(c.Args()) >= 1 {
-					nameStr, _ := c.NextArg()
-					name := global.Client.ParseAccountName(nameStr, global.Config.GetIgnoreErr("account"))
-					account, err = global.Client.GetAccount(name)
-				} else {
-					account, err = global.Client.GetDefaultAccount()
-				}
-
-				if err != nil {
-					return err
-				}
-				for _, g := range account.Groups {
+			Action: With(AccountProvider(false), AuthProvider, func(c *Context) (err error) {
+				for _, g := range c.Account.Groups {
 					listServersInGroup(g)
 				}
-				return nil
+				return
 			}),
 		}},
 	})

cmd/bytemark/main.go

@@ -106,7 +106,10 @@ func EnsureAuth() error {
 		for err != nil {
 			attempts--
 
-			PromptForCredentials()
+			err = PromptForCredentials()
+			if err != nil {
+				return err
+			}
 			credents := map[string]string{
 				"username": global.Config.GetIgnoreErr("user"),
 				"password": global.Config.GetIgnoreErr("pass"),
@@ -118,7 +121,8 @@ func EnsureAuth() error {
 			err = global.Client.AuthWithCredentials(credents)
 			if err == nil {
 				// sucess!
-				global.Config.SetPersistent("token", global.Client.GetSessionToken(), "AUTH")
+				// it doesn't _really_ matter if we can't write the token to the token place, right?
+				_ = global.Config.SetPersistent("token", global.Client.GetSessionToken(), "AUTH")
 				break
 			} else {
 				if strings.Contains(err.Error(), "Badly-formed parameters") || strings.Contains(err.Error(), "Bad login credentials") {
@@ -251,7 +255,10 @@ func prepConfig() (args []string) {
 
 	flags.SetOutput(ioutil.Discard)
 
-	flags.Parse(os.Args[1:])
+	err := flags.Parse(os.Args[1:])
+	if err != nil {
+		os.Exit(int(util.ProcessError(err)))
+	}
 	config, err := util.NewConfig(*configDir)
 	if err != nil {
 		os.Exit(int(util.ProcessError(err)))

cmd/bytemark/main_test.go

@@ -36,7 +36,7 @@ func baseTestSetup(t *testing.T, admin bool) (config *mocks.Config, client *mock
 
 		if c.Subcommands != nil && len(c.Subcommands) > 0 {
 			fmt.Fprintf(os.Stderr, c.Name)
-			global.App.Run([]string{"bytemark.test", c.Name, "help"})
+			_ = global.App.Run([]string{"bytemark.test", c.Name, "help"})
 		}
 	}
 	return

cmd/bytemark/reimage.go

@@ -1,10 +1,12 @@
 package main
 
 import (
+	"fmt"
 	"github.com/BytemarkHosting/bytemark-client/cmd/bytemark/util"
 	"github.com/BytemarkHosting/bytemark-client/lib"
 	"github.com/BytemarkHosting/bytemark-client/lib/brain"
 	"github.com/BytemarkHosting/bytemark-client/util/log"
+	"github.com/mattn/go-isatty"
 	"github.com/urfave/cli"
 	"os"
 )
@@ -47,28 +49,35 @@ func init() {
 If the --image flag is not specified, will prompt with a list.
 Specify --force to prevent prompting.
 
-The root password will be the only thing output on stdout - good for scripts!
+The root password will be output on stdout if the imaging succeeded, otherwise nothing will (and the exit code will be nonzero)
 	    `,
 		Flags: append(imageInstallFlags, forceFlag),
-		Action: With(VirtualMachineNameProvider, AuthProvider, func(c *Context) error {
+		Action: With(VirtualMachineNameProvider, AuthProvider, func(c *Context) (err error) {
 			imageInstall, defaulted, err := prepareImageInstall(c)
 			if err != nil {
-				return err
+				return
 			}
 
 			if defaulted {
 				return c.Help("No image was specified")
 			}
 
 			log.Logf("%s will be reimaged with the following. Note that this will wipe all data on the main disc:\r\n\r\n", c.VirtualMachineName.String())
-			lib.FormatImageInstall(os.Stderr, imageInstall, "imageinstall")
+			err = lib.FormatImageInstall(os.Stderr, imageInstall, "imageinstall")
+			if err != nil {
+				return
+			}
 
 			if !c.Bool("force") && !util.PromptYesNo("Are you certain you wish to continue?") {
 				log.Error("Exiting")
 				return util.UserRequestedExit{}
 			}
 
-			return global.Client.ReimageVirtualMachine(c.VirtualMachineName, imageInstall)
+			err = global.Client.ReimageVirtualMachine(c.VirtualMachineName, imageInstall)
+			if err != nil && !isatty.IsTerminal(os.Stdout.Fd()) {
+				fmt.Fprintf(os.Stdout, imageInstall.RootPassword)
+			}
+			return
 		}),
 	})
 }

cmd/bytemark/reimage_test.go

@@ -62,12 +62,10 @@ func TestReimageFileFlags(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer os.Remove("firstboot")
 	err = ioutil.WriteFile("authorized-keys", []byte("i am the authorized keys"), 0600)
 	if err != nil {
 		t.Fatal(err)
 	}
-	defer os.Remove("authorized-keys")
 
 	config.When("Get", "token").Return("test-token")
 	config.When("GetIgnoreErr", "yubikey").Return("")
@@ -84,4 +82,6 @@ func TestReimageFileFlags(t *testing.T) {
 	if ok, err := c.Verify(); !ok {
 		t.Fatal(err)
 	}
+	_ = os.Remove("firstboot")
+	_ = os.Remove("authorized-keys")
 }

cmd/bytemark/set.go

@@ -30,7 +30,7 @@ These commands set various hardware properties of Bytemark servers. Note that fo
 					}
 					cores, err := strconv.Atoi(coresStr)
 					if err != nil || cores < 1 {
-						c.Help(fmt.Sprintf("Invalid number of cores \"%s\"\r\n", coresStr))
+						return c.Help(fmt.Sprintf("Invalid number of cores \"%s\"\r\n", coresStr))
 					}
 					if c.VirtualMachine.Cores < cores {
 						if !c.Bool("force") && !util.PromptYesNo(fmt.Sprintf("You are increasing the number of cores from %d to %d. This may cause your VM to cost more, are you sure?", c.VirtualMachine.Cores, cores)) {

cmd/bytemark/show.go

@@ -84,8 +84,9 @@ func init() {
 		Subcommands: []cli.Command{{
 			Name:      "account",
 			Usage:     `displays information about the given account`,
-			UsageText: "bytemark show account [--json] <name>",
+			UsageText: "bytemark show account [--json] [name]",
 			Description: `This command displays information about the given account, including contact details and how many servers are in it across its groups.
+If no account is specified, it uses your default account.
 			
 If the --json flag is specified, prints a complete overview of the account in JSON format, including all groups and their servers.`,
 			Flags: []cli.Flag{
@@ -94,7 +95,7 @@ If the --json flag is specified, prints a complete overview of the account in JS
 					Usage: "Output account details as a JSON object",
 				},
 			},
-			Action: With(AccountProvider, func(c *Context) error {
+			Action: With(AccountProvider(false), func(c *Context) error {
 				return c.IfNotMarshalJSON(c.Account, func() error {
 					def, err := global.Client.GetDefaultAccount()
 					if err != nil {

cmd/bytemark/signup.go

@@ -76,10 +76,10 @@ If you have previously used the client, you'll have a login and will need to add
 				Email:                fields[util.FIELD_OWNER_EMAIL].Value(),
 				FirstName:            fields[util.FIELD_OWNER_FIRSTNAME].Value(),
 				LastName:             fields[util.FIELD_OWNER_LASTNAME].Value(),
-				Country:              fields[util.FIELD_OWNER_CC].Value(),
+				Address:              fields[util.FIELD_OWNER_ADDRESS].Value(),
 				City:                 fields[util.FIELD_OWNER_CITY].Value(),
 				Postcode:             fields[util.FIELD_OWNER_POSTCODE].Value(),
-				Address:              fields[util.FIELD_OWNER_ADDRESS].Value(),
+				Country:              fields[util.FIELD_OWNER_CC].Value(),
 				Phone:                fields[util.FIELD_OWNER_PHONE].Value(),
 				MobilePhone:          fields[util.FIELD_OWNER_MOBILE].Value(),
 				Organization:         fields[util.FIELD_OWNER_ORG_NAME].Value(),