[ARVADOS] created: 2.1.0-921-g4b5ec8651

Git user git at public.arvados.org
Mon Jun 14 18:28:15 UTC 2021


        at  4b5ec8651ba83f2a79fe40708021e03c86275093 (commit)


commit 4b5ec8651ba83f2a79fe40708021e03c86275093
Author: Tom Clegg <tom at curii.com>
Date:   Mon Jun 14 14:28:00 2021 -0400

    17803: Ensure keys with mismatched case don't get used.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml
index e2ef9899e..f0794a7e5 100644
--- a/lib/config/config.default.yml
+++ b/lib/config/config.default.yml
@@ -24,49 +24,42 @@ Clusters:
 
       # In each of the service sections below, the keys under
       # InternalURLs are the endpoints where the service should be
-      # listening, and reachable from other hosts in the cluster.
-      SAMPLE:
-        InternalURLs:
-          "http://host1.example:12345": {}
-          "http://host2.example:12345":
-            # Rendezvous is normally empty/omitted. When changing the
-            # URL of a Keepstore service, Rendezvous should be set to
-            # the old URL (with trailing slash omitted) to preserve
-            # rendezvous ordering.
-            Rendezvous: ""
-          SAMPLE:
-            Rendezvous: ""
-        ExternalURL: "-"
+      # listening, and reachable from other hosts in the
+      # cluster. Example:
+      #
+      # InternalURLs:
+      #   "http://host1.example:12345": {}
+      #   "http://host2.example:12345": {}
 
       RailsAPI:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: "-"
       Controller:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       Websocket:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       Keepbalance:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: "-"
       GitHTTP:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       GitSSH:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       DispatchCloud:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: "-"
       SSO:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       Keepproxy:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       WebDAV:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         # Base URL for Workbench inline preview.  If blank, use
         # WebDAVDownload instead, and disable inline preview.
         # If both are empty, downloading collections from workbench
@@ -105,7 +98,7 @@ Clusters:
         ExternalURL: ""
 
       WebDAVDownload:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         # Base URL for download links. If blank, serve links to WebDAV
         # with disposition=attachment query param.  Unlike preview links,
         # browsers do not render attachments, so there is no risk of XSS.
@@ -119,13 +112,19 @@ Clusters:
         ExternalURL: ""
 
       Keepstore:
-        InternalURLs: {}
+        InternalURLs:
+          SAMPLE:
+            # Rendezvous is normally empty/omitted. When changing the
+            # URL of a Keepstore service, Rendezvous should be set to
+            # the old URL (with trailing slash omitted) to preserve
+            # rendezvous ordering.
+            Rendezvous: ""
         ExternalURL: "-"
       Composer:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       WebShell:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         # ShellInABox service endpoint URL for a given VM.  If empty, do not
         # offer web shell logins.
         #
@@ -136,13 +135,13 @@ Clusters:
         # https://*.webshell.uuid_prefix.arvadosapi.com
         ExternalURL: ""
       Workbench1:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       Workbench2:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       Health:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: "-"
 
     PostgreSQL:
diff --git a/lib/config/generated_config.go b/lib/config/generated_config.go
index fbee937b3..d5e0f200b 100644
--- a/lib/config/generated_config.go
+++ b/lib/config/generated_config.go
@@ -30,49 +30,42 @@ Clusters:
 
       # In each of the service sections below, the keys under
       # InternalURLs are the endpoints where the service should be
-      # listening, and reachable from other hosts in the cluster.
-      SAMPLE:
-        InternalURLs:
-          "http://host1.example:12345": {}
-          "http://host2.example:12345":
-            # Rendezvous is normally empty/omitted. When changing the
-            # URL of a Keepstore service, Rendezvous should be set to
-            # the old URL (with trailing slash omitted) to preserve
-            # rendezvous ordering.
-            Rendezvous: ""
-          SAMPLE:
-            Rendezvous: ""
-        ExternalURL: "-"
+      # listening, and reachable from other hosts in the
+      # cluster. Example:
+      #
+      # InternalURLs:
+      #   "http://host1.example:12345": {}
+      #   "http://host2.example:12345": {}
 
       RailsAPI:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: "-"
       Controller:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       Websocket:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       Keepbalance:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: "-"
       GitHTTP:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       GitSSH:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       DispatchCloud:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: "-"
       SSO:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       Keepproxy:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       WebDAV:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         # Base URL for Workbench inline preview.  If blank, use
         # WebDAVDownload instead, and disable inline preview.
         # If both are empty, downloading collections from workbench
