[ARVADOS] updated: 1.3.0-2795-g62edf6175
Git user
git at public.arvados.org
Tue Aug 18 18:42:55 UTC 2020
Summary of changes:
sdk/go/arvados/fs_lookup.go | 38 +++++++++++++++++++++++++++++--------
sdk/go/arvados/fs_project.go | 3 +--
sdk/go/arvados/fs_project_test.go | 20 ++++++++++++++++++++
sdk/go/arvados/fs_site.go | 8 ++++----
sdk/go/arvados/fs_site_test.go | 2 +-
services/keep-web/s3.go | 10 ++++++++--
services/keep-web/s3_test.go | 40 +++++++++++++++++++++++++++++++++++++++
7 files changed, 104 insertions(+), 17 deletions(-)
via 62edf6175986bf062076b42f89ef472446d0d18e (commit)
via 00387550708e4e733061a23405c200b33e4c3aa9 (commit)
from b2af45c27eb49ccddfbdbfe824a56721acddad27 (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 62edf6175986bf062076b42f89ef472446d0d18e
Author: Tom Clegg <tom at tomclegg.ca>
Date: Tue Aug 18 14:40:40 2020 -0400
16535: Return 400 instead of 500 when unable to mkdir.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/services/keep-web/s3.go b/services/keep-web/s3.go
index d7d08668e..c77427540 100644
--- a/services/keep-web/s3.go
+++ b/services/keep-web/s3.go
@@ -159,8 +159,14 @@ func (h *handler) serveS3(w http.ResponseWriter, r *http.Request) bool {
http.Error(w, err.Error(), http.StatusBadRequest)
return true
}
- err := fs.Mkdir(dir, 0755)
- if err != nil && err != os.ErrExist {
+ err = fs.Mkdir(dir, 0755)
+ if err == arvados.ErrInvalidArgument {
+ // Cannot create a directory
+ // here.
+ err = fmt.Errorf("mkdir %q failed: %w", dir, err)
+ http.Error(w, err.Error(), http.StatusBadRequest)
+ return true
+ } else if err != nil && !os.IsExist(err) {
err = fmt.Errorf("mkdir %q failed: %w", dir, err)
http.Error(w, err.Error(), http.StatusInternalServerError)
return true
diff --git a/services/keep-web/s3_test.go b/services/keep-web/s3_test.go
index c76768a14..73553ff4d 100644
--- a/services/keep-web/s3_test.go
+++ b/services/keep-web/s3_test.go
@@ -220,6 +220,46 @@ func (s *IntegrationSuite) testS3PutObjectSuccess(c *check.C, bucket *s3.Bucket,
}
}
+func (s *IntegrationSuite) TestS3ProjectPutObjectNotSupported(c *check.C) {
+ stage := s.s3setup(c)
+ defer stage.teardown(c)
+ bucket := stage.projbucket
+
+ for _, trial := range []struct {
+ path string
+ size int
+ contentType string
+ }{
+ {
+ path: "newfile",
+ size: 1234,
+ contentType: "application/octet-stream",
+ }, {
+ path: "newdir/newfile",
+ size: 1234,
+ contentType: "application/octet-stream",
+ }, {
+ path: "newdir2/",
+ size: 0,
+ contentType: "application/x-directory",
+ },
+ } {
+ c.Logf("=== %v", trial)
+
+ _, err := bucket.GetReader(trial.path)
+ c.Assert(err, check.ErrorMatches, `404 Not Found`)
+
+ buf := make([]byte, trial.size)
+ rand.Read(buf)
+
+ err = bucket.PutReader(trial.path, bytes.NewReader(buf), int64(len(buf)), trial.contentType, s3.Private, s3.Options{})
+ c.Check(err, check.ErrorMatches, `400 Bad Request`)
+
+ _, err = bucket.GetReader(trial.path)
+ c.Assert(err, check.ErrorMatches, `404 Not Found`)
+ }
+}
+
func (s *IntegrationSuite) TestS3CollectionPutObjectFailure(c *check.C) {
stage := s.s3setup(c)
defer stage.teardown(c)
commit 00387550708e4e733061a23405c200b33e4c3aa9
Author: Tom Clegg <tom at tomclegg.ca>
Date: Tue Aug 18 14:35:12 2020 -0400
16535: Return "invalid arg" error when attempting mkdir in project.
Previously "not found" was returned, which was misleading.
Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at tomclegg.ca>
diff --git a/sdk/go/arvados/fs_lookup.go b/sdk/go/arvados/fs_lookup.go
index cb4ccfcf9..56b595323 100644
--- a/sdk/go/arvados/fs_lookup.go
+++ b/sdk/go/arvados/fs_lookup.go
@@ -66,22 +66,44 @@ func (ln *lookupnode) Readdir() ([]os.FileInfo, error) {
return ln.treenode.Readdir()
}
+// Child rejects (with ErrInvalidArgument) calls to add/replace
+// children, instead calling loadOne when a non-existing child is
+// looked up.
func (ln *lookupnode) Child(name string, replace func(inode) (inode, error)) (inode, error) {
ln.staleLock.Lock()
defer ln.staleLock.Unlock()
checkTime := time.Now()
+ var existing inode
+ var err error
if ln.stale(ln.staleAll) && ln.stale(ln.staleOne[name]) {
- _, err := ln.treenode.Child(name, func(inode) (inode, error) {
+ existing, err = ln.treenode.Child(name, func(inode) (inode, error) {
return ln.loadOne(ln, name)
})
- if err != nil {
- return nil, err
+ if err == nil && existing != nil {
+ if ln.staleOne == nil {
+ ln.staleOne = map[string]time.Time{name: checkTime}
+ } else {
+ ln.staleOne[name] = checkTime
+ }
}
- if ln.staleOne == nil {
- ln.staleOne = map[string]time.Time{name: checkTime}
- } else {
- ln.staleOne[name] = checkTime
+ } else {
+ existing, err = ln.treenode.Child(name, nil)
+ if err != nil && !os.IsNotExist(err) {
+ return existing, err
+ }
+ }
+ if replace != nil {
+ // Let the callback try to delete or replace the
+ // existing node; if it does, return
+ // ErrInvalidArgument.
+ if tryRepl, err := replace(existing); err != nil {
+ // Propagate error from callback
+ return existing, err
+ } else if tryRepl != existing {
+ return existing, ErrInvalidArgument
}
}
- return ln.treenode.Child(name, replace)
+ // Return original error from ln.treenode.Child() (it might be
+ // ErrNotExist).
+ return existing, err
}
diff --git a/sdk/go/arvados/fs_project.go b/sdk/go/arvados/fs_project.go
index c5eb03360..bf6391a74 100644
--- a/sdk/go/arvados/fs_project.go
+++ b/sdk/go/arvados/fs_project.go
@@ -6,7 +6,6 @@ package arvados
import (
"log"
- "os"
"strings"
)
@@ -57,7 +56,7 @@ func (fs *customFileSystem) projectsLoadOne(parent inode, uuid, name string) (in
// both "/" and the substitution string.
}
if len(contents.Items) == 0 {
- return nil, os.ErrNotExist
+ return nil, nil
}
coll := contents.Items[0]
diff --git a/sdk/go/arvados/fs_project_test.go b/sdk/go/arvados/fs_project_test.go
index dd35323b7..1d091f5a8 100644
--- a/sdk/go/arvados/fs_project_test.go
+++ b/sdk/go/arvados/fs_project_test.go
@@ -238,3 +238,23 @@ func (s *SiteFSSuite) TestProjectUpdatedByOther(c *check.C) {
_, err = s.fs.Open("/home/A Project/oob")
c.Check(err, check.IsNil) // parent dir still has old collection -- didn't reload, because Sync failed
}
+
+func (s *SiteFSSuite) TestProjectUnsupportedOperations(c *check.C) {
+ s.fs.MountByID("by_id")
+ s.fs.MountProject("home", "")
+
+ _, err := s.fs.OpenFile("/home/A Project/newfilename", os.O_CREATE|os.O_RDWR, 0)
+ c.Check(err, check.ErrorMatches, "invalid argument")
+
+ err = s.fs.Mkdir("/home/A Project/newdirname", 0)
+ c.Check(err, check.ErrorMatches, "invalid argument")
+
+ err = s.fs.Mkdir("/by_id/newdirname", 0)
+ c.Check(err, check.ErrorMatches, "invalid argument")
+
+ err = s.fs.Mkdir("/by_id/"+fixtureAProjectUUID+"/newdirname", 0)
+ c.Check(err, check.ErrorMatches, "invalid argument")
+
+ _, err = s.fs.OpenFile("/home/A Project", 0, 0)
+ c.Check(err, check.IsNil)
+}
diff --git a/sdk/go/arvados/fs_site.go b/sdk/go/arvados/fs_site.go
index 95b2f71c2..900893aa3 100644
--- a/sdk/go/arvados/fs_site.go
+++ b/sdk/go/arvados/fs_site.go
@@ -127,7 +127,7 @@ func (fs *customFileSystem) Stale(t time.Time) bool {
}
func (fs *customFileSystem) newNode(name string, perm os.FileMode, modTime time.Time) (node inode, err error) {
- return nil, ErrInvalidOperation
+ return nil, ErrInvalidArgument
}
func (fs *customFileSystem) mountByID(parent inode, id string) inode {
@@ -173,9 +173,9 @@ func (fs *customFileSystem) newProjectNode(root inode, name, uuid string) inode
}
}
-// vdirnode wraps an inode by ignoring any requests to add/replace
-// children, and calling a create() func when a non-existing child is
-// looked up.
+// vdirnode wraps an inode by rejecting (with ErrInvalidArgument)
+// calls that add/replace children directly, instead calling a
+// create() func when a non-existing child is looked up.
//
// create() can return either a new node, which will be added to the
// treenode, or nil for ENOENT.
diff --git a/sdk/go/arvados/fs_site_test.go b/sdk/go/arvados/fs_site_test.go
index 96ed74de8..d3111e1cb 100644
--- a/sdk/go/arvados/fs_site_test.go
+++ b/sdk/go/arvados/fs_site_test.go
@@ -103,7 +103,7 @@ func (s *SiteFSSuite) TestByUUIDAndPDH(c *check.C) {
c.Check(names, check.DeepEquals, []string{"baz"})
_, err = s.fs.OpenFile("/by_id/"+fixtureNonexistentCollection, os.O_RDWR|os.O_CREATE, 0755)
- c.Check(err, check.Equals, ErrInvalidOperation)
+ c.Check(err, check.Equals, ErrInvalidArgument)
err = s.fs.Rename("/by_id/"+fixtureFooCollection, "/by_id/beep")
c.Check(err, check.Equals, ErrInvalidArgument)
err = s.fs.Rename("/by_id/"+fixtureFooCollection+"/foo", "/by_id/beep")
-----------------------------------------------------------------------
hooks/post-receive
--
More information about the arvados-commits
mailing list