Skip to content

N
neoppod
Commit a4a9d69d authored by Kirill Smelkov's avatar Kirill Smelkov
Browse files
Options

X: Apply new URI scheme to NEO/go + some refactors and tests of URL parser 

/reviewed-by @kirr
/reviewed-on kirr/neo!4

* kirr/t+new-uri:
  Revert "Y client: Adjust URI scheme to move client-specific options to fragment"
  fixup! client.go: Fix URI client option parsing for supported + unsupported options
  client.go: Fix URI client option parsing for supported + unsupported options
  fixup! client_test: Add tests for NEO URI parser
  client_test: Add tests for NEO URI parser
  fixup! client: Refactor openClientByURL for easier testing
  client: Refactor openClientByURL for easier testing
  Y go/zodb: Handle common options in zurl in generic layer
parents dc36ffc7 c9490507
Hide whitespace changes
Inline Side-by-side
Showing with 275 additions and 98 deletions
go/neo/client.go
View file @ a4a9d69d
......@@ -37,6 +37,7 @@ import (
"lab.nexedi.com/kirr/go123/xnet"
"lab.nexedi.com/kirr/go123/xsync"
"lab.nexedi.com/kirr/neo/go/internal/log"
"lab.nexedi.com/kirr/neo/go/internal/task"
taskctx "lab.nexedi.com/kirr/neo/go/internal/xcontext/task"
"lab.nexedi.com/kirr/neo/go/internal/xurl"
......@@ -414,76 +415,23 @@ func (c *Client) Iterate(ctx context.Context, tidMin, tidMax zodb.Tid) zodb.ITxn
func openClientByURL(ctx context.Context, u *url.URL, opt *zodb.DriverOptions) (_ zodb.IStorageDriver, _ zodb.Tid, err error) {
// neo(s)://[credentials@]master1,master2,...,masterN/name?options
defer task.Runningf(&ctx, "neo: open %s", u)(&err)
var ssl bool
switch u.Scheme {
case "neo": ssl = false
case "neos": ssl = true
default: return nil, zodb.InvalidTid, fmt.Errorf("invalid scheme")
}
cred := u.User.String()
// ca=ca.crt;cert=my.crt;key=my.key
cred = strings.ReplaceAll(cred, ";", "&") // ; is no longer in default separators set https://github.com/golang/go/issues/25192
x, err := xurl.ParseQuery(cred)
if err != nil {
return nil, zodb.InvalidTid, fmt.Errorf("credentials: %s", err)
}
// xpop pops k from credentials, defaulting to $NEO_<K> if envok.
xpop := func(k string, envok bool) string {
v, ok := x[k]
if !ok && envok {
v = os.Getenv("NEO_"+strings.ToUpper(k))
}
delete(x, k)
return v
}
netcfg := neonet.Config{}
netcfg.LoNode = xpop("lonode", false)
if !ssl {
if len(x) != 0 {
return nil, zodb.InvalidTid, fmt.Errorf("credentials can be specified only with neos:// scheme")
}
} else {
netcfg.CA = xpop("ca", true)
netcfg.Cert = xpop("cert", true)
netcfg.Key = xpop("key", true)
if len(x) != 0 {
return nil, zodb.InvalidTid, fmt.Errorf("invalid credentials: %v", x)
}
}
name := u.Path
name = strings.TrimPrefix(name, "/")
if name == "" {
return nil, zodb.InvalidTid, fmt.Errorf("cluster name not specified")
}
q, err := xurl.ParseQuery(u.RawQuery)
urlinfo, err := parseURL(ctx, u)
if err != nil {
return nil, zodb.InvalidTid, err
}
if len(q) != 0 {
return nil, zodb.InvalidTid, fmt.Errorf("invalid query: %v", q)
}
if !opt.ReadOnly {
return nil, zodb.InvalidTid, fmt.Errorf("TODO write mode not implemented")
}
net, err := neonet.Join(ctx, netcfg)
net, err := neonet.Join(ctx, urlinfo.netcfg)
if err != nil {
return nil, zodb.InvalidTid, err
}
c := NewClient(name, strings.Split(u.Host, ","), net)
c := NewClient(urlinfo.name, strings.Split(urlinfo.masterAddr, ","), net)
c.ownNet = true
c.watchq = opt.Watchq
defer func() {
......@@ -533,6 +481,89 @@ func openClientByURL(ctx context.Context, u *url.URL, opt *zodb.DriverOptions) (
}
}
// parseURL extracts information from a NEO URI and puts this information into
// a urlInfo.
func parseURL(ctx context.Context, u *url.URL) (urlinfo *urlInfo, err error) {
// neo(s)://[credentials@]master1,master2,...,masterN/name?options
var ssl bool
switch u.Scheme {
case "neo": ssl = false
case "neos": ssl = true
default: return nil, fmt.Errorf("invalid scheme")
}
cred := u.User.String()
// ca=ca.crt;cert=my.crt;key=my.key
cred = strings.ReplaceAll(cred, ";", "&") // ; is no longer in default separators set https://github.com/golang/go/issues/25192
x, err := xurl.ParseQuery(cred)
if err != nil {
return nil, fmt.Errorf("credentials: %s", err)
}
// xpop pops k from credentials, defaulting to $NEO_<K> if envok.
xpop := func(k string, envok bool) string {
v, ok := x[k]
if !ok && envok {
v = os.Getenv("NEO_"+strings.ToUpper(k))
}
delete(x, k)
return v
}
netcfg := neonet.Config{}
netcfg.LoNode = xpop("lonode", false)
if !ssl {
if len(x) != 0 {
return nil, fmt.Errorf("credentials can be specified only with neos:// scheme")
}
} else {
netcfg.CA = xpop("ca", true)
netcfg.Cert = xpop("cert", true)
netcfg.Key = xpop("key", true)
if len(x) != 0 {
return nil, fmt.Errorf("invalid credentials: %v", x)
}
}
name := u.Path
name = strings.TrimPrefix(name, "/")
if name == "" {
return nil, fmt.Errorf("cluster name not specified")
}
q, err := xurl.ParseQuery(u.RawQuery)
if err != nil {
return nil, err
}
// pop not yet used client options
// (our neo client doesn't apply their effect yet)
for _, k := range []string {"compress", "cache-size", "logfile"} {
_, ok := q[k]
if ok {
delete(q, k)
log.Warningf(ctx, "TODO client doesn't support option '%q' yet", k)
}
}
if len(q) != 0 {
return nil, fmt.Errorf("invalid query: %v", q)
}
masterAddr := u.Host
return &urlInfo{masterAddr, name, netcfg}, nil
}
// urlInfo encapsulates data extracted from a NEO URI.
type urlInfo struct {
masterAddr string
name string
netcfg neonet.Config
}
// URL implements zodb.IStorageDriver.
func (c *Client) URL() string {
// XXX options if such were given to open are discarded
......
go/neo/client_test.go
View file @ a4a9d69d
......@@ -673,6 +673,61 @@ func TestWatch(t *testing.T) {
})
}
// TestParseURL ensures that parsing NEO URL works as expected (= following the
// scheme neo(s)://[credentials@]master1,master2,...,masterN/name?options)
func TestParseURL(t *testing.T) {
// Most simple valid URI
testParseURL(t, "neo://127.0.0.1/test", urlInfo{})
// With 2 masters
testParseURL(t, "neo://127.0.0.1,127.0.0.2/test", urlInfo{masterAddr: "127.0.0.1,127.0.0.2"})
// With ssl
u := "neos://ca=ca;cert=cert;key=key@127.0.0.1/test"
testParseURL(t, u, urlInfo{netcfg: neonet.Config{CA: "ca", Cert: "cert", Key: "key"}})
// With query parameters
u = "neo://127.0.0.1/test?compress=true&logfile=n.log&cache-size=256"
testParseURL(t, u, urlInfo{})
}
// testParseURL tests one zurl for correctness by comparing its parsed
// data with a user provided urlInfo.
//
// Hint: testParseURL automatically sets default to undefined fields of the
// user provided urlInfo.
func testParseURL(t *testing.T, zurl string, urlinfoOk urlInfo) {
e := func (format string, a ...interface{}) {
t.Errorf(zurl + ": " + format, a...)
}
urlinfoOk.setDefault()
u, err := url.Parse(zurl)
if err != nil {
t.Fatal(err)
}
urlinfo, err := parseURL(context.Background(), u)
if err != nil {
t.Fatal(err)
}
// Test urlInfo
if urlinfo.masterAddr != urlinfoOk.masterAddr {
e("masterAddr: got %q, want %q", urlinfo.masterAddr, urlinfoOk.masterAddr)
}
if urlinfo.name != urlinfoOk.name {
e("name: got %q, want %q", urlinfo.name, urlinfoOk.name)
}
if urlinfo.netcfg != urlinfoOk.netcfg {
e("netcfg: got %q, want %q", urlinfo.netcfg, urlinfoOk.netcfg)
}
}
// setDefault sets default test values for a urlInfo
func (urlinfo *urlInfo) setDefault() {
if urlinfo.masterAddr == "" {
urlinfo.masterAddr = "127.0.0.1"
}
if urlinfo.name == "" {
urlinfo.name = "test"
}
}
func neoOpen(zurl string, opt *zodb.DriverOptions) (_ *Client, at0 zodb.Tid, err error) {
defer xerr.Contextf(&err, "openneo %s", zurl)
......
go/zodb/storage.go
View file @ a4a9d69d
// Copyright (C) 2017-2021 Nexedi SA and Contributors.
// Copyright (C) 2017-2024 Nexedi SA and Contributors.
// Kirill Smelkov <kirr@nexedi.com>
//
// This program is free software: you can Use, Study, Modify and Redistribute
......@@ -25,11 +25,13 @@ import (
"fmt"
"log"
"net/url"
"strconv"
"strings"
"sync"
"lab.nexedi.com/kirr/go123/mem"
"lab.nexedi.com/kirr/go123/xcontext"
"lab.nexedi.com/kirr/go123/xerr"
)
// OpenOptions describes options for Open.
......@@ -95,7 +97,9 @@ func RegisterDriver(scheme string, opener DriverOpener) {
//
// It is similar to Open but returns low-level IStorageDriver instead of IStorage.
// Most users should use Open.
func OpenDriver(ctx context.Context, zurl string, opt *DriverOptions) (_ IStorageDriver, at0 Tid, _ error) {
func OpenDriver(ctx context.Context, zurl string, opt *DriverOptions) (_ IStorageDriver, at0 Tid, err error) {
defer xerr.Contextf(&err, "open driver %s %s", zurl, opt)
// no scheme -> file://
if !strings.Contains(zurl, ":") {
zurl = "file://" + zurl
......@@ -106,10 +110,84 @@ func OpenDriver(ctx context.Context, zurl string, opt *DriverOptions) (_ IStorag
return nil, InvalidTid, err
}
// XXX commonly handle some options from url -> opt?
// (e.g. ?readonly=1 -> opt.ReadOnly=true + remove ?readonly=1 from URL)
// ----//---- nocache
// handle common options: remove them from url and merge with opt, so
// that drivers need to handle only driver specific things in their
// open routine.
//
// For example:
//
// ?read-only=1 <-> opt.ReadOnly=true + remove ?read-only=1 from URL.
//
// We make sure there is no conflict in between opt and URL as those
// two sources of control could be both seen as priority ones
// depending on situation and point of view:
//
// - when a user invokes a program with url amended via &read-only=1
// he/she expects the storage to be opened in read-only mode
// independently of program defaults.
//
// - when a developer invokes Open with `ReadOnly: true` the developer
// also expects the storage to be opened in read-only mode
// independently of zurl.
//
// So it is a conflict of expectations if there is e.g. &read-only=false
// in the url, and ReadOnly: true in the program. We don't try to
// resolve such conflicts and simply report an error.
//
// It is sometimes handy to have control over storage parameters via
// zurl query, but given there is possibility for above-described
// conflict, we do not expose all common options to be tunable via zurl.
// Instead we expose there only the absolute minimum - e.g. only common
// options that are already well established in zodburi/py.
q, err := url.ParseQuery(u.RawQuery)
if err != nil {
return nil, InvalidTid, err
}
type XBool struct {
value bool
ok bool
}
boolOpt := func(name string) XBool {
x := XBool{}
if !q.Has(name) {
return x
}
x.ok = true
v := q.Get(name)
q.Del(name)
var err_ error
x.value, err_ = strconv.ParseBool(v)
if err == nil {
err = err_
}
return x
}
readonly := boolOpt("read-only")
readonly_ := boolOpt("read_only")
if err != nil {
return nil, InvalidTid, err
}
if readonly.ok && readonly_.ok {
if readonly.value != readonly_.value {
return nil, InvalidTid, fmt.Errorf("conflicting values in between read-only and read_only in url")
}
}
if !readonly.ok {
readonly = readonly_
}
if readonly.ok {
if readonly.value != opt.ReadOnly {
return nil, InvalidTid, fmt.Errorf("conflicting value in between read-only in url and program options")
}
}
u.RawQuery = q.Encode()
// lookup the driver in registry and tail opening to the driver
opener, ok := driverRegistry[u.Scheme]
if !ok {
return nil, InvalidTid, fmt.Errorf("zodb: URL scheme \"%s:\" not supported", u.Scheme)
......@@ -130,7 +208,9 @@ func OpenDriver(ctx context.Context, zurl string, opt *DriverOptions) (_ IStorag
// get support for well-known storages.
//
// Storage authors should register their storages with RegisterDriver.
func Open(ctx context.Context, zurl string, opt *OpenOptions) (IStorage, error) {
func Open(ctx context.Context, zurl string, opt *OpenOptions) (_ IStorage, err error) {
defer xerr.Contextf(&err, "open %s %s", zurl, opt)
drvWatchq := make(chan Event)
drvOpt := &DriverOptions{
ReadOnly: opt.ReadOnly,
......@@ -171,6 +251,37 @@ func Open(ctx context.Context, zurl string, opt *OpenOptions) (IStorage, error)
return stor, nil
}
// String represents OpenOptions in human-readable form.
//
// For example:
//
// (read-only, no-cache)
func (opt *OpenOptions) String() string {
v := []string{}
if opt.ReadOnly {
v = append(v, "read-only")
}
if opt.NoCache {
v = append(v, "no-cache")
}
return fmt.Sprintf("(%s)", strings.Join(v, ", "))
}
// String represents DriverOptions in human-readable form.
//
// For example
//
// (read-only, watch)
func (opt *DriverOptions) String() string {
v := []string{}
if opt.ReadOnly {
v = append(v, "read-only")
}
if opt.Watchq != nil {
v = append(v, "watch")
}
return fmt.Sprintf("(%s)", strings.Join(v, ", "))
}
// storage represents storage opened via Open.
......
neo/client/zodburi.py
View file @ a4a9d69d
......@@ -17,7 +17,7 @@
URI format:
neo(s)://[credentials@]master1,master2,...,masterN/name?server_options#client_options
neo(s)://[credentials@]master1,master2,...,masterN/name?options
"""
import ZODB.config
......@@ -30,9 +30,6 @@ from urlparse import urlsplit, parse_qsl
# _credopts defines which options correspond to credentials
_credopts = {'ca', 'cert', 'key'}
# _clientopts defines which options control client behaviour, not the storage itself
_clientopts = {'compress', 'read-only', 'logfile', 'cache-size'}
# neo_zconf_options returns set of zconfig options supported by NEO storage
def neo_zconf_options():
neo_schema = """<schema>
......@@ -49,15 +46,11 @@ def neo_zconf_options():
assert 'name' in options
options.remove('master_nodes') # comes in netloc
options.remove('name') # comes in path
for opt in _credopts.union(_clientopts):
for opt in _credopts:
assert opt in options, opt
return options
# _srvopts defines which options control server behaviour
_srvopts = neo_zconf_options() .difference(_credopts) .difference(_clientopts)
# canonical_opt_name returns "oPtion_nAme" as "option-name"
def canonical_opt_name(name):
return name.lower().replace('_', '-')
......@@ -68,6 +61,8 @@ def _resolve_uri(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)
# name is given as path
if path.startswith("/"):
......@@ -97,33 +92,21 @@ def _resolve_uri(uri):
raise ValueError("invalid uri: %s : unexpected credential %s" % (uri, k))
neokw[k] = v
# get server options from query
# 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 _credopts:
raise ValueError("invalid uri: %s : option %s must be in credentials" % (uri, k))
if k in _clientopts:
raise ValueError("invalid uri: %s : option %s must be in fragment" % (uri, k))
elif k in _srvopts:
neokw[k] = v
else:
raise ValueError("invalid uri: %s : invalid option %s" % (uri, k))
# get client option from fragment: only those that are defined by NEO
# schema go to storage - rest are returned as database options
for k, v in OrderedDict(parse_qsl(frag)).items():
if k in _credopts:
raise ValueError("invalid uri: %s : fragment option %s must be in credentials" % (uri, k))
if k in _srvopts:
raise ValueError("invalid uri: %s : fragment option %s must be in query" % (uri, k))
elif k in _clientopts:
elif k in neo_options:
neokw[k] = v
else:
# it might be option for storage, but not in canonical form e.g.
# read_only -> read-only (zodburi world settled on using "_" and
# ZConfig world on "-" as separators)
k2 = canonical_opt_name(k)
if k2 in _clientopts:
if k2 in neo_options:
neokw[k2] = v
# else keep this kv as db option
......
neo/tests/client/testZODBURI.py
View file @ a4a9d69d
......@@ -34,7 +34,7 @@ testv = [
""",
{}),
("neo://master1,master2:port2/db3#read_only=true",
("neo://master1,master2:port2/db3?read_only=true",
"""\
master_nodes\tmaster1 master2:port2
name\tdb3
......@@ -42,8 +42,8 @@ testv = [
""",
{}),
("neos://ca=qqq;cert=rrr;key=sss@[2001:67c:1254:2a::1]:1234,master2:port2/db4?dynamic_master_list=zzz"
"#read_only=false&compress=true&logfile=xxx&alpha=111"
("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
......@@ -51,10 +51,10 @@ testv = [
ca\tqqq
cert\trrr
key\tsss
dynamic_master_list\tzzz
read-only\tfalse
compress\ttrue
logfile\txxx
dynamic_master_list\tzzz
""",
{"alpha": "111", "beta": "222"}),
]
......@@ -64,8 +64,9 @@ testv = [
class ZODBURITests(unittest.TestCase):
def test_zodburi(self):
# invalid schema
# invalid schema / fragment
self.assertRaises(ValueError, _resolve_uri, "http://master/db")
self.assertRaises(ValueError, _resolve_uri, "neo://master/db#frag")
# master/db not fully specified
self.assertRaises(ValueError, _resolve_uri, "neo://master")
......@@ -74,12 +75,8 @@ class ZODBURITests(unittest.TestCase):
self.assertRaises(ValueError, _resolve_uri, "neo://master/db?master_nodes=a,b,c")
self.assertRaises(ValueError, _resolve_uri, "neo://master/db?name=zzz")
# option that corresponds to credential provided in query or fragment
# option that corresponds to credential provided in query
self.assertRaises(ValueError, _resolve_uri, "neos://master/db?ca=123")
self.assertRaises(ValueError, _resolve_uri, "neos://master/db#ca=123")
# option that corresponds to client provided in query
self.assertRaises(ValueError, _resolve_uri, "neos://master/db?compress=1")
# credentials with neo:// instead of neos://
self.assertRaises(ValueError, _resolve_uri, "neo://key:zzz@master/db")
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7