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

Efss backend fixes #3526

Merged
merged 61 commits into from
Dec 7, 2022
Merged
Show file tree
Hide file tree
Changes from 45 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
2d69b78
[draft] improve ocmd send sourcePath 404 error message
Oct 17, 2022
4b48eb0
print error message when forwarding invite fails
michielbdejong Oct 31, 2022
a77275b
Resolve https://github.com/cs3org/reva/issues/3365#issuecomment-12820…
michielbdejong Oct 31, 2022
53e12c0
changelog entry
Oct 18, 2022
ebdfe38
report unexpected response codes from EFSS API
Oct 18, 2022
a9123c6
Merge remote-tracking branch 'origin/master' into ocmd-error-messages…
michielbdejong Oct 31, 2022
5963484
fix
michielbdejong Oct 31, 2022
71b9eea
Add appropriate PR link in changelog entry
michielbdejong Nov 1, 2022
d81e817
Merge remote-tracking branch 'origin/master' into ocmd-error-messages…
michielbdejong Nov 1, 2022
f4fb5cc
Also allow 201 responses
michielbdejong Nov 1, 2022
15a3a44
Also throw on EFSS API response codes from other Nextcloud backends
michielbdejong Nov 1, 2022
0a7ebdb
built
michielbdejong Nov 1, 2022
549b147
Fix username in upload call path
michielbdejong Nov 3, 2022
e823c76
Improved error message
michielbdejong Nov 4, 2022
7ab36af
trying to debug https://github.com/pondersource/sciencemesh-php/issue…
michielbdejong Nov 8, 2022
26f101a
Fix https://github.com/pondersource/sciencemesh-php/issues/100
michielbdejong Nov 9, 2022
aa549e0
fix share object in OCM share backend
michielbdejong Nov 10, 2022
96d203d
Merge remote-tracking branch 'origin/ocmd-error-messages-backport' in…
michielbdejong Nov 10, 2022
a6e1af5
Fix double declaration of genID
michielbdejong Nov 14, 2022
49e71f5
info logging
michielbdejong Nov 14, 2022
369869b
syntax fix
michielbdejong Nov 15, 2022
09b6a35
Improve 'request not handled' error message
michielbdejong Nov 18, 2022
0f152b9
Init log
michielbdejong Nov 18, 2022
449fe4a
fix https://github.com/pondersource/sciencemesh-php/issues/114
michielbdejong Nov 18, 2022
3122a18
return JSON share object on ocm/send
michielbdejong Nov 18, 2022
301d71e
How do we tell Nextcloud which WebDAV server to mount this share from?
michielbdejong Nov 28, 2022
355791a
read webdav host from config
michielbdejong Nov 29, 2022
1178dc7
better error messages from OCM /share endpoint
michielbdejong Nov 29, 2022
19d3a41
Fix https://github.com/pondersource/sciencemesh-php/issues/91#issueco…
michielbdejong Nov 29, 2022
af0166d
Split owner string
michielbdejong Nov 29, 2022
6700346
return JSON from invite forward
michielbdejong Dec 5, 2022
f1171d7
message: Success
michielbdejong Dec 5, 2022
e74f2f1
Merge remote-tracking branch 'origin/master' into efss-backend-fixes
michielbdejong Dec 6, 2022
1f4c759
Fix merge
michielbdejong Dec 6, 2022
352052a
make lint (first try)
michielbdejong Dec 6, 2022
e7c9fae
make lint (second try)
michielbdejong Dec 6, 2022
b7cc776
remove unnecessary newlines
michielbdejong Dec 6, 2022
9c865f6
outdent
michielbdejong Dec 6, 2022
78c3556
gofmt -s pkg/storage/fs/nextcloud/nextcloud_server_mock.
michielbdejong Dec 6, 2022
888032f
Fix changelog for calens
glpatcern Dec 6, 2022
1951a12
Fix make test
michielbdejong Dec 6, 2022
eab29af
Merge remote-tracking branch 'origin/efss-backend-fixes' into efss-ba…
michielbdejong Dec 6, 2022
08d3122
fix lint
michielbdejong Dec 6, 2022
b3f67d2
Prepend /home to all paths in mock server
michielbdejong Dec 7, 2022
0f4adf9
Run make lint-fix
vascoguita Dec 7, 2022
8de7d03
Improve error message on error sending accept post request
michielbdejong Dec 7, 2022
dd625fe
tmpdir for gitpod
michielbdejong Dec 7, 2022
78ddbb0
reset server mock
michielbdejong Dec 7, 2022
e3c2a27
reset server mock
michielbdejong Dec 7, 2022
535b744
Use helpers.TempDir
michielbdejong Dec 7, 2022
ad79ced
integration tests now also use this TempDir
michielbdejong Dec 7, 2022
25ced32
apply https://github.com/cs3org/reva/pull/3526/files#r1042061882
michielbdejong Dec 7, 2022
9fcd8b8
tests passing
michielbdejong Dec 7, 2022
4b9b228
uncomment
michielbdejong Dec 7, 2022
c93ad76
Merge remote-tracking branch 'origin/efss-backend-fixes' into efss-ba…
michielbdejong Dec 7, 2022
5cf20d5
/workspace/reva/toolchain/golangci-lint run --fix
michielbdejong Dec 7, 2022
7bcee19
Merge remote-tracking branch 'origin/efss-backend-fixes' into efss-ba…
michielbdejong Dec 7, 2022
b3c409a
Fix case /some
michielbdejong Dec 7, 2022
9f1b623
make some paths relative
michielbdejong Dec 7, 2022
7ce8c44
original/location
michielbdejong Dec 7, 2022
75718f8
/workspace/reva/toolchain/golangci-lint run --fix
michielbdejong Dec 7, 2022
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
6 changes: 6 additions & 0 deletions changelog/unreleased/improve-ocmd-error-logs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Enhancement: Improve error logging in ocmd flow

