From cda84c122e5584dd07bf855ceec331d21490314c Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Mon, 1 Dec 2025 17:39:36 +0800 Subject: [PATCH] feat: refactor plugin config validation and enhance job error handling - Refactor plugin configuration validation into a dedicated method with improved error messages - Add utility functions for trimming whitespace from slices and parsing key=value parameters - Improve error handling for missing or invalid job names and parameters - Enhance Exec to validate configuration, clean job list, and parse parameters before triggering jobs - Update job triggering to provide more descriptive error reporting - Replace deprecated and less precise test assertions with more explicit ones - Add extensive unit tests for configuration validation, whitespace trimming, parameter parsing, and job triggering (including multiple jobs, parameters, remote token, and job lists with whitespace) - Improve test coverage and reliability for plugin behavior and edge cases Signed-off-by: Bo-Yi Wu --- jenkins_test.go | 20 ++- plugin.go | 111 ++++++++++----- plugin_test.go | 356 ++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 428 insertions(+), 59 deletions(-) diff --git a/jenkins_test.go b/jenkins_test.go index 3715f45..8fba659 100644 --- a/jenkins_test.go +++ b/jenkins_test.go @@ -1,6 +1,8 @@ package main import ( + "net/http" + "net/http/httptest" "net/url" "testing" @@ -32,12 +34,24 @@ func TestUnSupportProtocol(t *testing.T) { } func TestTriggerBuild(t *testing.T) { + // Create a mock Jenkins server + var receivedParams url.Values + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedParams = r.URL.Query() + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + auth := &Auth{ Username: "foo", Token: "bar", } - jenkins := NewJenkins(auth, "http://example.com", "remote-token", false) + jenkins := NewJenkins(auth, server.URL, "remote-token", false) - err := jenkins.trigger("drone-jenkins", url.Values{"param": []string{"value"}}) - assert.Nil(t, err) + params := url.Values{"param": []string{"value"}} + err := jenkins.trigger("drone-jenkins", params) + + assert.NoError(t, err) + assert.Equal(t, "value", receivedParams.Get("param")) + assert.Equal(t, "remote-token", receivedParams.Get("token")) } diff --git a/plugin.go b/plugin.go index ca57a4a..1623c29 100644 --- a/plugin.go +++ b/plugin.go @@ -2,70 +2,115 @@ package main import ( "errors" + "fmt" "log" "net/url" "strings" ) type ( - // Plugin values. + // Plugin represents the configuration for the Jenkins plugin. + // It contains all necessary credentials and settings to trigger Jenkins jobs. Plugin struct { - BaseURL string - Username string - Token string - RemoteToken string - Job []string - Insecure bool - Parameters []string + BaseURL string // Jenkins server base URL + Username string // Jenkins username for authentication + Token string // Jenkins API token for authentication + RemoteToken string // Optional remote trigger token for additional security + Job []string // List of Jenkins job names to trigger + Insecure bool // Whether to skip TLS certificate verification + Parameters []string // Job parameters in key=value format } ) -func trimElement(keys []string) []string { - newKeys := []string{} +// trimWhitespaceFromSlice removes empty and whitespace-only strings from a slice. +// It returns a new slice containing only non-empty trimmed strings. +func trimWhitespaceFromSlice(items []string) []string { + result := make([]string, 0, len(items)) - for _, value := range keys { - value = strings.Trim(value, " ") - if len(value) == 0 { - continue + for _, item := range items { + trimmed := strings.TrimSpace(item) + if trimmed != "" { + result = append(result, trimmed) } - newKeys = append(newKeys, value) } - return newKeys + return result } -// Exec executes the plugin. +// parseParameters converts a slice of key=value strings into url.Values. +// It logs a warning for any parameters that don't match the expected format. +func parseParameters(params []string) url.Values { + values := url.Values{} + + for _, param := range params { + parts := strings.SplitN(param, "=", 2) + if len(parts) != 2 { + log.Printf("warning: skipping invalid parameter format (expected key=value): %q", param) + continue + } + + key := strings.TrimSpace(parts[0]) + value := parts[1] // Keep value as-is to preserve intentional spaces + + if key == "" { + log.Printf("warning: skipping parameter with empty key: %q", param) + continue + } + + values.Add(key, value) + } + + return values +} + +// validateConfig checks that all required plugin configuration is present. +// It returns a descriptive error if any required field is missing. +func (p Plugin) validateConfig() error { + if p.BaseURL == "" { + return errors.New("jenkins base URL is required") + } + if p.Username == "" { + return errors.New("jenkins username is required") + } + if p.Token == "" { + return errors.New("jenkins API token is required") + } + return nil +} + +// Exec executes the plugin by triggering the configured Jenkins jobs. +// It validates the configuration, parses parameters, and triggers each job sequentially. +// Returns an error if validation fails or any job trigger fails. func (p Plugin) Exec() error { - if len(p.BaseURL) == 0 || len(p.Username) == 0 || len(p.Token) == 0 { - return errors.New("missing jenkins config") + // Validate required configuration + if err := p.validateConfig(); err != nil { + return fmt.Errorf("configuration error: %w", err) } - jobs := trimElement(p.Job) - + // Clean and validate job list + jobs := trimWhitespaceFromSlice(p.Job) if len(jobs) == 0 { - return errors.New("missing jenkins job") + return errors.New("at least one Jenkins job name is required") } + // Set up authentication auth := &Auth{ Username: p.Username, Token: p.Token, } + // Initialize Jenkins client jenkins := NewJenkins(auth, p.BaseURL, p.RemoteToken, p.Insecure) - params := url.Values{} - for _, v := range p.Parameters { - kv := strings.Split(v, "=") - if len(kv) == 2 { - params.Add(kv[0], kv[1]) - } - } + // Parse job parameters + params := parseParameters(p.Parameters) - for _, v := range jobs { - if err := jenkins.trigger(v, params); err != nil { - return err + // Trigger each job + for _, jobName := range jobs { + if err := jenkins.trigger(jobName, params); err != nil { + return fmt.Errorf("failed to trigger job %q: %w", jobName, err) } - log.Printf("trigger job %s success", v) + log.Printf("successfully triggered job: %s", jobName) } return nil diff --git a/plugin_test.go b/plugin_test.go index e26fc2b..4e1bd72 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -1,48 +1,276 @@ package main import ( + "net/http" + "net/http/httptest" + "net/url" "testing" "github.com/stretchr/testify/assert" ) -func TestMissingConfig(t *testing.T) { +// TestValidateConfig tests the validateConfig method +func TestValidateConfig(t *testing.T) { + tests := []struct { + name string + plugin Plugin + wantError bool + errorMsg string + }{ + { + name: "missing all config", + plugin: Plugin{}, + wantError: true, + errorMsg: "jenkins base URL is required", + }, + { + name: "missing username and token", + plugin: Plugin{ + BaseURL: "http://example.com", + }, + wantError: true, + errorMsg: "jenkins username is required", + }, + { + name: "missing token", + plugin: Plugin{ + BaseURL: "http://example.com", + Username: "foo", + }, + wantError: true, + errorMsg: "jenkins API token is required", + }, + { + name: "all required config present", + plugin: Plugin{ + BaseURL: "http://example.com", + Username: "foo", + Token: "bar", + }, + wantError: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.plugin.validateConfig() + if tt.wantError { + assert.Error(t, err) + assert.Contains(t, err.Error(), tt.errorMsg) + } else { + assert.NoError(t, err) + } + }) + } +} + +// TestTrimWhitespaceFromSlice tests the trimWhitespaceFromSlice function +func TestTrimWhitespaceFromSlice(t *testing.T) { + tests := []struct { + name string + input []string + expected []string + }{ + { + name: "remove empty and whitespace strings", + input: []string{"1", " ", "3"}, + expected: []string{"1", "3"}, + }, + { + name: "no whitespace strings", + input: []string{"1", "2"}, + expected: []string{"1", "2"}, + }, + { + name: "all whitespace", + input: []string{" ", "\t", "\n"}, + expected: []string{}, + }, + { + name: "empty slice", + input: []string{}, + expected: []string{}, + }, + { + name: "trim surrounding whitespace", + input: []string{" foo ", " bar ", "baz"}, + expected: []string{"foo", "bar", "baz"}, + }, + { + name: "mixed empty and valid", + input: []string{"", "valid", "", "also-valid", ""}, + expected: []string{"valid", "also-valid"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := trimWhitespaceFromSlice(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +// TestParseParameters tests the parseParameters function +func TestParseParameters(t *testing.T) { + tests := []struct { + name string + input []string + expected url.Values + }{ + { + name: "valid parameters", + input: []string{"key1=value1", "key2=value2"}, + expected: url.Values{ + "key1": []string{"value1"}, + "key2": []string{"value2"}, + }, + }, + { + name: "parameter with multiple equals signs", + input: []string{"key=value=with=equals"}, + expected: url.Values{ + "key": []string{"value=with=equals"}, + }, + }, + { + name: "parameter with spaces in value", + input: []string{"key=value with spaces"}, + expected: url.Values{ + "key": []string{"value with spaces"}, + }, + }, + { + name: "parameter with empty value", + input: []string{"key="}, + expected: url.Values{ + "key": []string{""}, + }, + }, + { + name: "invalid parameter format (no equals)", + input: []string{"invalid"}, + expected: url.Values{}, + }, + { + name: "parameter with empty key", + input: []string{"=value"}, + expected: url.Values{}, + }, + { + name: "mixed valid and invalid", + input: []string{"valid=yes", "invalid", "also=valid"}, + expected: url.Values{ + "valid": []string{"yes"}, + "also": []string{"valid"}, + }, + }, + { + name: "key with surrounding whitespace", + input: []string{" key =value"}, + expected: url.Values{ + "key": []string{"value"}, + }, + }, + { + name: "empty slice", + input: []string{}, + expected: url.Values{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := parseParameters(tt.input) + assert.Equal(t, tt.expected, result) + }) + } +} + +// TestExecMissingConfig tests Exec with missing configuration +func TestExecMissingConfig(t *testing.T) { var plugin Plugin err := plugin.Exec() - assert.NotNil(t, err) + assert.Error(t, err) + assert.Contains(t, err.Error(), "configuration error") + assert.Contains(t, err.Error(), "jenkins base URL is required") } -func TestMissingJenkinsConfig(t *testing.T) { +// TestExecMissingJenkinsUsername tests Exec with missing username +func TestExecMissingJenkinsUsername(t *testing.T) { plugin := Plugin{ BaseURL: "http://example.com", } err := plugin.Exec() - assert.NotNil(t, err) + assert.Error(t, err) + assert.Contains(t, err.Error(), "configuration error") + assert.Contains(t, err.Error(), "jenkins username is required") } -func TestMissingJenkinsJob(t *testing.T) { +// TestExecMissingJenkinsToken tests Exec with missing token +func TestExecMissingJenkinsToken(t *testing.T) { plugin := Plugin{ BaseURL: "http://example.com", Username: "foo", - Token: "bar", } err := plugin.Exec() - assert.NotNil(t, err) - plugin.Job = []string{" "} - - err = plugin.Exec() - assert.NotNil(t, err) + assert.Error(t, err) + assert.Contains(t, err.Error(), "configuration error") + assert.Contains(t, err.Error(), "jenkins API token is required") } -func TestPluginTriggerBuild(t *testing.T) { +// TestExecMissingJenkinsJob tests Exec with missing or empty job list +func TestExecMissingJenkinsJob(t *testing.T) { + tests := []struct { + name string + jobs []string + }{ + { + name: "no jobs", + jobs: []string{}, + }, + { + name: "only whitespace jobs", + jobs: []string{" ", "\t", "\n"}, + }, + { + name: "nil jobs", + jobs: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + plugin := Plugin{ + BaseURL: "http://example.com", + Username: "foo", + Token: "bar", + Job: tt.jobs, + } + + err := plugin.Exec() + assert.Error(t, err) + assert.Contains(t, err.Error(), "at least one Jenkins job name is required") + }) + } +} + +// TestExecTriggerBuild tests successful job triggering +func TestExecTriggerBuild(t *testing.T) { + // Create a mock Jenkins server + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + plugin := Plugin{ - BaseURL: "http://example.com", + BaseURL: server.URL, Username: "foo", Token: "bar", Job: []string{"drone-jenkins"}, @@ -50,19 +278,101 @@ func TestPluginTriggerBuild(t *testing.T) { err := plugin.Exec() - assert.Nil(t, err) + assert.NoError(t, err) } -func TestTrimElement(t *testing.T) { - var input, result []string +// TestExecTriggerMultipleJobs tests triggering multiple jobs +func TestExecTriggerMultipleJobs(t *testing.T) { + // Create a mock Jenkins server + jobsTriggered := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + jobsTriggered++ + w.WriteHeader(http.StatusOK) + })) + defer server.Close() - input = []string{"1", " ", "3"} - result = []string{"1", "3"} + plugin := Plugin{ + BaseURL: server.URL, + Username: "foo", + Token: "bar", + Job: []string{"job1", "job2", "job3"}, + } - assert.Equal(t, result, trimElement(input)) + err := plugin.Exec() - input = []string{"1", "2"} - result = []string{"1", "2"} - - assert.Equal(t, result, trimElement(input)) + assert.NoError(t, err) + assert.Equal(t, 3, jobsTriggered) +} + +// TestExecWithParameters tests job triggering with parameters +func TestExecWithParameters(t *testing.T) { + // Create a mock Jenkins server + var receivedQuery url.Values + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedQuery = r.URL.Query() + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + plugin := Plugin{ + BaseURL: server.URL, + Username: "foo", + Token: "bar", + Job: []string{"parameterized-job"}, + Parameters: []string{"branch=main", "environment=production"}, + } + + err := plugin.Exec() + + assert.NoError(t, err) + assert.Equal(t, "main", receivedQuery.Get("branch")) + assert.Equal(t, "production", receivedQuery.Get("environment")) +} + +// TestExecWithRemoteToken tests job triggering with remote token +func TestExecWithRemoteToken(t *testing.T) { + // Create a mock Jenkins server + var receivedToken string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + receivedToken = r.URL.Query().Get("token") + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + plugin := Plugin{ + BaseURL: server.URL, + Username: "foo", + Token: "bar", + RemoteToken: "remote-token-123", + Job: []string{"secure-job"}, + } + + err := plugin.Exec() + + assert.NoError(t, err) + assert.Equal(t, "remote-token-123", receivedToken) +} + +// TestExecWithJobsContainingWhitespace tests job list with whitespace +func TestExecWithJobsContainingWhitespace(t *testing.T) { + // Create a mock Jenkins server + jobsTriggered := 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + jobsTriggered++ + w.WriteHeader(http.StatusOK) + })) + defer server.Close() + + plugin := Plugin{ + BaseURL: server.URL, + Username: "foo", + Token: "bar", + Job: []string{" job1 ", "job2", " ", "job3"}, + } + + err := plugin.Exec() + + assert.NoError(t, err) + // Should trigger 3 jobs (whitespace-only entry should be filtered out) + assert.Equal(t, 3, jobsTriggered) }