From c2c14baab77353d2e04988bd74643338a0541993 Mon Sep 17 00:00:00 2001 From: Drew Stinnett Date: Wed, 28 Sep 2022 13:16:39 -0400 Subject: [PATCH 1/4] bugfix: handle mixing of job and normal tokens --- internal/client/gitlab.go | 28 ++++++++++++++++- internal/client/gitlab_test.go | 56 ++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 1 deletion(-) diff --git a/internal/client/gitlab.go b/internal/client/gitlab.go index b664da1b82d..c1a761381d8 100644 --- a/internal/client/gitlab.go +++ b/internal/client/gitlab.go @@ -47,7 +47,9 @@ func NewGitLab(ctx *context.Context, token string) (Client, error) { var client *gitlab.Client var err error - if ctx.Config.GitLabURLs.UseJobToken { + useJobClient := checkUseJobToken(*ctx, token) + + if useJobClient { client, err = gitlab.NewJobClient(token, options...) } else { client, err = gitlab.NewClient(token, options...) @@ -495,3 +497,27 @@ func (c *gitlabClient) getMilestoneByTitle(repo Repo, title string) (*gitlab.Mil return nil, nil } + +// checkUseJobToken examines the context and given token, and determines if We should use NewJobClient vs NewClient +func checkUseJobToken(ctx context.Context, token string) bool { + // The CI_JOB_TOKEN env var is set automatically in all GitLab runners. + // If this comes back as empty, we aren't in a functional GitLab runner + ciToken := os.Getenv("CI_JOB_TOKEN") + if ciToken == "" { + return false + } + + // We only want to use the JobToken client if we have specified + // UseJobToken. Older versions of GitLab don't work with this, so we + // want to be specific + if ctx.Config.GitLabURLs.UseJobToken { + // We may be creating a new client with a non-CI_JOB_TOKEN, for + // things like Homebrew publishing. We can't use the + // CI_JOB_TOKEN there + if token != ciToken { + return false + } + return true + } + return false +} diff --git a/internal/client/gitlab_test.go b/internal/client/gitlab_test.go index df06f9942c6..a054de1a117 100644 --- a/internal/client/gitlab_test.go +++ b/internal/client/gitlab_test.go @@ -546,3 +546,59 @@ func TestCloseMileston(t *testing.T) { err = client.CloseMilestone(ctx, repo, "never-will-exist") require.Error(t, err) } + +func TestCheckUseJobToken(t *testing.T) { + tests := []struct { + ctx context.Context + token string + ciToken string + want bool + desc string + }{ + { + ctx: context.Context{ + Config: config.Project{ + GitLabURLs: config.GitLabURLs{ + UseJobToken: true, + }, + }, + }, + token: "real-ci-token", + ciToken: "real-ci-token", + desc: "token and CI_JOB_TOKEN match so should return true", + want: true, + }, + { + ctx: context.Context{ + Config: config.Project{ + GitLabURLs: config.GitLabURLs{ + UseJobToken: true, + }, + }, + }, + token: "some-random-token", + ciToken: "real-ci-token", + desc: "token and CI_JOB_TOKEN do NOT match so should return false", + want: false, + }, + { + ctx: context.Context{ + Config: config.Project{ + GitLabURLs: config.GitLabURLs{ + UseJobToken: false, + }, + }, + }, + token: "real-ci-token", + ciToken: "real-ci-token", + desc: "token and CI_JOB_TOKEN match, however UseJobToken is set to false, so return false", + want: false, + }, + } + for _, tt := range tests { + os.Setenv("CI_JOB_TOKEN", tt.ciToken) + got := checkUseJobToken(tt.ctx, tt.token) + require.Equal(t, tt.want, got, tt.desc) + os.Unsetenv("CI_JOB_TOKEN") + } +} From 3c56f7be12b4002d8834c5dba4d65fbcba9e1c1c Mon Sep 17 00:00:00 2001 From: Drew Stinnett Date: Wed, 28 Sep 2022 14:58:05 -0400 Subject: [PATCH 2/4] refactor: streamline return bool Co-authored-by: Carlos Alexandro Becker --- internal/client/gitlab.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/internal/client/gitlab.go b/internal/client/gitlab.go index c1a761381d8..c7bb8bb175c 100644 --- a/internal/client/gitlab.go +++ b/internal/client/gitlab.go @@ -514,10 +514,7 @@ func checkUseJobToken(ctx context.Context, token string) bool { // We may be creating a new client with a non-CI_JOB_TOKEN, for // things like Homebrew publishing. We can't use the // CI_JOB_TOKEN there - if token != ciToken { - return false - } - return true + return token == ciToken } return false } From fc9d017671259cf2ed2b5b3a2db4e8e14ddb1362 Mon Sep 17 00:00:00 2001 From: Drew Stinnett Date: Wed, 28 Sep 2022 14:58:22 -0400 Subject: [PATCH 3/4] refactor: streamline useJobClient Co-authored-by: Carlos Alexandro Becker --- internal/client/gitlab.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/internal/client/gitlab.go b/internal/client/gitlab.go index c7bb8bb175c..e10adb1e18a 100644 --- a/internal/client/gitlab.go +++ b/internal/client/gitlab.go @@ -47,9 +47,7 @@ func NewGitLab(ctx *context.Context, token string) (Client, error) { var client *gitlab.Client var err error - useJobClient := checkUseJobToken(*ctx, token) - - if useJobClient { + if checkUseJobToken(*ctx, token) { client, err = gitlab.NewJobClient(token, options...) } else { client, err = gitlab.NewClient(token, options...) From aeb1959d32af3d6b089d72e5667ed0e321ea77cc Mon Sep 17 00:00:00 2001 From: Drew Stinnett Date: Wed, 28 Sep 2022 14:59:06 -0400 Subject: [PATCH 4/4] refactor: better testing environment set --- internal/client/gitlab_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/client/gitlab_test.go b/internal/client/gitlab_test.go index a054de1a117..87fc12f1dc6 100644 --- a/internal/client/gitlab_test.go +++ b/internal/client/gitlab_test.go @@ -596,9 +596,8 @@ func TestCheckUseJobToken(t *testing.T) { }, } for _, tt := range tests { - os.Setenv("CI_JOB_TOKEN", tt.ciToken) + t.Setenv("CI_JOB_TOKEN", tt.ciToken) got := checkUseJobToken(tt.ctx, tt.token) require.Equal(t, tt.want, got, tt.desc) - os.Unsetenv("CI_JOB_TOKEN") } }