https://github.com/cs3org/reva/pull/3524
glpatcern marked this conversation as resolved.
Show resolved Hide resolved
https://github.com/cs3org/reva/pull/3419
https://github.com/cs3org/reva/issues/3365
https://github.com/cs3org/reva/pull/3369
4 changes: 2 additions & 2 deletions internal/grpc/services/ocminvitemanager/ocminvitemanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ func (s *service) ForwardInvite(ctx context.Context, req *invitepb.ForwardInvite
err := s.im.ForwardInvite(ctx, req.InviteToken, req.OriginSystemProvider)
if err != nil {
return &invitepb.ForwardInviteResponse{
Status: status.NewInternal(ctx, err, "error forwarding invite"),
Status: status.NewInternal(ctx, err, "error forwarding invite:"+err.Error()),
}, nil
}

Expand Down Expand Up @@ -158,7 +158,7 @@ func (s *service) FindAcceptedUsers(ctx context.Context, req *invitepb.FindAccep
acceptedUsers, err := s.im.FindAcceptedUsers(ctx, req.Filter)
if err != nil {
return &invitepb.FindAcceptedUsersResponse{
Status: status.NewInternal(ctx, err, "error finding remote users"),
Status: status.NewInternal(ctx, err, "error finding remote users: "+err.Error()),
}, nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func (s *service) CreateOCMShare(ctx context.Context, req *ocm.CreateOCMShareReq

if err != nil {
return &ocm.CreateOCMShareResponse{
Status: status.NewInternal(ctx, err, "error creating share"),
Status: status.NewInternal(ctx, err, "error creating share: "+err.Error()),
}, nil
}

Expand Down
3 changes: 2 additions & 1 deletion internal/http/services/ocmd/invites.go
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ func (h *invitesHandler) forwardInvite(w http.ResponseWriter, r *http.Request) {
return
}

_, err = w.Write([]byte("Accepted invite from: " + html.EscapeString(providerDomain)))
_, err = w.Write([]byte("{\"message\": \"Success\", \"providerDomain\":\"" + html.EscapeString(providerDomain) + "\"}"))
if err != nil {
WriteError(w, r, APIErrorServerError, "error writing token data", err)
return
Expand Down Expand Up @@ -313,6 +313,7 @@ func (h *invitesHandler) findAcceptedUsers(w http.ResponseWriter, r *http.Reques
indentedResponse, _ := json.MarshalIndent(response, "", " ")
w.Header().Set("Content-Type", "application/json")
w.WriteHeader(http.StatusOK)
log.Debug().Msg("findAcceptedUsers json response: " + string(indentedResponse))
if _, err := w.Write(indentedResponse); err != nil {
log.Err(err).Msg("Error writing to ResponseWriter")
}
Expand Down
3 changes: 2 additions & 1 deletion internal/http/services/ocmd/ocmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,10 @@ func (s *svc) Handler() http.Handler {
return
case "send":
s.SendHandler.Handler().ServeHTTP(w, r)
return
}

log.Warn().Msg("request not handled")
log.Warn().Msgf("request not handled. Try e.g. 'ocm-provider', 'shares', 'notifications', 'invites', or 'send' instead of '%s'", head)
w.WriteHeader(http.StatusNotFound)
})
}
19 changes: 15 additions & 4 deletions internal/http/services/ocmd/send.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import (
"github.com/cs3org/reva/pkg/appctx"
ctxpkg "github.com/cs3org/reva/pkg/ctx"
"github.com/cs3org/reva/pkg/rgrpc/todo/pool"
"github.com/cs3org/reva/pkg/utils"
"google.golang.org/grpc/metadata"
)

Expand Down Expand Up @@ -109,13 +110,13 @@ func (h *sendHandler) Handler() http.Handler {
req := &provider.StatRequest{Ref: ref}
res2, err := gatewayClient.Stat(authCtx, req)
if err != nil {
log.Error().Msg("error sending: stat file/folder to share")
log.Error().Msg("gatewayClient.Stat operation failed; is the storage backend reachable?")
w.WriteHeader(http.StatusInternalServerError)
return
}

if res2.Status.Code != rpc.Code_CODE_OK {
log.Error().Msg("error returned: stat file/folder to share")
log.Error().Msgf("sourcePath %s does not exist on the storage backend", sourcePath)
w.WriteHeader(http.StatusInternalServerError)
return
}
Expand Down Expand Up @@ -183,17 +184,27 @@ func (h *sendHandler) Handler() http.Handler {

shareRes, err := gatewayClient.CreateOCMShare(authCtx, shareRequest)
if err != nil {
log.Error().Msg("error sending: CreateShare")
log.Error().Msg("error sending: CreateShare: " + err.Error())
w.WriteHeader(http.StatusInternalServerError)
return
}

if shareRes.Status.Code != rpc.Code_CODE_OK {
log.Error().Msg("error returned: CreateShare")
log.Error().Msg("error returned: CreateShare: " + err.Error())
w.WriteHeader(http.StatusInternalServerError)
return
}

responseBody, err := utils.MarshalProtoV1ToJSON(shareRes)
if err != nil {
log.Error().Msg("error encoding response: " + err.Error())
w.WriteHeader(http.StatusInternalServerError)
return
}
w.WriteHeader(http.StatusOK)
_, err = w.Write(responseBody)
if err != nil {
log.Error().Msg("error writing response body:" + err.Error())
}
})
}
14 changes: 10 additions & 4 deletions internal/http/services/ocmd/shares.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,13 @@ func (h *sharesHandler) createShare(w http.ResponseWriter, r *http.Request) {
}

if resource == "" || providerID == "" || owner == "" {
WriteError(w, r, APIErrorInvalidParameter, "missing details about resource to be shared", nil)
msg := fmt.Sprintf("missing details about resource to be shared (resource='%s', providerID='%s', owner='%s", resource, providerID, owner)
WriteError(w, r, APIErrorInvalidParameter, msg, nil)
return
}
if shareWith == "" || protocol["name"] == "" || meshProvider == "" {
WriteError(w, r, APIErrorInvalidParameter, "missing request parameters", nil)
msg := fmt.Sprintf("missing request parameters (shareWith='%s', protocol.name='%s', meshProvider='%s'", shareWith, protocol["name"], meshProvider)
WriteError(w, r, APIErrorInvalidParameter, msg, nil)
return
}

Expand Down Expand Up @@ -188,9 +190,13 @@ func (h *sharesHandler) createShare(w http.ResponseWriter, r *http.Request) {
return
}

ownerParts := strings.Split(owner, "@")
if len(ownerParts) != 2 {
WriteError(w, r, APIErrorInvalidParameter, "owner should be opaqueId@webDAVHost", nil)
}
ownerID := &userpb.UserId{
OpaqueId: owner,
Idp: meshProvider,
OpaqueId: ownerParts[0],
Idp: ownerParts[1],
Type: userpb.UserType_USER_TYPE_PRIMARY,
}
createShareReq := &ocmcore.CreateOCMCoreShareRequest{
Expand Down
5 changes: 5 additions & 0 deletions pkg/auth/manager/nextcloud/nextcloud.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ package nextcloud
import (
"context"
"encoding/json"
"fmt"
"io"
"net/http"
"strconv"
"strings"

authpb "github.com/cs3org/go-cs3apis/cs3/auth/provider/v1beta1"
Expand Down Expand Up @@ -141,6 +143,9 @@ func (am *Manager) do(ctx context.Context, a Action) (int, []byte, error) {
}

log.Info().Msgf("am.do response %d %s", resp.StatusCode, body)
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated {
return 0, nil, fmt.Errorf("Unexpected response code from EFSS API: " + strconv.Itoa(resp.StatusCode))
}
return resp.StatusCode, body, nil
}

Expand Down
8 changes: 8 additions & 0 deletions pkg/ocm/invite/manager/json/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
invitepb "github.com/cs3org/go-cs3apis/cs3/ocm/invite/v1beta1"
ocmprovider "github.com/cs3org/go-cs3apis/cs3/ocm/provider/v1beta1"
"github.com/cs3org/reva/pkg/appctx"
ctxpkg "github.com/cs3org/reva/pkg/ctx"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/ocm/invite"
Expand Down Expand Up @@ -272,7 +273,14 @@ func (m *manager) AcceptInvite(ctx context.Context, invite *invitepb.InviteToken

func (m *manager) GetAcceptedUser(ctx context.Context, remoteUserID *userpb.UserId) (*userpb.User, error) {
userKey := ctxpkg.ContextMustGetUser(ctx).GetId().GetOpaqueId()
log := appctx.GetLogger(ctx)
for _, acceptedUser := range m.model.AcceptedUsers[userKey] {
log.Info().Msgf("looking for '%s' at '%s' - considering '%s' at '%s'",
remoteUserID.OpaqueId,
remoteUserID.Idp,
acceptedUser.Id.GetOpaqueId(),
acceptedUser.Id.GetIdp(),
)
if (acceptedUser.Id.GetOpaqueId() == remoteUserID.OpaqueId) && (remoteUserID.Idp == "" || acceptedUser.Id.GetIdp() == remoteUserID.Idp) {
return acceptedUser, nil
}
Expand Down
12 changes: 11 additions & 1 deletion pkg/ocm/share/manager/json/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
ocm "github.com/cs3org/go-cs3apis/cs3/sharing/ocm/v1beta1"
provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
typespb "github.com/cs3org/go-cs3apis/cs3/types/v1beta1"
"github.com/cs3org/reva/pkg/appctx"
ctxpkg "github.com/cs3org/reva/pkg/ctx"
"github.com/cs3org/reva/pkg/errtypes"
"github.com/cs3org/reva/pkg/ocm/share"
Expand Down Expand Up @@ -264,6 +265,15 @@ func (m *mgr) Share(ctx context.Context, md *provider.ResourceId, g *ocm.ShareGr
protocol["name"] = "datatx"
}

log := appctx.GetLogger(ctx)
log.Info().Msg("pkg/ocm/share/manager/json calls sender.Send")
log.Info().Msgf("pkg/ocm/share/manager/json shareWith: %s", g.Grantee.GetUserId().OpaqueId)
log.Info().Msgf("pkg/ocm/share/manager/json name: %s", name)
log.Info().Msgf("pkg/ocm/share/manager/json providerId: %s", fmt.Sprintf("%s:%s", md.StorageId, md.OpaqueId))
log.Info().Msgf("pkg/ocm/share/manager/json owner: %s", userID.OpaqueId)
log.Info().Msgf("pkg/ocm/share/manager/json protocol: %s", protocol)
log.Info().Msgf("pkg/ocm/share/manager/json meshProvider: %s", userID.Idp)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These seems to be debug logs, can you remove them? Or log them better?


requestBodyMap := map[string]interface{}{
"shareWith": g.Grantee.GetUserId().OpaqueId,
"name": name,
Expand All @@ -272,7 +282,7 @@ func (m *mgr) Share(ctx context.Context, md *provider.ResourceId, g *ocm.ShareGr
"protocol": protocol,
"meshProvider": userID.Idp, // FIXME: move this into the 'owner' string?
}
err = sender.Send(requestBodyMap, pi)
err = sender.Send(ctx, requestBodyMap, pi)
if err != nil {
err = errors.Wrap(err, "error sending OCM POST")
return nil, err
Expand Down
Loading