Merge branch '17295-configured-cluster-ids-validation'

Closes #17295

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas@di-pentima.com.ar>

diff --git a/lib/config/export_test.go b/lib/config/export_test.go
index 7af117e385f405dbe5f0e8acb7d597f55ab77fe4..f11b65f452b797a0cbe05720f29c51343baa7779 100644 (file)
--- 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 7eb40391003bbe279abd2f684d942f0f72c55156..f682359379f747fbbdd571b3653485e3ece87eb4 100644 (file)
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -270,7 +270,18 @@ 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),
@@ -286,6 +297,17 @@ 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
 

diff --git a/lib/config/load_test.go b/lib/config/load_test.go
index c9ed37b835697e99ab8acb851ab2ab02d1898a63..91bd6a74392564e14b98e35bae96e872f7de8a79 100644 (file)
--- a/lib/config/load_test.go
+++ b/lib/config/load_test.go
@@ -330,6 +330,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:
+ abcde:
+  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: