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

enhancement: bump reva and remove explicit automount mountpoint naming #9377

Merged
merged 2 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/unreleased/bump-reva.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ Enhancement: Bump Reva

bumps reva version

https://github.com/owncloud/ocis/pull/9377
https://github.com/owncloud/ocis/pull/9330
https://github.com/owncloud/ocis/pull/9318
https://github.com/owncloud/ocis/pull/9269
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/cenkalti/backoff v2.2.1+incompatible
github.com/coreos/go-oidc/v3 v3.10.0
github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781
github.com/cs3org/reva/v2 v2.19.2-0.20240607120909-0a9c7b8bef6a
github.com/cs3org/reva/v2 v2.19.2-0.20240618080316-ed0273c9db9b
github.com/dhowden/tag v0.0.0-20230630033851-978a0926ee25
github.com/dutchcoders/go-clamd v0.0.0-20170520113014-b970184f4d9e
github.com/egirna/icap-client v0.1.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1025,8 +1025,8 @@ github.com/crewjam/saml v0.4.14 h1:g9FBNx62osKusnFzs3QTN5L9CVA/Egfgm+stJShzw/c=
github.com/crewjam/saml v0.4.14/go.mod h1:UVSZCf18jJkk6GpWNVqcyQJMD5HsRugBPf4I1nl2mME=
github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781 h1:BUdwkIlf8IS2FasrrPg8gGPHQPOrQ18MS1Oew2tmGtY=
github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781/go.mod h1:UXha4TguuB52H14EMoSsCqDj7k8a/t7g4gVP+bgY5LY=
github.com/cs3org/reva/v2 v2.19.2-0.20240607120909-0a9c7b8bef6a h1:D1Pg9i+pZhzn/h5397/ukX0XYYPBaUVLh6Zhe83vKUg=
github.com/cs3org/reva/v2 v2.19.2-0.20240607120909-0a9c7b8bef6a/go.mod h1:lKqw0VuP1NcZbhj0e6tGoAGq3tgWO/pLafVJyDK0yVI=
github.com/cs3org/reva/v2 v2.19.2-0.20240618080316-ed0273c9db9b h1:IPFiNd8Xev9WxAt+LkJUxnbyU8Y1rGi5Ha0S+ZNObr8=
github.com/cs3org/reva/v2 v2.19.2-0.20240618080316-ed0273c9db9b/go.mod h1:Rb2XnhpGKnH7k6WBFZlMygbyBxW6ma09Z4Uk+ro0v+A=
github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4=
github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg=
github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4=
Expand Down
126 changes: 9 additions & 117 deletions services/frontend/pkg/command/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,28 @@ package command
import (
"context"
"errors"
"fmt"
"path"
"path/filepath"
"slices"
"strconv"
"strings"

"github.com/cs3org/reva/v2/pkg/events"
"github.com/cs3org/reva/v2/pkg/events/stream"
"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
"github.com/cs3org/reva/v2/pkg/utils"
"go-micro.dev/v4/metadata"
"google.golang.org/protobuf/types/known/fieldmaskpb"

"github.com/owncloud/ocis/v2/ocis-pkg/log"
"github.com/owncloud/ocis/v2/ocis-pkg/middleware"
"github.com/owncloud/ocis/v2/ocis-pkg/registry"
"github.com/owncloud/ocis/v2/ocis-pkg/service/grpc"
"github.com/owncloud/ocis/v2/ocis-pkg/tracing"
"github.com/owncloud/ocis/v2/services/frontend/pkg/config"
"github.com/owncloud/ocis/v2/services/settings/pkg/store/defaults"
"go-micro.dev/v4/metadata"
"google.golang.org/protobuf/types/known/fieldmaskpb"

gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1"
group "github.com/cs3org/go-cs3apis/cs3/identity/group/v1beta1"
user "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
collaboration "github.com/cs3org/go-cs3apis/cs3/sharing/collaboration/v1beta1"
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"

settingssvc "github.com/owncloud/ocis/v2/protogen/gen/ocis/services/settings/v0"
)

Expand Down Expand Up @@ -116,41 +111,23 @@ func AutoAcceptShares(ev events.ShareCreated, autoAcceptDefault bool, l log.Logg
return
}

