Skip to content

Commit

Permalink
Fix a bunch of errorlint errors from our linters (#360)
Browse files Browse the repository at this point in the history
* Fix: golangci-lint errorlint

Fixes the errorlint raised by the linter

* Fix a bunch of errorlint errors from our linters

This fixes a lot of the error detecting by our linters, this skip the
errors related to the errors packages, we should refactor the usage or
our package instead.

* Fix errors.As

* fix whole file nolint

* singular

* make check shouldnt run linter

* should be !errors.Is
  • Loading branch information
ph authored Apr 27, 2022
1 parent 73c78c9 commit 9e1ccc9
Show file tree
Hide file tree
Showing 41 changed files with 99 additions and 91 deletions.
1 change: 0 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ notice:
.PHONY: check-ci
check-ci:
@mage update
@mage check
@$(MAKE) notice
@$(MAKE) check-no-changes

Expand Down
2 changes: 1 addition & 1 deletion dev-tools/mage/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,7 @@ func untar(sourceFile, destinationDir string) error {
for {
header, err := tarReader.Next()
if err != nil {
if err == io.EOF {
if errors.Is(err, io.EOF) {
break
}
return err
Expand Down
2 changes: 1 addition & 1 deletion dev-tools/mage/dockerbuilder.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func (b *dockerBuilder) dockerSave(tag string) error {
if _, err := os.Stat(distributionsDir); os.IsNotExist(err) {
err := os.MkdirAll(distributionsDir, 0750)
if err != nil {
return fmt.Errorf("cannot create folder for docker artifacts: %+v", err)
return fmt.Errorf("cannot create folder for docker artifacts: %w", err)
}
}
// Save the container as artifact
Expand Down
4 changes: 2 additions & 2 deletions dev-tools/mage/gotest.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,8 @@ func GoTest(ctx context.Context, params GoTestArgs) error {
var goTestErr *exec.ExitError
if err != nil {
// Command ran.
exitErr, ok := err.(*exec.ExitError)
if !ok {
var exitErr *exec.ExitError
if !errors.As(err, &exitErr) {
return errors.Wrap(err, "failed to execute go")
}

Expand Down
2 changes: 1 addition & 1 deletion dev-tools/mage/pkgtypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ func runFPM(spec PackageSpec, packageType PackageType) error {
}

if err := HaveDocker(); err != nil {
return fmt.Errorf("packaging %v files requires docker: %v", fpmPackageType, err)
return fmt.Errorf("packaging %v files requires docker: %w", fpmPackageType, err)
}

// Build a tar file as the input to FPM.
Expand Down
7 changes: 4 additions & 3 deletions dev-tools/packaging/package_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"bytes"
"compress/gzip"
"encoding/json"
"errors"
"flag"
"fmt"
"io"
Expand Down Expand Up @@ -586,7 +587,7 @@ func readDeb(debFile string, dataBuffer *bytes.Buffer) (*packageFile, error) {
for {
header, err := arReader.Next()
if err != nil {
if err == io.EOF {
if errors.Is(err, io.EOF) {
break
}
return nil, err
Expand Down Expand Up @@ -637,7 +638,7 @@ func readTarContents(tarName string, data io.Reader) (*packageFile, error) {
for {
header, err := tarReader.Next()
if err != nil {
if err == io.EOF {
if errors.Is(err, io.EOF) {
break
}
return nil, err
Expand Down Expand Up @@ -709,7 +710,7 @@ func readDocker(dockerFile string) (*packageFile, *dockerInfo, error) {
for {
header, err := tarReader.Next()
if err != nil {
if err == io.EOF {
if errors.Is(err, io.EOF) {
break
}
return nil, nil, err
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/agent/application/info/agent_id.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func updateLogLevel(level string) error {
func generateAgentID() (string, error) {
uid, err := uuid.NewV4()
if err != nil {
return "", fmt.Errorf("error while generating UUID for agent: %v", err)
return "", fmt.Errorf("error while generating UUID for agent: %w", err)
}

return uid.String(), nil
Expand Down Expand Up @@ -175,7 +175,7 @@ func loadAgentInfoWithBackoff(forceUpdate bool, logLevel string, createAgentID b
for i := 0; i <= maxRetriesloadAgentInfo; i++ {
backExp.Wait()
ai, err = loadAgentInfo(forceUpdate, logLevel, createAgentID)
if err != filelock.ErrAppAlreadyRunning {
if !errors.Is(err, filelock.ErrAppAlreadyRunning) {
break
}
}
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/agent/application/upgrade/step_unpack.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ func untar(version, archivePath string) (string, error) {
// pieces outside of data we already have and should not be overwritten as they are usually configs
for {
f, err := tr.Next()
if err == io.EOF {
if errors.Is(err, io.EOF) {
break
}
if err != nil {
Expand Down Expand Up @@ -198,7 +198,7 @@ func untar(version, archivePath string) (string, error) {
err = closeErr
}
if err != nil {
return "", fmt.Errorf("TarInstaller: error writing to %s: %v", abs, err)
return "", fmt.Errorf("TarInstaller: error writing to %s: %w", abs, err)
}
case mode.IsDir():
if err := os.MkdirAll(abs, 0755); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/agent/application/upgrade/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ func (u *Upgrader) Upgrade(ctx context.Context, a Action, reexecNow bool) (_ ree
}

if u.caps != nil {
if _, err := u.caps.Apply(a); err == capabilities.ErrBlocked {
if _, err := u.caps.Apply(a); errors.Is(err, capabilities.ErrBlocked) {
return nil, nil
}
}
Expand Down
28 changes: 14 additions & 14 deletions internal/pkg/agent/cmd/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,12 @@ func logContainerCmd(streams *cli.IOStreams, cmd *cobra.Command) error {
if logsPath != "" {
// log this entire command to a file as well as to the passed streams
if err := os.MkdirAll(logsPath, 0755); err != nil {
return fmt.Errorf("preparing LOGS_PATH(%s) failed: %s", logsPath, err)
return fmt.Errorf("preparing LOGS_PATH(%s) failed: %w", logsPath, err)
}
logPath := filepath.Join(logsPath, "elastic-agent-startup.log")
w, err := os.Create(logPath)
if err != nil {
return fmt.Errorf("opening startup log(%s) failed: %s", logPath, err)
return fmt.Errorf("opening startup log(%s) failed: %w", logPath, err)
}
defer w.Close()
streams.Out = io.MultiWriter(streams.Out, w)
Expand All @@ -187,12 +187,12 @@ func containerCmd(streams *cli.IOStreams, cmd *cobra.Command) error {
for _, f := range []string{"fleet-setup.yml", "credentials.yml"} {
c, err := config.LoadFile(filepath.Join(paths.Config(), f))
if err != nil && !os.IsNotExist(err) {
return fmt.Errorf("parsing config file(%s): %s", f, err)
return fmt.Errorf("parsing config file(%s): %w", f, err)
}
if c != nil {
err = c.Unpack(&cfg)
if err != nil {
return fmt.Errorf("unpacking config file(%s): %s", f, err)
return fmt.Errorf("unpacking config file(%s): %w", f, err)
}
// if in elastic cloud mode, only run the agent when configured
runAgent = true
Expand Down Expand Up @@ -649,7 +649,7 @@ func performGET(cfg setupConfig, client *kibana.Client, path string, response in
for i := 0; i < cfg.Kibana.RetryMaxCount; i++ {
code, result, err := client.Connection.Request("GET", path, nil, nil, nil)
if err != nil || code != 200 {
err = fmt.Errorf("http GET request to %s%s fails: %v. Response: %s",
err = fmt.Errorf("http GET request to %s%s fails: %w. Response: %s",
client.Connection.URL, path, err, truncateString(result))
fmt.Fprintf(writer, "%s failed: %s\n", msg, err)
<-time.After(cfg.Kibana.RetrySleepDuration)
Expand All @@ -668,7 +668,7 @@ func performPOST(cfg setupConfig, client *kibana.Client, path string, writer io.
for i := 0; i < cfg.Kibana.RetryMaxCount; i++ {
code, result, err := client.Connection.Request("POST", path, nil, nil, nil)
if err != nil || code >= 400 {
err = fmt.Errorf("http POST request to %s%s fails: %v. Response: %s",
err = fmt.Errorf("http POST request to %s%s fails: %w. Response: %s",
client.Connection.URL, path, err, truncateString(result))
lastErr = err
fmt.Fprintf(writer, "%s failed: %s\n", msg, err)
Expand Down Expand Up @@ -771,7 +771,7 @@ func setPaths(statePath, configPath, logsPath string, writePaths bool) error {
}
// ensure that the directory and sub-directory data exists
if err := os.MkdirAll(topPath, 0755); err != nil {
return fmt.Errorf("preparing STATE_PATH(%s) failed: %s", statePath, err)
return fmt.Errorf("preparing STATE_PATH(%s) failed: %w", statePath, err)
}
// ensure that the elastic-agent.yml exists in the state directory or if given in the config directory
baseConfig := filepath.Join(configPath, paths.DefaultConfigName)
Expand All @@ -783,7 +783,7 @@ func setPaths(statePath, configPath, logsPath string, writePaths bool) error {
// sync the downloads to the data directory
destDownloads := filepath.Join(statePath, "data", "downloads")
if err := syncDir(paths.Downloads(), destDownloads); err != nil {
return fmt.Errorf("syncing download directory to STATE_PATH(%s) failed: %s", statePath, err)
return fmt.Errorf("syncing download directory to STATE_PATH(%s) failed: %w", statePath, err)
}
originalInstall := paths.Install()
originalTop := paths.Top()
Expand All @@ -799,7 +799,7 @@ func setPaths(statePath, configPath, logsPath string, writePaths bool) error {
paths.SetLogs(logsPath)
// ensure that the logs directory exists
if err := os.MkdirAll(filepath.Join(logsPath), 0755); err != nil {
return fmt.Errorf("preparing LOGS_PATH(%s) failed: %s", logsPath, err)
return fmt.Errorf("preparing LOGS_PATH(%s) failed: %w", logsPath, err)
}
}
// persist the paths so other commands in the container will use the correct paths
Expand All @@ -821,19 +821,19 @@ func writeContainerPaths(original, statePath, configPath, logsPath string) error
pathFile := filepath.Join(original, "container-paths.yml")
fp, err := os.Create(pathFile)
if err != nil {
return fmt.Errorf("failed creating %s: %s", pathFile, err)
return fmt.Errorf("failed creating %s: %w", pathFile, err)
}
b, err := yaml.Marshal(containerPaths{
StatePath: statePath,
ConfigPath: configPath,
LogsPath: logsPath,
})
if err != nil {
return fmt.Errorf("failed to marshal for %s: %s", pathFile, err)
return fmt.Errorf("failed to marshal for %s: %w", pathFile, err)
}
_, err = fp.Write(b)
if err != nil {
return fmt.Errorf("failed to write %s: %s", pathFile, err)
return fmt.Errorf("failed to write %s: %w", pathFile, err)
}
return nil
}
Expand All @@ -847,12 +847,12 @@ func tryContainerLoadPaths() error {
}
cfg, err := config.LoadFile(pathFile)
if err != nil {
return fmt.Errorf("failed to load %s: %s", pathFile, err)
return fmt.Errorf("failed to load %s: %w", pathFile, err)
}
var paths containerPaths
err = cfg.Unpack(&paths)
if err != nil {
return fmt.Errorf("failed to unpack %s: %s", pathFile, err)
return fmt.Errorf("failed to unpack %s: %w", pathFile, err)
}
return setPaths(paths.StatePath, paths.ConfigPath, paths.LogsPath, false)
}
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/agent/cmd/enroll_cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ func waitForAgent(ctx context.Context, timeout time.Duration) error {
for {
backOff.Wait()
_, err := getDaemonStatus(innerCtx)
if err == context.Canceled {
if errors.Is(err, context.Canceled) {
resChan <- waitResult{err: err}
return
}
Expand Down Expand Up @@ -721,7 +721,7 @@ func waitForFleetServer(ctx context.Context, agentSubproc <-chan *os.ProcessStat
for {
backExp.Wait()
status, err := getDaemonStatus(innerCtx)
if err == context.Canceled {
if errors.Is(err, context.Canceled) {
resChan <- waitResult{err: err}
return
}
Expand Down Expand Up @@ -834,7 +834,7 @@ func safelyStoreAgentInfo(s saver, reader io.Reader) error {
for i := 0; i <= maxRetriesstoreAgentInfo; i++ {
backExp.Wait()
err = storeAgentInfo(s, reader)
if err != filelock.ErrAppAlreadyRunning {
if !errors.Is(err, filelock.ErrAppAlreadyRunning) {
break
}
}
Expand Down
13 changes: 7 additions & 6 deletions internal/pkg/agent/cmd/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package cmd

import (
"errors"
"fmt"
"os"
"os/exec"
Expand Down Expand Up @@ -49,7 +50,7 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command, args []string) error

isAdmin, err := install.HasRoot()
if err != nil {
return fmt.Errorf("unable to perform install command while checking for administrator rights, %v", err)
return fmt.Errorf("unable to perform install command while checking for administrator rights, %w", err)
}
if !isAdmin {
return fmt.Errorf("unable to perform install command, not executed with %s permissions", install.PermissionUser)
Expand All @@ -72,7 +73,7 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command, args []string) error
// check the lock to ensure that elastic-agent is not already running in this directory
locker := filelock.NewAppLocker(paths.Data(), paths.AgentLockFileName)
if err := locker.TryLock(); err != nil {
if err == filelock.ErrAppAlreadyRunning {
if errors.Is(err, filelock.ErrAppAlreadyRunning) {
return fmt.Errorf("cannot perform installation as Elastic Agent is already running from this directory")
}
return err
Expand Down Expand Up @@ -195,18 +196,18 @@ func installCmd(streams *cli.IOStreams, cmd *cobra.Command, args []string) error
enrollCmd.Stderr = os.Stderr
err = enrollCmd.Start()
if err != nil {
return fmt.Errorf("failed to execute enroll command: %s", err)
return fmt.Errorf("failed to execute enroll command: %w", err)
}
err = enrollCmd.Wait()
if err != nil {
if status != install.PackageInstall {
var exitErr *exec.ExitError
install.Uninstall(cfgFile)
exitErr, ok := err.(*exec.ExitError)
if ok {
if err != nil && errors.As(err, &exitErr) {
return fmt.Errorf("enroll command failed with exit code: %d", exitErr.ExitCode())
}
}
return fmt.Errorf("enroll command failed for unknown reason: %s", err)
return fmt.Errorf("enroll command failed for unknown reason: %w", err)
}
}

Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/agent/cmd/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ func statusCmd(streams *cli.IOStreams, cmd *cobra.Command, args []string) error
defer cancel()

status, err := getDaemonStatus(innerCtx)
if err == context.DeadlineExceeded {
if errors.Is(err, context.DeadlineExceeded) {
return errors.New("timed out after 30 seconds trying to connect to Elastic Agent daemon")
} else if err == context.Canceled {
} else if errors.Is(err, context.Canceled) {
return nil
} else if err != nil {
return fmt.Errorf("failed to communicate with Elastic Agent daemon: %s", err)
return fmt.Errorf("failed to communicate with Elastic Agent daemon: %w", err)
}

err = outputFunc(streams.Out, status)
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/agent/cmd/uninstall.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Unless -f is used this command will ask confirmation before performing removal.
func uninstallCmd(streams *cli.IOStreams, cmd *cobra.Command, args []string) error {
isAdmin, err := install.HasRoot()
if err != nil {
return fmt.Errorf("unable to perform command while checking for administrator rights, %v", err)
return fmt.Errorf("unable to perform command while checking for administrator rights, %w", err)
}
if !isAdmin {
return fmt.Errorf("unable to perform command, not executed with %s permissions", install.PermissionUser)
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/agent/cmd/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func watchCmd(streams *cli.IOStreams, cmd *cobra.Command, args []string) error {

locker := filelock.NewAppLocker(paths.Top(), watcherLockFile)
if err := locker.TryLock(); err != nil {
if err == filelock.ErrAppAlreadyRunning {
if errors.Is(err, filelock.ErrAppAlreadyRunning) {
log.Debugf("exiting, lock already exists")
return nil
}
Expand Down
6 changes: 5 additions & 1 deletion internal/pkg/agent/errors/error.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.

//
// nolint:errorlint // Postpone the change here until we refactor error handling.
//
// Package errors provides a small api to manage hierarchy of errors.
package errors

import (
Expand Down Expand Up @@ -132,6 +135,7 @@ func (e agentError) Meta() map[string]interface{} {
// do the heavy lifting ourselves.
func (e agentError) Equal(target error) bool {
targetErr, ok := target.(agentError)

if !ok {
return false
}
Expand Down
5 changes: 4 additions & 1 deletion internal/pkg/agent/errors/error_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
// or more contributor license agreements. Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.

//
// nolint:errorlint // Postpone the change here until we refactor error handling.
//
// Packages errors provides a small api to manager hierarchy or errors.
package errors

import (
Expand Down
Loading

0 comments on commit 9e1ccc9

Please sign in to comment.