chore: upgrade go version, add tests and comments, and fix arg names (#396)

Author: MeNsaaH

.golangci.yml

@@ -1,3 +1,4 @@
+version: "2"
 run:
   tests: false
 
@@ -6,10 +7,10 @@ linters:
     - bodyclose
     - durationcheck
     - errcheck
-    - gosimple
     - govet
     - ineffassign
     - staticcheck
+    - sloglint
     - unparam
     - unused
     - usestdlibvars

.tool-versions

@@ -1,6 +1,6 @@
 earthly 0.8.15
-golang 1.22.7
-golangci-lint 1.63.4
+golang 1.24.2
+golangci-lint 2.1.1
 helm 3.16.3
 helm-cr 1.6.1
 helm-ct 3.11.0

cmd/flags_test.go

@@ -51,3 +51,77 @@ func TestStringUsages(t *testing.T) {
 		})
 	}
 }
+
+func TestCombine(t *testing.T) {
+	tests := map[string]struct {
+		dst      DocOpt[any]
+		src      DocOpt[any]
+		expected DocOpt[any]
+	}{
+		"combine choices": {
+			dst: DocOpt[any]{},
+			src: DocOpt[any]{
+				choices: []string{"choice1", "choice2"},
+			},
+			expected: DocOpt[any]{
+				choices: []string{"choice1", "choice2"},
+			},
+		},
+		"combine default value": {
+			dst: DocOpt[any]{},
+			src: DocOpt[any]{
+				defaultValue: ptr[any]("default"),
+			},
+			expected: DocOpt[any]{
+				defaultValue: ptr[any]("default"),
+			},
+		},
+		"combine shorthand": {
+			dst: DocOpt[any]{},
+			src: DocOpt[any]{
+				shorthand: ptr("s"),
+			},
+			expected: DocOpt[any]{
+				shorthand: ptr("s"),
+			},
+		},
+		"combine all fields": {
+			dst: DocOpt[any]{},
+			src: DocOpt[any]{
+				choices:      []string{"choice1", "choice2"},
+				defaultValue: ptr[any]("default"),
+				shorthand:    ptr("s"),
+			},
+			expected: DocOpt[any]{
+				choices:      []string{"choice1", "choice2"},
+				defaultValue: ptr[any]("default"),
+				shorthand:    ptr("s"),
+			},
+		},
+		"preserve existing dst values when src is empty": {
+			dst: DocOpt[any]{
+				choices:      []string{"existing"},
+				defaultValue: ptr[any]("existing"),
+				shorthand:    ptr("e"),
+			},
+			src: DocOpt[any]{},
+			expected: DocOpt[any]{
+				choices:      []string{"existing"},
+				defaultValue: ptr[any]("existing"),
+				shorthand:    ptr("e"),
+			},
+		},
+	}
+
+	for testName, test := range tests {
+		t.Run(testName, func(t *testing.T) {
+			combine(&test.dst, test.src)
+			assert.Equal(t, test.expected, test.dst)
+		})
+	}
+}
+
+// Helper function to create pointers for test values
+func ptr[T any](v T) *T {
+	return &v
+}

cmd/locations.go

@@ -16,6 +16,10 @@ import (
 	"github.com/zapier/kubechecks/pkg/git"
 )
 