@@ -111,7 +104,7 @@ Clusters:
         ExternalURL: ""
 
       WebDAVDownload:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         # Base URL for download links. If blank, serve links to WebDAV
         # with disposition=attachment query param.  Unlike preview links,
         # browsers do not render attachments, so there is no risk of XSS.
@@ -125,13 +118,19 @@ Clusters:
         ExternalURL: ""
 
       Keepstore:
-        InternalURLs: {}
+        InternalURLs:
+          SAMPLE:
+            # Rendezvous is normally empty/omitted. When changing the
+            # URL of a Keepstore service, Rendezvous should be set to
+            # the old URL (with trailing slash omitted) to preserve
+            # rendezvous ordering.
+            Rendezvous: ""
         ExternalURL: "-"
       Composer:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       WebShell:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         # ShellInABox service endpoint URL for a given VM.  If empty, do not
         # offer web shell logins.
         #
@@ -142,13 +141,13 @@ Clusters:
         # https://*.webshell.uuid_prefix.arvadosapi.com
         ExternalURL: ""
       Workbench1:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       Workbench2:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: ""
       Health:
-        InternalURLs: {}
+        InternalURLs: {SAMPLE: {}}
         ExternalURL: "-"
 
     PostgreSQL:
diff --git a/lib/config/load.go b/lib/config/load.go
index 73f0a2445..15174ae9c 100644
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -361,6 +361,9 @@ func (ldr *Loader) logExtraKeys(expected, supplied map[string]interface{}, prefi
 		}
 		vexp, ok := expected[k]
 		if expected["SAMPLE"] != nil {
+			// use the SAMPLE entry's keys as the
+			// "expected" map when checking vsupp
+			// recursively.
 			vexp = expected["SAMPLE"]
 		} else if !ok {
 			// check for a case-insensitive match
@@ -368,6 +371,12 @@ func (ldr *Loader) logExtraKeys(expected, supplied map[string]interface{}, prefi
 			for ek := range expected {
 				if strings.EqualFold(k, ek) {
 					hint = " (perhaps you meant " + ek + "?)"
+					// If we don't delete this, it
+					// will end up getting merged,
+					// unpredictably
+					// merging/overriding the
+					// default.
+					delete(supplied, k)
 					break
 				}
 			}
diff --git a/lib/config/load_test.go b/lib/config/load_test.go
index 9d3258c19..3e0368cc0 100644
--- a/lib/config/load_test.go
+++ b/lib/config/load_test.go
@@ -197,22 +197,38 @@ Clusters:
     Collections:
      BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
     PostgreSQL: {}
-    BadKey: {}
+    BadKey1: {}
     Containers:
       RunTimeEngine: abc
     RemoteClusters:
       z2222:
         Host: z2222.arvadosapi.com
         Proxy: true
-        BadKey: badValue
+        BadKey2: badValue
+    Services:
+      KeepStore:
+        InternalURLs:
+          "http://host.example:12345": {}
+      # we use Keepproxy instead of Keepstore for the RendezVous test,
+      # to avoid the "keepstore has no volumes" warning
+      Keepproxy:
+        InternalURLs:
+          "http://host.example:12345":
+            # ideally we would reject Rendezvous here too, but
+            # currently we don't
+            RendezVous: x
+    ServiceS:
+      Keepstore:
+        InternalURLs:
+          "http://host.example:12345": {}
 `, &logbuf).Load()
 	c.Assert(err, check.IsNil)
 	c.Log(logbuf.String())
 	logs := strings.Split(strings.TrimSuffix(logbuf.String(), "\n"), "\n")
 	for _, log := range logs {
-		c.Check(log, check.Matches, `.*deprecated or unknown config entry:.*(RunTimeEngine.*RuntimeEngine|BadKey).*`)
+		c.Check(log, check.Matches, `.*deprecated or unknown config entry:.*(RunTimeEngine.*RuntimeEngine|BadKey1|BadKey2|KeepStore|ServiceS|RendezVous).*`)
 	}
-	c.Check(logs, check.HasLen, 3)
+	c.Check(logs, check.HasLen, 6)
 }
 
 func (s *LoadSuite) checkSAMPLEKeys(c *check.C, path string, x interface{}) {
@@ -325,7 +341,7 @@ func (s *LoadSuite) TestPostgreSQLKeyConflict(c *check.C) {
 Clusters:
  zzzzz:
   PostgreSQL:
-   connection:
+   Connection:
      DBName: dbname
      Host: host
 `, nil).Load()

commit 4e65a41bea4b892ef7232bfe6b9b20ca35380368
Author: Tom Clegg <tom at curii.com>
Date:   Sun Jun 13 10:00:45 2021 -0400

    17803: Warn when config keys are ignored due to case mismatch.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/config/load.go b/lib/config/load.go
index cc26cdaec..73f0a2445 100644
--- a/lib/config/load.go
+++ b/lib/config/load.go
@@ -354,20 +354,24 @@ func (ldr *Loader) logExtraKeys(expected, supplied map[string]interface{}, prefi
 	if ldr.Logger == nil {
 		return
 	}
-	allowed := map[string]interface{}{}
-	for k, v := range expected {
-		allowed[strings.ToLower(k)] = v
-	}
 	for k, vsupp := range supplied {
 		if k == "SAMPLE" {
 			// entry will be dropped in removeSampleKeys anyway
 			continue
 		}
-		vexp, ok := allowed[strings.ToLower(k)]
+		vexp, ok := expected[k]
 		if expected["SAMPLE"] != nil {
 			vexp = expected["SAMPLE"]
 		} else if !ok {
-			ldr.Logger.Warnf("deprecated or unknown config entry: %s%s", prefix, k)
+			// check for a case-insensitive match
+			hint := ""
+			for ek := range expected {
+				if strings.EqualFold(k, ek) {
+					hint = " (perhaps you meant " + ek + "?)"
+					break
+				}
+			}
+			ldr.Logger.Warnf("deprecated or unknown config entry: %s%s%s", prefix, k, hint)
 			continue
 		}
 		if vsupp, ok := vsupp.(map[string]interface{}); !ok {
diff --git a/lib/config/load_test.go b/lib/config/load_test.go
index 6c11ee780..9d3258c19 100644
--- a/lib/config/load_test.go
+++ b/lib/config/load_test.go
@@ -196,9 +196,10 @@ Clusters:
     SystemRootToken: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
     Collections:
      BlobSigningKey: aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
-    postgresql: {}
+    PostgreSQL: {}
     BadKey: {}
-    Containers: {}
+    Containers:
+      RunTimeEngine: abc
     RemoteClusters:
       z2222:
         Host: z2222.arvadosapi.com
@@ -206,11 +207,12 @@ Clusters:
         BadKey: badValue
 `, &logbuf).Load()
 	c.Assert(err, check.IsNil)
+	c.Log(logbuf.String())
 	logs := strings.Split(strings.TrimSuffix(logbuf.String(), "\n"), "\n")
 	for _, log := range logs {
-		c.Check(log, check.Matches, `.*deprecated or unknown config entry:.*BadKey.*`)
+		c.Check(log, check.Matches, `.*deprecated or unknown config entry:.*(RunTimeEngine.*RuntimeEngine|BadKey).*`)
 	}
-	c.Check(logs, check.HasLen, 2)
+	c.Check(logs, check.HasLen, 3)
 }
 
 func (s *LoadSuite) checkSAMPLEKeys(c *check.C, path string, x interface{}) {
@@ -322,7 +324,7 @@ func (s *LoadSuite) TestPostgreSQLKeyConflict(c *check.C) {
 	_, err := testLoader(c, `
 Clusters:
  zzzzz:
-  postgresql:
+  PostgreSQL:
    connection:
      DBName: dbname
      Host: host

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


hooks/post-receive
-- 




More information about the arvados-commits mailing list