From 2c4d9ea453d94d1b8686bb8596d453d13b239d6c Mon Sep 17 00:00:00 2001 From: Joe Lombrozo Date: Tue, 1 Oct 2024 16:06:32 -0400 Subject: [PATCH] add linting --- .golangci.yaml | 54 ++++++++++++++ cmd/container.go | 5 +- cmd/controller_cmd.go | 4 +- cmd/flags_test.go | 1 - cmd/locations.go | 2 +- cmd/locations_test.go | 18 ++--- cmd/root.go | 7 +- cmd/version.go | 5 +- hacks/env-to-docs.go | 6 +- pkg/affected_apps/argocd_matcher.go | 1 + pkg/affected_apps/argocd_matcher_test.go | 4 +- pkg/affected_apps/config_matcher.go | 15 ++-- pkg/affected_apps/config_matcher_test.go | 13 ++-- pkg/affected_apps/matcher.go | 3 +- pkg/affected_apps/multi_matcher.go | 1 + pkg/affected_apps/multi_matcher_test.go | 3 +- pkg/aisummary/diff_summary.go | 5 +- pkg/aisummary/openai_client.go | 8 +- pkg/app_watcher/app_watcher.go | 5 +- pkg/app_watcher/app_watcher_test.go | 25 ++++--- pkg/app_watcher/appset_watcher.go | 11 +-- pkg/app_watcher/appset_watcher_test.go | 18 +++-- pkg/appdir/app_directory.go | 2 +- pkg/appdir/app_directory_test.go | 4 + pkg/appdir/appset_directory.go | 5 +- pkg/appdir/appset_directory_test.go | 13 ++-- pkg/appdir/vcstoargomap.go | 1 + pkg/appdir/vcstoargomap_test.go | 30 ++++---- pkg/appdir/walk_kustomize_files_test.go | 9 ++- pkg/argo_client/applications.go | 14 ++-- pkg/argo_client/client.go | 3 +- pkg/checks/diff/diff.go | 25 ++++--- pkg/checks/hooks/check.go | 2 +- pkg/checks/hooks/check_test.go | 1 + pkg/checks/kubeconform/validate.go | 15 ++-- pkg/checks/kubeconform/validate_test.go | 12 +-- pkg/checks/preupgrade/kubepug.go | 6 +- pkg/checks/rego/check.go | 17 +++-- pkg/checks/rego/check_test.go | 7 +- pkg/commitState.go | 5 +- pkg/config/config_test.go | 4 +- pkg/container/main.go | 2 +- pkg/events/check.go | 12 ++- pkg/events/check_test.go | 11 +-- pkg/events/worker.go | 6 +- pkg/generator/applicationsets.go | 24 +++--- pkg/generator/cluster.go | 20 ++--- pkg/generator/cluster_test.go | 74 ++++++++++--------- pkg/generator/generator_spec_processor.go | 12 +-- .../generator_spec_processor_test.go | 6 +- pkg/generator/interface.go | 6 +- pkg/generator/list.go | 20 ++--- pkg/generator/list_test.go | 21 +++--- pkg/generator/matrix.go | 15 ++-- pkg/generator/matrix_test.go | 5 +- pkg/generator/merge.go | 12 +-- pkg/generator/merge_test.go | 20 ++--- pkg/generator/value_interpolation.go | 1 - pkg/git/repo.go | 28 ++++--- pkg/git/repo_test.go | 2 + pkg/kubernetes/api_client.go | 4 +- pkg/kubernetes/api_eks_client.go | 8 +- pkg/msg/message.go | 4 +- pkg/repoUrl.go | 2 +- pkg/repoUrl_test.go | 3 +- pkg/repo_config/config.go | 29 ++++---- pkg/repo_config/loader.go | 3 +- pkg/repo_config/loader_test.go | 5 +- pkg/server/hook_handler.go | 16 ++-- pkg/server/server.go | 5 +- pkg/vcs/client.go | 2 +- pkg/vcs/github_client/client.go | 9 +-- pkg/vcs/github_client/client_test.go | 44 +++++------ pkg/vcs/github_client/emoji.go | 2 +- pkg/vcs/github_client/message.go | 5 +- pkg/vcs/gitlab_client/backoff.go | 25 +++---- pkg/vcs/gitlab_client/client.go | 11 +-- pkg/vcs/gitlab_client/emoji.go | 2 +- pkg/vcs/gitlab_client/message.go | 7 +- pkg/vcs/gitlab_client/pipeline.go | 1 - pkg/vcs/gitlab_client/project.go | 2 +- pkg/vcs/gitlab_client/status.go | 3 +- pkg/vcs/repo.go | 2 +- pkg/vcs/types.go | 2 +- telemetry/helpers.go | 2 +- telemetry/telemetry.go | 40 +++++----- 86 files changed, 465 insertions(+), 459 deletions(-) create mode 100644 .golangci.yaml diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 00000000..b422da3e --- /dev/null +++ b/.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 diff --git a/cmd/container.go b/cmd/container.go index c447838a..2ea8ee44 100644 --- a/cmd/container.go +++ b/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) } diff --git a/cmd/controller_cmd.go b/cmd/controller_cmd.go index 5b8b0440..eb2e8516 100644 --- a/cmd/controller_cmd.go +++ b/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) diff --git a/cmd/flags_test.go b/cmd/flags_test.go index 73c52fff..f4ad57f9 100644 --- a/cmd/flags_test.go +++ b/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) }) diff --git a/cmd/locations.go b/cmd/locations.go index 47b5774d..cb7875bf 100644 --- a/cmd/locations.go +++ b/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). diff --git a/cmd/locations_test.go b/cmd/locations_test.go index b47af25a..61cf05b2 100644 --- a/cmd/locations_test.go +++ b/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) diff --git a/cmd/root.go b/cmd/root.go index 470f675b..096673d2 100644 --- a/cmd/root.go +++ b/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", @@ -130,15 +130,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") - } diff --git a/cmd/version.go b/cmd/version.go index 448e8fb2..3fc063cd 100644 --- a/cmd/version.go +++ b/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) }, } diff --git a/hacks/env-to-docs.go b/hacks/env-to-docs.go index 1fefcc5d..ab91e5bb 100644 --- a/hacks/env-to-docs.go +++ b/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") diff --git a/pkg/affected_apps/argocd_matcher.go b/pkg/affected_apps/argocd_matcher.go index 0c1b7ded..75acaca7 100644 --- a/pkg/affected_apps/argocd_matcher.go +++ b/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" diff --git a/pkg/affected_apps/argocd_matcher_test.go b/pkg/affected_apps/argocd_matcher_test.go index 0db01456..3d3f977d 100644 --- a/pkg/affected_apps/argocd_matcher_test.go +++ b/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) } diff --git a/pkg/affected_apps/config_matcher.go b/pkg/affected_apps/config_matcher.go index de5e8b87..89c79b51 100644 --- a/pkg/affected_apps/config_matcher.go +++ b/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,7 +176,7 @@ 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) @@ -185,12 +184,14 @@ func dirMatchForAppSet(changeDir, appSetDir string) bool { 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 } diff --git a/pkg/affected_apps/config_matcher_test.go b/pkg/affected_apps/config_matcher_test.go index 101c531c..600053fc 100644 --- a/pkg/affected_apps/config_matcher_test.go +++ b/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 } diff --git a/pkg/affected_apps/matcher.go b/pkg/affected_apps/matcher.go index a1c7a9a9..5d04a76c 100644 --- a/pkg/affected_apps/matcher.go +++ b/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 { diff --git a/pkg/affected_apps/multi_matcher.go b/pkg/affected_apps/multi_matcher.go index 5f49a99f..0db40804 100644 --- a/pkg/affected_apps/multi_matcher.go +++ b/pkg/affected_apps/multi_matcher.go @@ -4,6 +4,7 @@ import ( "context" "github.com/pkg/errors" + "github.com/zapier/kubechecks/pkg/git" ) diff --git a/pkg/affected_apps/multi_matcher_test.go b/pkg/affected_apps/multi_matcher_test.go index 16f4ee38..3e0e7891 100644 --- a/pkg/affected_apps/multi_matcher_test.go +++ b/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 { diff --git a/pkg/aisummary/diff_summary.go b/pkg/aisummary/diff_summary.go index 3b1163fa..34cb7d78 100644 --- a/pkg/aisummary/diff_summary.go +++ b/pkg/aisummary/diff_summary.go @@ -2,8 +2,8 @@ package aisummary import ( "context" - "fmt" + "github.com/rs/zerolog/log" "github.com/sashabaranov/go-openai" "go.opentelemetry.io/otel" @@ -34,12 +34,11 @@ func (c *OpenAiClient) SummarizeDiff(ctx context.Context, appName, diff string) resp, err := c.makeCompletionRequestWithBackoff(ctx, req) if err != nil { telemetry.SetError(span, err, "ChatCompletionStream error") - fmt.Printf("ChatCompletionStream error: %v\n", err) + log.Debug().Err(err).Msg("ChatCompletionStream") return "", err } return resp.Choices[0].Message.Content, nil - } return "", nil } diff --git a/pkg/aisummary/openai_client.go b/pkg/aisummary/openai_client.go index 2ad5d58c..b6a1414d 100644 --- a/pkg/aisummary/openai_client.go +++ b/pkg/aisummary/openai_client.go @@ -17,8 +17,10 @@ type OpenAiClient struct { enabled bool } -var openAiClient *OpenAiClient -var once sync.Once +var ( + openAiClient *OpenAiClient + once sync.Once +) func GetOpenAiClient(apiToken string) *OpenAiClient { once.Do(func() { @@ -35,7 +37,7 @@ func GetOpenAiClient(apiToken string) *OpenAiClient { } func createCompletionRequest(model, appName string, prompt string, content string, prefix string) openai.ChatCompletionRequest { - var summarizeRequest = openai.ChatCompletionRequest{ + summarizeRequest := openai.ChatCompletionRequest{ Model: model, MaxTokens: 500, Temperature: 0.4, diff --git a/pkg/app_watcher/app_watcher.go b/pkg/app_watcher/app_watcher.go index adb39c26..96d62581 100644 --- a/pkg/app_watcher/app_watcher.go +++ b/pkg/app_watcher/app_watcher.go @@ -20,7 +20,7 @@ import ( "github.com/zapier/kubechecks/pkg/config" ) -// ApplicationWatcher is the controller that watches ArgoCD Application resources via the Kubernetes API +// ApplicationWatcher is the controller that watches ArgoCD Application resources via the Kubernetes API. type ApplicationWatcher struct { applicationClientset appclientset.Interface appInformer cache.SharedIndexInformer @@ -68,7 +68,7 @@ func (ctrl *ApplicationWatcher) Run(ctx context.Context, processors int) { } // onAdd is the function executed when the informer notifies the -// presence of a new Application in the namespace +// presence of a new Application in the namespace. func (ctrl *ApplicationWatcher) onApplicationAdded(obj interface{}) { app, ok := canProcessApp(obj) if !ok { @@ -99,7 +99,6 @@ func (ctrl *ApplicationWatcher) onApplicationUpdated(old, new interface{}) { log.Info().Str("key", key).Msg("appwatcher: onApplicationUpdated") ctrl.vcsToArgoMap.UpdateApp(old.(*appv1alpha1.Application), new.(*appv1alpha1.Application)) } - } func (ctrl *ApplicationWatcher) onApplicationDeleted(obj interface{}) { diff --git a/pkg/app_watcher/app_watcher_test.go b/pkg/app_watcher/app_watcher_test.go index 4af7425c..ca549f8d 100644 --- a/pkg/app_watcher/app_watcher_test.go +++ b/pkg/app_watcher/app_watcher_test.go @@ -9,13 +9,15 @@ import ( appclientsetfake "github.com/argoproj/argo-cd/v2/pkg/client/clientset/versioned/fake" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/zapier/kubechecks/pkg/config" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "github.com/zapier/kubechecks/pkg/appdir" + "github.com/zapier/kubechecks/pkg/config" ) func initTestObjects(t *testing.T) *ApplicationWatcher { + t.Helper() + cfg, err := config.New() // Handle the error appropriately, e.g., log it or fail the test require.NoError(t, err, "failed to create config") @@ -57,7 +59,7 @@ func TestApplicationAdded(t *testing.T) { time.Sleep(time.Second * 1) - assert.Equal(t, len(appWatcher.vcsToArgoMap.GetMap()), 2) + assert.Len(t, appWatcher.vcsToArgoMap.GetMap(), 2) _, err := appWatcher.applicationClientset.ArgoprojV1alpha1().Applications("default").Create(ctx, &v1alpha1.Application{ ObjectMeta: metav1.ObjectMeta{Name: "test-app-3", Namespace: "default"}, @@ -70,7 +72,7 @@ func TestApplicationAdded(t *testing.T) { } time.Sleep(time.Second * 1) - assert.Equal(t, len(appWatcher.vcsToArgoMap.GetMap()), 3) + assert.Len(t, appWatcher.vcsToArgoMap.GetMap(), 3) } func TestApplicationUpdated(t *testing.T) { @@ -83,12 +85,12 @@ func TestApplicationUpdated(t *testing.T) { time.Sleep(time.Second * 1) - assert.Equal(t, len(ctrl.vcsToArgoMap.GetMap()), 2) + assert.Len(t, ctrl.vcsToArgoMap.GetMap(), 2) oldAppDirectory := ctrl.vcsToArgoMap.GetAppsInRepo("https://gitlab.com/test/repo.git") newAppDirectory := ctrl.vcsToArgoMap.GetAppsInRepo("https://gitlab.com/test/repo-3.git") - assert.Equal(t, oldAppDirectory.Count(), 1) - assert.Equal(t, newAppDirectory.Count(), 0) + assert.Equal(t, 1, oldAppDirectory.Count()) + assert.Equal(t, 0, newAppDirectory.Count()) // _, err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications("default").Update(ctx, &v1alpha1.Application{ ObjectMeta: metav1.ObjectMeta{Name: "test-app-1", Namespace: "default"}, @@ -102,8 +104,8 @@ func TestApplicationUpdated(t *testing.T) { time.Sleep(time.Second * 1) oldAppDirectory = ctrl.vcsToArgoMap.GetAppsInRepo("https://gitlab.com/test/repo.git") newAppDirectory = ctrl.vcsToArgoMap.GetAppsInRepo("https://gitlab.com/test/repo-3.git") - assert.Equal(t, oldAppDirectory.Count(), 0) - assert.Equal(t, newAppDirectory.Count(), 1) + assert.Equal(t, 0, oldAppDirectory.Count()) + assert.Equal(t, 1, newAppDirectory.Count()) } func TestApplicationDeleted(t *testing.T) { @@ -116,10 +118,10 @@ func TestApplicationDeleted(t *testing.T) { time.Sleep(time.Second * 1) - assert.Equal(t, len(ctrl.vcsToArgoMap.GetMap()), 2) + assert.Len(t, ctrl.vcsToArgoMap.GetMap(), 2) appDirectory := ctrl.vcsToArgoMap.GetAppsInRepo("https://gitlab.com/test/repo.git") - assert.Equal(t, appDirectory.Count(), 1) + assert.Equal(t, 1, appDirectory.Count()) // err := ctrl.applicationClientset.ArgoprojV1alpha1().Applications("default").Delete(ctx, "test-app-1", metav1.DeleteOptions{}) if err != nil { @@ -128,7 +130,7 @@ func TestApplicationDeleted(t *testing.T) { time.Sleep(time.Second * 1) appDirectory = ctrl.vcsToArgoMap.GetAppsInRepo("https://gitlab.com/test/repo.git") - assert.Equal(t, appDirectory.Count(), 0) + assert.Equal(t, 0, appDirectory.Count()) } // TestIsGitRepo will test various URLs against the isGitRepo function. @@ -236,7 +238,6 @@ func TestCanProcessApp(t *testing.T) { } for _, tc := range tests { - tc := tc t.Run(tc.name, func(t *testing.T) { app, canProcess := canProcessApp(tc.resource) diff --git a/pkg/app_watcher/appset_watcher.go b/pkg/app_watcher/appset_watcher.go index cc90fed6..9ed6f99d 100644 --- a/pkg/app_watcher/appset_watcher.go +++ b/pkg/app_watcher/appset_watcher.go @@ -11,14 +11,15 @@ import ( informers "github.com/argoproj/argo-cd/v2/pkg/client/informers/externalversions/application/v1alpha1" applisters "github.com/argoproj/argo-cd/v2/pkg/client/listers/application/v1alpha1" "github.com/rs/zerolog/log" - "github.com/zapier/kubechecks/pkg/appdir" - "github.com/zapier/kubechecks/pkg/config" "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/client-go/rest" "k8s.io/client-go/tools/cache" + + "github.com/zapier/kubechecks/pkg/appdir" + "github.com/zapier/kubechecks/pkg/config" ) -// ApplicationSetWatcher is the controller that watches ArgoCD Application resources via the Kubernetes API +// ApplicationSetWatcher is the controller that watches ArgoCD Application resources via the Kubernetes API. type ApplicationSetWatcher struct { applicationClientset appclientset.Interface appInformer cache.SharedIndexInformer @@ -81,7 +82,7 @@ func (ctrl *ApplicationSetWatcher) newApplicationSetInformerAndLister(refreshTim } // onAdd is the function executed when the informer notifies the -// presence of a new Application in the namespace +// presence of a new Application in the namespace. func (ctrl *ApplicationSetWatcher) onApplicationSetAdded(obj interface{}) { appSet, ok := canProcessAppSet(obj) if !ok { @@ -112,7 +113,6 @@ func (ctrl *ApplicationSetWatcher) onApplicationSetUpdated(old, new interface{}) log.Info().Str("key", key).Msg("appsetwatcher: onApplicationSetUpdated") ctrl.vcsToArgoMap.UpdateAppSet(old.(*appv1alpha1.ApplicationSet), new.(*appv1alpha1.ApplicationSet)) } - } func (ctrl *ApplicationSetWatcher) onApplicationSetDeleted(obj interface{}) { @@ -128,6 +128,7 @@ func (ctrl *ApplicationSetWatcher) onApplicationSetDeleted(obj interface{}) { log.Info().Str("key", key).Msg("appsetwatcher: onApplicationSetDeleted") ctrl.vcsToArgoMap.DeleteAppSet(app) } + func canProcessAppSet(obj interface{}) (*appv1alpha1.ApplicationSet, bool) { app, ok := obj.(*appv1alpha1.ApplicationSet) if !ok { diff --git a/pkg/app_watcher/appset_watcher_test.go b/pkg/app_watcher/appset_watcher_test.go index 6caa6c4c..12c0d375 100644 --- a/pkg/app_watcher/appset_watcher_test.go +++ b/pkg/app_watcher/appset_watcher_test.go @@ -9,12 +9,15 @@ import ( appclientsetfake "github.com/argoproj/argo-cd/v2/pkg/client/clientset/versioned/fake" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "github.com/zapier/kubechecks/pkg/appdir" "github.com/zapier/kubechecks/pkg/config" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func initTestObjectsForAppSets(t *testing.T) *ApplicationSetWatcher { + t.Helper() + cfg, err := config.New() // Handle the error appropriately, e.g., log it or fail the test require.NoError(t, err, "failed to create config") @@ -70,7 +73,7 @@ func TestApplicationSetWatcher_OnApplicationAdded(t *testing.T) { time.Sleep(time.Second * 1) - assert.Equal(t, 2, len(appWatcher.vcsToArgoMap.GetAppSetMap())) + assert.Len(t, appWatcher.vcsToArgoMap.GetAppSetMap(), 2) _, err := appWatcher.applicationClientset.ArgoprojV1alpha1().ApplicationSets("default").Create(ctx, &v1alpha1.ApplicationSet{ ObjectMeta: metav1.ObjectMeta{Name: "test-app-3", Namespace: "default"}, @@ -90,7 +93,7 @@ func TestApplicationSetWatcher_OnApplicationAdded(t *testing.T) { } time.Sleep(time.Second * 1) - assert.Equal(t, 3, len(appWatcher.vcsToArgoMap.GetAppSetMap())) + assert.Len(t, appWatcher.vcsToArgoMap.GetAppSetMap(), 3) } func TestApplicationSetWatcher_OnApplicationUpdated(t *testing.T) { @@ -103,7 +106,7 @@ func TestApplicationSetWatcher_OnApplicationUpdated(t *testing.T) { time.Sleep(time.Second * 1) - assert.Equal(t, len(ctrl.vcsToArgoMap.GetAppSetMap()), 2) + assert.Len(t, ctrl.vcsToArgoMap.GetAppSetMap(), 2) oldAppDirectory := ctrl.vcsToArgoMap.GetAppSetsInRepo("https://gitlab.com/test/repo.git") newAppDirectory := ctrl.vcsToArgoMap.GetAppSetsInRepo("https://gitlab.com/test/repo-3.git") @@ -129,8 +132,8 @@ func TestApplicationSetWatcher_OnApplicationUpdated(t *testing.T) { time.Sleep(time.Second * 1) oldAppDirectory = ctrl.vcsToArgoMap.GetAppSetsInRepo("https://gitlab.com/test/repo.git") newAppDirectory = ctrl.vcsToArgoMap.GetAppSetsInRepo("https://gitlab.com/test/repo-3.git") - assert.Equal(t, oldAppDirectory.Count(), 0) - assert.Equal(t, newAppDirectory.Count(), 1) + assert.Equal(t, 0, oldAppDirectory.Count()) + assert.Equal(t, 1, newAppDirectory.Count()) } func TestApplicationSetWatcher_OnApplicationDEleted(t *testing.T) { @@ -143,7 +146,7 @@ func TestApplicationSetWatcher_OnApplicationDEleted(t *testing.T) { time.Sleep(time.Second * 1) - assert.Equal(t, 2, len(ctrl.vcsToArgoMap.GetAppSetMap())) + assert.Len(t, ctrl.vcsToArgoMap.GetAppSetMap(), 2) appDirectory := ctrl.vcsToArgoMap.GetAppSetsInRepo("https://gitlab.com/test/repo.git") assert.Equal(t, 1, appDirectory.Count()) @@ -257,7 +260,6 @@ func Test_CanProcessAppSet(t *testing.T) { } for _, tc := range tests { - tc := tc t.Run(tc.name, func(t *testing.T) { app, canProcess := canProcessAppSet(tc.resource) diff --git a/pkg/appdir/app_directory.go b/pkg/appdir/app_directory.go index a915fe61..9b09c780 100644 --- a/pkg/appdir/app_directory.go +++ b/pkg/appdir/app_directory.go @@ -63,7 +63,7 @@ func (d *AppDirectory) ProcessApp(app v1alpha1.Application) { // // changeList: a slice of strings representing the paths of modified files. // targetBranch: the branch name to compare against the target revision of the applications. -// e.g. changeList = ["path/to/file1", "path/to/file2"] +// e.g. changeList = ["path/to/file1", "path/to/file2"]. func (d *AppDirectory) FindAppsBasedOnChangeList(changeList []string, targetBranch string) []v1alpha1.Application { log.Debug().Msgf("checking %d changes", len(changeList)) diff --git a/pkg/appdir/app_directory_test.go b/pkg/appdir/app_directory_test.go index b67f2aa8..24555115 100644 --- a/pkg/appdir/app_directory_test.go +++ b/pkg/appdir/app_directory_test.go @@ -107,6 +107,8 @@ func TestRemoveFromSlice(t *testing.T) { ints := []int{1, 2, 3, 4, 5} intsAfterRemoval := []int{1, 2, 4, 5} intsTest := func(t *testing.T) { + t.Helper() + result := removeFromSlice(ints, 3, func(a, b int) bool { return a == b }) if !reflect.DeepEqual(result, intsAfterRemoval) { t.Errorf("Expected %v, got %v", intsAfterRemoval, result) @@ -117,6 +119,8 @@ func TestRemoveFromSlice(t *testing.T) { strings := []string{"apple", "banana", "cherry", "date"} stringsAfterRemoval := []string{"apple", "cherry", "date"} stringsTest := func(t *testing.T) { + t.Helper() + result := removeFromSlice(strings, "banana", func(a, b string) bool { return a == b }) if !reflect.DeepEqual(result, stringsAfterRemoval) { t.Errorf("Expected %v, got %v", stringsAfterRemoval, result) diff --git a/pkg/appdir/appset_directory.go b/pkg/appdir/appset_directory.go index e9059371..12210dcd 100644 --- a/pkg/appdir/appset_directory.go +++ b/pkg/appdir/appset_directory.go @@ -8,8 +8,9 @@ import ( "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/rs/zerolog/log" - "github.com/zapier/kubechecks/pkg/git" "sigs.k8s.io/yaml" + + "github.com/zapier/kubechecks/pkg/git" ) type AppSetDirectory struct { @@ -172,7 +173,7 @@ func (d *AppSetDirectory) RemoveApp(app v1alpha1.ApplicationSet) { } } -// containsKindApplicationSet checks if the file contains kind: ApplicationSet +// containsKindApplicationSet checks if the file contains kind: ApplicationSet. func containsKindApplicationSet(path string) bool { file, err := os.Open(path) if err != nil { diff --git a/pkg/appdir/appset_directory_test.go b/pkg/appdir/appset_directory_test.go index b6aaf6d1..26a5cd24 100644 --- a/pkg/appdir/appset_directory_test.go +++ b/pkg/appdir/appset_directory_test.go @@ -7,13 +7,13 @@ import ( "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/stretchr/testify/assert" - "github.com/zapier/kubechecks/pkg/git" v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + "github.com/zapier/kubechecks/pkg/git" ) func TestAppSetDirectory_ProcessApp(t *testing.T) { - type args struct { app v1alpha1.ApplicationSet } @@ -69,7 +69,6 @@ func TestAppSetDirectory_ProcessApp(t *testing.T) { } func TestAppSetDirectory_FindAppsBasedOnChangeList(t *testing.T) { - tests := []struct { name string changeList []string @@ -196,12 +195,12 @@ spec: for fileName, content := range tt.mockFiles { absPath := filepath.Join(tempDir, fileName) cleanUpDirs = append(cleanUpDirs, absPath) - err := os.MkdirAll(filepath.Dir(absPath), 0755) + err := os.MkdirAll(filepath.Dir(absPath), 0o755) if err != nil { fatalErr = err break } - err = os.WriteFile(absPath, []byte(content), 0644) + err = os.WriteFile(absPath, []byte(content), 0o600) if err != nil { fatalErr = err break @@ -218,8 +217,10 @@ spec: } } -// cleanUpTmpFiles removes the temporary directories created for the test +// cleanUpTmpFiles removes the temporary directories created for the test. func cleanUpTmpFiles(t *testing.T, cleanUpDirs []string) { + t.Helper() + for _, dir := range cleanUpDirs { if err := os.RemoveAll(filepath.Dir(dir)); err != nil { t.Fatalf("failed to remove tmp folder %s", err) diff --git a/pkg/appdir/vcstoargomap.go b/pkg/appdir/vcstoargomap.go index 0becae94..4bf65aef 100644 --- a/pkg/appdir/vcstoargomap.go +++ b/pkg/appdir/vcstoargomap.go @@ -5,6 +5,7 @@ import ( "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/rs/zerolog/log" + "github.com/zapier/kubechecks/pkg" ) diff --git a/pkg/appdir/vcstoargomap_test.go b/pkg/appdir/vcstoargomap_test.go index 95a5d088..1be50536 100644 --- a/pkg/appdir/vcstoargomap_test.go +++ b/pkg/appdir/vcstoargomap_test.go @@ -26,8 +26,8 @@ func TestAddApp(t *testing.T) { v2a.AddApp(app1) appDir := v2a.GetAppsInRepo("https://github.com/argoproj/argo-cd.git") - assert.Equal(t, appDir.Count(), 1) - assert.Equal(t, len(appDir.appDirs["test-app-1"]), 1) + assert.Equal(t, 1, appDir.Count()) + assert.Len(t, appDir.appDirs["test-app-1"], 1) // Assertions to verify the behavior here. app2 := &v1alpha1.Application{ @@ -41,8 +41,8 @@ func TestAddApp(t *testing.T) { } v2a.AddApp(app2) - assert.Equal(t, appDir.Count(), 2) - assert.Equal(t, len(appDir.appDirs["test-app-2"]), 1) + assert.Equal(t, 2, appDir.Count()) + assert.Len(t, appDir.appDirs["test-app-2"], 1) } func TestDeleteApp(t *testing.T) { @@ -73,13 +73,13 @@ func TestDeleteApp(t *testing.T) { v2a.AddApp(app2) appDir := v2a.GetAppsInRepo("https://github.com/argoproj/argo-cd.git") - assert.Equal(t, appDir.Count(), 2) - assert.Equal(t, len(appDir.appDirs["test-app-1"]), 1) - assert.Equal(t, len(appDir.appDirs["test-app-2"]), 1) + assert.Equal(t, 2, appDir.Count()) + assert.Len(t, appDir.appDirs["test-app-1"], 1) + assert.Len(t, appDir.appDirs["test-app-2"], 1) v2a.DeleteApp(app2) - assert.Equal(t, appDir.Count(), 1) - assert.Equal(t, len(appDir.appDirs["test-app-2"]), 0) + assert.Equal(t, 1, appDir.Count()) + assert.Empty(t, appDir.appDirs["test-app-2"]) } func TestVcsToArgoMap_AddAppSet(t *testing.T) { @@ -137,7 +137,7 @@ func TestVcsToArgoMap_AddAppSet(t *testing.T) { appSetDirByRepo: tt.fields.appSetDirByRepo, } v2a.AddAppSet(tt.args.app) - assert.Equal(t, tt.expectedCount, len(v2a.appSetDirByRepo)) + assert.Len(t, v2a.appSetDirByRepo, tt.expectedCount) }) } } @@ -178,11 +178,11 @@ func TestVcsToArgoMap_DeleteAppSet(t *testing.T) { v2a.AddAppSet(app2) appDir := v2a.GetAppSetsInRepo("https://github.com/argoproj/argo-cd.git") - assert.Equal(t, appDir.Count(), 2) - assert.Equal(t, len(appDir.appSetDirs["test-app-1"]), 1) - assert.Equal(t, len(appDir.appSetDirs["test-app-2"]), 1) + assert.Equal(t, 2, appDir.Count()) + assert.Len(t, appDir.appSetDirs["test-app-1"], 1) + assert.Len(t, appDir.appSetDirs["test-app-2"], 1) v2a.DeleteAppSet(app2) - assert.Equal(t, appDir.Count(), 1) - assert.Equal(t, len(appDir.appSetDirs["test-app-2"]), 0) + assert.Equal(t, 1, appDir.Count()) + assert.Empty(t, appDir.appSetDirs["test-app-2"]) } diff --git a/pkg/appdir/walk_kustomize_files_test.go b/pkg/appdir/walk_kustomize_files_test.go index c5b77fa3..5946a304 100644 --- a/pkg/appdir/walk_kustomize_files_test.go +++ b/pkg/appdir/walk_kustomize_files_test.go @@ -41,7 +41,8 @@ resources: - ./overlays/dev - /common/overlays/prod - https://google.com/some/url -`)}, +`), + }, "test/app2/kustomization.yaml": { Data: toBytes(` @@ -55,14 +56,16 @@ resources: - file1.yaml - ../overlays/base - /common/overlays/prod -`)}, +`), + }, "test/overlays/base/kustomization.yaml": { Data: toBytes(` resources: - some-file1.yaml - some-file2.yaml - ../common -`)}, +`), + }, "test/overlays/common/kustomization.yaml": {Data: toBytes("hello: world")}, "test/app/file1.yaml": {Data: toBytes("hello: world")}, diff --git a/pkg/argo_client/applications.go b/pkg/argo_client/applications.go index 8694a555..3c6b2496 100644 --- a/pkg/argo_client/applications.go +++ b/pkg/argo_client/applications.go @@ -2,17 +2,15 @@ package argo_client import ( "context" - "fmt" "strings" - "github.com/argoproj/argo-cd/v2/pkg/apiclient/applicationset" - "github.com/rs/zerolog/log" - "go.opentelemetry.io/otel" - "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" + "github.com/argoproj/argo-cd/v2/pkg/apiclient/applicationset" "github.com/argoproj/argo-cd/v2/pkg/apiclient/cluster" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/pkg/errors" + "github.com/rs/zerolog/log" + "go.opentelemetry.io/otel" "github.com/zapier/kubechecks/telemetry" ) @@ -34,7 +32,7 @@ func (argo *ArgoClient) GetApplicationByName(ctx context.Context, name string) ( resp, err := appClient.Get(ctx, &application.ApplicationQuery{Name: &name}) if err != nil { telemetry.SetError(span, err, "Argo Get Application error") - return nil, fmt.Errorf("failed to retrieve the application: %v", err) + return nil, errors.Wrap(err, "failed to retrieve the application") } return resp, nil @@ -65,7 +63,7 @@ func (argo *ArgoClient) GetKubernetesVersionByApplication(ctx context.Context, a clusterResponse, err := clusterClient.Get(ctx, clusterRequest) if err != nil { telemetry.SetError(span, err, "Argo Get Cluster error") - return "", fmt.Errorf("failed to retrieve the destination Kubernetes cluster: %v", err) + return "", errors.Wrap(err, "failed to retrieve the destination Kubernetes cluster") } // Get Kubernetes version @@ -95,7 +93,7 @@ func (argo *ArgoClient) GetApplicationsByLabels(ctx context.Context, labels stri resp, err := appClient.List(ctx, &application.ApplicationQuery{Selector: &labels}) if err != nil { telemetry.SetError(span, err, "Argo List Application error") - return nil, fmt.Errorf("failed to retrieve applications from labels: %v", err) + return nil, errors.Wrap(err, "failed to retrieve applications from labels") } return resp, nil diff --git a/pkg/argo_client/client.go b/pkg/argo_client/client.go index ac3af7c9..ca609cf8 100644 --- a/pkg/argo_client/client.go +++ b/pkg/argo_client/client.go @@ -7,11 +7,10 @@ import ( "github.com/argoproj/argo-cd/v2/pkg/apiclient" "github.com/argoproj/argo-cd/v2/pkg/apiclient/application" "github.com/argoproj/argo-cd/v2/pkg/apiclient/applicationset" + "github.com/argoproj/argo-cd/v2/pkg/apiclient/cluster" "github.com/argoproj/argo-cd/v2/pkg/apiclient/settings" "github.com/rs/zerolog/log" - "github.com/argoproj/argo-cd/v2/pkg/apiclient/cluster" - "github.com/zapier/kubechecks/pkg/config" ) diff --git a/pkg/checks/diff/diff.go b/pkg/checks/diff/diff.go index 68391a60..0fdbcf76 100644 --- a/pkg/checks/diff/diff.go +++ b/pkg/checks/diff/diff.go @@ -22,6 +22,7 @@ import ( "github.com/argoproj/gitops-engine/pkg/utils/kube" "github.com/ghodss/yaml" "github.com/go-logr/zerologr" + "github.com/pkg/errors" "github.com/pmezard/go-difflib/difflib" "github.com/rs/zerolog/log" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" @@ -72,27 +73,27 @@ func Check(ctx context.Context, request checks.Request) (msg.Result, error) { argoSettings, err := getArgoSettings(ctx, request) if err != nil { - return msg.Result{}, err + return msg.Result{}, errors.Wrap(err, "failed to get Argo settings") } resources, err := getResources(ctx, request) if err != nil { - return msg.Result{}, err + return msg.Result{}, errors.Wrap(err, "failed to get resources") } liveObjs, err := cmdutil.LiveObjects(resources) if err != nil { telemetry.SetError(span, err, "Get Argo Live Objects") - return msg.Result{}, err + return msg.Result{}, errors.Wrap(err, "failed to get live objects") } groupedObjs, err := groupObjsByKey(unstructureds, liveObjs, app.Spec.Destination.Namespace) if err != nil { - return msg.Result{}, err + return msg.Result{}, errors.Wrap(err, "failed to group objects by key") } if items, err = groupObjsForDiff(resources, groupedObjs, items, argoSettings, app.Name); err != nil { - return msg.Result{}, err + return msg.Result{}, errors.Wrap(err, "failed to group objects for diff") } var diffBuffer strings.Builder @@ -107,13 +108,13 @@ func Check(ctx context.Context, request checks.Request) (msg.Result, error) { diffRes, err := generateDiff(ctx, request, argoSettings, item) if err != nil { - return msg.Result{}, err + return msg.Result{}, errors.Wrap(err, "failed to generate diff") } if diffRes.Modified || item.target == nil || item.live == nil { err := addResourceDiffToMessage(ctx, &diffBuffer, resourceId, item, diffRes) if err != nil { - return msg.Result{}, err + return msg.Result{}, errors.Wrap(err, "failed to add diff to message") } processResources(item, diffRes, request, &added, &modified, &removed) @@ -305,7 +306,7 @@ func groupObjsByKey(localObs []*unstructured.Unstructured, liveObjs []*unstructu func groupObjsForDiff(resources []*argoappv1.ResourceDiff, objs map[kube.ResourceKey]*unstructured.Unstructured, items []objKeyLiveTarget, argoSettings *settings.Settings, appName string) ([]objKeyLiveTarget, error) { resourceTracking := argo.NewResourceTracking() for _, res := range resources { - var live = &unstructured.Unstructured{} + live := &unstructured.Unstructured{} if err := json.Unmarshal([]byte(res.NormalizedLiveState), &live); err != nil { return nil, err } @@ -351,7 +352,7 @@ func (p *resourceInfoProvider) IsNamespaced(gk schema.GroupKind) (bool, error) { } // PrintDiff prints a diff between two unstructured objects to stdout using an external diff utility -// Honors the diff utility set in the KUBECTL_EXTERNAL_DIFF environment variable +// Honors the diff utility set in the KUBECTL_EXTERNAL_DIFF environment variable. func PrintDiff(w io.Writer, live *unstructured.Unstructured, target *unstructured.Unstructured) error { var err error targetData := []byte("") @@ -370,7 +371,7 @@ func PrintDiff(w io.Writer, live *unstructured.Unstructured, target *unstructure } } - diff := difflib.UnifiedDiff{ + theDiff := difflib.UnifiedDiff{ A: difflib.SplitLines(string(liveData)), B: difflib.SplitLines(string(targetData)), // FromFile: "Original", @@ -378,7 +379,7 @@ func PrintDiff(w io.Writer, live *unstructured.Unstructured, target *unstructure Context: 2, } - return difflib.WriteUnifiedDiff(w, diff) - //return difflib.GetUnifiedDiffString(diff) + return difflib.WriteUnifiedDiff(w, theDiff) + // return difflib.GetUnifiedDiffString(diff) // return dmp.DiffPrettyText(diff), nil } diff --git a/pkg/checks/hooks/check.go b/pkg/checks/hooks/check.go index c22e45ae..574f2937 100644 --- a/pkg/checks/hooks/check.go +++ b/pkg/checks/hooks/check.go @@ -135,7 +135,7 @@ func phasesAndWaves(obj *unstructured.Unstructured) ([]hookInfo, error) { } for hookType := range hookTypes(obj) { - hookInfos = append(hookInfos, hookInfo{hookType: hookType, hookWave: waveNum(syncWave)}) + hookInfos = append(hookInfos, hookInfo{hookType: hookType, hookWave: waveNum(syncWave)}) //nolint:gosec } return hookInfos, nil diff --git a/pkg/checks/hooks/check_test.go b/pkg/checks/hooks/check_test.go index 853247dc..d014dfd4 100644 --- a/pkg/checks/hooks/check_test.go +++ b/pkg/checks/hooks/check_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/zapier/kubechecks/pkg" "github.com/zapier/kubechecks/pkg/checks" ) diff --git a/pkg/checks/kubeconform/validate.go b/pkg/checks/kubeconform/validate.go index 71439ef3..1d8b1a5d 100644 --- a/pkg/checks/kubeconform/validate.go +++ b/pkg/checks/kubeconform/validate.go @@ -20,7 +20,7 @@ import ( var tracer = otel.Tracer("pkg/checks/kubeconform") -func getSchemaLocations(ctx context.Context, ctr container.Container, tempRepoPath string) []string { +func getSchemaLocations(ctr container.Container, tempRepoPath string) []string { cfg := ctr.Config locations := []string{ @@ -87,12 +87,12 @@ func argoCdAppValidate(ctx context.Context, ctr container.Container, appName, ta KubernetesVersion: targetKubernetesVersion, Strict: true, IgnoreMissingSchemas: false, - Debug: log.Debug().Enabled(), + Debug: log.Debug().Enabled(), //nolint: zerologlint } var ( outputString []string - schemaLocations = getSchemaLocations(ctx, ctr, tempRepoPath) + schemaLocations = getSchemaLocations(ctr, tempRepoPath) ) log.Debug().Msgf("cache location: %s", vOpts.Cache) @@ -101,7 +101,7 @@ func argoCdAppValidate(ctx context.Context, ctr container.Container, appName, ta v, err := validator.New(schemaLocations, vOpts) if err != nil { - return msg.Result{}, fmt.Errorf("could not create kubeconform validator: %v", err) + return msg.Result{}, errors.Wrap(err, "could not create kubeconform validator") } result := v.Validate("-", io.NopCloser(strings.NewReader(strings.Join(appManifests, "\n")))) var invalid, failedValidation bool @@ -127,11 +127,12 @@ func argoCdAppValidate(ctx context.Context, ctr container.Container, appName, ta } var cr msg.Result - if invalid { + switch { + case invalid: cr.State = pkg.StateWarning - } else if failedValidation { + case failedValidation: cr.State = pkg.StateFailure - } else { + default: cr.State = pkg.StateSuccess } diff --git a/pkg/checks/kubeconform/validate_test.go b/pkg/checks/kubeconform/validate_test.go index bd502b01..4f16027b 100644 --- a/pkg/checks/kubeconform/validate_test.go +++ b/pkg/checks/kubeconform/validate_test.go @@ -1,24 +1,20 @@ package kubeconform import ( - "context" - "fmt" "os" "strings" "testing" fixtures "github.com/go-git/go-git-fixtures/v4" "github.com/spf13/viper" - "github.com/stretchr/testify/assert" "github.com/zapier/kubechecks/pkg/container" ) func TestDefaultGetSchemaLocations(t *testing.T) { - ctx := context.TODO() ctr := container.Container{} - schemaLocations := getSchemaLocations(ctx, ctr, "/some/other/path") + schemaLocations := getSchemaLocations(ctr, "/some/other/path") // default schema location is "./schemas" assert.Len(t, schemaLocations, 1) @@ -26,7 +22,6 @@ func TestDefaultGetSchemaLocations(t *testing.T) { } func TestGetRemoteSchemaLocations(t *testing.T) { - ctx := context.TODO() ctr := container.Container{} if os.Getenv("CI") == "" { @@ -35,11 +30,10 @@ func TestGetRemoteSchemaLocations(t *testing.T) { basic := fixtures.Basic() fixture := basic.One() - fmt.Println(fixture.URL) // t.Setenv("KUBECHECKS_SCHEMAS_LOCATION", fixture.URL) // doesn't work because viper needs to initialize from root, which doesn't happen viper.Set("schemas-location", []string{fixture.URL}) - schemaLocations := getSchemaLocations(ctx, ctr, "/some/other/path") + schemaLocations := getSchemaLocations(ctr, "/some/other/path") hasTmpDirPrefix := strings.HasPrefix(schemaLocations[0], "/tmp/schemas") - assert.Equal(t, hasTmpDirPrefix, true, "invalid schemas location. Schema location should have prefix /tmp/schemas but has %s", schemaLocations[0]) + assert.Truef(t, hasTmpDirPrefix, "invalid schemas location. Schema location should have prefix /tmp/schemas but has %s", schemaLocations[0]) } diff --git a/pkg/checks/preupgrade/kubepug.go b/pkg/checks/preupgrade/kubepug.go index 6e6dc2d1..0f6f30cc 100644 --- a/pkg/checks/preupgrade/kubepug.go +++ b/pkg/checks/preupgrade/kubepug.go @@ -40,14 +40,14 @@ func checkApp(ctx context.Context, appName, targetKubernetesVersion string, mani tempDir, err := os.MkdirTemp("/tmp", "kubechecks-kubepug") if err != nil { logger.Error().Err(err).Msg("could not create temp directory to write manifests for kubepug check") - //return "", err + // return "", err return msg.Result{}, err } defer os.RemoveAll(tempDir) for i, manifest := range manifests { tmpFile := fmt.Sprintf("%s/%b.yaml", tempDir, i) - if err = os.WriteFile(tmpFile, []byte(manifest), 0666); err != nil { + if err = os.WriteFile(tmpFile, []byte(manifest), 0o600); err != nil { logger.Error().Err(err).Str("path", tmpFile).Msg("failed to write file") } } @@ -71,7 +71,6 @@ func checkApp(ctx context.Context, appName, targetKubernetesVersion string, mani } if len(result.DeprecatedAPIs) > 0 || len(result.DeletedAPIs) > 0 { - if len(result.DeprecatedAPIs) > 0 { outputString = append(outputString, "\n\n**Deprecated APIs**\n") buff := &bytes.Buffer{} @@ -117,7 +116,6 @@ func checkApp(ctx context.Context, appName, targetKubernetesVersion string, mani table.Render() outputString = append(outputString, buff.String()) } - } else { outputString = append(outputString, "No Deprecated or Deleted APIs found.") } diff --git a/pkg/checks/rego/check.go b/pkg/checks/rego/check.go index 53b02119..4e050171 100644 --- a/pkg/checks/rego/check.go +++ b/pkg/checks/rego/check.go @@ -48,9 +48,11 @@ func NewChecker(cfg config.ServerConfig) (*Checker, error) { return &c, nil } -var ErrResourceMustHaveKind = errors.New("resource does not have kind") -var ErrResourceMustHaveMetadata = errors.New("resource does not have metadata") -var ErrResourceMustHaveName = errors.New("resource does not have name") +var ( + ErrResourceMustHaveKind = errors.New("resource does not have kind") + ErrResourceMustHaveMetadata = errors.New("resource does not have metadata") + ErrResourceMustHaveName = errors.New("resource does not have name") +) func getFilenameFromRawManifest(manifest string) (string, error) { resource := make(map[string]interface{}) @@ -105,7 +107,7 @@ func dumpFiles(manifests []string) (string, error) { Int("size", len(manifestBytes)). Msg("dumping manifest") - if err = os.WriteFile(fullPath, manifestBytes, 0o666); err != nil { + if err = os.WriteFile(fullPath, manifestBytes, 0o600); err != nil { return result, errors.Wrapf(err, "failed to write %s", filename) } } @@ -177,11 +179,12 @@ func (c *Checker) Check(ctx context.Context, request checks.Request) (msg.Result } var cr msg.Result - if failures { + switch { + case failures: cr.State = pkg.StateFailure - } else if warnings { + case warnings: cr.State = pkg.StateWarning - } else { + default: cr.State = pkg.StateSuccess } diff --git a/pkg/checks/rego/check_test.go b/pkg/checks/rego/check_test.go index a4797541..7e940f53 100644 --- a/pkg/checks/rego/check_test.go +++ b/pkg/checks/rego/check_test.go @@ -18,7 +18,9 @@ import ( ) func mustWrite(t *testing.T, filePath, content string) { - err := os.WriteFile(filePath, []byte(content), 0o666) + t.Helper() + + err := os.WriteFile(filePath, []byte(content), 0o600) require.NoError(t, err) } @@ -149,8 +151,9 @@ warn[msg] { } for _, tc := range testcases { - tc := tc t.Run(tc.name, func(t *testing.T) { + t.Parallel() + policiesPath, err := os.MkdirTemp("", "kubechecks-test-policies-") require.NoError(t, err) diff --git a/pkg/commitState.go b/pkg/commitState.go index 3da4de1a..67577621 100644 --- a/pkg/commitState.go +++ b/pkg/commitState.go @@ -5,10 +5,10 @@ import ( "strings" ) -// CommitState is an enum for represnting the state of a commit for posting via CommitStatus +// CommitState is an enum for represnting the state of a commit for posting via CommitStatus. type CommitState uint8 -// must be in order of best to worst, in order for WorstState to work +// must be in order of best to worst, in order for WorstState to work. const ( StateNone CommitState = iota StateSkip @@ -64,6 +64,5 @@ func ParseCommitState(s string) (CommitState, error) { return StatePanic, nil default: return StateNone, fmt.Errorf("unknown commit state: %s", s) - } } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 2227f484..0650b51a 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -23,8 +23,8 @@ func TestNew(t *testing.T) { cfg, err := NewWithViper(v) require.NoError(t, err) assert.Equal(t, zerolog.InfoLevel, cfg.LogLevel) - assert.Equal(t, true, cfg.ArgoCDInsecure) - assert.Equal(t, true, cfg.ArgoCDPlainText) + assert.True(t, cfg.ArgoCDInsecure) + assert.True(t, cfg.ArgoCDPlainText) assert.Equal(t, pkg.StateWarning, cfg.WorstConfTestState, "worst states can be overridden") assert.Equal(t, time.Minute*10, cfg.RepoRefreshInterval) } diff --git a/pkg/container/main.go b/pkg/container/main.go index a330af3f..6bb25d38 100644 --- a/pkg/container/main.go +++ b/pkg/container/main.go @@ -5,7 +5,6 @@ import ( "io/fs" "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - client "github.com/zapier/kubechecks/pkg/kubernetes" "github.com/zapier/kubechecks/pkg" "github.com/zapier/kubechecks/pkg/app_watcher" @@ -13,6 +12,7 @@ import ( "github.com/zapier/kubechecks/pkg/argo_client" "github.com/zapier/kubechecks/pkg/config" "github.com/zapier/kubechecks/pkg/git" + client "github.com/zapier/kubechecks/pkg/kubernetes" "github.com/zapier/kubechecks/pkg/vcs" ) diff --git a/pkg/events/check.go b/pkg/events/check.go index 54b54bdf..225910eb 100644 --- a/pkg/events/check.go +++ b/pkg/events/check.go @@ -13,8 +13,6 @@ import ( "github.com/pkg/errors" "github.com/rs/zerolog" "github.com/rs/zerolog/log" - "github.com/zapier/kubechecks/pkg/generator" - "github.com/zapier/kubechecks/pkg/repo_config" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -22,8 +20,10 @@ import ( "github.com/zapier/kubechecks/pkg/affected_apps" "github.com/zapier/kubechecks/pkg/checks" "github.com/zapier/kubechecks/pkg/container" + "github.com/zapier/kubechecks/pkg/generator" "github.com/zapier/kubechecks/pkg/git" "github.com/zapier/kubechecks/pkg/msg" + "github.com/zapier/kubechecks/pkg/repo_config" "github.com/zapier/kubechecks/pkg/vcs" "github.com/zapier/kubechecks/telemetry" ) @@ -77,7 +77,6 @@ func GenerateMatcher(ce *CheckEvent, repo *git.Repo) error { } func NewCheckEvent(pullRequest vcs.PullRequest, ctr container.Container, repoManager repoManager, processors []checks.ProcessorEntry) *CheckEvent { - ce := &CheckEvent{ addedAppsSet: make(map[string]v1alpha1.Application), appChannel: make(chan *v1alpha1.Application, ctr.Config.MaxQueueSize), @@ -258,7 +257,7 @@ func (ce *CheckEvent) Process(ctx context.Context) error { ce.logger.Error().Err(err).Msg("Failed to tidy outdated comments") } - if len(ce.affectedItems.Applications) <= 0 && len(ce.affectedItems.ApplicationSets) <= 0 { + if len(ce.affectedItems.Applications) == 0 && len(ce.affectedItems.ApplicationSets) == 0 { ce.logger.Info().Msg("No affected apps or appsets, skipping") ce.ctr.VcsClient.PostMessage(ctx, ce.pullRequest, "No changes") return nil @@ -268,7 +267,6 @@ func (ce *CheckEvent) Process(ctx context.Context) error { ce.vcsNote = ce.createNote(ctx) for num := 0; num <= ce.ctr.Config.MaxConcurrenctChecks; num++ { - w := worker{ appChannel: ce.appChannel, ctr: ce.ctr, @@ -354,7 +352,7 @@ func (ce *CheckEvent) queueApp(app v1alpha1.Application) { } // CommitStatus sets the commit status on the MR -// To set the PR/MR status +// To set the PR/MR status. func (ce *CheckEvent) CommitStatus(ctx context.Context, status pkg.CommitState) { _, span := tracer.Start(ctx, "CommitStatus") defer span.End() @@ -375,7 +373,7 @@ Check kubechecks application logs for more information. ` ) -// Creates a generic Note struct that we can write into across all worker threads +// Creates a generic Note struct that we can write into across all worker threads. func (ce *CheckEvent) createNote(ctx context.Context) *msg.Message { ctx, span := otel.Tracer("check").Start(ctx, "createNote") defer span.End() diff --git a/pkg/events/check_test.go b/pkg/events/check_test.go index ff7505b4..a651184b 100644 --- a/pkg/events/check_test.go +++ b/pkg/events/check_test.go @@ -11,6 +11,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + affectedappsmocks "github.com/zapier/kubechecks/mocks/affected_apps/mocks" generatorsmocks "github.com/zapier/kubechecks/mocks/generator/mocks" "github.com/zapier/kubechecks/pkg/affected_apps" @@ -21,7 +23,6 @@ import ( "github.com/zapier/kubechecks/pkg/git" "github.com/zapier/kubechecks/pkg/msg" "github.com/zapier/kubechecks/pkg/vcs" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // TestCleanupGetManifestsError tests the cleanupGetManifestsError function. @@ -153,7 +154,6 @@ func TestCheckEvent_GenerateListOfAffectedApps(t *testing.T) { matcher affected_apps.Matcher } type args struct { - ctx context.Context repo *git.Repo targetBranch string initMatcherFn MatcherFn @@ -195,7 +195,6 @@ func TestCheckEvent_GenerateListOfAffectedApps(t *testing.T) { }), }, args: args{ - ctx: context.Background(), repo: &git.Repo{Directory: "/tmp"}, targetBranch: "HEAD", initMatcherFn: MockInitMatcherFn(), @@ -225,7 +224,6 @@ func TestCheckEvent_GenerateListOfAffectedApps(t *testing.T) { }), }, args: args{ - ctx: context.Background(), repo: &git.Repo{Directory: "/tmp"}, targetBranch: "HEAD", initMatcherFn: MockInitMatcherFn(), @@ -262,7 +260,6 @@ func TestCheckEvent_GenerateListOfAffectedApps(t *testing.T) { }), }, args: args{ - ctx: context.Background(), repo: &git.Repo{Directory: "/tmp"}, targetBranch: "HEAD", initMatcherFn: MockInitMatcherFn(), @@ -289,8 +286,7 @@ func TestCheckEvent_GenerateListOfAffectedApps(t *testing.T) { generator: tt.fields.generator, matcher: tt.fields.matcher, } - tt.wantErr(t, ce.GenerateListOfAffectedApps(tt.args.ctx, tt.args.repo, tt.args.targetBranch, tt.args.initMatcherFn), fmt.Sprintf("GenerateListOfAffectedApps(%v, %v, %v, %v)", tt.args.ctx, tt.args.repo, tt.args.targetBranch, tt.args.initMatcherFn)) - + tt.wantErr(t, ce.GenerateListOfAffectedApps(context.Background(), tt.args.repo, tt.args.targetBranch, tt.args.initMatcherFn), fmt.Sprintf("GenerateListOfAffectedApps(%v, %v, %v, %v)", context.Background(), tt.args.repo, tt.args.targetBranch, tt.args.initMatcherFn)) }) } } @@ -308,6 +304,7 @@ func MockGenerator(methodName string, returns []interface{}) generator.AppsGener return mockClient } + func MockInitMatcherFn() MatcherFn { return func(ce *CheckEvent, repo *git.Repo) error { return nil diff --git a/pkg/events/worker.go b/pkg/events/worker.go index 58f0e47b..c1e87628 100644 --- a/pkg/events/worker.go +++ b/pkg/events/worker.go @@ -5,11 +5,11 @@ import ( "fmt" "sync/atomic" + "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" + "github.com/rs/zerolog" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/trace" - "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - "github.com/rs/zerolog" "github.com/zapier/kubechecks/pkg" "github.com/zapier/kubechecks/pkg/argo_client" "github.com/zapier/kubechecks/pkg/checks" @@ -31,7 +31,7 @@ type worker struct { queueApp, removeApp func(application v1alpha1.Application) } -// process apps +// process apps. func (w *worker) run(ctx context.Context) { for app := range w.appChannel { if app != nil { diff --git a/pkg/generator/applicationsets.go b/pkg/generator/applicationsets.go index 9e23c827..5e771dbd 100644 --- a/pkg/generator/applicationsets.go +++ b/pkg/generator/applicationsets.go @@ -7,30 +7,30 @@ import ( "github.com/argoproj/argo-cd/v2/applicationset/utils" argov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - "github.com/zapier/kubechecks/pkg/container" + "github.com/rs/zerolog/log" "k8s.io/apimachinery/pkg/util/strategicpatch" "k8s.io/client-go/kubernetes" "sigs.k8s.io/controller-runtime/pkg/client" + + "github.com/zapier/kubechecks/pkg/container" ) func New() AppsGenerator { return &gen{} } -type gen struct { -} +type gen struct{} type AppsGenerator interface { GenerateApplicationSetApps(ctx context.Context, appset argov1alpha1.ApplicationSet, ctr *container.Container) ([]argov1alpha1.Application, error) } func (c *gen) GenerateApplicationSetApps(ctx context.Context, appset argov1alpha1.ApplicationSet, ctr *container.Container) ([]argov1alpha1.Application, error) { - appSetGenerators := getGenerators(ctx, *ctr.KubeClientSet.ControllerClient(), ctr.KubeClientSet.ClientSet(), ctr.Config.ArgoCDNamespace) apps, appsetReason, err := generateApplications(appset, appSetGenerators) if err != nil { - fmt.Printf("error generating applications: %v, appset reason: %v", err, appsetReason) + log.Error().Err(err).Msgf("error generating applications, appset reason: %v", appsetReason) return nil, fmt.Errorf("error generating applications: %w", err) } return apps, nil @@ -38,9 +38,8 @@ func (c *gen) GenerateApplicationSetApps(ctx context.Context, appset argov1alpha // GetGenerators returns the generators that will be used to generate applications for the ApplicationSet // -// only support List and Clusters generators +// only support List and Clusters generators. func getGenerators(ctx context.Context, c client.Client, k8sClient kubernetes.Interface, namespace string) map[string]Generator { - terminalGenerators := map[string]Generator{ "List": NewListGenerator(), "Clusters": NewClusterGenerator(c, ctx, k8sClient, namespace), @@ -62,7 +61,7 @@ func getGenerators(ctx context.Context, c client.Client, k8sClient kubernetes.In return topLevelGenerators } -// generateApplications generates applications from the ApplicationSet +// generateApplications generates applications from the ApplicationSet. func generateApplications(applicationSetInfo argov1alpha1.ApplicationSet, g map[string]Generator) ( []argov1alpha1.Application, argov1alpha1.ApplicationSetReasonType, error, ) { @@ -87,7 +86,7 @@ func generateApplications(applicationSetInfo argov1alpha1.ApplicationSet, g map[ for _, p := range a.Params { app, err := renderer.RenderTemplateParams(tmplApplication, applicationSetInfo.Spec.SyncPolicy, p, applicationSetInfo.Spec.GoTemplate, applicationSetInfo.Spec.GoTemplateOptions) if err != nil { - //logCtx.WithError(err).WithField("params", a.Params).WithField("generator", requestedGenerator). + // logCtx.WithError(err).WithField("params", a.Params).WithField("generator", requestedGenerator). // Error("error generating application from params") if firstError == nil { @@ -117,8 +116,8 @@ func generateApplications(applicationSetInfo argov1alpha1.ApplicationSet, g map[ } } - //logCtx.WithField("generator", requestedGenerator).Infof("generated %d applications", len(res)) - //logCtx.WithField("generator", requestedGenerator).Debugf("apps from generator: %+v", res) + // logCtx.WithField("generator", requestedGenerator).Infof("generated %d applications", len(res)) + // logCtx.WithField("generator", requestedGenerator).Debugf("apps from generator: %+v", res) } return res, applicationSetReason, firstError @@ -147,14 +146,12 @@ func getTempApplication(applicationSetTemplate argov1alpha1.ApplicationSetTempla } func applyTemplatePatch(app *argov1alpha1.Application, templatePatch string) (*argov1alpha1.Application, error) { - appString, err := json.Marshal(app) if err != nil { return nil, fmt.Errorf("error while marhsalling Application %w", err) } convertedTemplatePatch, err := utils.ConvertYAMLToJSON(templatePatch) - if err != nil { return nil, fmt.Errorf("error while converting template to json %q: %w", convertedTemplatePatch, err) } @@ -164,7 +161,6 @@ func applyTemplatePatch(app *argov1alpha1.Application, templatePatch string) (*a } data, err := strategicpatch.StrategicMergePatch(appString, []byte(convertedTemplatePatch), argov1alpha1.Application{}) - if err != nil { return nil, fmt.Errorf("error while applying templatePatch template to json %q: %w", convertedTemplatePatch, err) } diff --git a/pkg/generator/cluster.go b/pkg/generator/cluster.go index c2b38778..868704dc 100644 --- a/pkg/generator/cluster.go +++ b/pkg/generator/cluster.go @@ -5,21 +5,18 @@ import ( "fmt" "time" - "github.com/rs/zerolog/log" - + "github.com/argoproj/argo-cd/v2/applicationset/utils" + argoappsetv1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/argoproj/argo-cd/v2/util/settings" - + "github.com/rs/zerolog/log" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes" "sigs.k8s.io/controller-runtime/pkg/client" - - "github.com/argoproj/argo-cd/v2/applicationset/utils" - argoappsetv1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" ) const ( - ArgoCDSecretTypeLabel = "argocd.argoproj.io/secret-type" + ArgoCDSecretTypeLabel = "argocd.argoproj.io/secret-type" //nolint:gosec ArgoCDSecretTypeCluster = "cluster" ) @@ -28,7 +25,7 @@ var _ Generator = (*ClusterGenerator)(nil) // ClusterGenerator generates Applications for some or all clusters registered with ArgoCD. type ClusterGenerator struct { client.Client - ctx context.Context + ctx context.Context //nolint:containedctx clientset kubernetes.Interface // namespace is the Argo CD namespace namespace string @@ -38,7 +35,6 @@ type ClusterGenerator struct { var render = &utils.Render{} func NewClusterGenerator(c client.Client, ctx context.Context, clientset kubernetes.Interface, namespace string) Generator { - settingsManager := settings.NewSettingsManager(ctx, clientset, namespace) g := &ClusterGenerator{ @@ -52,7 +48,7 @@ func NewClusterGenerator(c client.Client, ctx context.Context, clientset kuberne } // GetRequeueAfter never requeue the cluster generator because the `clusterSecretEventHandler` will requeue the appsets -// when the cluster secrets change +// when the cluster secrets change. func (g *ClusterGenerator) GetRequeueAfter(_ *argoappsetv1alpha1.ApplicationSetGenerator) time.Duration { return NoRequeueAfter } @@ -62,7 +58,6 @@ func (g *ClusterGenerator) GetTemplate(appSetGenerator *argoappsetv1alpha1.Appli } func (g *ClusterGenerator) GenerateParams(appSetGenerator *argoappsetv1alpha1.ApplicationSetGenerator, appSet *argoappsetv1alpha1.ApplicationSet) ([]map[string]interface{}, error) { - if appSetGenerator == nil { return nil, EmptyAppSetGeneratorError } @@ -95,12 +90,10 @@ func (g *ClusterGenerator) GenerateParams(appSetGenerator *argoappsetv1alpha1.Ap var secretsFound []corev1.Secret for _, cluster := range clustersFromArgoCD.Items { - // If there is a secret for this cluster, then it's a non-local cluster, so it will be // handled by the next step. if secretForCluster, exists := clusterSecrets[cluster.Name]; exists { secretsFound = append(secretsFound, secretForCluster) - } else if !ignoreLocalClusters { // If there is no secret for the cluster, it's the local cluster, so handle it here. params := map[string]interface{}{} @@ -185,5 +178,4 @@ func (g *ClusterGenerator) getSecretsByClusterName(appSetGenerator *argoappsetv1 } return res, nil - } diff --git a/pkg/generator/cluster_test.go b/pkg/generator/cluster_test.go index 248c1d6d..77e12d4e 100644 --- a/pkg/generator/cluster_test.go +++ b/pkg/generator/cluster_test.go @@ -5,18 +5,16 @@ import ( "fmt" "testing" + "github.com/argoproj/argo-cd/v2/applicationset/utils" + argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + kubefake "k8s.io/client-go/kubernetes/fake" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" - - kubefake "k8s.io/client-go/kubernetes/fake" - - "github.com/argoproj/argo-cd/v2/applicationset/utils" - argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - - "github.com/stretchr/testify/assert" ) type possiblyErroringFakeCtrlRuntimeClient struct { @@ -104,11 +102,15 @@ func TestGenerateParams(t *testing.T) { "aaa": "{{ server }}", "no-op": "{{ this-does-not-exist }}", }, expected: []map[string]interface{}{ - {"values.lol1": "lol", "values.lol2": "{{values.lol1}}{{values.lol1}}", "values.lol3": "{{values.lol2}}{{values.lol2}}{{values.lol2}}", "values.foo": "bar", "values.bar": "production", "values.no-op": "{{ this-does-not-exist }}", "values.bat": "production", "values.aaa": "https://production-01.example.com", "name": "production_01/west", "nameNormalized": "production-01-west", "server": "https://production-01.example.com", "metadata.labels.environment": "production", "metadata.labels.org": "bar", - "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "production"}, + { + "values.lol1": "lol", "values.lol2": "{{values.lol1}}{{values.lol1}}", "values.lol3": "{{values.lol2}}{{values.lol2}}{{values.lol2}}", "values.foo": "bar", "values.bar": "production", "values.no-op": "{{ this-does-not-exist }}", "values.bat": "production", "values.aaa": "https://production-01.example.com", "name": "production_01/west", "nameNormalized": "production-01-west", "server": "https://production-01.example.com", "metadata.labels.environment": "production", "metadata.labels.org": "bar", + "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "production", + }, - {"values.lol1": "lol", "values.lol2": "{{values.lol1}}{{values.lol1}}", "values.lol3": "{{values.lol2}}{{values.lol2}}{{values.lol2}}", "values.foo": "bar", "values.bar": "staging", "values.no-op": "{{ this-does-not-exist }}", "values.bat": "staging", "values.aaa": "https://staging-01.example.com", "name": "staging-01", "nameNormalized": "staging-01", "server": "https://staging-01.example.com", "metadata.labels.environment": "staging", "metadata.labels.org": "foo", - "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "staging"}, + { + "values.lol1": "lol", "values.lol2": "{{values.lol1}}{{values.lol1}}", "values.lol3": "{{values.lol2}}{{values.lol2}}{{values.lol2}}", "values.foo": "bar", "values.bar": "staging", "values.no-op": "{{ this-does-not-exist }}", "values.bat": "staging", "values.aaa": "https://staging-01.example.com", "name": "staging-01", "nameNormalized": "staging-01", "server": "https://staging-01.example.com", "metadata.labels.environment": "staging", "metadata.labels.org": "foo", + "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "staging", + }, {"values.lol1": "lol", "values.lol2": "{{values.lol1}}{{values.lol1}}", "values.lol3": "{{values.lol2}}{{values.lol2}}{{values.lol2}}", "values.foo": "bar", "values.bar": "{{ metadata.annotations.foo.argoproj.io }}", "values.no-op": "{{ this-does-not-exist }}", "values.bat": "{{ metadata.labels.environment }}", "values.aaa": "https://kubernetes.default.svc", "nameNormalized": "in-cluster", "name": "in-cluster", "server": "https://kubernetes.default.svc"}, }, @@ -124,11 +126,15 @@ func TestGenerateParams(t *testing.T) { }, values: nil, expected: []map[string]interface{}{ - {"name": "production_01/west", "nameNormalized": "production-01-west", "server": "https://production-01.example.com", "metadata.labels.environment": "production", "metadata.labels.org": "bar", - "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "production"}, + { + "name": "production_01/west", "nameNormalized": "production-01-west", "server": "https://production-01.example.com", "metadata.labels.environment": "production", "metadata.labels.org": "bar", + "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "production", + }, - {"name": "staging-01", "nameNormalized": "staging-01", "server": "https://staging-01.example.com", "metadata.labels.environment": "staging", "metadata.labels.org": "foo", - "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "staging"}, + { + "name": "staging-01", "nameNormalized": "staging-01", "server": "https://staging-01.example.com", "metadata.labels.environment": "staging", "metadata.labels.org": "foo", + "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "staging", + }, }, clientError: false, expectedError: nil, @@ -144,8 +150,10 @@ func TestGenerateParams(t *testing.T) { "foo": "bar", }, expected: []map[string]interface{}{ - {"values.foo": "bar", "name": "production_01/west", "nameNormalized": "production-01-west", "server": "https://production-01.example.com", "metadata.labels.environment": "production", "metadata.labels.org": "bar", - "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "production"}, + { + "values.foo": "bar", "name": "production_01/west", "nameNormalized": "production-01-west", "server": "https://production-01.example.com", "metadata.labels.environment": "production", "metadata.labels.org": "bar", + "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "production", + }, }, clientError: false, expectedError: nil, @@ -168,10 +176,14 @@ func TestGenerateParams(t *testing.T) { "foo": "bar", }, expected: []map[string]interface{}{ - {"values.foo": "bar", "name": "staging-01", "nameNormalized": "staging-01", "server": "https://staging-01.example.com", "metadata.labels.environment": "staging", "metadata.labels.org": "foo", - "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "staging"}, - {"values.foo": "bar", "name": "production_01/west", "nameNormalized": "production-01-west", "server": "https://production-01.example.com", "metadata.labels.environment": "production", "metadata.labels.org": "bar", - "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "production"}, + { + "values.foo": "bar", "name": "staging-01", "nameNormalized": "staging-01", "server": "https://staging-01.example.com", "metadata.labels.environment": "staging", "metadata.labels.org": "foo", + "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "staging", + }, + { + "values.foo": "bar", "name": "production_01/west", "nameNormalized": "production-01-west", "server": "https://production-01.example.com", "metadata.labels.environment": "production", "metadata.labels.org": "bar", + "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "production", + }, }, clientError: false, expectedError: nil, @@ -197,8 +209,10 @@ func TestGenerateParams(t *testing.T) { "name": "baz", }, expected: []map[string]interface{}{ - {"values.name": "baz", "name": "staging-01", "nameNormalized": "staging-01", "server": "https://staging-01.example.com", "metadata.labels.environment": "staging", "metadata.labels.org": "foo", - "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "staging"}, + { + "values.name": "baz", "name": "staging-01", "nameNormalized": "staging-01", "server": "https://staging-01.example.com", "metadata.labels.environment": "staging", "metadata.labels.org": "foo", + "metadata.labels.argocd.argoproj.io/secret-type": "cluster", "metadata.annotations.foo.argoproj.io": "staging", + }, }, clientError: false, expectedError: nil, @@ -220,9 +234,7 @@ func TestGenerateParams(t *testing.T) { } for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - appClientset := kubefake.NewSimpleClientset(runtimeClusters...) fakeClient := fake.NewClientBuilder().WithObjects(clusters...).Build() @@ -231,7 +243,7 @@ func TestGenerateParams(t *testing.T) { testCase.clientError, } - var clusterGenerator = NewClusterGenerator(cl, context.Background(), appClientset, "namespace") + clusterGenerator := NewClusterGenerator(cl, context.Background(), appClientset, "namespace") applicationSetInfo := argoprojiov1alpha1.ApplicationSet{ ObjectMeta: metav1.ObjectMeta{ @@ -250,10 +262,9 @@ func TestGenerateParams(t *testing.T) { if testCase.expectedError != nil { assert.EqualError(t, err, testCase.expectedError.Error()) } else { - assert.NoError(t, err) + require.NoError(t, err) assert.ElementsMatch(t, testCase.expected, got) } - }) } } @@ -594,9 +605,7 @@ func TestGenerateParamsGoTemplate(t *testing.T) { } for _, testCase := range testCases { - t.Run(testCase.name, func(t *testing.T) { - appClientset := kubefake.NewSimpleClientset(runtimeClusters...) fakeClient := fake.NewClientBuilder().WithObjects(clusters...).Build() @@ -605,7 +614,7 @@ func TestGenerateParamsGoTemplate(t *testing.T) { testCase.clientError, } - var clusterGenerator = NewClusterGenerator(cl, context.Background(), appClientset, "namespace") + clusterGenerator := NewClusterGenerator(cl, context.Background(), appClientset, "namespace") applicationSetInfo := argoprojiov1alpha1.ApplicationSet{ ObjectMeta: metav1.ObjectMeta{ @@ -626,10 +635,9 @@ func TestGenerateParamsGoTemplate(t *testing.T) { if testCase.expectedError != nil { assert.EqualError(t, err, testCase.expectedError.Error()) } else { - assert.NoError(t, err) + require.NoError(t, err) assert.ElementsMatch(t, testCase.expected, got) } - }) } } diff --git a/pkg/generator/generator_spec_processor.go b/pkg/generator/generator_spec_processor.go index 318db380..c1e1a6cd 100644 --- a/pkg/generator/generator_spec_processor.go +++ b/pkg/generator/generator_spec_processor.go @@ -4,16 +4,12 @@ import ( "fmt" "reflect" - "github.com/jeremywohl/flatten" - "github.com/argoproj/argo-cd/v2/applicationset/utils" - - "k8s.io/apimachinery/pkg/labels" - argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - "github.com/imdario/mergo" + "github.com/jeremywohl/flatten" "github.com/rs/zerolog/log" + "k8s.io/apimachinery/pkg/labels" ) const ( @@ -25,7 +21,7 @@ type TransformResult struct { Template argoprojiov1alpha1.ApplicationSetTemplate } -// Transform a spec generator to list of paramSets and a template +// Transform a spec generator to list of paramSets and a template. func Transform(requestedGenerator argoprojiov1alpha1.ApplicationSetGenerator, allGenerators map[string]Generator, baseTemplate argoprojiov1alpha1.ApplicationSetTemplate, appSet *argoprojiov1alpha1.ApplicationSet, genParams map[string]interface{}) ([]TransformResult, error) { // This is a custom version of the `LabelSelectorAsSelector` that is in k8s.io/apimachinery. This has been copied // verbatim from that package, with the difference that we do not have any restrictions on label values. This is done @@ -100,7 +96,7 @@ func GetRelevantGenerators(requestedGenerator *argoprojiov1alpha1.ApplicationSet var res []Generator v := reflect.Indirect(reflect.ValueOf(requestedGenerator)) - for i := 0; i < v.NumField(); i++ { + for i := range v.NumField() { field := v.Field(i) if !field.CanInterface() { continue diff --git a/pkg/generator/generator_spec_processor_test.go b/pkg/generator/generator_spec_processor_test.go index 54350dd9..3971c85c 100644 --- a/pkg/generator/generator_spec_processor_test.go +++ b/pkg/generator/generator_spec_processor_test.go @@ -4,15 +4,13 @@ import ( "context" "testing" + argov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - argov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - - corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/runtime" kubefake "k8s.io/client-go/kubernetes/fake" crtclient "sigs.k8s.io/controller-runtime/pkg/client" diff --git a/pkg/generator/interface.go b/pkg/generator/interface.go index c08dcc93..24940c6c 100644 --- a/pkg/generator/interface.go +++ b/pkg/generator/interface.go @@ -23,5 +23,7 @@ type Generator interface { GetTemplate(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator) *argoprojiov1alpha1.ApplicationSetTemplate } -var EmptyAppSetGeneratorError = errors.New("ApplicationSet is empty") -var NoRequeueAfter time.Duration +var ( + EmptyAppSetGeneratorError = errors.New("ApplicationSet is empty") + NoRequeueAfter time.Duration +) diff --git a/pkg/generator/list.go b/pkg/generator/list.go index 9b9e65f2..0796eb1a 100644 --- a/pkg/generator/list.go +++ b/pkg/generator/list.go @@ -5,15 +5,14 @@ import ( "fmt" "time" - "sigs.k8s.io/yaml" - argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" + "github.com/pkg/errors" + "sigs.k8s.io/yaml" ) var _ Generator = (*ListGenerator)(nil) -type ListGenerator struct { -} +type ListGenerator struct{} func NewListGenerator() Generator { g := &ListGenerator{} @@ -28,6 +27,8 @@ func (g *ListGenerator) GetTemplate(appSetGenerator *argoprojiov1alpha1.Applicat return &appSetGenerator.List.Template } +var ErrInvalidValuesType = errors.New("invalid type") + func (g *ListGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, appSet *argoprojiov1alpha1.ApplicationSet) ([]map[string]interface{}, error) { if appSetGenerator == nil { return nil, EmptyAppSetGeneratorError @@ -44,7 +45,7 @@ func (g *ListGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Appli var element map[string]interface{} err := json.Unmarshal(tmpItem.Raw, &element) if err != nil { - return nil, fmt.Errorf("error unmarshling list element %v", err) + return nil, errors.Wrap(err, "error unmarshling list element") } if appSet.Spec.GoTemplate { @@ -54,19 +55,19 @@ func (g *ListGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Appli if key == "values" { values, ok := (value).(map[string]interface{}) if !ok { - return nil, fmt.Errorf("error parsing values map") + return nil, ErrInvalidValuesType } for k, v := range values { value, ok := v.(string) if !ok { - return nil, fmt.Errorf("error parsing value as string %v", err) + return nil, errors.Wrap(err, "error parsing value as string") } params[fmt.Sprintf("values.%s", k)] = value } } else { v, ok := value.(string) if !ok { - return nil, fmt.Errorf("error parsing value as string %v", err) + return nil, errors.Wrap(err, "error parsing value as string") } params[key] = v } @@ -77,11 +78,10 @@ func (g *ListGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Appli // Append elements from ElementsYaml to the response if len(appSetGenerator.List.ElementsYaml) > 0 { - var yamlElements []map[string]interface{} err := yaml.Unmarshal([]byte(appSetGenerator.List.ElementsYaml), &yamlElements) if err != nil { - return nil, fmt.Errorf("error unmarshling decoded ElementsYaml %v", err) + return nil, errors.Wrap(err, "error unmarshling decoded ElementsYaml") } res = append(res, yamlElements...) } diff --git a/pkg/generator/list_test.go b/pkg/generator/list_test.go index 272879ed..b6861d0d 100644 --- a/pkg/generator/list_test.go +++ b/pkg/generator/list_test.go @@ -3,11 +3,11 @@ package generator import ( "testing" + argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" ) func TestGenerateListParams(t *testing.T) { @@ -25,8 +25,7 @@ func TestGenerateListParams(t *testing.T) { } for _, testCase := range testCases { - - var listGenerator = NewListGenerator() + listGenerator := NewListGenerator() applicationSetInfo := argoprojiov1alpha1.ApplicationSet{ ObjectMeta: metav1.ObjectMeta{ @@ -38,11 +37,11 @@ func TestGenerateListParams(t *testing.T) { got, err := listGenerator.GenerateParams(&argoprojiov1alpha1.ApplicationSetGenerator{ List: &argoprojiov1alpha1.ListGenerator{ Elements: testCase.elements, - }}, &applicationSetInfo) + }, + }, &applicationSetInfo) - assert.NoError(t, err) + require.NoError(t, err) assert.ElementsMatch(t, testCase.expected, got) - } } @@ -61,8 +60,7 @@ func TestGenerateListParamsGoTemplate(t *testing.T) { } for _, testCase := range testCases { - - var listGenerator = NewListGenerator() + listGenerator := NewListGenerator() applicationSetInfo := argoprojiov1alpha1.ApplicationSet{ ObjectMeta: metav1.ObjectMeta{ @@ -76,9 +74,10 @@ func TestGenerateListParamsGoTemplate(t *testing.T) { got, err := listGenerator.GenerateParams(&argoprojiov1alpha1.ApplicationSetGenerator{ List: &argoprojiov1alpha1.ListGenerator{ Elements: testCase.elements, - }}, &applicationSetInfo) + }, + }, &applicationSetInfo) - assert.NoError(t, err) + require.NoError(t, err) assert.ElementsMatch(t, testCase.expected, got) } } diff --git a/pkg/generator/matrix.go b/pkg/generator/matrix.go index 650122d4..4412b98d 100644 --- a/pkg/generator/matrix.go +++ b/pkg/generator/matrix.go @@ -4,11 +4,10 @@ import ( "fmt" "time" - "github.com/imdario/mergo" - "github.com/argoproj/argo-cd/v2/applicationset/utils" argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - + "github.com/imdario/mergo" + "github.com/pkg/errors" "github.com/rs/zerolog/log" ) @@ -33,7 +32,6 @@ func NewMatrixGenerator(supportedGenerators map[string]Generator) Generator { } func (m *MatrixGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator, appSet *argoprojiov1alpha1.ApplicationSet) ([]map[string]interface{}, error) { - if appSetGenerator.Matrix == nil { return nil, EmptyAppSetGeneratorError } @@ -58,7 +56,6 @@ func (m *MatrixGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.App return nil, fmt.Errorf("failed to get params for second generator in the matrix generator: %w", err) } for _, b := range g1 { - if appSet.Spec.GoTemplate { tmp := map[string]interface{}{} if err := mergo.Merge(&tmp, b, mergo.WithOverride); err != nil { @@ -120,13 +117,12 @@ func (m *MatrixGenerator) getParams(appSetBaseGenerator argoprojiov1alpha1.Appli argoprojiov1alpha1.ApplicationSetTemplate{}, appSet, params) - if err != nil { - return nil, fmt.Errorf("child generator returned an error on parameter generation: %v", err) + return nil, errors.Wrap(err, "child generator returned an error on parameter generation") } if len(t) == 0 { - return nil, fmt.Errorf("child generator generated no parameters") + return nil, ErrNoParameters } if len(t) > 1 { @@ -136,6 +132,8 @@ func (m *MatrixGenerator) getParams(appSetBaseGenerator argoprojiov1alpha1.Appli return t[0].Params, nil } +var ErrNoParameters = errors.New("child generator generated no parameters") + const maxDuration time.Duration = 1<<63 - 1 func (m *MatrixGenerator) GetRequeueAfter(appSetGenerator *argoprojiov1alpha1.ApplicationSetGenerator) time.Duration { @@ -172,7 +170,6 @@ func (m *MatrixGenerator) GetRequeueAfter(appSetGenerator *argoprojiov1alpha1.Ap } else { return NoRequeueAfter } - } func getMatrixGenerator(r argoprojiov1alpha1.ApplicationSetNestedGenerator) (*argoprojiov1alpha1.MatrixGenerator, error) { diff --git a/pkg/generator/matrix_test.go b/pkg/generator/matrix_test.go index 31b0d1c3..fd5a040e 100644 --- a/pkg/generator/matrix_test.go +++ b/pkg/generator/matrix_test.go @@ -4,18 +4,16 @@ import ( "testing" "time" + argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" - - argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" ) func TestMatrixGenerate(t *testing.T) { - listGenerator := &argoprojiov1alpha1.ListGenerator{ Elements: []apiextensionsv1.JSON{{Raw: []byte(`{"cluster": "Cluster","url": "Url", "templated": "test-{{path.basenameNormalized}}"}`)}}, } @@ -127,7 +125,6 @@ func TestMatrixGenerate(t *testing.T) { } func TestMatrixGenerateGoTemplate(t *testing.T) { - listGenerator := &argoprojiov1alpha1.ListGenerator{ Elements: []apiextensionsv1.JSON{{Raw: []byte(`{"cluster": "Cluster","url": "Url"}`)}}, } diff --git a/pkg/generator/merge.go b/pkg/generator/merge.go index 965a3a8a..f8a85c0b 100644 --- a/pkg/generator/merge.go +++ b/pkg/generator/merge.go @@ -5,11 +5,10 @@ import ( "fmt" "time" - "github.com/imdario/mergo" - "github.com/argoproj/argo-cd/v2/applicationset/utils" argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" - + "github.com/imdario/mergo" + "github.com/pkg/errors" "github.com/rs/zerolog/log" ) @@ -77,7 +76,6 @@ func (m *MergeGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Appl for mergeKeyValue, baseParamSet := range baseParamSetsByMergeKey { if overrideParamSet, exists := paramSetsByMergeKey[mergeKeyValue]; exists { - if appSet.Spec.GoTemplate { if err := mergo.Merge(&baseParamSet, overrideParamSet, mergo.WithOverride); err != nil { return nil, fmt.Errorf("error merging base param set with override param set: %w", err) @@ -95,7 +93,7 @@ func (m *MergeGenerator) GenerateParams(appSetGenerator *argoprojiov1alpha1.Appl } mergedParamSets := make([]map[string]interface{}, len(baseParamSetsByMergeKey)) - var i = 0 + i := 0 for _, mergedParamSet := range baseParamSetsByMergeKey { mergedParamSets[i] = mergedParamSet i += 1 @@ -177,9 +175,8 @@ func (m *MergeGenerator) getParams(appSetBaseGenerator argoprojiov1alpha1.Applic argoprojiov1alpha1.ApplicationSetTemplate{}, appSet, map[string]interface{}{}) - if err != nil { - return nil, fmt.Errorf("child generator returned an error on parameter generation: %v", err) + return nil, errors.Wrap(err, "child generator returned an error on parameter generation") } if len(t) == 0 { @@ -227,7 +224,6 @@ func (m *MergeGenerator) GetRequeueAfter(appSetGenerator *argoprojiov1alpha1.App } else { return NoRequeueAfter } - } func getMergeGenerator(r argoprojiov1alpha1.ApplicationSetNestedGenerator) (*argoprojiov1alpha1.MergeGenerator, error) { diff --git a/pkg/generator/merge_test.go b/pkg/generator/merge_test.go index 5bac849d..234d405f 100644 --- a/pkg/generator/merge_test.go +++ b/pkg/generator/merge_test.go @@ -5,10 +5,10 @@ import ( "fmt" "testing" + argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" - - argoprojiov1alpha1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" ) func getNestedListGenerator(json string) *argoprojiov1alpha1.ApplicationSetNestedGenerator { @@ -49,6 +49,7 @@ func listOfMapsToSet(maps []map[string]interface{}) (map[string]bool, error) { } func TestMergeGenerate(t *testing.T) { + t.Parallel() testCases := []struct { name string @@ -156,7 +157,7 @@ func TestMergeGenerate(t *testing.T) { appSet := &argoprojiov1alpha1.ApplicationSet{} - var mergeGenerator = NewMergeGenerator( + mergeGenerator := NewMergeGenerator( map[string]Generator{ "List": &ListGenerator{}, "Matrix": &MatrixGenerator{ @@ -184,12 +185,10 @@ func TestMergeGenerate(t *testing.T) { assert.EqualError(t, err, testCaseCopy.expectedErr.Error()) } else { expectedSet, err := listOfMapsToSet(testCaseCopy.expected) - assert.NoError(t, err) + require.NoError(t, err) actualSet, err := listOfMapsToSet(got) - assert.NoError(t, err) - - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, expectedSet, actualSet) } }) @@ -197,6 +196,7 @@ func TestMergeGenerate(t *testing.T) { } func toAPIExtensionsJSON(t *testing.T, g interface{}) *apiextensionsv1.JSON { + t.Helper() resVal, err := json.Marshal(g) if err != nil { @@ -210,6 +210,8 @@ func toAPIExtensionsJSON(t *testing.T, g interface{}) *apiextensionsv1.JSON { } func TestParamSetsAreUniqueByMergeKeys(t *testing.T) { + t.Parallel() + testCases := []struct { name string mergeKeys []string @@ -341,11 +343,9 @@ func TestParamSetsAreUniqueByMergeKeys(t *testing.T) { if testCaseCopy.expectedErr != nil { assert.EqualError(t, err, testCaseCopy.expectedErr.Error()) } else { - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, testCaseCopy.expected, got) } - }) - } } diff --git a/pkg/generator/value_interpolation.go b/pkg/generator/value_interpolation.go index 2ab74782..237f5838 100644 --- a/pkg/generator/value_interpolation.go +++ b/pkg/generator/value_interpolation.go @@ -12,7 +12,6 @@ func appendTemplatedValues(values map[string]string, params map[string]interface for key, value := range values { result, err := replaceTemplatedString(value, params, useGoTemplate, goTemplateOptions) - if err != nil { return fmt.Errorf("failed to replace templated string: %w", err) } diff --git a/pkg/git/repo.go b/pkg/git/repo.go index 05652154..eee4f27f 100644 --- a/pkg/git/repo.go +++ b/pkg/git/repo.go @@ -68,14 +68,14 @@ func (r *Repo) Clone(ctx context.Context) error { args = append(args, "--branch", r.BranchName) } - cmd := r.execCommand("git", args...) + cmd := r.execGit(args...) out, err := cmd.CombinedOutput() if err != nil { log.Error().Err(err).Msgf("unable to clone repository, %s", out) return err } - if log.Trace().Enabled() { + if log.Trace().Enabled() { //nolint: zerologlint if err = filepath.WalkDir(r.Directory, printFile); err != nil { log.Warn().Err(err).Msg("failed to walk directory") } @@ -89,13 +89,13 @@ func printFile(s string, d fs.DirEntry, err error) error { return err } if !d.IsDir() { - println(s) + log.Debug().Msg(s) } return nil } func (r *Repo) GetRemoteHead() (string, error) { - cmd := r.execCommand("git", "symbolic-ref", "refs/remotes/origin/HEAD", "--short") + cmd := r.execGit("symbolic-ref", "refs/remotes/origin/HEAD", "--short") out, err := cmd.CombinedOutput() if err != nil { return "", errors.Wrap(err, "failed to determine which branch HEAD points to") @@ -118,7 +118,7 @@ func (r *Repo) MergeIntoTarget(ctx context.Context, sha string) error { )) defer span.End() - cmd := r.execCommand("git", "merge", sha) + cmd := r.execGit("merge", sha) out, err := cmd.CombinedOutput() if err != nil { telemetry.SetError(span, err, "merge commit into branch") @@ -129,18 +129,18 @@ func (r *Repo) MergeIntoTarget(ctx context.Context, sha string) error { return nil } -func (r *Repo) Update(ctx context.Context) error { - cmd := r.execCommand("git", "pull") +func (r *Repo) Update() error { + cmd := r.execGit("pull") cmd.Stdout = os.Stdout cmd.Stderr = os.Stdout return cmd.Run() } -func (r *Repo) execCommand(name string, args ...string) *exec.Cmd { +func (r *Repo) execGit(args ...string) *exec.Cmd { argsToLog := r.censorVcsToken(args) log.Debug().Strs("args", argsToLog).Msg("building command") - cmd := exec.Command(name, args...) + cmd := exec.Command("git", args...) if r.Directory != "" { cmd.Dir = r.Directory } @@ -176,7 +176,7 @@ func censorVcsToken(cfg config.ServerConfig, args []string) []string { return argsToLog } -// SetCredentials ensures Git auth is set up for cloning +// SetCredentials ensures Git auth is set up for cloning. func SetCredentials(cfg config.ServerConfig, vcsClient vcs.Client) error { email := vcsClient.Email() username := vcsClient.Username() @@ -200,9 +200,7 @@ func SetCredentials(cfg config.ServerConfig, vcsClient vcs.Client) error { homedir, err := os.UserHomeDir() if err != nil { - if err != nil { - return errors.Wrap(err, "unable to get home directory") - } + return errors.Wrap(err, "unable to get home directory") } outfile, err := os.Create(fmt.Sprintf("%s/.git-credentials", homedir)) if err != nil { @@ -250,14 +248,14 @@ func getCloneUrl(user string, cfg config.ServerConfig) (string, error) { return fmt.Sprintf("%s://%s:%s@%s", scheme, user, vcsToken, hostname), nil } -// GetListOfChangedFiles returns a list of files that have changed between the current branch and the target branch +// GetListOfChangedFiles returns a list of files that have changed between the current branch and the target branch. func (r *Repo) GetListOfChangedFiles(ctx context.Context) ([]string, error) { _, span := tracer.Start(ctx, "RepoGetListOfChangedFiles") defer span.End() var fileList []string - cmd := r.execCommand("git", "diff", "--name-only", fmt.Sprintf("%s/%s", "origin", r.BranchName)) + cmd := r.execGit("diff", "--name-only", fmt.Sprintf("%s/%s", "origin", r.BranchName)) pipe, _ := cmd.StdoutPipe() var wg sync.WaitGroup scanner := bufio.NewScanner(pipe) diff --git a/pkg/git/repo_test.go b/pkg/git/repo_test.go index 966944f6..ddbbfb95 100644 --- a/pkg/git/repo_test.go +++ b/pkg/git/repo_test.go @@ -14,6 +14,8 @@ import ( ) func wipe(t *testing.T, path string) { + t.Helper() + err := os.RemoveAll(path) require.NoError(t, err) } diff --git a/pkg/kubernetes/api_client.go b/pkg/kubernetes/api_client.go index 42add3d6..883296b4 100644 --- a/pkg/kubernetes/api_client.go +++ b/pkg/kubernetes/api_client.go @@ -10,7 +10,7 @@ import ( controllerClient "sigs.k8s.io/controller-runtime/pkg/client" ) -// ClusterTypes must match with the cmd/root.go kubernetes-type flag +// ClusterTypes must match with the cmd/root.go kubernetes-type flag. const ( ClusterTypeEKS = "eks" ClusterTypeLOCAL = "local" @@ -29,7 +29,6 @@ type NewClientInput struct { // New creates new Kubernetes clients with the specified options. func New(input *NewClientInput, opts ...NewClientOption) (Interface, error) { - var ( k8sConfig *rest.Config err error @@ -50,7 +49,6 @@ func New(input *NewClientInput, opts ...NewClientOption) (Interface, error) { return nil, err } } - } input.restConfig = k8sConfig diff --git a/pkg/kubernetes/api_eks_client.go b/pkg/kubernetes/api_eks_client.go index 1acd7a39..754d049f 100644 --- a/pkg/kubernetes/api_eks_client.go +++ b/pkg/kubernetes/api_eks_client.go @@ -12,10 +12,10 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/config" "github.com/aws/aws-sdk-go-v2/service/eks" + "github.com/aws/aws-sdk-go-v2/service/sts" smithyhttp "github.com/aws/smithy-go/transport/http" + "github.com/pkg/errors" "k8s.io/client-go/rest" - - "github.com/aws/aws-sdk-go-v2/service/sts" ) const ( @@ -143,7 +143,7 @@ func (t *TokenRefresher) refreshToken(ctx context.Context) error { if time.Now().After(t.tokenExpiration) { token, err := getEKSToken(ctx, t.awsConfig, t.clusterID) if err != nil { - return fmt.Errorf("unable to refresh EKS token, %v", err) + return errors.Wrap(err, "unable to refresh EKS token") } t.token = *token @@ -165,7 +165,7 @@ type transportWrapper struct { refresher *TokenRefresher } -// RoundTrip will perform the refrsh token before each request +// RoundTrip will perform the refrsh token before each request. func (t *transportWrapper) RoundTrip(req *http.Request) (*http.Response, error) { if err := t.refresher.refreshToken(req.Context()); err != nil { // log the error and continue with the request diff --git a/pkg/msg/message.go b/pkg/msg/message.go index f9b1c08a..e2042605 100644 --- a/pkg/msg/message.go +++ b/pkg/msg/message.go @@ -49,7 +49,7 @@ type toEmoji interface { // Message type that allows concurrent updates // Has a reference to the owner/repo (ie zapier/kubechecks), -// the PR/MR id, and the actual messsage +// the PR/MR id, and the actual messsage. type Message struct { Name string Owner string @@ -141,7 +141,7 @@ func (m *Message) buildFooter(start time.Time, commitSHA, labelFilter string, sh return fmt.Sprintf(" _Done: Pod: %s, Dur: %v, SHA: %s%s_ \n", hostname, duration, pkg.GitCommit, envStr) } -// BuildComment iterates the map of all apps in this message, building a final comment from their current state +// BuildComment iterates the map of all apps in this message, building a final comment from their current state. func (m *Message) BuildComment(ctx context.Context, start time.Time, commitSHA, labelFilter string, showDebugInfo bool) string { _, span := tracer.Start(ctx, "buildComment") defer span.End() diff --git a/pkg/repoUrl.go b/pkg/repoUrl.go index cfc5a677..a32ff0bd 100644 --- a/pkg/repoUrl.go +++ b/pkg/repoUrl.go @@ -5,7 +5,7 @@ import ( "net/url" "strings" - "github.com/chainguard-dev/git-urls" + giturls "github.com/chainguard-dev/git-urls" ) type RepoURL struct { diff --git a/pkg/repoUrl_test.go b/pkg/repoUrl_test.go index a4bc786e..cb46651e 100644 --- a/pkg/repoUrl_test.go +++ b/pkg/repoUrl_test.go @@ -5,9 +5,8 @@ import ( "net/url" "testing" - "github.com/stretchr/testify/require" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestNormalizeStrings(t *testing.T) { diff --git a/pkg/repo_config/config.go b/pkg/repo_config/config.go index 4f71b50c..4ddc5bbf 100644 --- a/pkg/repo_config/config.go +++ b/pkg/repo_config/config.go @@ -1,9 +1,8 @@ package repo_config import ( - "fmt" - "github.com/creasty/defaults" + "github.com/pkg/errors" ) type Config struct { @@ -12,19 +11,19 @@ type Config struct { } type ArgoCdApplicationConfig struct { - Name string `yaml:"name" validate:"empty=false"` - Cluster string `yaml:"cluster" validate:"empty=false"` - Path string `yaml:"path" validate:"empty=false"` + Name string `validate:"empty=false" yaml:"name"` + Cluster string `validate:"empty=false" yaml:"cluster"` + Path string `validate:"empty=false" yaml:"path"` AdditionalPaths []string `yaml:"additionalPaths"` - EnableConfTest bool `yaml:"enableConfTest" default:"true"` - EnableKubeConform bool `yaml:"enableKubeConform" default:"true"` - EnableKubePug bool `yaml:"enableKubePug" default:"true"` + EnableConfTest bool `default:"true" yaml:"enableConfTest"` + EnableKubeConform bool `default:"true" yaml:"enableKubeConform"` + EnableKubePug bool `default:"true" yaml:"enableKubePug"` } func (s *ArgoCdApplicationConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { err := defaults.Set(s) if err != nil { - return fmt.Errorf("failed to set defaults for project config: %v", err) + return errors.Wrap(err, "failed to set defaults for project config") } type plain ArgoCdApplicationConfig @@ -36,17 +35,17 @@ func (s *ArgoCdApplicationConfig) UnmarshalYAML(unmarshal func(interface{}) erro } type ArgocdApplicationSetConfig struct { - Name string `yaml:"name" validate:"empty=false"` - Paths []string `yaml:"paths" validate:"empty=false"` - EnableConfTest bool `yaml:"enableConfTest" default:"true"` - EnableKubeConform bool `yaml:"enableKubeConform" default:"true"` - EnableKubePug bool `yaml:"enableKubePug" default:"true"` + Name string `validate:"empty=false" yaml:"name"` + Paths []string `validate:"empty=false" yaml:"paths"` + EnableConfTest bool `default:"true" yaml:"enableConfTest"` + EnableKubeConform bool `default:"true" yaml:"enableKubeConform"` + EnableKubePug bool `default:"true" yaml:"enableKubePug"` } func (s *ArgocdApplicationSetConfig) UnmarshalYAML(unmarshal func(interface{}) error) error { err := defaults.Set(s) if err != nil { - return fmt.Errorf("failed to set defaults for project config: %v", err) + return errors.Wrap(err, "failed to set defaults for project config") } type plain ArgocdApplicationSetConfig diff --git a/pkg/repo_config/loader.go b/pkg/repo_config/loader.go index a97b424b..4311e30c 100644 --- a/pkg/repo_config/loader.go +++ b/pkg/repo_config/loader.go @@ -1,7 +1,6 @@ package repo_config import ( - "fmt" "os" "path/filepath" @@ -73,7 +72,7 @@ func LoadRepoConfigBytes(b []byte) (*Config, error) { cfg := &Config{} err := yaml.Unmarshal(b, cfg) if err != nil { - return nil, fmt.Errorf("could not parse Project config file (.kubechecks.yaml): %v", err) + return nil, errors.Wrap(err, "could not parse Project config file (.kubechecks.yaml)") } if err := validate.Validate(cfg); err != nil { diff --git a/pkg/repo_config/loader_test.go b/pkg/repo_config/loader_test.go index caab0975..437dd1ec 100644 --- a/pkg/repo_config/loader_test.go +++ b/pkg/repo_config/loader_test.go @@ -101,7 +101,7 @@ func Test_loadProjectConfigFile(t *testing.T) { return } - assert.Equal(t, got, tt.want, "Configs are not the same.") + assert.Equal(t, tt.want, got, "Configs are not the same.") }) } } @@ -163,7 +163,6 @@ func TestLoadRepoConfig(t *testing.T) { want *Config wantErr bool }{ - { name: "yaml", args: args{ @@ -251,7 +250,7 @@ func TestLoadRepoConfig(t *testing.T) { t.Errorf("LoadRepoConfig() error = %v, wantErr %v", err, tt.wantErr) return } - assert.Equal(t, got, tt.want, "Configs are not the same.") + assert.Equal(t, tt.want, got, "Configs are not the same.") }) } } diff --git a/pkg/server/hook_handler.go b/pkg/server/hook_handler.go index 2f8929e0..92657103 100644 --- a/pkg/server/hook_handler.go +++ b/pkg/server/hook_handler.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/labstack/echo/v4" + "github.com/pkg/errors" "github.com/rs/zerolog/log" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/attribute" @@ -32,6 +33,7 @@ func NewVCSHookHandler(ctr container.Container, processors []checks.ProcessorEnt processors: processors, } } + func (h *VCSHookHandler) AttachHandlers(grp *echo.Group) { projectHookPath := fmt.Sprintf("/%s/project", h.ctr.VcsClient.GetName()) grp.POST(projectHookPath, h.groupHandler) @@ -49,15 +51,14 @@ func (h *VCSHookHandler) groupHandler(c echo.Context) error { pr, err := h.ctr.VcsClient.ParseHook(c.Request(), payload) if err != nil { - switch err { - case vcs.ErrInvalidType: + if errors.Is(err, vcs.ErrInvalidType) { log.Debug().Msg("Ignoring event, not a merge request") return c.String(http.StatusOK, "Skipped") - default: - // TODO: do something ELSE with the error - log.Error().Err(err).Msg("Failed to create a repository locally") - return echo.ErrBadRequest } + + // TODO: do something ELSE with the error + log.Error().Err(err).Msg("Failed to create a repository locally") + return echo.ErrBadRequest } // We now have a generic repo with all the info we need to start processing an event. Hand off to the event processor @@ -76,8 +77,7 @@ func (h *VCSHookHandler) processCheckEvent(ctx context.Context, pullRequest vcs. ProcessCheckEvent(ctx, pullRequest, h.ctr, h.processors) } -type RepoDirectory struct { -} +type RepoDirectory struct{} func ProcessCheckEvent(ctx context.Context, pr vcs.PullRequest, ctr container.Container, processors []checks.ProcessorEntry) { ctx, span := tracer.Start(ctx, "processCheckEvent", diff --git a/pkg/server/server.go b/pkg/server/server.go index 0196d78f..2cb80c27 100644 --- a/pkg/server/server.go +++ b/pkg/server/server.go @@ -2,7 +2,6 @@ package server import ( "context" - "fmt" "net/url" "strings" @@ -53,9 +52,9 @@ func (s *Server) Start(ctx context.Context) { ghHooks := NewVCSHookHandler(s.ctr, s.processors) ghHooks.AttachHandlers(hooksGroup) - fmt.Println("Method\tPath") + log.Debug().Msg("Method\tPath") for _, r := range e.Routes() { - fmt.Printf("%s\t%s\n", r.Method, r.Path) + log.Warn().Msgf("%s\t%s\n", r.Method, r.Path) } if err := e.Start(":8080"); err != nil { diff --git a/pkg/vcs/client.go b/pkg/vcs/client.go index dfd9a27f..dcf8c4c8 100644 --- a/pkg/vcs/client.go +++ b/pkg/vcs/client.go @@ -10,7 +10,7 @@ const ( ) var ( - // ErrInvalidType is a sentinel error for use in client implementations + // ErrInvalidType is a sentinel error for use in client implementations. ErrInvalidType = errors.New("invalid event type") ErrHookNotFound = errors.New("webhook not found") ) diff --git a/pkg/vcs/github_client/client.go b/pkg/vcs/github_client/client.go index 6b53fd78..09b3484a 100644 --- a/pkg/vcs/github_client/client.go +++ b/pkg/vcs/github_client/client.go @@ -9,7 +9,7 @@ import ( "strconv" "strings" - "github.com/chainguard-dev/git-urls" + giturls "github.com/chainguard-dev/git-urls" "github.com/google/go-github/v62/github" "github.com/pkg/errors" "github.com/rs/zerolog/log" @@ -32,7 +32,7 @@ type Client struct { username, email string } -// GClient is a struct that holds the services for the GitHub client +// GClient is a struct that holds the services for the GitHub client. type GClient struct { PullRequests PullRequestsServices Repositories RepositoriesServices @@ -40,8 +40,8 @@ type GClient struct { } // CreateGithubClient creates a new GitHub client using the auth token provided. We -// can't validate the token at this point, so if it exists we assume it works -func CreateGithubClient(cfg config.ServerConfig) (*Client, error) { +// can't validate the token at this point, so if it exists we assume it works. +func CreateGithubClient(ctx context.Context, cfg config.ServerConfig) (*Client, error) { var ( err error googleClient *github.Client @@ -54,7 +54,6 @@ func CreateGithubClient(cfg config.ServerConfig) (*Client, error) { log.Fatal().Msg("github token needs to be set") } log.Debug().Msgf("Token Length - %d", len(t)) - ctx := context.Background() ts := oauth2.StaticTokenSource( &oauth2.Token{AccessToken: t}, ) diff --git a/pkg/vcs/github_client/client_test.go b/pkg/vcs/github_client/client_test.go index bd0704f3..db6e0b63 100644 --- a/pkg/vcs/github_client/client_test.go +++ b/pkg/vcs/github_client/client_test.go @@ -10,12 +10,13 @@ import ( "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + githubMocks "github.com/zapier/kubechecks/mocks/github_client/mocks" "github.com/zapier/kubechecks/pkg/config" "github.com/zapier/kubechecks/pkg/vcs" ) -// MockGitHubMethod is a generic function to mock GitHub client methods +// MockGitHubMethod is a generic function to mock GitHub client methods. func MockGitHubMethod(methodName string, returns []interface{}) *GClient { mockClient := new(githubMocks.MockRepositoriesServices) mockClient.On(methodName, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(returns...) @@ -51,7 +52,6 @@ func TestParseRepo(t *testing.T) { } for _, tc := range testcases { - tc := tc t.Run(tc.name, func(t *testing.T) { owner, repo := parseRepo(tc.input) assert.Equal(t, tc.expectedOwner, owner) @@ -69,7 +69,6 @@ func TestClient_CreateHook(t *testing.T) { email string } type args struct { - ctx context.Context ownerAndRepoName string webhookUrl string webhookSecret string @@ -88,7 +87,8 @@ func TestClient_CreateHook(t *testing.T) { []interface{}{ &github.Hook{}, &github.Response{Response: &http.Response{StatusCode: http.StatusOK}}, - nil}), + nil, + }), cfg: config.ServerConfig{ VcsToken: "ghp_helloworld", VcsType: "github", @@ -97,7 +97,6 @@ func TestClient_CreateHook(t *testing.T) { email: "dummy@zapier.com", }, args: args{ - ctx: context.Background(), ownerAndRepoName: "https://dummy-bot:********@github.com/dummy-bot-zapier/test-repo.git", webhookUrl: "https://dummywebhooks.local", webhookSecret: "dummy-webhook-secret", @@ -112,7 +111,8 @@ func TestClient_CreateHook(t *testing.T) { []interface{}{ nil, &github.Response{Response: &http.Response{StatusCode: http.StatusBadRequest}}, - fmt.Errorf("mock bad request")}), + fmt.Errorf("mock bad request"), + }), cfg: config.ServerConfig{ VcsToken: "ghp_helloworld", VcsType: "github", @@ -121,7 +121,6 @@ func TestClient_CreateHook(t *testing.T) { email: "dummy@zapier.com", }, args: args{ - ctx: context.Background(), ownerAndRepoName: "https://dummy-bot:********@github.com/dummy-bot-zapier/test-repo.git", webhookUrl: "https://dummywebhooks.local", webhookSecret: "dummy-webhook-secret", @@ -136,7 +135,8 @@ func TestClient_CreateHook(t *testing.T) { []interface{}{ nil, nil, - fmt.Errorf("mock network error")}), + fmt.Errorf("mock network error"), + }), cfg: config.ServerConfig{ VcsToken: "ghp_helloworld", VcsType: "github", @@ -145,7 +145,6 @@ func TestClient_CreateHook(t *testing.T) { email: "dummy@zapier.com", }, args: args{ - ctx: context.Background(), ownerAndRepoName: "https://dummy-bot:********@github.com/dummy-bot-zapier/test-repo.git", webhookUrl: "https://dummywebhooks.local", webhookSecret: "dummy-webhook-secret", @@ -155,6 +154,7 @@ func TestClient_CreateHook(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() c := &Client{ shurcoolClient: tt.fields.shurcoolClient, googleClient: tt.fields.googleClient, @@ -162,7 +162,7 @@ func TestClient_CreateHook(t *testing.T) { username: tt.fields.username, email: tt.fields.email, } - tt.wantErr(t, c.CreateHook(tt.args.ctx, tt.args.ownerAndRepoName, tt.args.webhookUrl, tt.args.webhookSecret), fmt.Sprintf("CreateHook(%v, %v, %v, %v)", tt.args.ctx, tt.args.ownerAndRepoName, tt.args.webhookUrl, tt.args.webhookSecret)) + tt.wantErr(t, c.CreateHook(ctx, tt.args.ownerAndRepoName, tt.args.webhookUrl, tt.args.webhookSecret), fmt.Sprintf("CreateHook(%v, %v, %v, %v)", ctx, tt.args.ownerAndRepoName, tt.args.webhookUrl, tt.args.webhookSecret)) }) } } @@ -176,7 +176,6 @@ func TestClient_GetHookByUrl(t *testing.T) { email string } type args struct { - ctx context.Context ownerAndRepoName string webhookUrl string } @@ -205,7 +204,8 @@ func TestClient_GetHookByUrl(t *testing.T) { }, }, &github.Response{Response: &http.Response{StatusCode: http.StatusOK}}, - nil}), + nil, + }), cfg: config.ServerConfig{ VcsToken: "ghp_helloworld", VcsType: "github", @@ -214,7 +214,6 @@ func TestClient_GetHookByUrl(t *testing.T) { email: "dummy@zapier.com", }, args: args{ - ctx: context.Background(), ownerAndRepoName: "https://dummy-bot:********@github.com/dummy-bot-zapier/test-repo.git", webhookUrl: "https://dummywebhooks.local", }, @@ -242,7 +241,8 @@ func TestClient_GetHookByUrl(t *testing.T) { }, }, &github.Response{Response: &http.Response{StatusCode: http.StatusOK}}, - nil}), + nil, + }), cfg: config.ServerConfig{ VcsToken: "ghp_helloworld", VcsType: "github", @@ -251,7 +251,6 @@ func TestClient_GetHookByUrl(t *testing.T) { email: "dummy@zapier.com", }, args: args{ - ctx: context.Background(), ownerAndRepoName: "https://dummy-bot:********@github.com/dummy-bot-zapier/test-repo.git", webhookUrl: "https://dummywebhooks.local", }, @@ -266,7 +265,8 @@ func TestClient_GetHookByUrl(t *testing.T) { []interface{}{ nil, &github.Response{Response: &http.Response{StatusCode: http.StatusOK}}, - nil}), + nil, + }), cfg: config.ServerConfig{ VcsToken: "ghp_helloworld", VcsType: "github", @@ -275,7 +275,6 @@ func TestClient_GetHookByUrl(t *testing.T) { email: "dummy@zapier.com", }, args: args{ - ctx: context.Background(), ownerAndRepoName: "https://dummy-bot:********@github.com/dummy-bot-zapier/test-repo.git", webhookUrl: "https://dummywebhooks.local", }, @@ -290,7 +289,8 @@ func TestClient_GetHookByUrl(t *testing.T) { []interface{}{ nil, &github.Response{Response: &http.Response{StatusCode: http.StatusBadRequest}}, - fmt.Errorf("mock bad request")}), + fmt.Errorf("mock bad request"), + }), cfg: config.ServerConfig{ VcsToken: "ghp_helloworld", VcsType: "github", @@ -299,7 +299,6 @@ func TestClient_GetHookByUrl(t *testing.T) { email: "dummy@zapier.com", }, args: args{ - ctx: context.Background(), ownerAndRepoName: "https://dummy-bot:********@github.com/dummy-bot-zapier/test-repo.git", webhookUrl: "https://dummywebhooks.local", }, @@ -309,6 +308,7 @@ func TestClient_GetHookByUrl(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() c := &Client{ shurcoolClient: tt.fields.shurcoolClient, googleClient: tt.fields.googleClient, @@ -316,11 +316,11 @@ func TestClient_GetHookByUrl(t *testing.T) { username: tt.fields.username, email: tt.fields.email, } - got, err := c.GetHookByUrl(tt.args.ctx, tt.args.ownerAndRepoName, tt.args.webhookUrl) - if !tt.wantErr(t, err, fmt.Sprintf("GetHookByUrl(%v, %v, %v)", tt.args.ctx, tt.args.ownerAndRepoName, tt.args.webhookUrl)) { + got, err := c.GetHookByUrl(ctx, tt.args.ownerAndRepoName, tt.args.webhookUrl) + if !tt.wantErr(t, err, fmt.Sprintf("GetHookByUrl(%v, %v, %v)", ctx, tt.args.ownerAndRepoName, tt.args.webhookUrl)) { return } - assert.Equalf(t, tt.want, got, "GetHookByUrl(%v, %v, %v)", tt.args.ctx, tt.args.ownerAndRepoName, tt.args.webhookUrl) + assert.Equalf(t, tt.want, got, "GetHookByUrl(%v, %v, %v)", ctx, tt.args.ownerAndRepoName, tt.args.webhookUrl) }) } } diff --git a/pkg/vcs/github_client/emoji.go b/pkg/vcs/github_client/emoji.go index 5467b86d..dc2eea24 100644 --- a/pkg/vcs/github_client/emoji.go +++ b/pkg/vcs/github_client/emoji.go @@ -14,7 +14,7 @@ var stateEmoji = map[pkg.CommitState]string{ const defaultEmoji = ":interrobang:" -// ToEmoji returns a string representation of this state for use in the request +// ToEmoji returns a string representation of this state for use in the request. func (c *Client) ToEmoji(s pkg.CommitState) string { if emoji, ok := stateEmoji[s]; ok { return emoji diff --git a/pkg/vcs/github_client/message.go b/pkg/vcs/github_client/message.go index 41064a7f..674fa7c2 100644 --- a/pkg/vcs/github_client/message.go +++ b/pkg/vcs/github_client/message.go @@ -34,7 +34,6 @@ func (c *Client) PostMessage(ctx context.Context, pr vcs.PullRequest, message st pr.CheckID, &github.IssueComment{Body: &message}, ) - if err != nil { telemetry.SetError(span, err, "Create Pull Request comment") log.Error().Err(err).Msg("could not post message to PR") @@ -62,7 +61,6 @@ func (c *Client) UpdateMessage(ctx context.Context, m *msg.Message, msg string) int64(m.NoteID), &github.IssueComment{Body: &msg}, ) - if err != nil { telemetry.SetError(span, err, "Update Pull Request comment") log.Error().Err(err).Msgf("could not update message to PR, msg: %s, response: %+v", msg, resp) @@ -77,7 +75,7 @@ func (c *Client) UpdateMessage(ctx context.Context, m *msg.Message, msg string) // Pull all comments for the specified PR, and delete any comments that already exist from the bot // This is different from updating an existing message, as this will delete comments from previous runs of the bot -// Whereas updates occur mid-execution +// Whereas updates occur mid-execution. func (c *Client) pruneOldComments(ctx context.Context, pr vcs.PullRequest, comments []*github.IssueComment) error { _, span := tracer.Start(ctx, "pruneOldComments") defer span.End() @@ -126,7 +124,6 @@ func (c *Client) hideOutdatedMessages(ctx context.Context, pr vcs.PullRequest, c } return nil - } func (c *Client) TidyOutdatedComments(ctx context.Context, pr vcs.PullRequest) error { diff --git a/pkg/vcs/gitlab_client/backoff.go b/pkg/vcs/gitlab_client/backoff.go index c1a86b22..1a3df369 100644 --- a/pkg/vcs/gitlab_client/backoff.go +++ b/pkg/vcs/gitlab_client/backoff.go @@ -1,17 +1,16 @@ package gitlab_client import ( - "fmt" + "net/http" "time" "github.com/cenkalti/backoff/v4" - "github.com/rs/zerolog/log" + "github.com/pkg/errors" "github.com/xanzy/go-gitlab" ) -// getBackOff returns a backoff pointer to use to retry requests +// getBackOff returns a backoff pointer to use to retry requests. func getBackOff() *backoff.ExponentialBackOff { - // Lets setup backoff logic to retry this request for 1 minute bOff := backoff.NewExponentialBackOff() bOff.InitialInterval = 60 * time.Second @@ -22,23 +21,17 @@ func getBackOff() *backoff.ExponentialBackOff { return bOff } +var ErrRateLimited = errors.New("rate limited") + func checkReturnForBackoff(resp *gitlab.Response, err error) error { // if the error is nil lets check it out - if resp != nil { - if resp.StatusCode == 429 { - log.Warn().Msg("being rate limited doing backoff") - return fmt.Errorf("%s", "Rate Limited") - } + if resp != nil && resp.StatusCode == http.StatusTooManyRequests { + return ErrRateLimited } + if err != nil { - // Lets check the error and see if we need to trigger backoff - switch err.(type) { - default: - // If it is not one of the above errors lets skip the backoff logic - return &backoff.PermanentError{Err: err} - } + return &backoff.PermanentError{Err: err} } - // Return nil as the error passed in must have been nil as it passed the switch statement return nil } diff --git a/pkg/vcs/gitlab_client/client.go b/pkg/vcs/gitlab_client/client.go index 989f3d2d..49bf1f32 100644 --- a/pkg/vcs/gitlab_client/client.go +++ b/pkg/vcs/gitlab_client/client.go @@ -9,7 +9,7 @@ import ( "strconv" "strings" - "github.com/chainguard-dev/git-urls" + giturls "github.com/chainguard-dev/git-urls" "github.com/pkg/errors" "github.com/rs/zerolog/log" "github.com/xanzy/go-gitlab" @@ -19,8 +19,6 @@ import ( "github.com/zapier/kubechecks/pkg/vcs" ) -const GitlabTokenHeader = "X-Gitlab-Token" - type Client struct { c *gitlab.Client cfg config.ServerConfig @@ -74,10 +72,10 @@ func (c *Client) GetName() string { return "gitlab" } -// VerifyHook returns an err if the webhook isn't valid +// VerifyHook returns an err if the webhook isn't valid. func (c *Client) VerifyHook(r *http.Request, secret string) ([]byte, error) { // If we have a secret, and the secret doesn't match, return an error - if secret != "" && secret != r.Header.Get(GitlabTokenHeader) { + if secret != "" && secret != r.Header.Get("X-Gitlab-Token") { return nil, fmt.Errorf("invalid secret") } @@ -88,7 +86,7 @@ func (c *Client) VerifyHook(r *http.Request, secret string) ([]byte, error) { var nilPr vcs.PullRequest -// ParseHook parses and validates a webhook event; return an err if this isn't valid +// ParseHook parses and validates a webhook event; return an err if this isn't valid. func (c *Client) ParseHook(r *http.Request, request []byte) (vcs.PullRequest, error) { eventRequest, err := gitlab.ParseHook(gitlab.HookEventType(r), request) if err != nil { @@ -166,7 +164,6 @@ func (c *Client) CreateHook(ctx context.Context, repoName, webhookUrl, webhookSe MergeRequestsEvents: pkg.Pointer(true), Token: pkg.Pointer(webhookSecret), }) - if err != nil { return errors.Wrap(err, "failed to create project webhook") } diff --git a/pkg/vcs/gitlab_client/emoji.go b/pkg/vcs/gitlab_client/emoji.go index 8f19fd8c..ddc84f87 100644 --- a/pkg/vcs/gitlab_client/emoji.go +++ b/pkg/vcs/gitlab_client/emoji.go @@ -14,7 +14,7 @@ var stateEmoji = map[pkg.CommitState]string{ const defaultEmoji = ":interrobang:" -// ToEmoji returns a string representation of this state for use in the request +// ToEmoji returns a string representation of this state for use in the request. func (c *Client) ToEmoji(s pkg.CommitState) string { if emoji, ok := stateEmoji[s]; ok { return emoji diff --git a/pkg/vcs/gitlab_client/message.go b/pkg/vcs/gitlab_client/message.go index 38e38dc5..f3b3ac67 100644 --- a/pkg/vcs/gitlab_client/message.go +++ b/pkg/vcs/gitlab_client/message.go @@ -46,7 +46,6 @@ func (c *Client) hideOutdatedMessages(ctx context.Context, projectName string, m // loop through notes and collapse any that are from the current user for _, note := range notes { - // Do not try to hide the note if // note user is not the gitlabTokenUser // note is an internal system note such as notes on commit messages @@ -73,7 +72,6 @@ func (c *Client) hideOutdatedMessages(ctx context.Context, projectName string, m _, _, err := c.c.Notes.UpdateMergeRequestNote(projectName, mergeRequestID, note.ID, &gitlab.UpdateMergeRequestNoteOptions{ Body: &newBody, }) - if err != nil { telemetry.SetError(span, err, "Hide Existing Merge Request Check Note") return fmt.Errorf("could not hide note %d for merge request: %w", note.ID, err) @@ -94,7 +92,6 @@ func (c *Client) UpdateMessage(ctx context.Context, m *msg.Message, message stri n, _, err := c.c.Notes.UpdateMergeRequestNote(m.Name, m.CheckID, m.NoteID, &gitlab.UpdateMergeRequestNoteOptions{ Body: pkg.Pointer(message), }) - if err != nil { log.Error().Err(err).Msg("could not update message to MR") return err @@ -105,7 +102,7 @@ func (c *Client) UpdateMessage(ctx context.Context, m *msg.Message, message stri return nil } -// Iterate over all comments for the Merge Request, deleting any from the authenticated user +// Iterate over all comments for the Merge Request, deleting any from the authenticated user. func (c *Client) pruneOldComments(ctx context.Context, projectName string, mrID int, notes []*gitlab.Note) error { _, span := tracer.Start(ctx, "pruneOldComments") defer span.End() @@ -143,7 +140,6 @@ func (c *Client) TidyOutdatedComments(ctx context.Context, pr vcs.PullRequest) e Page: nextPage, }, }) - if err != nil { telemetry.SetError(span, err, "Tidy Outdated Comments") return fmt.Errorf("could not fetch notes for merge request: %w", err) @@ -159,5 +155,4 @@ func (c *Client) TidyOutdatedComments(ctx context.Context, pr vcs.PullRequest) e return c.pruneOldComments(ctx, pr.FullName, pr.CheckID, allNotes) } return c.hideOutdatedMessages(ctx, pr.FullName, pr.CheckID, allNotes) - } diff --git a/pkg/vcs/gitlab_client/pipeline.go b/pkg/vcs/gitlab_client/pipeline.go index 40bdd7e5..199112ab 100644 --- a/pkg/vcs/gitlab_client/pipeline.go +++ b/pkg/vcs/gitlab_client/pipeline.go @@ -17,7 +17,6 @@ func (c *Client) GetPipelinesForCommit(projectName string, commitSHA string) ([] } return pipelines, nil - } func (c *Client) GetLastPipelinesForCommit(projectName string, commitSHA string) *gitlab.PipelineInfo { diff --git a/pkg/vcs/gitlab_client/project.go b/pkg/vcs/gitlab_client/project.go index 374884e0..696117f0 100644 --- a/pkg/vcs/gitlab_client/project.go +++ b/pkg/vcs/gitlab_client/project.go @@ -11,7 +11,7 @@ import ( "github.com/zapier/kubechecks/pkg/repo_config" ) -// GetProjectByID gets a project by the given Project Name or ID +// GetProjectByID gets a project by the given Project Name or ID. func (c *Client) GetProjectByID(project int) (*gitlab.Project, error) { var proj *gitlab.Project err := backoff.Retry(func() error { diff --git a/pkg/vcs/gitlab_client/status.go b/pkg/vcs/gitlab_client/status.go index d17188bb..0c81b981 100644 --- a/pkg/vcs/gitlab_client/status.go +++ b/pkg/vcs/gitlab_client/status.go @@ -82,9 +82,8 @@ func (c *Client) setCommitStatus(projectWithNS string, commitSHA string, status return commitStatus, err } -// configureBackOff returns a backoff configuration to use to retry requests +// configureBackOff returns a backoff configuration to use to retry requests. func configureBackOff() *backoff.ExponentialBackOff { - // Lets setup backoff logic to retry this request for 30 seconds expBackOff := backoff.NewExponentialBackOff() expBackOff.MaxInterval = 10 * time.Second diff --git a/pkg/vcs/repo.go b/pkg/vcs/repo.go index 4ec42bb7..34730101 100644 --- a/pkg/vcs/repo.go +++ b/pkg/vcs/repo.go @@ -4,7 +4,7 @@ import ( "github.com/zapier/kubechecks/pkg/config" ) -// PullRequest represents an PR/MR +// PullRequest represents an PR/MR. type PullRequest struct { BaseRef string // base ref is the branch that the PR is being merged into HeadRef string // head ref is the branch that the PR is coming from diff --git a/pkg/vcs/types.go b/pkg/vcs/types.go index 8b0cf652..41d2aa20 100644 --- a/pkg/vcs/types.go +++ b/pkg/vcs/types.go @@ -14,7 +14,7 @@ type WebHookConfig struct { Events []string } -// Client represents a VCS client +// Client represents a VCS client. type Client interface { // PostMessage takes in project name in form "owner/repo" (ie zapier/kubechecks), the PR/MR id, and the actual message PostMessage(context.Context, PullRequest, string) *msg.Message diff --git a/telemetry/helpers.go b/telemetry/helpers.go index d27bb277..2536dbf5 100644 --- a/telemetry/helpers.go +++ b/telemetry/helpers.go @@ -44,7 +44,7 @@ func GetTraceID(ctx context.Context) string { return strconv.FormatUint(traceIDToUint64(tID), 10) } -// traceIDToUint64 converts 128bit traceId to 64 bit uint64 +// traceIDToUint64 converts 128bit traceId to 64 bit uint64. func traceIDToUint64(b [16]byte) uint64 { return binary.BigEndian.Uint64(b[len(b)-8:]) } diff --git a/telemetry/telemetry.go b/telemetry/telemetry.go index 531e801e..5747369d 100644 --- a/telemetry/telemetry.go +++ b/telemetry/telemetry.go @@ -23,17 +23,16 @@ import ( const DefaultMetricInterval = 2 type BaseTelemetry struct { - c context.Context traceProvider *sdktrace.TracerProvider metric *metric.MeterProvider } -func (bt *BaseTelemetry) Shutdown() { +func (bt *BaseTelemetry) Shutdown(ctx context.Context) { if bt.traceProvider != nil { - _ = bt.traceProvider.Shutdown(bt.c) + _ = bt.traceProvider.Shutdown(ctx) } if bt.metric != nil { - _ = bt.metric.Shutdown(bt.c) + _ = bt.metric.Shutdown(ctx) } } @@ -54,9 +53,8 @@ func (ot *OperatorTelemetry) StartMetricCollectors() error { func Init(ctx context.Context, serviceName, gitTag, gitCommit string, otelEnabled bool, otelHost, otelPort string) (*OperatorTelemetry, error) { log.Info().Msg("Initializing telemetry") bt := &OperatorTelemetry{ - BaseTelemetry: &BaseTelemetry{ - c: ctx, - }} + BaseTelemetry: &BaseTelemetry{}, + } res, err := resource.New(ctx, resource.WithAttributes( @@ -69,7 +67,7 @@ func Init(ctx context.Context, serviceName, gitTag, gitCommit string, otelEnable return nil, fmt.Errorf("failed to create resource: %w", err) } - err = bt.initProviders(res, otelEnabled, otelHost, otelPort) + err = bt.initProviders(ctx, res, otelEnabled, otelHost, otelPort) if err != nil { return bt, err } @@ -80,7 +78,7 @@ func Init(ctx context.Context, serviceName, gitTag, gitCommit string, otelEnable return bt, nil } -func createGRPCConn(ctx context.Context, enabled bool, otelHost string, otelPort string) (*grpc.ClientConn, error) { +func createGRPCConn(enabled bool, otelHost string, otelPort string) (*grpc.ClientConn, error) { if !enabled { log.Info().Msg("otel disabled") return nil, nil @@ -102,18 +100,18 @@ func createGRPCConn(ctx context.Context, enabled bool, otelHost string, otelPort return conn, err } -func (bt *BaseTelemetry) initProviders(res *resource.Resource, enabled bool, otelHost string, otelPort string) error { - conn, err := createGRPCConn(bt.c, enabled, otelHost, otelPort) +func (bt *BaseTelemetry) initProviders(ctx context.Context, res *resource.Resource, enabled bool, otelHost, otelPort string) error { + conn, err := createGRPCConn(enabled, otelHost, otelPort) if err != nil { return err } - err = bt.initTrace(conn, res) + err = bt.initTrace(ctx, conn, res) if err != nil { return err } - err = bt.initMetric(conn, res) + err = bt.initMetric(ctx, conn, res) if err != nil { return err } @@ -121,12 +119,12 @@ func (bt *BaseTelemetry) initProviders(res *resource.Resource, enabled bool, ote return nil } -func (bt *BaseTelemetry) initTrace(conn *grpc.ClientConn, res *resource.Resource) error { +func (bt *BaseTelemetry) initTrace(ctx context.Context, conn *grpc.ClientConn, res *resource.Resource) error { if conn == nil { return nil } - err := bt.initOTLPTrace(conn, res) + err := bt.initOTLPTrace(ctx, conn, res) if err != nil { log.Error().Err(err).Msg("unable to init tracer") } @@ -134,12 +132,12 @@ func (bt *BaseTelemetry) initTrace(conn *grpc.ClientConn, res *resource.Resource return nil } -func (bt *BaseTelemetry) initMetric(conn *grpc.ClientConn, res *resource.Resource) error { +func (bt *BaseTelemetry) initMetric(ctx context.Context, conn *grpc.ClientConn, res *resource.Resource) error { if conn == nil { return nil } - err := bt.initOTLPMetric(conn, res) + err := bt.initOTLPMetric(ctx, conn, res) if err != nil { log.Error().Err(err).Msg("unable to init metrics") return err @@ -148,9 +146,9 @@ func (bt *BaseTelemetry) initMetric(conn *grpc.ClientConn, res *resource.Resourc return nil } -func (bt *BaseTelemetry) initOTLPTrace(conn *grpc.ClientConn, res *resource.Resource) error { +func (bt *BaseTelemetry) initOTLPTrace(ctx context.Context, conn *grpc.ClientConn, res *resource.Resource) error { // tracer grpc client - traceExporter, err := otlptracegrpc.New(bt.c, + traceExporter, err := otlptracegrpc.New(ctx, otlptracegrpc.WithInsecure(), otlptracegrpc.WithGRPCConn(conn), ) @@ -171,9 +169,9 @@ func (bt *BaseTelemetry) initOTLPTrace(conn *grpc.ClientConn, res *resource.Reso return nil } -func (bt *BaseTelemetry) initOTLPMetric(conn *grpc.ClientConn, res *resource.Resource) error { +func (bt *BaseTelemetry) initOTLPMetric(ctx context.Context, conn *grpc.ClientConn, res *resource.Resource) error { mClient, err := otlpmetricgrpc.New( - bt.c, + ctx, otlpmetricgrpc.WithInsecure(), otlpmetricgrpc.WithGRPCConn(conn), )