From e670642a8d5a61316a3f396dafccc21689a643ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 8 Dec 2021 07:54:43 +0000 Subject: [PATCH 1/8] spaces-registry: rename rule to provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/storage/registry/spaces/spaces.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go index a73277c01a..e17118036f 100644 --- a/pkg/storage/registry/spaces/spaces.go +++ b/pkg/storage/registry/spaces/spaces.go @@ -103,22 +103,22 @@ func (c *config) init() { } } - // cleanup rule paths - for _, rule := range c.Providers { + // cleanup provider paths + for _, provider := range c.Providers { // if the path template is not explicitly set use the mountpath as path template - if rule.PathTemplate == "" && strings.HasPrefix(rule.MountPath, "/") { + if provider.PathTemplate == "" && strings.HasPrefix(provider.MountPath, "/") { // TODO err if the path is a regex - rule.PathTemplate = rule.MountPath + provider.PathTemplate = provider.MountPath } // cleanup path template - rule.PathTemplate = filepath.Clean(rule.PathTemplate) + provider.PathTemplate = filepath.Clean(provider.PathTemplate) // compile given template tpl var err error - rule.template, err = template.New("path_template").Funcs(sprig.TxtFuncMap()).Parse(rule.PathTemplate) + provider.template, err = template.New("path_template").Funcs(sprig.TxtFuncMap()).Parse(provider.PathTemplate) if err != nil { - logger.New().Fatal().Err(err).Interface("rule", rule).Msg("error parsing template") + logger.New().Fatal().Err(err).Interface("provider", provider).Msg("error parsing template") } // TODO connect to provider, (List Spaces,) ListContainerStream From 0933eb8a41a66015e1202d36dc382e37ebf85f7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 8 Dec 2021 08:30:58 +0000 Subject: [PATCH 2/8] remove alias config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/storage/registry/spaces/spaces.go | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go index e17118036f..2f727ebb02 100644 --- a/pkg/storage/registry/spaces/spaces.go +++ b/pkg/storage/registry/spaces/spaces.go @@ -53,11 +53,10 @@ func init() { } type provider struct { - Mapping string `mapstructure:"mapping"` - MountPath string `mapstructure:"mount_path"` - Aliases map[string]string `mapstructure:"aliases"` - AllowedUserAgents []string `mapstructure:"allowed_user_agents"` - PathTemplate string `mapstructure:"path_template"` + Mapping string `mapstructure:"mapping"` + MountPath string `mapstructure:"mount_path"` + AllowedUserAgents []string `mapstructure:"allowed_user_agents"` + PathTemplate string `mapstructure:"path_template"` template *template.Template // filters SpaceType string `mapstructure:"space_type"` @@ -142,9 +141,8 @@ func New(m map[string]interface{}, getClientFunc GetStorageProviderServiceClient } c.init() r := ®istry{ - c: c, - resources: make(map[string][]*registrypb.ProviderInfo), - //aliases: make(map[string]map[string]*spaceAndProvider), + c: c, + resources: make(map[string][]*registrypb.ProviderInfo), resourceNameCache: make(map[string]string), getStorageProviderServiceClient: getClientFunc, } @@ -172,9 +170,7 @@ type registry struct { // the template to use when determining the home provider homeTemplate *template.Template // a map of resources to providers - resources map[string][]*registrypb.ProviderInfo - // a map of paths/aliases to spaces and providers - // aliases map[string]map[string]*spaceAndProvider + resources map[string][]*registrypb.ProviderInfo resourceNameCache map[string]string getStorageProviderServiceClient GetStorageProviderServiceClientFunc From 3f7c9e97c7012c245ae77496eb9d3e29b20ab3fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 8 Dec 2021 10:18:14 +0000 Subject: [PATCH 3/8] multiple spaces per provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../multiple-spaces-per-provider.md | 5 + pkg/storage/registry/spaces/spaces.go | 230 ++++++++++-------- pkg/storage/registry/spaces/spaces_test.go | 98 +++++--- tests/oc-integration-tests/drone/gateway.toml | 10 +- tests/oc-integration-tests/local/gateway.toml | 10 +- 5 files changed, 207 insertions(+), 146 deletions(-) create mode 100644 changelog/unreleased/multiple-spaces-per-provider.md diff --git a/changelog/unreleased/multiple-spaces-per-provider.md b/changelog/unreleased/multiple-spaces-per-provider.md new file mode 100644 index 0000000000..90801755c7 --- /dev/null +++ b/changelog/unreleased/multiple-spaces-per-provider.md @@ -0,0 +1,5 @@ +Change: Allow multiple space configurations per provider + +The spaces registry can now use multiple space configurations to allow personal and project spaces on the same provider + +https://github.com/cs3org/reva/pull/2340 diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go index 2f727ebb02..653ce51774 100644 --- a/pkg/storage/registry/spaces/spaces.go +++ b/pkg/storage/registry/spaces/spaces.go @@ -52,16 +52,31 @@ func init() { pkgregistry.Register("spaces", NewDefault) } -type provider struct { - Mapping string `mapstructure:"mapping"` - MountPath string `mapstructure:"mount_path"` - AllowedUserAgents []string `mapstructure:"allowed_user_agents"` - PathTemplate string `mapstructure:"path_template"` - template *template.Template +type spaceConfig struct { + // MountPoint determines where a space is mounted. Can be a regex + // It is used to determine which storage provider is responsible when only a path is given in the request + MountPoint string `mapstructure:"mount_point"` + // PathTemplate is used to build the path of an individual space. Layouts can access {{.Space...}} and {{.CurrentUser...}} + PathTemplate string `mapstructure:"path_template"` + template *template.Template // filters - SpaceType string `mapstructure:"space_type"` - SpaceOwnerSelf bool `mapstructure:"space_owner_self"` - SpaceID string `mapstructure:"space_id"` + OwnerIsCurrentUser bool `mapstructure:"owner_is_current_user"` + ID string `mapstructure:"id"` + // TODO description? +} + +// SpacePath generates a layout based on space data. +func (sc *spaceConfig) SpacePath(u *userpb.User, space *providerpb.StorageSpace) (string, error) { + b := bytes.Buffer{} + if err := sc.template.Execute(&b, templateData{CurrentUser: u, Space: space}); err != nil { + return "", err + } + return b.String(), nil +} + +type provider struct { + // Spaces is a map from space type to space config + Spaces map[string]*spaceConfig `mapstructure:"spaces"` } type templateData struct { @@ -74,15 +89,6 @@ type StorageProviderClient interface { ListStorageSpaces(ctx context.Context, in *providerpb.ListStorageSpacesRequest, opts ...grpc.CallOption) (*providerpb.ListStorageSpacesResponse, error) } -// WithSpace generates a layout based on space data. -func (p *provider) ProviderPath(u *userpb.User, s *providerpb.StorageSpace) (string, error) { - b := bytes.Buffer{} - if err := p.template.Execute(&b, templateData{CurrentUser: u, Space: s}); err != nil { - return "", err - } - return b.String(), nil -} - type config struct { Providers map[string]*provider `mapstructure:"providers"` HomeTemplate string `mapstructure:"home_template"` @@ -97,27 +103,35 @@ func (c *config) init() { if len(c.Providers) == 0 { c.Providers = map[string]*provider{ sharedconf.GetGatewaySVC(""): { - MountPath: "/", + Spaces: map[string]*spaceConfig{ + "personal": {MountPoint: "/users", PathTemplate: "/users/{{.Space.Owner.Id.OpaqueId}}"}, + "project": {MountPoint: "/projects", PathTemplate: "/projects/{{.Space.Name}}"}, + "share": {MountPoint: "/users/{{.CurrentUser.Id.OpaqueId}}/Shares", PathTemplate: "/users/{{.CurrentUser.Id.OpaqueId}}/Shares/{{.Space.Name}}"}, + "public": {MountPoint: "/public"}, + }, }, } } - // cleanup provider paths + // cleanup space paths for _, provider := range c.Providers { - // if the path template is not explicitly set use the mountpath as path template - if provider.PathTemplate == "" && strings.HasPrefix(provider.MountPath, "/") { - // TODO err if the path is a regex - provider.PathTemplate = provider.MountPath - } + for _, space := range provider.Spaces { - // cleanup path template - provider.PathTemplate = filepath.Clean(provider.PathTemplate) + // if the path template is not explicitly set use the mountpath as path template + if space.PathTemplate == "" && strings.HasPrefix(space.MountPoint, "/") { + // TODO err if the path is a regex + space.PathTemplate = space.MountPoint + } - // compile given template tpl - var err error - provider.template, err = template.New("path_template").Funcs(sprig.TxtFuncMap()).Parse(provider.PathTemplate) - if err != nil { - logger.New().Fatal().Err(err).Interface("provider", provider).Msg("error parsing template") + // cleanup path templates + space.PathTemplate = filepath.Join("/", space.PathTemplate) + + // compile given template tpl + var err error + space.template, err = template.New("path_template").Funcs(sprig.TxtFuncMap()).Parse(space.PathTemplate) + if err != nil { + logger.New().Fatal().Err(err).Interface("space", space).Msg("error parsing template") + } } // TODO connect to provider, (List Spaces,) ListContainerStream @@ -178,33 +192,35 @@ type registry struct { // GetProvider return the storage provider for the given spaces according to the rule configuration func (r *registry) GetProvider(ctx context.Context, space *providerpb.StorageSpace) (*registrypb.ProviderInfo, error) { - for address, rule := range r.c.Providers { - mountPath := "" - var err error - if space.SpaceType != "" && rule.SpaceType != space.SpaceType { - continue - } - if space.Owner != nil { - mountPath, err = rule.ProviderPath(nil, space) - if err != nil { + for address, provider := range r.c.Providers { + for spaceType, sc := range provider.Spaces { + spacePath := "" + var err error + if space.SpaceType != "" && spaceType != space.SpaceType { continue } - match, err := regexp.MatchString(rule.MountPath, mountPath) - if err != nil { - continue + if space.Owner != nil { + spacePath, err = sc.SpacePath(nil, space) + if err != nil { + continue + } + match, err := regexp.MatchString(sc.MountPoint, spacePath) + if err != nil { + continue + } + if !match { + continue + } } - if !match { + pi := ®istrypb.ProviderInfo{Address: address} + opaque, err := spacePathsToOpaque(map[string]string{"unused": spacePath}) + if err != nil { + appctx.GetLogger(ctx).Debug().Err(err).Msg("marshaling space paths map failed, continuing") continue } + pi.Opaque = opaque + return pi, nil // return the first match we find } - pi := ®istrypb.ProviderInfo{Address: address} - opaque, err := spacePathsToOpaque(map[string]string{"unused": mountPath}) - if err != nil { - appctx.GetLogger(ctx).Debug().Err(err).Msg("marshaling space paths map failed, continuing") - continue - } - pi.Opaque = opaque - return pi, nil } return nil, errtypes.NotFound("no provider found for space") } @@ -265,52 +281,56 @@ func (r *registry) ListProviders(ctx context.Context, filters map[string]string) // for share spaces the res.StorageId tells the registry the spaceid and res.OpaqueId is a node in that space func (r *registry) findProvidersForResource(ctx context.Context, id string) []*registrypb.ProviderInfo { currentUser := ctxpkg.ContextMustGetUser(ctx) - for address, rule := range r.c.Providers { + for address, provider := range r.c.Providers { p := ®istrypb.ProviderInfo{ Address: address, ProviderId: id, } - filters := []*providerpb.ListStorageSpacesRequest_Filter{} - if rule.SpaceType != "" { + filters := []*providerpb.ListStorageSpacesRequest_Filter{{ + Type: providerpb.ListStorageSpacesRequest_Filter_TYPE_ID, + Term: &providerpb.ListStorageSpacesRequest_Filter_Id{ + Id: &providerpb.StorageSpaceId{ + OpaqueId: id, + }, + }, + }} + for spaceType, _ := range provider.Spaces { // add filter to id based request if it is configured filters = append(filters, &providerpb.ListStorageSpacesRequest_Filter{ Type: providerpb.ListStorageSpacesRequest_Filter_TYPE_SPACE_TYPE, Term: &providerpb.ListStorageSpacesRequest_Filter_SpaceType{ - SpaceType: rule.SpaceType, + SpaceType: spaceType, }, }) } - filters = append(filters, &providerpb.ListStorageSpacesRequest_Filter{ - Type: providerpb.ListStorageSpacesRequest_Filter_TYPE_ID, - Term: &providerpb.ListStorageSpacesRequest_Filter_Id{ - Id: &providerpb.StorageSpaceId{ - OpaqueId: id, - }, - }, - }) spaces, err := r.findStorageSpaceOnProvider(ctx, address, filters) if err != nil { - appctx.GetLogger(ctx).Debug().Err(err).Interface("rule", rule).Msg("findStorageSpaceOnProvider by id failed, continuing") + appctx.GetLogger(ctx).Debug().Err(err).Interface("provider", provider).Msg("findStorageSpaceOnProvider by id failed, continuing") continue } if len(spaces) > 0 { - space := spaces[0] // there shouldn't be multiple - providerPath, err := rule.ProviderPath(currentUser, space) - if err != nil { - appctx.GetLogger(ctx).Error().Err(err).Interface("rule", rule).Interface("space", space).Msg("failed to execute template, continuing") - continue + space := spaces[0] // there should not be multiple per provider + + for spaceType, sc := range provider.Spaces { + if spaceType == space.SpaceType { + providerPath, err := sc.SpacePath(currentUser, space) + if err != nil { + appctx.GetLogger(ctx).Error().Err(err).Interface("provider", provider).Interface("space", space).Msg("failed to execute template, continuing") + continue + } + spacePaths := map[string]string{ + space.Id.OpaqueId: providerPath, + } + p.Opaque, err = spacePathsToOpaque(spacePaths) + if err != nil { + appctx.GetLogger(ctx).Debug().Err(err).Msg("marshaling space paths map failed, continuing") + continue + } + return []*registrypb.ProviderInfo{p} + } } - spacePaths := map[string]string{ - space.Id.OpaqueId: providerPath, - } - p.Opaque, err = spacePathsToOpaque(spacePaths) - if err != nil { - appctx.GetLogger(ctx).Debug().Err(err).Msg("marshaling space paths map failed, continuing") - continue - } - return []*registrypb.ProviderInfo{p} } } return []*registrypb.ProviderInfo{} @@ -325,49 +345,55 @@ func (r *registry) findProvidersForAbsolutePathReference(ctx context.Context, pa var deepestMountSpace *providerpb.StorageSpace var deepestMountPathProvider *registrypb.ProviderInfo providers := map[string]map[string]string{} - for address, rule := range r.c.Providers { + for address, provider := range r.c.Providers { p := ®istrypb.ProviderInfo{ Address: address, } var spaces []*providerpb.StorageSpace var err error filters := []*providerpb.ListStorageSpacesRequest_Filter{} - if rule.SpaceOwnerSelf { - filters = append(filters, &providerpb.ListStorageSpacesRequest_Filter{ - Type: providerpb.ListStorageSpacesRequest_Filter_TYPE_OWNER, - Term: &providerpb.ListStorageSpacesRequest_Filter_Owner{ - Owner: currentUser.Id, - }, - }) - } - if rule.SpaceType != "" { + for spaceType, sc := range provider.Spaces { + filters = append(filters, &providerpb.ListStorageSpacesRequest_Filter{ Type: providerpb.ListStorageSpacesRequest_Filter_TYPE_SPACE_TYPE, Term: &providerpb.ListStorageSpacesRequest_Filter_SpaceType{ - SpaceType: rule.SpaceType, - }, - }) - } - if rule.SpaceID != "" { - filters = append(filters, &providerpb.ListStorageSpacesRequest_Filter{ - Type: providerpb.ListStorageSpacesRequest_Filter_TYPE_ID, - Term: &providerpb.ListStorageSpacesRequest_Filter_Id{ - Id: &providerpb.StorageSpaceId{OpaqueId: rule.SpaceID}, + SpaceType: spaceType, }, }) + if sc.OwnerIsCurrentUser { + filters = append(filters, &providerpb.ListStorageSpacesRequest_Filter{ + Type: providerpb.ListStorageSpacesRequest_Filter_TYPE_OWNER, + Term: &providerpb.ListStorageSpacesRequest_Filter_Owner{ + Owner: currentUser.Id, + }, + }) + } + if sc.ID != "" { + filters = append(filters, &providerpb.ListStorageSpacesRequest_Filter{ + Type: providerpb.ListStorageSpacesRequest_Filter_TYPE_ID, + Term: &providerpb.ListStorageSpacesRequest_Filter_Id{ + Id: &providerpb.StorageSpaceId{OpaqueId: sc.ID}, + }, + }) + } } spaces, err = r.findStorageSpaceOnProvider(ctx, p.Address, filters) if err != nil { - appctx.GetLogger(ctx).Debug().Err(err).Interface("rule", rule).Msg("findStorageSpaceOnProvider failed, continuing") + appctx.GetLogger(ctx).Debug().Err(err).Interface("provider", provider).Msg("findStorageSpaceOnProvider failed, continuing") continue } spacePaths := map[string]string{} for _, space := range spaces { - spacePath, err := rule.ProviderPath(currentUser, space) + var sc *spaceConfig + var ok bool + if sc, ok = provider.Spaces[space.SpaceType]; !ok { + continue + } + spacePath, err := sc.SpacePath(currentUser, space) if err != nil { - appctx.GetLogger(ctx).Error().Err(err).Interface("rule", rule).Interface("space", space).Msg("failed to execute template, continuing") + appctx.GetLogger(ctx).Error().Err(err).Interface("provider", provider).Interface("space", space).Msg("failed to execute template, continuing") continue } diff --git a/pkg/storage/registry/spaces/spaces_test.go b/pkg/storage/registry/spaces/spaces_test.go index 74c48db51e..0736687dd0 100644 --- a/pkg/storage/registry/spaces/spaces_test.go +++ b/pkg/storage/registry/spaces/spaces_test.go @@ -66,10 +66,11 @@ var _ = Describe("Static", func() { func(_ context.Context, req *provider.ListStorageSpacesRequest, _ ...grpc.CallOption) *provider.ListStorageSpacesResponse { spaces := []*provider.StorageSpace{ { - Id: &provider.StorageSpaceId{OpaqueId: "foospace"}, - Root: &provider.ResourceId{StorageId: "foospace", OpaqueId: "foospace"}, - Name: "Foo space", - Owner: alice, + Id: &provider.StorageSpaceId{OpaqueId: "foospace"}, + Root: &provider.ResourceId{StorageId: "foospace", OpaqueId: "foospace"}, + Name: "Foo space", + SpaceType: "personal", + Owner: alice, }, } for _, f := range req.Filters { @@ -86,10 +87,11 @@ var _ = Describe("Static", func() { func(_ context.Context, req *provider.ListStorageSpacesRequest, _ ...grpc.CallOption) *provider.ListStorageSpacesResponse { spaces := []*provider.StorageSpace{ { - Id: &provider.StorageSpaceId{OpaqueId: "barspace"}, - Root: &provider.ResourceId{StorageId: "barspace", OpaqueId: "barspace"}, - Name: "Bar space", - Owner: alice, + Id: &provider.StorageSpaceId{OpaqueId: "barspace"}, + Root: &provider.ResourceId{StorageId: "barspace", OpaqueId: "barspace"}, + Name: "Bar space", + SpaceType: "personal", + Owner: alice, }, } for _, f := range req.Filters { @@ -105,16 +107,18 @@ var _ = Describe("Static", func() { bazClient.On("ListStorageSpaces", mock.Anything, mock.Anything).Return( func(_ context.Context, req *provider.ListStorageSpacesRequest, _ ...grpc.CallOption) *provider.ListStorageSpacesResponse { space1 := &provider.StorageSpace{ - Id: &provider.StorageSpaceId{OpaqueId: "bazspace1"}, - Root: &provider.ResourceId{StorageId: "bazspace1", OpaqueId: "bazspace1"}, - Name: "Baz space 1", - Owner: alice, + Id: &provider.StorageSpaceId{OpaqueId: "bazspace1"}, + Root: &provider.ResourceId{StorageId: "bazspace1", OpaqueId: "bazspace1"}, + Name: "Baz space 1", + SpaceType: "project", + Owner: alice, } space2 := &provider.StorageSpace{ - Id: &provider.StorageSpaceId{OpaqueId: "bazspace2"}, - Root: &provider.ResourceId{StorageId: "bazspace2", OpaqueId: "bazspace2"}, - Name: "Baz space 2", - Owner: alice, + Id: &provider.StorageSpaceId{OpaqueId: "bazspace2"}, + Root: &provider.ResourceId{StorageId: "bazspace2", OpaqueId: "bazspace2"}, + Name: "Baz space 2", + SpaceType: "project", + Owner: alice, } spaces := []*provider.StorageSpace{space1, space2} for _, f := range req.Filters { @@ -167,8 +171,12 @@ var _ = Describe("Static", func() { rules = map[string]interface{}{ "providers": map[string]interface{}{ "127.0.0.1:13020": map[string]interface{}{ - "space_type": "personal", - "mount_path": "/thepath"}, + "spaces": map[string]interface{}{ + "personal": map[string]interface{}{ + "mount_point": "/thepath", + }, + }, + }, }, } @@ -195,19 +203,28 @@ var _ = Describe("Static", func() { "home_provider": "/users/{{.Id.OpaqueId}}", "providers": map[string]interface{}{ "127.0.0.1:13020": map[string]interface{}{ - "mount_path": "/users/[a-k]", - "path_template": "/users/{{.Space.Owner.Username}}", - "space_type": "personal", + "spaces": map[string]interface{}{ + "personal": map[string]interface{}{ + "mount_point": "/users/[a-k]", + "path_template": "/users/{{.Space.Owner.Username}}", + }, + }, }, "127.0.0.1:13021": map[string]interface{}{ - "mount_path": "/users/[l-z]", - "path_template": "/users/{{.Space.Owner.Username}}", - "space_type": "personal", + "spaces": map[string]interface{}{ + "personal": map[string]interface{}{ + "mount_point": "/users/[l-z]", + "path_template": "/users/{{.Space.Owner.Username}}", + }, + }, }, "127.0.0.1:13022": map[string]interface{}{ - "mount_path": "/projects", - "path_template": "/projects/{{.Space.Name}}", - "space_type": "project", + "spaces": map[string]interface{}{ + "project": map[string]interface{}{ + "mount_point": "/projects", + "path_template": "/projects/{{.Space.Name}}", + }, + }, }, }, } @@ -338,19 +355,28 @@ var _ = Describe("Static", func() { "home_provider": "/users/{{.Id.OpaqueId}}", "providers": map[string]interface{}{ "127.0.0.1:13020": map[string]interface{}{ - "mount_path": "/foo", - "path_template": "/foo", - "space_type": "project", + "spaces": map[string]interface{}{ + "personal": map[string]interface{}{ + "mount_point": "/foo", + "path_template": "/foo", + }, + }, }, "127.0.0.1:13021": map[string]interface{}{ - "mount_path": "/foo/bar", - "path_template": "/foo/bar", - "space_type": "project", + "spaces": map[string]interface{}{ + "personal": map[string]interface{}{ + "mount_point": "/foo/bar", + "path_template": "/foo/bar", + }, + }, }, "127.0.0.1:13022": map[string]interface{}{ - "mount_path": "/foo/bar/baz", - "path_template": "/foo/bar/baz", - "space_type": "project", + "spaces": map[string]interface{}{ + "project": map[string]interface{}{ + "mount_point": "/foo/bar/baz", + "path_template": "/foo/bar/baz", + }, + }, }, }, } diff --git a/tests/oc-integration-tests/drone/gateway.toml b/tests/oc-integration-tests/drone/gateway.toml index 03ec7b40fd..e33ee39dc6 100644 --- a/tests/oc-integration-tests/drone/gateway.toml +++ b/tests/oc-integration-tests/drone/gateway.toml @@ -55,9 +55,9 @@ driver = "spaces" [grpc.services.storageregistry.drivers.spaces] home_template = "/users/{{.Id.OpaqueId}}" -[grpc.services.storageregistry.drivers.spaces.providers] ## obviously, we do not want to define a rule for every user and space, which is why we can define naming rules: -"localhost:11000" = {"mount_path" = "/users", "space_type" = "personal", "path_template" = "/users/{{.Space.Owner.Id.OpaqueId}}", "description" = "personal spaces"} +[grpc.services.storageregistry.drivers.spaces.providers."localhost:11000".spaces] +"personal" = { "mount_point" = "/users", "path_template" = "/users/{{.Space.Owner.Id.OpaqueId}}" } ## users can be spread over multiple providers like this: #"localhost:11000" = {"mount_path" = "/users/[0-9]", "space_type" = "personal", "path_template" = "/users/{{.Space.Owner.Id.OpaqueId}}", "description" = "personal spaces 0-9"} @@ -65,14 +65,16 @@ home_template = "/users/{{.Id.OpaqueId}}" ## the virtual /Shares folder of every user is routed like this: ## whenever the path matches the pattern /users/{{.CurrentUser.Id.OpaqueId}}/Shares we forward requests to the sharesstorageprovider -"localhost:14000" = {"mount_path" = "/users/{{.CurrentUser.Id.OpaqueId}}/Shares", "space_type" = "share", "path_template" = "/users/{{.CurrentUser.Id.OpaqueId}}/Shares/{{.Space.Name}}", "description" = "shares"} +[grpc.services.storageregistry.drivers.spaces.providers."localhost:14000".spaces] +"share" = { "mount_point" = "/users/{{.CurrentUser.Id.OpaqueId}}/Shares", "path_template" = "/users/{{.CurrentUser.Id.OpaqueId}}/Shares/{{.Space.Name}}" } ## An alternative would be used to mount shares outside of the users home: #"localhost:14000" = {"mount_path" = "/shares", "space_type" = "share", "path_template" = "/shares/{{.Space.Name}}", "description" = "shares"} ## While public shares are mounted at /public logged in end will should never see that path because it is only created by the spaces registry when ## a public link is accessed. -"localhost:13000" = {"mount_path" = "/public", "space_type" = "public", "path_template" = "/public", "description" = "public links"} +[grpc.services.storageregistry.drivers.spaces.providers."localhost:13000".spaces] +"public" = { "mount_point" = "/public", "path_template" = "/public" } [http] address = "0.0.0.0:19001" diff --git a/tests/oc-integration-tests/local/gateway.toml b/tests/oc-integration-tests/local/gateway.toml index 8d8ca61879..0f577cf628 100644 --- a/tests/oc-integration-tests/local/gateway.toml +++ b/tests/oc-integration-tests/local/gateway.toml @@ -61,9 +61,9 @@ driver = "spaces" [grpc.services.storageregistry.drivers.spaces] home_template = "/users/{{.Id.OpaqueId}}" -[grpc.services.storageregistry.drivers.spaces.providers] ## obviously, we do not want to define a rule for every user and space, which is why we can define naming rules: -"localhost:11000" = {"mount_path" = "/users", "space_type" = "personal", "path_template" = "/users/{{.Space.Owner.Id.OpaqueId}}", "description" = "personal spaces"} +[grpc.services.storageregistry.drivers.spaces.providers."localhost:11000".spaces] +"personal" = { "mount_point" = "/users", "path_template" = "/users/{{.Space.Owner.Id.OpaqueId}}" } ## users can be spread over multiple providers like this: #"localhost:11000" = {"mount_path" = "/users/[0-9]", "space_type" = "personal", "path_template" = "/users/{{.Space.Owner.Id.OpaqueId}}", "description" = "personal spaces 0-9"} @@ -71,14 +71,16 @@ home_template = "/users/{{.Id.OpaqueId}}" ## the virtual /Shares folder of every user is routed like this: ## whenever the path matches the pattern /users/{{.CurrentUser.Id.OpaqueId}}/Shares we forward requests to the sharesstorageprovider -"localhost:14000" = {"mount_path" = "/users/{{.CurrentUser.Id.OpaqueId}}/Shares", "space_type" = "share", "path_template" = "/users/{{.CurrentUser.Id.OpaqueId}}/Shares/{{.Space.Name}}", "description" = "shares"} +[grpc.services.storageregistry.drivers.spaces.providers."localhost:14000".spaces] +"share" = { "mount_point" = "/users/{{.CurrentUser.Id.OpaqueId}}/Shares", "path_template" = "/users/{{.CurrentUser.Id.OpaqueId}}/Shares/{{.Space.Name}}" } ## An alternative would be used to mount shares outside of the users home: #"localhost:14000" = {"mount_path" = "/shares", "space_type" = "share", "path_template" = "/shares/{{.Space.Name}}", "description" = "shares"} ## While public shares are mounted at /public logged in end will should never see that path because it is only created by the spaces registry when ## a public link is accessed. -"localhost:13000" = {"mount_path" = "/public", "space_type" = "public", "path_template" = "/public", "description" = "public links"} +[grpc.services.storageregistry.drivers.spaces.providers."localhost:13000".spaces] +"public" = { "mount_point" = "/public", "path_template" = "/public" } [http] address = "0.0.0.0:19001" From e341e143c735ec4b966863e0e618246bc4f91e5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 8 Dec 2021 14:09:07 +0000 Subject: [PATCH 4/8] fix integration tests, incorporate feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/storage/registry/spaces/spaces.go | 22 +++++++++++-------- .../grpc/fixtures/gateway-sharded.toml | 13 ++++++----- tests/integration/grpc/fixtures/gateway.toml | 8 ++++--- 3 files changed, 26 insertions(+), 17 deletions(-) diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go index 653ce51774..772e9901e2 100644 --- a/pkg/storage/registry/spaces/spaces.go +++ b/pkg/storage/registry/spaces/spaces.go @@ -66,9 +66,9 @@ type spaceConfig struct { } // SpacePath generates a layout based on space data. -func (sc *spaceConfig) SpacePath(u *userpb.User, space *providerpb.StorageSpace) (string, error) { +func (sc *spaceConfig) SpacePath(currentUser *userpb.User, space *providerpb.StorageSpace) (string, error) { b := bytes.Buffer{} - if err := sc.template.Execute(&b, templateData{CurrentUser: u, Space: space}); err != nil { + if err := sc.template.Execute(&b, templateData{CurrentUser: currentUser, Space: space}); err != nil { return "", err } return b.String(), nil @@ -117,9 +117,10 @@ func (c *config) init() { for _, provider := range c.Providers { for _, space := range provider.Spaces { - // if the path template is not explicitly set use the mountpath as path template + // if the path template is not explicitly set use the mount point as path template + // do not use the mount point if it is a regex (starting with ^ - TODO add test) if space.PathTemplate == "" && strings.HasPrefix(space.MountPoint, "/") { - // TODO err if the path is a regex + // TODO err if the mount point is a regex space.PathTemplate = space.MountPoint } @@ -294,7 +295,7 @@ func (r *registry) findProvidersForResource(ctx context.Context, id string) []*r }, }, }} - for spaceType, _ := range provider.Spaces { + for spaceType := range provider.Spaces { // add filter to id based request if it is configured filters = append(filters, &providerpb.ListStorageSpacesRequest_Filter{ Type: providerpb.ListStorageSpacesRequest_Filter_TYPE_SPACE_TYPE, @@ -309,9 +310,9 @@ func (r *registry) findProvidersForResource(ctx context.Context, id string) []*r continue } - if len(spaces) > 0 { - space := spaces[0] // there should not be multiple per provider - + switch len(spaces) { + case 1: + space := spaces[0] for spaceType, sc := range provider.Spaces { if spaceType == space.SpaceType { providerPath, err := sc.SpacePath(currentUser, space) @@ -330,8 +331,11 @@ func (r *registry) findProvidersForResource(ctx context.Context, id string) []*r return []*registrypb.ProviderInfo{p} } } - + case 2: + // there should not be multiple spaces with the same id per provider + appctx.GetLogger(ctx).Error().Err(err).Interface("provider", provider).Interface("spaces", spaces).Msg("multiple spaces returned, ignoring") } + } return []*registrypb.ProviderInfo{} } diff --git a/tests/integration/grpc/fixtures/gateway-sharded.toml b/tests/integration/grpc/fixtures/gateway-sharded.toml index 292daec6a4..bd4c1298e0 100644 --- a/tests/integration/grpc/fixtures/gateway-sharded.toml +++ b/tests/integration/grpc/fixtures/gateway-sharded.toml @@ -22,11 +22,14 @@ driver = "spaces" [grpc.services.storageregistry.drivers.spaces] home_template = "/users/{{.Id.OpaqueId}}" -[grpc.services.storageregistry.drivers.spaces.providers] -"{{storage_address}}" = {"mount_path" = "/projects/[a-k]", "space_type" = "project", "path_template" = "/projects/{{.Space.Name}}"} -"{{storage2_address}}" = {"mount_path" = "/projects/[l-z]", "space_type" = "project", "path_template" = "/projects/{{.Space.Name}}"} - -"/users" = {"address" = "{{homestorage_address}}", "space_type" = "personal", "path_template" = "/users/{{.Space.Owner.Id.OpaqueId}}"} +# a sharded /projects folder +[grpc.services.storageregistry.drivers.spaces.providers."{{storage_address}}".spaces] +"project" = { "mount_point" = "/projects/[a-k]", "path_template" = "/projects/{{.Space.Name}}" } +[grpc.services.storageregistry.drivers.spaces.providers."{{storage2_address}}".spaces] +"project" = { "mount_point" = "/projects/[l-z]", "path_template" = "/projects/{{.Space.Name}}" } + +[grpc.services.storageregistry.drivers.spaces.providers."{{homestorage_address}}".spaces] +"personal" = { "mount_point" = "/users", "path_template" = "/users/{{.Space.Owner.Id.OpaqueId}}" } [http] address = "{{grpc_address+1}}" diff --git a/tests/integration/grpc/fixtures/gateway.toml b/tests/integration/grpc/fixtures/gateway.toml index e4cacce1d3..8dd59da13a 100644 --- a/tests/integration/grpc/fixtures/gateway.toml +++ b/tests/integration/grpc/fixtures/gateway.toml @@ -22,9 +22,11 @@ driver = "spaces" [grpc.services.storageregistry.drivers.spaces] home_template = "/users/{{.Id.OpaqueId}}" -[grpc.services.storageregistry.drivers.spaces.providers] -"{{storage_address}}" = {"mount_path" = "/users", "space_type" = "personal", "path_template" = "/users/{{.Space.Owner.Id.OpaqueId}}"} -"{{storage2_address}}" = {"mount_path" = "/users/{{.CurrentUser.Id.OpaqueId}}/Projects", "space_type" = "project", "path_template" = "/users/{{.Space.Owner.Id.OpaqueId}}/Projects/{{.Space.Id.OpaqueId}}"} +[grpc.services.storageregistry.drivers.spaces.providers."{{storage_address}}".spaces] +"personal" = { "mount_point" = "/users", "path_template" = "/users/{{.Space.Owner.Id.OpaqueId}}" } + +[grpc.services.storageregistry.drivers.spaces.providers."{{storage2_address}}".spaces] +"project" = { "mount_point" = "/users/{{.CurrentUser.Id.OpaqueId}}/Projects", "path_template" = "/users/{{.Space.Owner.Id.OpaqueId}}/Projects/{{.Space.Id.OpaqueId}}" } [http] address = "{{grpc_address+1}}" From 05a4ce281e400ed5d5b9a7ae38a92f0496002cfc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 8 Dec 2021 14:46:32 +0000 Subject: [PATCH 5/8] drop unneded config option MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- tests/oc-integration-tests/local/gateway.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/oc-integration-tests/local/gateway.toml b/tests/oc-integration-tests/local/gateway.toml index 0f577cf628..108dcb3410 100644 --- a/tests/oc-integration-tests/local/gateway.toml +++ b/tests/oc-integration-tests/local/gateway.toml @@ -80,7 +80,7 @@ home_template = "/users/{{.Id.OpaqueId}}" ## While public shares are mounted at /public logged in end will should never see that path because it is only created by the spaces registry when ## a public link is accessed. [grpc.services.storageregistry.drivers.spaces.providers."localhost:13000".spaces] -"public" = { "mount_point" = "/public", "path_template" = "/public" } +"public" = { "mount_point" = "/public" } [http] address = "0.0.0.0:19001" From 42298c5cc5915ee9208e315ab1102653438c5fc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 8 Dec 2021 14:47:51 +0000 Subject: [PATCH 6/8] always return type of space to let registry build correct path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/storage/utils/decomposedfs/spaces.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/pkg/storage/utils/decomposedfs/spaces.go b/pkg/storage/utils/decomposedfs/spaces.go index d8eea35dfe..661d0f4a57 100644 --- a/pkg/storage/utils/decomposedfs/spaces.go +++ b/pkg/storage/utils/decomposedfs/spaces.go @@ -249,7 +249,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide } // TODO apply more filters - space, err := fs.storageSpaceFromNode(ctx, n, matches[i], spaceType, permissions) + space, err := fs.storageSpaceFromNode(ctx, n, matches[i], permissions) if err != nil { if _, ok := err.(errtypes.IsPermissionDenied); !ok { appctx.GetLogger(ctx).Error().Err(err).Interface("node", n).Msg("could not convert to storage space") @@ -266,8 +266,7 @@ func (fs *Decomposedfs) ListStorageSpaces(ctx context.Context, filter []*provide if err != nil { return nil, err } - // TODO do we have a better space type than "node" when looking up a space for a node? - space, err := fs.storageSpaceFromNode(ctx, n, n.InternalPath(), "node", permissions) + space, err := fs.storageSpaceFromNode(ctx, n, n.InternalPath(), permissions) if err != nil { return nil, err } @@ -395,7 +394,7 @@ func (fs *Decomposedfs) createStorageSpace(ctx context.Context, spaceType, space return nil } -func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, nodePath, spaceType string, permissions map[string]struct{}) (*provider.StorageSpace, error) { +func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, nodePath string, permissions map[string]struct{}) (*provider.StorageSpace, error) { owner, err := n.Owner() if err != nil { return nil, err @@ -412,6 +411,18 @@ func (fs *Decomposedfs) storageSpaceFromNode(ctx context.Context, n *node.Node, return nil, err } + glob := filepath.Join(fs.o.Root, "spaces", "*", n.SpaceRoot.ID) + matches, err := filepath.Glob(glob) + if err != nil { + return nil, err + } + + if len(matches) != 1 { + return nil, errtypes.InternalError("expected only one match for " + glob) + } + + spaceType := filepath.Base(filepath.Dir(matches[0])) + space := &provider.StorageSpace{ Id: &provider.StorageSpaceId{OpaqueId: n.SpaceRoot.ID}, Root: &provider.ResourceId{ From 24e9d123246ffc24cd5001452db8d074d098f2a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 8 Dec 2021 14:48:30 +0000 Subject: [PATCH 7/8] spaces registry: treat not found as empty list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/storage/registry/spaces/spaces.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go index 772e9901e2..a5509ed314 100644 --- a/pkg/storage/registry/spaces/spaces.go +++ b/pkg/storage/registry/spaces/spaces.go @@ -476,7 +476,7 @@ func (r *registry) findStorageSpaceOnProvider(ctx context.Context, addr string, if err != nil { return nil, err } - if res.Status.Code != rpc.Code_CODE_OK { + if res.Status.Code != rpc.Code_CODE_OK && res.Status.Code != rpc.Code_CODE_NOT_FOUND { return nil, status.NewErrorFromCode(res.Status.Code, "spaces registry") } return res.StorageSpaces, nil From d0cfd990f71c27a8f528fd4b5ed121242bdc49e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Wed, 8 Dec 2021 15:21:35 +0000 Subject: [PATCH 8/8] add feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- pkg/storage/registry/spaces/spaces.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/storage/registry/spaces/spaces.go b/pkg/storage/registry/spaces/spaces.go index a5509ed314..079eeb4974 100644 --- a/pkg/storage/registry/spaces/spaces.go +++ b/pkg/storage/registry/spaces/spaces.go @@ -117,10 +117,12 @@ func (c *config) init() { for _, provider := range c.Providers { for _, space := range provider.Spaces { + if space.MountPoint == "" { + space.MountPoint = "/" + } + // if the path template is not explicitly set use the mount point as path template - // do not use the mount point if it is a regex (starting with ^ - TODO add test) - if space.PathTemplate == "" && strings.HasPrefix(space.MountPoint, "/") { - // TODO err if the mount point is a regex + if space.PathTemplate == "" { space.PathTemplate = space.MountPoint } @@ -311,6 +313,8 @@ func (r *registry) findProvidersForResource(ctx context.Context, id string) []*r } switch len(spaces) { + case 0: + // nothing to do, will continue with next provider case 1: space := spaces[0] for spaceType, sc := range provider.Spaces { @@ -331,7 +335,7 @@ func (r *registry) findProvidersForResource(ctx context.Context, id string) []*r return []*registrypb.ProviderInfo{p} } } - case 2: + default: // there should not be multiple spaces with the same id per provider appctx.GetLogger(ctx).Error().Err(err).Interface("provider", provider).Interface("spaces", spaces).Msg("multiple spaces returned, ignoring") }