From a96dda4f43eb4526bee5ce2e4e14f92b0cba2985 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Sun, 27 Apr 2025 21:17:13 +0200 Subject: [PATCH] chore: enable errorlint Signed-off-by: Matthieu MOREL --- .golangci.yml | 1 + cmd/helm/helm.go | 6 ++-- cmd/helm/helm_test.go | 8 ++--- internal/chart/v3/loader/archive.go | 8 ++--- internal/chart/v3/loader/load.go | 2 +- internal/chart/v3/metadata_test.go | 3 +- internal/chart/v3/util/dependencies.go | 4 ++- internal/chart/v3/util/dependencies_test.go | 10 +++---- internal/sympath/walk.go | 9 +++--- internal/third_party/dep/fs/fs.go | 3 +- internal/third_party/dep/fs/fs_test.go | 10 +++---- internal/third_party/dep/fs/rename.go | 7 +++-- pkg/action/install.go | 4 +-- pkg/action/package_test.go | 6 ++-- pkg/action/upgrade.go | 4 +-- pkg/action/validate.go | 14 ++++----- pkg/chart/v2/loader/archive.go | 10 +++---- pkg/chart/v2/loader/load.go | 2 +- pkg/chart/v2/metadata_test.go | 6 ++-- pkg/chart/v2/util/dependencies.go | 7 +++-- pkg/chart/v2/util/dependencies_test.go | 11 +++---- pkg/cmd/dependency_build.go | 4 ++- pkg/cmd/dependency_update_test.go | 8 ++--- pkg/cmd/lint.go | 2 +- pkg/cmd/load_plugins.go | 4 ++- pkg/cmd/plugin.go | 4 ++- pkg/cmd/plugin_test.go | 13 ++++---- pkg/cmd/plugin_uninstall.go | 2 +- pkg/cmd/plugin_update.go | 2 +- pkg/cmd/rollback.go | 2 +- pkg/cmd/search_hub.go | 2 +- pkg/cmd/search_repo.go | 2 +- pkg/cmd/template.go | 2 +- pkg/cmd/upgrade.go | 3 +- pkg/downloader/chart_downloader.go | 2 +- pkg/downloader/chart_downloader_test.go | 7 +++-- pkg/downloader/manager.go | 8 ++--- pkg/engine/engine.go | 15 ++++++---- pkg/engine/engine_test.go | 5 ---- pkg/getter/plugingetter.go | 4 ++- pkg/kube/client.go | 4 +-- pkg/kube/wait.go | 3 +- pkg/lint/rules/chartfile.go | 2 +- pkg/lint/rules/crds.go | 2 +- pkg/lint/rules/deprecations_test.go | 9 ++++-- pkg/lint/rules/template.go | 2 +- pkg/lint/rules/template_test.go | 5 +++- pkg/plugin/installer/http_installer.go | 2 +- pkg/plugin/installer/local_installer_test.go | 6 ++-- pkg/provenance/sign_test.go | 6 ++-- pkg/registry/transport.go | 3 +- pkg/registry/util.go | 2 +- pkg/repo/chartrepo_test.go | 6 ++-- pkg/repo/index.go | 4 +-- pkg/repo/index_test.go | 7 ++--- pkg/storage/driver/cfgmaps_test.go | 15 ++++------ pkg/storage/driver/secrets_test.go | 15 ++++------ pkg/storage/driver/sql.go | 4 +-- pkg/storage/driver/sql_test.go | 7 ++--- pkg/storage/storage_test.go | 6 ++-- pkg/strvals/literal_parser.go | 7 +++-- pkg/strvals/parser.go | 31 ++++++++++---------- 62 files changed, 189 insertions(+), 185 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index a9b13c35f..07af7d9bb 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -20,6 +20,7 @@ linters: enable: - depguard - dupl + - errorlint - gomodguard - govet - ineffassign diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index 05e7e7ba2..b557eb52e 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -17,6 +17,7 @@ limitations under the License. package main // import "helm.sh/helm/v4/cmd/helm" import ( + "errors" "log/slog" "os" @@ -41,8 +42,9 @@ func main() { } if err := cmd.Execute(); err != nil { - switch e := err.(type) { - case helmcmd.PluginError: + var e helmcmd.PluginError + switch { + case errors.As(err, &e): os.Exit(e.Code) default: os.Exit(1) diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go index 5431daad0..1ad66993a 100644 --- a/cmd/helm/helm_test.go +++ b/cmd/helm/helm_test.go @@ -22,6 +22,8 @@ import ( "os/exec" "runtime" "testing" + + "github.com/stretchr/testify/require" ) func TestPluginExitCode(t *testing.T) { @@ -57,11 +59,9 @@ func TestPluginExitCode(t *testing.T) { cmd.Stdout = stdout cmd.Stderr = stderr err := cmd.Run() - exiterr, ok := err.(*exec.ExitError) + var exiterr *exec.ExitError - if !ok { - t.Fatalf("Unexpected error returned by os.Exit: %T", err) - } + require.ErrorAs(t, err, &exiterr, "Unexpected error returned by os.Exit: %T", err) if stdout.String() != "" { t.Errorf("Expected no write to stdout: Got %q", stdout.String()) diff --git a/internal/chart/v3/loader/archive.go b/internal/chart/v3/loader/archive.go index 311959d56..36e30ed5b 100644 --- a/internal/chart/v3/loader/archive.go +++ b/internal/chart/v3/loader/archive.go @@ -72,8 +72,8 @@ func LoadFile(name string) (*chart.Chart, error) { c, err := LoadArchive(raw) if err != nil { - if err == gzip.ErrHeader { - return nil, fmt.Errorf("file '%s' does not appear to be a valid chart file (details: %s)", name, err) + if errors.Is(err, gzip.ErrHeader) { + return nil, fmt.Errorf("file '%s' does not appear to be a valid chart file (details: %w)", name, err) } } return c, err @@ -91,7 +91,7 @@ func ensureArchive(name string, raw *os.File) error { buffer := make([]byte, 512) _, err := raw.Read(buffer) if err != nil && err != io.EOF { - return fmt.Errorf("file '%s' cannot be read: %s", name, err) + return fmt.Errorf("file '%s' cannot be read: %w", name, err) } // Helm may identify achieve of the application/x-gzip as application/vnd.ms-fontobject. @@ -131,7 +131,7 @@ func LoadArchiveFiles(in io.Reader) ([]*BufferedFile, error) { for { b := bytes.NewBuffer(nil) hd, err := tr.Next() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } if err != nil { diff --git a/internal/chart/v3/loader/load.go b/internal/chart/v3/loader/load.go index 30bafdad4..60b53640b 100644 --- a/internal/chart/v3/loader/load.go +++ b/internal/chart/v3/loader/load.go @@ -186,7 +186,7 @@ func LoadValues(data io.Reader) (map[string]interface{}, error) { currentMap := map[string]interface{}{} raw, err := reader.Read() if err != nil { - if err == io.EOF { + if errors.Is(err, io.EOF) { break } return nil, fmt.Errorf("error reading yaml document: %w", err) diff --git a/internal/chart/v3/metadata_test.go b/internal/chart/v3/metadata_test.go index 596a03695..3d773e7f4 100644 --- a/internal/chart/v3/metadata_test.go +++ b/internal/chart/v3/metadata_test.go @@ -16,6 +16,7 @@ limitations under the License. package v3 import ( + "errors" "testing" ) @@ -181,7 +182,7 @@ func TestValidate(t *testing.T) { for _, tt := range tests { result := tt.md.Validate() - if result != tt.err { + if !errors.Is(result, tt.err) { t.Errorf("expected %q, got %q in test %q", tt.err, result, tt.name) } } diff --git a/internal/chart/v3/util/dependencies.go b/internal/chart/v3/util/dependencies.go index bd5032ce4..26c4675a8 100644 --- a/internal/chart/v3/util/dependencies.go +++ b/internal/chart/v3/util/dependencies.go @@ -16,6 +16,7 @@ limitations under the License. package util import ( + "errors" "log/slog" "strings" @@ -42,6 +43,7 @@ func processDependencyConditions(reqs []*chart.Dependency, cvals Values, cpath s if len(c) > 0 { // retrieve value vv, err := cvals.PathValue(cpath + c) + var errNoValue ErrNoValue if err == nil { // if not bool, warn if bv, ok := vv.(bool); ok { @@ -49,7 +51,7 @@ func processDependencyConditions(reqs []*chart.Dependency, cvals Values, cpath s break } slog.Warn("returned non-bool value", "path", c, "chart", r.Name) - } else if _, ok := err.(ErrNoValue); !ok { + } else if !errors.As(err, &errNoValue) { // this is a real error slog.Warn("the method PathValue returned error", slog.Any("error", err)) } diff --git a/internal/chart/v3/util/dependencies_test.go b/internal/chart/v3/util/dependencies_test.go index 55839fe65..09dbbdee4 100644 --- a/internal/chart/v3/util/dependencies_test.go +++ b/internal/chart/v3/util/dependencies_test.go @@ -21,6 +21,8 @@ import ( "strconv" "testing" + "github.com/stretchr/testify/require" + chart "helm.sh/helm/v4/internal/chart/v3" "helm.sh/helm/v4/internal/chart/v3/loader" ) @@ -250,12 +252,8 @@ func TestProcessDependencyImportValues(t *testing.T) { if err == nil { t.Error("expect nil value not found but found it") } - switch xerr := err.(type) { - case ErrNoValue: - // We found what we expected - default: - t.Errorf("expected an ErrNoValue but got %q instead", xerr) - } + var errNoValue ErrNoValue + require.ErrorAs(t, err, &errNoValue, "expected an ErrNoValue but got %q instead", err) c = loadChart(t, "testdata/subpop") if err := processDependencyImportValues(c, true); err != nil { diff --git a/internal/sympath/walk.go b/internal/sympath/walk.go index f67b9f1b9..324276571 100644 --- a/internal/sympath/walk.go +++ b/internal/sympath/walk.go @@ -21,6 +21,7 @@ limitations under the License. package sympath import ( + "errors" "fmt" "log/slog" "os" @@ -40,7 +41,7 @@ func Walk(root string, walkFn filepath.WalkFunc) error { } else { err = symwalk(root, info, walkFn) } - if err == filepath.SkipDir { + if errors.Is(err, filepath.SkipDir) { return nil } return err @@ -75,7 +76,7 @@ func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { if info, err = os.Lstat(resolved); err != nil { return err } - if err := symwalk(path, info, walkFn); err != nil && err != filepath.SkipDir { + if err := symwalk(path, info, walkFn); err != nil && !errors.Is(err, filepath.SkipDir) { return err } return nil @@ -98,13 +99,13 @@ func symwalk(path string, info os.FileInfo, walkFn filepath.WalkFunc) error { filename := filepath.Join(path, name) fileInfo, err := os.Lstat(filename) if err != nil { - if err := walkFn(filename, fileInfo, err); err != nil && err != filepath.SkipDir { + if err := walkFn(filename, fileInfo, err); err != nil && !errors.Is(err, filepath.SkipDir) { return err } } else { err = symwalk(filename, fileInfo, walkFn) if err != nil { - if (!fileInfo.IsDir() && !IsSymlink(fileInfo)) || err != filepath.SkipDir { + if (!fileInfo.IsDir() && !IsSymlink(fileInfo)) || !errors.Is(err, filepath.SkipDir) { return err } } diff --git a/internal/third_party/dep/fs/fs.go b/internal/third_party/dep/fs/fs.go index 717eff04d..6023a44a8 100644 --- a/internal/third_party/dep/fs/fs.go +++ b/internal/third_party/dep/fs/fs.go @@ -164,7 +164,8 @@ func copyFile(src, dst string) (err error) { // // ERROR_PRIVILEGE_NOT_HELD is 1314 (0x522): // https://msdn.microsoft.com/en-us/library/windows/desktop/ms681385(v=vs.85).aspx - if lerr, ok := err.(*os.LinkError); ok && lerr.Err != syscall.Errno(1314) { + var lerr *os.LinkError + if errors.As(err, &lerr) && !errors.Is(lerr.Err, syscall.Errno(1314)) { return err } } else { diff --git a/internal/third_party/dep/fs/fs_test.go b/internal/third_party/dep/fs/fs_test.go index 4c59d17fe..1e83e956d 100644 --- a/internal/third_party/dep/fs/fs_test.go +++ b/internal/third_party/dep/fs/fs_test.go @@ -36,6 +36,8 @@ import ( "path/filepath" "runtime" "testing" + + "github.com/stretchr/testify/require" ) func TestRenameWithFallback(t *testing.T) { @@ -234,9 +236,7 @@ func TestCopyDirFail_SrcIsNotDir(t *testing.T) { t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir) } - if err != errSrcNotDir { - t.Fatalf("expected %v error for CopyDir(%s, %s), got %s", errSrcNotDir, srcdir, dstdir, err) - } + require.ErrorIsf(t, err, errSrcNotDir, "expected %v error for CopyDir(%s, %s), got %s", errSrcNotDir, srcdir, dstdir, err) } @@ -260,9 +260,7 @@ func TestCopyDirFail_DstExists(t *testing.T) { t.Fatalf("expected error for CopyDir(%s, %s), got none", srcdir, dstdir) } - if err != errDstExist { - t.Fatalf("expected %v error for CopyDir(%s, %s), got %s", errDstExist, srcdir, dstdir, err) - } + require.ErrorIsf(t, err, errDstExist, "expected %v error for CopyDir(%s, %s), got %s", errDstExist, srcdir, dstdir, err) } func TestCopyDirFailOpen(t *testing.T) { diff --git a/internal/third_party/dep/fs/rename.go b/internal/third_party/dep/fs/rename.go index 5f13b1ca3..455cd85a5 100644 --- a/internal/third_party/dep/fs/rename.go +++ b/internal/third_party/dep/fs/rename.go @@ -34,6 +34,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. package fs import ( + "errors" "fmt" "os" "syscall" @@ -46,10 +47,10 @@ func renameFallback(err error, src, dst string) error { // copy if we detect that case. syscall.EXDEV is the common name for the // cross device link error which has varying output text across different // operating systems. - terr, ok := err.(*os.LinkError) - if !ok { + var terr *os.LinkError + if !errors.As(err, &terr) { return err - } else if terr.Err != syscall.EXDEV { + } else if !errors.Is(terr.Err, syscall.EXDEV) { return fmt.Errorf("link error: cannot rename %s to %s: %w", src, dst, terr) } diff --git a/pkg/action/install.go b/pkg/action/install.go index d8efa5d5d..5612f3325 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -460,7 +460,7 @@ func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.Resource // pre-install hooks if !i.DisableHooks { if err := i.cfg.execHook(rel, release.HookPreInstall, i.WaitStrategy, i.Timeout); err != nil { - return rel, fmt.Errorf("failed pre-install: %s", err) + return rel, fmt.Errorf("failed pre-install: %w", err) } } @@ -496,7 +496,7 @@ func (i *Install) performInstall(rel *release.Release, toBeAdopted kube.Resource if !i.DisableHooks { if err := i.cfg.execHook(rel, release.HookPostInstall, i.WaitStrategy, i.Timeout); err != nil { - return rel, fmt.Errorf("failed post-install: %s", err) + return rel, fmt.Errorf("failed post-install: %w", err) } } diff --git a/pkg/action/package_test.go b/pkg/action/package_test.go index 12bea10dd..3f9811b26 100644 --- a/pkg/action/package_test.go +++ b/pkg/action/package_test.go @@ -22,6 +22,7 @@ import ( "testing" "github.com/Masterminds/semver/v3" + "github.com/stretchr/testify/require" "helm.sh/helm/v4/internal/test/ensure" ) @@ -144,10 +145,7 @@ func TestValidateVersion(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { if err := validateVersion(tt.args.ver); err != nil { - if err != tt.wantErr { - t.Errorf("Expected {%v}, got {%v}", tt.wantErr, err) - } - + require.ErrorIsf(t, err, tt.wantErr, "Expected {%v}, got {%v}", tt.wantErr, err) } }) } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 0567c8de2..c3b9ac776 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -419,7 +419,7 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele if !u.DisableHooks { if err := u.cfg.execHook(upgradedRelease, release.HookPreUpgrade, u.WaitStrategy, u.Timeout); err != nil { - u.reportToPerformUpgrade(c, upgradedRelease, kube.ResourceList{}, fmt.Errorf("pre-upgrade hooks failed: %s", err)) + u.reportToPerformUpgrade(c, upgradedRelease, kube.ResourceList{}, fmt.Errorf("pre-upgrade hooks failed: %w", err)) return } } else { @@ -456,7 +456,7 @@ func (u *Upgrade) releasingUpgrade(c chan<- resultMessage, upgradedRelease *rele // post-upgrade hooks if !u.DisableHooks { if err := u.cfg.execHook(upgradedRelease, release.HookPostUpgrade, u.WaitStrategy, u.Timeout); err != nil { - u.reportToPerformUpgrade(c, upgradedRelease, results.Created, fmt.Errorf("post-upgrade hooks failed: %s", err)) + u.reportToPerformUpgrade(c, upgradedRelease, results.Created, fmt.Errorf("post-upgrade hooks failed: %w", err)) return } } diff --git a/pkg/action/validate.go b/pkg/action/validate.go index 761ccba47..95ac91a9b 100644 --- a/pkg/action/validate.go +++ b/pkg/action/validate.go @@ -81,7 +81,7 @@ func existingResourceConflict(resources kube.ResourceList, releaseName, releaseN // Allow adoption of the resource if it is managed by Helm and is annotated with correct release name and namespace. if err := checkOwnership(existing, releaseName, releaseNamespace); err != nil { - return fmt.Errorf("%s exists and cannot be imported into the current release: %s", resourceString(info), err) + return fmt.Errorf("%s exists and cannot be imported into the current release: %w", resourceString(info), err) } requireUpdate.Append(info) @@ -103,13 +103,13 @@ func checkOwnership(obj runtime.Object, releaseName, releaseNamespace string) er var errs []error if err := requireValue(lbls, appManagedByLabel, appManagedByHelm); err != nil { - errs = append(errs, fmt.Errorf("label validation error: %s", err)) + errs = append(errs, fmt.Errorf("label validation error: %w", err)) } if err := requireValue(annos, helmReleaseNameAnnotation, releaseName); err != nil { - errs = append(errs, fmt.Errorf("annotation validation error: %s", err)) + errs = append(errs, fmt.Errorf("annotation validation error: %w", err)) } if err := requireValue(annos, helmReleaseNamespaceAnnotation, releaseNamespace); err != nil { - errs = append(errs, fmt.Errorf("annotation validation error: %s", err)) + errs = append(errs, fmt.Errorf("annotation validation error: %w", err)) } if len(errs) > 0 { @@ -141,7 +141,7 @@ func setMetadataVisitor(releaseName, releaseNamespace string, forceOwnership boo if !forceOwnership { if err := checkOwnership(info.Object, releaseName, releaseNamespace); err != nil { - return fmt.Errorf("%s cannot be owned: %s", resourceString(info), err) + return fmt.Errorf("%s cannot be owned: %w", resourceString(info), err) } } @@ -149,7 +149,7 @@ func setMetadataVisitor(releaseName, releaseNamespace string, forceOwnership boo appManagedByLabel: appManagedByHelm, }); err != nil { return fmt.Errorf( - "%s labels could not be updated: %s", + "%s labels could not be updated: %w", resourceString(info), err, ) } @@ -159,7 +159,7 @@ func setMetadataVisitor(releaseName, releaseNamespace string, forceOwnership boo helmReleaseNamespaceAnnotation: releaseNamespace, }); err != nil { return fmt.Errorf( - "%s annotations could not be updated: %s", + "%s annotations could not be updated: %w", resourceString(info), err, ) } diff --git a/pkg/chart/v2/loader/archive.go b/pkg/chart/v2/loader/archive.go index b9f370f56..8da2c8cb3 100644 --- a/pkg/chart/v2/loader/archive.go +++ b/pkg/chart/v2/loader/archive.go @@ -72,8 +72,8 @@ func LoadFile(name string) (*chart.Chart, error) { c, err := LoadArchive(raw) if err != nil { - if err == gzip.ErrHeader { - return nil, fmt.Errorf("file '%s' does not appear to be a valid chart file (details: %s)", name, err) + if errors.Is(err, gzip.ErrHeader) { + return nil, fmt.Errorf("file '%s' does not appear to be a valid chart file (details: %w)", name, err) } } return c, err @@ -90,8 +90,8 @@ func ensureArchive(name string, raw *os.File) error { // Check the file format to give us a chance to provide the user with more actionable feedback. buffer := make([]byte, 512) _, err := raw.Read(buffer) - if err != nil && err != io.EOF { - return fmt.Errorf("file '%s' cannot be read: %s", name, err) + if err != nil && !errors.Is(err, io.EOF) { + return fmt.Errorf("file '%s' cannot be read: %w", name, err) } // Helm may identify achieve of the application/x-gzip as application/vnd.ms-fontobject. @@ -131,7 +131,7 @@ func LoadArchiveFiles(in io.Reader) ([]*BufferedFile, error) { for { b := bytes.NewBuffer(nil) hd, err := tr.Next() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } if err != nil { diff --git a/pkg/chart/v2/loader/load.go b/pkg/chart/v2/loader/load.go index 75c73e959..0fa3fd1d6 100644 --- a/pkg/chart/v2/loader/load.go +++ b/pkg/chart/v2/loader/load.go @@ -218,7 +218,7 @@ func LoadValues(data io.Reader) (map[string]interface{}, error) { currentMap := map[string]interface{}{} raw, err := reader.Read() if err != nil { - if err == io.EOF { + if errors.Is(err, io.EOF) { break } return nil, fmt.Errorf("error reading yaml document: %w", err) diff --git a/pkg/chart/v2/metadata_test.go b/pkg/chart/v2/metadata_test.go index 7892f0209..645d3e4e4 100644 --- a/pkg/chart/v2/metadata_test.go +++ b/pkg/chart/v2/metadata_test.go @@ -17,6 +17,8 @@ package v2 import ( "testing" + + "github.com/stretchr/testify/require" ) func TestValidate(t *testing.T) { @@ -181,9 +183,7 @@ func TestValidate(t *testing.T) { for _, tt := range tests { result := tt.md.Validate() - if result != tt.err { - t.Errorf("expected %q, got %q in test %q", tt.err, result, tt.name) - } + require.ErrorIsf(t, result, tt.err, "expected %q, got %q in test %q", tt.err, result, tt.name) } } diff --git a/pkg/chart/v2/util/dependencies.go b/pkg/chart/v2/util/dependencies.go index f34144526..de0cbfde9 100644 --- a/pkg/chart/v2/util/dependencies.go +++ b/pkg/chart/v2/util/dependencies.go @@ -16,6 +16,7 @@ limitations under the License. package util import ( + "errors" "log/slog" "strings" @@ -42,14 +43,16 @@ func processDependencyConditions(reqs []*chart.Dependency, cvals Values, cpath s if len(c) > 0 { // retrieve value vv, err := cvals.PathValue(cpath + c) - if err == nil { + var errNoValue ErrNoValue + switch { + case err == nil: // if not bool, warn if bv, ok := vv.(bool); ok { r.Enabled = bv break } slog.Warn("returned non-bool value", "path", c, "chart", r.Name) - } else if _, ok := err.(ErrNoValue); !ok { + case !errors.As(err, &errNoValue): // this is a real error slog.Warn("the method PathValue returned error", slog.Any("error", err)) } diff --git a/pkg/chart/v2/util/dependencies_test.go b/pkg/chart/v2/util/dependencies_test.go index d645d7bf5..fc5482fac 100644 --- a/pkg/chart/v2/util/dependencies_test.go +++ b/pkg/chart/v2/util/dependencies_test.go @@ -21,6 +21,8 @@ import ( "strconv" "testing" + "github.com/stretchr/testify/require" + chart "helm.sh/helm/v4/pkg/chart/v2" "helm.sh/helm/v4/pkg/chart/v2/loader" ) @@ -250,12 +252,8 @@ func TestProcessDependencyImportValues(t *testing.T) { if err == nil { t.Error("expect nil value not found but found it") } - switch xerr := err.(type) { - case ErrNoValue: - // We found what we expected - default: - t.Errorf("expected an ErrNoValue but got %q instead", xerr) - } + var xerr ErrNoValue + require.ErrorAs(t, err, &xerr, "expected an ErrNoValue but got %q instead", err) c = loadChart(t, "testdata/subpop") if err := processDependencyImportValues(c, true); err != nil { @@ -458,7 +456,6 @@ func TestDependentChartAliases(t *testing.T) { if aliasChart := getAliasDependency(c.Dependencies(), req[2]); aliasChart != nil { t.Fatalf("expected no chart but got %s", aliasChart.Name()) } - } func TestDependentChartWithSubChartsAbsentInDependency(t *testing.T) { diff --git a/pkg/cmd/dependency_build.go b/pkg/cmd/dependency_build.go index 16907facf..3363d0c37 100644 --- a/pkg/cmd/dependency_build.go +++ b/pkg/cmd/dependency_build.go @@ -16,6 +16,7 @@ limitations under the License. package cmd import ( + "errors" "fmt" "io" "os" @@ -75,7 +76,8 @@ func newDependencyBuildCmd(out io.Writer) *cobra.Command { man.Verify = downloader.VerifyIfPossible } err = man.Build() - if e, ok := err.(downloader.ErrRepoNotFound); ok { + var e downloader.ErrRepoNotFound + if errors.As(err, &e) { return fmt.Errorf("%s. Please add the missing repos via 'helm repo add'", e.Error()) } return err diff --git a/pkg/cmd/dependency_update_test.go b/pkg/cmd/dependency_update_test.go index 9646c6816..7e7625a03 100644 --- a/pkg/cmd/dependency_update_test.go +++ b/pkg/cmd/dependency_update_test.go @@ -16,7 +16,6 @@ limitations under the License. package cmd import ( - "errors" "fmt" "io/fs" "os" @@ -24,6 +23,8 @@ import ( "strings" "testing" + "github.com/stretchr/testify/require" + "helm.sh/helm/v4/internal/test/ensure" chart "helm.sh/helm/v4/pkg/chart/v2" chartutil "helm.sh/helm/v4/pkg/chart/v2/util" @@ -204,9 +205,8 @@ func TestDependencyUpdateCmd_DoNotDeleteOldChartsOnError(t *testing.T) { // Make sure tmpcharts-x is deleted tmpPath := filepath.Join(dir(chartname), fmt.Sprintf("tmpcharts-%d", os.Getpid())) - if _, err := os.Stat(tmpPath); !errors.Is(err, fs.ErrNotExist) { - t.Fatalf("tmpcharts dir still exists") - } + _, err = os.Stat(tmpPath) + require.ErrorIsf(t, err, fs.ErrNotExist, "tmpcharts dir still exists") } func TestDependencyUpdateCmd_WithRepoThatWasNotAdded(t *testing.T) { diff --git a/pkg/cmd/lint.go b/pkg/cmd/lint.go index 78083a7ea..ee4fa9da5 100644 --- a/pkg/cmd/lint.go +++ b/pkg/cmd/lint.go @@ -60,7 +60,7 @@ func newLintCmd(out io.Writer) *cobra.Command { if kubeVersion != "" { parsedKubeVersion, err := chartutil.ParseKubeVersion(kubeVersion) if err != nil { - return fmt.Errorf("invalid kube version '%s': %s", kubeVersion, err) + return fmt.Errorf("invalid kube version '%s': %w", kubeVersion, err) } client.KubeVersion = parsedKubeVersion } diff --git a/pkg/cmd/load_plugins.go b/pkg/cmd/load_plugins.go index 5c7f618eb..347ce6c21 100644 --- a/pkg/cmd/load_plugins.go +++ b/pkg/cmd/load_plugins.go @@ -17,6 +17,7 @@ package cmd import ( "bytes" + "errors" "fmt" "io" "log" @@ -134,7 +135,8 @@ func callPluginExecutable(pluginName string, main string, argv []string, out io. prog.Stdout = out prog.Stderr = os.Stderr if err := prog.Run(); err != nil { - if eerr, ok := err.(*exec.ExitError); ok { + var eerr *exec.ExitError + if errors.As(err, &eerr) { os.Stderr.Write(eerr.Stderr) status := eerr.Sys().(syscall.WaitStatus) return PluginError{ diff --git a/pkg/cmd/plugin.go b/pkg/cmd/plugin.go index a2bb838df..d3e39e4b1 100644 --- a/pkg/cmd/plugin.go +++ b/pkg/cmd/plugin.go @@ -16,6 +16,7 @@ limitations under the License. package cmd import ( + "errors" "fmt" "io" "log/slog" @@ -71,7 +72,8 @@ func runHook(p *plugin.Plugin, event string) error { prog.Stdout, prog.Stderr = os.Stdout, os.Stderr if err := prog.Run(); err != nil { - if eerr, ok := err.(*exec.ExitError); ok { + var eerr *exec.ExitError + if errors.As(err, &eerr) { os.Stderr.Write(eerr.Stderr) return fmt.Errorf("plugin %s hook for %q exited with error", event, p.Metadata.Name) } diff --git a/pkg/cmd/plugin_test.go b/pkg/cmd/plugin_test.go index 74f7a276a..2e69c7d91 100644 --- a/pkg/cmd/plugin_test.go +++ b/pkg/cmd/plugin_test.go @@ -25,6 +25,7 @@ import ( "github.com/spf13/cobra" "github.com/spf13/pflag" + "github.com/stretchr/testify/require" release "helm.sh/helm/v4/pkg/release/v1" ) @@ -142,10 +143,8 @@ func TestLoadPlugins(t *testing.T) { if runtime.GOOS != "windows" { if err := pp.RunE(pp, tt.args); err != nil { if tt.code > 0 { - perr, ok := err.(PluginError) - if !ok { - t.Errorf("Expected %s to return pluginError: got %v(%T)", tt.use, err, err) - } + var perr PluginError + require.ErrorAs(t, err, &perr, "Expected %s to return pluginError: got %v(%T)", tt.use, err, err) if perr.Code != tt.code { t.Errorf("Expected %s to return %d: got %d", tt.use, tt.code, perr.Code) } @@ -217,10 +216,8 @@ func TestLoadPluginsWithSpace(t *testing.T) { if runtime.GOOS != "windows" { if err := pp.RunE(pp, tt.args); err != nil { if tt.code > 0 { - perr, ok := err.(PluginError) - if !ok { - t.Errorf("Expected %s to return pluginError: got %v(%T)", tt.use, err, err) - } + var perr PluginError + require.ErrorAs(t, err, &perr, "Expected %s to return pluginError: got %v(%T)", tt.use, err, err) if perr.Code != tt.code { t.Errorf("Expected %s to return %d: got %d", tt.use, tt.code, perr.Code) } diff --git a/pkg/cmd/plugin_uninstall.go b/pkg/cmd/plugin_uninstall.go index ec73ad6df..7c48f81f1 100644 --- a/pkg/cmd/plugin_uninstall.go +++ b/pkg/cmd/plugin_uninstall.go @@ -69,7 +69,7 @@ func (o *pluginUninstallOptions) run(out io.Writer) error { for _, name := range o.names { if found := findPlugin(plugins, name); found != nil { if err := uninstallPlugin(found); err != nil { - errorPlugins = append(errorPlugins, fmt.Errorf("failed to uninstall plugin %s, got error (%v)", name, err)) + errorPlugins = append(errorPlugins, fmt.Errorf("failed to uninstall plugin %s, got error (%w)", name, err)) } else { fmt.Fprintf(out, "Uninstalled plugin: %s\n", name) } diff --git a/pkg/cmd/plugin_update.go b/pkg/cmd/plugin_update.go index 59d884877..c5aaa1fad 100644 --- a/pkg/cmd/plugin_update.go +++ b/pkg/cmd/plugin_update.go @@ -72,7 +72,7 @@ func (o *pluginUpdateOptions) run(out io.Writer) error { for _, name := range o.names { if found := findPlugin(plugins, name); found != nil { if err := updatePlugin(found); err != nil { - errorPlugins = append(errorPlugins, fmt.Errorf("failed to update plugin %s, got error (%v)", name, err)) + errorPlugins = append(errorPlugins, fmt.Errorf("failed to update plugin %s, got error (%w)", name, err)) } else { fmt.Fprintf(out, "Updated plugin: %s\n", name) } diff --git a/pkg/cmd/rollback.go b/pkg/cmd/rollback.go index 4b7f3016d..6dea544b9 100644 --- a/pkg/cmd/rollback.go +++ b/pkg/cmd/rollback.go @@ -61,7 +61,7 @@ func newRollbackCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { if len(args) > 1 { ver, err := strconv.Atoi(args[1]) if err != nil { - return fmt.Errorf("could not convert revision to a number: %v", err) + return fmt.Errorf("could not convert revision to a number: %w", err) } client.Version = ver } diff --git a/pkg/cmd/search_hub.go b/pkg/cmd/search_hub.go index cfeeec59b..e96ac6cf7 100644 --- a/pkg/cmd/search_hub.go +++ b/pkg/cmd/search_hub.go @@ -141,7 +141,7 @@ func (h *hubSearchWriter) WriteTable(out io.Writer) error { _, err := out.Write([]byte("No results found\n")) if err != nil { - return fmt.Errorf("unable to write results: %s", err) + return fmt.Errorf("unable to write results: %w", err) } return nil } diff --git a/pkg/cmd/search_repo.go b/pkg/cmd/search_repo.go index dffa0d1c4..f9cd18f13 100644 --- a/pkg/cmd/search_repo.go +++ b/pkg/cmd/search_repo.go @@ -221,7 +221,7 @@ func (r *repoSearchWriter) WriteTable(out io.Writer) error { _, err := out.Write([]byte("No results found\n")) if err != nil { - return fmt.Errorf("unable to write results: %s", err) + return fmt.Errorf("unable to write results: %w", err) } return nil } diff --git a/pkg/cmd/template.go b/pkg/cmd/template.go index ac20a45b3..0183e305a 100644 --- a/pkg/cmd/template.go +++ b/pkg/cmd/template.go @@ -71,7 +71,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { if kubeVersion != "" { parsedKubeVersion, err := chartutil.ParseKubeVersion(kubeVersion) if err != nil { - return fmt.Errorf("invalid kube version '%s': %s", kubeVersion, err) + return fmt.Errorf("invalid kube version '%s': %w", kubeVersion, err) } client.KubeVersion = parsedKubeVersion } diff --git a/pkg/cmd/upgrade.go b/pkg/cmd/upgrade.go index c3288286b..0428fdbed 100644 --- a/pkg/cmd/upgrade.go +++ b/pkg/cmd/upgrade.go @@ -18,6 +18,7 @@ package cmd import ( "context" + "errors" "fmt" "io" "log" @@ -122,7 +123,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { histClient := action.NewHistory(cfg) histClient.Max = 1 versions, err := histClient.Run(args[0]) - if err == driver.ErrReleaseNotFound || isReleaseUninstalled(versions) { + if errors.Is(err, driver.ErrReleaseNotFound) || isReleaseUninstalled(versions) { // Only print this to stdout for table output if outfmt == output.Table { fmt.Fprintf(out, "Release %q does not exist. Installing it now.\n", args[0]) diff --git a/pkg/downloader/chart_downloader.go b/pkg/downloader/chart_downloader.go index 529fd788e..c5f26af48 100644 --- a/pkg/downloader/chart_downloader.go +++ b/pkg/downloader/chart_downloader.go @@ -187,7 +187,7 @@ func (c *ChartDownloader) ResolveChartVersion(ref, version string) (*url.URL, er if err != nil { // If there is no special config, return the default HTTP client and // swallow the error. - if err == ErrNoOwnerRepo { + if errors.Is(err, ErrNoOwnerRepo) { // Make sure to add the ref URL as the URL for the getter c.Options = append(c.Options, getter.WithURL(ref)) return u, nil diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index a2e09eae5..936d90aa5 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -20,6 +20,8 @@ import ( "path/filepath" "testing" + "github.com/stretchr/testify/require" + "helm.sh/helm/v4/internal/test/ensure" "helm.sh/helm/v4/pkg/cli" "helm.sh/helm/v4/pkg/getter" @@ -362,7 +364,6 @@ func TestScanReposForURL(t *testing.T) { // A lookup failure should produce an ErrNoOwnerRepo u = "https://no.such.repo/foo/bar-1.23.4.tgz" - if _, err = c.scanReposForURL(u, rf); err != ErrNoOwnerRepo { - t.Fatalf("expected ErrNoOwnerRepo, got %v", err) - } + _, err = c.scanReposForURL(u, rf) + require.ErrorIs(t, err, ErrNoOwnerRepo, "expected ErrNoOwnerRepo, got %v", err) } diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index b43165975..61eb767d1 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -258,7 +258,7 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { return err } } else { - return fmt.Errorf("unable to retrieve file info for '%s': %v", destPath, err) + return fmt.Errorf("unable to retrieve file info for '%s': %w", destPath, err) } // Prepare tmpPath @@ -278,17 +278,17 @@ func (m *Manager) downloadAll(deps []*chart.Dependency) error { chartPath := filepath.Join(destPath, dep.Name) ch, err := loader.LoadDir(chartPath) if err != nil { - return fmt.Errorf("unable to load chart '%s': %v", chartPath, err) + return fmt.Errorf("unable to load chart '%s': %w", chartPath, err) } constraint, err := semver.NewConstraint(dep.Version) if err != nil { - return fmt.Errorf("dependency %s has an invalid version/constraint format: %s", dep.Name, err) + return fmt.Errorf("dependency %s has an invalid version/constraint format: %w", dep.Name, err) } v, err := semver.NewVersion(ch.Metadata.Version) if err != nil { - return fmt.Errorf("invalid version %s for dependency %s: %s", dep.Version, dep.Name, err) + return fmt.Errorf("invalid version %s for dependency %s: %w", dep.Version, dep.Name, err) } if !constraint.Check(v) { diff --git a/pkg/engine/engine.go b/pkg/engine/engine.go index 6e47a0e39..9326471e7 100644 --- a/pkg/engine/engine.go +++ b/pkg/engine/engine.go @@ -129,9 +129,11 @@ type renderable struct { basePath string } -const warnStartDelim = "HELM_ERR_START" -const warnEndDelim = "HELM_ERR_END" -const recursionMaxNums = 1000 +const ( + warnStartDelim = "HELM_ERR_START" + warnEndDelim = "HELM_ERR_END" + recursionMaxNums = 1000 +) var warnRegex = regexp.MustCompile(warnStartDelim + `((?s).*)` + warnEndDelim) @@ -331,7 +333,7 @@ func cleanupParseError(filename string, err error) error { tokens := strings.Split(err.Error(), ": ") if len(tokens) == 1 { // This might happen if a non-templating error occurs - return fmt.Errorf("parse error in (%s): %s", filename, err) + return fmt.Errorf("parse error in (%s): %w", filename, err) } // The first token is "template" // The second token is either "filename:lineno" or "filename:lineNo:columnNo" @@ -368,14 +370,15 @@ func reformatExecErrorMsg(filename string, err error) error { // If the regex's can parse out details from that error message such as the line number, template it failed on, // and error description, then it will construct a new error that displays these details in a structured way. // If there are issues with parsing the error message, the err passed into the function should return instead. - if _, isExecError := err.(template.ExecError); !isExecError { + var execErr template.ExecError + if !errors.As(err, &execErr) { return err } tokens := strings.SplitN(err.Error(), ": ", 3) if len(tokens) != 3 { // This might happen if a non-templating error occurs - return fmt.Errorf("execution error in (%s): %s", filename, err) + return fmt.Errorf("execution error in (%s): %w", filename, err) } // The first token is "template" diff --git a/pkg/engine/engine_test.go b/pkg/engine/engine_test.go index f4228fbd7..10ed8e4f1 100644 --- a/pkg/engine/engine_test.go +++ b/pkg/engine/engine_test.go @@ -25,7 +25,6 @@ import ( "text/template" "github.com/stretchr/testify/assert" - "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/runtime/schema" @@ -646,7 +645,6 @@ func TestRenderDependency(t *testing.T) { if out["outerchart/templates/outer"] != expect { t.Errorf("Expected %q, got %q", expect, out["outer"]) } - } func TestRenderNestedValues(t *testing.T) { @@ -800,7 +798,6 @@ func TestRenderBuiltinValues(t *testing.T) { t.Errorf("Expected %q, got %q", expect, out[file]) } } - } func TestAlterFuncMap_include(t *testing.T) { @@ -993,7 +990,6 @@ func TestAlterFuncMap_tplinclude(t *testing.T) { if got := out["TplFunction/templates/base"]; got != expect { t.Errorf("Expected %q, got %q (%v)", expect, got, out) } - } func TestRenderRecursionLimit(t *testing.T) { @@ -1048,7 +1044,6 @@ func TestRenderRecursionLimit(t *testing.T) { if got := out["overlook/templates/quote"]; got != expect { t.Errorf("Expected %q, got %q (%v)", expect, got, out) } - } func TestRenderLoadTemplateForTplFromFile(t *testing.T) { diff --git a/pkg/getter/plugingetter.go b/pkg/getter/plugingetter.go index 3b8185543..6bfabb571 100644 --- a/pkg/getter/plugingetter.go +++ b/pkg/getter/plugingetter.go @@ -17,6 +17,7 @@ package getter import ( "bytes" + "errors" "fmt" "os" "os/exec" @@ -82,7 +83,8 @@ func (p *pluginGetter) Get(href string, options ...Option) (*bytes.Buffer, error prog.Stdout = buf prog.Stderr = os.Stderr if err := prog.Run(); err != nil { - if eerr, ok := err.(*exec.ExitError); ok { + var eerr *exec.ExitError + if errors.As(err, &eerr) { os.Stderr.Write(eerr.Stderr) return nil, fmt.Errorf("plugin %q exited with error", p.command) } diff --git a/pkg/kube/client.go b/pkg/kube/client.go index 78ed4e088..e22414a0a 100644 --- a/pkg/kube/client.go +++ b/pkg/kube/client.go @@ -180,7 +180,7 @@ func (c *Client) getKubeClient() (kubernetes.Interface, error) { // IsReachable tests connectivity to the cluster. func (c *Client) IsReachable() error { client, err := c.getKubeClient() - if err == genericclioptions.ErrEmptyConfig { + if errors.Is(err, genericclioptions.ErrEmptyConfig) { // re-replace kubernetes ErrEmptyConfig error with a friendly error // moar workarounds for Kubernetes API breaking. return errors.New("kubernetes cluster unreachable") @@ -720,7 +720,7 @@ func updateResource(_ *Client, target *resource.Info, currentObj runtime.Object, func (c *Client) GetPodList(namespace string, listOptions metav1.ListOptions) (*v1.PodList, error) { podList, err := c.kubeClient.CoreV1().Pods(namespace).List(context.Background(), listOptions) if err != nil { - return nil, fmt.Errorf("failed to get pod list with options: %+v with error: %v", listOptions, err) + return nil, fmt.Errorf("failed to get pod list with options: %+v with error: %w", listOptions, err) } return podList, nil } diff --git a/pkg/kube/wait.go b/pkg/kube/wait.go index 8a3bacdcc..3aec9ffbc 100644 --- a/pkg/kube/wait.go +++ b/pkg/kube/wait.go @@ -103,7 +103,8 @@ func (hw *legacyWaiter) isRetryableError(err error, resource *resource.Info) boo return false } slog.Debug("error received when checking resource status", "resource", resource.Name, slog.Any("error", err)) - if ev, ok := err.(*apierrors.StatusError); ok { + var ev *apierrors.StatusError + if errors.As(err, &ev) { statusCode := ev.Status().Code retryable := hw.isRetryableHTTPStatusCode(statusCode) slog.Debug("status code received", "resource", resource.Name, "statusCode", statusCode, "retryable", retryable) diff --git a/pkg/lint/rules/chartfile.go b/pkg/lint/rules/chartfile.go index 724c3f2ea..6dbc715bb 100644 --- a/pkg/lint/rules/chartfile.go +++ b/pkg/lint/rules/chartfile.go @@ -152,7 +152,7 @@ func validateChartVersion(cf *chart.Metadata) error { valid, msg := c.Validate(version) if !valid && len(msg) > 0 { - return fmt.Errorf("version %v", msg[0]) + return fmt.Errorf("version %w", msg[0]) } return nil diff --git a/pkg/lint/rules/crds.go b/pkg/lint/rules/crds.go index 1b8a73139..d62686deb 100644 --- a/pkg/lint/rules/crds.go +++ b/pkg/lint/rules/crds.go @@ -70,7 +70,7 @@ func Crds(linter *support.Linter) { var yamlStruct *k8sYamlStruct err := decoder.Decode(&yamlStruct) - if err == io.EOF { + if errors.Is(err, io.EOF) { break } diff --git a/pkg/lint/rules/deprecations_test.go b/pkg/lint/rules/deprecations_test.go index 6add843ce..34fdc450e 100644 --- a/pkg/lint/rules/deprecations_test.go +++ b/pkg/lint/rules/deprecations_test.go @@ -16,7 +16,11 @@ limitations under the License. package rules // import "helm.sh/helm/v4/pkg/lint/rules" -import "testing" +import ( + "testing" + + "github.com/stretchr/testify/require" +) func TestValidateNoDeprecations(t *testing.T) { deprecated := &k8sYamlStruct{ @@ -27,7 +31,8 @@ func TestValidateNoDeprecations(t *testing.T) { if err == nil { t.Fatal("Expected deprecated extension to be flagged") } - depErr := err.(deprecatedAPIError) + var depErr deprecatedAPIError + require.ErrorAs(t, err, &depErr) if depErr.Message == "" { t.Fatalf("Expected error message to be non-blank: %v", err) } diff --git a/pkg/lint/rules/template.go b/pkg/lint/rules/template.go index b36153ec6..b6a8c07a0 100644 --- a/pkg/lint/rules/template.go +++ b/pkg/lint/rules/template.go @@ -148,7 +148,7 @@ func TemplatesWithSkipSchemaValidation(linter *support.Linter, values map[string var yamlStruct *k8sYamlStruct err := decoder.Decode(&yamlStruct) - if err == io.EOF { + if errors.Is(err, io.EOF) { break } diff --git a/pkg/lint/rules/template_test.go b/pkg/lint/rules/template_test.go index 787bd6e4b..247c344b0 100644 --- a/pkg/lint/rules/template_test.go +++ b/pkg/lint/rules/template_test.go @@ -23,6 +23,8 @@ import ( "strings" "testing" + "github.com/stretchr/testify/require" + chart "helm.sh/helm/v4/pkg/chart/v2" chartutil "helm.sh/helm/v4/pkg/chart/v2/util" "helm.sh/helm/v4/pkg/lint/support" @@ -215,7 +217,8 @@ func TestDeprecatedAPIFails(t *testing.T) { t.Fatalf("Expected 1 lint error, got %d", l) } - err := linter.Messages[0].Err.(deprecatedAPIError) + var err deprecatedAPIError + require.ErrorAs(t, linter.Messages[0].Err, &err) if err.Deprecated != "apps/v1beta1 Deployment" { t.Errorf("Surprised to learn that %q is deprecated", err.Deprecated) } diff --git a/pkg/plugin/installer/http_installer.go b/pkg/plugin/installer/http_installer.go index 3bcf71208..c2c04ce11 100644 --- a/pkg/plugin/installer/http_installer.go +++ b/pkg/plugin/installer/http_installer.go @@ -234,7 +234,7 @@ func (g *TarGzExtractor) Extract(buffer *bytes.Buffer, targetDir string) error { tarReader := tar.NewReader(uncompressedStream) for { header, err := tarReader.Next() - if err == io.EOF { + if errors.Is(err, io.EOF) { break } if err != nil { diff --git a/pkg/plugin/installer/local_installer_test.go b/pkg/plugin/installer/local_installer_test.go index 9effcd2c4..d9c33958d 100644 --- a/pkg/plugin/installer/local_installer_test.go +++ b/pkg/plugin/installer/local_installer_test.go @@ -20,6 +20,8 @@ import ( "path/filepath" "testing" + "github.com/stretchr/testify/assert" + "helm.sh/helm/v4/internal/test/ensure" "helm.sh/helm/v4/pkg/helmpath" ) @@ -61,7 +63,5 @@ func TestLocalInstallerNotAFolder(t *testing.T) { if err == nil { t.Fatal("expected error") } - if err != ErrPluginNotAFolder { - t.Fatalf("expected error to equal: %q", err) - } + assert.ErrorIs(t, err, ErrPluginNotAFolder, "expected error to equal: %q", err) } diff --git a/pkg/provenance/sign_test.go b/pkg/provenance/sign_test.go index 9a60fd19c..d419e6c95 100644 --- a/pkg/provenance/sign_test.go +++ b/pkg/provenance/sign_test.go @@ -17,6 +17,7 @@ package provenance import ( "crypto" + "errors" "fmt" "io" "os" @@ -323,8 +324,9 @@ func TestVerify(t *testing.T) { t.Errorf("Expected %s to fail.", testTamperedSigBlock) } - switch err.(type) { - case pgperrors.SignatureError: + var errCase0 pgperrors.SignatureError + switch { + case errors.As(err, &errCase0): t.Logf("Tampered sig block error: %s (%T)", err, err) default: t.Errorf("Expected invalid signature error, got %q (%T)", err, err) diff --git a/pkg/registry/transport.go b/pkg/registry/transport.go index a82229e2f..2e024f612 100644 --- a/pkg/registry/transport.go +++ b/pkg/registry/transport.go @@ -18,6 +18,7 @@ package registry import ( "bytes" + "errors" "fmt" "io" "log/slog" @@ -137,7 +138,7 @@ func logResponseBody(resp *http.Response) string { Closer: body, } // read the body up to limit+1 to check if the body exceeds the limit - if _, err := io.CopyN(buf, body, payloadSizeLimit+1); err != nil && err != io.EOF { + if _, err := io.CopyN(buf, body, payloadSizeLimit+1); err != nil && !errors.Is(err, io.EOF) { return fmt.Sprintf(" Error reading response body: %v", err) } diff --git a/pkg/registry/util.go b/pkg/registry/util.go index b31ab63fe..d4b8abc49 100644 --- a/pkg/registry/util.go +++ b/pkg/registry/util.go @@ -102,7 +102,7 @@ func NewRegistryClientWithTLS(out io.Writer, certFile, keyFile, caFile string, i tlsutil.WithCAFile(caFile), ) if err != nil { - return nil, fmt.Errorf("can't create TLS config for client: %s", err) + return nil, fmt.Errorf("can't create TLS config for client: %w", err) } // Create a new registry client registryClient, err := NewClient( diff --git a/pkg/repo/chartrepo_test.go b/pkg/repo/chartrepo_test.go index 05e034dd8..30c1bced5 100644 --- a/pkg/repo/chartrepo_test.go +++ b/pkg/repo/chartrepo_test.go @@ -18,7 +18,6 @@ package repo import ( "bytes" - "errors" "net/http" "net/http/httptest" "os" @@ -27,6 +26,7 @@ import ( "testing" "time" + "github.com/stretchr/testify/require" "sigs.k8s.io/yaml" "helm.sh/helm/v4/pkg/cli" @@ -203,9 +203,7 @@ func TestErrorFindChartInRepoURL(t *testing.T) { } else if err.Error() != `chart "nginx1" not found in `+srv.URL+` repository` { t.Errorf("Expected error for chart not found, but got a different error (%v)", err) } - if !errors.Is(err, ChartNotFoundError{}) { - t.Errorf("error is not of correct error type structure") - } + require.ErrorIsf(t, err, ChartNotFoundError{}, "error is not of correct error type structure") if _, err = FindChartInRepoURL(srv.URL, "nginx1", g, WithChartVersion("0.1.0")); err == nil { t.Errorf("Expected error for chart not found, but did not get any errors") diff --git a/pkg/repo/index.go b/pkg/repo/index.go index c26d7581c..1aba10218 100644 --- a/pkg/repo/index.go +++ b/pkg/repo/index.go @@ -401,8 +401,8 @@ func jsonOrYamlUnmarshal(b []byte, i interface{}) error { // And repository indexes may be generated by older/non-compliant software, which doesn't // conform to all validations. func ignoreSkippableChartValidationError(err error) error { - verr, ok := err.(chart.ValidationError) - if !ok { + var verr chart.ValidationError + if !errors.As(err, &verr) { return err } diff --git a/pkg/repo/index_test.go b/pkg/repo/index_test.go index d40719b12..d75dcb5d4 100644 --- a/pkg/repo/index_test.go +++ b/pkg/repo/index_test.go @@ -28,6 +28,8 @@ import ( "strings" "testing" + "github.com/stretchr/testify/require" + chart "helm.sh/helm/v4/pkg/chart/v2" "helm.sh/helm/v4/pkg/cli" "helm.sh/helm/v4/pkg/getter" @@ -639,10 +641,7 @@ func TestIgnoreSkippableChartValidationError(t *testing.T) { return } - if tc.Input != result { - t.Error("expected the result equal to input") - } - + require.ErrorIs(t, tc.Input, result, "expected the result equal to input") }) } } diff --git a/pkg/storage/driver/cfgmaps_test.go b/pkg/storage/driver/cfgmaps_test.go index a563eb7d9..40d990def 100644 --- a/pkg/storage/driver/cfgmaps_test.go +++ b/pkg/storage/driver/cfgmaps_test.go @@ -16,10 +16,11 @@ package driver import ( "encoding/base64" "encoding/json" - "errors" "reflect" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" rspb "helm.sh/helm/v4/pkg/release/v1" @@ -160,9 +161,7 @@ func TestConfigMapQuery(t *testing.T) { } _, err = cfgmaps.Query(map[string]string{"name": "notExist"}) - if err != ErrReleaseNotFound { - t.Errorf("Expected {%v}, got {%v}", ErrReleaseNotFound, err) - } + assert.ErrorIsf(t, err, ErrReleaseNotFound, "Expected {%v}, got {%v}", ErrReleaseNotFound, err) } func TestConfigMapCreate(t *testing.T) { @@ -231,9 +230,7 @@ func TestConfigMapDelete(t *testing.T) { // perform the delete on a non-existent release _, err := cfgmaps.Delete("nonexistent") - if err != ErrReleaseNotFound { - t.Fatalf("Expected ErrReleaseNotFound: got {%v}", err) - } + require.ErrorIsf(t, err, ErrReleaseNotFound, "Expected ErrReleaseNotFound: got {%v}", err) // perform the delete rls, err := cfgmaps.Delete(key) @@ -244,7 +241,5 @@ func TestConfigMapDelete(t *testing.T) { t.Errorf("Expected {%v}, got {%v}", rel, rls) } _, err = cfgmaps.Get(key) - if !errors.Is(err, ErrReleaseNotFound) { - t.Errorf("Expected {%v}, got {%v}", ErrReleaseNotFound, err) - } + require.ErrorIsf(t, err, ErrReleaseNotFound, "Expected {%v}, got {%v}", ErrReleaseNotFound, err) } diff --git a/pkg/storage/driver/secrets_test.go b/pkg/storage/driver/secrets_test.go index 9e45bae67..f9cab62bb 100644 --- a/pkg/storage/driver/secrets_test.go +++ b/pkg/storage/driver/secrets_test.go @@ -16,10 +16,11 @@ package driver import ( "encoding/base64" "encoding/json" - "errors" "reflect" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" v1 "k8s.io/api/core/v1" rspb "helm.sh/helm/v4/pkg/release/v1" @@ -160,9 +161,7 @@ func TestSecretQuery(t *testing.T) { } _, err = secrets.Query(map[string]string{"name": "notExist"}) - if err != ErrReleaseNotFound { - t.Errorf("Expected {%v}, got {%v}", ErrReleaseNotFound, err) - } + assert.ErrorIsf(t, err, ErrReleaseNotFound, "Expected {%v}, got {%v}", ErrReleaseNotFound, err) } func TestSecretCreate(t *testing.T) { @@ -231,9 +230,7 @@ func TestSecretDelete(t *testing.T) { // perform the delete on a non-existing release _, err := secrets.Delete("nonexistent") - if err != ErrReleaseNotFound { - t.Fatalf("Expected ErrReleaseNotFound, got: {%v}", err) - } + require.ErrorIsf(t, err, ErrReleaseNotFound, "Expected ErrReleaseNotFound, got: {%v}", err) // perform the delete rls, err := secrets.Delete(key) @@ -244,7 +241,5 @@ func TestSecretDelete(t *testing.T) { t.Errorf("Expected {%v}, got {%v}", rel, rls) } _, err = secrets.Get(key) - if !errors.Is(err, ErrReleaseNotFound) { - t.Errorf("Expected {%v}, got {%v}", ErrReleaseNotFound, err) - } + require.ErrorIsf(t, err, ErrReleaseNotFound, "Expected {%v}, got {%v}", ErrReleaseNotFound, err) } diff --git a/pkg/storage/driver/sql.go b/pkg/storage/driver/sql.go index 46f6c6b2e..ba2fb75e0 100644 --- a/pkg/storage/driver/sql.go +++ b/pkg/storage/driver/sql.go @@ -460,7 +460,7 @@ func (s *SQL) Create(key string, rls *rspb.Release) error { transaction, err := s.db.Beginx() if err != nil { slog.Debug("failed to start SQL transaction", slog.Any("error", err)) - return fmt.Errorf("error beginning transaction: %v", err) + return fmt.Errorf("error beginning transaction: %w", err) } insertQuery, args, err := s.statementBuilder. @@ -594,7 +594,7 @@ func (s *SQL) Delete(key string) (*rspb.Release, error) { transaction, err := s.db.Beginx() if err != nil { slog.Debug("failed to start SQL transaction", slog.Any("error", err)) - return nil, fmt.Errorf("error beginning transaction: %v", err) + return nil, fmt.Errorf("error beginning transaction: %w", err) } selectQuery, args, err := s.statementBuilder. diff --git a/pkg/storage/driver/sql_test.go b/pkg/storage/driver/sql_test.go index bd2918aad..c5f776886 100644 --- a/pkg/storage/driver/sql_test.go +++ b/pkg/storage/driver/sql_test.go @@ -22,6 +22,7 @@ import ( sqlmock "github.com/DATA-DOG/go-sqlmock" migrate "github.com/rubenv/sql-migrate" + "github.com/stretchr/testify/require" rspb "helm.sh/helm/v4/pkg/release/v1" ) @@ -412,11 +413,7 @@ func TestSqlQuery(t *testing.T) { mockGetReleaseCustomLabels(mock, "", deployedRelease.Namespace, deployedRelease.Labels) _, err := sqlDriver.Query(labelSetUnknown) - if err == nil { - t.Errorf("Expected error {%v}, got nil", ErrReleaseNotFound) - } else if err != ErrReleaseNotFound { - t.Fatalf("failed to query for unknown smug-pigeon release: %v", err) - } + require.ErrorIsf(t, err, ErrReleaseNotFound, "failed to query for unknown smug-pigeon release: %v", err) results, err := sqlDriver.Query(labelSetDeployed) if err != nil { diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index d3025eca3..a70dfd8d7 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -22,6 +22,8 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/require" + rspb "helm.sh/helm/v4/pkg/release/v1" "helm.sh/helm/v4/pkg/storage/driver" ) @@ -329,9 +331,7 @@ func TestMaxHistoryErrorHandling(t *testing.T) { rls2 := ReleaseTestData{Name: name, Version: 2, Status: rspb.StatusSuperseded}.ToRelease() wantErr := errMaxHistoryMockDriverSomethingHappened gotErr := storage.Create(rls2) - if !errors.Is(gotErr, wantErr) { - t.Fatalf("Storing release 'angry-bird' (v2) should return the error %#v, but returned %#v", wantErr, gotErr) - } + require.ErrorIsf(t, gotErr, wantErr, "Storing release 'angry-bird' (v2) should return the error %#v, but returned %#v", wantErr, gotErr) } func TestStorageRemoveLeastRecent(t *testing.T) { diff --git a/pkg/strvals/literal_parser.go b/pkg/strvals/literal_parser.go index d34e5e854..c2a824220 100644 --- a/pkg/strvals/literal_parser.go +++ b/pkg/strvals/literal_parser.go @@ -17,6 +17,7 @@ package strvals import ( "bytes" + "errors" "fmt" "io" "strconv" @@ -66,7 +67,7 @@ func (t *literalParser) parse() error { if err == nil { continue } - if err == io.EOF { + if errors.Is(err, io.EOF) { return nil } return err @@ -105,7 +106,7 @@ func (t *literalParser) key(data map[string]interface{}, nestedNameLevel int) (r case lastRune == '=': // found end of key: swallow the '=' and get the value value, err := t.val() - if err == nil && err != io.EOF { + if err == nil && !errors.Is(err, io.EOF) { return err } set(data, string(key), string(value)) @@ -183,7 +184,7 @@ func (t *literalParser) listItem(list []interface{}, i, nestedNameLevel int) ([] case lastRune == '=': value, err := t.val() - if err != nil && err != io.EOF { + if err != nil && !errors.Is(err, io.EOF) { return list, err } return setIndex(list, i, string(value)) diff --git a/pkg/strvals/parser.go b/pkg/strvals/parser.go index c65e98c84..7fb3cc500 100644 --- a/pkg/strvals/parser.go +++ b/pkg/strvals/parser.go @@ -161,7 +161,7 @@ func (t *parser) parse() error { if err == nil { continue } - if err == io.EOF { + if errors.Is(err, io.EOF) { return nil } return err @@ -190,8 +190,8 @@ func (t *parser) key(data map[string]interface{}, nestedNameLevel int) (reterr e return err } return fmt.Errorf("key %q has no value", string(k)) - //set(data, string(k), "") - //return err + // set(data, string(k), "") + // return err case last == '[': // We are in a list index context, so we need to set an index. i, err := t.keyIndex() @@ -237,19 +237,19 @@ func (t *parser) key(data map[string]interface{}, nestedNameLevel int) (reterr e _, err = t.emptyVal() return err } - //End of key. Consume =, Get value. + // End of key. Consume =, Get value. // FIXME: Get value list first vl, e := t.valList() - switch e { - case nil: + switch { + case e == nil: set(data, string(k), vl) return nil - case io.EOF: + case errors.Is(e, io.EOF): set(data, string(k), "") return e - case ErrNotList: + case errors.Is(e, ErrNotList): rs, e := t.val() - if e != nil && e != io.EOF { + if e != nil && !errors.Is(e, io.EOF) { return e } v, e := t.reader(rs) @@ -330,7 +330,6 @@ func (t *parser) keyIndex() (int, error) { } // v should be the index return strconv.Atoi(string(v)) - } func (t *parser) listItem(list []interface{}, i, nestedNameLevel int) ([]interface{}, error) { @@ -373,14 +372,14 @@ func (t *parser) listItem(list []interface{}, i, nestedNameLevel int) ([]interfa return list, err } vl, e := t.valList() - switch e { - case nil: + switch { + case e == nil: return setIndex(list, i, vl) - case io.EOF: + case errors.Is(e, io.EOF): return setIndex(list, i, "") - case ErrNotList: + case errors.Is(e, ErrNotList): rs, e := t.val() - if e != nil && e != io.EOF { + if e != nil && !errors.Is(e, io.EOF) { return list, e } v, e := t.reader(rs) @@ -479,7 +478,7 @@ func (t *parser) valList() ([]interface{}, error) { for { switch rs, last, err := runesUntil(t.sc, stop); { case err != nil: - if err == io.EOF { + if errors.Is(err, io.EOF) { err = errors.New("list must terminate with '}'") } return list, err