From e1985fadc9b0df6ddad524d445551e5e8d360727 Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Fri, 8 May 2026 22:49:00 +0800 Subject: [PATCH] refactor: extract repeated string literals into constants - Add tokenParam const in jenkins.go and reuse across main.go - Add shared test_helpers_test.go with test constants - Remove unused //nolint:gosec directive in jenkins.go - Resolve golangci-lint v2.12 goconst and nolintlint warnings --- jenkins.go | 10 ++-- jenkins_test.go | 82 ++++++++++++++--------------- main.go | 6 +-- plugin_test.go | 122 +++++++++++++++++++++---------------------- test_helpers_test.go | 18 +++++++ 5 files changed, 129 insertions(+), 109 deletions(-) create mode 100644 test_helpers_test.go diff --git a/jenkins.go b/jenkins.go index ca268b6..708c616 100644 --- a/jenkins.go +++ b/jenkins.go @@ -19,6 +19,8 @@ import ( "github.com/yassinebenaid/godump" ) +const tokenParam = "token" + type ( // Auth contain username and token Auth struct { @@ -233,7 +235,7 @@ func (jenkins *Jenkins) sendRequest( req.Header.Set(crumb.CrumbRequestField, crumb.Crumb) } - return jenkins.Client.Do(req) //nolint:gosec // user-configured Jenkins URL + return jenkins.Client.Do(req) } func (jenkins *Jenkins) get( @@ -487,14 +489,14 @@ func (jenkins *Jenkins) trigger(ctx context.Context, job string, params url.Valu if params == nil { params = url.Values{} } - params.Set("token", jenkins.Token) + params.Set(tokenParam, jenkins.Token) } var urlPath string // Check if params contains build parameters (excluding 'token') hasBuildParams := false for key := range params { - if key != "token" { + if key != tokenParam { hasBuildParams = true break } @@ -524,7 +526,7 @@ func (jenkins *Jenkins) trigger(ctx context.Context, job string, params url.Valu // Create a copy of params with masked token for display displayParams := url.Values{} for key, values := range params { - if key == "token" { + if key == tokenParam { // Mask token values for security displayParams[key] = []string{"***MASKED***"} } else { diff --git a/jenkins_test.go b/jenkins_test.go index 9d1e397..7286737 100644 --- a/jenkins_test.go +++ b/jenkins_test.go @@ -22,7 +22,7 @@ func TestParseJobPath(t *testing.T) { jenkins, err := NewJenkins( context.Background(), auth, - "http://example.com", + testExampleURL, "", false, "", @@ -38,8 +38,8 @@ func TestParseJobPath(t *testing.T) { func TestUnSupportProtocol(t *testing.T) { auth := &Auth{ - Username: "foo", - Token: "bar", + Username: testUserFoo, + Token: testUserBar, } jenkins, err := NewJenkins(context.Background(), auth, "example.com", "", false, "", false) assert.NoError(t, err) @@ -60,8 +60,8 @@ func TestTriggerBuild(t *testing.T) { defer server.Close() auth := &Auth{ - Username: "foo", - Token: "bar", + Username: testUserFoo, + Token: testUserBar, } jenkins, err := NewJenkins( context.Background(), @@ -129,8 +129,8 @@ func TestPostAndGetLocation(t *testing.T) { defer server.Close() auth := &Auth{ - Username: "test", - Token: "test", + Username: testUserName, + Token: testUserName, } jenkins, err := NewJenkins(context.Background(), auth, server.URL, "", false, "", false) assert.NoError(t, err) @@ -206,8 +206,8 @@ func TestGetQueueItem(t *testing.T) { defer server.Close() auth := &Auth{ - Username: "test", - Token: "test", + Username: testUserName, + Token: testUserName, } jenkins, err := NewJenkins(context.Background(), auth, server.URL, "", false, "", false) assert.NoError(t, err) @@ -242,7 +242,7 @@ func TestGetBuildInfo(t *testing.T) { }{ { name: "build in progress", - jobName: "test-job", + jobName: testJobName, buildNumber: 123, responseBody: `{"number":123,"building":true,"duration":0,"result":null,` + `"url":"http://jenkins.example.com/job/test-job/123/"}`, @@ -253,7 +253,7 @@ func TestGetBuildInfo(t *testing.T) { }, { name: "build completed successfully", - jobName: "test-job", + jobName: testJobName, buildNumber: 124, responseBody: `{"number":124,"building":false,"duration":5000,"result":"SUCCESS",` + `"url":"http://jenkins.example.com/job/test-job/124/"}`, @@ -264,7 +264,7 @@ func TestGetBuildInfo(t *testing.T) { }, { name: "build failed", - jobName: "test-job", + jobName: testJobName, buildNumber: 125, responseBody: `{"number":125,"building":false,"duration":3000,"result":"FAILURE",` + `"url":"http://jenkins.example.com/job/test-job/125/"}`, @@ -275,7 +275,7 @@ func TestGetBuildInfo(t *testing.T) { }, { name: "build not found", - jobName: "test-job", + jobName: testJobName, buildNumber: 999, responseBody: "Not Found", responseStatus: http.StatusNotFound, @@ -295,8 +295,8 @@ func TestGetBuildInfo(t *testing.T) { defer server.Close() auth := &Auth{ - Username: "test", - Token: "test", + Username: testUserName, + Token: testUserName, } jenkins, err := NewJenkins(context.Background(), auth, server.URL, "", false, "", false) assert.NoError(t, err) @@ -358,15 +358,15 @@ func TestWaitForCompletion(t *testing.T) { defer server.Close() auth := &Auth{ - Username: "test", - Token: "test", + Username: testUserName, + Token: testUserName, } jenkins, err := NewJenkins(context.Background(), auth, server.URL, "", false, "", false) assert.NoError(t, err) buildInfo, err := jenkins.waitForCompletion( context.Background(), - "test-job", + testJobName, queueID, 100*time.Millisecond, 5*time.Second, @@ -392,15 +392,15 @@ func TestWaitForCompletion(t *testing.T) { defer server.Close() auth := &Auth{ - Username: "test", - Token: "test", + Username: testUserName, + Token: testUserName, } jenkins, err := NewJenkins(context.Background(), auth, server.URL, "", false, "", false) assert.NoError(t, err) buildInfo, err := jenkins.waitForCompletion( context.Background(), - "test-job", + testJobName, queueID, 50*time.Millisecond, 200*time.Millisecond, @@ -434,15 +434,15 @@ func TestWaitForCompletion(t *testing.T) { defer server.Close() auth := &Auth{ - Username: "test", - Token: "test", + Username: testUserName, + Token: testUserName, } jenkins, err := NewJenkins(context.Background(), auth, server.URL, "", false, "", false) assert.NoError(t, err) buildInfo, err := jenkins.waitForCompletion( context.Background(), - "test-job", + testJobName, queueID, 50*time.Millisecond, 200*time.Millisecond, @@ -485,15 +485,15 @@ func TestWaitForCompletion(t *testing.T) { defer server.Close() auth := &Auth{ - Username: "test", - Token: "test", + Username: testUserName, + Token: testUserName, } jenkins, err := NewJenkins(context.Background(), auth, server.URL, "", false, "", false) assert.NoError(t, err) buildInfo, err := jenkins.waitForCompletion( context.Background(), - "test-job", + testJobName, queueID, 50*time.Millisecond, 5*time.Second, @@ -626,8 +626,8 @@ func TestLoadCACert(t *testing.T) { func TestNewJenkinsWithCACert(t *testing.T) { t.Run("with valid CA certificate", func(t *testing.T) { auth := &Auth{ - Username: "test", - Token: "test", + Username: testUserName, + Token: testUserName, } jenkins, err := NewJenkins( context.Background(), @@ -650,8 +650,8 @@ func TestNewJenkinsWithCACert(t *testing.T) { assert.NoError(t, err) auth := &Auth{ - Username: "test", - Token: "test", + Username: testUserName, + Token: testUserName, } jenkins, err := NewJenkins( context.Background(), @@ -668,8 +668,8 @@ func TestNewJenkinsWithCACert(t *testing.T) { t.Run("with invalid CA certificate content", func(t *testing.T) { auth := &Auth{ - Username: "test", - Token: "test", + Username: testUserName, + Token: testUserName, } jenkins, err := NewJenkins( context.Background(), @@ -687,8 +687,8 @@ func TestNewJenkinsWithCACert(t *testing.T) { t.Run("with invalid PEM format", func(t *testing.T) { auth := &Auth{ - Username: "test", - Token: "test", + Username: testUserName, + Token: testUserName, } invalidPEM := "-----BEGIN CERTIFICATE-----\ninvalid-base64-data\n-----END CERTIFICATE-----" jenkins, err := NewJenkins( @@ -707,8 +707,8 @@ func TestNewJenkinsWithCACert(t *testing.T) { t.Run("with nonexistent file path", func(t *testing.T) { auth := &Auth{ - Username: "test", - Token: "test", + Username: testUserName, + Token: testUserName, } jenkins, err := NewJenkins( context.Background(), @@ -726,8 +726,8 @@ func TestNewJenkinsWithCACert(t *testing.T) { t.Run("insecure flag takes precedence over CA cert", func(t *testing.T) { auth := &Auth{ - Username: "test", - Token: "test", + Username: testUserName, + Token: testUserName, } // When insecure is true, CA cert should be ignored jenkins, err := NewJenkins( @@ -745,8 +745,8 @@ func TestNewJenkinsWithCACert(t *testing.T) { t.Run("without CA certificate uses default client", func(t *testing.T) { auth := &Auth{ - Username: "test", - Token: "test", + Username: testUserName, + Token: testUserName, } jenkins, err := NewJenkins( context.Background(), diff --git a/main.go b/main.go index afb5152..1126307 100644 --- a/main.go +++ b/main.go @@ -71,7 +71,7 @@ func main() { EnvVars: []string{"PLUGIN_USER", "JENKINS_USER", "INPUT_USER"}, }, &cli.StringFlag{ - Name: "token", + Name: tokenParam, Aliases: []string{"t"}, Usage: "jenkins API token for authentication", EnvVars: []string{"PLUGIN_TOKEN", "JENKINS_TOKEN", "INPUT_TOKEN"}, @@ -175,7 +175,7 @@ func run(c *cli.Context) error { } // Validate authentication: either (user + token) or remote-token must be provided - hasUserAuth := c.String("user") != "" && c.String("token") != "" + hasUserAuth := c.String("user") != "" && c.String(tokenParam) != "" hasRemoteToken := c.String("remote-token") != "" if !hasUserAuth && !hasRemoteToken { @@ -185,7 +185,7 @@ func run(c *cli.Context) error { plugin := Plugin{ BaseURL: c.String("host"), Username: c.String("user"), - Token: c.String("token"), + Token: c.String(tokenParam), RemoteToken: c.String("remote-token"), Job: c.StringSlice("job"), Insecure: c.Bool("insecure"), diff --git a/plugin_test.go b/plugin_test.go index 06d2d59..2d9231e 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -34,53 +34,53 @@ func TestValidateConfig(t *testing.T) { { name: "missing authentication", plugin: Plugin{ - BaseURL: "http://example.com", + BaseURL: testExampleURL, }, wantError: true, - errorMsg: "authentication required", + errorMsg: testAuthRequiredErr, }, { name: "missing token (only username)", plugin: Plugin{ - BaseURL: "http://example.com", - Username: "foo", + BaseURL: testExampleURL, + Username: testUserFoo, }, wantError: true, - errorMsg: "authentication required", + errorMsg: testAuthRequiredErr, }, { name: "missing username (only token)", plugin: Plugin{ - BaseURL: "http://example.com", - Token: "bar", + BaseURL: testExampleURL, + Token: testUserBar, }, wantError: true, - errorMsg: "authentication required", + errorMsg: testAuthRequiredErr, }, { name: "user and token auth", plugin: Plugin{ - BaseURL: "http://example.com", - Username: "foo", - Token: "bar", + BaseURL: testExampleURL, + Username: testUserFoo, + Token: testUserBar, }, wantError: false, }, { name: "remote token auth", plugin: Plugin{ - BaseURL: "http://example.com", - RemoteToken: "remote-token-123", + BaseURL: testExampleURL, + RemoteToken: testRemoteTokenValue, }, wantError: false, }, { name: "both auth methods", plugin: Plugin{ - BaseURL: "http://example.com", - Username: "foo", - Token: "bar", - RemoteToken: "remote-token-123", + BaseURL: testExampleURL, + Username: testUserFoo, + Token: testUserBar, + RemoteToken: testRemoteTokenValue, }, wantError: false, }, @@ -118,7 +118,7 @@ func TestTrimWhitespaceFromSlice(t *testing.T) { }, { name: "all whitespace", - input: []string{" ", "\t", "\n"}, + input: []string{testWhitespaceVal, "\t", "\n"}, expected: []string{}, }, { @@ -129,12 +129,12 @@ func TestTrimWhitespaceFromSlice(t *testing.T) { { name: "trim surrounding whitespace", input: []string{" foo ", " bar ", "baz"}, - expected: []string{"foo", "bar", "baz"}, + expected: []string{testUserFoo, testUserBar, "baz"}, }, { name: "mixed empty and valid", - input: []string{"", "valid", "", "also-valid", ""}, - expected: []string{"valid", "also-valid"}, + input: []string{"", testValidStr, "", "also-valid", ""}, + expected: []string{testValidStr, "also-valid"}, }, } @@ -157,29 +157,29 @@ func TestParseParameters(t *testing.T) { name: "valid parameters", input: "key1=value1\nkey2=value2", expected: url.Values{ - "key1": []string{"value1"}, - "key2": []string{"value2"}, + testParamKey1: []string{testParamValue1}, + testParamKey2: []string{testParamValue2}, }, }, { name: "parameter with multiple equals signs", input: "key=value=with=equals", expected: url.Values{ - "key": []string{"value=with=equals"}, + testParamKey: []string{"value=with=equals"}, }, }, { name: "parameter with spaces in value", input: "key=value with spaces", expected: url.Values{ - "key": []string{"value with spaces"}, + testParamKey: []string{"value with spaces"}, }, }, { name: "parameter with empty value", input: "key=", expected: url.Values{ - "key": []string{""}, + testParamKey: []string{""}, }, }, { @@ -196,15 +196,15 @@ func TestParseParameters(t *testing.T) { name: "mixed valid and invalid", input: "valid=yes\ninvalid\nalso=valid", expected: url.Values{ - "valid": []string{"yes"}, - "also": []string{"valid"}, + testValidStr: []string{"yes"}, + "also": []string{testValidStr}, }, }, { name: "key with surrounding whitespace", input: " key =value", expected: url.Values{ - "key": []string{"value"}, + testParamKey: []string{"value"}, }, }, { @@ -216,16 +216,16 @@ func TestParseParameters(t *testing.T) { name: "multiple empty lines", input: "key1=value1\n\n\nkey2=value2", expected: url.Values{ - "key1": []string{"value1"}, - "key2": []string{"value2"}, + testParamKey1: []string{testParamValue1}, + testParamKey2: []string{testParamValue2}, }, }, { name: "lines with whitespace only", input: "key1=value1\n \n\t\nkey2=value2", expected: url.Values{ - "key1": []string{"value1"}, - "key2": []string{"value2"}, + testParamKey1: []string{testParamValue1}, + testParamKey2: []string{testParamValue2}, }, }, } @@ -252,28 +252,28 @@ func TestExecMissingConfig(t *testing.T) { // TestExecMissingJenkinsUsername tests Exec with missing username func TestExecMissingJenkinsUsername(t *testing.T) { plugin := Plugin{ - BaseURL: "http://example.com", + BaseURL: testExampleURL, } err := plugin.Exec(context.Background()) assert.Error(t, err) assert.Contains(t, err.Error(), "configuration error") - assert.Contains(t, err.Error(), "authentication required") + assert.Contains(t, err.Error(), testAuthRequiredErr) } // TestExecMissingJenkinsToken tests Exec with missing token func TestExecMissingJenkinsToken(t *testing.T) { plugin := Plugin{ - BaseURL: "http://example.com", - Username: "foo", + BaseURL: testExampleURL, + Username: testUserFoo, } err := plugin.Exec(context.Background()) assert.Error(t, err) assert.Contains(t, err.Error(), "configuration error") - assert.Contains(t, err.Error(), "authentication required") + assert.Contains(t, err.Error(), testAuthRequiredErr) } // TestExecMissingJenkinsJob tests Exec with missing or empty job list @@ -288,7 +288,7 @@ func TestExecMissingJenkinsJob(t *testing.T) { }, { name: "only whitespace jobs", - jobs: []string{" ", "\t", "\n"}, + jobs: []string{testWhitespaceVal, "\t", "\n"}, }, { name: "nil jobs", @@ -299,9 +299,9 @@ func TestExecMissingJenkinsJob(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { plugin := Plugin{ - BaseURL: "http://example.com", - Username: "foo", - Token: "bar", + BaseURL: testExampleURL, + Username: testUserFoo, + Token: testUserBar, Job: tt.jobs, } @@ -326,8 +326,8 @@ func TestExecTriggerBuild(t *testing.T) { plugin := Plugin{ BaseURL: server.URL, - Username: "foo", - Token: "bar", + Username: testUserFoo, + Token: testUserBar, Job: []string{"drone-jenkins"}, } @@ -353,8 +353,8 @@ func TestExecTriggerMultipleJobs(t *testing.T) { plugin := Plugin{ BaseURL: server.URL, - Username: "foo", - Token: "bar", + Username: testUserFoo, + Token: testUserBar, Job: []string{"job1", "job2", "job3"}, } @@ -377,8 +377,8 @@ func TestExecWithParameters(t *testing.T) { plugin := Plugin{ BaseURL: server.URL, - Username: "foo", - Token: "bar", + Username: testUserFoo, + Token: testUserBar, Job: []string{"parameterized-job"}, Parameters: "branch=main\nenvironment=production", } @@ -403,16 +403,16 @@ func TestExecWithRemoteToken(t *testing.T) { plugin := Plugin{ BaseURL: server.URL, - Username: "foo", - Token: "bar", - RemoteToken: "remote-token-123", + Username: testUserFoo, + Token: testUserBar, + RemoteToken: testRemoteTokenValue, Job: []string{"secure-job"}, } err := plugin.Exec(context.Background()) assert.NoError(t, err) - assert.Equal(t, "remote-token-123", receivedToken) + assert.Equal(t, testRemoteTokenValue, receivedToken) } // TestExecWithJobsContainingWhitespace tests job list with whitespace @@ -432,9 +432,9 @@ func TestExecWithJobsContainingWhitespace(t *testing.T) { plugin := Plugin{ BaseURL: server.URL, - Username: "foo", - Token: "bar", - Job: []string{" job1 ", "job2", " ", "job3"}, + Username: testUserFoo, + Token: testUserBar, + Job: []string{" job1 ", "job2", testWhitespaceVal, "job3"}, } err := plugin.Exec(context.Background()) @@ -467,9 +467,9 @@ func TestExecWithWaitSuccess(t *testing.T) { plugin := Plugin{ BaseURL: server.URL, - Username: "foo", - Token: "bar", - Job: []string{"test-job"}, + Username: testUserFoo, + Token: testUserBar, + Job: []string{testJobName}, Wait: true, } @@ -501,9 +501,9 @@ func TestExecWithWaitFailure(t *testing.T) { plugin := Plugin{ BaseURL: server.URL, - Username: "foo", - Token: "bar", - Job: []string{"test-job"}, + Username: testUserFoo, + Token: testUserBar, + Job: []string{testJobName}, Wait: true, } diff --git a/test_helpers_test.go b/test_helpers_test.go new file mode 100644 index 0000000..1ca063b --- /dev/null +++ b/test_helpers_test.go @@ -0,0 +1,18 @@ +package main + +const ( + testUserFoo = "foo" + testUserBar = "bar" + testUserName = "test" + testJobName = "test-job" + testExampleURL = "http://example.com" + testAuthRequiredErr = "authentication required" + testRemoteTokenValue = "remote-token-123" + testWhitespaceVal = " " + testValidStr = "valid" + testParamKey = "key" + testParamKey1 = "key1" + testParamKey2 = "key2" + testParamValue1 = "value1" + testParamValue2 = "value2" +)