From cc2b5ed2f6c115a1853a1411f12f98e43adae656 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Tue, 28 Nov 2023 17:12:50 +0100 Subject: [PATCH 1/4] Bump reva to latest edge For https://github.com/cs3org/reva/pull/4366 --- go.mod | 2 +- go.sum | 4 +- .../reva/v2/cmd/revad/runtime/runtime.go | 81 +------------------ .../cs3org/reva/v2/pkg/logger/logger.go | 77 ++++++++++++++++++ .../utils/decomposedfs/upload/processing.go | 20 +++-- vendor/modules.txt | 2 +- 6 files changed, 95 insertions(+), 91 deletions(-) diff --git a/go.mod b/go.mod index 913253d9b62..262f3e8dd2d 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/coreos/go-oidc v2.2.1+incompatible github.com/coreos/go-oidc/v3 v3.7.0 github.com/cs3org/go-cs3apis v0.0.0-20231023073225-7748710e0781 - github.com/cs3org/reva/v2 v2.16.1-0.20231127154323-1e3027b83df5 + github.com/cs3org/reva/v2 v2.16.1-0.20231128104331-ea8d1336afc9 github.com/dhowden/tag v0.0.0-20230630033851-978a0926ee25 github.com/disintegration/imaging v1.6.2 github.com/dutchcoders/go-clamd v0.0.0-20170520113014-b970184f4d9e diff --git a/go.sum b/go.sum index 4241666f2d0..4c2dc3095fa 100644 --- a/go.sum +++ b/go.sum @@ -1017,8 +1017,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.16.1-0.20231127154323-1e3027b83df5 h1:+R3E25HVI3Ms9fzgPy8QNkby3u8x7oKpRerB/A9l5iM= -github.com/cs3org/reva/v2 v2.16.1-0.20231127154323-1e3027b83df5/go.mod h1:zcrrYVsBv/DwhpyO2/W5hoSZ/k6az6Z2EYQok65uqZY= +github.com/cs3org/reva/v2 v2.16.1-0.20231128104331-ea8d1336afc9 h1:5vKQcL1hPHEZKu9e8C9rl0ap3ofMBznmoSgi4lRYXec= +github.com/cs3org/reva/v2 v2.16.1-0.20231128104331-ea8d1336afc9/go.mod h1:zcrrYVsBv/DwhpyO2/W5hoSZ/k6az6Z2EYQok65uqZY= github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= diff --git a/vendor/github.com/cs3org/reva/v2/cmd/revad/runtime/runtime.go b/vendor/github.com/cs3org/reva/v2/cmd/revad/runtime/runtime.go index cc908d3e222..6bff8a9f797 100644 --- a/vendor/github.com/cs3org/reva/v2/cmd/revad/runtime/runtime.go +++ b/vendor/github.com/cs3org/reva/v2/cmd/revad/runtime/runtime.go @@ -20,7 +20,6 @@ package runtime import ( "fmt" - "io" "log" "net" "os" @@ -43,9 +42,8 @@ import ( // Run runs a reva server with the given config file and pid file. func Run(mainConf map[string]interface{}, pidFile, logLevel string) { - logConf := parseLogConfOrDie(mainConf["log"], logLevel) - logger := initLogger(logConf) - RunWithOptions(mainConf, pidFile, WithLogger(logger)) + log := logger.InitLoggerOrDie(mainConf["log"], logLevel) + RunWithOptions(mainConf, pidFile, WithLogger(log)) } // RunWithOptions runs a reva server with the given config file, pid file and options. @@ -180,15 +178,6 @@ func initCPUCount(conf *coreConf, log *zerolog.Logger) { log.Info().Msgf("running on %d cpus", ncpus) } -func initLogger(conf *logConf) *zerolog.Logger { - log, err := newLogger(conf) - if err != nil { - fmt.Fprintf(os.Stderr, "error creating logger, exiting ...") - os.Exit(1) - } - return log -} - func handlePIDFlag(l *zerolog.Logger, pidFile string, gracefulShutdownTimeout int) (*grace.Watcher, error) { w := grace.NewWatcher(grace.WithPIDFile(pidFile), grace.WithLogger(l.With().Str("pkg", "grace").Logger()), @@ -222,46 +211,6 @@ func start(mainConf map[string]interface{}, servers map[string]grace.Server, lis watcher.TrapSignals() } -func newLogger(conf *logConf) (*zerolog.Logger, error) { - // TODO(labkode): use debug level rather than info as default until reaching a stable version. - // Helps having smaller development files. - if conf.Level == "" { - conf.Level = zerolog.DebugLevel.String() - } - - var opts []logger.Option - opts = append(opts, logger.WithLevel(conf.Level)) - - w, err := getWriter(conf.Output) - if err != nil { - return nil, err - } - - opts = append(opts, logger.WithWriter(w, logger.Mode(conf.Mode))) - - l := logger.New(opts...) - sub := l.With().Int("pid", os.Getpid()).Logger() - return &sub, nil -} - -func getWriter(out string) (io.Writer, error) { - if out == "stderr" || out == "" { - return os.Stderr, nil - } - - if out == "stdout" { - return os.Stdout, nil - } - - fd, err := os.OpenFile(out, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) - if err != nil { - err = errors.Wrap(err, "error creating log file: "+out) - return nil, err - } - - return fd, nil -} - func getGRPCServer(conf interface{}, l *zerolog.Logger, tp trace.TracerProvider) (*rgrpc.Server, error) { sub := l.With().Str("pkg", "rgrpc").Logger() s, err := rgrpc.NewServer(conf, sub, tp) @@ -348,32 +297,6 @@ func parseSharedConfOrDie(v interface{}) { } } -func parseLogConfOrDie(v interface{}, logLevel string) *logConf { - c := &logConf{} - if err := mapstructure.Decode(v, c); err != nil { - fmt.Fprintf(os.Stderr, "error decoding log config: %s\n", err.Error()) - os.Exit(1) - } - - // if mode is not set, we use console mode, easier for devs - if c.Mode == "" { - c.Mode = "console" - } - - // Give priority to the log level passed through the command line. - if logLevel != "" { - c.Level = logLevel - } - - return c -} - -type logConf struct { - Output string `mapstructure:"output"` - Mode string `mapstructure:"mode"` - Level string `mapstructure:"level"` -} - func isEnabledHTTP(conf map[string]interface{}) bool { return isEnabled("http", conf) } diff --git a/vendor/github.com/cs3org/reva/v2/pkg/logger/logger.go b/vendor/github.com/cs3org/reva/v2/pkg/logger/logger.go index 7c35a7ecd29..b2f82cdcfb5 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/logger/logger.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/logger/logger.go @@ -19,10 +19,13 @@ package logger import ( + "fmt" "io" "os" "time" + "github.com/mitchellh/mapstructure" + "github.com/pkg/errors" "github.com/rs/zerolog" ) @@ -85,3 +88,77 @@ func parseLevel(v string) zerolog.Level { return lvl } + +func InitLoggerOrDie(v interface{}, logLevel string) *zerolog.Logger { + conf := ParseLogConfOrDie(v, logLevel) + log, err := FromConfig(conf) + if err != nil { + fmt.Fprintf(os.Stderr, "error creating logger, exiting ...") + os.Exit(1) + } + return log +} + +func ParseLogConfOrDie(v interface{}, logLevel string) *LogConf { + c := &LogConf{} + if err := mapstructure.Decode(v, c); err != nil { + fmt.Fprintf(os.Stderr, "error decoding log config: %s\n", err.Error()) + os.Exit(1) + } + + // if mode is not set, we use console mode, easier for devs + if c.Mode == "" { + c.Mode = "console" + } + + // Give priority to the log level passed through the command line. + if logLevel != "" { + c.Level = logLevel + } + + return c +} + +type LogConf struct { + Output string `mapstructure:"output"` + Mode string `mapstructure:"mode"` + Level string `mapstructure:"level"` +} + +func FromConfig(conf *LogConf) (*zerolog.Logger, error) { + if conf.Level == "" { + conf.Level = zerolog.DebugLevel.String() + } + + var opts []Option + opts = append(opts, WithLevel(conf.Level)) + + w, err := getWriter(conf.Output) + if err != nil { + return nil, err + } + + opts = append(opts, WithWriter(w, Mode(conf.Mode))) + + l := New(opts...) + sub := l.With().Int("pid", os.Getpid()).Logger() + return &sub, nil +} + +func getWriter(out string) (io.Writer, error) { + if out == "stderr" || out == "" { + return os.Stderr, nil + } + + if out == "stdout" { + return os.Stdout, nil + } + + fd, err := os.OpenFile(out, os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0o644) + if err != nil { + err = errors.Wrap(err, "error creating log file: "+out) + return nil, err + } + + return fd, nil +} diff --git a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/upload/processing.go b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/upload/processing.go index d70f7f44a81..462716562e4 100644 --- a/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/upload/processing.go +++ b/vendor/github.com/cs3org/reva/v2/pkg/storage/utils/decomposedfs/upload/processing.go @@ -224,17 +224,21 @@ func Get(ctx context.Context, id string, lu *lookup.Lookup, tp Tree, fsRoot stri } ctx = ctxpkg.ContextSetUser(ctx, u) - // TODO configure the logger the same way ... store and add traceid in file info - - var opts []logger.Option - opts = append(opts, logger.WithLevel(info.Storage["LogLevel"])) - opts = append(opts, logger.WithWriter(os.Stderr, logger.ConsoleMode)) - l := logger.New(opts...) - - sub := l.With().Int("pid", os.Getpid()).Logger() + // restore logger from file info + log, err := logger.FromConfig(&logger.LogConf{ + Output: "stderr", // TODO use config from decomposedfs + Mode: "json", // TODO use config from decomposedfs + Level: info.Storage["LogLevel"], + }) + if err != nil { + return nil, err + } + sub := log.With().Int("pid", os.Getpid()).Logger() ctx = appctx.WithLogger(ctx, &sub) + // TODO store and add traceid in file info + up := buildUpload(ctx, info, info.Storage["BinPath"], infoPath, lu, tp, pub, async, tknopts) up.versionsPath = info.MetaData["versionsPath"] up.SizeDiff, _ = strconv.ParseInt(info.MetaData["sizeDiff"], 10, 64) diff --git a/vendor/modules.txt b/vendor/modules.txt index 7140ff2e427..70ad8b9f4eb 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -357,7 +357,7 @@ github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1 github.com/cs3org/go-cs3apis/cs3/storage/registry/v1beta1 github.com/cs3org/go-cs3apis/cs3/tx/v1beta1 github.com/cs3org/go-cs3apis/cs3/types/v1beta1 -# github.com/cs3org/reva/v2 v2.16.1-0.20231127154323-1e3027b83df5 +# github.com/cs3org/reva/v2 v2.16.1-0.20231128104331-ea8d1336afc9 ## explicit; go 1.20 github.com/cs3org/reva/v2/cmd/revad/internal/grace github.com/cs3org/reva/v2/cmd/revad/runtime From f3a2b73400d71f58228404ddac6dc9c8603328d3 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 29 Nov 2023 10:11:12 +0100 Subject: [PATCH 2/4] graph/errorcode: Map a few more CS3 status codes --- services/graph/pkg/errorcode/cs3.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/services/graph/pkg/errorcode/cs3.go b/services/graph/pkg/errorcode/cs3.go index a8956cb79b5..129e464e3d3 100644 --- a/services/graph/pkg/errorcode/cs3.go +++ b/services/graph/pkg/errorcode/cs3.go @@ -40,8 +40,16 @@ func FromCS3Status(status *cs3rpc.Status, ignore ...cs3rpc.Code) *Error { err.errorCode = NameAlreadyExists case code == cs3rpc.Code_CODE_FAILED_PRECONDITION: err.errorCode = PreconditionFailed + case code == cs3rpc.Code_CODE_OUT_OF_RANGE: + err.errorCode = InvalidRange case code == cs3rpc.Code_CODE_UNIMPLEMENTED: err.errorCode = NotSupported + case code == cs3rpc.Code_CODE_UNAVAILABLE: + err.errorCode = ServiceNotAvailable + case code == cs3rpc.Code_CODE_INSUFFICIENT_STORAGE: + err.errorCode = QuotaLimitReached + case code == cs3rpc.Code_CODE_LOCKED: + err.errorCode = ItemIsLocked } return err From 7052375fe556e6717fc5c432193b98365fa1a7a2 Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Wed, 29 Nov 2023 11:23:15 +0100 Subject: [PATCH 3/4] graph/errocode: Rework FromCS3Status The handling of 'error' has been moved from FromStat() to FromCS3Status(). It's generally useful for other users of FromCS3Status() --- services/graph/pkg/errorcode/cs3.go | 23 +++++----- services/graph/pkg/errorcode/cs3_test.go | 48 +++++++++++---------- services/graph/pkg/service/v0/driveitems.go | 9 +--- 3 files changed, 39 insertions(+), 41 deletions(-) diff --git a/services/graph/pkg/errorcode/cs3.go b/services/graph/pkg/errorcode/cs3.go index 129e464e3d3..756cf271a46 100644 --- a/services/graph/pkg/errorcode/cs3.go +++ b/services/graph/pkg/errorcode/cs3.go @@ -7,15 +7,21 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" ) -// FromCS3Status converts a CS3 status code into a corresponding local Error representation. +// FromCS3Status converts a CS3 status code and an error into a corresponding local Error representation. // -// It evaluates the provided CS3 status code and returns an equivalent graph Error. +// It takes a *cs3rpc.Status, an error, and a variadic parameter of type cs3rpc.Code. +// If the error is not nil, it creates an Error object with the error message and a GeneralException code. +// If the error is nil, it evaluates the provided CS3 status code and returns an equivalent graph Error. // If the CS3 status code does not have a direct equivalent within the app, // or is ignored, a general purpose Error is returned. // // 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, ignore ...cs3rpc.Code) *Error { +func FromCS3Status(status *cs3rpc.Status, inerr error, ignore ...cs3rpc.Code) *Error { + if inerr != nil { + return &Error{msg: inerr.Error(), errorCode: GeneralException} + } + err := &Error{errorCode: GeneralException, msg: "unspecified error has occurred"} if status != nil { @@ -58,13 +64,8 @@ func FromCS3Status(status *cs3rpc.Status, ignore ...cs3rpc.Code) *Error { // FromStat transforms a *provider.StatResponse object and an error into an *Error. // // It takes a stat of type *provider.StatResponse, an error, and a variadic parameter of type cs3rpc.Code. -// If the error is not nil, it creates an Error object with the error message and a GeneralException code. -// If the error is nil, it invokes the FromCS3Status function with the StatResponse Status and the ignore codes. +// It invokes the FromCS3Status function with the StatResponse Status and the ignore codes. func FromStat(stat *provider.StatResponse, err error, ignore ...cs3rpc.Code) *Error { - switch { - case err != nil: - return &Error{msg: err.Error(), errorCode: GeneralException} - default: - return FromCS3Status(stat.GetStatus(), ignore...) - } + // TODO: look into ResourceInfo to get the postprocessing state and map that to 425 status? + return FromCS3Status(stat.GetStatus(), err, ignore...) } diff --git a/services/graph/pkg/errorcode/cs3_test.go b/services/graph/pkg/errorcode/cs3_test.go index b53fe37116c..22616342e59 100644 --- a/services/graph/pkg/errorcode/cs3_test.go +++ b/services/graph/pkg/errorcode/cs3_test.go @@ -15,35 +15,37 @@ import ( func TestFromCS3Status(t *testing.T) { var tests = []struct { status *cs3rpc.Status + err error ignore []cs3rpc.Code result *errorcode.Error }{ - {nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "unspecified error has occurred"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_OK}, nil, nil}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_NOT_FOUND}, []cs3rpc.Code{cs3rpc.Code_CODE_NOT_FOUND}, nil}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_PERMISSION_DENIED}, []cs3rpc.Code{cs3rpc.Code_CODE_NOT_FOUND, cs3rpc.Code_CODE_PERMISSION_DENIED}, nil}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_NOT_FOUND, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.ItemNotFound, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_PERMISSION_DENIED, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.AccessDenied, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAUTHENTICATED, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.Unauthenticated, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID_ARGUMENT, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.InvalidRequest, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_ALREADY_EXISTS, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.NameAlreadyExists, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_FAILED_PRECONDITION, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.PreconditionFailed, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNIMPLEMENTED, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.NotSupported, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_CANCELLED, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNKNOWN, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_RESOURCE_EXHAUSTED, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_ABORTED, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_OUT_OF_RANGE, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INTERNAL, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAVAILABLE, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_REDIRECTION, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INSUFFICIENT_STORAGE, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, - {&cs3rpc.Status{Code: cs3rpc.Code_CODE_LOCKED, Message: "msg"}, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {nil, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "unspecified error has occurred"))}, + {nil, errors.New("test error"), nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "test error"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_OK}, nil, nil, nil}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_NOT_FOUND}, nil, []cs3rpc.Code{cs3rpc.Code_CODE_NOT_FOUND}, nil}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_PERMISSION_DENIED}, nil, []cs3rpc.Code{cs3rpc.Code_CODE_NOT_FOUND, cs3rpc.Code_CODE_PERMISSION_DENIED}, nil}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_NOT_FOUND, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.ItemNotFound, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_PERMISSION_DENIED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.AccessDenied, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAUTHENTICATED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.Unauthenticated, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID_ARGUMENT, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.InvalidRequest, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_ALREADY_EXISTS, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.NameAlreadyExists, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_FAILED_PRECONDITION, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.PreconditionFailed, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNIMPLEMENTED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.NotSupported, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INVALID, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_CANCELLED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNKNOWN, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_RESOURCE_EXHAUSTED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_ABORTED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_OUT_OF_RANGE, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.InvalidRange, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INTERNAL, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_UNAVAILABLE, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.ServiceNotAvailable, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_REDIRECTION, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.GeneralException, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_INSUFFICIENT_STORAGE, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.QuotaLimitReached, "msg"))}, + {&cs3rpc.Status{Code: cs3rpc.Code_CODE_LOCKED, Message: "msg"}, nil, nil, conversions.ToPointer(errorcode.New(errorcode.ItemIsLocked, "msg"))}, } for _, test := range tests { - if output := errorcode.FromCS3Status(test.status, test.ignore...); !reflect.DeepEqual(output, test.result) { + if output := errorcode.FromCS3Status(test.status, test.err, test.ignore...); !reflect.DeepEqual(output, test.result) { t.Error("Test Failed: {} expected, recieved: {}", test.result, output) } } diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 7124c3995a9..0f341fb05fb 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -536,13 +536,8 @@ func (g Graph) DeletePermission(w http.ResponseWriter, r *http.Request) { }, }, }) - switch { - case err != nil: - fallthrough - case getShareResp.Status.GetCode() != cs3rpc.Code_CODE_OK: - g.logger.Debug().Err(err).Interface("permissionID", permissionID).Interface("GetShare", getShareResp).Msg("GetShare failed") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) - return + if errCode := errorcode.FromCS3Status(getShareResp.GetStatus(), err); errCode != nil { + return nil, err } sharedResourceId := getShareResp.GetShare().GetResourceId() From c487a6146f08dfbfa86a250d24ece4462d4750ac Mon Sep 17 00:00:00 2001 From: Ralf Haferkamp Date: Tue, 28 Nov 2023 17:13:30 +0100 Subject: [PATCH 4/4] graph sharing: delete link permission Allow to delete link permissions (i.e. Public Shares) --- services/graph/pkg/errorcode/errorcode.go | 4 + services/graph/pkg/service/v0/driveitems.go | 143 +++++++++++++----- .../graph/pkg/service/v0/driveitems_test.go | 97 +++++++----- 3 files changed, 162 insertions(+), 82 deletions(-) diff --git a/services/graph/pkg/errorcode/errorcode.go b/services/graph/pkg/errorcode/errorcode.go index dfa0129e9b8..fdaf94033ab 100644 --- a/services/graph/pkg/errorcode/errorcode.go +++ b/services/graph/pkg/errorcode/errorcode.go @@ -154,6 +154,10 @@ func (e Error) Error() string { return errString } +func (e Error) GetCode() ErrorCode { + return e.errorCode +} + // 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 diff --git a/services/graph/pkg/service/v0/driveitems.go b/services/graph/pkg/service/v0/driveitems.go index 0f341fb05fb..14046815dbc 100644 --- a/services/graph/pkg/service/v0/driveitems.go +++ b/services/graph/pkg/service/v0/driveitems.go @@ -504,14 +504,7 @@ func (g Graph) Invite(w http.ResponseWriter, r *http.Request) { // DeletePermission removes a Permission from a Drive item func (g Graph) DeletePermission(w http.ResponseWriter, r *http.Request) { - gatewayClient, err := g.gatewaySelector.Next() - if err != nil { - g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) - return - } - - _, itemID, err := g.extractDriveAndDriveItem(r) + _, itemID, err := g.GetDriveAndItemIDParam(r) if err != nil { errorcode.RenderError(w, r, err) return @@ -526,6 +519,56 @@ func (g Graph) DeletePermission(w http.ResponseWriter, r *http.Request) { } ctx := r.Context() + isUserPermission := true + + // Check if the id is refering to a User Share + sharedResourceId, err := g.getUserPermissionResourceID(ctx, permissionID) + var errcode *errorcode.Error + if err != nil && errors.As(err, &errcode) && errcode.GetCode() == errorcode.ItemNotFound { + // there is no user share with that ID, so lets check if it is refering to a public link + isUserPermission = false + sharedResourceId, err = g.getLinkPermissionResourceID(ctx, permissionID) + } + + if err != nil { + errorcode.RenderError(w, r, err) + return + } + + // The resourceID of the shared resource need to match the item ID from the Request Path + // otherwise this is an invalid Request. + if sharedResourceId.GetStorageId() != itemID.GetStorageId() || + sharedResourceId.GetSpaceId() != itemID.GetSpaceId() || + sharedResourceId.GetOpaqueId() != itemID.GetOpaqueId() { + g.logger.Debug().Msg("resourceID of shared does not match itemID") + errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "permissionID and itemID do not match") + return + } + + if isUserPermission { + err = g.removeUserShare(ctx, permissionID) + } else { + err = g.removePublicShare(ctx, permissionID) + } + + if err != nil { + errorcode.RenderError(w, r, err) + return + } + + render.Status(r, http.StatusNoContent) + render.NoContent(w, r) + + return +} + +func (g Graph) getUserPermissionResourceID(ctx context.Context, permissionID string) (*storageprovider.ResourceId, error) { + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") + return nil, err + } + getShareResp, err := gatewayClient.GetShare(ctx, &collaboration.GetShareRequest{ Ref: &collaboration.ShareReference{ @@ -537,18 +580,16 @@ func (g Graph) DeletePermission(w http.ResponseWriter, r *http.Request) { }, }) if errCode := errorcode.FromCS3Status(getShareResp.GetStatus(), err); errCode != nil { - return nil, err + return nil, errCode } + return getShareResp.Share.GetResourceId(), nil +} - sharedResourceId := getShareResp.GetShare().GetResourceId() - // The resourceID of the shared resource need to matched the item ID from the Request Path - // otherwise this is an invalid Request. - if sharedResourceId.GetStorageId() != itemID.GetStorageId() || - sharedResourceId.GetSpaceId() != itemID.GetSpaceId() || - sharedResourceId.GetOpaqueId() != itemID.GetOpaqueId() { - g.logger.Debug().Msg("resourceID of shared does not match itemID") - errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "permissionID and itemID do not match") - return +func (g Graph) removeUserShare(ctx context.Context, permissionID string) error { + gatewayClient, err := g.gatewaySelector.Next() + if err != nil { + g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") + return err } removeShareResp, err := gatewayClient.RemoveShare(ctx, @@ -561,38 +602,60 @@ func (g Graph) DeletePermission(w http.ResponseWriter, r *http.Request) { }, }, }) - switch { - case err != nil: - fallthrough - case removeShareResp.Status.GetCode() != cs3rpc.Code_CODE_OK: - g.logger.Debug().Err(err).Interface("permissionID", permissionID).Interface("GetShare", getShareResp).Msg("GetShare failed") - errorcode.GeneralException.Render(w, r, http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)) - return - } - render.Status(r, http.StatusNoContent) - render.NoContent(w, r) - return + if errCode := errorcode.FromCS3Status(removeShareResp.GetStatus(), err); errCode != nil { + return errCode + } + // We need to return an untyped nil here otherwise the error==nil check won't work + return nil } -func (g Graph) extractDriveAndDriveItem(r *http.Request) (driveID storageprovider.ResourceId, itemID storageprovider.ResourceId, err error) { - driveID, err = parseIDParam(r, "driveID") +func (g Graph) getLinkPermissionResourceID(ctx context.Context, permissionID string) (*storageprovider.ResourceId, error) { + gatewayClient, err := g.gatewaySelector.Next() if err != nil { - g.logger.Debug().Err(err).Msg("could not parse driveID") - return driveID, itemID, errorcode.New(errorcode.InvalidRequest, "invalid driveID") + g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") + return nil, err } - itemID, err = parseIDParam(r, "itemID") + getPublicShareResp, err := gatewayClient.GetPublicShare(ctx, + &link.GetPublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: permissionID, + }, + }, + }, + }, + ) + if errCode := errorcode.FromCS3Status(getPublicShareResp.GetStatus(), err); errCode != nil { + return nil, errCode + } + return getPublicShareResp.Share.GetResourceId(), nil +} + +func (g Graph) removePublicShare(ctx context.Context, permissionID string) error { + gatewayClient, err := g.gatewaySelector.Next() if err != nil { - g.logger.Debug().Err(err).Msg("could not parse itemID") - return driveID, itemID, errorcode.New(errorcode.InvalidRequest, "invalid itemID") + g.logger.Debug().Err(err).Msg("selecting gatewaySelector failed") + return err } - if driveID.GetStorageId() != itemID.GetStorageId() || driveID.GetSpaceId() != itemID.GetSpaceId() { - g.logger.Debug().Interface("driveID", driveID).Interface("itemID", itemID).Msg("driveID and itemID do not match") - return driveID, itemID, errorcode.New(errorcode.InvalidRequest, "driveID and itemID do not match") + removePublicShareResp, err := gatewayClient.RemovePublicShare(ctx, + &link.RemovePublicShareRequest{ + Ref: &link.PublicShareReference{ + Spec: &link.PublicShareReference_Id{ + Id: &link.PublicShareId{ + OpaqueId: permissionID, + }, + }, + }, + }) + if errcode := errorcode.FromCS3Status(removePublicShareResp.GetStatus(), err); errcode != nil { + return errcode } - return driveID, itemID, nil + // We need to return an untyped nil here otherwise the error==nil check won't work + return nil } func (g Graph) getDriveItem(ctx context.Context, ref storageprovider.Reference) (*libregraph.DriveItem, error) { diff --git a/services/graph/pkg/service/v0/driveitems_test.go b/services/graph/pkg/service/v0/driveitems_test.go index a3ef19371ff..827bbc217b3 100644 --- a/services/graph/pkg/service/v0/driveitems_test.go +++ b/services/graph/pkg/service/v0/driveitems_test.go @@ -105,39 +105,43 @@ var _ = Describe("Driveitems", func() { }) Describe("DeletePermission", func() { - It("validates the driveID", func() { - rctx := chi.NewRouteContext() - rctx.URLParams.Add("driveID", "") - - ctx = context.WithValue(context.Background(), chi.RouteCtxKey, rctx) - - svc.DeletePermission( - rr, - httptest.NewRequest(http.MethodPost, "/", nil).WithContext(ctx), + It("deletes a user permission as expected", func() { + getShareMock := gatewayClient.On("GetShare", + mock.Anything, + mock.MatchedBy(func(req *collaboration.GetShareRequest) bool { + return req.GetRef().GetId().GetOpaqueId() == "permissionid" + }), ) + getShareMockResponse := &collaboration.GetShareResponse{ + Status: status.NewOK(ctx), + Share: &collaboration.Share{ + Id: &collaboration.ShareId{ + OpaqueId: "permissionid", + }, + ResourceId: &provider.ResourceId{ + StorageId: "1", + SpaceId: "2", + OpaqueId: "3", + }, + }, + } + getShareMock.Return(getShareMockResponse, nil) - Expect(rr.Code).To(Equal(http.StatusBadRequest)) - }) - - It("validates the itemID", func() { - rctx := chi.NewRouteContext() - rctx.URLParams.Add("driveID", "f0042750-23c5-441c-9f2c-ff7c53e5bd2a$cd621428-dfbe-44c1-9393-65bf0dd440a6!cd621428-dfbe-44c1-9393-65bf0dd440a6") - rctx.URLParams.Add("itemID", "") - - ctx = context.WithValue(context.Background(), chi.RouteCtxKey, rctx) - - svc.DeletePermission( - rr, - httptest.NewRequest(http.MethodPost, "/", nil).WithContext(ctx), + rmShareMock := gatewayClient.On("RemoveShare", + mock.Anything, + mock.MatchedBy(func(req *collaboration.RemoveShareRequest) bool { + return req.GetRef().GetId().GetOpaqueId() == "permissionid" + }), ) + rmShareMockResponse := &collaboration.RemoveShareResponse{ + Status: status.NewOK(ctx), + } + rmShareMock.Return(rmShareMockResponse, nil) - Expect(rr.Code).To(Equal(http.StatusBadRequest)) - }) - - It("checks if the itemID and driveID is compatible to each other", func() { rctx := chi.NewRouteContext() - rctx.URLParams.Add("driveID", "1$2!3") - rctx.URLParams.Add("itemID", "4$5!6") + rctx.URLParams.Add("driveID", "1$2") + rctx.URLParams.Add("itemID", "1$2!3") + rctx.URLParams.Add("permissionID", "permissionid") ctx = context.WithValue(context.Background(), chi.RouteCtxKey, rctx) @@ -146,20 +150,30 @@ var _ = Describe("Driveitems", func() { httptest.NewRequest(http.MethodPost, "/", nil).WithContext(ctx), ) - Expect(rr.Code).To(Equal(http.StatusBadRequest)) + Expect(rr.Code).To(Equal(http.StatusNoContent)) }) - - It("deletes a permission as expected", func() { + It("deletes a link permission as expected", func() { getShareMock := gatewayClient.On("GetShare", mock.Anything, mock.MatchedBy(func(req *collaboration.GetShareRequest) bool { - return req.GetRef().GetId().GetOpaqueId() == "permissionid" + return req.GetRef().GetId().GetOpaqueId() == "linkpermissionid" }), ) getShareMockResponse := &collaboration.GetShareResponse{ + Status: status.NewNotFound(ctx, "not found"), + } + getShareMock.Return(getShareMockResponse, nil) + + getPublicShareMock := gatewayClient.On("GetPublicShare", + mock.Anything, + mock.MatchedBy(func(req *link.GetPublicShareRequest) bool { + return req.GetRef().GetId().GetOpaqueId() == "linkpermissionid" + }), + ) + getPublicShareMock.Return(&link.GetPublicShareResponse{ Status: status.NewOK(ctx), - Share: &collaboration.Share{ - Id: &collaboration.ShareId{ + Share: &link.PublicShare{ + Id: &link.PublicShareId{ OpaqueId: "permissionid", }, ResourceId: &provider.ResourceId{ @@ -168,24 +182,23 @@ var _ = Describe("Driveitems", func() { OpaqueId: "3", }, }, - } - getShareMock.Return(getShareMockResponse, nil) + }, nil) - rmShareMock := gatewayClient.On("RemoveShare", + rmPublicShareMock := gatewayClient.On("RemovePublicShare", mock.Anything, - mock.MatchedBy(func(req *collaboration.RemoveShareRequest) bool { - return req.GetRef().GetId().GetOpaqueId() == "permissionid" + mock.MatchedBy(func(req *link.RemovePublicShareRequest) bool { + return req.GetRef().GetId().GetOpaqueId() == "linkpermissionid" }), ) - rmShareMockResponse := &collaboration.RemoveShareResponse{ + rmPublicShareMockResponse := &link.RemovePublicShareResponse{ Status: status.NewOK(ctx), } - rmShareMock.Return(rmShareMockResponse, nil) + rmPublicShareMock.Return(rmPublicShareMockResponse, nil) rctx := chi.NewRouteContext() rctx.URLParams.Add("driveID", "1$2") rctx.URLParams.Add("itemID", "1$2!3") - rctx.URLParams.Add("permissionID", "permissionid") + rctx.URLParams.Add("permissionID", "linkpermissionid") ctx = context.WithValue(context.Background(), chi.RouteCtxKey, rctx)