[ARVADOS] created: 2.1.0-737-g727c8c37b

[Git user]

        at  727c8c37baa64fe63bec04aacea870ea47a7daf0 (commit)


commit 727c8c37baa64fe63bec04aacea870ea47a7daf0
Author: Tom Clegg <tom at curii.com>
Date:   Mon Apr 26 15:11:58 2021 -0400

    16997: Test map key order in check/dump. Fix duplicate warnings.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/config/cmd.go b/lib/config/cmd.go
index 347e8519a..8e638e6ec 100644
--- a/lib/config/cmd.go
+++ b/lib/config/cmd.go
@@ -12,7 +12,6 @@ import (
 	"os"
 	"os/exec"
 
-	"git.arvados.org/arvados.git/sdk/go/arvados"
 	"git.arvados.org/arvados.git/sdk/go/ctxlog"
 	"github.com/ghodss/yaml"
 	"github.com/sirupsen/logrus"
@@ -120,16 +119,15 @@ func (checkCommand) RunCommand(prog string, args []string, stdin io.Reader, stdo
 	if err != nil {
 		return 1
 	}
+	// Reset() to avoid printing the same warnings twice when they
+	// are logged by both without-legacy and with-legacy loads.
+	logbuf.Reset()
 	loader.SkipDeprecated = false
 	loader.SkipLegacy = false
 	withDepr, err := loader.Load()
 	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)
@@ -165,28 +163,9 @@ func (checkCommand) RunCommand(prog string, args []string, stdin io.Reader, stdo
 			return 1
 		}
 	}
-
-	if problems {
-		return 1
-	}
 	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
 
 type defaultsCommand struct{}
diff --git a/lib/config/cmd_test.go b/lib/config/cmd_test.go
index bb8d7dca1..d4d2f3d63 100644
--- a/lib/config/cmd_test.go
+++ b/lib/config/cmd_test.go
@@ -134,6 +134,18 @@ Clusters:
 	c.Check(stderr.String(), check.Matches, `(?ms).*unexpected object in config entry: Clusters.z1234.PostgreSQL.ConnectionPool"\n.*`)
 }
 
+func (s *CommandSuite) TestCheck_DuplicateWarnings(c *check.C) {
+	var stdout, stderr bytes.Buffer
+	in := `
+Clusters:
+ z1234: {}
+`
+	code := CheckCommand.RunCommand("arvados config-check", []string{"-config", "-"}, bytes.NewBufferString(in), &stdout, &stderr)
+	c.Check(code, check.Equals, 1)
+	c.Check(stderr.String(), check.Matches, `(?ms).*SystemRootToken.*`)
+	c.Check(stderr.String(), check.Not(check.Matches), `(?ms).*SystemRootToken.*SystemRootToken.*`)
+}
+
 func (s *CommandSuite) TestDump_Formatting(c *check.C) {
 	var stdout, stderr bytes.Buffer
 	in := `
@@ -168,3 +180,56 @@ Clusters:
 	c.Check(stdout.String(), check.Matches, `(?ms).*\n *ManagementToken: secret\n.*`)
 	c.Check(stdout.String(), check.Not(check.Matches), `(?ms).*UnknownKey.*`)
 }
+
+func (s *CommandSuite) TestDump_KeyOrder(c *check.C) {
+	in := `
+Clusters:
+ z1234:
+  Login:
+   Test:
+    Users:
+     a: {}
+     d: {}
+     c: {}
+     b: {}
+     e: {}
+`
+	for trial := 0; trial < 20; trial++ {
+		var stdout, stderr bytes.Buffer
+		code := DumpCommand.RunCommand("arvados config-dump", []string{"-config", "-"}, bytes.NewBufferString(in), &stdout, &stderr)
+		c.Assert(code, check.Equals, 0)
+		if !c.Check(stdout.String(), check.Matches, `(?ms).*a:.*b:.*c:.*d:.*e:.*`) {
+			c.Logf("config-dump did not use lexical key order on trial %d", trial)
+			c.Log("stdout:\n", stdout.String())
+			c.Log("stderr:\n", stderr.String())
+			c.FailNow()
+		}
+	}
+}
+
+func (s *CommandSuite) TestCheck_KeyOrder(c *check.C) {
+	in := `
+Clusters:
+ z1234:
+  ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+  SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+  Collections:
+   BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+  InstanceTypes:
+   a: {}
+   d: {}
+   c: {}
+   b: {}
+   e: {}
+`
+	for trial := 0; trial < 20; trial++ {
+		var stdout, stderr bytes.Buffer
+		code := CheckCommand.RunCommand("arvados config-check", []string{"-config=-", "-strict=true"}, bytes.NewBufferString(in), &stdout, &stderr)
+		if !c.Check(code, check.Equals, 0) || stdout.String() != "" || stderr.String() != "" {
+			c.Logf("config-check returned error or non-empty output on trial %d", trial)
+			c.Log("stdout:\n", stdout.String())
+			c.Log("stderr:\n", stderr.String())
+			c.FailNow()
+		}
+	}
+}
diff --git a/lib/config/load.go b/lib/config/load.go
index b42e1a3c1..292e3f6d6 100644
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -313,11 +313,15 @@ var acceptableTokenLength = 32
 
 func (ldr *Loader) checkToken(label, token string) error {
 	if token == "" {
-		ldr.Logger.Warnf("%s: secret token is not set (use %d+ random characters from a-z, A-Z, 0-9)", label, acceptableTokenLength)
+		if ldr.Logger != nil {
+			ldr.Logger.Warnf("%s: secret token is not set (use %d+ random characters from a-z, A-Z, 0-9)", label, acceptableTokenLength)
+		}
 	} else if !acceptableTokenRe.MatchString(token) {
 		return fmt.Errorf("%s: unacceptable characters in token (only a-z, A-Z, 0-9 are acceptable)", label)
 	} else if len(token) < acceptableTokenLength {
-		ldr.Logger.Warnf("%s: token is too short (should be at least %d characters)", label, acceptableTokenLength)
+		if ldr.Logger != nil {
+			ldr.Logger.Warnf("%s: token is too short (should be at least %d characters)", label, acceptableTokenLength)
+		}
 	}
 	return nil
 }

-----------------------------------------------------------------------


hooks/post-receive
--