15720: Warn about empty ManagementToken or SystemRootToken.

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tclegg@veritasgenetics.com>

diff --git a/doc/admin/management-token.html.textile.liquid b/doc/admin/management-token.html.textile.liquid
index 5380f38f9c40711722744e1281d6ee6dfff6d858..cf3e273ceba13f9b54c5be7f1bfc5aa5c1a921c7 100644 (file)
--- a/doc/admin/management-token.html.textile.liquid
+++ b/doc/admin/management-token.html.textile.liquid
@@ -16,17 +16,6 @@ Services must have ManagementToken configured.  This is used to authorize access
 
 To access a monitoring endpoint, the requester must provide the HTTP header @Authorization: Bearer (ManagementToken)@.
 
-h2. API server
-
-Set @ManagementToken@ in the appropriate section of @application.yml@
-
-<pre>
-production:
-  # Token to be included in all healthcheck requests. Disabled by default.
-  # Server expects request header of the format "Authorization: Bearer xxx"
-  ManagementToken: xxx
-</pre>
-
 h2. Node Manager
 
 Set @port@ (the listen port) and @ManagementToken@ in the @Manage@ section of @node-manager.ini@.
@@ -45,12 +34,26 @@ Set @port@ (the listen port) and @ManagementToken@ in the @Manage@ section of @n
 ManagementToken = xxx
 </pre>
 
-h2. Other services
+h2. API server and other services
 
-The following services also support monitoring.  Set @ManagementToken@ in the respective yaml config file for each service.
+The following services also support monitoring.
 
+* API server
+* arv-git-httpd
+* controller
+* keep-balance
+* keepproxy
 * keepstore
 * keep-web
-* keepproxy
-* arv-git-httpd
 * websockets
+
+Set @ManagementToken@ in the appropriate section of @/etc/arvados/config.yml@.
+
+<notextile>
+<pre><code>Clusters:
+  <span class="userinput">uuid_prefix</span>:
+    # Token to be included in all healthcheck requests. Disabled by default.
+    # Server expects request header of the format "Authorization: Bearer xxx"
+    ManagementToken: xxx
+</code></pre>
+</notextile>

