Y client: Fix URI scheme to move credentials out of query

- Move ca/cert/key out of query part into credentials part of URL
- As a consequence move cluster name out of credentials part into path part
- Allow both neo:// and neos:// schemes to be used.
  neo:// means alwasy without SSL and neos:// means always with SSL.
  Even if neos:// URL comes without credentials embedded into it - they
  can be obtained from elsewhere.

I need this change in wendelin.core 2 which needs to take a zurl and
compute a mountpoint patch that corresponds to it, so that multiple
several zopes that use the same NEO storage, could be associated with
the same single WCFS instance.

neo/client/zodburi.py

 #
-# Copyright (C) 2017-2019  Nexedi SA
+# Copyright (C) 2017-2020  Nexedi SA
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -17,7 +17,7 @@
 
 URI format:
 
-    neo://name@master1,master2,...,masterN?options
+    neo(s)://[credentials@]master1,master2,...,masterN/name?options
 """
 
 import ZODB.config
@@ -27,6 +27,9 @@ from cStringIO import StringIO
 from collections import OrderedDict
 from urlparse import urlsplit, parse_qsl
 
+# _credopts defines which options correspond to credentials
+_credopts = {'ca', 'cert', 'key'}
+
 # neo_zconf_options returns set of zconfig options supported by NEO storage
 def neo_zconf_options():
     neo_schema = """<schema>
@@ -41,6 +44,8 @@ def neo_zconf_options():
     options = {k for k, _ in neo_storage_zconf}
     assert 'master_nodes' in options
     assert 'name' in options
+    for opt in _credopts:
+        assert opt in options, opt
 
     return options
 
@@ -52,27 +57,48 @@ def canonical_opt_name(name):
 def _resolve_uri(uri):
     scheme, netloc, path, query, frag = urlsplit(uri)
 
-    if scheme != "neo":
-        raise ValueError("invalid uri: %s : expected neo:// scheme" % uri)
-    if path != "":
-        raise ValueError("invalid uri: %s : non-empty path" % uri)
+    if scheme not in ("neo", "neos"):
+        raise ValueError("invalid uri: %s : expected neo:// or neos:// scheme" % uri)
     if frag != "":
         raise ValueError("invalid uri: %s : non-empty fragment" % uri)
 
-    # extract master list and name from netloc
-    name, masterloc = netloc.split('@', 1)
+    # name is given as path
+    if path.startswith("/"):
+        path = path[1:]
+    name = path
+    if name == '':
+        raise ValueError("invalid uri: %s : cluster name not specified" % uri)
+
+    # extract master list and credentials from netloc
+    cred, masterloc = '', netloc
+    if '@' in netloc:
+        cred, masterloc = netloc.split('@', 1)
     master_list = masterloc.split(',')
 
     neokw = OrderedDict()
     neokw['master_nodes'] = ' '.join(master_list)
     neokw['name'] = name
 
+    # parse credentials
+    if cred:
+        if scheme != "neos":
+            raise ValueError("invalid uri: %s : credentials can be provided only with neos:// scheme" % uri)
+        # ca:ca.crt;cert:my.crt;key:my.key
+        credv = cred.split(';')
+        for arg in credv:
+            k,v = arg.split(':', 1)
+            if k not in _credopts:
+                raise ValueError("invalid uri: %s : unexpected credential %s" % (uri, k))
+            neokw[k] = v
+
     # get options from query: only those that are defined by NEO schema go to
     # storage - rest are returned as database options
     dbkw = {}
     neo_options = neo_zconf_options()
     for k, v in OrderedDict(parse_qsl(query)).items():
-        if k in neo_options:
+        if k in _credopts:
+            raise ValueError("invalid uri: %s : option %s must be in credentials" % (uri, k))
+        elif k in neo_options:
             neokw[k] = v
         else:
             # it might be option for storage, but not in canonical form e.g.

neo/tests/client/testZODBURI.py

 #
-# Copyright (C) 2017-2019  Nexedi SA
+# Copyright (C) 2017-2020  Nexedi SA
 #
 # This program is free software; you can redistribute it and/or
 # modify it under the terms of the GNU General Public License
