[ARVADOS] updated: 2.1.0-89-g11b7e13a3

[Git user]

Summary of changes:
 lib/config/export_test.go |  2 +-
 lib/config/load.go        | 40 ++++++++++++++++++++++++++++++++++++++++
 lib/config/load_test.go   | 39 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 80 insertions(+), 1 deletion(-)

       via  11b7e13a3effb4c7d0b355d1680c3e26865e3abb (commit)
       via  133a323234fbf8bc15bf165b7afd47e2125f5857 (commit)
      from  cf605d6d16573079ab306fc47679a1bd089a1418 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 11b7e13a3effb4c7d0b355d1680c3e26865e3abb
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Fri Feb 19 12:48:32 2021 -0300

    17295: Adds cluster ID validation on the config file.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/lib/config/export_test.go b/lib/config/export_test.go
index 7af117e38..f11b65f45 100644
--- a/lib/config/export_test.go
+++ b/lib/config/export_test.go
@@ -17,7 +17,7 @@ var _ = check.Suite(&ExportSuite{})
 type ExportSuite struct{}
 
 func (s *ExportSuite) TestExport(c *check.C) {
-	confdata := strings.Replace(string(DefaultYAML), "SAMPLE", "testkey", -1)
+	confdata := strings.Replace(string(DefaultYAML), "SAMPLE", "12345", -1)
 	cfg, err := testLoader(c, confdata, nil).Load()
 	c.Assert(err, check.IsNil)
 	cluster, err := cfg.GetCluster("xxxxx")
diff --git a/lib/config/load.go b/lib/config/load.go
index be6181bbe..f68235937 100644
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -13,6 +13,7 @@ import (
 	"io"
 	"io/ioutil"
 	"os"
+	"regexp"
 	"strings"
 
 	"git.arvados.org/arvados.git/sdk/go/arvados"
@@ -269,7 +270,21 @@ func (ldr *Loader) Load() (*arvados.Config, error) {
 
 	// Check for known mistakes
 	for id, cc := range cfg.Clusters {
+		for remote, _ := range cc.RemoteClusters {
+			if remote == "*" || remote == "SAMPLE" {
+				continue
+			}
+			err = ldr.checkClusterID(fmt.Sprintf("Clusters.%s.RemoteClusters.%s", id, remote), remote, true)
+			if err != nil {
+				return nil, err
+			}
+		}
 		for _, err = range []error{
+			ldr.checkClusterID(fmt.Sprintf("Clusters.%s", id), id, false),
+			ldr.checkClusterID(fmt.Sprintf("Clusters.%s.Login.LoginCluster", id), cc.Login.LoginCluster, true),
+			ldr.checkToken(fmt.Sprintf("Clusters.%s.ManagementToken", id), cc.ManagementToken),
+			ldr.checkToken(fmt.Sprintf("Clusters.%s.SystemRootToken", id), cc.SystemRootToken),
+			ldr.checkToken(fmt.Sprintf("Clusters.%s.Collections.BlobSigningKey", id), cc.Collections.BlobSigningKey),
 			checkKeyConflict(fmt.Sprintf("Clusters.%s.PostgreSQL.Connection", id), cc.PostgreSQL.Connection),
 			ldr.checkEmptyKeepstores(cc),
 			ldr.checkUnlistedKeepstores(cc),
@@ -282,6 +297,31 @@ func (ldr *Loader) Load() (*arvados.Config, error) {
 	return &cfg, nil
 }
 
+var acceptableClusterIDRe = regexp.MustCompile(`^[a-z0-9]{5}$`)
+
+func (ldr *Loader) checkClusterID(label, clusterID string, emptyStringOk bool) error {
+	if emptyStringOk && clusterID == "" {
+		return nil
+	} else if !acceptableClusterIDRe.MatchString(clusterID) {
+		return fmt.Errorf("%s: cluster ID should be 5 alphanumeric characters", label)
+	}
+	return nil
+}
+
+var acceptableTokenRe = regexp.MustCompile(`^[a-zA-Z0-9]+$`)
+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)
+	} 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)
+	}
+	return nil
+}
+
 func checkKeyConflict(label string, m map[string]string) error {
 	saw := map[string]bool{}
 	for k := range m {
diff --git a/lib/config/load_test.go b/lib/config/load_test.go
index 303f1d959..0fe40a69e 100644
--- a/lib/config/load_test.go
+++ b/lib/config/load_test.go
@@ -312,7 +312,7 @@ Clusters:
     Proxy: true
 `, `
 Clusters:
- abcdef:
+ abcde:
   ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
   SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
   Collections:

commit 133a323234fbf8bc15bf165b7afd47e2125f5857
Author: Lucas Di Pentima <lucas at di-pentima.com.ar>
Date:   Fri Feb 19 12:45:11 2021 -0300

    17295: Adds tests exposing the bug.
    
    Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas at di-pentima.com.ar>

diff --git a/lib/config/load_test.go b/lib/config/load_test.go
index 58ddf950e..303f1d959 100644
--- a/lib/config/load_test.go
+++ b/lib/config/load_test.go
@@ -291,6 +291,45 @@ Clusters:
 	c.Check(err, check.ErrorMatches, `Clusters.zzzzz.PostgreSQL.Connection: multiple entries for "(dbname|host)".*`)
 }
 
+func (s *LoadSuite) TestBadClusterIDs(c *check.C) {
+	for _, data := range []string{`
+Clusters:
+ 123456:
+  ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+  SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+  Collections:
+   BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+`, `
+Clusters:
+ 12345:
+  ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+  SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+  Collections:
+   BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+  RemoteClusters:
+   Zzzzz:
+    Host: Zzzzz.arvadosapi.com
+    Proxy: true
+`, `
+Clusters:
+ abcdef:
+  ManagementToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+  SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+  Collections:
+   BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
+  Login:
+   LoginCluster: zz-zz
+`,
+	} {
+		c.Log(data)
+		v, err := testLoader(c, data, nil).Load()
+		if v != nil {
+			c.Logf("%#v", v.Clusters)
+		}
+		c.Check(err, check.ErrorMatches, `.*cluster ID should be 5 alphanumeric characters.*`)
+	}
+}
+
 func (s *LoadSuite) TestBadType(c *check.C) {
 	for _, data := range []string{`
 Clusters:

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


hooks/post-receive
--