From 49a6a6120f81a1c1045c79a3de7a44e849057c65 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 27 Mar 2019 19:42:09 +0000 Subject: [PATCH] cmd/gomote: add -firewall flag to run, defaulting to off While debugging golang/go#25386 I found that the firewall defaulting to on made debugging modules hard. So make gomote default to no outbound firewall and make it opt-in for people who want to reproduce the builder more. Also in this CL: start to use server's builder config, not local state (follow up to CL 169678 which was incomplete). It's still incomplete in this CL, but there's now a panic with more useful information to users telling them to update their binary. Updates golang/go#30929 Change-Id: I17bded71919af1e7a9181866a1349eb72da40051 Reviewed-on: https://go-review.googlesource.com/c/build/+/170397 Reviewed-by: Dmitri Shuralyov --- cmd/gomote/destroy.go | 2 +- cmd/gomote/get.go | 2 +- cmd/gomote/list.go | 44 +++++++++++++++++++++++++++++++------------ cmd/gomote/ls.go | 2 +- cmd/gomote/ping.go | 2 +- cmd/gomote/put.go | 4 ++-- cmd/gomote/rm.go | 2 +- cmd/gomote/run.go | 3 +++ cmd/gomote/ssh.go | 2 +- 9 files changed, 43 insertions(+), 20 deletions(-) diff --git a/cmd/gomote/destroy.go b/cmd/gomote/destroy.go index aa3d7f033a..ab61534dce 100644 --- a/cmd/gomote/destroy.go +++ b/cmd/gomote/destroy.go @@ -23,7 +23,7 @@ func destroy(args []string) error { fs.Usage() } name := fs.Arg(0) - bc, _, err := clientAndConf(name) + bc, err := remoteClient(name) if err != nil { return err } diff --git a/cmd/gomote/get.go b/cmd/gomote/get.go index 12a0540bee..65b633dd02 100644 --- a/cmd/gomote/get.go +++ b/cmd/gomote/get.go @@ -29,7 +29,7 @@ func getTar(args []string) error { } name := fs.Arg(0) - bc, _, err := clientAndConf(name) + bc, err := remoteClient(name) if err != nil { return err } diff --git a/cmd/gomote/list.go b/cmd/gomote/list.go index bf3e6494cc..62a5bf2b79 100644 --- a/cmd/gomote/list.go +++ b/cmd/gomote/list.go @@ -43,7 +43,18 @@ func list(args []string) error { return nil } -// clientAndConfig returns a buildlet.Client and its build config for +// remoteClient returns a buildlet.Client for a named remote buildlet +// (a buildlet connection owned by the build coordinator). +// +// As a special case, if name contains '@', the name is expected to be +// of the form @ip[:port]. For example, +// "windows-amd64-race@10.0.0.1". +func remoteClient(name string) (*buildlet.Client, error) { + bc, _, err := clientAndCondConf(name, false) + return bc, err +} + +// clientAndConf returns a buildlet.Client and its build config for // a named remote buildlet (a buildlet connection owned by the build // coordinator). // @@ -51,8 +62,10 @@ func list(args []string) error { // of the form @ip[:port]. For example, // "windows-amd64-race@10.0.0.1". func clientAndConf(name string) (bc *buildlet.Client, conf *dashboard.BuildConfig, err error) { - var ok bool + return clientAndCondConf(name, true) +} +func clientAndCondConf(name string, withConf bool) (bc *buildlet.Client, conf *dashboard.BuildConfig, err error) { if strings.Contains(name, "@") { f := strings.SplitN(name, "@", 2) if len(f) != 2 { @@ -60,10 +73,13 @@ func clientAndConf(name string) (bc *buildlet.Client, conf *dashboard.BuildConfi return } builderType := f[0] - conf, ok = dashboard.Builders[builderType] - if !ok { - err = fmt.Errorf("unknown builder type %q (name %q)", builderType, name) - return + if withConf { + var ok bool + conf, ok = dashboard.Builders[builderType] + if !ok { + err = fmt.Errorf("unknown builder type %q (name %q)", builderType, name) + return + } } ipPort := f[1] if !strings.Contains(ipPort, ":") { @@ -82,14 +98,12 @@ func clientAndConf(name string) (bc *buildlet.Client, conf *dashboard.BuildConfi if err != nil { return } + var builderType string + var ok bool for _, rb := range rbs { if rb.Name == name { - conf, ok = dashboard.Builders[rb.BuilderType] - if !ok { - err = fmt.Errorf("builder %q exists, but unknown builder type %q", name, rb.BuilderType) - return - } - break + ok = true + builderType = rb.BuilderType } } if !ok { @@ -101,5 +115,11 @@ func clientAndConf(name string) (bc *buildlet.Client, conf *dashboard.BuildConfi if err != nil { return } + + conf, ok = dashboard.Builders[builderType] + if !ok { + log.Fatalf("Builder type %q not known to this gomote binary. Update your gomote binary. TODO: teach gomote to fetch build configs from the server (Issue 30929)", builderType) + } + return bc, conf, nil } diff --git a/cmd/gomote/ls.go b/cmd/gomote/ls.go index de76ffd1e7..7305fb7ca0 100644 --- a/cmd/gomote/ls.go +++ b/cmd/gomote/ls.go @@ -35,7 +35,7 @@ func ls(args []string) error { dir = fs.Arg(1) } name := fs.Arg(0) - bc, _, err := clientAndConf(name) + bc, err := remoteClient(name) if err != nil { return err } diff --git a/cmd/gomote/ping.go b/cmd/gomote/ping.go index 7789945d7f..c7ea4e8b4c 100644 --- a/cmd/gomote/ping.go +++ b/cmd/gomote/ping.go @@ -23,7 +23,7 @@ func ping(args []string) error { fs.Usage() } name := fs.Arg(0) - bc, _, err := clientAndConf(name) + bc, err := remoteClient(name) if err != nil { return err } diff --git a/cmd/gomote/put.go b/cmd/gomote/put.go index a9d93af8d1..89880ffdc7 100644 --- a/cmd/gomote/put.go +++ b/cmd/gomote/put.go @@ -46,7 +46,7 @@ func putTar(args []string) error { } name := fs.Arg(0) - bc, _, err := clientAndConf(name) + bc, err := remoteClient(name) if err != nil { return err } @@ -125,7 +125,7 @@ func put(args []string) error { fs.Usage() } - bc, _, err := clientAndConf(fs.Arg(0)) + bc, err := remoteClient(fs.Arg(0)) if err != nil { return err } diff --git a/cmd/gomote/rm.go b/cmd/gomote/rm.go index d8b8ee0a8b..baa0bb70b1 100644 --- a/cmd/gomote/rm.go +++ b/cmd/gomote/rm.go @@ -25,7 +25,7 @@ func rm(args []string) error { } name := fs.Arg(0) args = fs.Args()[1:] - bc, _, err := clientAndConf(name) + bc, err := remoteClient(name) if err != nil { return err } diff --git a/cmd/gomote/run.go b/cmd/gomote/run.go index e0dc2c3edc..f89b255897 100644 --- a/cmd/gomote/run.go +++ b/cmd/gomote/run.go @@ -28,6 +28,8 @@ func run(args []string) error { fs.BoolVar(&debug, "debug", false, "write debug info about the command's execution before it begins") var env stringSlice fs.Var(&env, "e", "Environment variable KEY=value. The -e flag may be repeated multiple times to add multiple things to the environment.") + var firewall bool + fs.BoolVar(&firewall, "firewall", false, "Enable outbound firewall on machine. This is on by default on many builders (where supported) but disabled by default on gomote for ease of debugging. Once any command has been run with the -firewall flag on, it's on for the lifetime of that gomote instance.") var path string fs.StringVar(&path, "path", "", "Comma-separated list of ExecOpts.Path elements. The special string 'EMPTY' means to run without any $PATH. The empty string (default) does not modify the $PATH. Otherwise, the following expansions apply: the string '$PATH' expands to the current PATH element(s), the substring '$WORKDIR' expands to the buildlet's temp workdir.") @@ -67,6 +69,7 @@ func run(args []string) error { } else if path != "" { pathOpt = strings.Split(path, ",") } + env = append(env, "GO_DISABLE_OUTBOUND_NETWORK="+fmt.Sprint(firewall)) remoteErr, execErr := bc.Exec(cmd, buildlet.ExecOpts{ Dir: dir, diff --git a/cmd/gomote/ssh.go b/cmd/gomote/ssh.go index 9a918fcdba..87a473ed6d 100644 --- a/cmd/gomote/ssh.go +++ b/cmd/gomote/ssh.go @@ -30,7 +30,7 @@ func ssh(args []string) error { fs.Usage() } name := fs.Arg(0) - _, _, err := clientAndConf(name) + _, err := remoteClient(name) if err != nil { return err }