From 25216c28965440874ef5a35a62e09c4c03545f15 Mon Sep 17 00:00:00 2001 From: Joseph Lombrozo Date: Fri, 23 Feb 2024 21:13:52 -0500 Subject: [PATCH 1/5] whoops Signed-off-by: Joseph Lombrozo --- cmd/container.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/container.go b/cmd/container.go index b39cd989..d076eeab 100644 --- a/cmd/container.go +++ b/cmd/container.go @@ -22,9 +22,9 @@ func newContainer(ctx context.Context, cfg config.ServerConfig) (container.Conta ctr.Config = cfg switch cfg.VcsType { - case "github": - ctr.VcsClient, err = gitlab_client.CreateGitlabClient(cfg) case "gitlab": + ctr.VcsClient, err = gitlab_client.CreateGitlabClient(cfg) + case "github": ctr.VcsClient, err = github_client.CreateGithubClient(cfg) default: err = fmt.Errorf("unknown vcs-type: %q", cfg.VcsType) From a40c17e857ab554f5880543d0751a6ab6139bbb7 Mon Sep 17 00:00:00 2001 From: djeebus Date: Mon, 26 Feb 2024 09:45:54 -0500 Subject: [PATCH 2/5] add some debugging --- cmd/container.go | 10 ++++++---- pkg/argo_client/client.go | 18 +++++++++++++----- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/cmd/container.go b/cmd/container.go index d076eeab..04621960 100644 --- a/cmd/container.go +++ b/cmd/container.go @@ -30,22 +30,24 @@ func newContainer(ctx context.Context, cfg config.ServerConfig) (container.Conta err = fmt.Errorf("unknown vcs-type: %q", cfg.VcsType) } if err != nil { - return ctr, err + return ctr, errors.Wrap(err, "failed to create vcs client") } - ctr.ArgoClient = argo_client.NewArgoClient(cfg) + if ctr.ArgoClient, err = argo_client.NewArgoClient(cfg); err != nil { + return ctr, errors.Wrap(err, "failed to create argo client") + } vcsToArgoMap := appdir.NewVcsToArgoMap() ctr.VcsToArgoMap = vcsToArgoMap if cfg.MonitorAllApplications { if err = buildAppsMap(ctx, ctr.ArgoClient, ctr.VcsToArgoMap); err != nil { - return ctr, err + return ctr, errors.Wrap(err, "failed to build apps map") } ctr.ApplicationWatcher, err = app_watcher.NewApplicationWatcher(vcsToArgoMap) if err != nil { - return ctr, err + return ctr, errors.Wrap(err, "failed to create watch applications") } go ctr.ApplicationWatcher.Run(ctx, 1) diff --git a/pkg/argo_client/client.go b/pkg/argo_client/client.go index a281ea29..fda0df09 100644 --- a/pkg/argo_client/client.go +++ b/pkg/argo_client/client.go @@ -18,21 +18,29 @@ type ArgoClient struct { client apiclient.Client } -func NewArgoClient(cfg config.ServerConfig) *ArgoClient { - clientOptions := &apiclient.ClientOptions{ +func NewArgoClient(cfg config.ServerConfig) (*ArgoClient, error) { + opts := &apiclient.ClientOptions{ ServerAddr: cfg.ArgoCDServerAddr, AuthToken: cfg.ArgoCDToken, GRPCWebRootPath: cfg.ArgoCDPathPrefix, Insecure: cfg.ArgoCDInsecure, } - argo, err := apiclient.NewClient(clientOptions) + + log.Info(). + Str("server-addr", opts.ServerAddr). + Int("auth-token", len(opts.AuthToken)). + Str("grpc-web-root-path", opts.GRPCWebRootPath). + Bool("insecure", cfg.ArgoCDInsecure). + Msg("ArgoCD client configuration") + + argo, err := apiclient.NewClient(opts) if err != nil { - log.Fatal().Err(err).Msg("could not create ArgoCD API client") + return nil, err } return &ArgoClient{ client: argo, - } + }, nil } // GetApplicationClient has related argocd diff code https://github.com/argoproj/argo-cd/blob/d3ff9757c460ae1a6a11e1231251b5d27aadcdd1/cmd/argocd/commands/app.go#L899 From bb38dfbe56ec6847fe4bb1087b3e341ebb557ecb Mon Sep 17 00:00:00 2001 From: djeebus Date: Mon, 26 Feb 2024 13:45:10 -0500 Subject: [PATCH 3/5] fix unused arg, log err --- cmd/container.go | 5 +++-- pkg/app_watcher/app_watcher.go | 7 +++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/cmd/container.go b/cmd/container.go index 04621960..bf977b0f 100644 --- a/cmd/container.go +++ b/cmd/container.go @@ -18,8 +18,9 @@ import ( func newContainer(ctx context.Context, cfg config.ServerConfig) (container.Container, error) { var err error - ctr := container.Container{} - ctr.Config = cfg + var ctr = container.Container{ + Config: cfg, + } switch cfg.VcsType { case "gitlab": diff --git a/pkg/app_watcher/app_watcher.go b/pkg/app_watcher/app_watcher.go index 302a048e..997a55d8 100644 --- a/pkg/app_watcher/app_watcher.go +++ b/pkg/app_watcher/app_watcher.go @@ -40,6 +40,7 @@ func NewApplicationWatcher(vcsToArgoMap appdir.VcsToArgoMap) (*ApplicationWatche ctrl := ApplicationWatcher{ applicationClientset: appClient, + vcsToArgoMap: vcsToArgoMap, } appInformer, appLister := ctrl.newApplicationInformerAndLister(time.Second * 30) @@ -126,13 +127,15 @@ func (ctrl *ApplicationWatcher) newApplicationInformerAndLister(refreshTimeout t ) lister := applisters.NewApplicationLister(informer.GetIndexer()) - informer.AddEventHandler( + if _, err := informer.AddEventHandler( cache.ResourceEventHandlerFuncs{ AddFunc: ctrl.onApplicationAdded, UpdateFunc: ctrl.onApplicationUpdated, DeleteFunc: ctrl.onApplicationDeleted, }, - ) + ); err != nil { + log.Error().Err(err).Msg("failed to add event handlers") + } return informer, lister } From f4c11e87e5acf197579bfd1b96c21c99299e4787 Mon Sep 17 00:00:00 2001 From: djeebus Date: Mon, 26 Feb 2024 14:50:59 -0500 Subject: [PATCH 4/5] more clean up, pass config around --- pkg/vcs/github_client/client.go | 9 +++++++-- pkg/vcs/github_client/message.go | 2 +- pkg/vcs/gitlab_client/client.go | 23 +++++++++++++---------- pkg/vcs/repo.go | 7 +++++-- pkg/vcs/repo_test.go | 6 ++++++ 5 files changed, 32 insertions(+), 15 deletions(-) diff --git a/pkg/vcs/github_client/client.go b/pkg/vcs/github_client/client.go index b184d0ee..c9f60cdd 100644 --- a/pkg/vcs/github_client/client.go +++ b/pkg/vcs/github_client/client.go @@ -22,9 +22,9 @@ import ( type Client struct { shurcoolClient *githubv4.Client googleClient *github.Client + cfg config.ServerConfig - tidyOutdatedCommentsMode string - username, email string + username, email string } // CreateGithubClient creates a new GitHub client using the auth token provided. We @@ -68,6 +68,7 @@ func CreateGithubClient(cfg config.ServerConfig) (*Client, error) { } client := &Client{ + cfg: cfg, googleClient: googleClient, shurcoolClient: shurcoolClient, } @@ -147,6 +148,8 @@ func (c *Client) buildRepoFromEvent(event *github.PullRequestEvent) *vcs.Repo { Username: c.username, Email: c.email, Labels: labels, + + Config: c.cfg, } } @@ -313,6 +316,8 @@ func (c *Client) LoadHook(ctx context.Context, id string) (*vcs.Repo, error) { Username: userName, Email: userEmail, Labels: labels, + + Config: c.cfg, }, nil } diff --git a/pkg/vcs/github_client/message.go b/pkg/vcs/github_client/message.go index f5497c76..cb3f5c96 100644 --- a/pkg/vcs/github_client/message.go +++ b/pkg/vcs/github_client/message.go @@ -154,7 +154,7 @@ func (c *Client) TidyOutdatedComments(ctx context.Context, repo *vcs.Repo) error nextPage = resp.NextPage } - if strings.ToLower(c.tidyOutdatedCommentsMode) == "delete" { + if strings.ToLower(c.cfg.TidyOutdatedCommentsMode) == "delete" { return c.pruneOldComments(ctx, repo, allComments) } return c.hideOutdatedMessages(ctx, repo, allComments) diff --git a/pkg/vcs/gitlab_client/client.go b/pkg/vcs/gitlab_client/client.go index 4c653858..21e45f58 100644 --- a/pkg/vcs/gitlab_client/client.go +++ b/pkg/vcs/gitlab_client/client.go @@ -22,10 +22,10 @@ import ( const GitlabTokenHeader = "X-Gitlab-Token" type Client struct { - *gitlab.Client + c *gitlab.Client + cfg config.ServerConfig - tidyOutdatedCommentsMode string - username, email string + username, email string } func CreateGitlabClient(cfg config.ServerConfig) (*Client, error) { @@ -54,11 +54,10 @@ func CreateGitlabClient(cfg config.ServerConfig) (*Client, error) { } client := &Client{ - Client: c, + c: c, + cfg: cfg, username: user.Username, email: user.Email, - - tidyOutdatedCommentsMode: cfg.TidyOutdatedCommentsMode, } if client.username == "" { client.username = vcs.DefaultVcsUsername @@ -132,7 +131,7 @@ func (c *Client) GetHookByUrl(ctx context.Context, repoName, webhookUrl string) if err != nil { return nil, errors.Wrap(err, "failed to parse repo url") } - webhooks, _, err := c.Client.Projects.ListProjectHooks(pid, nil) + webhooks, _, err := c.c.Projects.ListProjectHooks(pid, nil) if err != nil { return nil, errors.Wrap(err, "failed to list project webhooks") } @@ -160,7 +159,7 @@ func (c *Client) CreateHook(ctx context.Context, repoName, webhookUrl, webhookSe return errors.Wrap(err, "failed to parse repo name") } - _, _, err = c.Client.Projects.AddProjectHook(pid, &gitlab.AddProjectHookOptions{ + _, _, err = c.c.Projects.AddProjectHook(pid, &gitlab.AddProjectHookOptions{ URL: pkg.Pointer(webhookUrl), MergeRequestsEvents: pkg.Pointer(true), Token: pkg.Pointer(webhookSecret), @@ -187,12 +186,12 @@ func (c *Client) LoadHook(ctx context.Context, id string) (*vcs.Repo, error) { return nil, errors.Wrap(err, "failed to parse merge request number") } - project, _, err := c.Projects.GetProject(repoPath, nil) + project, _, err := c.c.Projects.GetProject(repoPath, nil) if err != nil { return nil, errors.Wrapf(err, "failed to get project '%s'", repoPath) } - mergeRequest, _, err := c.MergeRequests.GetMergeRequest(repoPath, int(mrNumber), nil) + mergeRequest, _, err := c.c.MergeRequests.GetMergeRequest(repoPath, int(mrNumber), nil) if err != nil { return nil, errors.Wrapf(err, "failed to get merge request '%d' in project '%s'", mrNumber, repoPath) } @@ -212,6 +211,8 @@ func (c *Client) LoadHook(ctx context.Context, id string) (*vcs.Repo, error) { Username: c.username, Email: c.email, Labels: mergeRequest.Labels, + + Config: c.cfg, }, nil } @@ -234,5 +235,7 @@ func (c *Client) buildRepoFromEvent(event *gitlab.MergeEvent) *vcs.Repo { Username: c.username, Email: c.email, Labels: labels, + + Config: c.cfg, } } diff --git a/pkg/vcs/repo.go b/pkg/vcs/repo.go index 9b561b4b..fb31f530 100644 --- a/pkg/vcs/repo.go +++ b/pkg/vcs/repo.go @@ -39,7 +39,7 @@ type Repo struct { Email string // Email of auth'd client Labels []string // Labels associated with the MR/PR - cfg config.ServerConfig + Config config.ServerConfig } func (r *Repo) CloneRepoLocal(ctx context.Context, repoDir string) error { @@ -208,13 +208,16 @@ func walk(s string, d fs.DirEntry, err error) error { } func (r *Repo) execCommand(name string, args ...string) *exec.Cmd { - cmd := execCommand(r.cfg, name, args...) + cmd := execCommand(r.Config, name, args...) cmd.Dir = r.RepoDir return cmd } func censorVcsToken(cfg config.ServerConfig, args []string) []string { vcsToken := cfg.VcsToken + if len(vcsToken) == 0 { + return args + } var argsToLog []string for _, arg := range args { diff --git a/pkg/vcs/repo_test.go b/pkg/vcs/repo_test.go index f913473e..37df302d 100644 --- a/pkg/vcs/repo_test.go +++ b/pkg/vcs/repo_test.go @@ -72,3 +72,9 @@ func TestCensorVcsToken(t *testing.T) { result := censorVcsToken(cfg, []string{"one", "two", "three"}) assert.Equal(t, []string{"one", "two", "t********e"}, result) } + +func TestCensorEmptyVcsToken(t *testing.T) { + cfg := config.ServerConfig{VcsToken: ""} + result := censorVcsToken(cfg, []string{"one", "two", "three"}) + assert.Equal(t, []string{"one", "two", "te"}, result) +} From 0a73e21158002e1ff820dfd92d4dc3d7aa5c70ae Mon Sep 17 00:00:00 2001 From: djeebus Date: Tue, 27 Feb 2024 09:49:12 -0500 Subject: [PATCH 5/5] fix client issues --- pkg/vcs/gitlab_client/merge.go | 2 +- pkg/vcs/gitlab_client/message.go | 12 ++++++------ pkg/vcs/gitlab_client/pipeline.go | 2 +- pkg/vcs/gitlab_client/project.go | 6 +++--- pkg/vcs/gitlab_client/status.go | 2 +- pkg/vcs/repo_test.go | 2 +- 6 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pkg/vcs/gitlab_client/merge.go b/pkg/vcs/gitlab_client/merge.go index 5fc328f7..087d08d0 100644 --- a/pkg/vcs/gitlab_client/merge.go +++ b/pkg/vcs/gitlab_client/merge.go @@ -27,7 +27,7 @@ func (c *Client) GetMergeChanges(ctx context.Context, projectId int, mergeReqId defer span.End() var changes []*Changes - diffs, _, err := c.MergeRequests.ListMergeRequestDiffs(projectId, mergeReqId, &gitlab.ListMergeRequestDiffsOptions{}) + diffs, _, err := c.c.MergeRequests.ListMergeRequestDiffs(projectId, mergeReqId, &gitlab.ListMergeRequestDiffsOptions{}) if err != nil { telemetry.SetError(span, err, "Get MergeRequest Changes") return changes, err diff --git a/pkg/vcs/gitlab_client/message.go b/pkg/vcs/gitlab_client/message.go index aaeb9264..492a9625 100644 --- a/pkg/vcs/gitlab_client/message.go +++ b/pkg/vcs/gitlab_client/message.go @@ -26,7 +26,7 @@ func (c *Client) PostMessage(ctx context.Context, repo *vcs.Repo, mergeRequestID message = message[:MaxCommentLength] } - n, _, err := c.Notes.CreateMergeRequestNote( + n, _, err := c.c.Notes.CreateMergeRequestNote( repo.FullName, mergeRequestID, &gitlab.CreateMergeRequestNoteOptions{ Body: pkg.Pointer(message), @@ -71,7 +71,7 @@ func (c *Client) hideOutdatedMessages(ctx context.Context, projectName string, m log.Debug().Str("projectName", projectName).Int("mr", mergeRequestID).Msgf("Updating comment %d as outdated", note.ID) - _, _, err := c.Notes.UpdateMergeRequestNote(projectName, mergeRequestID, note.ID, &gitlab.UpdateMergeRequestNoteOptions{ + _, _, err := c.c.Notes.UpdateMergeRequestNote(projectName, mergeRequestID, note.ID, &gitlab.UpdateMergeRequestNoteOptions{ Body: &newBody, }) @@ -92,7 +92,7 @@ func (c *Client) UpdateMessage(ctx context.Context, m *msg.Message, message stri message = message[:MaxCommentLength] } - n, _, err := c.Notes.UpdateMergeRequestNote(m.Name, m.CheckID, m.NoteID, &gitlab.UpdateMergeRequestNoteOptions{ + n, _, err := c.c.Notes.UpdateMergeRequestNote(m.Name, m.CheckID, m.NoteID, &gitlab.UpdateMergeRequestNoteOptions{ Body: pkg.Pointer(message), }) @@ -116,7 +116,7 @@ func (c *Client) pruneOldComments(ctx context.Context, projectName string, mrID for _, note := range notes { if note.Author.Username == c.username { log.Debug().Int("mr", mrID).Int("note", note.ID).Msg("deleting old comment") - _, err := c.Notes.DeleteMergeRequestNote(projectName, mrID, note.ID) + _, err := c.c.Notes.DeleteMergeRequestNote(projectName, mrID, note.ID) if err != nil { telemetry.SetError(span, err, "Prune Old Comments") return fmt.Errorf("could not delete old comment: %w", err) @@ -137,7 +137,7 @@ func (c *Client) TidyOutdatedComments(ctx context.Context, repo *vcs.Repo) error for { // list merge request notes - notes, resp, err := c.Notes.ListMergeRequestNotes(repo.FullName, repo.CheckID, &gitlab.ListMergeRequestNotesOptions{ + notes, resp, err := c.c.Notes.ListMergeRequestNotes(repo.FullName, repo.CheckID, &gitlab.ListMergeRequestNotesOptions{ Sort: pkg.Pointer("asc"), OrderBy: pkg.Pointer("created_at"), ListOptions: gitlab.ListOptions{ @@ -156,7 +156,7 @@ func (c *Client) TidyOutdatedComments(ctx context.Context, repo *vcs.Repo) error nextPage = resp.NextPage } - if strings.ToLower(c.tidyOutdatedCommentsMode) == "delete" { + if strings.ToLower(c.cfg.TidyOutdatedCommentsMode) == "delete" { return c.pruneOldComments(ctx, repo.FullName, repo.CheckID, allNotes) } return c.hideOutdatedMessages(ctx, repo.FullName, repo.CheckID, allNotes) diff --git a/pkg/vcs/gitlab_client/pipeline.go b/pkg/vcs/gitlab_client/pipeline.go index 41e26bb4..40bdd7e5 100644 --- a/pkg/vcs/gitlab_client/pipeline.go +++ b/pkg/vcs/gitlab_client/pipeline.go @@ -8,7 +8,7 @@ import ( ) func (c *Client) GetPipelinesForCommit(projectName string, commitSHA string) ([]*gitlab.PipelineInfo, error) { - pipelines, _, err := c.Pipelines.ListProjectPipelines(projectName, &gitlab.ListProjectPipelinesOptions{ + pipelines, _, err := c.c.Pipelines.ListProjectPipelines(projectName, &gitlab.ListProjectPipelinesOptions{ SHA: pkg.Pointer(commitSHA), }) if err != nil { diff --git a/pkg/vcs/gitlab_client/project.go b/pkg/vcs/gitlab_client/project.go index 67489b12..930254fb 100644 --- a/pkg/vcs/gitlab_client/project.go +++ b/pkg/vcs/gitlab_client/project.go @@ -12,13 +12,13 @@ import ( "github.com/zapier/kubechecks/pkg/repo_config" ) -// GetProjectByIDorName 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 { var err error var resp *gitlab.Response - proj, resp, err = c.Projects.GetProject(project, nil) + proj, resp, err = c.c.Projects.GetProject(project, nil) return checkReturnForBackoff(resp, err) }, getBackOff()) return proj, err @@ -30,7 +30,7 @@ func (c *Client) GetRepoConfigFile(ctx context.Context, projectId int, mergeReqI // check MR branch for _, file := range repo_config.RepoConfigFilenameVariations() { - b, _, err := c.RepositoryFiles.GetRawFile( + b, _, err := c.c.RepositoryFiles.GetRawFile( projectId, file, &gitlab.GetRawFileOptions{Ref: pkg.Pointer("HEAD")}, diff --git a/pkg/vcs/gitlab_client/status.go b/pkg/vcs/gitlab_client/status.go index 126e170d..7f6a3874 100644 --- a/pkg/vcs/gitlab_client/status.go +++ b/pkg/vcs/gitlab_client/status.go @@ -78,7 +78,7 @@ func convertState(state pkg.CommitState) gitlab.BuildStateValue { } func (c *Client) setCommitStatus(projectWithNS string, commitSHA string, status *gitlab.SetCommitStatusOptions) (*gitlab.CommitStatus, error) { - commitStatus, _, err := c.Commits.SetCommitStatus(projectWithNS, commitSHA, status) + commitStatus, _, err := c.c.Commits.SetCommitStatus(projectWithNS, commitSHA, status) return commitStatus, err } diff --git a/pkg/vcs/repo_test.go b/pkg/vcs/repo_test.go index 37df302d..25f4fb50 100644 --- a/pkg/vcs/repo_test.go +++ b/pkg/vcs/repo_test.go @@ -76,5 +76,5 @@ func TestCensorVcsToken(t *testing.T) { func TestCensorEmptyVcsToken(t *testing.T) { cfg := config.ServerConfig{VcsToken: ""} result := censorVcsToken(cfg, []string{"one", "two", "three"}) - assert.Equal(t, []string{"one", "two", "te"}, result) + assert.Equal(t, []string{"one", "two", "three"}, result) }