diff --git a/.golangci.yml b/.golangci.yml index 7eb920fd0..6a9a032d3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -24,6 +24,7 @@ linters: - unused - whitespace - gofumpt + - errorlint run: timeout: 5m diff --git a/.vscode/settings.json b/.vscode/settings.json index 2c9329d33..4315da366 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -14,5 +14,9 @@ "./web" ], "prettier.configPath": "./web/.prettierrc.js", - "prettier.ignorePath": "./web/.prettierignore" + "prettier.ignorePath": "./web/.prettierignore", + "cSpell.words": [ + "doublestar", + "multierr" + ] } diff --git a/go.mod b/go.mod index 19473806f..0960d61d5 100644 --- a/go.mod +++ b/go.mod @@ -41,6 +41,7 @@ require ( github.com/urfave/cli/v2 v2.20.2 github.com/xanzy/go-gitlab v0.73.1 github.com/xeipuuv/gojsonschema v1.2.0 + go.uber.org/multierr v1.9.0 golang.org/x/crypto v0.1.0 golang.org/x/exp v0.0.0-20221031165847-c99f073a8326 golang.org/x/net v0.4.0 @@ -127,7 +128,6 @@ require ( github.com/xeipuuv/gojsonreference v0.0.0-20180127040603-bd5ef7bd5415 // indirect github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect go.uber.org/atomic v1.10.0 // indirect - go.uber.org/multierr v1.8.0 // indirect go.uber.org/zap v1.23.0 // indirect golang.org/x/mod v0.6.0 // indirect golang.org/x/sys v0.3.0 // indirect diff --git a/go.sum b/go.sum index 387b2ff47..f1d730450 100644 --- a/go.sum +++ b/go.sum @@ -739,8 +739,8 @@ go.uber.org/multierr v1.1.0/go.mod h1:wR5kodmAFQ0UK8QlbwjlSNy0Z68gJhDJUG5sjR94q/ go.uber.org/multierr v1.3.0/go.mod h1:VgVr7evmIr6uPjLBxg28wmKNXyqE9akIJ5XnfpiKl+4= go.uber.org/multierr v1.5.0/go.mod h1:FeouvMocqHpRaaGuG9EjoKcStLC43Zu/fmqdUMPcKYU= go.uber.org/multierr v1.6.0/go.mod h1:cdWPpRnG4AhwMwsgIHip0KRBQjJy5kYEpYjJxpXp9iU= -go.uber.org/multierr v1.8.0 h1:dg6GjLku4EH+249NNmoIciG9N/jURbDG+pFlTkhzIC8= -go.uber.org/multierr v1.8.0/go.mod h1:7EAYxJLBy9rStEaz58O2t4Uvip6FSURkq8/ppBp95ak= +go.uber.org/multierr v1.9.0 h1:7fIwc/ZtS0q++VgcfqFDxSBZVv/Xo49/SYnDFupUwlI= +go.uber.org/multierr v1.9.0/go.mod h1:X2jQV1h+kxSjClGpnseKVIxpmcjrj7MNnI0bnlfKTVQ= go.uber.org/tools v0.0.0-20190618225709-2cfd321de3ee/go.mod h1:vJERXedbb3MVM5f9Ejo0C68/HhF8uaILCdgjnY+goOA= go.uber.org/zap v1.9.1/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= go.uber.org/zap v1.10.0/go.mod h1:vwi/ZaCAaUcBkycHslxD9B2zi4UTXhF60s6SWpuDF0Q= diff --git a/pipeline/backend/local/local.go b/pipeline/backend/local/local.go index e75de3066..164255971 100644 --- a/pipeline/backend/local/local.go +++ b/pipeline/backend/local/local.go @@ -16,6 +16,7 @@ package local import ( "context" + "errors" "fmt" "io" "os" @@ -133,11 +134,14 @@ func (e *local) Exec(ctx context.Context, step *types.Step) error { func (e *local) Wait(context.Context, *types.Step) (*types.State, error) { err := e.cmd.Wait() ExitCode := 0 - if eerr, ok := err.(*exec.ExitError); ok { - ExitCode = eerr.ExitCode() + + var execExitError *exec.ExitError + if errors.As(err, &execExitError) { + ExitCode = execExitError.ExitCode() // Non-zero exit code is a pipeline failure, but not an agent error. err = nil } + return &types.State{ Exited: true, ExitCode: ExitCode, diff --git a/pipeline/frontend/yaml/compiler/compiler_test.go b/pipeline/frontend/yaml/compiler/compiler_test.go index 7524af5f0..9f1bf2189 100644 --- a/pipeline/frontend/yaml/compiler/compiler_test.go +++ b/pipeline/frontend/yaml/compiler/compiler_test.go @@ -16,11 +16,11 @@ func TestSecretAvailable(t *testing.T) { } assert.True(t, secret.Available(&yaml.Container{ Image: "golang", - Commands: types.Stringorslice(strslice.StrSlice{"echo 'this is not a plugin'"}), + Commands: types.StringOrSlice(strslice.StrSlice{"echo 'this is not a plugin'"}), })) assert.False(t, secret.Available(&yaml.Container{ Image: "not-golang", - Commands: types.Stringorslice(strslice.StrSlice{"echo 'this is not a plugin'"}), + Commands: types.StringOrSlice(strslice.StrSlice{"echo 'this is not a plugin'"}), })) // secret only available for "golang" plugin secret = Secret{ @@ -29,14 +29,14 @@ func TestSecretAvailable(t *testing.T) { } assert.True(t, secret.Available(&yaml.Container{ Image: "golang", - Commands: types.Stringorslice(strslice.StrSlice{}), + Commands: types.StringOrSlice(strslice.StrSlice{}), })) assert.False(t, secret.Available(&yaml.Container{ Image: "not-golang", - Commands: types.Stringorslice(strslice.StrSlice{}), + Commands: types.StringOrSlice(strslice.StrSlice{}), })) assert.False(t, secret.Available(&yaml.Container{ Image: "not-golang", - Commands: types.Stringorslice(strslice.StrSlice{"echo 'this is not a plugin'"}), + Commands: types.StringOrSlice(strslice.StrSlice{"echo 'this is not a plugin'"}), })) } diff --git a/pipeline/frontend/yaml/config.go b/pipeline/frontend/yaml/config.go index 025302e77..c5c73b9fb 100644 --- a/pipeline/frontend/yaml/config.go +++ b/pipeline/frontend/yaml/config.go @@ -11,7 +11,7 @@ type ( // Config defines a pipeline configuration. Config struct { When constraint.When `yaml:"when,omitempty"` - Cache types.Stringorslice + Cache types.StringOrSlice Platform string Workspace Workspace Clone Containers diff --git a/pipeline/frontend/yaml/config_test.go b/pipeline/frontend/yaml/config_test.go index 97d54af8b..ce8f9d11f 100644 --- a/pipeline/frontend/yaml/config_test.go +++ b/pipeline/frontend/yaml/config_test.go @@ -32,10 +32,10 @@ func TestParse(t *testing.T) { g.Assert(out.Services.Containers[0].Image).Equal("mysql") g.Assert(out.Pipeline.Containers[0].Name).Equal("test") g.Assert(out.Pipeline.Containers[0].Image).Equal("golang") - g.Assert(out.Pipeline.Containers[0].Commands).Equal(types.Stringorslice{"go install", "go test"}) + g.Assert(out.Pipeline.Containers[0].Commands).Equal(types.StringOrSlice{"go install", "go test"}) g.Assert(out.Pipeline.Containers[1].Name).Equal("build") g.Assert(out.Pipeline.Containers[1].Image).Equal("golang") - g.Assert(out.Pipeline.Containers[1].Commands).Equal(types.Stringorslice{"go build"}) + g.Assert(out.Pipeline.Containers[1].Commands).Equal(types.StringOrSlice{"go build"}) g.Assert(out.Pipeline.Containers[2].Name).Equal("notify") g.Assert(out.Pipeline.Containers[2].Image).Equal("slack") // g.Assert(out.Pipeline.Containers[2].NetworkMode).Equal("container:name") diff --git a/pipeline/frontend/yaml/constraint/constraint.go b/pipeline/frontend/yaml/constraint/constraint.go index 6db26f69b..489fd7c24 100644 --- a/pipeline/frontend/yaml/constraint/constraint.go +++ b/pipeline/frontend/yaml/constraint/constraint.go @@ -6,6 +6,7 @@ import ( "github.com/antonmedv/expr" "github.com/bmatcuk/doublestar/v4" + "go.uber.org/multierr" "gopkg.in/yaml.v3" "github.com/woodpecker-ci/woodpecker/pipeline/frontend" @@ -238,11 +239,11 @@ func (c *List) Excludes(v string) bool { // UnmarshalYAML unmarshals the constraint. func (c *List) UnmarshalYAML(value *yaml.Node) error { out1 := struct { - Include types.Stringorslice - Exclude types.Stringorslice + Include types.StringOrSlice + Exclude types.StringOrSlice }{} - var out2 types.Stringorslice + var out2 types.StringOrSlice err1 := value.Decode(&out1) err2 := value.Decode(&out2) @@ -255,7 +256,7 @@ func (c *List) UnmarshalYAML(value *yaml.Node) error { if err1 != nil && err2 != nil { y, _ := yaml.Marshal(value) - return fmt.Errorf("Could not parse condition: %s", y) + return fmt.Errorf("Could not parse condition: %s: %w", y, multierr.Combine(err1, err2)) } return nil @@ -291,7 +292,7 @@ func (c *Map) Match(params map[string]string) bool { return true } -// UnmarshalYAML unmarshals the constraint map. +// UnmarshalYAML unmarshal the constraint map. func (c *Map) UnmarshalYAML(unmarshal func(interface{}) error) error { out1 := struct { Include map[string]string @@ -314,15 +315,15 @@ func (c *Map) UnmarshalYAML(unmarshal func(interface{}) error) error { return nil } -// UnmarshalYAML unmarshals the constraint. +// UnmarshalYAML unmarshal the constraint. func (c *Path) UnmarshalYAML(value *yaml.Node) error { out1 := struct { - Include types.Stringorslice `yaml:"include,omitempty"` - Exclude types.Stringorslice `yaml:"exclude,omitempty"` + Include types.StringOrSlice `yaml:"include,omitempty"` + Exclude types.StringOrSlice `yaml:"exclude,omitempty"` IgnoreMessage string `yaml:"ignore_message,omitempty"` }{} - var out2 types.Stringorslice + var out2 types.StringOrSlice err1 := value.Decode(&out1) err2 := value.Decode(&out2) diff --git a/pipeline/frontend/yaml/container.go b/pipeline/frontend/yaml/container.go index f6957a18c..139ad339b 100644 --- a/pipeline/frontend/yaml/container.go +++ b/pipeline/frontend/yaml/container.go @@ -27,15 +27,15 @@ type ( AuthConfig AuthConfig `yaml:"auth_config,omitempty"` CapAdd []string `yaml:"cap_add,omitempty"` CapDrop []string `yaml:"cap_drop,omitempty"` - Commands types.Stringorslice `yaml:"commands,omitempty"` + Commands types.StringOrSlice `yaml:"commands,omitempty"` CPUQuota types.StringorInt `yaml:"cpu_quota,omitempty"` CPUSet string `yaml:"cpuset,omitempty"` CPUShares types.StringorInt `yaml:"cpu_shares,omitempty"` Detached bool `yaml:"detach,omitempty"` Devices []string `yaml:"devices,omitempty"` Tmpfs []string `yaml:"tmpfs,omitempty"` - DNS types.Stringorslice `yaml:"dns,omitempty"` - DNSSearch types.Stringorslice `yaml:"dns_search,omitempty"` + DNS types.StringOrSlice `yaml:"dns,omitempty"` + DNSSearch types.StringOrSlice `yaml:"dns_search,omitempty"` Directory string `yaml:"directory,omitempty"` Environment types.SliceorMap `yaml:"environment,omitempty"` ExtraHosts []string `yaml:"extra_hosts,omitempty"` diff --git a/pipeline/frontend/yaml/container_test.go b/pipeline/frontend/yaml/container_test.go index 8fe709a1c..2a22fa964 100644 --- a/pipeline/frontend/yaml/container_test.go +++ b/pipeline/frontend/yaml/container_test.go @@ -74,15 +74,15 @@ func TestUnmarshalContainer(t *testing.T) { }, CapAdd: []string{"ALL"}, CapDrop: []string{"NET_ADMIN", "SYS_ADMIN"}, - Commands: types.Stringorslice{"go build", "go test"}, + Commands: types.StringOrSlice{"go build", "go test"}, CPUQuota: types.StringorInt(11), CPUSet: "1,2", CPUShares: types.StringorInt(99), Detached: true, Devices: []string{"/dev/ttyUSB0:/dev/ttyUSB0"}, Directory: "example/", - DNS: types.Stringorslice{"8.8.8.8"}, - DNSSearch: types.Stringorslice{"example.com"}, + DNS: types.StringOrSlice{"8.8.8.8"}, + DNSSearch: types.StringOrSlice{"example.com"}, Environment: types.SliceorMap{"RACK_ENV": "development", "SHOW": "true"}, ExtraHosts: []string{"somehost:162.242.195.82", "otherhost:50.31.209.229"}, Image: "golang:latest", @@ -102,7 +102,7 @@ func TestUnmarshalContainer(t *testing.T) { Pull: true, Privileged: true, ShmSize: types.MemStringorInt(1024), - Tmpfs: types.Stringorslice{"/var/lib/test"}, + Tmpfs: types.StringOrSlice{"/var/lib/test"}, Volumes: types.Volumes{ Volumes: []*types.Volume{ {Source: "", Destination: "/var/lib/mysql"}, @@ -302,9 +302,9 @@ func stringsToInterface(val ...string) []interface{} { func TestIsPlugin(t *testing.T) { assert.True(t, (&Container{}).IsPlugin()) assert.True(t, (&Container{ - Commands: types.Stringorslice(strslice.StrSlice{}), + Commands: types.StringOrSlice(strslice.StrSlice{}), }).IsPlugin()) assert.False(t, (&Container{ - Commands: types.Stringorslice(strslice.StrSlice{"echo 'this is not a plugin'"}), + Commands: types.StringOrSlice(strslice.StrSlice{"echo 'this is not a plugin'"}), }).IsPlugin()) } diff --git a/pipeline/frontend/yaml/error.go b/pipeline/frontend/yaml/error.go index 46c0a2e1d..26600c636 100644 --- a/pipeline/frontend/yaml/error.go +++ b/pipeline/frontend/yaml/error.go @@ -14,6 +14,8 @@ package yaml +import "errors" + // PipelineParseError is an error that occurs when the pipeline parsing fails. type PipelineParseError struct { Err error @@ -23,8 +25,8 @@ func (e PipelineParseError) Error() string { return e.Err.Error() } -func (e PipelineParseError) Is(target error) bool { - _, ok1 := target.(PipelineParseError) - _, ok2 := target.(*PipelineParseError) - return ok1 || ok2 +func (e PipelineParseError) Is(err error) bool { + target1 := PipelineParseError{} + target2 := &target1 + return errors.As(err, &target1) || errors.As(err, &target2) } diff --git a/pipeline/frontend/yaml/types/types_yaml.go b/pipeline/frontend/yaml/types/types_yaml.go index 62c09575a..5e551934b 100644 --- a/pipeline/frontend/yaml/types/types_yaml.go +++ b/pipeline/frontend/yaml/types/types_yaml.go @@ -59,12 +59,12 @@ func (s *MemStringorInt) UnmarshalYAML(unmarshal func(interface{}) error) error return errors.New("Failed to unmarshal MemStringorInt") } -// Stringorslice represents +// StringOrSlice represents // Using engine-api Strslice and augment it with YAML marshaling stuff. a string or an array of strings. -type Stringorslice strslice.StrSlice +type StringOrSlice strslice.StrSlice // UnmarshalYAML implements the Unmarshaler interface. -func (s *Stringorslice) UnmarshalYAML(unmarshal func(interface{}) error) error { +func (s *StringOrSlice) UnmarshalYAML(unmarshal func(interface{}) error) error { var stringType string if err := unmarshal(&stringType); err == nil { *s = []string{stringType} @@ -81,7 +81,7 @@ func (s *Stringorslice) UnmarshalYAML(unmarshal func(interface{}) error) error { return nil } - return errors.New("Failed to unmarshal Stringorslice") + return errors.New("Failed to unmarshal StringOrSlice") } // SliceorMap represents a slice or a map of strings. diff --git a/pipeline/frontend/yaml/types/types_yaml_test.go b/pipeline/frontend/yaml/types/types_yaml_test.go index 87869a25a..58e00710a 100644 --- a/pipeline/frontend/yaml/types/types_yaml_test.go +++ b/pipeline/frontend/yaml/types/types_yaml_test.go @@ -30,25 +30,25 @@ func TestStringorIntYaml(t *testing.T) { } } -type StructStringorslice struct { - Foo Stringorslice +type StructStringOrSlice struct { + Foo StringOrSlice } -func TestStringorsliceYaml(t *testing.T) { +func TestStringOrSliceYaml(t *testing.T) { str := `{foo: [bar, baz]}` - s := StructStringorslice{} + s := StructStringOrSlice{} assert.NoError(t, yaml.Unmarshal([]byte(str), &s)) - assert.Equal(t, Stringorslice{"bar", "baz"}, s.Foo) + assert.Equal(t, StringOrSlice{"bar", "baz"}, s.Foo) d, err := yaml.Marshal(&s) assert.Nil(t, err) - s2 := StructStringorslice{} + s2 := StructStringOrSlice{} assert.NoError(t, yaml.Unmarshal(d, &s2)) - assert.Equal(t, Stringorslice{"bar", "baz"}, s2.Foo) + assert.Equal(t, StringOrSlice{"bar", "baz"}, s2.Foo) } type StructSliceorMap struct { diff --git a/server/api/helper.go b/server/api/helper.go index 1968b14e6..e123e63c5 100644 --- a/server/api/helper.go +++ b/server/api/helper.go @@ -15,6 +15,7 @@ package api import ( + "errors" "net/http" "github.com/gin-gonic/gin" @@ -28,12 +29,12 @@ import ( ) func handlePipelineErr(c *gin.Context, err error) { - if pipeline.IsErrNotFound(err) { - c.String(http.StatusNotFound, "%v", err) - } else if pipeline.IsErrBadRequest(err) { - c.String(http.StatusBadRequest, "%v", err) - } else if pipeline.IsErrFiltered(err) { - c.String(http.StatusNoContent, "%v", err) + if errors.Is(err, &pipeline.ErrNotFound{}) { + c.String(http.StatusNotFound, "%s", err) + } else if errors.Is(err, &pipeline.ErrBadRequest{}) { + c.String(http.StatusBadRequest, "%s", err) + } else if errors.Is(err, &pipeline.ErrFiltered{}) { + c.String(http.StatusNoContent, "%s", err) } else { _ = c.AbortWithError(http.StatusInternalServerError, err) } diff --git a/server/cron/cron.go b/server/cron/cron.go index f1f5f52c9..939386ba7 100644 --- a/server/cron/cron.go +++ b/server/cron/cron.go @@ -72,7 +72,7 @@ func CalcNewNext(schedule string, now time.Time) (time.Time, error) { c, err := cron.Parse(schedule) if err != nil { - return time.Time{}, fmt.Errorf("cron parse schedule: %v", err) + return time.Time{}, fmt.Errorf("cron parse schedule: %w", err) } return c.Next(now), nil } diff --git a/server/forge/coding/internal/coding.go b/server/forge/coding/internal/coding.go index 5baacd91d..3b89ea6d0 100644 --- a/server/forge/coding/internal/coding.go +++ b/server/forge/coding/internal/coding.go @@ -72,13 +72,13 @@ func (c *Client) Do(method, u string, params url.Values) ([]byte, error) { req, err = http.NewRequestWithContext(c.ctx, "GET", rawURL+"?"+params.Encode(), nil) } if err != nil { - return nil, fmt.Errorf("fail to create request for url %q: %v", rawURL, err) + return nil, fmt.Errorf("fail to create request for url %q: %w", rawURL, err) } req.Header.Set("User-Agent", c.agent) resp, err := c.client.Do(req) if err != nil { - return nil, fmt.Errorf("fail to request %s %s: %v", req.Method, req.URL, err) + return nil, fmt.Errorf("fail to request %s %s: %w", req.Method, req.URL, err) } if resp.StatusCode != http.StatusOK { return nil, fmt.Errorf("%s %s respond %d", req.Method, req.URL, resp.StatusCode) @@ -86,13 +86,13 @@ func (c *Client) Do(method, u string, params url.Values) ([]byte, error) { defer resp.Body.Close() body, err := io.ReadAll(resp.Body) if err != nil { - return nil, fmt.Errorf("fail to read response from %s %s: %v", req.Method, req.URL.String(), err) + return nil, fmt.Errorf("fail to read response from %s %s: %w", req.Method, req.URL.String(), err) } apiResp := &GenericAPIResponse{} err = json.Unmarshal(body, apiResp) if err != nil { - return nil, fmt.Errorf("fail to parse response from %s %s: %v", req.Method, req.URL.String(), err) + return nil, fmt.Errorf("fail to parse response from %s %s: %w", req.Method, req.URL.String(), err) } if apiResp.Code != 0 { return nil, fmt.Errorf("Coding OAuth API respond error: %s", string(body)) diff --git a/server/forge/configFetcher.go b/server/forge/configFetcher.go index 53c8656e0..98cac9411 100644 --- a/server/forge/configFetcher.go +++ b/server/forge/configFetcher.go @@ -75,7 +75,7 @@ func (cf *configFetcher) Fetch(ctx context.Context) (files []*types.FileMeta, er newConfigs, useOld, err := cf.configExtension.FetchConfig(fetchCtx, cf.repo, cf.pipeline, files) if err != nil { log.Error().Msg("Got error " + err.Error()) - return nil, fmt.Errorf("On Fetching config via http : %s", err) + return nil, fmt.Errorf("On Fetching config via http : %w", err) } if !useOld { @@ -104,7 +104,7 @@ func (cf *configFetcher) fetch(c context.Context, timeout time.Duration, config return fileMeta, err } - return nil, fmt.Errorf("user defined config '%s' not found: %s", config, err) + return nil, fmt.Errorf("user defined config '%s' not found: %w", config, err) } log.Trace().Msgf("ConfigFetch[%s]: user did not defined own config, following default procedure", cf.repo.FullName) @@ -118,7 +118,7 @@ func (cf *configFetcher) fetch(c context.Context, timeout time.Duration, config case <-ctx.Done(): return nil, ctx.Err() default: - return []*types.FileMeta{}, fmt.Errorf("ConfigFetcher: Fallback did not find config: %s", err) + return []*types.FileMeta{}, fmt.Errorf("ConfigFetcher: Fallback did not find config: %w", err) } } diff --git a/server/forge/gitea/gitea.go b/server/forge/gitea/gitea.go index a87740f4a..d98a57601 100644 --- a/server/forge/gitea/gitea.go +++ b/server/forge/gitea/gitea.go @@ -314,7 +314,7 @@ func (c *Gitea) Dir(ctx context.Context, u *model.User, r *model.Repo, b *model. if m, _ := filepath.Match(f, e.Path); m && e.Type == "blob" { data, err := c.File(ctx, u, r, b, e.Path) if err != nil { - return nil, fmt.Errorf("multi-pipeline cannot get %s: %s", e.Path, err) + return nil, fmt.Errorf("multi-pipeline cannot get %s: %w", e.Path, err) } configs = append(configs, &forge_types.FileMeta{ diff --git a/server/forge/gitlab/gitlab.go b/server/forge/gitlab/gitlab.go index 4643f48cb..6bd62653a 100644 --- a/server/forge/gitlab/gitlab.go +++ b/server/forge/gitlab/gitlab.go @@ -119,7 +119,7 @@ func (g *GitLab) Login(ctx context.Context, res http.ResponseWriter, req *http.R token, err := config.Exchange(oauth2Ctx, code) if err != nil { - return nil, fmt.Errorf("Error exchanging token. %s", err) + return nil, fmt.Errorf("Error exchanging token. %w", err) } client, err := newClient(g.URL, token.AccessToken, g.SkipVerify) diff --git a/server/forge/userSyncer.go b/server/forge/userSyncer.go index 4c2c71e47..ad464590e 100644 --- a/server/forge/userSyncer.go +++ b/server/forge/userSyncer.go @@ -86,7 +86,7 @@ func (s *Syncer) Sync(ctx context.Context, user *model.User, flatPermissions boo } else { forgePerm, err := s.Forge.Perm(ctx, user, repo) if err != nil { - return fmt.Errorf("could not fetch permission of repo '%s': %v", repo.FullName, err) + return fmt.Errorf("could not fetch permission of repo '%s': %w", repo.FullName, err) } repo.Perm.Pull = forgePerm.Pull repo.Perm.Push = forgePerm.Push diff --git a/server/model/cron.go b/server/model/cron.go index 1b4c5343c..0be5807a5 100644 --- a/server/model/cron.go +++ b/server/model/cron.go @@ -49,7 +49,7 @@ func (c *Cron) Validate() error { _, err := cron.Parse(c.Schedule) if err != nil { - return fmt.Errorf("can't parse schedule: %v", err) + return fmt.Errorf("can't parse schedule: %w", err) } return nil diff --git a/server/model/secret.go b/server/model/secret.go index 429dcf24e..f221c469d 100644 --- a/server/model/secret.go +++ b/server/model/secret.go @@ -24,9 +24,10 @@ import ( ) var ( - errSecretNameInvalid = errors.New("Invalid Secret Name") - errSecretValueInvalid = errors.New("Invalid Secret Value") - errSecretEventInvalid = errors.New("Invalid Secret Event") + ErrSecretNameInvalid = errors.New("Invalid Secret Name") + ErrSecretImageInvalid = errors.New("Invalid Secret Image") + ErrSecretValueInvalid = errors.New("Invalid Secret Value") + ErrSecretEventInvalid = errors.New("Invalid Secret Event") ) // SecretService defines a service for managing secrets. @@ -125,27 +126,27 @@ var validDockerImageString = regexp.MustCompile( func (s *Secret) Validate() error { for _, event := range s.Events { if !ValidateWebhookEvent(event) { - return fmt.Errorf("%s: '%s'", errSecretEventInvalid, event) + return fmt.Errorf("%w: '%s'", ErrSecretEventInvalid, event) } } if len(s.Events) == 0 { - return fmt.Errorf("%s: no event specified", errSecretEventInvalid) + return fmt.Errorf("%w: no event specified", ErrSecretEventInvalid) } for _, image := range s.Images { if len(image) == 0 { - return fmt.Errorf("empty image in images") + return fmt.Errorf("%w: empty image in images", ErrSecretImageInvalid) } if !validDockerImageString.MatchString(image) { - return fmt.Errorf("image '%s' do not match regexp '%s'", image, validDockerImageString.String()) + return fmt.Errorf("%w: image '%s' do not match regexp '%s'", ErrSecretImageInvalid, image, validDockerImageString.String()) } } switch { case len(s.Name) == 0: - return errSecretNameInvalid + return fmt.Errorf("%w: empty name", ErrSecretNameInvalid) case len(s.Value) == 0: - return errSecretValueInvalid + return fmt.Errorf("%w: empty value", ErrSecretValueInvalid) default: return nil } diff --git a/server/model/user_test.go b/server/model/user_test.go index 8f7cba41f..22a5a321f 100644 --- a/server/model/user_test.go +++ b/server/model/user_test.go @@ -14,7 +14,10 @@ package model -import "testing" +import ( + "errors" + "testing" +) func TestUserValidate(t *testing.T) { tests := []struct { @@ -57,8 +60,8 @@ func TestUserValidate(t *testing.T) { for _, test := range tests { err := test.user.Validate() - if want, got := test.err, err; want != got { - t.Errorf("Want user validation error %s, got %s", want, got) + if !errors.Is(err, test.err) { + t.Errorf("Want user validation error %s, got %s", test.err, err) } } } diff --git a/server/pipeline/approve.go b/server/pipeline/approve.go index 6e7d34467..e2e27e937 100644 --- a/server/pipeline/approve.go +++ b/server/pipeline/approve.go @@ -41,7 +41,7 @@ func Approve(ctx context.Context, store store.Store, currentPipeline *model.Pipe } if currentPipeline, err = UpdateToStatusPending(store, *currentPipeline, user.Login); err != nil { - return nil, fmt.Errorf("error updating pipeline. %s", err) + return nil, fmt.Errorf("error updating pipeline. %w", err) } var yamls []*forge_types.FileMeta diff --git a/server/pipeline/cancel.go b/server/pipeline/cancel.go index dbecca84e..b1325f817 100644 --- a/server/pipeline/cancel.go +++ b/server/pipeline/cancel.go @@ -29,12 +29,12 @@ import ( // Cancel the pipeline and returns the status. func Cancel(ctx context.Context, store store.Store, repo *model.Repo, pipeline *model.Pipeline) error { if pipeline.Status != model.StatusRunning && pipeline.Status != model.StatusPending && pipeline.Status != model.StatusBlocked { - return ErrBadRequest{Msg: "Cannot cancel a non-running or non-pending or non-blocked pipeline"} + return &ErrBadRequest{Msg: "Cannot cancel a non-running or non-pending or non-blocked pipeline"} } steps, err := store.StepList(pipeline) if err != nil { - return ErrNotFound{Msg: err.Error()} + return &ErrNotFound{Msg: err.Error()} } // First cancel/evict steps in the queue in one go @@ -92,7 +92,7 @@ func Cancel(ctx context.Context, store store.Store, repo *model.Repo, pipeline * steps, err = store.StepList(killedBuild) if err != nil { - return ErrNotFound{Msg: err.Error()} + return &ErrNotFound{Msg: err.Error()} } if killedBuild.Steps, err = model.Tree(steps); err != nil { return err diff --git a/server/pipeline/decline.go b/server/pipeline/decline.go index 6e6d2c5a3..fe6a5c706 100644 --- a/server/pipeline/decline.go +++ b/server/pipeline/decline.go @@ -31,7 +31,7 @@ func Decline(ctx context.Context, store store.Store, pipeline *model.Pipeline, u _, err := UpdateToStatusDeclined(store, *pipeline, user.Login) if err != nil { - return nil, fmt.Errorf("error updating pipeline. %s", err) + return nil, fmt.Errorf("error updating pipeline. %w", err) } if pipeline.Steps, err = store.StepList(pipeline); err != nil { diff --git a/server/pipeline/errors.go b/server/pipeline/errors.go index f18c7b899..fc403dff8 100644 --- a/server/pipeline/errors.go +++ b/server/pipeline/errors.go @@ -22,11 +22,6 @@ func (e ErrNotFound) Error() string { return e.Msg } -func IsErrNotFound(err error) bool { - _, ok := err.(ErrNotFound) - return ok -} - type ErrBadRequest struct { Msg string } @@ -35,11 +30,6 @@ func (e ErrBadRequest) Error() string { return e.Msg } -func IsErrBadRequest(err error) bool { - _, ok := err.(ErrBadRequest) - return ok -} - type ErrFiltered struct { Msg string } @@ -47,8 +37,3 @@ type ErrFiltered struct { func (e ErrFiltered) Error() string { return "ignoring hook: " + e.Msg } - -func IsErrFiltered(err error) bool { - _, ok := err.(ErrFiltered) - return ok -} diff --git a/server/pipeline/restart.go b/server/pipeline/restart.go index 8e7c14c96..a994870da 100644 --- a/server/pipeline/restart.go +++ b/server/pipeline/restart.go @@ -34,7 +34,7 @@ func Restart(ctx context.Context, store store.Store, lastBuild *model.Pipeline, switch lastBuild.Status { case model.StatusDeclined, model.StatusBlocked: - return nil, ErrBadRequest{Msg: fmt.Sprintf("cannot restart a pipeline with status %s", lastBuild.Status)} + return nil, &ErrBadRequest{Msg: fmt.Sprintf("cannot restart a pipeline with status %s", lastBuild.Status)} } var pipelineFiles []*forge_types.FileMeta @@ -44,7 +44,7 @@ func Restart(ctx context.Context, store store.Store, lastBuild *model.Pipeline, if err != nil { msg := fmt.Sprintf("failure to get pipeline config for %s. %s", repo.FullName, err) log.Error().Msgf(msg) - return nil, ErrNotFound{Msg: msg} + return nil, &ErrNotFound{Msg: msg} } for _, y := range configs { @@ -60,7 +60,7 @@ func Restart(ctx context.Context, store store.Store, lastBuild *model.Pipeline, newConfig, useOld, err := server.Config.Services.ConfigService.FetchConfig(ctx, repo, lastBuild, currentFileMeta) if err != nil { - return nil, ErrBadRequest{ + return nil, &ErrBadRequest{ Msg: fmt.Sprintf("On fetching external pipeline config: %s", err), } } diff --git a/server/plugins/encryption/aes_builder.go b/server/plugins/encryption/aes_builder.go index ece66c0e6..3b98e02d0 100644 --- a/server/plugins/encryption/aes_builder.go +++ b/server/plugins/encryption/aes_builder.go @@ -15,6 +15,7 @@ package encryption import ( + "errors" "fmt" "github.com/urfave/cli/v2" @@ -56,7 +57,7 @@ func (c aesConfiguration) Build() (model.EncryptionService, error) { } err = svc.validateKey() - if err == errEncryptionNotEnabled { + if errors.Is(err, errEncryptionNotEnabled) { err = svc.enable() } if err != nil { diff --git a/server/plugins/encryption/tink_builder.go b/server/plugins/encryption/tink_builder.go index 4dce14dc5..2a8ab4da8 100644 --- a/server/plugins/encryption/tink_builder.go +++ b/server/plugins/encryption/tink_builder.go @@ -15,6 +15,7 @@ package encryption import ( + "errors" "fmt" "github.com/urfave/cli/v2" @@ -48,20 +49,19 @@ func (c tinkConfiguration) Build() (model.EncryptionService, error) { keysetFileWatcher: nil, clients: c.clients, } - err := svc.initClients() - if err != nil { + + if err := svc.initClients(); err != nil { return nil, fmt.Errorf(errTemplateFailedInitializingClients, err) } - err = svc.loadKeyset() - if err != nil { + if err := svc.loadKeyset(); err != nil { return nil, fmt.Errorf(errTemplateTinkFailedLoadingKeyset, err) } - err = svc.validateKeyset() - if err == errEncryptionNotEnabled { + err := svc.validateKeyset() + if errors.Is(err, errEncryptionNotEnabled) { err = svc.enable() - } else if err == errEncryptionKeyRotated { + } else if errors.Is(err, errEncryptionKeyRotated) { err = svc.rotate() } @@ -69,8 +69,7 @@ func (c tinkConfiguration) Build() (model.EncryptionService, error) { return nil, fmt.Errorf(errTemplateTinkFailedValidatingKeyset, err) } - err = svc.initFileWatcher() - if err != nil { + if err := svc.initFileWatcher(); err != nil { return nil, fmt.Errorf(errTemplateTinkFailedInitializeFileWatcher, err) } return svc, nil diff --git a/server/plugins/encryption/tink_state.go b/server/plugins/encryption/tink_state.go index e7d539d4f..f15bf180a 100644 --- a/server/plugins/encryption/tink_state.go +++ b/server/plugins/encryption/tink_state.go @@ -15,33 +15,34 @@ package encryption import ( + "errors" "fmt" "github.com/rs/zerolog/log" ) func (svc *tinkEncryptionService) enable() error { - err := svc.callbackOnEnable() - if err != nil { + if err := svc.callbackOnEnable(); err != nil { return fmt.Errorf(errTemplateFailedEnablingEncryption, err) } - err = svc.updateCiphertextSample() - if err != nil { + + if err := svc.updateCiphertextSample(); err != nil { return fmt.Errorf(errTemplateFailedEnablingEncryption, err) } + log.Warn().Msg(logMessageEncryptionEnabled) return nil } func (svc *tinkEncryptionService) disable() error { - err := svc.callbackOnDisable() - if err != nil { + if err := svc.callbackOnDisable(); err != nil { return fmt.Errorf(errTemplateFailedDisablingEncryption, err) } - err = svc.deleteCiphertextSample() - if err != nil { + + if err := svc.deleteCiphertextSample(); err != nil { return fmt.Errorf(errTemplateFailedDisablingEncryption, err) } + log.Warn().Msg(logMessageEncryptionDisabled) return nil } @@ -55,26 +56,24 @@ func (svc *tinkEncryptionService) rotate() error { keysetFileWatcher: nil, clients: svc.clients, } - err := newSvc.loadKeyset() - if err != nil { + + if err := newSvc.loadKeyset(); err != nil { return fmt.Errorf(errTemplateFailedRotatingEncryption, err) } - err = newSvc.validateKeyset() - if err == errEncryptionKeyRotated { + err := newSvc.validateKeyset() + if errors.Is(err, errEncryptionKeyRotated) { err = newSvc.updateCiphertextSample() } if err != nil { return fmt.Errorf(errTemplateFailedRotatingEncryption, err) } - err = newSvc.callbackOnRotation() - if err != nil { + if err := newSvc.callbackOnRotation(); err != nil { return fmt.Errorf(errTemplateFailedRotatingEncryption, err) } - err = newSvc.initFileWatcher() - if err != nil { + if err := newSvc.initFileWatcher(); err != nil { return fmt.Errorf(errTemplateFailedRotatingEncryption, err) } return nil @@ -85,26 +84,25 @@ func (svc *tinkEncryptionService) updateCiphertextSample() error { if err != nil { return fmt.Errorf(errTemplateFailedUpdatingServerConfig, err) } - err = svc.store.ServerConfigSet(ciphertextSampleConfigKey, ciphertext) - if err != nil { + + if err := svc.store.ServerConfigSet(ciphertextSampleConfigKey, ciphertext); err != nil { return fmt.Errorf(errTemplateFailedUpdatingServerConfig, err) } + log.Info().Msg(logMessageEncryptionKeyRegistered) return nil } func (svc *tinkEncryptionService) deleteCiphertextSample() error { - err := svc.store.ServerConfigDelete(ciphertextSampleConfigKey) - if err != nil { - err = fmt.Errorf(errTemplateFailedUpdatingServerConfig, err) + if err := svc.store.ServerConfigDelete(ciphertextSampleConfigKey); err != nil { + return fmt.Errorf(errTemplateFailedUpdatingServerConfig, err) } - return err + return nil } func (svc *tinkEncryptionService) initClients() error { for _, client := range svc.clients { - err := client.SetEncryptionService(svc) - if err != nil { + if err := client.SetEncryptionService(svc); err != nil { return err } } @@ -114,8 +112,7 @@ func (svc *tinkEncryptionService) initClients() error { func (svc *tinkEncryptionService) callbackOnEnable() error { for _, client := range svc.clients { - err := client.EnableEncryption() - if err != nil { + if err := client.EnableEncryption(); err != nil { return err } } @@ -125,8 +122,7 @@ func (svc *tinkEncryptionService) callbackOnEnable() error { func (svc *tinkEncryptionService) callbackOnRotation() error { for _, client := range svc.clients { - err := client.MigrateEncryption(svc) - if err != nil { + if err := client.MigrateEncryption(svc); err != nil { return err } } @@ -136,8 +132,7 @@ func (svc *tinkEncryptionService) callbackOnRotation() error { func (svc *tinkEncryptionService) callbackOnDisable() error { for _, client := range svc.clients { - err := client.MigrateEncryption(&noEncryption{}) - if err != nil { + if err := client.MigrateEncryption(&noEncryption{}); err != nil { return err } } diff --git a/server/pubsub/pub_test.go b/server/pubsub/pub_test.go index 9635494cf..8ca99b230 100644 --- a/server/pubsub/pub_test.go +++ b/server/pubsub/pub_test.go @@ -2,6 +2,7 @@ package pubsub import ( "context" + "errors" "sync" "testing" "time" @@ -56,7 +57,7 @@ func TestPublishNotFound(t *testing.T) { ) broker := New() err := broker.Publish(context.Background(), testTopic, testMessage) - if err != ErrNotFound { + if !errors.Is(err, ErrNotFound) { t.Errorf("Expect Not Found error when topic does not exist") } } @@ -68,7 +69,7 @@ func TestSubscribeNotFound(t *testing.T) { ) broker := New() err := broker.Subscribe(context.Background(), testTopic, testCallback) - if err != ErrNotFound { + if !errors.Is(err, ErrNotFound) { t.Errorf("Expect Not Found error when topic does not exist") } } diff --git a/server/queue/fifo_test.go b/server/queue/fifo_test.go index 5db7b2ee8..668ae6cbf 100644 --- a/server/queue/fifo_test.go +++ b/server/queue/fifo_test.go @@ -2,6 +2,7 @@ package queue import ( "context" + "errors" "fmt" "sync" "testing" @@ -116,7 +117,7 @@ func TestFifoEvict(t *testing.T) { if len(info.Pending) != 0 { t.Errorf("expect pending queue has zero items") } - if err := q.Evict(noContext, t1.ID); err != ErrNotFound { + if err := q.Evict(noContext, t1.ID); !errors.Is(err, ErrNotFound) { t.Errorf("expect not found error when evicting item not in queue, got %s", err) } } diff --git a/server/store/datastore/migration/000_legacy_to_xorm.go b/server/store/datastore/migration/000_legacy_to_xorm.go index 498c62e27..5de5f5b5e 100644 --- a/server/store/datastore/migration/000_legacy_to_xorm.go +++ b/server/store/datastore/migration/000_legacy_to_xorm.go @@ -76,7 +76,7 @@ var legacy2Xorm = task{ } { exist, err := sess.Exist(&migrations{mig}) if err != nil { - return fmt.Errorf("test migration existence: %v", err) + return fmt.Errorf("test migration existence: %w", err) } if !exist { log.Error().Msgf("migration step '%s' missing, please upgrade to last stable v0.14.x version first", mig) @@ -96,10 +96,10 @@ var legacy2Xorm = task{ return err } if _, err := sess.Exec("INSERT INTO build_config (config_id, build_id) SELECT config_id,build_id FROM old_build_config;"); err != nil { - return fmt.Errorf("unable to set copy data into temp table %s. Error: %v", "old_build_config", err) + return fmt.Errorf("unable to set copy data into temp table %s. Error: %w", "old_build_config", err) } if err := sess.DropTable("old_build_config"); err != nil { - return fmt.Errorf("could not drop table '%s': %v", "old_build_config", err) + return fmt.Errorf("could not drop table '%s': %w", "old_build_config", err) } } @@ -120,7 +120,7 @@ var legacy2Xorm = task{ "DROP INDEX IF EXISTS ix_perms_user ON perms;", } { if _, err := sess.Exec(exec); err != nil { - return fmt.Errorf("exec: '%s' failed: %v", exec, err) + return fmt.Errorf("exec: '%s' failed: %w", exec, err) } } case schemas.SQLITE, schemas.POSTGRES: @@ -138,7 +138,7 @@ var legacy2Xorm = task{ "DROP INDEX IF EXISTS ix_perms_user;", } { if _, err := sess.Exec(exec); err != nil { - return fmt.Errorf("exec: '%s' failed: %v", exec, err) + return fmt.Errorf("exec: '%s' failed: %w", exec, err) } } default: diff --git a/server/store/datastore/migration/common.go b/server/store/datastore/migration/common.go index 646c88478..ab1d48cea 100644 --- a/server/store/datastore/migration/common.go +++ b/server/store/datastore/migration/common.go @@ -152,7 +152,7 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin cols += "DROP COLUMN `" + col + "` CASCADE" } if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` %s", tableName, cols)); err != nil { - return fmt.Errorf("drop table `%s` columns %v: %v", tableName, columnNames, err) + return fmt.Errorf("drop table `%s` columns %v: %w", tableName, columnNames, err) } case schemas.MYSQL: // Drop indexes on columns first @@ -180,7 +180,7 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin cols += "DROP COLUMN `" + col + "`" } if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` %s", tableName, cols)); err != nil { - return fmt.Errorf("drop table `%s` columns %v: %v", tableName, columnNames, err) + return fmt.Errorf("drop table `%s` columns %v: %w", tableName, columnNames, err) } case schemas.MSSQL: cols := "" @@ -194,27 +194,27 @@ func dropTableColumns(sess *xorm.Session, tableName string, columnNames ...strin tableName, strings.ReplaceAll(cols, "`", "'")) constraints := make([]string, 0) if err := sess.SQL(sql).Find(&constraints); err != nil { - return fmt.Errorf("find constraints: %v", err) + return fmt.Errorf("find constraints: %w", err) } for _, constraint := range constraints { if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` DROP CONSTRAINT `%s`", tableName, constraint)); err != nil { - return fmt.Errorf("drop table `%s` default constraint `%s`: %v", tableName, constraint, err) + return fmt.Errorf("drop table `%s` default constraint `%s`: %w", tableName, constraint, err) } } sql = fmt.Sprintf("SELECT DISTINCT Name FROM sys.indexes INNER JOIN sys.index_columns ON indexes.index_id = index_columns.index_id AND indexes.object_id = index_columns.object_id WHERE indexes.object_id = OBJECT_ID('%[1]s') AND index_columns.column_id IN (SELECT column_id FROM sys.columns WHERE LOWER(name) IN (%[2]s) AND object_id = OBJECT_ID('%[1]s'))", tableName, strings.ReplaceAll(cols, "`", "'")) constraints = make([]string, 0) if err := sess.SQL(sql).Find(&constraints); err != nil { - return fmt.Errorf("find constraints: %v", err) + return fmt.Errorf("find constraints: %w", err) } for _, constraint := range constraints { - if _, err := sess.Exec(fmt.Sprintf("DROP INDEX `%[2]s` ON `%[1]s`", tableName, constraint)); err != nil { - return fmt.Errorf("drop index `%[2]s` on `%[1]s`: %v", tableName, constraint, err) + if _, err := sess.Exec(fmt.Sprintf("DROP INDEX `%s` ON `%s`", constraint, tableName)); err != nil { + return fmt.Errorf("drop index `%s` on `%s`: %w", constraint, tableName, err) } } if _, err := sess.Exec(fmt.Sprintf("ALTER TABLE `%s` DROP COLUMN %s", tableName, cols)); err != nil { - return fmt.Errorf("drop table `%s` columns %v: %v", tableName, columnNames, err) + return fmt.Errorf("drop table `%s` columns %v: %w", tableName, columnNames, err) } default: return fmt.Errorf("dialect '%s' not supported", dialect) diff --git a/server/store/datastore/migration/migration.go b/server/store/datastore/migration/migration.go index 6d03d7904..39aff3316 100644 --- a/server/store/datastore/migration/migration.go +++ b/server/store/datastore/migration/migration.go @@ -175,7 +175,7 @@ type syncEngine interface { func syncAll(sess syncEngine) error { for _, bean := range allBeans { if err := sess.Sync2(bean); err != nil { - return fmt.Errorf("sync2 error '%s': %v", reflect.TypeOf(bean), err) + return fmt.Errorf("sync2 error '%s': %w", reflect.TypeOf(bean), err) } } return nil