From c7e66072ef4632ce5bba6b94f697205d62e15846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Mon, 14 Feb 2022 10:13:45 +0100 Subject: [PATCH] decomposedfs: refactor xattrs package errors (#2540) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../unreleased/decomposedfs-xattr-errors.md | 5 +++ pkg/storage/utils/decomposedfs/node/node.go | 26 ++++++------- .../utils/decomposedfs/node/permissions.go | 15 +------- .../decomposedfs/node/permissions_darwin.go | 37 ------------------- .../permissions_unix.go => xattrs/errors.go} | 19 ++++++++-- .../utils/decomposedfs/xattrs/xattrs.go | 14 +++++++ 6 files changed, 49 insertions(+), 67 deletions(-) create mode 100644 changelog/unreleased/decomposedfs-xattr-errors.md delete mode 100644 pkg/storage/utils/decomposedfs/node/permissions_darwin.go rename pkg/storage/utils/decomposedfs/{node/permissions_unix.go => xattrs/errors.go} (60%) diff --git a/changelog/unreleased/decomposedfs-xattr-errors.md b/changelog/unreleased/decomposedfs-xattr-errors.md new file mode 100644 index 0000000000..3b21b1baf8 --- /dev/null +++ b/changelog/unreleased/decomposedfs-xattr-errors.md @@ -0,0 +1,5 @@ +Enhancement: Refactored the xattrs package in the decomposedfs + +The xattrs package now uses the xattr.ENOATTR instead of os.ENODATA or os.ENOATTR to check attribute existence. + +https://github.com/cs3org/reva/pull/2540 diff --git a/pkg/storage/utils/decomposedfs/node/node.go b/pkg/storage/utils/decomposedfs/node/node.go index 6368da6e8e..dfa428c46e 100644 --- a/pkg/storage/utils/decomposedfs/node/node.go +++ b/pkg/storage/utils/decomposedfs/node/node.go @@ -181,9 +181,9 @@ func ReadNode(ctx context.Context, lu PathLookup, id string) (n *Node, err error switch { case err == nil: n.ParentID = attr - case isAttrUnset(err): + case xattrs.IsAttrUnset(err): return nil, errtypes.InternalError(err.Error()) - case isNotFound(err): + case xattrs.IsNotExist(err): return n, nil // swallow not found, the node defaults to exists = false default: return nil, errtypes.InternalError(err.Error()) @@ -216,7 +216,7 @@ func ReadNode(ctx context.Context, lu PathLookup, id string) (n *Node, err error // Check if parent exists. Otherwise this node is part of a deleted subtree _, err = os.Stat(lu.InternalPath(n.ParentID)) if err != nil { - if isNotFound(err) { + if os.IsNotExist(err) { return nil, errtypes.NotFound(err.Error()) } return nil, err @@ -320,7 +320,7 @@ func (n *Node) Owner() (*userpb.UserId, error) { switch { case err == nil: owner.OpaqueId = attr - case isAttrUnset(err), isNotFound(err): + case xattrs.IsAttrUnset(err), xattrs.IsNotExist(err): fallthrough default: return nil, err @@ -331,7 +331,7 @@ func (n *Node) Owner() (*userpb.UserId, error) { switch { case err == nil: owner.Idp = attr - case isAttrUnset(err), isNotFound(err): + case xattrs.IsAttrUnset(err), xattrs.IsNotExist(err): fallthrough default: return nil, err @@ -342,7 +342,7 @@ func (n *Node) Owner() (*userpb.UserId, error) { switch { case err == nil: owner.Type = utils.UserTypeMap(attr) - case isAttrUnset(err), isNotFound(err): + case xattrs.IsAttrUnset(err), xattrs.IsNotExist(err): fallthrough default: // TODO the user type defaults to invalid, which is the case @@ -681,9 +681,9 @@ func readChecksumIntoResourceChecksum(ctx context.Context, nodePath, algo string Type: storageprovider.PKG2GRPCXS(algo), Sum: hex.EncodeToString([]byte(v)), } - case isAttrUnset(err): + case xattrs.IsAttrUnset(err): appctx.GetLogger(ctx).Debug().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("checksum not set") - case isNotFound(err): + case xattrs.IsNotExist(err): appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("file not fount") default: appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("could not read checksum") @@ -703,9 +703,9 @@ func readChecksumIntoOpaque(ctx context.Context, nodePath, algo string, ri *prov Decoder: "plain", Value: []byte(hex.EncodeToString([]byte(v))), } - case isAttrUnset(err): + case xattrs.IsAttrUnset(err): appctx.GetLogger(ctx).Debug().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("checksum not set") - case isNotFound(err): + case xattrs.IsNotExist(err): appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("file not fount") default: appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("algorithm", algo).Msg("could not read checksum") @@ -735,9 +735,9 @@ func readQuotaIntoOpaque(ctx context.Context, nodePath string, ri *provider.Reso } else { appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Str("quota", v).Msg("malformed quota") } - case isAttrUnset(err): + case xattrs.IsAttrUnset(err): appctx.GetLogger(ctx).Debug().Err(err).Str("nodepath", nodePath).Msg("quota not set") - case isNotFound(err): + case xattrs.IsNotExist(err): appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Msg("file not found when reading quota") default: appctx.GetLogger(ctx).Error().Err(err).Str("nodepath", nodePath).Msg("could not read quota") @@ -862,7 +862,7 @@ func (n *Node) ReadUserPermissions(ctx context.Context, u *userpb.User) (ap prov switch { case err == nil: AddPermissions(&ap, g.GetPermissions()) - case isAttrUnset(err): + case xattrs.IsAttrUnset(err): err = nil appctx.GetLogger(ctx).Error().Interface("node", n).Str("grant", grantees[i]).Interface("grantees", grantees).Msg("grant vanished from node after listing") // continue with next segment diff --git a/pkg/storage/utils/decomposedfs/node/permissions.go b/pkg/storage/utils/decomposedfs/node/permissions.go index 4622045278..24d06ef976 100644 --- a/pkg/storage/utils/decomposedfs/node/permissions.go +++ b/pkg/storage/utils/decomposedfs/node/permissions.go @@ -21,7 +21,6 @@ package node import ( "context" "strings" - "syscall" userv1beta1 "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -30,7 +29,6 @@ import ( "github.com/cs3org/reva/pkg/storage/utils/decomposedfs/xattrs" "github.com/cs3org/reva/pkg/utils" "github.com/pkg/errors" - "github.com/pkg/xattr" ) // NoPermissions represents an empty set of permissions @@ -252,7 +250,7 @@ func nodeHasPermission(ctx context.Context, cn *Node, groupsMap map[string]bool, if check(g.GetPermissions()) { return true } - case isAttrUnset(err): + case xattrs.IsAttrUnset(err): appctx.GetLogger(ctx).Error().Interface("node", cn.ID).Str("grant", grantees[i]).Interface("grantees", grantees).Msg("grant vanished from node after listing") default: appctx.GetLogger(ctx).Error().Err(err).Interface("node", cn.ID).Str("grant", grantees[i]).Msg("error reading permissions") @@ -290,14 +288,3 @@ func (p *Permissions) getUserAndPermissions(ctx context.Context, n *Node) (*user } return u, nil } - -// The os not exists error is buried inside the xattr error, -// so we cannot just use os.IsNotExists(). -func isNotFound(err error) bool { - if xerr, ok := err.(*xattr.Error); ok { - if serr, ok2 := xerr.Err.(syscall.Errno); ok2 { - return serr == syscall.ENOENT - } - } - return false -} diff --git a/pkg/storage/utils/decomposedfs/node/permissions_darwin.go b/pkg/storage/utils/decomposedfs/node/permissions_darwin.go deleted file mode 100644 index a44b30df19..0000000000 --- a/pkg/storage/utils/decomposedfs/node/permissions_darwin.go +++ /dev/null @@ -1,37 +0,0 @@ -// Copyright 2018-2021 CERN -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. -// -// In applying this license, CERN does not waive the privileges and immunities -// granted to it by virtue of its status as an Intergovernmental Organization -// or submit itself to any jurisdiction. - -//go:build darwin -// +build darwin - -package node - -import ( - "syscall" - - "github.com/pkg/xattr" -) - -func isAttrUnset(err error) bool { - if xerr, ok := err.(*xattr.Error); ok { - if serr, ok2 := xerr.Err.(syscall.Errno); ok2 { - return serr == syscall.ENOATTR - } - } - return false -} diff --git a/pkg/storage/utils/decomposedfs/node/permissions_unix.go b/pkg/storage/utils/decomposedfs/xattrs/errors.go similarity index 60% rename from pkg/storage/utils/decomposedfs/node/permissions_unix.go rename to pkg/storage/utils/decomposedfs/xattrs/errors.go index d10efca03a..fb91cebd4c 100644 --- a/pkg/storage/utils/decomposedfs/node/permissions_unix.go +++ b/pkg/storage/utils/decomposedfs/xattrs/errors.go @@ -19,7 +19,7 @@ //go:build !darwin // +build !darwin -package node +package xattrs import ( "syscall" @@ -27,10 +27,23 @@ import ( "github.com/pkg/xattr" ) -func isAttrUnset(err error) bool { +// IsNotExist checks if there is a os not exists error buried inside the xattr error, +// as we cannot just use os.IsNotExist(). +func IsNotExist(err error) bool { if xerr, ok := err.(*xattr.Error); ok { if serr, ok2 := xerr.Err.(syscall.Errno); ok2 { - return serr == syscall.ENODATA + return serr == syscall.ENOENT + } + } + return false +} + +// IsAttrUnset checks the xattr.ENOATTR from the xattr package which redifines it as ENODATA on platforms that do not natively support it (eg. linux) +// see https://github.com/pkg/xattr/blob/8725d4ccc0fcef59c8d9f0eaf606b3c6f962467a/xattr_linux.go#L19-L22 +func IsAttrUnset(err error) bool { + if xerr, ok := err.(*xattr.Error); ok { + if serr, ok2 := xerr.Err.(syscall.Errno); ok2 { + return serr == xattr.ENOATTR } } return false diff --git a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go index 2f7b3f1942..b9b14b6636 100644 --- a/pkg/storage/utils/decomposedfs/xattrs/xattrs.go +++ b/pkg/storage/utils/decomposedfs/xattrs/xattrs.go @@ -19,6 +19,7 @@ package xattrs import ( + "strconv" "strings" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" @@ -165,6 +166,19 @@ func Get(filePath, key string) (string, error) { return val, nil } +// GetInt64 reads a string as int64 from the xattrs +func GetInt64(filePath, key string) (int64, error) { + attr, err := Get(filePath, key) + if err != nil { + return 0, err + } + v, err := strconv.ParseInt(attr, 10, 64) + if err != nil { + return 0, errors.Wrapf(err, "invalid xattr format") + } + return v, nil +} + // All reads all extended attributes for a node func All(filePath string) (map[string]string, error) { attrNames, err := xattr.List(filePath)