Compare commits

...

2 Commits

Author SHA1 Message Date
appleboy 1fbf3e5cd6 style: streamline authentication error handling in config validation
- Simplify authentication error message in config validation

Signed-off-by: appleboy <appleboy.tw@gmail.com>
2025-12-27 11:23:25 +08:00
appleboy 95a5eb125f feat: refactor authentication logic and broaden test coverage
- Improve authentication checks to only require username and token when both are provided
- Update validation logic to allow either (username and token) or remote-token for authentication
- Enhance test coverage for various authentication scenarios
- Refine error messages to indicate a generic authentication requirement instead of specifying missing username or token

Signed-off-by: appleboy <appleboy.tw@gmail.com>
2025-12-27 11:13:06 +08:00
3 changed files with 58 additions and 21 deletions
+9 -5
View File
@@ -224,7 +224,7 @@ func (jenkins *Jenkins) sendRequest(
req *http.Request, req *http.Request,
crumb *CrumbResponse, crumb *CrumbResponse,
) (*http.Response, error) { ) (*http.Response, error) {
if jenkins.Auth != nil { if jenkins.Auth != nil && jenkins.Auth.Username != "" && jenkins.Auth.Token != "" {
req.SetBasicAuth(jenkins.Auth.Username, jenkins.Auth.Token) req.SetBasicAuth(jenkins.Auth.Username, jenkins.Auth.Token)
} }
@@ -277,10 +277,14 @@ func (jenkins *Jenkins) postAndGetLocation(
path string, path string,
params url.Values, params url.Values,
) (int, error) { ) (int, error) {
// Fetch CSRF crumb before POST request // Fetch CSRF crumb before POST request (only if authenticated)
crumb, err := jenkins.getCrumb(ctx) var crumb *CrumbResponse
if err != nil { if jenkins.Auth != nil && jenkins.Auth.Username != "" && jenkins.Auth.Token != "" {
return 0, fmt.Errorf("failed to get crumb: %w", err) var err error
crumb, err = jenkins.getCrumb(ctx)
if err != nil {
return 0, fmt.Errorf("failed to get crumb: %w", err)
}
} }
requestURL := jenkins.buildURL(path, params) requestURL := jenkins.buildURL(path, params)
+15 -9
View File
@@ -88,12 +88,15 @@ func (p Plugin) validateConfig() error {
if p.BaseURL == "" { if p.BaseURL == "" {
return errors.New("jenkins base URL is required") return errors.New("jenkins base URL is required")
} }
if p.Username == "" {
return errors.New("jenkins username is required") // Validate authentication: either (user + token) or remote-token must be provided
} hasUserAuth := p.Username != "" && p.Token != ""
if p.Token == "" { hasRemoteToken := p.RemoteToken != ""
return errors.New("jenkins API token is required")
if !hasUserAuth && !hasRemoteToken {
return errors.New("authentication required")
} }
return nil return nil
} }
@@ -113,10 +116,13 @@ func (p Plugin) Exec(ctx context.Context) error {
return errors.New("at least one Jenkins job name is required") return errors.New("at least one Jenkins job name is required")
} }
// Set up authentication // Set up authentication (only if username and token are provided)
auth := &Auth{ var auth *Auth
Username: p.Username, if p.Username != "" && p.Token != "" {
Token: p.Token, auth = &Auth{
Username: p.Username,
Token: p.Token,
}
} }
// Initialize Jenkins client // Initialize Jenkins client
+34 -7
View File
@@ -32,24 +32,33 @@ func TestValidateConfig(t *testing.T) {
errorMsg: "jenkins base URL is required", errorMsg: "jenkins base URL is required",
}, },
{ {
name: "missing username and token", name: "missing authentication",
plugin: Plugin{ plugin: Plugin{
BaseURL: "http://example.com", BaseURL: "http://example.com",
}, },
wantError: true, wantError: true,
errorMsg: "jenkins username is required", errorMsg: "authentication required",
}, },
{ {
name: "missing token", name: "missing token (only username)",
plugin: Plugin{ plugin: Plugin{
BaseURL: "http://example.com", BaseURL: "http://example.com",
Username: "foo", Username: "foo",
}, },
wantError: true, wantError: true,
errorMsg: "jenkins API token is required", errorMsg: "authentication required",
}, },
{ {
name: "all required config present", name: "missing username (only token)",
plugin: Plugin{
BaseURL: "http://example.com",
Token: "bar",
},
wantError: true,
errorMsg: "authentication required",
},
{
name: "user and token auth",
plugin: Plugin{ plugin: Plugin{
BaseURL: "http://example.com", BaseURL: "http://example.com",
Username: "foo", Username: "foo",
@@ -57,6 +66,24 @@ func TestValidateConfig(t *testing.T) {
}, },
wantError: false, wantError: false,
}, },
{
name: "remote token auth",
plugin: Plugin{
BaseURL: "http://example.com",
RemoteToken: "remote-token-123",
},
wantError: false,
},
{
name: "both auth methods",
plugin: Plugin{
BaseURL: "http://example.com",
Username: "foo",
Token: "bar",
RemoteToken: "remote-token-123",
},
wantError: false,
},
} }
for _, tt := range tests { for _, tt := range tests {
@@ -232,7 +259,7 @@ func TestExecMissingJenkinsUsername(t *testing.T) {
assert.Error(t, err) assert.Error(t, err)
assert.Contains(t, err.Error(), "configuration error") assert.Contains(t, err.Error(), "configuration error")
assert.Contains(t, err.Error(), "jenkins username is required") assert.Contains(t, err.Error(), "authentication required")
} }
// TestExecMissingJenkinsToken tests Exec with missing token // TestExecMissingJenkinsToken tests Exec with missing token
@@ -246,7 +273,7 @@ func TestExecMissingJenkinsToken(t *testing.T) {
assert.Error(t, err) assert.Error(t, err)
assert.Contains(t, err.Error(), "configuration error") assert.Contains(t, err.Error(), "configuration error")
assert.Contains(t, err.Error(), "jenkins API token is required") assert.Contains(t, err.Error(), "authentication required")
} }
// TestExecMissingJenkinsJob tests Exec with missing or empty job list // TestExecMissingJenkinsJob tests Exec with missing or empty job list