From 4b5ec8651ba83f2a79fe40708021e03c86275093 Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Mon, 14 Jun 2021 14:28:00 -0400 Subject: [PATCH] 17803: Ensure keys with mismatched case don't get used. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- lib/config/config.default.yml | 59 +++++++++++++++++----------------- lib/config/generated_config.go | 59 +++++++++++++++++----------------- lib/config/load.go | 9 ++++++ lib/config/load_test.go | 26 ++++++++++++--- 4 files changed, 88 insertions(+), 65 deletions(-) diff --git a/lib/config/config.default.yml b/lib/config/config.default.yml index e2ef9899e5..f0794a7e53 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 fbee937b39..d5e0f200b8 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 73f0a24457..15174ae9cf 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 9d3258c195..3e0368cc03 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() -- 2.30.2