Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API Cleanup: SpaceID #3006

Merged
merged 12 commits into from
Jul 12, 2022
4 changes: 2 additions & 2 deletions .drone.star
Original file line number Diff line number Diff line change
Expand Up @@ -808,7 +808,7 @@ def cs3ApiValidatorOcis():
},
{
"name": "cs3api-validator-ocis",
"image": "owncloud/cs3api-validator:0.1.0",
"image": "owncloud/cs3api-validator:0.2.0",
"commands": [
"/usr/bin/cs3api-validator /var/lib/cs3api-validator --endpoint=revad-services:19000",
],
Expand Down Expand Up @@ -861,7 +861,7 @@ def cs3ApiValidatorS3NG():
},
{
"name": "cs3api-validator-S3NG",
"image": "owncloud/cs3api-validator:0.1.0",
"image": "owncloud/cs3api-validator:0.2.0",
"commands": [
"/usr/bin/cs3api-validator /var/lib/cs3api-validator --endpoint=revad-services:19000",
],
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ test-integration: build-ci

# This is needed for osx because this os does not support static linking
# Use the build target without the static flag
test-integration-osx: build
cd tests/integration && go test -race ./...
test-integration-osx: off build
cd tests/integration && go test -race -v ./...

litmus-test-old: build
cd tests/oc-integration-tests/local && ../../../cmd/revad/revad -c frontend.toml &
Expand Down
5 changes: 5 additions & 0 deletions changelog/unreleased/space-id.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Change: Use spaceID on the cs3api

We introduced a new spaceID field on the cs3api to implement the spaces feature in a cleaner way.

https://github.com/cs3org/reva/pull/3006
2 changes: 1 addition & 1 deletion cmd/reva/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func uploadCommand() *command {

info := res2.Info

fmt.Printf("File uploaded: %s:%s %d %s\n", info.Id.StorageId, info.Id.OpaqueId, info.Size, info.Path)
fmt.Printf("File uploaded: %s %d %s\n", info.Id, info.Size, info.Path)

return nil
}
Expand Down
3 changes: 1 addition & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ require (
github.com/cheggaaa/pb v1.0.29
github.com/coreos/go-oidc v2.2.1+incompatible
github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e
github.com/cs3org/go-cs3apis v0.0.0-20220512100524-551800f020d8
github.com/cs3org/go-cs3apis v0.0.0-20220621145831-c38cca0796c2
github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8
github.com/dgraph-io/ristretto v0.1.0
github.com/eventials/go-tus v0.0.0-20200718001131-45c7ec8f5d59
Expand Down Expand Up @@ -90,7 +90,6 @@ require (
go 1.16

replace (
github.com/cs3org/go-cs3apis => github.com/micbar/go-cs3apis v0.0.0-20220617090231-703c04619761 // temp fork
github.com/eventials/go-tus => github.com/andrewmostello/go-tus v0.0.0-20200314041820-904a9904af9a
github.com/oleiade/reflections => github.com/oleiade/reflections v1.0.1
)
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,8 @@ github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ3
github.com/creack/pty v1.1.11/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e h1:tqSPWQeueWTKnJVMJffz4pz0o1WuQxJ28+5x5JgaHD8=
github.com/cs3org/cato v0.0.0-20200828125504-e418fc54dd5e/go.mod h1:XJEZ3/EQuI3BXTp/6DUzFr850vlxq11I6satRtz0YQ4=
github.com/cs3org/go-cs3apis v0.0.0-20220621145831-c38cca0796c2 h1:o/ovJzS4pyL/rZgp0MtC4Q7JIle5DikimilTLBw2TjY=
github.com/cs3org/go-cs3apis v0.0.0-20220621145831-c38cca0796c2/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8 h1:Z9lwXumT5ACSmJ7WGnFl+OMLLjpz5uR2fyz7dC255FI=
github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8/go.mod h1:4abs/jPXcmJzYoYGF91JF9Uq9s/KL5n1jvFDix8KcqY=
github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4=
Expand Down Expand Up @@ -667,8 +669,6 @@ github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0j
github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0=
github.com/maxymania/go-system v0.0.0-20170110133659-647cc364bf0b h1:Q53idHrTuQDDHyXaxZ6pUl0I9uyD6Z6uKFK3ocX6LzI=
github.com/maxymania/go-system v0.0.0-20170110133659-647cc364bf0b/go.mod h1:KirJrATYGbTyUwVR26xIkaipRqRcMRXBf8N5dacvGus=
github.com/micbar/go-cs3apis v0.0.0-20220617090231-703c04619761 h1:vpejExni287rOeBsJxzCDSYJ4d981KXq7HnnaCt2Wn4=
github.com/micbar/go-cs3apis v0.0.0-20220617090231-703c04619761/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
github.com/miekg/dns v1.0.14/go.mod h1:W1PPwlIAgtquWBMBEV9nkV9Cazfe8ScdGz/Lj7v3Nrg=
github.com/miekg/dns v1.1.26/go.mod h1:bPDLeHnStXmXAq1m/Ch/hvfNHr14JKNPMBo3VZKjuso=
github.com/miekg/dns v1.1.40/go.mod h1:KNUDUusw/aVsxyTYZM1oqvCicbwhgbNgztCETuNZ7xM=
Expand Down
11 changes: 9 additions & 2 deletions internal/grpc/interceptors/auth/scope.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,9 +237,16 @@ func extractRefFromListProvidersReq(v *registry.ListStorageProvidersRequest) (*p
ref := &provider.Reference{}
if v.Opaque != nil && v.Opaque.Map != nil {
if e, ok := v.Opaque.Map["storage_id"]; ok {
ref.ResourceId = &provider.ResourceId{
StorageId: string(e.Value),
if ref.ResourceId == nil {
ref.ResourceId = &provider.ResourceId{}
}
ref.ResourceId.StorageId = string(e.Value)
}
if e, ok := v.Opaque.Map["space_id"]; ok {
if ref.ResourceId == nil {
ref.ResourceId = &provider.ResourceId{}
}
ref.ResourceId.SpaceId = string(e.Value)
}
if e, ok := v.Opaque.Map["opaque_id"]; ok {
if ref.ResourceId == nil {
Expand Down
28 changes: 16 additions & 12 deletions internal/grpc/services/gateway/storageprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,11 +231,11 @@ func (s *svc) ListStorageSpaces(ctx context.Context, req *provider.ListStorageSp
for _, f := range req.Filters {
switch f.Type {
case provider.ListStorageSpacesRequest_Filter_TYPE_ID:
sid, oid, err := storagespace.SplitID(f.GetId().OpaqueId)
sid, spid, oid, err := storagespace.SplitID(f.GetId().OpaqueId)
if err != nil {
continue
}
filters["storage_id"], filters["opaque_id"] = sid, oid
filters["storage_id"], filters["space_id"], filters["opaque_id"] = sid, spid, oid
case provider.ListStorageSpacesRequest_Filter_TYPE_OWNER:
filters["owner_idp"] = f.GetOwner().Idp
filters["owner_id"] = f.GetOwner().OpaqueId
Expand Down Expand Up @@ -318,17 +318,14 @@ func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorag
_, purge = opaque.Map["purge"]
}

storageid, opaqeid, err := storagespace.SplitID(req.Id.OpaqueId)
rid, err := storagespace.ParseID(req.Id.OpaqueId)
if err != nil {
return &provider.DeleteStorageSpaceResponse{
Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not split space id %s", req.GetId().GetOpaqueId()), err),
Status: status.NewStatusFromErrType(ctx, fmt.Sprintf("gateway could not parse space id %s", req.GetId().GetOpaqueId()), err),
}, nil
}

ref := &provider.Reference{ResourceId: &provider.ResourceId{
StorageId: storageid,
OpaqueId: opaqeid,
}}
ref := &provider.Reference{ResourceId: &rid}
c, _, err := s.find(ctx, ref)
if err != nil {
return &provider.DeleteStorageSpaceResponse{
Expand Down Expand Up @@ -359,7 +356,7 @@ func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorag
log.Debug().Msg("purging storage space")
// List all shares in this storage space
lsRes, err := s.ListShares(ctx, &collaborationv1beta1.ListSharesRequest{
Filters: []*collaborationv1beta1.Filter{share.StorageIDFilter(storageid)},
Filters: []*collaborationv1beta1.Filter{share.SpaceIDFilter(id.SpaceId)}, // FIXME rename the filter? @c0rby
})
switch {
case err != nil:
Expand All @@ -384,7 +381,7 @@ func (s *svc) DeleteStorageSpace(ctx context.Context, req *provider.DeleteStorag

// List all public shares in this storage space
lpsRes, err := s.ListPublicShares(ctx, &linkv1beta1.ListPublicSharesRequest{
Filters: []*linkv1beta1.ListPublicSharesRequest_Filter{publicshare.StorageIDFilter(storageid)},
Filters: []*linkv1beta1.ListPublicSharesRequest_Filter{publicshare.StorageIDFilter(id.SpaceId)}, // FIXME rename the filter? @c0rby
})
switch {
case err != nil:
Expand Down Expand Up @@ -1102,7 +1099,9 @@ func (s *svc) findSpaces(ctx context.Context, ref *provider.Reference) ([]*regis
case ref == nil:
return nil, errtypes.BadRequest("missing reference")
case ref.ResourceId != nil:
// no action needed in that case
if ref.ResourceId.OpaqueId == "" {
ref.ResourceId.OpaqueId = ref.ResourceId.SpaceId
}
case ref.Path != "": // TODO implement a mount path cache in the registry?
// nothing to do here either
default:
Expand All @@ -1115,6 +1114,7 @@ func (s *svc) findSpaces(ctx context.Context, ref *provider.Reference) ([]*regis
}
if ref.ResourceId != nil {
filters["storage_id"] = ref.ResourceId.StorageId
filters["space_id"] = ref.ResourceId.SpaceId
filters["opaque_id"] = ref.ResourceId.OpaqueId
}

Expand All @@ -1132,7 +1132,9 @@ func (s *svc) findSingleSpace(ctx context.Context, ref *provider.Reference) ([]*
case ref == nil:
return nil, errtypes.BadRequest("missing reference")
case ref.ResourceId != nil:
// no action needed in that case
if ref.ResourceId.OpaqueId == "" {
ref.ResourceId.OpaqueId = ref.ResourceId.SpaceId
}
case ref.Path != "": // TODO implement a mount path cache in the registry?
// nothing to do here either
default:
Expand All @@ -1146,6 +1148,7 @@ func (s *svc) findSingleSpace(ctx context.Context, ref *provider.Reference) ([]*
}
if ref.ResourceId != nil {
filters["storage_id"] = ref.ResourceId.StorageId
filters["space_id"] = ref.ResourceId.SpaceId
filters["opaque_id"] = ref.ResourceId.OpaqueId
}

Expand Down Expand Up @@ -1208,6 +1211,7 @@ func unwrap(ref *provider.Reference, mountPoint string, root *provider.ResourceI
return &provider.Reference{
ResourceId: &provider.ResourceId{
StorageId: ref.ResourceId.StorageId,
SpaceId: ref.ResourceId.SpaceId,
OpaqueId: ref.ResourceId.OpaqueId,
},
Path: ref.Path,
Expand Down
16 changes: 8 additions & 8 deletions internal/grpc/services/gateway/storageprovidercache.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ func (c Caches) RemoveStat(user *userpb.User, res *provider.ResourceId) {
sid := ""
oid := ""
if res != nil {
sid = "sid:" + res.StorageId
sid = "sid:" + res.SpaceId
oid = "oid:" + res.OpaqueId
}

Expand Down Expand Up @@ -133,7 +133,7 @@ func (c Caches) RemoveListStorageProviders(res *provider.ResourceId) {
if res == nil {
return
}
sid := res.StorageId
sid := res.SpaceId

cache := c[listproviders]
for _, key := range cache.GetKeys() {
Expand Down Expand Up @@ -182,9 +182,9 @@ func (c *cachedRegistryClient) ListStorageProviders(ctx context.Context, in *reg

user := ctxpkg.ContextMustGetUser(ctx)

storageID := sdk.DecodeOpaqueMap(in.Opaque)["storage_id"]
spaceID := sdk.DecodeOpaqueMap(in.Opaque)["space_id"]

key := user.GetId().GetOpaqueId() + "!" + storageID
key := user.GetId().GetOpaqueId() + "!" + spaceID
if key != "!" {
s := &registry.ListStorageProvidersResponse{}
if err := pullFromCache(cache, key, s); err == nil {
Expand All @@ -198,9 +198,9 @@ func (c *cachedRegistryClient) ListStorageProviders(ctx context.Context, in *reg
return nil, err
case resp.Status.Code != rpc.Code_CODE_OK:
return resp, nil
case storageID == "":
case spaceID == "":
return resp, nil
case storageID == utils.ShareStorageProviderID: // TODO do we need to compare providerid and spaceid separately?
case spaceID == utils.ShareStorageSpaceID: // TODO do we need to compare providerid and spaceid separately?
return resp, nil
default:
return resp, pushToCache(cache, key, resp)
Expand Down Expand Up @@ -230,15 +230,15 @@ type cachedAPIClient struct {
// a key looks like: uid:1234-1233!sid:5678-5677!oid:9923-9934!path:/path/to/source
// as you see it adds "uid:"/"sid:"/"oid:" prefixes to the uuids so they can be differentiated
func statKey(user *userpb.User, ref *provider.Reference, metaDataKeys, fieldMaskPaths []string) string {
if ref == nil || ref.ResourceId == nil || ref.ResourceId.StorageId == "" {
if ref == nil || ref.ResourceId == nil || ref.ResourceId.SpaceId == "" {
return ""
}

key := strings.Builder{}
key.WriteString("uid:")
key.WriteString(user.Id.OpaqueId)
key.WriteString("!sid:")
key.WriteString(ref.ResourceId.StorageId)
key.WriteString(ref.ResourceId.SpaceId)
key.WriteString("!oid:")
key.WriteString(ref.ResourceId.OpaqueId)
key.WriteString("!path:")
Expand Down
7 changes: 3 additions & 4 deletions internal/grpc/services/gateway/usershareprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import (
"github.com/cs3org/reva/v2/pkg/rgrpc/status"
"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
"github.com/cs3org/reva/v2/pkg/storage/utils/grants"
"github.com/cs3org/reva/v2/pkg/storagespace"
"github.com/cs3org/reva/v2/pkg/utils"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -717,11 +716,11 @@ func refIsSpaceRoot(ref *provider.ResourceId) bool {
if ref == nil {
return false
}
if ref.StorageId == "" || ref.OpaqueId == "" {
if ref.SpaceId == "" || ref.OpaqueId == "" {
return false
}
_, sid := storagespace.SplitStorageID(ref.GetStorageId())
return sid == ref.OpaqueId

return ref.SpaceId == ref.OpaqueId
}

func shareIsSpaceRoot(key *collaboration.ShareKey) bool {
Expand Down
Loading