More static registry fixes (#2370)

* Fix overriding the provider path in rules

* Expose "user_layout" and "disable_home" config options

* Fix GetPathById()

* Fixup provider path

* Increase precision when calculating the etag

* Improve tests, test etag propagation

* fix hound issue

* Add changelog

* Fix linter issue

* Fix listing directories using the static registry

* Fix wrong paths when listing directories

* Do not mutate the reference parameter

* Fix local integration tests config

changelog/unreleased/static-registry2.md

@@ -0,0 +1,5 @@
+Change: Fix static registry regressions
+
+We fixed some smaller issues with using the static registry which were introduced with the spaces registry changes.
+
+https://github.com/cs3org/reva/pull/2370

internal/grpc/services/gateway/storageprovider.go

@@ -948,8 +948,15 @@ func (s *svc) ListContainer(ctx context.Context, req *provider.ListContainerRequ
 				}
 
 				if utils.IsAbsoluteReference(req.Ref) {
+					var prefix string
+					if utils.IsAbsolutePathReference(providerRef) {
+						prefix = mountPath
+					} else {
+						prefix = path.Join(mountPath, providerRef.Path)
+					}
 					for j := range rsp.Infos {
-						rsp.Infos[j].Path = path.Join(mountPath, providerRef.Path, rsp.Infos[j].Path)
+
+						rsp.Infos[j].Path = path.Join(prefix, rsp.Infos[j].Path)
 					}
 				}
 				for i := range rsp.Infos {
@@ -1597,18 +1604,18 @@ func (s *svc) findProviders(ctx context.Context, ref *provider.Reference) ([]*re
 	return res.Providers, nil
 }
 
-// unwrap takes a reference and makes it relative to the given mountPoint, optionally
+// unwrap takes a reference and builds a reference for the provider. can be absolute or relative to a root node
 func unwrap(ref *provider.Reference, mountPoint string, root *provider.ResourceId) *provider.Reference {
 	if utils.IsAbsolutePathReference(ref) {
-		relativeRef := &provider.Reference{
+		providerRef := &provider.Reference{
 			Path: strings.TrimPrefix(ref.Path, mountPoint),
 		}
 		// if we have a root use it and make the path relative
 		if root != nil {
-			relativeRef.ResourceId = root
-			relativeRef.Path = utils.MakeRelativePath(relativeRef.Path)
+			providerRef.ResourceId = root
+			providerRef.Path = utils.MakeRelativePath(providerRef.Path)
 		}
-		return relativeRef
+		return providerRef
 	}
 	// build a copy to avoid side effects
 	return &provider.Reference{

pkg/storage/fs/local/local.go

@@ -33,6 +33,8 @@ func init() {
 type config struct {
 	Root        string `mapstructure:"root" docs:"/var/tmp/reva/;Path of root directory for user storage."`
 	ShareFolder string `mapstructure:"share_folder" docs:"/MyShares;Path for storing share references."`
+	UserLayout  string `mapstructure:"user_layout" docs:"{{.Username}};Template used for building the user's root path."`
+	DisableHome bool   `mapstructure:"disable_home" docs:"false;Enable/disable special /home handling."`
 }
 
 func parseConfig(m map[string]interface{}) (*config, error) {
@@ -55,7 +57,8 @@ func New(m map[string]interface{}) (storage.FS, error) {
 	conf := localfs.Config{
 		Root:        c.Root,
 		ShareFolder: c.ShareFolder,
-		DisableHome: true,
+		UserLayout:  c.UserLayout,
+		DisableHome: c.DisableHome,
 	}
 	return localfs.NewLocalFS(&conf)
 }

pkg/storage/registry/static/static.go

@@ -192,9 +192,12 @@ func (b *reg) ListProviders(ctx context.Context, filters map[string]string) ([]*
 				}
 				match = &registrypb.ProviderInfo{
 					ProviderId:   id,
-					ProviderPath: m,
+					ProviderPath: rule.ProviderPath,
 					Address:      addr,
 				}
+				if match.ProviderPath == "" {
+					match.ProviderPath = m
+				}
 			}
 			// Check if the current rule forms a part of a reference spread across storage providers.
 			if strings.HasPrefix(prefix, fn) {

pkg/storage/utils/decomposedfs/decomposedfs.go

@@ -283,15 +283,18 @@ func (fs *Decomposedfs) CreateDir(ctx context.Context, ref *provider.Reference)
 		return errtypes.BadRequest("Invalid path: " + ref.Path)
 	}
 
-	ref.Path = path.Dir(ref.Path)
+	parentRef := &provider.Reference{
+		ResourceId: ref.ResourceId,
+		Path:       path.Dir(ref.Path),
+	}
 
 	// verify parent exists
 	var n *node.Node
-	if n, err = fs.lu.NodeFromResource(ctx, ref); err != nil {
+	if n, err = fs.lu.NodeFromResource(ctx, parentRef); err != nil {
 		return
 	}
 	if !n.Exists {
-		return errtypes.NotFound(ref.Path)
+		return errtypes.NotFound(parentRef.Path)
 	}
 
 	ok, err := fs.p.HasPermission(ctx, n, func(rp *provider.ResourcePermissions) bool {
@@ -309,7 +312,7 @@ func (fs *Decomposedfs) CreateDir(ctx context.Context, ref *provider.Reference)
 		return
 	}
 	if n.Exists {
-		return errtypes.AlreadyExists(ref.Path)
+		return errtypes.AlreadyExists(parentRef.Path)
 	}
 
 	if err = fs.tp.CreateDir(ctx, n); err != nil {

pkg/storage/utils/localfs/localfs.go

@@ -432,7 +432,11 @@ func (fs *localfs) GetPathByID(ctx context.Context, ref *provider.ResourceId) (s
 			return "", err
 		}
 	}
-	return url.QueryUnescape(strings.TrimPrefix(ref.OpaqueId, "fileid-"+layout))
+	unescapedID, err := url.QueryUnescape(ref.OpaqueId)
+	if err != nil {
+		return "", err
+	}
+	return strings.TrimPrefix(unescapedID, "fileid-"+layout), nil
 }
 
 func (fs *localfs) DenyGrant(ctx context.Context, ref *provider.Reference, g *provider.Grantee) error {

pkg/storage/utils/localfs/localfs_unix.go

@@ -43,7 +43,7 @@ import (
 func calcEtag(ctx context.Context, fi os.FileInfo) string {
 	log := appctx.GetLogger(ctx)
 	h := md5.New()
-	err := binary.Write(h, binary.BigEndian, fi.ModTime().Unix())
+	err := binary.Write(h, binary.BigEndian, fi.ModTime().UnixNano())
 	if err != nil {
 		log.Error().Err(err).Msg("error writing mtime")
 	}

tests/integration/grpc/fixtures/gateway-static.toml

@@ -24,10 +24,11 @@ home_provider = "/home"
 
 [grpc.services.storageregistry.drivers.static.rules]
 "/home" = {"mapping" = "/home-{{substr 0 1 .Id.OpaqueId}}", "aliases" = {"/home-[0-9a-e]" = {"address" = "{{storage_address}}", "provider_id" = "{{storage_id}}"}, "/home-[f-z]" = {"address" = "{{storage2_address}}", "provider_id" = "{{storage2_id}}"}}}
+
+"/users/[0-9a-e]" = {address = "{{storage_address}}", "provider_id" = "{{storage_id}}", "provider_path" = "/users"}
 "{{storage_id}}" = {address = "{{storage_address}}", "provider_id" = "{{storage_id}}"}
+"/users/[f-z]" = {address = "{{storage2_address}}", "provider_id" = "{{storage2_id}}", "provider_path" = "/users"}
 "{{storage2_id}}" = {address = "{{storage2_address}}", "provider_id" = "{{storage2_id}}"}
-"/users/[0-9a-e]" = {address = "{{storage_address}}", "provider_id" = "{{storage_id}}"}
-"/users/[f-z]" = {address = "{{storage2_address}}", "provider_id" = "{{storage2_id}}"}
 
 [http]
 address = "{{grpc_address+1}}"

tests/integration/grpc/fixtures/storageprovider-local.toml

@@ -0,0 +1,10 @@
+[grpc]
+address = "{{grpc_address}}"
+
+[grpc.services.storageprovider]
+driver = "local"
+
+[grpc.services.storageprovider.drivers.local]
+disable_home = {{disable_home}}
+root = "{{root}}/storage"
+user_layout = "{{substr 0 1 .Id.OpaqueId}}/{{.Id.OpaqueId}}"

tests/integration/grpc/gateway_storageprovider_static_test.go

@@ -76,8 +76,8 @@ var _ = Describe("gateway using a static registry and a shard setup", func() {
 		dependencies = map[string]string{
 			"gateway":  "gateway-static.toml",
 			"users":    "userprovider-json.toml",
-			"storage":  "storageprovider-owncloud.toml",
-			"storage2": "storageprovider-owncloud.toml",
+			"storage":  "storageprovider-local.toml",
+			"storage2": "storageprovider-local.toml",
 		}
 		redisAddress := os.Getenv("REDIS_ADDRESS")
 		if redisAddress == "" {
@@ -124,9 +124,9 @@ var _ = Describe("gateway using a static registry and a shard setup", func() {
 		}
 	})
 
-	Context("with a home jail", func() {
+	Context("with a mapping based home jail", func() {
 		BeforeEach(func() {
-			variables["enable_home"] = "true"
+			variables["disable_home"] = "false"
 		})
 
 		It("creates a home directory on the correct provider", func() {
@@ -136,18 +136,18 @@ var _ = Describe("gateway using a static registry and a shard setup", func() {
 			Expect(statRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_NOT_FOUND))
 
 			res, err := serviceClient.CreateHome(marieCtx, &storagep.CreateHomeRequest{})
-			Expect(err).ToNot(HaveOccurred())
 			Expect(res.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+			Expect(err).ToNot(HaveOccurred())
 
 			statRes, err = serviceClient.Stat(marieCtx, &storagep.StatRequest{Ref: homeRef})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(statRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
 
 			// the mapping considers the opaque id: f... -> storage2
-			fi, err := os.Stat(path.Join(revads["storage2"].StorageRoot, marie.Id.OpaqueId, "files"))
+			fi, err := os.Stat(path.Join(revads["storage2"].StorageRoot, "data/f", marie.Id.OpaqueId))
 			Expect(err).ToNot(HaveOccurred())
 			Expect(fi.IsDir()).To(BeTrue())
-			_, err = os.Stat(path.Join(revads["storage"].StorageRoot, marie.Id.OpaqueId, "files"))
+			_, err = os.Stat(path.Join(revads["storage"].StorageRoot, "data/f", marie.Id.OpaqueId))
 			Expect(err).To(HaveOccurred())
 
 			ghRes, err := serviceClient.GetHome(marieCtx, &storagep.GetHomeRequest{})
@@ -168,10 +168,10 @@ var _ = Describe("gateway using a static registry and a shard setup", func() {
 			Expect(statRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
 
 			// the mapping considers the opaque id: e... -> storage
-			fi, err = os.Stat(path.Join(revads["storage"].StorageRoot, einstein.Id.OpaqueId, "files"))
+			fi, err = os.Stat(path.Join(revads["storage"].StorageRoot, "data/e", einstein.Id.OpaqueId))
 			Expect(err).ToNot(HaveOccurred())
 			Expect(fi.IsDir()).To(BeTrue())
-			_, err = os.Stat(path.Join(revads["storage2"].StorageRoot, einstein.Id.OpaqueId, "files"))
+			_, err = os.Stat(path.Join(revads["storage2"].StorageRoot, "data/e", einstein.Id.OpaqueId))
 			Expect(err).To(HaveOccurred())
 
 			ghRes, err = serviceClient.GetHome(einsteinCtx, &storagep.GetHomeRequest{})
@@ -188,6 +188,13 @@ var _ = Describe("gateway using a static registry and a shard setup", func() {
 
 			It("creates and lists a new directory", func() {
 				newRef := &storagep.Reference{Path: "/home/newdir"}
+
+				listRes, err := serviceClient.ListContainer(marieCtx, &storagep.ListContainerRequest{Ref: homeRef})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(listRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+				Expect(len(listRes.Infos)).To(Equal(1))
+				Expect(listRes.Infos[0].Path).To(Equal("/home/MyShares"))
+
 				statRes, err := serviceClient.Stat(marieCtx, &storagep.StatRequest{Ref: newRef})
 				Expect(err).ToNot(HaveOccurred())
 				Expect(statRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_NOT_FOUND))
@@ -200,11 +207,15 @@ var _ = Describe("gateway using a static registry and a shard setup", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(statRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
 
-				listRes, err := serviceClient.ListContainer(marieCtx, &storagep.ListContainerRequest{Ref: homeRef})
+				listRes, err = serviceClient.ListContainer(marieCtx, &storagep.ListContainerRequest{Ref: homeRef})
 				Expect(err).ToNot(HaveOccurred())
 				Expect(listRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
-				Expect(len(listRes.Infos)).To(Equal(1))
-				Expect(listRes.Infos[0].Path).To(Equal(newRef.Path))
+				Expect(len(listRes.Infos)).To(Equal(2))
+				paths := []string{}
+				for _, i := range listRes.Infos {
+					paths = append(paths, i.Path)
+				}
+				Expect(paths).To(ConsistOf("/home/MyShares", newRef.Path))
 
 				listRes, err = serviceClient.ListContainer(marieCtx, &storagep.ListContainerRequest{Ref: newRef})
 				Expect(err).ToNot(HaveOccurred())
@@ -266,9 +277,17 @@ var _ = Describe("gateway using a static registry and a shard setup", func() {
 		})
 	})
 
-	Context("without home jail", func() {
+	Context("with a sharded /users mount", func() {
+		var (
+			homePath  = "/users/f/f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"
+			rootRef   = &storagep.Reference{Path: path.Join("/users")}
+			baseRef   = &storagep.Reference{Path: path.Join("/users/f")}
+			homeRef   = &storagep.Reference{Path: homePath}
+			subdirRef = &storagep.Reference{Path: path.Join(homePath, "subdir")}
+		)
+
 		BeforeEach(func() {
-			variables["enable_home"] = "false"
+			variables["disable_home"] = "true"
 		})
 
 		It("merges the results of both /users providers", func() {
@@ -282,15 +301,125 @@ var _ = Describe("gateway using a static registry and a shard setup", func() {
 			Expect(lRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
 			Expect(len(lRes.Infos)).To(Equal(0))
 
-			res, err := serviceClient.CreateHome(marieCtx, &storagep.CreateHomeRequest{})
+			res, err := serviceClient.CreateContainer(einsteinCtx, &storagep.CreateContainerRequest{
+				Ref: &storagep.Reference{
+					Path: path.Join("/users/e"),
+				},
+			})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(res.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+
+			res, err = serviceClient.CreateContainer(einsteinCtx, &storagep.CreateContainerRequest{
+				Ref: &storagep.Reference{
+					Path: path.Join("/users/e", einstein.Id.OpaqueId),
+				},
+			})
+			Expect(err).ToNot(HaveOccurred())
 			Expect(res.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+
+			lRes, err = serviceClient.ListContainer(einsteinCtx, &storagep.ListContainerRequest{Ref: &storagep.Reference{Path: "/users/e"}})
 			Expect(err).ToNot(HaveOccurred())
+			Expect(lRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+			Expect(len(lRes.Infos)).To(Equal(1))
+			Expect(lRes.Infos[0].Path).To(Equal("/users/e/e4fb0282-fabf-4cff-b1ee-90bdc01c4eef"))
+
+			lRes, err = serviceClient.ListContainer(einsteinCtx, &storagep.ListContainerRequest{Ref: &storagep.Reference{Path: "/users/d"}})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(lRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+			Expect(len(lRes.Infos)).To(Equal(0))
+
+			res, err = serviceClient.CreateContainer(einsteinCtx, &storagep.CreateContainerRequest{
+				Ref: &storagep.Reference{
+					Path: path.Join("/users/f"),
+				},
+			})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(res.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+
+			res, err = serviceClient.CreateContainer(marieCtx, &storagep.CreateContainerRequest{
+				Ref: &storagep.Reference{
+					Path: path.Join("/users/f", marie.Id.OpaqueId),
+				},
+			})
+			Expect(err).ToNot(HaveOccurred())
+			Expect(res.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
 
 			lRes, err = serviceClient.ListContainer(marieCtx, &storagep.ListContainerRequest{Ref: &storagep.Reference{Path: "/users/f"}})
 			Expect(err).ToNot(HaveOccurred())
 			Expect(lRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
 			Expect(len(lRes.Infos)).To(Equal(1))
 			Expect(lRes.Infos[0].Path).To(Equal("/users/f/f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c"))
 		})
+
+		Context("with a user home", func() {
+			JustBeforeEach(func() {
+				res, err := serviceClient.CreateContainer(marieCtx, &storagep.CreateContainerRequest{
+					Ref: &storagep.Reference{
+						Path: path.Join("/users/f"),
+					},
+				})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(res.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+				res, err = serviceClient.CreateContainer(marieCtx, &storagep.CreateContainerRequest{
+					Ref: &storagep.Reference{
+						Path: path.Join("/users/f", marie.Id.OpaqueId),
+					},
+				})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(res.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+			})
+
+			It("provides access to the user home", func() {
+				newRef := &storagep.Reference{Path: path.Join(homePath, "newName")}
+
+				createRes, err := serviceClient.CreateContainer(marieCtx, &storagep.CreateContainerRequest{Ref: subdirRef})
+				Expect(createRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+				Expect(err).ToNot(HaveOccurred())
+
+				statRes, err := serviceClient.Stat(marieCtx, &storagep.StatRequest{Ref: homeRef})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(statRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+				Expect(statRes.Info.Path).To(Equal(homePath))
+
+				lRes, err := serviceClient.ListContainer(marieCtx, &storagep.ListContainerRequest{Ref: homeRef})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(lRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+				Expect(len(lRes.Infos)).To(Equal(1))
+				Expect(lRes.Infos[0].Path).To(Equal(subdirRef.Path))
+
+				mRes, err := serviceClient.Move(marieCtx, &storagep.MoveRequest{Source: subdirRef, Destination: newRef})
+				Expect(mRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+				Expect(err).ToNot(HaveOccurred())
+
+				dRes, err := serviceClient.Delete(marieCtx, &storagep.DeleteRequest{Ref: newRef})
+				Expect(dRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+				Expect(err).ToNot(HaveOccurred())
+
+				statRes, err = serviceClient.Stat(marieCtx, &storagep.StatRequest{Ref: newRef})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(statRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_NOT_FOUND))
+			})
+
+			It("propagates the etag to the root", func() {
+				getEtag := func(r *storagep.Reference) string {
+					statRes, err := serviceClient.Stat(marieCtx, &storagep.StatRequest{Ref: r})
+					Expect(err).ToNot(HaveOccurred())
+					Expect(statRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+					return statRes.Info.Etag
+				}
+
+				rootEtag := getEtag(rootRef)
+				baseEtag := getEtag(baseRef)
+				userEtag := getEtag(homeRef)
+
+				createRes, err := serviceClient.CreateContainer(marieCtx, &storagep.CreateContainerRequest{Ref: subdirRef})
+				Expect(err).ToNot(HaveOccurred())
+				Expect(createRes.Status.Code).To(Equal(rpcv1beta1.Code_CODE_OK))
+
+				Expect(getEtag(homeRef)).ToNot(Equal(userEtag))
+				Expect(getEtag(baseRef)).ToNot(Equal(baseEtag))
+				Expect(getEtag(rootRef)).ToNot(Equal(rootEtag))
+			})
+		})
 	})
 })