diff --git a/lib/config/cmd.go b/lib/config/cmd.go
index e9ceaca8642af7dbb7e993fa0b52de6e81566d63..1ca278391a829f63963930538416f496e2497a1b 100644 (file)
--- a/lib/config/cmd.go
+++ b/lib/config/cmd.go
@@ -12,6 +12,7 @@ import (
        "os"
        "os/exec"
 
+       "git.curoverse.com/arvados.git/sdk/go/arvados"
        "git.curoverse.com/arvados.git/sdk/go/ctxlog"
        "github.com/ghodss/yaml"
        "github.com/sirupsen/logrus"
@@ -124,6 +125,10 @@ func (checkCommand) RunCommand(prog string, args []string, stdin io.Reader, stdo
        if err != nil {
                return 1
        }
+       problems := false
+       if warnAboutProblems(logger, withDepr) {
+               problems = true
+       }
        cmd := exec.Command("diff", "-u", "--label", "without-deprecated-configs", "--label", "relying-on-deprecated-configs", "/dev/fd/3", "/dev/fd/4")
        for _, obj := range []interface{}{withoutDepr, withDepr} {
                y, _ := yaml.Marshal(obj)
@@ -153,7 +158,27 @@ func (checkCommand) RunCommand(prog string, args []string, stdin io.Reader, stdo
        if logbuf.Len() > 0 {
                return 1
        }
-       return 0
+
+       if problems {
+               return 1
+       } else {
+               return 0
+       }
+}
+
+func warnAboutProblems(logger logrus.FieldLogger, cfg *arvados.Config) bool {
+       warned := false
+       for id, cc := range cfg.Clusters {
+               if cc.SystemRootToken == "" {
+                       logger.Warnf("Clusters.%s.SystemRootToken is empty; see https://doc.arvados.org/master/install/install-keepstore.html", id)
+                       warned = true
+               }
+               if cc.ManagementToken == "" {
+                       logger.Warnf("Clusters.%s.ManagementToken is empty; see https://doc.arvados.org/admin/management-token.html", id)
+                       warned = true
+               }
+       }
+       return warned
 }
 
 var DumpDefaultsCommand defaultsCommand

diff --git a/lib/config/cmd_test.go b/lib/config/cmd_test.go
index fb1cba38b4857d4b253933158f3f8a64a002cb48..c275e4c35b37903e9d1f20e6e5c232e026f0d4ee 100644 (file)
--- a/lib/config/cmd_test.go
+++ b/lib/config/cmd_test.go
@@ -30,25 +30,27 @@ func (s *CommandSuite) SetUpSuite(c *check.C) {
        os.Unsetenv("ARVADOS_API_TOKEN")
 }
 
-func (s *CommandSuite) TestBadArg(c *check.C) {
+func (s *CommandSuite) TestDump_BadArg(c *check.C) {
        var stderr bytes.Buffer
        code := DumpCommand.RunCommand("arvados config-dump", []string{"-badarg"}, bytes.NewBuffer(nil), bytes.NewBuffer(nil), &stderr)
        c.Check(code, check.Equals, 2)
        c.Check(stderr.String(), check.Matches, `(?ms)flag provided but not defined: -badarg\nUsage:\n.*`)
 }
 
-func (s *CommandSuite) TestEmptyInput(c *check.C) {
+func (s *CommandSuite) TestDump_EmptyInput(c *check.C) {
        var stdout, stderr bytes.Buffer
        code := DumpCommand.RunCommand("arvados config-dump", []string{"-config", "-"}, &bytes.Buffer{}, &stdout, &stderr)
        c.Check(code, check.Equals, 1)
        c.Check(stderr.String(), check.Matches, `config does not define any clusters\n`)
 }
 
-func (s *CommandSuite) TestCheckNoDeprecatedKeys(c *check.C) {
+func (s *CommandSuite) TestCheck_NoWarnings(c *check.C) {
        var stdout, stderr bytes.Buffer
        in := `
 Clusters:
  z1234:
+  ManagementToken: xyzzy
+  SystemRootToken: xyzzy
   API:
     MaxItemsPerResponse: 1234
   PostgreSQL:
@@ -73,7 +75,7 @@ Clusters:
        c.Check(stderr.String(), check.Equals, "")
 }
 
-func (s *CommandSuite) TestCheckDeprecatedKeys(c *check.C) {
+func (s *CommandSuite) TestCheck_DeprecatedKeys(c *check.C) {
        var stdout, stderr bytes.Buffer
        in := `
 Clusters:
@@ -86,7 +88,7 @@ Clusters:
        c.Check(stdout.String(), check.Matches, `(?ms).*\n\- +.*MaxItemsPerResponse: 1000\n\+ +MaxItemsPerResponse: 1234\n.*`)
 }
 
-func (s *CommandSuite) TestCheckOldKeepstoreConfigFile(c *check.C) {
+func (s *CommandSuite) TestCheck_OldKeepstoreConfigFile(c *check.C) {
        f, err := ioutil.TempFile("", "")
        c.Assert(err, check.IsNil)
        defer os.Remove(f.Name())
@@ -106,7 +108,7 @@ Clusters:
        c.Check(stderr.String(), check.Matches, `(?ms).*you should remove the legacy keepstore config file.*\n`)
 }
 
-func (s *CommandSuite) TestCheckUnknownKey(c *check.C) {
+func (s *CommandSuite) TestCheck_UnknownKey(c *check.C) {
        var stdout, stderr bytes.Buffer
        in := `
 Clusters:
@@ -130,7 +132,7 @@ Clusters:
        c.Check(stderr.String(), check.Matches, `(?ms).*unexpected object in config entry: Clusters.z1234.PostgreSQL.ConnectionPool"\n.*`)
 }
 
-func (s *CommandSuite) TestDumpFormatting(c *check.C) {
+func (s *CommandSuite) TestDump_Formatting(c *check.C) {
        var stdout, stderr bytes.Buffer
        in := `
 Clusters:
@@ -149,7 +151,7 @@ Clusters:
        c.Check(stdout.String(), check.Matches, `(?ms).*http://localhost:12345: {}\n.*`)
 }
 
-func (s *CommandSuite) TestDumpUnknownKey(c *check.C) {
+func (s *CommandSuite) TestDump_UnknownKey(c *check.C) {
        var stdout, stderr bytes.Buffer
        in := `
 Clusters: