add linting, clean up a bunch of code

.golangci.yaml

@@ -0,0 +1,54 @@
+linters:
+  enable-all: true
+
+  disable:
+    - dupl
+    - funlen
+    - gochecknoinits
+    - gocognit
+    - goconst
+    - godox
+    - gomoddirectives
+    - lll
+    - makezero
+    - nestif
+    - prealloc
+    - promlinter
+    - revive
+    - cyclop
+    - depguard
+    - dupword
+    - err113
+    - errname
+    - exhaustive
+    - exhaustruct
+    - forcetypeassert
+    - gochecknoglobals
+    - gomnd
+    - inamedparam
+    - interfacebloat
+    - ireturn
+    - maintidx
+    - mnd
+    - nilnil
+    - nlreturn
+    - paralleltest
+    - perfsprint
+    - predeclared
+    - stylecheck
+    - tagliatelle
+    - testpackage
+    - varnamelen
+    - wrapcheck
+    - wsl
+
+    - exportloopref  # deprecated
+    - execinquery  # deprecated
+    - gomnd  # deprecated
+
+linters-settings:
+  gci:
+    sections:
+      - standard
+      - default
+      - localmodule

cmd/container.go

@@ -6,6 +6,7 @@ import (
 
 	"github.com/pkg/errors"
 	"github.com/rs/zerolog/log"
+
 	"github.com/zapier/kubechecks/pkg/app_watcher"
 	"github.com/zapier/kubechecks/pkg/appdir"
 	"github.com/zapier/kubechecks/pkg/argo_client"
@@ -20,7 +21,7 @@ import (
 func newContainer(ctx context.Context, cfg config.ServerConfig, watchApps bool) (container.Container, error) {
 	var err error
 
-	var ctr = container.Container{
+	ctr := container.Container{
 		Config:      cfg,
 		RepoManager: git.NewRepoManager(cfg),
 	}
@@ -30,7 +31,7 @@ func newContainer(ctx context.Context, cfg config.ServerConfig, watchApps bool)
 	case "gitlab":
 		ctr.VcsClient, err = gitlab_client.CreateGitlabClient(cfg)
 	case "github":
-		ctr.VcsClient, err = github_client.CreateGithubClient(cfg)
+		ctr.VcsClient, err = github_client.CreateGithubClient(ctx, cfg)
 	default:
 		err = fmt.Errorf("unknown vcs-type: %q", cfg.VcsType)
 	}

cmd/controller_cmd.go

@@ -22,7 +22,7 @@ import (
 	"github.com/zapier/kubechecks/telemetry"
 )
 
-// ControllerCmd represents the run command
+// ControllerCmd represents the run command.
 var ControllerCmd = &cobra.Command{
 	Use:   "controller",
 	Short: "Start the VCS Webhook handler.",
@@ -67,7 +67,7 @@ var ControllerCmd = &cobra.Command{
 		if err != nil {
 			log.Panic().Err(err).Msg("Failed to initialize telemetry")
 		}
-		defer t.Shutdown()
+		defer t.Shutdown(ctx)
 
 		log.Info().Msgf("starting web server")
 		startWebserver(ctx, ctr, processors)

cmd/flags_test.go

@@ -45,7 +45,6 @@ func TestStringUsages(t *testing.T) {
 
 	for testName, test := range tests {
 		t.Run(testName, func(t *testing.T) {
-
 			actual := generateUsage(test.opt, test.usage, test.name)
 			assert.Equal(t, test.expected, actual)
 		})

cmd/locations.go

@@ -64,7 +64,7 @@ func maybeCloneGitUrl(ctx context.Context, repoManager cloner, repoRefreshDurati
 				case <-tick:
 				}
 
-				if err := repo.Update(ctx); err != nil {
+				if err := repo.Update(); err != nil {
 					log.Warn().
 						Err(err).
 						Str("path", repo.Directory).

cmd/locations_test.go

@@ -40,7 +40,6 @@ func TestMaybeCloneGitUrl_NonGitUrl(t *testing.T) {
 	}
 
 	for _, tc := range testcases {
-		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			fc := &fakeCloner{result: nil, err: nil}
 			actual, err := maybeCloneGitUrl(ctx, fc, time.Duration(0), tc.input, testUsername)
@@ -52,13 +51,13 @@ func TestMaybeCloneGitUrl_NonGitUrl(t *testing.T) {
 	}
 }
 
-const testRoot = "/tmp/path"
-const testUsername = "username"
+const (
+	testRoot     = "/tmp/path"
+	testUsername = "username"
+)
 
 func TestMaybeCloneGitUrl_HappyPath(t *testing.T) {
-	var (
-		ctx = context.TODO()
-	)
+	ctx := context.TODO()
 
 	type expected struct {
 		path, cloneUrl, branch string
@@ -134,7 +133,6 @@ func TestMaybeCloneGitUrl_HappyPath(t *testing.T) {
 	}
 
 	for _, tc := range testcases {
-		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			fc := &fakeCloner{result: &git.Repo{Directory: testRoot}, err: nil}
 			actual, err := maybeCloneGitUrl(ctx, fc, time.Duration(0), tc.input, testUsername)
@@ -147,9 +145,7 @@ func TestMaybeCloneGitUrl_HappyPath(t *testing.T) {
 }
 
 func TestMaybeCloneGitUrl_URLError(t *testing.T) {
-	var (
-		ctx = context.TODO()
-	)
+	ctx := context.TODO()
 
 	testcases := []struct {
 		name, input, expected string
@@ -162,7 +158,6 @@ func TestMaybeCloneGitUrl_URLError(t *testing.T) {
 	}
 
 	for _, tc := range testcases {
-		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			fc := &fakeCloner{result: &git.Repo{Directory: testRoot}, err: nil}
 			result, err := maybeCloneGitUrl(ctx, fc, time.Duration(0), tc.input, testUsername)
@@ -186,7 +181,6 @@ func TestMaybeCloneGitUrl_CloneError(t *testing.T) {
 	}
 
 	for _, tc := range testcases {
-		tc := tc
 		t.Run(tc.name, func(t *testing.T) {
 			ctx := context.Background()
 			ctx, cancel := context.WithCancel(ctx)

cmd/root.go

@@ -11,7 +11,7 @@ import (
 	"github.com/spf13/viper"
 )
 
-// RootCmd represents the base command when called without any subcommands
+// RootCmd represents the base command when called without any subcommands.
 var RootCmd = &cobra.Command{
 	Use:              "kubechecks",
 	Short:            "Argo Git Hooks",
@@ -142,15 +142,14 @@ func setupLogOutput() {
 	// set logrus log level to overwrite the logs exporting from argo-cd package
 	logrusLevel := logrus.ErrorLevel
 	if viper.GetBool("persist_log_level") {
-		if log.Debug().Enabled() {
+		if log.Debug().Enabled() { //nolint: zerologlint
 			logrusLevel = logrus.DebugLevel
 		}
-		if log.Trace().Enabled() {
+		if log.Trace().Enabled() { //nolint: zerologlint
 			logrusLevel = logrus.TraceLevel
 		}
 	}
 
 	logrus.StandardLogger().Level = logrusLevel
 	log.Info().Str("log_level", logrus.StandardLogger().Level.String()).Msg("setting logrus log level")
-
 }

cmd/version.go

@@ -1,8 +1,7 @@
 package cmd
 
 import (
-	"fmt"
-
+	"github.com/rs/zerolog/log"
 	"github.com/spf13/cobra"
 
 	"github.com/zapier/kubechecks/pkg"
@@ -13,7 +12,7 @@ var versionCmd = &cobra.Command{
 	Short: "List version information",
 	Long:  ``,
 	Run: func(cmd *cobra.Command, args []string) {
-		fmt.Printf("kubechecks\nVersion:%s\nSHA%s\n", pkg.GitTag, pkg.GitCommit)
+		log.Info().Msgf("kubechecks\nVersion:%s\nSHA%s\n", pkg.GitTag, pkg.GitCommit)
 	},
 }
 

hacks/env-to-docs.go

@@ -21,8 +21,10 @@ type option struct {
 	Default string
 }
 
-var UsageEnvVar = regexp.MustCompile(` \(KUBECHECKS_[_A-Z0-9]+\)`)
-var UsageDefaultValue = regexp.MustCompile(`Defaults to \.?(.*)+\.`)
+var (
+	UsageEnvVar       = regexp.MustCompile(` \(KUBECHECKS_[_A-Z0-9]+\)`)
+	UsageDefaultValue = regexp.MustCompile(`Defaults to \.?(.*)+\.`)
+)
 
 func main() {
 	outputFilename := filepath.Join("docs", "usage.md")

pkg/affected_apps/argocd_matcher.go

@@ -5,6 +5,7 @@ import (
 	"os"
 
 	"github.com/rs/zerolog/log"
+
 	"github.com/zapier/kubechecks/pkg/appdir"
 	"github.com/zapier/kubechecks/pkg/container"
 	"github.com/zapier/kubechecks/pkg/git"

pkg/affected_apps/argocd_matcher_test.go

@@ -38,6 +38,6 @@ func TestFindAffectedAppsWithNilAppsDirectory(t *testing.T) {
 
 	// verify results
 	require.NoError(t, err)
-	require.Len(t, items.Applications, 0)
-	require.Len(t, items.ApplicationSets, 0)
+	require.Empty(t, items.Applications)
+	require.Empty(t, items.ApplicationSets)
 }

pkg/affected_apps/config_matcher.go

@@ -9,10 +9,10 @@ import (
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	"github.com/pkg/errors"
 	"github.com/rs/zerolog/log"
-	"github.com/zapier/kubechecks/pkg/git"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	"github.com/zapier/kubechecks/pkg/container"
+	"github.com/zapier/kubechecks/pkg/git"
 	"github.com/zapier/kubechecks/pkg/repo_config"
 )
 
@@ -123,13 +123,12 @@ func (b *ConfigMatcher) applicationsForDir(dir string) []*repo_config.ArgoCdAppl
 				continue
 			}
 		}
-
 	}
 
 	return apps
 }
 
-// appsFromApplicationSetForDir: Get the list of apps managed by an applicationset from dir
+// appsFromApplicationSetForDir: Get the list of apps managed by an applicationset from dir.
 func (b *ConfigMatcher) appsFromApplicationSetForDir(ctx context.Context, dir string) ([]*repo_config.ArgocdApplicationSetConfig, []*repo_config.ArgoCdApplicationConfig, error) {
 	var appsets []*repo_config.ArgocdApplicationSetConfig
 	for _, appset := range b.cfg.ApplicationSets {
@@ -177,20 +176,22 @@ func dirMatchForApp(changeDir, appDir string) bool {
 	return false
 }
 
-// Any files modified under appset subdirectories assumes the appset is modified
+// Any files modified under appset subdirectories assumes the appset is modified.
 func dirMatchForAppSet(changeDir, appSetDir string) bool {
 	// normalize dir for matching
 	appSetDir = path.Clean(appSetDir)
 	changeDir = path.Clean(changeDir)
 
 	log.Debug().Msgf("appSetDir: %s; changeDir: %s", appSetDir, changeDir)
 
-	if strings.HasSuffix(changeDir, appSetDir) {
+	switch {
+	case strings.HasSuffix(changeDir, appSetDir):
 		return true
-	} else if strings.HasPrefix(changeDir, appSetDir) {
+	case strings.HasPrefix(changeDir, appSetDir):
 		return true
-	} else if changeDir == "." && appSetDir == "/" {
+	case changeDir == "." && appSetDir == "/":
 		return true
 	}
+
 	return false
 }

pkg/affected_apps/config_matcher_test.go

@@ -39,7 +39,6 @@ func Test_dirMatchForApp(t *testing.T) {
 }
 
 func TestConfigMatcher_triggeredApps(t *testing.T) {
-
 	type args struct {
 		modifiedFiles []string
 	}
@@ -95,8 +94,6 @@ func TestConfigMatcher_triggeredApps(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		tt := tt
-
 		t.Run(tt.name, func(t *testing.T) {
 			mockArgoClient := newMockArgoClient()
 
@@ -116,8 +113,7 @@ func newMockArgoClient() argoClient {
 	return new(mockArgoClient)
 }
 
-type mockArgoClient struct {
-}
+type mockArgoClient struct{}
 
 func (m mockArgoClient) GetApplications(ctx context.Context) (*v1alpha1.ApplicationList, error) {
 	return new(v1alpha1.ApplicationList), nil
@@ -130,10 +126,11 @@ func (m mockArgoClient) GetApplicationsByAppset(ctx context.Context, appsetName
 var _ argoClient = new(mockArgoClient)
 
 func testLoadConfig(t *testing.T, configDir string) *repo_config.Config {
+	t.Helper()
+
 	cfg, err := repo_config.LoadRepoConfig(configDir)
-	if err != nil {
-		t.Errorf("could not load test config from dir (%s): %v", configDir, err)
-	}
+	assert.NoErrorf(t, err, "could not load test config from dir (%s)", configDir)
+
 	return cfg
 }
 

pkg/affected_apps/matcher.go

@@ -5,6 +5,7 @@ import (
 	"path"
 
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
+
 	"github.com/zapier/kubechecks/pkg/git"
 )
 
@@ -53,7 +54,7 @@ type Matcher interface {
 }
 
 // modifiedDirs filters a list of changed files down to a list
-// the unique dirs containing the changed files
+// the unique dirs containing the changed files.
 func modifiedDirs(changeList []string) []string {
 	dirMap := map[string]bool{}
 	for _, file := range changeList {

pkg/affected_apps/multi_matcher.go

@@ -4,6 +4,7 @@ import (
 	"context"
 
 	"github.com/pkg/errors"
+
 	"github.com/zapier/kubechecks/pkg/git"
 )
 

pkg/affected_apps/multi_matcher_test.go

@@ -6,8 +6,9 @@ import (
 
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	"github.com/stretchr/testify/require"
-	"github.com/zapier/kubechecks/pkg/git"
 	v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/zapier/kubechecks/pkg/git"
 )
 
 type fakeMatcher struct {