@@ -20,21 +20,21 @@ from neo.client.zodburi import _resolve_uri
 
 testv = [
     # [] of (uri, zconf_ok, dbkw_ok)
-    ("neo://dbname@master",
+    ("neo://master/dbname",
      """\
   master_nodes\tmaster
   name\tdbname
 """,
      {}),
 
-    ("neo://db2@master1:port1,master2:port2,master3:port3",
+    ("neo://master1:port1,master2:port2,master3:port3/db2",
      """\
   master_nodes\tmaster1:port1 master2:port2 master3:port3
   name\tdb2
 """,
      {}),
 
-    ("neo://db3@master1,master2:port2?read_only=true",
+    ("neo://master1,master2:port2/db3?read_only=true",
      """\
   master_nodes\tmaster1 master2:port2
   name\tdb3
@@ -42,19 +42,19 @@ testv = [
 """,
      {}),
 
-    ("neo://db4@[2001:67c:1254:2a::1]:1234,master2:port2?read_only=false"
-     "&compress=true&logfile=xxx&alpha=111&dynamic_master_list=zzz&ca=qqq"
-     "&cert=rrr&key=sss&beta=222",
+    ("neos://ca:qqq;cert:rrr;key:sss@[2001:67c:1254:2a::1]:1234,master2:port2/db4?read_only=false"
+     "&compress=true&logfile=xxx&alpha=111&dynamic_master_list=zzz"
+     "&beta=222",
      """\
   master_nodes\t[2001:67c:1254:2a::1]:1234 master2:port2
   name\tdb4
+  ca\tqqq
+  cert\trrr
+  key\tsss
   read-only\tfalse
   compress\ttrue
   logfile\txxx
   dynamic_master_list\tzzz
-  ca\tqqq
-  cert\trrr
-  key\tsss
 """,
      {"alpha": "111", "beta": "222"}),
 ]
@@ -64,14 +64,20 @@ testv = [
 class ZODBURITests(unittest.TestCase):
 
     def test_zodburi(self):
-        # invalid schema / path / fragment
-        self.assertRaises(ValueError, _resolve_uri, "http://db@master")
-        self.assertRaises(ValueError, _resolve_uri, "neo://db@master/path")
-        self.assertRaises(ValueError, _resolve_uri, "neo://db@master#frag")
+        # invalid schema / fragment
+        self.assertRaises(ValueError, _resolve_uri, "http://master/db")
+        self.assertRaises(ValueError, _resolve_uri, "neo://master/db#frag")
 
-        # db @ master not fully specified
+        # master/db not fully specified
         self.assertRaises(ValueError, _resolve_uri, "neo://master")
 
+        # option that corresponds to credential provided in query
+        self.assertRaises(ValueError, _resolve_uri, "neos://master/db?ca=123")
+
+        # credentials with neo:// instead of neos://
+        self.assertRaises(ValueError, _resolve_uri, "neo://key:zzz@master/db")
+
+
         # verify zodburi resolver produces expected zconfig
         for uri, zconf_ok, dbkw_ok in testv:
             zconf_ok = "%import neo.client\n<NEOStorage>\n" + zconf_ok + \

[Julien Muchembled]

Even if neos:// URL comes without credentials embedded into it - they can be obtained from elsewhere.

Can you clarify this ? (for the moment, I don't see the need of neos)

I need this change in wendelin.core 2 which needs to take a zurl and compute a mountpoint patch that corresponds to it, so that multiple several zopes that use the same NEO storage, could be associated with the same single WCFS instance.

I don't understand how moving credentials out of query helps to do that. I even say it's the contrary, for example because paths to SSL files can differ. Anyway, ip:port of masters are enough to identify a NEO db.

[Julien Muchembled]

(maybe I missed some place where credentials are automatically removed from the URL before some specific uses)

[Kirill Smelkov]

Thanks for feedback. I've created merge-request for this change (hereby patch + 8c974485) and follow up with replies there: nexedi/neoppod!18 (closed).