Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add linter (2949) #3053

Merged
merged 61 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from 60 commits
Commits
Show all changes
61 commits
Select commit Hold shift + click to select a range
358dd41
add errcheck linting
mkcp Sep 11, 2024
655dd76
linter ignore some intentionally unhandled errs
mkcp Sep 12, 2024
cce62ce
handle some env var set and unset errors in tests
mkcp Sep 12, 2024
ae05ebb
make some notes in cmd/destroy.go for later
mkcp Sep 12, 2024
992aa3c
direct handle some deferred cleanups to add direct error handling
mkcp Sep 12, 2024
81e54f7
capture errors from cmd/root
mkcp Sep 13, 2024
9b2b2e9
capture errors on src/config/config.go. checking ci
mkcp Sep 13, 2024
a508aa7
fix: ensure we capture and propagate errors in src/internal
mkcp Sep 25, 2024
6f53a29
fix: propagate or log unhandled errors in src/cmd
mkcp Sep 25, 2024
4283faf
fix: check for or intentionally ignore some dangling errors in tests
mkcp Sep 25, 2024
cd5fdc7
fix: use RemoveAll not remove so it doesnt error every time
mkcp Sep 25, 2024
9059544
fix: disabling the err catch to see if it's something i changed
mkcp Sep 25, 2024
4a0d6ef
fix: should fix the directory not empty panic
mkcp Sep 25, 2024
4da92b9
fix: see if CI is flaking with the original tests. probably just need…
mkcp Sep 25, 2024
29c5d08
fix: these tests should clean up properly and not error
mkcp Sep 25, 2024
dfdf78a
fix: actually cleaning up the test with RemovalAll seems to cause a p…
mkcp Sep 25, 2024
5a4241e
fix: reintroduce the ext_in_cluster test error catches
mkcp Sep 25, 2024
ac3db17
fix: missed a spot
mkcp Sep 25, 2024
3b274f7
fix: e2e 24 - can we assert that this error exists or do we have to i…
mkcp Sep 25, 2024
a9d6266
fix: address some fixmes before merge
mkcp Sep 25, 2024
ccabf6c
fix: does this work now?
mkcp Sep 25, 2024
8a56cd5
fix: handle and propagate errors in pkg/utils
mkcp Sep 25, 2024
5addccc
fix: propagate any errors in ColorPrintYAML
mkcp Sep 25, 2024
7c04dff
fix: propagate any errors in src/pkg/message
mkcp Sep 25, 2024
acdb601
fix: propagate any errors in src/pkg/variables
mkcp Sep 25, 2024
803b55e
fix: propagate any errors in src/pkg/layout
mkcp Sep 25, 2024
ffc2d20
fix: srcfile is already closed and the error handled, no need to defe…
mkcp Sep 25, 2024
35cf75e
fix: ensure errors from deferred resource cleanup are passed back to …
mkcp Sep 26, 2024
53a175b
chore: undo linter rule for CI so we can merge incremental changes
mkcp Sep 26, 2024
391296f
chore: undo the other linter rule
mkcp Sep 26, 2024
2c61d4c
chore: extract test changes to another branch
mkcp Sep 26, 2024
423c96a
fix: remove nolints for now to make ci happy. we'll have to readd later
mkcp Sep 26, 2024
41265d3
Merge branch 'main' into mkcp/2949-errcheck-linter
mkcp Sep 26, 2024
d2334bb
chore: address some review comments
mkcp Sep 30, 2024
a043273
chore: fix some bugs in nil returns for errors.Join and address revie…
mkcp Sep 30, 2024
59c3113
review comment
mkcp Sep 30, 2024
7f53091
chore: missed some slogs
mkcp Sep 30, 2024
1602735
chore: gofmt cmd/tools/helm.go
mkcp Sep 30, 2024
7d4da41
Merge branch 'main' into mkcp/2949-errcheck-linter
mkcp Sep 30, 2024
91b28ba
chore: mark some todos for lint ignores
mkcp Sep 30, 2024
910027f
fix review comments
mkcp Oct 1, 2024
e56681c
fix: actually store the error values in the deferred cleanup
mkcp Oct 1, 2024
193254c
a/b testing if inline closes not defers fixes windows tests
mkcp Oct 1, 2024
bb4b8b6
a/b testing... is it really two closes that fixes it?
mkcp Oct 1, 2024
6c86974
a/b testing... what about inline only, no defer inside the loop?
mkcp Oct 1, 2024
ff0df92
a/b testing... no inline inside loop. defer only. and defer outside o…
mkcp Oct 1, 2024
df0d3b4
fix: ensure we close the file before trying to remove it
mkcp Oct 1, 2024
2afa430
fix: if the file is already closed, don't join to err return
mkcp Oct 1, 2024
22b6bf5
ignore intentionally blank errs
mkcp Oct 1, 2024
1a47fb5
return cluster errors in cmd/package
mkcp Oct 1, 2024
6243854
add linter rule for errcheck empty. ignore some dirs
mkcp Oct 1, 2024
025bd87
Merge branch 'main' into mkcp/2949-add-linter
mkcp Oct 2, 2024
75a4532
remove bigbang linter exception
mkcp Oct 2, 2024
350df60
remove extra slog import
mkcp Oct 2, 2024
c2ff39b
cleanup some more merge cruft
mkcp Oct 2, 2024
c4b1789
remove comment and add linter ignore for CI
mkcp Oct 2, 2024
f94e75d
Is this defer cleanup timing out CI?
mkcp Oct 2, 2024
e1269a4
do not error if we fail to find a cluster here
mkcp Oct 2, 2024
74e9c2f
fixed this in another branch, snuck thru
mkcp Oct 2, 2024
f346300
Merge branch 'main' into mkcp/2949-add-linter
mkcp Oct 3, 2024
29ffbbf
Update src/test/external/ext_out_cluster_test.go
mkcp Oct 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ linters-settings:
testifylint:
enable-all: true
errcheck:
check-blank: true
check-type-assertions: true
exclude-functions:
- (*github.com/spf13/cobra.Command).Help
Expand All @@ -68,10 +69,13 @@ linters-settings:
issues:
# Revive rules that are disabled by default.
include:
- EXC0001
- EXC0012
- EXC0013
- EXC0014
- EXC0015
# Exclude linting code copied from Helm.
exclude-dirs:
- "src/cmd/tools/helm"
- "src/cmd/tools/helm" # Exclude linting code copied from Helm.
- "src/internal/packager"
- "src/pkg/packager" # TODO(mkcp): Delete packager rules once refactor is complete
- "src/internal/packager2" # TODO(mkcp): Delete packager rules once refactor is complete
18 changes: 12 additions & 6 deletions src/cmd/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ var packageMirrorCmd = &cobra.Command{
pkgConfig.PkgOpts.SkipSignatureValidation = true
}
},
RunE: func(cmd *cobra.Command, args []string) error {
RunE: func(cmd *cobra.Command, args []string) (err error) {
var c *cluster.Cluster
if dns.IsServiceURL(pkgConfig.InitOpts.RegistryInfo.Address) || dns.IsServiceURL(pkgConfig.InitOpts.GitServer.Address) {
var err error
Expand Down Expand Up @@ -157,8 +157,11 @@ var packageMirrorCmd = &cobra.Command{
if err != nil {
return err
}
//nolint: errcheck // ignore
defer pkgLayout.Cleanup()
defer func() {
// Cleanup package files
err = errors.Join(err, pkgLayout.Cleanup())
}()

mirrorOpt := packager2.MirrorOptions{
Cluster: c,
PkgLayout: pkgLayout,
Expand Down Expand Up @@ -194,7 +197,7 @@ var packageInspectCmd = &cobra.Command{
return err
}

cluster, _ := cluster.NewCluster()
cluster, _ := cluster.NewCluster() //nolint:errcheck
inspectOpt := packager2.ZarfInspectOptions{
Source: src,
SkipSignatureValidation: pkgConfig.PkgOpts.SkipSignatureValidation,
Expand All @@ -211,7 +214,10 @@ var packageInspectCmd = &cobra.Command{
return fmt.Errorf("failed to inspect package: %w", err)
}
for _, image := range output {
fmt.Fprintln(os.Stdout, "-", image)
_, err := fmt.Fprintln(os.Stdout, "-", image)
if err != nil {
return err
}
}
}

Expand Down Expand Up @@ -291,7 +297,7 @@ var packageRemoveCmd = &cobra.Command{
filters.ByLocalOS(runtime.GOOS),
filters.BySelectState(pkgConfig.PkgOpts.OptionalComponents),
)
cluster, _ := cluster.NewCluster()
cluster, _ := cluster.NewCluster() //nolint:errcheck
removeOpt := packager2.RemoveOptions{
Source: packageSource,
Cluster: cluster,
Expand Down
6 changes: 3 additions & 3 deletions src/pkg/utils/cosign.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,14 +233,14 @@ func GetCosignArtifacts(image string) ([]string, error) {

// Return empty if we don't have a signature on the image
var remoteOpts []ociremote.Option
simg, _ := ociremote.SignedEntity(ref, remoteOpts...) // TODO(mkcp): //nolint:errcheck
simg, _ := ociremote.SignedEntity(ref, remoteOpts...) //nolint:errcheck
if simg == nil {
return nil, nil
}

// Errors are dogsled because these functions always return a name.Tag which we can check for layers
sigRef, _ := ociremote.SignatureTag(ref, remoteOpts...) // TODO(mkcp): //nolint:errcheck
attRef, _ := ociremote.AttestationTag(ref, remoteOpts...) // TODO(mkcp): //nolint:errcheck
sigRef, _ := ociremote.SignatureTag(ref, remoteOpts...) //nolint:errcheck
attRef, _ := ociremote.AttestationTag(ref, remoteOpts...) //nolint:errcheck

ss, err := simg.Signatures()
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions src/test/e2e/20_zarf_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ func TestZarfInit(t *testing.T) {
verifyZarfServiceLabels(t)

// Special sizing-hacking for reducing resources where Kind + CI eats a lot of free cycles (ignore errors)
_, _, _ = e2e.Kubectl(t, "scale", "deploy", "-n", "kube-system", "coredns", "--replicas=1") // TODO(mkcp): intentionally ignored, mark nolint
_, _, _ = e2e.Kubectl(t, "scale", "deploy", "-n", "zarf", "agent-hook", "--replicas=1") // TODO(mkcp): intentionally ignored, mark nolint
_, _, _ = e2e.Kubectl(t, "scale", "deploy", "-n", "kube-system", "coredns", "--replicas=1") //nolint:errcheck
_, _, _ = e2e.Kubectl(t, "scale", "deploy", "-n", "zarf", "agent-hook", "--replicas=1") //nolint:errcheck
}

func checkLogForSensitiveState(t *testing.T, logText string, zarfState types.ZarfState) {
Expand Down
2 changes: 1 addition & 1 deletion src/test/e2e/28_wait_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func TestNoWait(t *testing.T) {
case <-time.After(30 * time.Second):
t.Error("Timeout waiting for zarf deploy (it tried to wait)")
t.Log("Removing hanging namespace...")
_, _, _ = e2e.Kubectl(t, "delete", "namespace", "no-wait", "--force=true", "--wait=false", "--grace-period=0") // TODO(mkcp): intentionally ignored, mark nolint
_, _, _ = e2e.Kubectl(t, "delete", "namespace", "no-wait", "--force=true", "--wait=false", "--grace-period=0") //nolint:errcheck
}
require.NoError(t, err, stdOut, stdErr)

Expand Down
8 changes: 4 additions & 4 deletions src/test/external/ext_out_cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ func (suite *ExtOutClusterTestSuite) SetupSuite() {
// Teardown any leftovers from previous tests
// NOTE(mkcp): We dogsled these errors because some of these commands will error if they don't cleanup a resource,
// which is ok. A better solution would be checking for none or unexpected kinds of errors.
_ = exec.CmdWithPrint("k3d", "cluster", "delete", clusterName) // TODO(mkcp): intentionally ignored, mark nolint
_ = exec.CmdWithPrint("k3d", "registry", "delete", registryHost) // TODO(mkcp): intentionally ignored, mark nolint
_ = exec.CmdWithPrint("docker", "compose", "down") // TODO(mkcp): intentionally ignored, mark nolint
_ = exec.CmdWithPrint("docker", "network", "remove", network) // TODO(mkcp): intentionally ignored, mark nolint
_ = exec.CmdWithPrint("k3d", "cluster", "delete", clusterName) //nolint:errcheck
_ = exec.CmdWithPrint("docker", "rm", "-f", "k3d-"+registryHost) //nolint:errcheck
mkcp marked this conversation as resolved.
Show resolved Hide resolved
_ = exec.CmdWithPrint("docker", "compose", "down") //nolint:errcheck
_ = exec.CmdWithPrint("docker", "network", "remove", network) //nolint:errcheck

// Setup a network for everything to live inside
err := exec.CmdWithPrint("docker", "network", "create", "--driver=bridge", "--subnet="+subnet, "--gateway="+gateway, network)
Expand Down