uids, err := getUserIDs(ctx, gatewaySelector, ev.GranteeUserID, ev.GranteeGroupID)
userIDs, err := getUserIDs(ctx, gatewaySelector, ev.GranteeUserID, ev.GranteeGroupID)
if err != nil {
l.Error().Err(err).Msg("cannot get grantees")
return
}

gwc, err = gatewaySelector.Next()
if err != nil {
l.Error().Err(err).Msg("cannot get gateway client")
return
}
info, err := utils.GetResourceByID(ctx, ev.ItemID, gwc)
if err != nil {
l.Error().Err(err).Msg("error getting resource")
return
}

for _, uid := range uids {
for _, uid := range userIDs {
if !autoAcceptShares(ctx, uid, autoAcceptDefault, vs) {
continue
}

mp, err := getMountPoint(ctx, l, ev.ItemID, uid, gatewaySelector, info)
if err != nil {
l.Error().Err(err).Msg("error getting mount point")
continue

}

gwc, err := gatewaySelector.Next()
if err != nil {
l.Error().Err(err).Msg("cannot get gateway client")
continue
}
resp, err := gwc.UpdateReceivedShare(ctx, updateShareRequest(ev.ShareID, uid, mp))
resp, err := gwc.UpdateReceivedShare(ctx, updateShareRequest(ev.ShareID, uid))
if err != nil {
l.Error().Err(err).Msg("error sending grpc request")
continue
Expand All @@ -163,66 +140,6 @@ func AutoAcceptShares(ev events.ShareCreated, autoAcceptDefault bool, l log.Logg

}

func getMountPoint(ctx context.Context, l log.Logger, itemid *provider.ResourceId, uid *user.UserId, gatewaySelector pool.Selectable[gateway.GatewayAPIClient], info *provider.ResourceInfo) (string, error) {
lrs, err := getSharesList(ctx, gatewaySelector, uid)
if err != nil {
return "", err
}

// we need to sort the received shares by mount point in order to make things easier to evaluate.
base := path.Base(info.GetPath())
mount := base
mounts := make([]string, 0, len(lrs.Shares))
var exists bool

for _, s := range lrs.Shares {
if s.State != collaboration.ShareState_SHARE_STATE_ACCEPTED {
// we don't care about unaccepted shares
continue
}

if utils.ResourceIDEqual(s.GetShare().GetResourceId(), itemid) {
// a share to the same resource already exists and is mounted
return s.GetMountPoint().GetPath(), nil
}

if s.GetMountPoint().GetPath() == mount {
// does the shared resource still exist?
gwc, err := gatewaySelector.Next()
if err != nil {
l.Error().Err(err).Msg("cannot get gateway client")
continue
}
_, err = utils.GetResourceByID(ctx, s.GetShare().GetResourceId(), gwc)
if err == nil {
exists = true
}
// TODO we could delete shares here if the stat returns code NOT FOUND ... but listening for file deletes would be better
}
// collect all mount points
mounts = append(mounts, s.GetMountPoint().GetPath())
}

// If the mount point really already exists, we need to insert a number into the filename
if exists {
// now we have a list of shares, we want to iterate over all of them and check for name collisions agents a mount points list
for i := 1; i <= len(mounts)+1; i++ {
ext := filepath.Ext(base)
name := strings.TrimSuffix(base, ext)
// be smart about .tar.(gz|bz) files
if strings.HasSuffix(name, ".tar") {
name = strings.TrimSuffix(name, ".tar")
ext = ".tar" + ext
}
mount = name + " (" + strconv.Itoa(i) + ")" + ext
if !slices.Contains(mounts, mount) {
return mount, nil
}
}
}
return mount, nil
}

func getUserIDs(ctx context.Context, gatewaySelector pool.Selectable[gateway.GatewayAPIClient], uid *user.UserId, gid *group.GroupId) ([]*user.UserId, error) {
if uid != nil {
return []*user.UserId{uid}, nil
Expand Down Expand Up @@ -257,40 +174,15 @@ func autoAcceptShares(ctx context.Context, uid *user.UserId, defaultValue bool,
return defaultValue
}

func updateShareRequest(shareID *collaboration.ShareId, uid *user.UserId, path string) *collaboration.UpdateReceivedShareRequest {
func updateShareRequest(shareID *collaboration.ShareId, uid *user.UserId) *collaboration.UpdateReceivedShareRequest {
return &collaboration.UpdateReceivedShareRequest{
Opaque: utils.AppendJSONToOpaque(nil, "userid", uid),
Share: &collaboration.ReceivedShare{
Share: &collaboration.Share{
Id: shareID,
},
MountPoint: &provider.Reference{
Path: path,
},
State: collaboration.ShareState_SHARE_STATE_ACCEPTED,
},
UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"state", "mount_point"}},
}
}

// getSharesList gets the list of all shares for the given user.
func getSharesList(ctx context.Context, gatewaySelector pool.Selectable[gateway.GatewayAPIClient], uid *user.UserId) (*collaboration.ListReceivedSharesResponse, error) {
gwc, err := gatewaySelector.Next()
if err != nil {
return nil, err
}
shares, err := gwc.ListReceivedShares(ctx, &collaboration.ListReceivedSharesRequest{
Opaque: utils.AppendJSONToOpaque(nil, "userid", uid),
})
if err != nil {
return nil, err
}

if shares.Status.Code != rpc.Code_CODE_OK {
if shares.Status.Code == rpc.Code_CODE_NOT_FOUND {
return nil, fmt.Errorf("not found")
}
return nil, fmt.Errorf(shares.GetStatus().GetMessage())
UpdateMask: &fieldmaskpb.FieldMask{Paths: []string{"state"}},
}
return shares, nil
}
8 changes: 5 additions & 3 deletions services/graph/pkg/errorcode/cs3.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

cs3rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"

"github.com/cs3org/reva/v2/pkg/utils"
)

Expand All @@ -19,12 +20,13 @@ import (
// This function is particularly useful when dealing with CS3 responses,
// and a unified error handling within the application is necessary.
func FromCS3Status(status *cs3rpc.Status, inerr error, ignore ...cs3rpc.Code) error {
err := Error{errorCode: GeneralException, msg: "unspecified error has occurred", origin: ErrorOriginCS3}

if inerr != nil {
return Error{msg: inerr.Error(), errorCode: GeneralException}
err.msg = inerr.Error()
return err
}

err := Error{errorCode: GeneralException, msg: "unspecified error has occurred"}

if status != nil {
err.msg = status.GetMessage()
}
Expand Down
7 changes: 6 additions & 1 deletion services/graph/pkg/errorcode/cs3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ func TestFromCS3Status(t *testing.T) {
}

for _, test := range tests {
var e errorcode.Error
if errors.As(test.expected, &e) {
test.expected = e.WithOrigin(errorcode.ErrorOriginCS3)
}

if got := errorcode.FromCS3Status(test.status, test.err, test.ignore...); !reflect.DeepEqual(got, test.expected) {
t.Error("Test Failed: {} expected, received: {}", test.expected, got)
}
Expand All @@ -56,7 +61,7 @@ func TestFromStat(t *testing.T) {
err error
result error
}{
{nil, errors.New("some error"), errorcode.New(errorcode.GeneralException, "some error")},
{nil, errors.New("some error"), errorcode.New(errorcode.GeneralException, "some error").WithOrigin(errorcode.ErrorOriginCS3)},
{&provider.StatResponse{Status: &cs3rpc.Status{Code: cs3rpc.Code_CODE_OK}}, nil, nil},
}

Expand Down
51 changes: 43 additions & 8 deletions services/graph/pkg/errorcode/errorcode.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,28 @@ import (
libregraph "github.com/owncloud/libre-graph-api-go"
)

// ErrorCode defines code as used in MS Graph - see https://docs.microsoft.com/en-us/graph/errors?context=graph%2Fapi%2F1.0&view=graph-rest-1.0
type ErrorCode int

// Error defines a custom error struct, containing and MS Graph error code and a textual error message
type Error struct {
errorCode ErrorCode
msg string
origin ErrorOrigin
}

// ErrorOrigin gives information about where the error originated
type ErrorOrigin int

const (
// ErrorOriginUnknown is the default error source
// and indicates that the error does not have any information about its origin
ErrorOriginUnknown ErrorOrigin = iota

// ErrorOriginCS3 indicates that the error originated from a CS3 service
ErrorOriginCS3
)

// ErrorCode defines code as used in MS Graph - see https://docs.microsoft.com/en-us/graph/errors?context=graph%2Fapi%2F1.0&view=graph-rest-1.0
type ErrorCode int

// List taken from https://github.com/microsoft/microsoft-graph-docs-1/blob/main/concepts/errors.md#code-property
const (
// AccessDenied defines the error if the caller doesn't have permission to perform the action.
Expand Down Expand Up @@ -148,7 +161,7 @@ func (e ErrorCode) String() string {
return errorCodes[e]
}

// Error return the concatenation of the error string and optional message
// Error returns the concatenation of the error string and optional message
func (e Error) Error() string {
errString := errorCodes[e.errorCode]
if e.msg != "" {
Expand All @@ -161,13 +174,35 @@ func (e Error) GetCode() ErrorCode {
return e.errorCode
}

// GetOrigin returns the source of the error
func (e Error) GetOrigin() ErrorOrigin {
return e.origin
}

// WithOrigin returns a new Error with the provided origin
func (e Error) WithOrigin(o ErrorOrigin) Error {
e.origin = o
return e
}

// RenderError render the Graph Error based on a code or default one
func RenderError(w http.ResponseWriter, r *http.Request, err error) {
var errcode Error
if errors.As(err, &errcode) {
errcode.Render(w, r)
e, ok := ToError(err)
if !ok {
GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
return
}

GeneralException.Render(w, r, http.StatusInternalServerError, err.Error())
e.Render(w, r)
}

// ToError checks if the error is of type Error and returns it,
// the second parameter indicates if the error conversion was successful
func ToError(err error) (Error, bool) {
var e Error
if errors.As(err, &e) {
return e, true
}

return Error{}, false
}
17 changes: 9 additions & 8 deletions services/graph/pkg/service/v0/api_driveitem_permissions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,6 @@ import (
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
storageprovider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
roleconversions "github.com/cs3org/reva/v2/pkg/conversions"
revactx "github.com/cs3org/reva/v2/pkg/ctx"
"github.com/cs3org/reva/v2/pkg/rgrpc/status"
"github.com/cs3org/reva/v2/pkg/storagespace"
"github.com/cs3org/reva/v2/pkg/utils"
cs3mocks "github.com/cs3org/reva/v2/tests/cs3mocks/mocks"
"github.com/go-chi/chi/v5"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
Expand All @@ -31,6 +25,13 @@ import (
"github.com/tidwall/gjson"
"google.golang.org/grpc"

roleconversions "github.com/cs3org/reva/v2/pkg/conversions"
revactx "github.com/cs3org/reva/v2/pkg/ctx"
"github.com/cs3org/reva/v2/pkg/rgrpc/status"
"github.com/cs3org/reva/v2/pkg/storagespace"
"github.com/cs3org/reva/v2/pkg/utils"
cs3mocks "github.com/cs3org/reva/v2/tests/cs3mocks/mocks"

"github.com/owncloud/ocis/v2/ocis-pkg/log"
"github.com/owncloud/ocis/v2/services/graph/mocks"
"github.com/owncloud/ocis/v2/services/graph/pkg/config/defaults"
Expand Down Expand Up @@ -249,7 +250,7 @@ var _ = Describe("DriveItemPermissionsService", func() {
statResponse.Status = status.NewNotFound(context.Background(), "not found")
permission, err := driveItemPermissionsService.Invite(context.Background(), driveItemId, driveItemInvite)
Expect(err).To(HaveOccurred())
Expect(err).To(MatchError(errorcode.New(errorcode.ItemNotFound, "not found")))
Expect(err).To(MatchError(errorcode.New(errorcode.ItemNotFound, "not found").WithOrigin(errorcode.ErrorOriginCS3)))
Expect(permission).To(BeZero())
})
})
Expand Down Expand Up @@ -999,7 +1000,7 @@ var _ = Describe("DriveItemPermissionsService", func() {

driveItemPermission.SetExpirationDateTime(expiration)
res, err := driveItemPermissionsService.UpdatePermission(context.Background(), driveItemId, "permissionid", driveItemPermission)
Expect(err).To(MatchError(errorcode.New(errorcode.InvalidRequest, "expiration date is in the past")))
Expect(err).To(MatchError(errorcode.New(errorcode.InvalidRequest, "expiration date is in the past").WithOrigin(errorcode.ErrorOriginCS3)))
Expect(res).To(BeZero())
})
})
Expand Down
Loading