From c7548576f058b8f5462f1564b1c5a4e916a4adcb Mon Sep 17 00:00:00 2001 From: Bo-Yi Wu Date: Wed, 8 Apr 2026 23:01:00 +0800 Subject: [PATCH] fix: resolve all golangci-lint errors for stricter linter config - Replace interface{} with any and fix value receiver mutation in log method - Use fmt.Fprintln instead of forbidden fmt.Println in Exec output - Fix appendAssign by splitting append and assignment into two lines - Replace assert.Nil/NotNil with assert.NoError/Error for error checks - Upgrade error assertions to require when followed by dependent assertions - Replace os.Setenv with t.Setenv in all test functions - Restructure TestFindEnvs to use t.Setenv with unique prefixes Co-Authored-By: Claude Opus 4.6 (1M context) --- .golangci.yaml | 121 +++++++++++++++++++++++++++++++----------- plugin.go | 24 +++++---- plugin_test.go | 141 +++++++++++++++++++++++-------------------------- 3 files changed, 170 insertions(+), 116 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index a3c8de0..dab57df 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -1,35 +1,89 @@ version: "2" +output: + sort-order: + - file linters: + default: none enable: - - asciicheck - - durationcheck - - errorlint - - gosec - - misspell + - bidichk + - bodyclose + - depguard + - errcheck + - forbidigo + - gocheckcompilerdirectives + - gocritic + - govet + - ineffassign + - mirror + - modernize - nakedret - - nilerr + - nilnil - nolintlint - perfsprint - revive + - staticcheck + - testifylint + - unconvert + - unparam + - unused - usestdlibvars + - usetesting - wastedassign settings: - gosec: - includes: - - G102 - - G106 - - G108 - - G109 - - G111 - - G112 - - G201 - - G203 + depguard: + rules: + main: + deny: + - pkg: io/ioutil + desc: use os or io instead + - pkg: golang.org/x/exp + desc: it's experimental and unreliable + - pkg: github.com/pkg/errors + desc: use builtin errors package instead + nolintlint: + allow-unused: false + require-explanation: true + require-specific: true + gocritic: + enabled-checks: + - equalFold + disabled-checks: [] + revive: + severity: error + rules: + - name: blank-imports + - name: constant-logical-expr + - name: context-as-argument + - name: context-keys-type + - name: dot-imports + - name: empty-lines + - name: error-return + - name: error-strings + - name: exported + - name: identical-branches + - name: if-return + - name: increment-decrement + - name: modifies-value-receiver + - name: package-comments + - name: redefines-builtin-id + - name: superfluous-else + - name: time-naming + - name: unexported-return + - name: var-declaration + - name: var-naming + disabled: true + staticcheck: + checks: + - all + testifylint: {} + usetesting: + os-temp-dir: true perfsprint: - int-conversion: true - err-error: true - errorf: true - sprintf1: true - strconcat: true + concat-loop: false + govet: + enable: + - nilness + - unusedwrite exclusions: generated: lax presets: @@ -37,19 +91,24 @@ linters: - common-false-positives - legacy - std-error-handling - paths: - - third_party$ - - builtin$ - - examples$ + rules: + - linters: + - errcheck + - staticcheck + - unparam + path: _test\.go +issues: + max-issues-per-linter: 0 + max-same-issues: 0 formatters: enable: - - gci - gofmt - - goimports + - gofumpt - golines + settings: + gofumpt: + extra-rules: true exclusions: generated: lax - paths: - - third_party$ - - builtin$ - - examples$ +run: + timeout: 10m diff --git a/plugin.go b/plugin.go index 87d89f8..66064f4 100644 --- a/plugin.go +++ b/plugin.go @@ -128,7 +128,8 @@ func (p Plugin) exec(host string, wg *sync.WaitGroup, errChannel chan error) { } } - p.Config.Script = append(env, p.scriptCommands()...) + env = append(env, p.scriptCommands()...) + p.Config.Script = env if p.Config.Debug && len(env) > 0 { p.log(host, "======ENV======") @@ -181,16 +182,17 @@ func (p Plugin) format(format string, args ...string) string { } // log output to console -func (p Plugin) log(host string, message ...interface{}) { - if p.Writer == nil { - p.Writer = os.Stdout +func (p Plugin) log(host string, message ...any) { + w := p.Writer + if w == nil { + w = os.Stdout } if count := len(p.Config.Host); count == 1 { - fmt.Fprintf(p.Writer, "%s", fmt.Sprintln(message...)) + fmt.Fprintf(w, "%s", fmt.Sprintln(message...)) return } - fmt.Fprintf(p.Writer, "%s: %s", host, fmt.Sprintln(message...)) + fmt.Fprintf(w, "%s: %s", host, fmt.Sprintln(message...)) } // Exec executes the plugin. @@ -238,9 +240,13 @@ func (p Plugin) Exec() error { } } - fmt.Println("===============================================") - fmt.Println("✅ Successfully executed commands to all hosts.") - fmt.Println("===============================================") + w := p.Writer + if w == nil { + w = os.Stdout + } + fmt.Fprintln(w, "===============================================") + fmt.Fprintln(w, "✅ Successfully executed commands to all hosts.") + fmt.Fprintln(w, "===============================================") return nil } diff --git a/plugin_test.go b/plugin_test.go index abd6d74..47f32ac 100644 --- a/plugin_test.go +++ b/plugin_test.go @@ -12,6 +12,7 @@ import ( "github.com/appleboy/easyssh-proxy" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/testcontainers/testcontainers-go" "github.com/testcontainers/testcontainers-go/wait" "golang.org/x/crypto/ssh" @@ -22,7 +23,7 @@ func TestMissingHostOrUser(t *testing.T) { err := plugin.Exec() - assert.NotNil(t, err) + require.Error(t, err) assert.Equal(t, errMissingHost, err) } @@ -37,7 +38,7 @@ func TestMissingKeyOrPassword(t *testing.T) { err := plugin.Exec() - assert.NotNil(t, err) + require.Error(t, err) assert.Equal(t, errMissingPasswordOrKey, err) } @@ -54,7 +55,7 @@ func TestIncorrectPassword(t *testing.T) { } err := plugin.Exec() - assert.NotNil(t, err) + assert.Error(t, err) } func TestSSHScriptFromRawKey(t *testing.T) { @@ -97,7 +98,7 @@ ib4KbP5ovZlrjL++akMQ7V2fHzuQIFWnCkDA5c2ZAqzlM+ZN+HRG7gWur7Bt4XH1 } err := plugin.Exec() - assert.Nil(t, err) + assert.NoError(t, err) } func TestSSHScriptFromKeyFile(t *testing.T) { @@ -113,7 +114,7 @@ func TestSSHScriptFromKeyFile(t *testing.T) { } err := plugin.Exec() - assert.Nil(t, err) + assert.NoError(t, err) } func TestSSHIPv4Only(t *testing.T) { @@ -130,7 +131,7 @@ func TestSSHIPv4Only(t *testing.T) { } err := plugin.Exec() - assert.Nil(t, err) + assert.NoError(t, err) } func TestSSHIPv6OnlyError(t *testing.T) { @@ -147,7 +148,7 @@ func TestSSHIPv6OnlyError(t *testing.T) { } err := plugin.Exec() - assert.NotNil(t, err) + assert.Error(t, err) } func TestStreamFromSSHCommand(t *testing.T) { @@ -167,7 +168,7 @@ func TestStreamFromSSHCommand(t *testing.T) { } err := plugin.Exec() - assert.Nil(t, err) + assert.NoError(t, err) } func TestSSHScriptWithError(t *testing.T) { @@ -184,7 +185,7 @@ func TestSSHScriptWithError(t *testing.T) { err := plugin.Exec() // Process exited with status 1 - assert.NotNil(t, err) + assert.Error(t, err) } func TestSSHCommandTimeOut(t *testing.T) { @@ -200,7 +201,7 @@ func TestSSHCommandTimeOut(t *testing.T) { } err := plugin.Exec() - assert.NotNil(t, err) + assert.Error(t, err) } func TestProxyCommand(t *testing.T) { @@ -222,7 +223,7 @@ func TestProxyCommand(t *testing.T) { } err := plugin.Exec() - assert.Nil(t, err) + assert.NoError(t, err) } func TestSSHCommandError(t *testing.T) { @@ -238,7 +239,7 @@ func TestSSHCommandError(t *testing.T) { } err := plugin.Exec() - assert.NotNil(t, err) + assert.Error(t, err) } func TestSSHCommandExitCodeError(t *testing.T) { @@ -260,11 +261,11 @@ func TestSSHCommandExitCodeError(t *testing.T) { } err := plugin.Exec() - assert.NotNil(t, err) + assert.Error(t, err) } func TestSetENV(t *testing.T) { - os.Setenv("FOO", `' 1) '`) + t.Setenv("FOO", `' 1) '`) plugin := Plugin{ Config: Config{ Host: []string{"localhost"}, @@ -285,12 +286,12 @@ func TestSetENV(t *testing.T) { } err := plugin.Exec() - assert.Nil(t, err) + assert.NoError(t, err) } func TestSetExistingENV(t *testing.T) { - os.Setenv("FOO", "Value for foo") - os.Setenv("BAR", "") + t.Setenv("FOO", "Value for foo") + t.Setenv("BAR", "") plugin := Plugin{ Config: Config{ Host: []string{"localhost"}, @@ -318,7 +319,7 @@ func TestSetExistingENV(t *testing.T) { } err := plugin.Exec() - assert.Nil(t, err) + assert.NoError(t, err) } func TestSyncMode(t *testing.T) { @@ -339,7 +340,7 @@ func TestSyncMode(t *testing.T) { } err := plugin.Exec() - assert.Nil(t, err) + assert.NoError(t, err) } func Test_escapeArg(t *testing.T) { @@ -416,7 +417,7 @@ func TestCommandOutput(t *testing.T) { } err := plugin.Exec() - assert.Nil(t, err) + require.NoError(t, err) assert.Equal(t, unindent(expected), unindent(buffer.String())) } @@ -439,7 +440,7 @@ func TestWrongFingerprint(t *testing.T) { } err := plugin.Exec() - assert.NotNil(t, err) + assert.Error(t, err) } func getHostPublicKeyFile(keypath string) (ssh.PublicKey, error) { @@ -467,7 +468,7 @@ func TestFingerprint(t *testing.T) { ) hostKey, err := getHostPublicKeyFile("/etc/ssh/ssh_host_rsa_key.pub") - assert.NoError(t, err) + require.NoError(t, err) plugin := Plugin{ Config: Config{ @@ -485,7 +486,7 @@ func TestFingerprint(t *testing.T) { } err = plugin.Exec() - assert.Nil(t, err) + require.NoError(t, err) assert.Equal(t, unindent(expected), unindent(buffer.String())) } @@ -515,7 +516,7 @@ func TestScriptStopWithMultipleHostAndSyncMode(t *testing.T) { } err := plugin.Exec() - assert.NotNil(t, err) + require.Error(t, err) assert.Equal(t, unindent(expected), unindent(buffer.String())) } @@ -545,7 +546,7 @@ func TestScriptStop(t *testing.T) { } err := plugin.Exec() - assert.NotNil(t, err) + require.Error(t, err) assert.Equal(t, unindent(expected), unindent(buffer.String())) } @@ -575,7 +576,7 @@ func TestNoneScriptStop(t *testing.T) { } err := plugin.Exec() - assert.NotNil(t, err) + require.Error(t, err) assert.Equal(t, unindent(expected), unindent(buffer.String())) } @@ -612,13 +613,13 @@ func TestEnvOutput(t *testing.T) { ` ) - os.Setenv("ENV_1", `test`) - os.Setenv("ENV_2", `test test`) - os.Setenv("ENV_3", `test `) - os.Setenv("ENV_4", ` test test `) - os.Setenv("ENV_5", `test'`) - os.Setenv("ENV_6", `test"`) - os.Setenv("ENV_7", `test,!#;?.@$~'"`) + t.Setenv("ENV_1", `test`) + t.Setenv("ENV_2", `test test`) + t.Setenv("ENV_3", `test `) + t.Setenv("ENV_4", ` test test `) + t.Setenv("ENV_5", `test'`) + t.Setenv("ENV_6", `test"`) + t.Setenv("ENV_7", `test,!#;?.@$~'"`) plugin := Plugin{ Config: Config{ @@ -650,7 +651,7 @@ func TestEnvOutput(t *testing.T) { } err := plugin.Exec() - assert.Nil(t, err) + require.NoError(t, err) assert.Equal(t, unindent(expected), unindent(buffer.String())) } @@ -777,7 +778,7 @@ func TestUseInsecureCipher(t *testing.T) { } err := plugin.Exec() - assert.NotNil(t, err) + require.Error(t, err) assert.Equal(t, unindent(expected), unindent(buffer.String())) } @@ -857,49 +858,37 @@ func TestPlugin_hostPort(t *testing.T) { } func TestFindEnvs(t *testing.T) { - testEnvs := []string{ - "INPUT_FOO", - "INPUT_BAR", - "NO_PREFIX", - "INPUT_FOOBAR", - } - - origEnviron := os.Environ() - os.Clearenv() - for _, env := range testEnvs { - os.Setenv(env, "dummyValue") - } - - defer func() { - os.Clearenv() - for _, env := range origEnviron { - pair := strings.SplitN(env, "=", 2) - os.Setenv(pair[0], pair[1]) - } - }() + t.Setenv("DRONETEST_INPUT_FOO", "dummyValue") + t.Setenv("DRONETEST_INPUT_BAR", "dummyValue") + t.Setenv("DRONETEST_NO_PREFIX", "dummyValue") + t.Setenv("DRONETEST_INPUT_FOOBAR", "dummyValue") t.Run("Find single prefix", func(t *testing.T) { - expected := []string{"INPUT_FOO", "INPUT_BAR", "INPUT_FOOBAR"} - result := findEnvs("INPUT_") - if !reflect.DeepEqual(result, expected) { - t.Errorf("Expected %v, but got %v", expected, result) - } + result := findEnvs("DRONETEST_INPUT_") + assert.ElementsMatch( + t, + []string{"DRONETEST_INPUT_FOO", "DRONETEST_INPUT_BAR", "DRONETEST_INPUT_FOOBAR"}, + result, + ) }) t.Run("Find multiple prefixes", func(t *testing.T) { - expected := []string{"INPUT_FOO", "INPUT_BAR", "NO_PREFIX", "INPUT_FOOBAR"} - result := findEnvs("INPUT_", "NO_PREFIX") - if !reflect.DeepEqual(result, expected) { - t.Errorf("Expected %v, but got %v", expected, result) - } + result := findEnvs("DRONETEST_INPUT_", "DRONETEST_NO_PREFIX") + assert.ElementsMatch( + t, + []string{ + "DRONETEST_INPUT_FOO", + "DRONETEST_INPUT_BAR", + "DRONETEST_NO_PREFIX", + "DRONETEST_INPUT_FOOBAR", + }, + result, + ) }) t.Run("Find non-existing prefix", func(t *testing.T) { - expected := []string{} - result := findEnvs("NON_EXISTING_") - if !reflect.DeepEqual(result, expected) { - t.Errorf("Expected %v, but got %v", expected, result) - } + result := findEnvs("ZZZZNONEXISTING_") + assert.Empty(t, result) }) } @@ -913,9 +902,9 @@ func TestAllEnvs(t *testing.T) { ` ) - os.Setenv("INPUT_1", `foobar`) - os.Setenv("GITHUB_2", `foobar`) - os.Setenv("PLUGIN_3", `foobar`) + t.Setenv("INPUT_1", `foobar`) + t.Setenv("GITHUB_2", `foobar`) + t.Setenv("PLUGIN_3", `foobar`) plugin := Plugin{ Config: Config{ @@ -942,7 +931,7 @@ func TestAllEnvs(t *testing.T) { } err := plugin.Exec() - assert.Nil(t, err) + require.NoError(t, err) assert.Equal(t, unindent(expected), unindent(buffer.String())) } @@ -1026,7 +1015,7 @@ func runSSHContainerTest(t *testing.T, cfg SSHTestConfig) { Writer: &buffer, } - assert.Nil(t, plugin.Exec()) + require.NoError(t, plugin.Exec()) assert.Equal(t, unindent(cfg.Expected), unindent(buffer.String())) } @@ -1092,6 +1081,6 @@ func TestCommandWithIPv6(t *testing.T) { }, Writer: &buffer, } - assert.Nil(t, plugin.Exec()) + require.NoError(t, plugin.Exec()) assert.Equal(t, unindent(expected), unindent(buffer.String())) }