[arvados] updated: 2.7.0-6011-gdc6b5b04d0
git repository hosting
git at public.arvados.org
Thu Feb 22 16:29:26 UTC 2024
Summary of changes:
services/keepstore/keepstore.go | 78 ++++++++++++++++++++++--------------
services/keepstore/keepstore_test.go | 22 +++++++++-
services/keepstore/pull_worker.go | 2 +-
services/keepstore/router.go | 2 +-
services/keepstore/trash_worker.go | 2 +-
5 files changed, 70 insertions(+), 36 deletions(-)
via dc6b5b04d0b0ea40e9e9caa499f070f95b99fa07 (commit)
via 5b4d6b3be363cd47a8191ac1da42f5a2c13ef72d (commit)
via 26411493537eeb54ddafa2e9e29a5723edcd0316 (commit)
from 62168c2db5c36de2362cd1d5785b598b187bbef3 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
commit dc6b5b04d0b0ea40e9e9caa499f070f95b99fa07
Author: Tom Clegg <tom at curii.com>
Date: Thu Feb 22 11:25:39 2024 -0500
2960: Use getLocatorInfo to get block size in blockReadRemote.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index b29c881769..60d062e1e3 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -322,7 +322,10 @@ func (ks *keepstore) blockReadRemote(ctx context.Context, opts arvados.BlockRead
}
var remoteClient *keepclient.KeepClient
var parts []string
- var size int
+ li, err := getLocatorInfo(opts.Locator)
+ if err != nil {
+ return 0, err
+ }
for i, part := range strings.Split(opts.Locator, "+") {
switch {
case i == 0:
@@ -344,8 +347,6 @@ func (ks *keepstore) blockReadRemote(ctx context.Context, opts arvados.BlockRead
}
remoteClient = kc
part = "A" + part[7:]
- case len(part) > 0 && part[0] >= '0' && part[0] <= '9':
- size, _ = strconv.Atoi(part)
}
parts = append(parts, part)
}
@@ -356,8 +357,8 @@ func (ks *keepstore) blockReadRemote(ctx context.Context, opts arvados.BlockRead
if opts.LocalLocator == nil {
// Read from remote cluster and stream response back
// to caller
- if rw, ok := opts.WriteTo.(http.ResponseWriter); ok && size > 0 {
- rw.Header().Set("Content-Length", fmt.Sprintf("%d", size))
+ if rw, ok := opts.WriteTo.(http.ResponseWriter); ok && li.size > 0 {
+ rw.Header().Set("Content-Length", fmt.Sprintf("%d", li.size))
}
return remoteClient.BlockRead(ctx, arvados.BlockReadOptions{
Locator: locator,
commit 5b4d6b3be363cd47a8191ac1da42f5a2c13ef72d
Author: Tom Clegg <tom at curii.com>
Date: Thu Feb 22 11:01:45 2024 -0500
2960: Rewrite getLocatorInfo and add some more error checks.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index d85961fd2a..b29c881769 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -719,31 +719,44 @@ type locatorInfo struct {
func getLocatorInfo(loc string) (locatorInfo, error) {
var li locatorInfo
- for i, part := range strings.Split(loc, "+") {
- if i == 0 {
- if len(part) != 32 {
+ plus := 0 // number of '+' chars seen so far
+ partlen := 0 // chars since last '+'
+ for i, c := range loc + "+" {
+ if c == '+' {
+ if partlen == 0 {
+ // double/leading/trailing '+'
return li, errInvalidLocator
}
- li.hash = part
+ if plus == 0 {
+ if i != 32 {
+ return li, errInvalidLocator
+ }
+ li.hash = loc[:i]
+ }
+ if plus == 1 {
+ if size, err := strconv.Atoi(loc[i-partlen : i]); err == nil {
+ li.size = size
+ }
+ }
+ plus++
+ partlen = 0
continue
}
- if i == 1 {
- if size, err := strconv.Atoi(part); err == nil {
- li.size = size
- continue
+ partlen++
+ if partlen == 1 {
+ if c == 'A' {
+ li.signed = true
+ }
+ if c == 'R' {
+ li.remote = true
+ }
+ if plus > 1 && c >= '0' && c <= '9' {
+ // size, if present at all, must come first
+ return li, errInvalidLocator
}
}
- if len(part) == 0 {
- return li, errInvalidLocator
- }
- if part[0] == 'A' {
- li.signed = true
- }
- if part[0] == 'R' {
- li.remote = true
- }
- if part[0] >= '0' && part[0] <= '9' {
- // size, if present at all, must come first
+ if plus == 0 && !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f')) {
+ // non-hexadecimal char in hash part
return li, errInvalidLocator
}
}
diff --git a/services/keepstore/keepstore_test.go b/services/keepstore/keepstore_test.go
index 79d51829fe..f9d9888f98 100644
--- a/services/keepstore/keepstore_test.go
+++ b/services/keepstore/keepstore_test.go
@@ -606,7 +606,7 @@ func (s *keepstoreSuite) TestBlockWrite_SkipReadOnly(c *C) {
c.Check(ks.mounts["zzzzz-nyw5e-222222222222222"].volume.(*stubVolume).stubLog.String(), HasLen, 0)
}
-func (s *keepstoreSuite) TestParseLocator(c *C) {
+func (s *keepstoreSuite) TestGetLocatorInfo(c *C) {
for _, trial := range []struct {
locator string
ok bool
@@ -622,6 +622,15 @@ func (s *keepstoreSuite) TestParseLocator(c *C) {
ok: true, expect: locatorInfo{size: 1234, remote: true}},
{locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+12345+Zexample+Rzzzzz-abcdef",
ok: true, expect: locatorInfo{size: 12345, remote: true}},
+ {locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+123456+👶🦈+Rzzzzz-abcdef",
+ ok: true, expect: locatorInfo{size: 123456, remote: true}},
+ // invalid: bad hash char
+ {locator: "aaaaaaaaaaaaaazaaaaaaaaaaaaaaaaa+1234",
+ ok: false},
+ {locator: "aaaaaaaaaaaaaaFaaaaaaaaaaaaaaaaa+1234",
+ ok: false},
+ {locator: "aaaaaaaaaaaaaa⛵aaaaaaaaaaaaaaaaa+1234",
+ ok: false},
// invalid: hash length != 32
{locator: "",
ok: false},
@@ -636,6 +645,15 @@ func (s *keepstoreSuite) TestParseLocator(c *C) {
// invalid: first hint is not size
{locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+Abcdef+1234",
ok: false},
+ // invalid: leading/trailing/double +
+ {locator: "+aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+1234",
+ ok: false},
+ {locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+1234+",
+ ok: false},
+ {locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa++1234",
+ ok: false},
+ {locator: "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa+1234++Abcdef at abcdef",
+ ok: false},
} {
c.Logf("=== %s", trial.locator)
li, err := getLocatorInfo(trial.locator)
commit 26411493537eeb54ddafa2e9e29a5723edcd0316
Author: Tom Clegg <tom at curii.com>
Date: Thu Feb 22 10:12:18 2024 -0500
2960: Rename parseLocator to getLocatorInfo, add comments.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>
diff --git a/services/keepstore/keepstore.go b/services/keepstore/keepstore.go
index c9a8023059..d85961fd2a 100644
--- a/services/keepstore/keepstore.go
+++ b/services/keepstore/keepstore.go
@@ -223,7 +223,7 @@ func (ks *keepstore) signLocator(token, locator string) string {
}
func (ks *keepstore) BlockRead(ctx context.Context, opts arvados.BlockReadOptions) (n int, err error) {
- li, err := parseLocator(opts.Locator)
+ li, err := getLocatorInfo(opts.Locator)
if err != nil {
return 0, err
}
@@ -611,7 +611,7 @@ func (ce *checkEqual) WriteAt(p []byte, offset int64) (int, error) {
}
func (ks *keepstore) BlockUntrash(ctx context.Context, locator string) error {
- li, err := parseLocator(locator)
+ li, err := getLocatorInfo(locator)
if err != nil {
return err
}
@@ -631,7 +631,7 @@ func (ks *keepstore) BlockUntrash(ctx context.Context, locator string) error {
}
func (ks *keepstore) BlockTouch(ctx context.Context, locator string) error {
- li, err := parseLocator(locator)
+ li, err := getLocatorInfo(locator)
if err != nil {
return err
}
@@ -655,7 +655,7 @@ func (ks *keepstore) BlockTrash(ctx context.Context, locator string) error {
if !ks.cluster.Collections.BlobTrash {
return errMethodNotAllowed
}
- li, err := parseLocator(locator)
+ li, err := getLocatorInfo(locator)
if err != nil {
return err
}
@@ -708,14 +708,16 @@ func ctxToken(ctx context.Context) string {
}
}
+// locatorInfo expresses the attributes of a locator that are relevant
+// for keepstore decision-making.
type locatorInfo struct {
hash string
size int
- remote bool
- signed bool
+ remote bool // locator has a +R hint
+ signed bool // locator has a +A hint
}
-func parseLocator(loc string) (locatorInfo, error) {
+func getLocatorInfo(loc string) (locatorInfo, error) {
var li locatorInfo
for i, part := range strings.Split(loc, "+") {
if i == 0 {
diff --git a/services/keepstore/keepstore_test.go b/services/keepstore/keepstore_test.go
index 28049506f6..79d51829fe 100644
--- a/services/keepstore/keepstore_test.go
+++ b/services/keepstore/keepstore_test.go
@@ -638,7 +638,7 @@ func (s *keepstoreSuite) TestParseLocator(c *C) {
ok: false},
} {
c.Logf("=== %s", trial.locator)
- li, err := parseLocator(trial.locator)
+ li, err := getLocatorInfo(trial.locator)
if !trial.ok {
c.Check(err, NotNil)
continue
diff --git a/services/keepstore/pull_worker.go b/services/keepstore/pull_worker.go
index c131de02cb..dc5eabaa15 100644
--- a/services/keepstore/pull_worker.go
+++ b/services/keepstore/pull_worker.go
@@ -120,7 +120,7 @@ func (p *puller) runWorker(ctx context.Context) {
logger := p.keepstore.logger.WithField("locator", item.Locator)
- li, err := parseLocator(item.Locator)
+ li, err := getLocatorInfo(item.Locator)
if err != nil {
logger.Warn("ignoring pull request for invalid locator")
return
diff --git a/services/keepstore/router.go b/services/keepstore/router.go
index 256bc18c26..0c8182c6ea 100644
--- a/services/keepstore/router.go
+++ b/services/keepstore/router.go
@@ -85,7 +85,7 @@ func (rtr *router) handleBlockRead(w http.ResponseWriter, req *http.Request) {
out := w
if req.Method == http.MethodHead {
out = discardWrite{ResponseWriter: w}
- } else if li, err := parseLocator(mux.Vars(req)["locator"]); err != nil {
+ } else if li, err := getLocatorInfo(mux.Vars(req)["locator"]); err != nil {
rtr.handleError(w, req, err)
return
} else if li.size == 0 && li.hash != "d41d8cd98f00b204e9800998ecf8427e" {
diff --git a/services/keepstore/trash_worker.go b/services/keepstore/trash_worker.go
index d704c3a7d5..819c25acc1 100644
--- a/services/keepstore/trash_worker.go
+++ b/services/keepstore/trash_worker.go
@@ -107,7 +107,7 @@ func (t *trasher) runWorker(ctx context.Context, mntsAllowTrash []*mount) {
defer t.inprogress.Add(-1)
logger := t.keepstore.logger.WithField("locator", item.Locator)
- li, err := parseLocator(item.Locator)
+ li, err := getLocatorInfo(item.Locator)
if err != nil {
logger.Warn("ignoring trash request for invalid locator")
return
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list