+// processLocations processes a list of locations, which can be either file paths or Git repository URLs.
+// For Git URLs, it clones the repository and updates the location to point to the local clone.
+// For file paths, it leaves them unchanged.
+// The function returns an error if any Git repository fails to clone.
 func processLocations(ctx context.Context, ctr container.Container, locations []string) error {
 	for index, location := range locations {
 		if newLocation, err := maybeCloneGitUrl(ctx, ctr.RepoManager, ctr.Config.RepoRefreshInterval, location, ctr.VcsClient.Username(), ctr.Config.RepoShallowClone); err != nil {

cmd/process.go

@@ -7,9 +7,10 @@ import (
 	"github.com/argoproj/argo-cd/v2/common"
 	"github.com/rs/zerolog/log"
 	"github.com/spf13/cobra"
-	"github.com/zapier/kubechecks/pkg/container"
 
+	"github.com/zapier/kubechecks/pkg"
 	"github.com/zapier/kubechecks/pkg/config"
+	"github.com/zapier/kubechecks/pkg/container"
 	"github.com/zapier/kubechecks/pkg/server"
 )
 
@@ -24,9 +25,7 @@ var processCmd = &cobra.Command{
 		if err != nil {
 			log.Fatal().Err(err).Msg("fail to create ssh data dir")
 		}
-		defer func() {
-			os.RemoveAll(tempPath)
-		}()
+		defer pkg.WithErrorLogging(func() error { return os.RemoveAll(tempPath) }, "failed to remove temp directory")
 
 		// symlink local ssh known hosts to argocd ssh known hosts
 		homeDir, err := os.UserHomeDir()

cmd/root.go

@@ -108,7 +108,7 @@ func init() {
 	int64Flag(flags, "max-queue-size", "Size of app diff check queue.",
 		newInt64Opts().
 			withDefault(1024))
-	int64Flag(flags, "max-concurrenct-checks", "Number of concurrent checks to run.",
+	int64Flag(flags, "max-concurrent-checks", "Number of concurrent checks to run.",
 		newInt64Opts().
 			withDefault(32))
 	boolFlag(flags, "enable-hooks-renderer", "Render hooks.", newBoolOpts().withDefault(true))
@@ -125,7 +125,7 @@ func init() {
 	stringFlag(flags, "identifier", "Identifier for the kubechecks instance. Used to differentiate between multiple kubechecks instances.",
 		newStringOpts().
 			withDefault(""))
-	stringFlag(flags, "generated-store", "URL for the kubepug generated store.",
+	stringFlag(flags, "kubepug-generated-store", "URL for the kubepug generated store.",
 		newStringOpts().
 			withDefault("https://kubepug.xyz/data/data.json"))
 

docs/usage.md

@@ -51,17 +51,17 @@ The full list of supported environment variables is described below:
 |`KUBECHECKS_ENABLE_PREUPGRADE`|Enable preupgrade checks.|`true`|
 |`KUBECHECKS_ENSURE_WEBHOOKS`|Ensure that webhooks are created in repositories referenced by argo.|`false`|
 |`KUBECHECKS_FALLBACK_K8S_VERSION`|Fallback target Kubernetes version for schema / upgrade checks.|`1.23.0`|
-|`KUBECHECKS_GENERATED_STORE`|URL for the kubepug generated store.|`https://kubepug.xyz/data/data.json`|
 |`KUBECHECKS_GITHUB_APP_ID`|Github App ID.|`0`|
 |`KUBECHECKS_GITHUB_INSTALLATION_ID`|Github Installation ID.|`0`|
 |`KUBECHECKS_GITHUB_PRIVATE_KEY`|Github App Private Key.||
 |`KUBECHECKS_IDENTIFIER`|Identifier for the kubechecks instance. Used to differentiate between multiple kubechecks instances.||
+|`KUBECHECKS_KUBEPUG_GENERATED_STORE`|URL for the kubepug generated store.|`https://kubepug.xyz/data/data.json`|
 |`KUBECHECKS_KUBERNETES_CLUSTERID`|Kubernetes Cluster ID, must be specified if kubernetes-type is eks.||
 |`KUBECHECKS_KUBERNETES_CONFIG`|Path to your kubernetes config file, used to monitor applications.||
 |`KUBECHECKS_KUBERNETES_TYPE`|Kubernetes Type One of eks, or local.|`local`|
 |`KUBECHECKS_LABEL_FILTER`|(Optional) If set, The label that must be set on an MR (as "kubechecks:<value>") for kubechecks to process the merge request webhook.||
 |`KUBECHECKS_LOG_LEVEL`|Set the log output level. One of error, warn, info, debug, trace.|`info`|
-|`KUBECHECKS_MAX_CONCURRENCT_CHECKS`|Number of concurrent checks to run.|`32`|
+|`KUBECHECKS_MAX_CONCURRENT_CHECKS`|Number of concurrent checks to run.|`32`|
 |`KUBECHECKS_MAX_QUEUE_SIZE`|Size of app diff check queue.|`1024`|
 |`KUBECHECKS_MONITOR_ALL_APPLICATIONS`|Monitor all applications in argocd automatically.|`true`|
 |`KUBECHECKS_OPENAI_API_TOKEN`|OpenAI API Token.||

go.mod

@@ -1,8 +1,6 @@
 module github.com/zapier/kubechecks
 
-go 1.22.0
-
-toolchain go1.22.7
+go 1.24
 
 require (
 	github.com/argoproj/argo-cd/v2 v2.13.1

pkg/appdir/app_directory.go

@@ -8,11 +8,21 @@ import (
 	"github.com/rs/zerolog/log"
 )
 
+// AppDirectory manages the mapping between applications and their associated directories and files.
+// It provides functionality to track which applications are affected by changes in specific directories or files,
+// and maintains a collection of Argo CD applications.
 type AppDirectory struct {
-	appDirs  map[string][]string // directory -> array of app names
-	appFiles map[string][]string // file path -> array of app names
+	// appDirs maps directory paths to the names of applications that use those directories.
+	// This is used to quickly identify which applications are affected when changes occur in a directory.
+	appDirs map[string][]string
 
-	appsMap map[string]v1alpha1.Application // app name -> app stub
+	// appFiles maps file paths to the names of applications that use those files.
+	// This is used to quickly identify which applications are affected when specific files change.
+	appFiles map[string][]string
+
+	// appsMap stores the full Argo CD application definitions, indexed by application name.
+	// This serves as the source of truth for application configurations.
+	appsMap map[string]v1alpha1.Application
 }
 
 func NewAppDirectory() *AppDirectory {

pkg/appdir/appset_directory.go

@@ -8,15 +8,26 @@ import (
 
 	"github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
 	"github.com/rs/zerolog/log"
+	"github.com/zapier/kubechecks/pkg"
 	"github.com/zapier/kubechecks/pkg/git"
 	"sigs.k8s.io/yaml"
 )
 
+// AppSetDirectory manages the mapping between ApplicationSets and their associated directories and files.
+// It provides functionality to track which ApplicationSets are affected by changes in specific directories or files,
+// and maintains a collection of Argo CD ApplicationSets.
 type AppSetDirectory struct {
-	appSetDirs  map[string][]string // directory -> array of app names
-	appSetFiles map[string][]string // file path -> array of app names
+	// appSetDirs maps directory paths to the names of ApplicationSets that use those directories.
+	// This is used to quickly identify which ApplicationSets are affected when changes occur in a directory.
+	appSetDirs map[string][]string
 
-	appSetsMap map[string]v1alpha1.ApplicationSet // app name -> app stub
+	// appSetFiles maps file paths to the names of ApplicationSets that use those files.
+	// This is used to quickly identify which ApplicationSets are affected when specific files change.
+	appSetFiles map[string][]string
+
+	// appSetsMap stores the full Argo CD ApplicationSet definitions, indexed by ApplicationSet name.
+	// This serves as the source of truth for ApplicationSet configurations.
+	appSetsMap map[string]v1alpha1.ApplicationSet
 }
 
 func NewAppSetDirectory() *AppSetDirectory {
@@ -179,11 +190,7 @@ func containsKindApplicationSet(path string) bool {
 		log.Error().Err(err).Stack().Msgf("failed to open file %s: %v", path, err)
 		return false
 	}
-	defer func() {
-		if err := file.Close(); err != nil {
-			log.Warn().Err(err).Stack().Msgf("failed to close file %s: %v", path, err)
-		}
-	}()
+	defer pkg.WithErrorLogging(file.Close, "failed to close file")
 
 	scanner := bufio.NewScanner(file)
 	for scanner.Scan() {