[arvados] created: 2.7.0-5594-g6c7c27f15f

git repository hosting git at public.arvados.org
Mon Dec 11 15:25:36 UTC 2023


        at  6c7c27f15f02cac915023590b4dfde6c4fd0af0b (commit)


commit 6c7c27f15f02cac915023590b4dfde6c4fd0af0b
Author: Tom Clegg <tom at curii.com>
Date:   Mon Dec 11 10:22:30 2023 -0500

    21214: Include filter groups in sitefs.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/sdk/go/arvados/fs_project.go b/sdk/go/arvados/fs_project.go
index a68e83945e..ed9bb0c08d 100644
--- a/sdk/go/arvados/fs_project.go
+++ b/sdk/go/arvados/fs_project.go
@@ -38,7 +38,7 @@ func (fs *customFileSystem) projectsLoadOne(parent inode, uuid, name string) (in
 			Filters: []Filter{
 				{"name", "=", strings.Replace(name, subst, "/", -1)},
 				{"uuid", "is_a", []string{"arvados#collection", "arvados#group"}},
-				{"groups.group_class", "=", "project"},
+				{"groups.group_class", "in", []string{"project", "filter"}},
 			},
 			Select: []string{"uuid", "name", "modified_at", "properties"},
 		})
@@ -104,7 +104,7 @@ func (fs *customFileSystem) projectsLoadAll(parent inode, uuid string) ([]inode,
 			{"uuid", "is_a", class},
 		}
 		if class == "arvados#group" {
-			filters = append(filters, Filter{"group_class", "=", "project"})
+			filters = append(filters, Filter{"groups.group_class", "in", []string{"project", "filter"}})
 		}
 
 		params := ResourceListParams{
diff --git a/sdk/go/arvados/fs_project_test.go b/sdk/go/arvados/fs_project_test.go
index d3dac7a14f..5a675100f7 100644
--- a/sdk/go/arvados/fs_project_test.go
+++ b/sdk/go/arvados/fs_project_test.go
@@ -42,61 +42,62 @@ func (sc *spyingClient) RequestAndDecode(dst interface{}, method, path string, b
 func (s *SiteFSSuite) TestFilterGroup(c *check.C) {
 	// Make sure that a collection and group that match the filter are present,
 	// and that a group that does not match the filter is not present.
-	s.fs.MountProject("fg", fixtureThisFilterGroupUUID)
-
-	_, err := s.fs.OpenFile("/fg/baz_file", 0, 0)
-	c.Assert(err, check.IsNil)
 
-	_, err = s.fs.OpenFile("/fg/A Subproject", 0, 0)
-	c.Assert(err, check.IsNil)
+	checkOpen := func(path string, exists bool) {
+		f, err := s.fs.Open(path)
+		if exists {
+			if c.Check(err, check.IsNil) {
+				c.Check(f.Close(), check.IsNil)
+			}
+		} else {
+			c.Check(err, check.Equals, os.ErrNotExist)
+		}
+	}
 
-	_, err = s.fs.OpenFile("/fg/A Project", 0, 0)
-	c.Assert(err, check.Not(check.IsNil))
+	checkOpen("/users/active/This filter group/baz_file", true)
+	checkOpen("/users/active/This filter group/A Subproject", true)
+	checkOpen("/users/active/This filter group/A Project", false)
+	s.fs.MountProject("fg", fixtureThisFilterGroupUUID)
+	checkOpen("/fg/baz_file", true)
+	checkOpen("/fg/A Subproject", true)
+	checkOpen("/fg/A Project", false)
+	s.fs.MountProject("home", "")
+	checkOpen("/home/A filter group with an is_a collection filter/baz_file", true)
+	checkOpen("/home/A filter group with an is_a collection filter/baz_file/baz", true)
+	checkOpen("/home/A filter group with an is_a collection filter/A Subproject", false)
+	checkOpen("/home/A filter group with an is_a collection filter/A Project", false)
 
 	// An empty filter means everything that is visible should be returned.
+	checkOpen("/users/active/A filter group without filters/baz_file", true)
+	checkOpen("/users/active/A filter group without filters/A Subproject", true)
+	checkOpen("/users/active/A filter group without filters/A Project", true)
 	s.fs.MountProject("fg2", fixtureAFilterGroupTwoUUID)
-
-	_, err = s.fs.OpenFile("/fg2/baz_file", 0, 0)
-	c.Assert(err, check.IsNil)
-
-	_, err = s.fs.OpenFile("/fg2/A Subproject", 0, 0)
-	c.Assert(err, check.IsNil)
-
-	_, err = s.fs.OpenFile("/fg2/A Project", 0, 0)
-	c.Assert(err, check.IsNil)
+	checkOpen("/fg2/baz_file", true)
+	checkOpen("/fg2/A Subproject", true)
+	checkOpen("/fg2/A Project", true)
 
 	// An 'is_a' 'arvados#collection' filter means only collections should be returned.
+	checkOpen("/users/active/A filter group with an is_a collection filter/baz_file", true)
+	checkOpen("/users/active/A filter group with an is_a collection filter/baz_file/baz", true)
+	checkOpen("/users/active/A filter group with an is_a collection filter/A Subproject", false)
+	checkOpen("/users/active/A filter group with an is_a collection filter/A Project", false)
 	s.fs.MountProject("fg3", fixtureAFilterGroupThreeUUID)
-
-	_, err = s.fs.OpenFile("/fg3/baz_file", 0, 0)
-	c.Assert(err, check.IsNil)
-
-	_, err = s.fs.OpenFile("/fg3/A Subproject", 0, 0)
-	c.Assert(err, check.Not(check.IsNil))
+	checkOpen("/fg3/baz_file", true)
+	checkOpen("/fg3/baz_file/baz", true)
+	checkOpen("/fg3/A Subproject", false)
 
 	// An 'exists' 'arvados#collection' filter means only collections with certain properties should be returned.
 	s.fs.MountProject("fg4", fixtureAFilterGroupFourUUID)
-
-	_, err = s.fs.Stat("/fg4/collection with list property with odd values")
-	c.Assert(err, check.IsNil)
-
-	_, err = s.fs.Stat("/fg4/collection with list property with even values")
-	c.Assert(err, check.IsNil)
+	checkOpen("/fg4/collection with list property with odd values", true)
+	checkOpen("/fg4/collection with list property with even values", true)
+	checkOpen("/fg4/baz_file", false)
 
 	// A 'contains' 'arvados#collection' filter means only collections with certain properties should be returned.
 	s.fs.MountProject("fg5", fixtureAFilterGroupFiveUUID)
-
-	_, err = s.fs.Stat("/fg5/collection with list property with odd values")
-	c.Assert(err, check.IsNil)
-
-	_, err = s.fs.Stat("/fg5/collection with list property with string value")
-	c.Assert(err, check.IsNil)
-
-	_, err = s.fs.Stat("/fg5/collection with prop2 5")
-	c.Assert(err, check.Not(check.IsNil))
-
-	_, err = s.fs.Stat("/fg5/collection with list property with even values")
-	c.Assert(err, check.Not(check.IsNil))
+	checkOpen("/fg5/collection with list property with odd values", true)
+	checkOpen("/fg5/collection with list property with string value", true)
+	checkOpen("/fg5/collection with prop2 5", false)
+	checkOpen("/fg5/collection with list property with even values", false)
 }
 
 func (s *SiteFSSuite) TestCurrentUserHome(c *check.C) {
diff --git a/sdk/go/arvadostest/fixtures.go b/sdk/go/arvadostest/fixtures.go
index ac12f7ae13..3b8a618fea 100644
--- a/sdk/go/arvadostest/fixtures.go
+++ b/sdk/go/arvadostest/fixtures.go
@@ -37,8 +37,9 @@ const (
 	StorageClassesDesiredArchiveConfirmedDefault = "zzzzz-4zz18-3t236wr12769qqa"
 	EmptyCollectionUUID                          = "zzzzz-4zz18-gs9ooj1h9sd5mde"
 
-	AProjectUUID    = "zzzzz-j7d0g-v955i6s2oi1cbso"
-	ASubprojectUUID = "zzzzz-j7d0g-axqo7eu9pwvna1x"
+	AProjectUUID     = "zzzzz-j7d0g-v955i6s2oi1cbso"
+	ASubprojectUUID  = "zzzzz-j7d0g-axqo7eu9pwvna1x"
+	AFilterGroupUUID = "zzzzz-j7d0g-thisfiltergroup"
 
 	FooAndBarFilesInDirUUID = "zzzzz-4zz18-foonbarfilesdir"
 	FooAndBarFilesInDirPDH  = "870369fc72738603c2fad16664e50e2d+58"
diff --git a/services/keep-web/handler_test.go b/services/keep-web/handler_test.go
index c14789889d..42bfd80fa5 100644
--- a/services/keep-web/handler_test.go
+++ b/services/keep-web/handler_test.go
@@ -1105,6 +1105,17 @@ func (s *IntegrationSuite) TestDirectoryListingWithNoAnonymousToken(c *check.C)
 }
 
 func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
+	// The "ownership cycle" test fixtures are reachable from the
+	// "filter group without filters" group, causing webdav's
+	// walkfs to recurse indefinitely. Avoid that by deleting one
+	// of the bogus fixtures.
+	arv := arvados.NewClientFromEnv()
+	err := arv.RequestAndDecode(nil, "DELETE", "arvados/v1/groups/zzzzz-j7d0g-cx2al9cqkmsf1hs", nil, nil)
+	if err != nil {
+		c.Assert(err, check.FitsTypeOf, &arvados.TransactionError{})
+		c.Check(err.(*arvados.TransactionError).StatusCode, check.Equals, 404)
+	}
+
 	s.handler.Cluster.Services.WebDAVDownload.ExternalURL.Host = "download.example.com"
 	authHeader := http.Header{
 		"Authorization": {"OAuth2 " + arvadostest.ActiveToken},
@@ -1241,6 +1252,30 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
 			expect:  []string{"waz"},
 			cutDirs: 2,
 		},
+		{
+			uri:     "download.example.com/users/active/This filter group/",
+			header:  authHeader,
+			expect:  []string{"A Subproject/"},
+			cutDirs: 3,
+		},
+		{
+			uri:     "download.example.com/users/active/This filter group/A Subproject",
+			header:  authHeader,
+			expect:  []string{"baz_file/"},
+			cutDirs: 4,
+		},
+		{
+			uri:     "download.example.com/by_id/" + arvadostest.AFilterGroupUUID,
+			header:  authHeader,
+			expect:  []string{"A Subproject/"},
+			cutDirs: 2,
+		},
+		{
+			uri:     "download.example.com/by_id/" + arvadostest.AFilterGroupUUID + "/A Subproject",
+			header:  authHeader,
+			expect:  []string{"baz_file/"},
+			cutDirs: 3,
+		},
 	} {
 		comment := check.Commentf("HTML: %q => %q", trial.uri, trial.expect)
 		resp := httptest.NewRecorder()
@@ -1278,6 +1313,7 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
 		} else {
 			c.Check(resp.Code, check.Equals, http.StatusOK, comment)
 			for _, e := range trial.expect {
+				e = strings.Replace(e, " ", "%20", -1)
 				c.Check(resp.Body.String(), check.Matches, `(?ms).*href="./`+e+`".*`, comment)
 			}
 			c.Check(resp.Body.String(), check.Matches, `(?ms).*--cut-dirs=`+fmt.Sprintf("%d", trial.cutDirs)+` .*`, comment)
@@ -1320,6 +1356,7 @@ func (s *IntegrationSuite) testDirectoryListing(c *check.C) {
 				} else {
 					e = filepath.Join(u.Path, e)
 				}
+				e = strings.Replace(e, " ", "%20", -1)
 				c.Check(resp.Body.String(), check.Matches, `(?ms).*<D:href>`+e+`</D:href>.*`, comment)
 			}
 		}

commit e9f7d73dc0d84ee7127b8f83a4955521e8091708
Author: Tom Clegg <tom at curii.com>
Date:   Sat Dec 9 15:56:10 2023 -0500

    21214: Prevent filter groups from matching other/same filter groups.
    
    Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom at curii.com>

diff --git a/lib/controller/localdb/group.go b/lib/controller/localdb/group.go
index 418fd6b8b7..a21a062db8 100644
--- a/lib/controller/localdb/group.go
+++ b/lib/controller/localdb/group.go
@@ -97,6 +97,12 @@ func (conn *Conn) GroupContents(ctx context.Context, options arvados.GroupConten
 				filter.Operand = tmp[2]
 				options.Filters = append(options.Filters, filter)
 			}
+			// Prevent infinite-depth trees caused by a
+			// filter group whose filters match [other
+			// filter groups that match] itself. Such
+			// cycles are supported by sitefs, but they
+			// cause havoc in webdav. See #21214.
+			options.Filters = append(options.Filters, arvados.Filter{"groups.group_class", "=", "project"})
 		} else {
 			return resp, fmt.Errorf("filter unparsable: not an array\n")
 		}

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list