From 80352b3f7af556b7acbfb34c9289d67b6df87706 Mon Sep 17 00:00:00 2001 From: TristanHoladay <40547442+TristanHoladay@users.noreply.github.com> Date: Mon, 20 May 2024 20:11:48 -0600 Subject: [PATCH 1/6] fix: automated dead code detection. --- .golangci.yaml | 9 +++++++++ .pre-commit-config.yaml | 3 ++- src/pkg/bundle/deploy.go | 3 --- src/pkg/bundle/remote.go | 3 --- src/pkg/bundle/remove.go | 3 --- src/pkg/bundle/tarball.go | 3 --- src/pkg/bundle/tui/deploy/handlers.go | 2 +- src/pkg/bundler/fetcher/local.go | 2 -- src/pkg/bundler/localbundle.go | 5 ++++- src/pkg/utils/oci.go | 2 -- 10 files changed, 16 insertions(+), 19 deletions(-) create mode 100644 .golangci.yaml diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 00000000..080a4fe5 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,9 @@ +linters-settings: + govet: + enable: + - nilness + + unused: + # Mark all local variables as used. + # default: true + local-variables-are-used: false diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b01a0565..669f4639 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -24,8 +24,9 @@ repos: - repo: https://github.com/golangci/golangci-lint rev: v1.55.2 hooks: - - id: golangci-lint + - id: golangci-lint-full args: [--timeout=5m] + linters: - repo: local hooks: - id: check-docs-and-schema diff --git a/src/pkg/bundle/deploy.go b/src/pkg/bundle/deploy.go index 3f767ebb..b584e94c 100644 --- a/src/pkg/bundle/deploy.go +++ b/src/pkg/bundle/deploy.go @@ -134,9 +134,6 @@ func deployPackages(packages []types.Package, resume bool, b *Bundle) error { } pkgClient := packager.NewOrDie(&pkgCfg, packager.WithSource(source), packager.WithTemp(opts.PackageSource)) - if err != nil { - return err - } deploy.Program.Send(fmt.Sprintf("newPackage:%s:%d", pkg.Name, i)) diff --git a/src/pkg/bundle/remote.go b/src/pkg/bundle/remote.go index e2ceb927..4fb59f36 100644 --- a/src/pkg/bundle/remote.go +++ b/src/pkg/bundle/remote.go @@ -177,9 +177,6 @@ func (op *ociProvider) LoadBundle(opts types.BundlePullOptions, _ int) (*types.U // grab sha of zarf image manifest and pull it down sha := strings.Split(pkg.Ref, "@sha256:")[1] // this is where we use the SHA appended to the Zarf pkg inside the bundle manifestDesc := rootManifest.Locate(sha) - if err != nil { - return nil, nil, err - } manifestBytes, err := op.FetchLayer(ctx, manifestDesc) if err != nil { return nil, nil, err diff --git a/src/pkg/bundle/remove.go b/src/pkg/bundle/remove.go index 21a152b2..3af1c30d 100644 --- a/src/pkg/bundle/remove.go +++ b/src/pkg/bundle/remove.go @@ -99,9 +99,6 @@ func removePackages(packagesToRemove []types.Package, b *Bundle) error { } pkgClient := packager.NewOrDie(&pkgCfg, packager.WithSource(source), packager.WithTemp(pkgTmp)) - if err != nil { - return err - } defer pkgClient.ClearTempPaths() if err := pkgClient.Remove(); err != nil { diff --git a/src/pkg/bundle/tarball.go b/src/pkg/bundle/tarball.go index 990a7ce3..6553b4d9 100644 --- a/src/pkg/bundle/tarball.go +++ b/src/pkg/bundle/tarball.go @@ -279,9 +279,6 @@ func (tp *tarballBundleProvider) PublishBundle(bundle types.UDSBundle, remote *o // copy bundle copyOpts := utils.CreateCopyOpts(layersToPush, config.CommonOptions.OCIConcurrency) - if err != nil { - return err - } progressBar := message.NewProgressBar(estimatedBytes, fmt.Sprintf("Publishing %s:%s", remote.Repo().Reference.Repository, remote.Repo().Reference.Reference)) defer progressBar.Stop() remote.SetProgressWriter(progressBar) diff --git a/src/pkg/bundle/tui/deploy/handlers.go b/src/pkg/bundle/tui/deploy/handlers.go index ae7b06b1..cac18cee 100644 --- a/src/pkg/bundle/tui/deploy/handlers.go +++ b/src/pkg/bundle/tui/deploy/handlers.go @@ -165,7 +165,7 @@ func (m *Model) handleDeployTick() (tea.Model, tea.Cmd) { // handle upgrade scenario by resetting the component progress, otherwise increment it if p.resetProgress { // if upgraded len(deployedPkg.DeployedComponents) will be equal to the number of components in the package - if deployedPkg != nil && len(deployedPkg.DeployedComponents) > 0 { + if len(deployedPkg.DeployedComponents) > 0 { m.packages[i].resetProgress = false } break diff --git a/src/pkg/bundler/fetcher/local.go b/src/pkg/bundler/fetcher/local.go index 2067bc25..71f7c946 100644 --- a/src/pkg/bundler/fetcher/local.go +++ b/src/pkg/bundler/fetcher/local.go @@ -198,8 +198,6 @@ func (f *localFetcher) toBundle(pkg zarfTypes.ZarfPackage, pkgTmp string) ([]oci if exists, err := f.cfg.Store.Exists(ctx, desc); !exists && err == nil { if err := f.cfg.Store.Push(ctx, desc, layer); err != nil { return nil, err - } else if err != nil { - return nil, err } } descs = append(descs, desc) diff --git a/src/pkg/bundler/localbundle.go b/src/pkg/bundler/localbundle.go index dd4fed6f..dfb61519 100644 --- a/src/pkg/bundler/localbundle.go +++ b/src/pkg/bundler/localbundle.go @@ -222,7 +222,10 @@ func writeTarball(bundle *types.UDSBundle, artifactPathMap types.PathMap, output filename := fmt.Sprintf("%s%s-%s-%s.tar.zst", config.BundlePrefix, bundle.Metadata.Name, bundle.Metadata.Architecture, bundle.Metadata.Version) if !helpers.IsDir(outputDir) { - os.MkdirAll(outputDir, 0755) + err := os.MkdirAll(outputDir, 0755) + if err != nil { + return err + } } dst := filepath.Join(outputDir, filename) diff --git a/src/pkg/utils/oci.go b/src/pkg/utils/oci.go index 164a0f8f..53a67b17 100644 --- a/src/pkg/utils/oci.go +++ b/src/pkg/utils/oci.go @@ -85,11 +85,9 @@ func ToOCIRemote(t any, mediaType string, remote *oci.OrasRemote) (*ocispec.Desc func CreateCopyOpts(layersToPull []ocispec.Descriptor, concurrency int) oras.CopyOptions { var copyOpts oras.CopyOptions copyOpts.Concurrency = concurrency - estimatedBytes := int64(0) var shas []string for _, layer := range layersToPull { if len(layer.Digest.String()) > 0 { - estimatedBytes += layer.Size shas = append(shas, layer.Digest.Encoded()) } } From 55d723ab89aed215fc84777acbf4dc3543be9a27 Mon Sep 17 00:00:00 2001 From: TristanHoladay <40547442+TristanHoladay@users.noreply.github.com> Date: Mon, 20 May 2024 20:36:48 -0600 Subject: [PATCH 2/6] remove duplicate .golangci.yaml; remove double imports --- .golangci.yaml | 9 --------- .golangci.yml | 13 +++++++++++++ src/cmd/root.go | 3 +-- src/pkg/sources/tarball.go | 3 +-- 4 files changed, 15 insertions(+), 13 deletions(-) delete mode 100644 .golangci.yaml diff --git a/.golangci.yaml b/.golangci.yaml deleted file mode 100644 index 080a4fe5..00000000 --- a/.golangci.yaml +++ /dev/null @@ -1,9 +0,0 @@ -linters-settings: - govet: - enable: - - nilness - - unused: - # Mark all local variables as used. - # default: true - local-variables-are-used: false diff --git a/.golangci.yml b/.golangci.yml index 5e329d86..fcd41c9d 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -22,3 +22,16 @@ linters: - goimports - gosec - govet + - staticcheck + + +linters-settings: + govet: + enable: + - nilness + + unused: + # Mark all local variables as used. + # default: true + local-variables-are-used: false + diff --git a/src/cmd/root.go b/src/cmd/root.go index 123f0a03..4bfcb0e7 100644 --- a/src/cmd/root.go +++ b/src/cmd/root.go @@ -11,7 +11,6 @@ import ( "github.com/defenseunicorns/uds-cli/src/config" "github.com/defenseunicorns/uds-cli/src/config/lang" "github.com/defenseunicorns/uds-cli/src/types" - "github.com/defenseunicorns/zarf/src/cmd/common" zarfCommon "github.com/defenseunicorns/zarf/src/cmd/common" "github.com/defenseunicorns/zarf/src/pkg/message" "github.com/spf13/cobra" @@ -28,7 +27,7 @@ var rootCmd = &cobra.Command{ Use: "uds COMMAND", PersistentPreRun: func(cmd *cobra.Command, _ []string) { // Skip for vendor-only commands - if common.CheckVendorOnlyFromPath(cmd) { + if zarfCommon.CheckVendorOnlyFromPath(cmd) { return } diff --git a/src/pkg/sources/tarball.go b/src/pkg/sources/tarball.go index 3768050f..b1586980 100644 --- a/src/pkg/sources/tarball.go +++ b/src/pkg/sources/tarball.go @@ -20,7 +20,6 @@ import ( "github.com/defenseunicorns/zarf/src/pkg/packager/filters" "github.com/defenseunicorns/zarf/src/pkg/packager/sources" zarfUtils "github.com/defenseunicorns/zarf/src/pkg/utils" - "github.com/defenseunicorns/zarf/src/types" zarfTypes "github.com/defenseunicorns/zarf/src/types" av4 "github.com/mholt/archiver/v4" ocispec "github.com/opencontainers/image-spec/specs-go/v1" @@ -60,7 +59,7 @@ func (t *TarballBundle) LoadPackage(dst *layout.PackagePaths, filter filters.Com } // if in dev mode and package is a zarf init config, return an empty package - if config.Dev && pkg.Kind == types.ZarfInitConfig { + if config.Dev && pkg.Kind == zarfTypes.ZarfInitConfig { return zarfTypes.ZarfPackage{}, nil, nil } From 75166f402ca1645e2fe7313e1e8a79f84f7c5087 Mon Sep 17 00:00:00 2001 From: TristanHoladay <40547442+TristanHoladay@users.noreply.github.com> Date: Mon, 20 May 2024 20:45:38 -0600 Subject: [PATCH 3/6] add nilerr linter to golangci config; fix return of nil err for ValidateArch() --- .golangci.yml | 3 +-- src/pkg/bundle/remote.go | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index fcd41c9d..0f18203f 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -23,7 +23,7 @@ linters: - gosec - govet - staticcheck - + - nilerr linters-settings: govet: @@ -34,4 +34,3 @@ linters-settings: # Mark all local variables as used. # default: true local-variables-are-used: false - diff --git a/src/pkg/bundle/remote.go b/src/pkg/bundle/remote.go index 4fb59f36..d17bb7b0 100644 --- a/src/pkg/bundle/remote.go +++ b/src/pkg/bundle/remote.go @@ -308,7 +308,7 @@ func ValidateArch(arch string) error { } if c != nil { clusterArchs, err = c.GetArchitectures() - if err == nil { + if err != nil { return err } // check if bundle arch is in clusterArchs From 25dbd6a1a57e6b259cea882b4fc3c2ce67cef5e6 Mon Sep 17 00:00:00 2001 From: TristanHoladay <40547442+TristanHoladay@users.noreply.github.com> Date: Mon, 20 May 2024 21:06:54 -0600 Subject: [PATCH 4/6] enable all govet linter analyzers except 3 --- .golangci.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 0f18203f..543f150b 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -27,8 +27,13 @@ linters: linters-settings: govet: - enable: - - nilness + enable-all: true + disable: + - shadow + - fieldalignment + - unusedwrite + nolintlint: + require-specific: true unused: # Mark all local variables as used. From acbdc564248f2d17ede8f0a3a9dcd3602772e5f2 Mon Sep 17 00:00:00 2001 From: TristanHoladay <40547442+TristanHoladay@users.noreply.github.com> Date: Tue, 21 May 2024 13:07:56 -0600 Subject: [PATCH 5/6] remove unused vars --- src/pkg/sources/remote.go | 2 -- src/pkg/utils/utils.go | 4 +--- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/pkg/sources/remote.go b/src/pkg/sources/remote.go index aad9c576..2b280c03 100644 --- a/src/pkg/sources/remote.go +++ b/src/pkg/sources/remote.go @@ -183,7 +183,6 @@ func (r *RemoteBundle) downloadPkgFromRemoteBundle() ([]ocispec.Descriptor, erro estimatedBytes := int64(0) layersToPull := []ocispec.Descriptor{pkgManifestDesc} layersInBundle := []ocispec.Descriptor{pkgManifestDesc} - numLayersVerified := 0.0 for _, layer := range pkgManifest.Layers { ok, err := r.Remote.Repo().Blobs().Exists(ctx, layer) @@ -191,7 +190,6 @@ func (r *RemoteBundle) downloadPkgFromRemoteBundle() ([]ocispec.Descriptor, erro return nil, err } progressBar.Add(1) - numLayersVerified++ if ok { estimatedBytes += layer.Size layersInBundle = append(layersInBundle, layer) diff --git a/src/pkg/utils/utils.go b/src/pkg/utils/utils.go index 6e4f74c4..483e7eac 100644 --- a/src/pkg/utils/utils.go +++ b/src/pkg/utils/utils.go @@ -66,11 +66,9 @@ func ConfigureLogs(cmd *cobra.Command) error { return err } - logWriter := io.MultiWriter(logFile) - // use Zarf pterm output message.Notef("Saving log file to %s", tmpLogLocation) - logWriter = io.MultiWriter(os.Stderr, logFile) + logWriter := io.MultiWriter(os.Stderr, logFile) pterm.SetDefaultOutput(logWriter) message.Debugf(fmt.Sprintf("Saving log file to %s", tmpLogLocation)) return nil From bfac1f8f43eda0666c8e9eced39b0371b7e2d88f Mon Sep 17 00:00:00 2001 From: TristanHoladay <40547442+TristanHoladay@users.noreply.github.com> Date: Tue, 21 May 2024 13:51:08 -0600 Subject: [PATCH 6/6] remove revive action and add to golanci-lint config --- .github/workflows/scan-lint.yaml | 9 --------- .golangci.yml | 29 +++++++++++++++++++++++++++-- revive.toml | 31 ------------------------------- 3 files changed, 27 insertions(+), 42 deletions(-) delete mode 100644 revive.toml diff --git a/.github/workflows/scan-lint.yaml b/.github/workflows/scan-lint.yaml index c4af665a..709da1d4 100644 --- a/.github/workflows/scan-lint.yaml +++ b/.github/workflows/scan-lint.yaml @@ -24,12 +24,3 @@ jobs: uses: pre-commit/action@f7acafac0271bdd064cdfa1b13f17b4350e565ed # with: extra_args: --all-files --verbose # pre-commit run --all-files --verbose - - - name: Run Revive Action by pulling pre-built image - uses: docker://morphy/revive-action:v2@sha256:087d4e61077087755711ab7e9fae3cc899b7bb07ff8f6a30c3dfb240b1620ae8 - with: - config: revive.toml - # Exclude patterns, separated by semicolons (optional) - exclude: "src/cmd/viper.go;src/config/lang/lang.go" - # Path pattern (default: ./...) - path: "./src/..." diff --git a/.golangci.yml b/.golangci.yml index 543f150b..378a58a9 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -24,6 +24,7 @@ linters: - govet - staticcheck - nilerr + - revive linters-settings: govet: @@ -36,6 +37,30 @@ linters-settings: require-specific: true unused: - # Mark all local variables as used. - # default: true local-variables-are-used: false + + revive: + rules: + - name: blank-imports + - name: context-as-argument + - name: context-keys-type + - name: dot-imports + - name: error-return + - name: error-strings + - name: error-naming + - name: exported + - name: if-return + - name: increment-decrement + - name: var-declaration + - name: package-comments + - name: range + - name: receiver-naming + - name: time-naming + - name: unexported-return + - name: indent-error-flow + - name: errorf + - name: empty-block + - name: superfluous-else + - name: unused-parameter + - name: unreachable-code + - name: redefines-builtin-id diff --git a/revive.toml b/revive.toml deleted file mode 100644 index ebda8f5c..00000000 --- a/revive.toml +++ /dev/null @@ -1,31 +0,0 @@ -ignoreGeneratedHeader = false -severity = "warning" -confidence = 0.8 -errorCode = 0 -warningCode = 0 -formatter = "stylish" - -[rule.blank-imports] -[rule.context-as-argument] -[rule.context-keys-type] -[rule.dot-imports] -[rule.error-return] -[rule.error-strings] -[rule.error-naming] -[rule.exported] -[rule.if-return] -[rule.increment-decrement] -[rule.var-naming] -[rule.var-declaration] -[rule.package-comments] -[rule.range] -[rule.receiver-naming] -[rule.time-naming] -[rule.unexported-return] -[rule.indent-error-flow] -[rule.errorf] -[rule.empty-block] -[rule.superfluous-else] -[rule.unused-parameter] -[rule.unreachable-code] -[rule.redefines-builtin-id]