diff --git a/.golangci.yaml b/.golangci.yaml index 4df26105db..29a1bd2991 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -26,7 +26,7 @@ linters: - revive - goimports - unconvert - - exportloopref + - copyloopvar - misspell - gocritic - prealloc diff --git a/Makefile b/Makefile index ea4233dfe2..e4cf40a255 100644 --- a/Makefile +++ b/Makefile @@ -30,7 +30,7 @@ toolchain-clean: $(GOLANGCI_LINT): @mkdir -p $(@D) - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | BINDIR=$(@D) sh -s v1.55.2 + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | BINDIR=$(@D) sh -s v1.61.0 .PHONY: check-changelog lint: $(GOLANGCI_LINT) diff --git a/changelog/unreleased/bump-golangci-lint.md b/changelog/unreleased/bump-golangci-lint.md new file mode 100644 index 0000000000..feee535381 --- /dev/null +++ b/changelog/unreleased/bump-golangci-lint.md @@ -0,0 +1,3 @@ +Enhancement: Bump golangci-lint to 1.61.0 + +https://github.com/cs3org/reva/pull/4890 diff --git a/internal/grpc/services/storageprovider/storageprovider.go b/internal/grpc/services/storageprovider/storageprovider.go index 872105788e..80c6ef0b84 100644 --- a/internal/grpc/services/storageprovider/storageprovider.go +++ b/internal/grpc/services/storageprovider/storageprovider.go @@ -720,7 +720,7 @@ func (s *Service) Delete(ctx context.Context, req *provider.DeleteRequest) (*pro if req.Opaque != nil { if _, ok := req.Opaque.Map["deleting_shared_resource"]; ok { // it is a binary key; its existence signals true. Although, do not assume. - ctx = context.WithValue(ctx, appctx.DeletingSharedResource, true) + ctx = appctx.WithDeletingSharedResource(ctx) } } @@ -1095,6 +1095,16 @@ func (s *Service) DenyGrant(ctx context.Context, req *provider.DenyGrantRequest) return res, nil } +type spaceTypeKey struct{} + +func WithSpaceType(ctx context.Context, spaceType string) context.Context { + return context.WithValue(ctx, spaceTypeKey{}, spaceType) +} +func SpaceTypeFromContext(ctx context.Context) (string, bool) { + spaceType, ok := ctx.Value(spaceTypeKey{}).(string) + return spaceType, ok +} + func (s *Service) AddGrant(ctx context.Context, req *provider.AddGrantRequest) (*provider.AddGrantResponse, error) { ctx = ctxpkg.ContextSetLockID(ctx, req.LockId) @@ -1102,13 +1112,7 @@ func (s *Service) AddGrant(ctx context.Context, req *provider.AddGrantRequest) ( // FIXME these should be part of the AddGrantRequest object // https://github.com/owncloud/ocis/issues/4312 if utils.ExistsInOpaque(req.Opaque, "spacegrant") { - ctx = context.WithValue( - ctx, - utils.SpaceGrant, - struct{ SpaceType string }{ - SpaceType: utils.ReadPlainFromOpaque(req.Opaque, "spacetype"), - }, - ) + ctx = WithSpaceType(ctx, utils.ReadPlainFromOpaque(req.Opaque, "spacetype")) } // check grantee type is valid @@ -1137,13 +1141,7 @@ func (s *Service) UpdateGrant(ctx context.Context, req *provider.UpdateGrantRequ // FIXME these should be part of the AddGrantRequest object // https://github.com/owncloud/ocis/issues/4312 if utils.ExistsInOpaque(req.Opaque, "spacegrant") { - ctx = context.WithValue( - ctx, - utils.SpaceGrant, - struct{ SpaceType string }{ - SpaceType: utils.ReadPlainFromOpaque(req.Opaque, "spacetype"), - }, - ) + ctx = WithSpaceType(ctx, utils.ReadPlainFromOpaque(req.Opaque, "spacetype")) } // check grantee type is valid @@ -1174,7 +1172,7 @@ func (s *Service) RemoveGrant(ctx context.Context, req *provider.RemoveGrantRequ // FIXME these should be part of the RemoveGrantRequest object // https://github.com/owncloud/ocis/issues/4312 if utils.ExistsInOpaque(req.Opaque, "spacegrant") { - ctx = context.WithValue(ctx, utils.SpaceGrant, struct{}{}) + ctx = WithSpaceType(ctx, "") } err := s.Storage.RemoveGrant(ctx, req.Ref, req.Grant) diff --git a/internal/http/services/owncloud/ocdav/errors/error.go b/internal/http/services/owncloud/ocdav/errors/error.go index 2709eb5ca3..69cc31afd8 100644 --- a/internal/http/services/owncloud/ocdav/errors/error.go +++ b/internal/http/services/owncloud/ocdav/errors/error.go @@ -21,7 +21,6 @@ package errors import ( "bytes" "encoding/xml" - "fmt" "net/http" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" @@ -210,6 +209,6 @@ func NewErrFromStatus(s *rpc.Status) error { case rpc.Code_CODE_UNIMPLEMENTED: return ErrNotImplemented default: - return fmt.Errorf(s.GetMessage()) + return errors.New(s.GetMessage()) } } diff --git a/internal/http/services/owncloud/ocdav/mkcol.go b/internal/http/services/owncloud/ocdav/mkcol.go index 1a1f57e75b..4e88461671 100644 --- a/internal/http/services/owncloud/ocdav/mkcol.go +++ b/internal/http/services/owncloud/ocdav/mkcol.go @@ -148,13 +148,13 @@ func (s *svc) handleMkcol(ctx context.Context, w http.ResponseWriter, r *http.Re } return http.StatusForbidden, errors.New(sRes.Status.Message) case res.Status.Code == rpc.Code_CODE_ABORTED: - return http.StatusPreconditionFailed, fmt.Errorf(res.Status.Message) + return http.StatusPreconditionFailed, errors.New(res.Status.Message) case res.Status.Code == rpc.Code_CODE_FAILED_PRECONDITION: // https://www.rfc-editor.org/rfc/rfc4918#section-9.3.1: // 409 (Conflict) - A collection cannot be made at the Request-URI until // one or more intermediate collections have been created. The server // MUST NOT create those intermediate collections automatically. - return http.StatusConflict, fmt.Errorf(res.Status.Message) + return http.StatusConflict, errors.New(res.Status.Message) case res.Status.Code == rpc.Code_CODE_ALREADY_EXISTS: // https://www.rfc-editor.org/rfc/rfc4918#section-9.3.1: // 405 (Method Not Allowed) - MKCOL can only be executed on an unmapped URL. diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go index 9502862446..a36972142f 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/pending.go @@ -344,7 +344,7 @@ func getSharedResource(ctx context.Context, client gateway.GatewayAPIClient, res e := fmt.Errorf("not found") return nil, arbitraryOcsResponse(response.MetaNotFound.StatusCode, e.Error()) } - e := fmt.Errorf(res.GetStatus().GetMessage()) + errors.New(res.GetStatus().GetMessage()) return nil, arbitraryOcsResponse(response.MetaServerError.StatusCode, e.Error()) } diff --git a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go index 5d483fbd47..29221fa859 100644 --- a/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go +++ b/internal/http/services/owncloud/ocs/handlers/apps/sharing/shares/shares_test.go @@ -671,7 +671,6 @@ var _ = Describe("The ocs API", func() { Context("when change the permission", func() { for _, perm := range []string{"4", "5", "15"} { - perm := perm It("the password exists. update succeed", func() { form := url.Values{} form.Add("permissions", perm) diff --git a/pkg/appctx/appctx.go b/pkg/appctx/appctx.go index 78fe950d19..067cd4d844 100644 --- a/pkg/appctx/appctx.go +++ b/pkg/appctx/appctx.go @@ -27,8 +27,15 @@ import ( "go.opentelemetry.io/otel/trace" ) -// DeletingSharedResource flags to a storage a shared resource is being deleted not by the owner. -var DeletingSharedResource struct{} +// deletingSharedResource flags to a storage a shared resource is being deleted not by the owner. +type deletingSharedResource struct{} + +func WithDeletingSharedResource(ctx context.Context) context.Context { + return context.WithValue(ctx, deletingSharedResource{}, struct{}{}) +} +func DeletingSharedResourceFromContext(ctx context.Context) bool { + return ctx.Value(deletingSharedResource{}) != nil +} // WithLogger returns a context with an associated logger. func WithLogger(ctx context.Context, l *zerolog.Logger) context.Context { diff --git a/pkg/mentix/exchangers/importers/importer.go b/pkg/mentix/exchangers/importers/importer.go index bd1c58672b..517518c8e4 100755 --- a/pkg/mentix/exchangers/importers/importer.go +++ b/pkg/mentix/exchangers/importers/importer.go @@ -19,6 +19,7 @@ package importers import ( + "errors" "fmt" "strings" "sync" @@ -70,7 +71,7 @@ func (importer *BaseImporter) Process(connectors *connectors.Collection) (bool, var err error if len(processErrs) != 0 { - err = fmt.Errorf(strings.Join(processErrs, "; ")) + err = errors.New(strings.Join(processErrs, "; ")) } return true, err } diff --git a/pkg/ocm/share/repository/nextcloud/nextcloud.go b/pkg/ocm/share/repository/nextcloud/nextcloud.go index 0ce4ac817a..7b89faf06f 100644 --- a/pkg/ocm/share/repository/nextcloud/nextcloud.go +++ b/pkg/ocm/share/repository/nextcloud/nextcloud.go @@ -25,7 +25,6 @@ import ( "fmt" "io" "net/http" - "strconv" "strings" userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" @@ -431,7 +430,7 @@ func (sm *Manager) do(ctx context.Context, a Action, username string) (int, []by 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 0, nil, fmt.Errorf("Unexpected response code from EFSS API: %d", resp.StatusCode) } return resp.StatusCode, body, nil } diff --git a/pkg/storage/fs/posix/tree/tree.go b/pkg/storage/fs/posix/tree/tree.go index be08736b9a..f41ab1dfb9 100644 --- a/pkg/storage/fs/posix/tree/tree.go +++ b/pkg/storage/fs/posix/tree/tree.go @@ -467,9 +467,7 @@ func (t *Tree) Delete(ctx context.Context, n *node.Node) error { // remove entry from cache immediately to avoid inconsistencies defer func() { _ = t.idCache.Delete(path) }() - deletingSharedResource := ctx.Value(appctx.DeletingSharedResource) - - if deletingSharedResource != nil && deletingSharedResource.(bool) { + if appctx.DeletingSharedResourceFromContext(ctx) { src := filepath.Join(n.ParentPath(), n.Name) return os.RemoveAll(src) } diff --git a/pkg/storage/utils/decomposedfs/grants.go b/pkg/storage/utils/decomposedfs/grants.go index 227a2d5d49..167fb6a5d6 100644 --- a/pkg/storage/utils/decomposedfs/grants.go +++ b/pkg/storage/utils/decomposedfs/grants.go @@ -24,6 +24,7 @@ import ( "strings" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/cs3org/reva/v2/internal/grpc/services/storageprovider" "github.com/cs3org/reva/v2/pkg/appctx" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" "github.com/cs3org/reva/v2/pkg/errtypes" @@ -227,7 +228,8 @@ func (fs *Decomposedfs) RemoveGrant(ctx context.Context, ref *provider.Reference } func isShareGrant(ctx context.Context) bool { - return ctx.Value(utils.SpaceGrant) == nil + _, ok := storageprovider.SpaceTypeFromContext(ctx) + return !ok } // UpdateGrant updates a grant on a resource @@ -306,16 +308,12 @@ func (fs *Decomposedfs) loadGrant(ctx context.Context, ref *provider.Reference, } func (fs *Decomposedfs) storeGrant(ctx context.Context, n *node.Node, g *provider.Grant) error { - var spaceType string - spaceGrant := ctx.Value(utils.SpaceGrant) - // this is not a grant on a space root we are just adding a share - if spaceGrant == nil { + // if is a grant to a space root, the receiver needs the space type to update the indexes + spaceType, ok := storageprovider.SpaceTypeFromContext(ctx) + if !ok { + // this is not a grant on a space root we are just adding a share spaceType = spaceTypeShare } - // this is a grant to a space root, the receiver needs the space type to update the indexes - if sg, ok := spaceGrant.(struct{ SpaceType string }); ok && sg.SpaceType != "" { - spaceType = sg.SpaceType - } // set the grant e := ace.FromGrant(g) diff --git a/pkg/storage/utils/decomposedfs/spaces.go b/pkg/storage/utils/decomposedfs/spaces.go index ea51869e2c..c8f4268a33 100644 --- a/pkg/storage/utils/decomposedfs/spaces.go +++ b/pkg/storage/utils/decomposedfs/spaces.go @@ -34,6 +34,7 @@ import ( v1beta11 "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" types "github.com/cs3org/go-cs3apis/cs3/types/v1beta1" + "github.com/cs3org/reva/v2/internal/grpc/services/storageprovider" "github.com/cs3org/reva/v2/pkg/appctx" ocsconv "github.com/cs3org/reva/v2/pkg/conversions" ctxpkg "github.com/cs3org/reva/v2/pkg/ctx" @@ -65,7 +66,7 @@ const ( // CreateStorageSpace creates a storage space func (fs *Decomposedfs) CreateStorageSpace(ctx context.Context, req *provider.CreateStorageSpaceRequest) (*provider.CreateStorageSpaceResponse, error) { - ctx = context.WithValue(ctx, utils.SpaceGrant, struct{}{}) + ctx = storageprovider.WithSpaceType(ctx, "") u := ctxpkg.ContextMustGetUser(ctx) // "everything is a resource" this is the unique ID for the Space resource. @@ -195,7 +196,7 @@ func (fs *Decomposedfs) CreateStorageSpace(ctx context.Context, req *provider.Cr return nil, err } - ctx = context.WithValue(ctx, utils.SpaceGrant, struct{ SpaceType string }{SpaceType: req.Type}) + ctx = storageprovider.WithSpaceType(ctx, req.Type) if req.Type != _spaceTypePersonal { if err := fs.AddGrant(ctx, &provider.Reference{ diff --git a/pkg/storage/utils/decomposedfs/tree/tree.go b/pkg/storage/utils/decomposedfs/tree/tree.go index 58dc0c4e1f..31ade9af4f 100644 --- a/pkg/storage/utils/decomposedfs/tree/tree.go +++ b/pkg/storage/utils/decomposedfs/tree/tree.go @@ -440,9 +440,7 @@ func (t *Tree) Delete(ctx context.Context, n *node.Node) (err error) { // remove entry from cache immediately to avoid inconsistencies defer func() { _ = t.idCache.Delete(path) }() - deletingSharedResource := ctx.Value(appctx.DeletingSharedResource) - - if deletingSharedResource != nil && deletingSharedResource.(bool) { + if appctx.DeletingSharedResourceFromContext(ctx) { src := filepath.Join(n.ParentPath(), n.Name) return os.Remove(src) } diff --git a/pkg/storage/utils/decomposedfs/upload/store_test.go b/pkg/storage/utils/decomposedfs/upload/store_test.go index c72b27c9de..1473c9e296 100644 --- a/pkg/storage/utils/decomposedfs/upload/store_test.go +++ b/pkg/storage/utils/decomposedfs/upload/store_test.go @@ -38,13 +38,13 @@ func TestInitNewNode(t *testing.T) { err := os.MkdirAll(rootNode.InternalPath(), 0700) if err != nil { - t.Fatalf(err.Error()) + t.Fatal(err.Error()) } n := node.New("e48c4e7a-beac-4b82-b991-a5cff7b8c39c", "930b7a2e-b745-41e1-8a9b-712582021842", "e48c4e7a-beac-4b82-b991-a5cff7b8c39c", "newchild", 10, "26493c53-2634-45f8-949f-dc07b88df9b0", providerv1beta1.ResourceType_RESOURCE_TYPE_FILE, &userv1beta1.UserId{}, lookup) n.SpaceRoot = rootNode unlock, err := store.tp.InitNewNode(context.Background(), n, 10) if err != nil { - t.Fatalf(err.Error()) + t.Fatal(err.Error()) } defer func() { _ = unlock() diff --git a/pkg/user/manager/ldap/ldap_test.go b/pkg/user/manager/ldap/ldap_test.go index 3f97367a95..90e854efa1 100644 --- a/pkg/user/manager/ldap/ldap_test.go +++ b/pkg/user/manager/ldap/ldap_test.go @@ -63,6 +63,6 @@ func TestUserManager(t *testing.T) { // positive tests for New _, err = New(map[string]interface{}{}) if err != nil { - t.Fatalf(err.Error()) + t.Fatal(err.Error()) } } diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go index 6d5a7ff335..ca138ead72 100644 --- a/pkg/utils/utils_test.go +++ b/pkg/utils/utils_test.go @@ -37,7 +37,6 @@ var skipTests = []struct { func TestSkip(t *testing.T) { for _, tt := range skipTests { - tt := tt t.Run(tt.name, func(t *testing.T) { r := Skip(tt.url, tt.base) if r != tt.out {