From 05fdaefa7862b156d549e44b4bb1ed174f501e86 Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Fri, 26 Jul 2024 15:42:06 +0200 Subject: [PATCH] fix: error formatting and comparison and enable errorlint (#2771) Signed-off-by: Philip Laine --- .golangci.yaml | 1 + src/cmd/common/viper.go | 14 ++++++-------- src/extensions/bigbang/bigbang.go | 4 ++-- src/internal/packager/helm/chart.go | 6 +++--- src/internal/packager/helm/post-render.go | 2 +- src/internal/packager/helm/repo.go | 15 +++++++++------ src/pkg/layout/component.go | 6 ------ src/pkg/packager/sources/oci.go | 2 +- src/pkg/packager/sources/split.go | 4 ++-- src/pkg/packager/sources/tarball.go | 2 +- src/pkg/utils/yaml.go | 7 ++++--- 11 files changed, 30 insertions(+), 33 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 9426dd911e..58aaa823f9 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -15,6 +15,7 @@ linters: - nolintlint - testifylint - whitespace + - errorlint linters-settings: govet: enable-all: true diff --git a/src/cmd/common/viper.go b/src/cmd/common/viper.go index bbd0f64c0c..a28e30544e 100644 --- a/src/cmd/common/viper.go +++ b/src/cmd/common/viper.go @@ -5,6 +5,7 @@ package common import ( + "errors" "os" "strings" @@ -166,16 +167,13 @@ func printViperConfigUsed() { if !vInitialized { return } - // Optional, so ignore file not found errors - if vConfigError != nil { - // Config file not found; ignore - if _, ok := vConfigError.(viper.ConfigFileNotFoundError); !ok { - message.WarnErrf(vConfigError, lang.CmdViperErrLoadingConfigFile, vConfigError.Error()) - } - } else { - message.Notef(lang.CmdViperInfoUsingConfigFile, v.ConfigFileUsed()) + var notFoundErr *viper.ConfigFileNotFoundError + if vConfigError != nil && !errors.As(vConfigError, ¬FoundErr) { + message.WarnErrf(vConfigError, lang.CmdViperErrLoadingConfigFile, vConfigError.Error()) + return } + message.Notef(lang.CmdViperInfoUsingConfigFile, v.ConfigFileUsed()) } func setDefaults() { diff --git a/src/extensions/bigbang/bigbang.go b/src/extensions/bigbang/bigbang.go index ebe068f003..7eedf66255 100644 --- a/src/extensions/bigbang/bigbang.go +++ b/src/extensions/bigbang/bigbang.go @@ -50,12 +50,12 @@ func Run(ctx context.Context, YOLO bool, tmpPaths *layout.ComponentPaths, c type validVersionResponse, err := isValidVersion(cfg.Version) if err != nil { - return c, fmt.Errorf("invalid Big Bang version: %s, parsing issue %s", cfg.Version, err) + return c, fmt.Errorf("could not parse the Big Bang version %s: %w", cfg.Version, err) } // Make sure the version is valid. if !validVersionResponse { - return c, fmt.Errorf("invalid Big Bang version: %s, must be at least %s", cfg.Version, bbMinRequiredVersion) + return c, fmt.Errorf("Big Bang version %s must be at least %s", cfg.Version, bbMinRequiredVersion) } // Print the banner for Big Bang. diff --git a/src/internal/packager/helm/chart.go b/src/internal/packager/helm/chart.go index bca8773294..26f81e1a9b 100644 --- a/src/internal/packager/helm/chart.go +++ b/src/internal/packager/helm/chart.go @@ -149,7 +149,7 @@ func (h *Helm) TemplateChart(ctx context.Context) (manifest string, chartValues if h.kubeVersion != "" { parsedKubeVersion, err := chartutil.ParseKubeVersion(h.kubeVersion) if err != nil { - return "", nil, fmt.Errorf("invalid kube version '%s': %s", h.kubeVersion, err) + return "", nil, fmt.Errorf("invalid kube version %s: %w", h.kubeVersion, err) } client.KubeVersion = parsedKubeVersion } @@ -392,13 +392,13 @@ func (h *Helm) migrateDeprecatedAPIs(latestRelease *release.Release) error { // parse to unstructured to have access to more data than just the name rawData := &unstructured.Unstructured{} if err := yaml.Unmarshal([]byte(resource.Content), rawData); err != nil { - return fmt.Errorf("failed to unmarshal manifest: %#v", err) + return fmt.Errorf("failed to unmarshal manifest: %w", err) } rawData, manifestModified, _ := handleDeprecations(rawData, *kubeGitVersion) manifestContent, err := yaml.Marshal(rawData) if err != nil { - return fmt.Errorf("failed to marshal raw manifest after deprecation check: %#v", err) + return fmt.Errorf("failed to marshal raw manifest after deprecation check: %w", err) } // If this is not a bad object, place it back into the manifest diff --git a/src/internal/packager/helm/post-render.go b/src/internal/packager/helm/post-render.go index b115b33ddf..c316375acb 100644 --- a/src/internal/packager/helm/post-render.go +++ b/src/internal/packager/helm/post-render.go @@ -222,7 +222,7 @@ func (r *renderer) editHelmResources(ctx context.Context, resources []releaseuti // parse to unstructured to have access to more data than just the name rawData := &unstructured.Unstructured{} if err := yaml.Unmarshal([]byte(resource.Content), rawData); err != nil { - return fmt.Errorf("failed to unmarshal manifest: %#v", err) + return fmt.Errorf("failed to unmarshal manifest: %w", err) } switch rawData.GetKind() { diff --git a/src/internal/packager/helm/repo.go b/src/internal/packager/helm/repo.go index 9397a7f760..dad0e59aab 100644 --- a/src/internal/packager/helm/repo.go +++ b/src/internal/packager/helm/repo.go @@ -6,6 +6,7 @@ package helm import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -307,18 +308,20 @@ func (h *Helm) buildChartDependencies() error { // Build the deps from the helm chart err = man.Build() - if e, ok := err.(downloader.ErrRepoNotFound); ok { + var notFoundErr *downloader.ErrRepoNotFound + if errors.As(err, ¬FoundErr) { // If we encounter a repo not found error point the user to `zarf tools helm repo add` - message.Warnf("%s. Please add the missing repo(s) via the following:", e.Error()) - for _, repository := range e.Repos { + message.Warnf("%s. Please add the missing repo(s) via the following:", notFoundErr.Error()) + for _, repository := range notFoundErr.Repos { message.ZarfCommand(fmt.Sprintf("tools helm repo add %s", repository)) } - } else if err != nil { - // Warn the user of any issues but don't fail - any actual issues will cause a fail during packaging (e.g. the charts we are building may exist already, we just can't get updates) + return err + } + if err != nil { message.ZarfCommand("tools helm dependency build --verify") message.Warnf("Unable to perform a rebuild of Helm dependencies: %s", err.Error()) + return err } - return nil } diff --git a/src/pkg/layout/component.go b/src/pkg/layout/component.go index f52e6bbe06..ec5e82507b 100644 --- a/src/pkg/layout/component.go +++ b/src/pkg/layout/component.go @@ -38,12 +38,6 @@ type Components struct { // ErrNotLoaded is returned when a path is not loaded. var ErrNotLoaded = fmt.Errorf("not loaded") -// IsNotLoaded checks if an error is ErrNotLoaded. -func IsNotLoaded(err error) bool { - u, ok := err.(*fs.PathError) - return ok && u.Unwrap() == ErrNotLoaded -} - // Archive archives a component. func (c *Components) Archive(component types.ZarfComponent, cleanupTemp bool) (err error) { name := component.Name diff --git a/src/pkg/packager/sources/oci.go b/src/pkg/packager/sources/oci.go index 33e4ba3fad..f22547d520 100644 --- a/src/pkg/packager/sources/oci.go +++ b/src/pkg/packager/sources/oci.go @@ -86,7 +86,7 @@ func (s *OCISource) LoadPackage(ctx context.Context, dst *layout.PackagePaths, f if unarchiveAll { for _, component := range pkg.Components { if err := dst.Components.Unarchive(component); err != nil { - if layout.IsNotLoaded(err) { + if errors.Is(err, layout.ErrNotLoaded) { _, err := dst.Components.Create(component) if err != nil { return pkg, nil, err diff --git a/src/pkg/packager/sources/split.go b/src/pkg/packager/sources/split.go index 473aa0008a..8176b68882 100644 --- a/src/pkg/packager/sources/split.go +++ b/src/pkg/packager/sources/split.go @@ -36,7 +36,7 @@ func (s *SplitTarballSource) Collect(_ context.Context, dir string) (string, err pattern := strings.Replace(s.PackageSource, ".part000", ".part*", 1) fileList, err := filepath.Glob(pattern) if err != nil { - return "", fmt.Errorf("unable to find split tarball files: %s", err) + return "", fmt.Errorf("unable to find split tarball files: %w", err) } // Ensure the files are in order so they are appended in the correct order @@ -46,7 +46,7 @@ func (s *SplitTarballSource) Collect(_ context.Context, dir string) (string, err // Create the new package pkgFile, err := os.Create(reassembled) if err != nil { - return "", fmt.Errorf("unable to create new package file: %s", err) + return "", fmt.Errorf("unable to create new package file: %w", err) } defer pkgFile.Close() diff --git a/src/pkg/packager/sources/tarball.go b/src/pkg/packager/sources/tarball.go index b99253c0a5..9cbbf1b071 100644 --- a/src/pkg/packager/sources/tarball.go +++ b/src/pkg/packager/sources/tarball.go @@ -114,7 +114,7 @@ func (s *TarballSource) LoadPackage(_ context.Context, dst *layout.PackagePaths, if unarchiveAll { for _, component := range pkg.Components { if err := dst.Components.Unarchive(component); err != nil { - if layout.IsNotLoaded(err) { + if errors.Is(err, layout.ErrNotLoaded) { _, err := dst.Components.Create(component) if err != nil { return pkg, nil, err diff --git a/src/pkg/utils/yaml.go b/src/pkg/utils/yaml.go index 5f86202970..f3fdaa53b7 100644 --- a/src/pkg/utils/yaml.go +++ b/src/pkg/utils/yaml.go @@ -8,6 +8,7 @@ package utils import ( "bytes" + "errors" "fmt" "io" "io/fs" @@ -196,7 +197,7 @@ func SplitYAML(yamlData []byte) ([]*unstructured.Unstructured, error) { for _, yml := range ymls { u := &unstructured.Unstructured{} if err := k8syaml.Unmarshal([]byte(yml), u); err != nil { - return objs, fmt.Errorf("failed to unmarshal manifest: %#v", err) + return objs, fmt.Errorf("failed to unmarshal manifest: %w", err) } objs = append(objs, u) } @@ -216,10 +217,10 @@ func SplitYAMLToString(yamlData []byte) ([]string, error) { for { ext := runtime.RawExtension{} if err := d.Decode(&ext); err != nil { - if err == io.EOF { + if errors.Is(err, io.EOF) { break } - return objs, fmt.Errorf("failed to unmarshal manifest: %#v", err) + return objs, fmt.Errorf("failed to unmarshal manifest: %w", err) } ext.Raw = bytes.TrimSpace(ext.Raw) if len(ext.Raw) == 0 || bytes.Equal(ext.Raw, []byte("null")) {