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

Enable gofmt linter in golangci-lint and apply gofmt (STACKED ON #3466) #3463

Merged
merged 1 commit into from
Nov 22, 2022
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: 0 additions & 1 deletion .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ linters:
- gosec # TODO: consider enabling the 'gosec' linter to inspect source code for security problems.
- tagliatelle # TODO: consider enabling the 'tagliatelle' linter to check the struct tags.
- stylecheck # TODO: consider enabling the 'stylecheck' linter to enforce style rules.
- gofmt # TODO: consider enabling the 'gofmt' linter to check whether code was gofmt-ed.
- usestdlibvars # TODO: consider enabling the 'usestdlibvars' linter to detect the possibility to use variables/constants from the Go standard library.
- thelper # TODO: consider enabling the 'thelper' linter to detect golang test helpers without t.Helper() call and check the consistency of test helpers.
- staticcheck # TODO: consider enabling the 'staticcheck' linter to find bugs and performance issues, offer simplifications, and enforce style rules.
Expand Down
3 changes: 3 additions & 0 deletions changelog/unreleased/enhancement-gofmt.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Enhancement: Enable gofmt linter in golangci-lint and apply gofmt

https://github.com/cs3org/reva/pull/3463
3 changes: 2 additions & 1 deletion cmd/revad/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,8 @@ func getHTTPServer(conf interface{}, l *zerolog.Logger) (*rhttp.Server, error) {
return s, nil
}

// adjustCPU parses string cpu and sets GOMAXPROCS
// adjustCPU parses string cpu and sets GOMAXPROCS
//
// according to its value. It accepts either
// a number (e.g. 3) or a percent (e.g. 50%).
// Default is to use all available cores.
Expand Down
1 change: 1 addition & 0 deletions internal/grpc/interceptors/eventsmiddleware/events.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ func init() {

// NewUnary returns a new unary interceptor that emits events when needed
// no lint because of the switch statement that should be extendable
//
//nolint:gocritic
func NewUnary(m map[string]interface{}) (grpc.UnaryServerInterceptor, int, error) {
publisher, err := publisherFromConfig(m)
Expand Down
4 changes: 2 additions & 2 deletions internal/grpc/services/gateway/usershareprovider.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,8 @@ func (s *svc) GetReceivedShare(ctx context.Context, req *collaboration.GetReceiv

// When updating a received share:
// if the update contains update for displayName:
// 1) if received share is mounted: we also do a rename in the storage
// 2) if received share is not mounted: we only rename in user share provider.
// 1. if received share is mounted: we also do a rename in the storage
// 2. if received share is not mounted: we only rename in user share provider.
func (s *svc) UpdateReceivedShare(ctx context.Context, req *collaboration.UpdateReceivedShareRequest) (*collaboration.UpdateReceivedShareResponse, error) {
log := appctx.GetLogger(ctx)

Expand Down
2 changes: 1 addition & 1 deletion internal/http/services/owncloud/ocdav/ocdav_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// 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
// 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// 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
// 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,
Expand Down
3 changes: 2 additions & 1 deletion pkg/mentix/key/apikey.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ const (

// GenerateAPIKey generates a new (random) API key which also contains flags and a (salted) hash.
// An API key has the following format:
// <RandomString:30><Flags:2><SaltedHash:32>
//
// <RandomString:30><Flags:2><SaltedHash:32>
func GenerateAPIKey(salt string, flags int) (APIKey, error) {
if len(salt) == 0 {
return "", errors.Errorf("no salt specified")
Expand Down
26 changes: 13 additions & 13 deletions pkg/registry/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,20 @@ config example:
---
services:
authprovider:
basic:
name: auth-basic
nodes:
- address: 0.0.0.0:1234
metadata:
version: v0.1.0
bearer:
name: auth-bearer
nodes:
- address: 0.0.0.0:5678
metadata:
version: v0.1.0
authprovider:
basic:
name: auth-basic
nodes:
- address: 0.0.0.0:1234
metadata:
version: v0.1.0
bearer:
name: auth-bearer
nodes:
- address: 0.0.0.0:5678
metadata:
version: v0.1.0
*/
func TestParseConfig(t *testing.T) {
type args struct {
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/fs/cephfs/connections.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ type connections struct {
groupCache *ristretto.Cache
}

//TODO: make configurable/add to options
// TODO: make configurable/add to options
var usrLimit int64 = 1e4

func newCache() (c *connections, err error) {
Expand Down
6 changes: 3 additions & 3 deletions pkg/storage/fs/nextcloud/nextcloud_server_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,9 @@ var responses = map[string]Response{
`POST /apps/sciencemesh/~tester/api/storage/GetQuota `: {200, `{"totalBytes":456,"usedBytes":123}`, serverStateEmpty},
`POST /apps/sciencemesh/~tester/api/storage/CreateReference {"path":"some/file/path.txt","url":"http://bing.com/search?q=dotnet"}`: {200, ``, serverStateEmpty},
`POST /apps/sciencemesh/~tester/api/storage/Shutdown `: {200, ``, serverStateEmpty},
`POST /apps/sciencemesh/~tester/api/storage/SetArbitraryMetadata {"ref":{"resource_id":{"storage_id":"storage-id","opaque_id":"opaque-id"},"path":"some/file/path.txt"},"md":{"metadata":{"arbi":"trary","meta":"data"}}}`: {200, ``, serverStateEmpty},
`POST /apps/sciencemesh/~tester/api/storage/UnsetArbitraryMetadata {"ref":{"resource_id":{"storage_id":"storage-id","opaque_id":"opaque-id"},"path":"some/file/path.txt"},"keys":["arbi"]}`: {200, ``, serverStateEmpty},
`POST /apps/sciencemesh/~tester/api/storage/ListStorageSpaces [{"type":3,"Term":{"Owner":{"idp":"0.0.0.0:19000","opaque_id":"f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c","type":1}}},{"type":2,"Term":{"Id":{"opaque_id":"opaque-id"}}},{"type":4,"Term":{"SpaceType":"home"}}]`: {200, ` [{"opaque":{"map":{"bar":{"value":"c2FtYQ=="},"foo":{"value":"c2FtYQ=="}}},"id":{"opaque_id":"some-opaque-storage-space-id"},"owner":{"id":{"idp":"some-idp","opaque_id":"some-opaque-user-id","type":1}},"root":{"storage_id":"some-storage-ud","opaque_id":"some-opaque-root-id"},"name":"My Storage Space","quota":{"quota_max_bytes":456,"quota_max_files":123},"space_type":"home","mtime":{"seconds":1234567890}}]`, serverStateEmpty},
`POST /apps/sciencemesh/~tester/api/storage/SetArbitraryMetadata {"ref":{"resource_id":{"storage_id":"storage-id","opaque_id":"opaque-id"},"path":"some/file/path.txt"},"md":{"metadata":{"arbi":"trary","meta":"data"}}}`: {200, ``, serverStateEmpty},
`POST /apps/sciencemesh/~tester/api/storage/UnsetArbitraryMetadata {"ref":{"resource_id":{"storage_id":"storage-id","opaque_id":"opaque-id"},"path":"some/file/path.txt"},"keys":["arbi"]}`: {200, ``, serverStateEmpty},
`POST /apps/sciencemesh/~tester/api/storage/ListStorageSpaces [{"type":3,"Term":{"Owner":{"idp":"0.0.0.0:19000","opaque_id":"f7fbf8c8-139b-4376-b307-cf0a8c2d0d9c","type":1}}},{"type":2,"Term":{"Id":{"opaque_id":"opaque-id"}}},{"type":4,"Term":{"SpaceType":"home"}}]`: {200, ` [{"opaque":{"map":{"bar":{"value":"c2FtYQ=="},"foo":{"value":"c2FtYQ=="}}},"id":{"opaque_id":"some-opaque-storage-space-id"},"owner":{"id":{"idp":"some-idp","opaque_id":"some-opaque-user-id","type":1}},"root":{"storage_id":"some-storage-ud","opaque_id":"some-opaque-root-id"},"name":"My Storage Space","quota":{"quota_max_bytes":456,"quota_max_files":123},"space_type":"home","mtime":{"seconds":1234567890}}]`, serverStateEmpty},
`POST /apps/sciencemesh/~tester/api/storage/CreateStorageSpace {"opaque":{"map":{"bar":{"value":"c2FtYQ=="},"foo":{"value":"c2FtYQ=="}}},"owner":{"id":{"idp":"some-idp","opaque_id":"some-opaque-user-id","type":1}},"type":"home","name":"My Storage Space","quota":{"quota_max_bytes":456,"quota_max_files":123}}`: {200, `{"storage_space":{"opaque":{"map":{"bar":{"value":"c2FtYQ=="},"foo":{"value":"c2FtYQ=="}}},"id":{"opaque_id":"some-opaque-storage-space-id"},"owner":{"id":{"idp":"some-idp","opaque_id":"some-opaque-user-id","type":1}},"root":{"storage_id":"some-storage-ud","opaque_id":"some-opaque-root-id"},"name":"My Storage Space","quota":{"quota_max_bytes":456,"quota_max_files":123},"space_type":"home","mtime":{"seconds":1234567890}}}`, serverStateEmpty},
}

Expand Down
43 changes: 23 additions & 20 deletions pkg/storage/utils/ace/ace.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,31 +39,31 @@ import (
// see https://linux.die.net/man/5/nfs4_acl:
// the extended attributes will look like this
// "user.oc.grant.<type>:<flags>:<principal>:<permissions>"
// - *type* will be limited to A for now
// - *type* will be limited to A for now
// A: Allow - allow *principal* to perform actions requiring *permissions*
// In the future we can use:
// In the future we can use:
// U: aUdit - log any attempted access by principal which requires
// permissions.
// permissions.
// L: aLarm - generate a system alarm at any attempted access by
// principal which requires permissions
// principal which requires permissions
// D: for Deny is not recommended
// - *flags* for now empty or g for group, no inheritance yet
// - *flags* for now empty or g for group, no inheritance yet
// - d directory-inherit - newly-created subdirectories will inherit the
// ACE.
// ACE.
// - f file-inherit - newly-created files will inherit the ACE, minus its
// inheritance flags. Newly-created subdirectories
// will inherit the ACE; if directory-inherit is not
// also specified in the parent ACE, inherit-only will
// be added to the inherited ACE.
// inheritance flags. Newly-created subdirectories
// will inherit the ACE; if directory-inherit is not
// also specified in the parent ACE, inherit-only will
// be added to the inherited ACE.
// - n no-propagate-inherit - newly-created subdirectories will inherit
// the ACE, minus its inheritance flags.
// the ACE, minus its inheritance flags.
// - i inherit-only - the ACE is not considered in permissions checks,
// but it is heritable; however, the inherit-only
// flag is stripped from inherited ACEs.
// - *principal* a named user, group or special principal
// but it is heritable; however, the inherit-only
// flag is stripped from inherited ACEs.
// - *principal* a named user, group or special principal
// - the oidc sub@iss maps nicely to this
// - 'OWNER@', 'GROUP@', and 'EVERYONE@', which are, respectively, analogous to the POSIX user/group/other
// - *permissions*
// - *permissions*
// - r read-data (files) / list-directory (directories)
// - w write-data (files) / create-file (directories)
// - a append-data (files) / create-subdirectory (directories)
Expand All @@ -78,15 +78,18 @@ import (
// - C write-ACL - write the file/directory NFSv4 ACL.
// - o write-owner - change ownership of the file/directory.
// - y synchronize - allow clients to use synchronous I/O with the server.
//
// TODO implement OWNER@ as "user.oc.grant.A::OWNER@:rwaDxtTnNcCy"
// attribute names are limited to 255 chars by the linux kernel vfs, values to 64 kb
// ext3 extended attributes must fit inside a single filesystem block ... 4096 bytes
// that leaves us with "user.oc.grant.A::someonewithaslightlylongersubject@whateverissuer:rwaDxtTnNcCy" ~80 chars
// 4096/80 = 51 shares ... with luck we might move the actual permissions to the value, saving ~15 chars
// 4096/64 = 64 shares ... still meh ... we can do better by using ints instead of strings for principals
// "user.oc.grant.u:100000" is pretty neat, but we can still do better: base64 encode the int
// "user.oc.grant.u:6Jqg" but base64 always has at least 4 chars, maybe hex is better for smaller numbers
// well use 4 chars in addition to the ace: "user.oc.grant.u:////" = 65535 -> 18 chars
//
// "user.oc.grant.u:100000" is pretty neat, but we can still do better: base64 encode the int
// "user.oc.grant.u:6Jqg" but base64 always has at least 4 chars, maybe hex is better for smaller numbers
// well use 4 chars in addition to the ace: "user.oc.grant.u:////" = 65535 -> 18 chars
//
// 4096/18 = 227 shares
// still ... ext attrs for this are not infinite scale ...
// so .. attach shares via fileid.
Expand All @@ -95,8 +98,8 @@ import (
//
// whatever ... 50 shares is good enough. If more is needed we can delegate to the metadata
// if "user.oc.grant.M" is present look inside the metadata app.
// - if we cannot set an ace we might get an io error.
// in that case convert all shares to metadata and try to set "user.oc.grant.m"
// - if we cannot set an ace we might get an io error.
// in that case convert all shares to metadata and try to set "user.oc.grant.m"
//
// what about metadata like share creator, share time, expiry?
// - creator is same as owner, but can be set
Expand Down
6 changes: 3 additions & 3 deletions pkg/storage/utils/decomposedfs/testhelpers/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,9 @@ type TestEnv struct {
// NewTestEnv prepares a test environment on disk
// The storage contains some directories and a file:
//
// /dir1/
// /dir1/file1
// /dir1/subdir1/
// /dir1/
// /dir1/file1
// /dir1/subdir1/
func NewTestEnv() (*TestEnv, error) {
tmpRoot, err := helpers.TempDir("reva-unit-tests-*-root")
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion pkg/utils/resourceid/owncloud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// 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
// 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,
Expand Down
6 changes: 3 additions & 3 deletions tests/integration/grpc/grpc_suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,15 +62,15 @@ type Revad struct {
// Placeholders in the config files can be replaced the variables from the
// `variables` map, e.g. the config
//
// redis = "{{redis_address}}"
// redis = "{{redis_address}}"
//
// and the variables map
//
// variables = map[string]string{"redis_address": "localhost:6379"}
// variables = map[string]string{"redis_address": "localhost:6379"}
//
// will result in the config
//
// redis = "localhost:6379"
// redis = "localhost:6379"
//
// Special variables are created for the revad addresses, e.g. having a
// `storage` and a `users` revad will make `storage_address` and
Expand Down