Skip to content

Commit d25b31d

Browse files
authored
Fix golangci-lint errors and enforce lint in CI (#759)
* enforce lint in CI Signed-off-by: Aravindhan Ayyanathan <aravindhan.a@est.tech> * Address copilot review comments Signed-off-by: Aravindhan Ayyanathan <aravindhan.a@est.tech> * Unify golangci-lint setup and fix kfn lint errors Signed-off-by: Aravindhan Ayyanathan <aravindhan.a@est.tech> * Address review comments. Signed-off-by: Aravindhan Ayyanathan <aravindhan.a@est.tech> --------- Signed-off-by: Aravindhan Ayyanathan <aravindhan.a@est.tech>
1 parent d7f5964 commit d25b31d

11 files changed

Lines changed: 87 additions & 58 deletions

File tree

File renamed without changes.

go/Makefile

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,22 @@
1+
# Copyright 2026 The kpt Authors
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
GOLANGCI_LINT_VERSION ?= 2.12.2
16+
117
.PHONY: all
218
all: fix vet fmt test lint
319

4-
GOPATH := $(shell go env GOPATH)
5-
GOBIN := $(shell go env GOPATH)/bin
6-
OUT_DIR := .out
720
MODULES = $(shell find . -name 'go.mod' -print)
821

922
.PHONY: fix
@@ -14,18 +27,15 @@ fix: $(MODULES)
1427
fmt: $(MODULES)
1528
@for f in $(^D); do (cd $$f; echo "Formatting $$f"; go fmt ./...); done
1629

17-
.PHONY: install-golangci-lint
18-
install-golangci-lint:
19-
go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest
20-
2130
.PHONY: lint
22-
lint: install-golangci-lint lint-modules
23-
24-
.PHONY: lint-modules
25-
lint-modules: $(MODULES)
31+
lint: $(MODULES)
32+
@if ! command -v golangci-lint >/dev/null 2>&1 || \
33+
! golangci-lint version --short 2>/dev/null | grep -qx "$(GOLANGCI_LINT_VERSION)"; then \
34+
echo "Installing golangci-lint v$(GOLANGCI_LINT_VERSION)..."; \
35+
go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v$(GOLANGCI_LINT_VERSION); \
36+
fi
2637
@for f in $(^D); do \
27-
(cd $$f; echo "Checking golangci-lint $$f"; \
28-
$(GOBIN)/golangci-lint run ./...); \
38+
(cd $$f; echo "Linting $$f"; golangci-lint run ./...) || exit 1; \
2939
done
3040

3141
.PHONY: test

go/fn/internal/docs/render.go

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2025 The kpt Authors
1+
// Copyright 2025-2026 The kpt Authors
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -40,29 +40,42 @@ type DocOutput struct {
4040
// RenderHelp writes formatted help text to w.
4141
// If sections are empty and metadata is zero-value, writes a minimal
4242
// "no documentation available" message.
43-
func RenderHelp(w io.Writer, sections Sections, meta Metadata) {
43+
// Returns the first write error encountered.
44+
func RenderHelp(w io.Writer, sections Sections, meta Metadata) error {
4445
if sections.Short == "" && sections.Long == "" && sections.Examples == "" && isMetadataEmpty(meta) {
45-
fmt.Fprint(w, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n")
46-
return
46+
_, err := fmt.Fprint(w, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n")
47+
return err
4748
}
4849

4950
if sections.Short != "" {
50-
fmt.Fprintf(w, "%s\n", sections.Short)
51+
if _, err := fmt.Fprintf(w, "%s\n", sections.Short); err != nil {
52+
return err
53+
}
5154
}
5255

5356
if sections.Long != "" {
5457
if sections.Short != "" {
55-
fmt.Fprint(w, "\n")
58+
if _, err := fmt.Fprint(w, "\n"); err != nil {
59+
return err
60+
}
61+
}
62+
if _, err := fmt.Fprintf(w, "%s\n", sections.Long); err != nil {
63+
return err
5664
}
57-
fmt.Fprintf(w, "%s\n", sections.Long)
5865
}
5966

6067
if sections.Examples != "" {
6168
if sections.Short != "" || sections.Long != "" {
62-
fmt.Fprint(w, "\n")
69+
if _, err := fmt.Fprint(w, "\n"); err != nil {
70+
return err
71+
}
72+
}
73+
if _, err := fmt.Fprintf(w, "Examples:\n%s\n", sections.Examples); err != nil {
74+
return err
6375
}
64-
fmt.Fprintf(w, "Examples:\n%s\n", sections.Examples)
6576
}
77+
78+
return nil
6679
}
6780

6881
// isMetadataEmpty reports whether all fields of meta are zero-value.

go/fn/internal/docs/render_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"strings"
2222
"testing"
2323

24+
"github.com/stretchr/testify/require"
2425
"pgregory.net/rapid"
2526
)
2627

@@ -84,7 +85,7 @@ func TestProperty4_HelpOutputExcludesCobraBoilerplate(t *testing.T) {
8485

8586
// Render help output.
8687
var buf bytes.Buffer
87-
RenderHelp(&buf, sections, meta)
88+
require.NoError(t, RenderHelp(&buf, sections, meta), "RenderHelp failed")
8889
output := buf.String()
8990

9091
// Assert that the help output does NOT contain cobra-style boilerplate.
@@ -218,7 +219,7 @@ func TestProperty3_HelpOutputContainsParsedSections(t *testing.T) {
218219

219220
// Render help output.
220221
var buf bytes.Buffer
221-
RenderHelp(&buf, sections, Metadata{})
222+
require.NoError(t, RenderHelp(&buf, sections, Metadata{}), "RenderHelp failed")
222223
output := buf.String()
223224

224225
// Assert that the help output contains each parsed section.
@@ -250,7 +251,7 @@ func TestRenderHelp_FullSectionsAndMetadata(t *testing.T) {
250251
}
251252

252253
var buf bytes.Buffer
253-
RenderHelp(&buf, sections, meta)
254+
require.NoError(t, RenderHelp(&buf, sections, meta), "RenderHelp failed")
254255
output := buf.String()
255256

256257
// Verify output contains the Short description.
@@ -283,7 +284,7 @@ func TestRenderHelp_EmptySections(t *testing.T) {
283284
meta := Metadata{}
284285

285286
var buf bytes.Buffer
286-
RenderHelp(&buf, sections, meta)
287+
require.NoError(t, RenderHelp(&buf, sections, meta), "RenderHelp failed")
287288
output := buf.String()
288289

289290
expected := "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n"

go/fn/run.go

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"fmt"
1919
"io"
2020
"os"
21+
"path/filepath"
2122
"slices"
2223
"strings"
2324

@@ -141,8 +142,8 @@ func AsMain(input any, opts ...Option) error {
141142
// handleHelp renders help text to STDOUT based on registered docs.
142143
func handleHelp(cfg *mainConfig) error {
143144
if cfg.readme == nil && cfg.metadata == nil {
144-
fmt.Fprint(os.Stdout, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n")
145-
return nil
145+
_, err := fmt.Fprint(os.Stdout, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n")
146+
return err
146147
}
147148

148149
sections := docs.ParseMarkers(cfg.readme)
@@ -152,15 +153,14 @@ func handleHelp(cfg *mainConfig) error {
152153
meta = docs.Metadata{}
153154
}
154155

155-
docs.RenderHelp(os.Stdout, sections, meta)
156-
return nil
156+
return docs.RenderHelp(os.Stdout, sections, meta)
157157
}
158158

159159
// handleDoc renders JSON documentation to STDOUT based on registered docs.
160160
func handleDoc(cfg *mainConfig) error {
161161
if cfg.readme == nil && cfg.metadata == nil {
162-
fmt.Fprint(os.Stdout, "{}")
163-
return nil
162+
_, err := fmt.Fprint(os.Stdout, "{}")
163+
return err
164164
}
165165

166166
sections := docs.ParseMarkers(cfg.readme)
@@ -183,20 +183,18 @@ func readFilesAsResourceList(paths []string) (*ResourceList, error) {
183183
FunctionConfig: NewEmptyKubeObject(),
184184
}
185185
for _, path := range paths {
186-
data, err := os.ReadFile(path)
186+
cleanPath := filepath.Clean(path)
187+
data, err := os.ReadFile(cleanPath)
187188
if err != nil {
188-
if os.IsNotExist(err) {
189-
return nil, fmt.Errorf("file not found: %s", path)
190-
}
191-
return nil, fmt.Errorf("failed to read file %s: %v", path, err)
189+
return nil, fmt.Errorf("file %s: %w", path, err)
192190
}
193191
// Empty files are valid — proceed with no items from this file.
194192
if len(strings.TrimSpace(string(data))) == 0 {
195193
continue
196194
}
197195
objects, err := ParseKubeObjects(data)
198196
if err != nil {
199-
return nil, fmt.Errorf("failed to parse KRM resources from %s: %v", path, err)
197+
return nil, fmt.Errorf("failed to parse KRM resources from %s: %w", path, err)
200198
}
201199
for _, obj := range objects {
202200
rl.Items = append(rl.Items, obj)

go/fn/run_filemode_property_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,9 @@ func genKRMResource() *rapid.Generator[string] {
3838
for i := range numEntries {
3939
key := rapid.StringMatching(`[a-z][a-z0-9]{1,8}`).Draw(t, fmt.Sprintf("key%d", i))
4040
value := rapid.StringMatching(`[a-zA-Z0-9]{1,15}`).Draw(t, fmt.Sprintf("value%d", i))
41-
dataLines.WriteString(fmt.Sprintf(" %s: %s\n", key, value))
41+
if _, err := fmt.Fprintf(&dataLines, " %s: %s\n", key, value); err != nil {
42+
t.Errorf("failed to write data line: %v", err)
43+
}
4244
}
4345
return fmt.Sprintf(`apiVersion: v1
4446
kind: ConfigMap
@@ -65,7 +67,11 @@ func TestProperty6_FileModeEquivalence(t *testing.T) {
6567
if err != nil {
6668
t.Fatalf("failed to create temp dir: %v", err)
6769
}
68-
defer os.RemoveAll(tmpDir)
70+
t.Cleanup(func() {
71+
if err := os.RemoveAll(tmpDir); err != nil {
72+
t.Errorf("failed to remove temp dir %s: %v", tmpDir, err)
73+
}
74+
})
6975

7076
var filePaths []string
7177
for i, res := range resources {

go/fn/run_filemode_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ func TestFileMode_NonExistentFile(t *testing.T) {
108108

109109
err := AsMain(noopProcessor)
110110
require.Error(t, err, "non-existent file should return an error")
111-
assert.Contains(t, err.Error(), "file not found")
111+
assert.Contains(t, err.Error(), "no such file or directory")
112112
assert.Contains(t, err.Error(), nonExistentPath, "error should include the file path")
113113
}
114114

@@ -244,7 +244,7 @@ func TestReadFilesAsResourceList_NonExistentFile(t *testing.T) {
244244
rl, err := readFilesAsResourceList([]string{nonExistentPath})
245245
require.Error(t, err)
246246
assert.Nil(t, rl)
247-
assert.Contains(t, err.Error(), "file not found")
247+
assert.Contains(t, err.Error(), "no such file or directory")
248248
assert.Contains(t, err.Error(), nonExistentPath)
249249
}
250250

@@ -422,7 +422,7 @@ func TestFileMode_NonExistentAmongValid(t *testing.T) {
422422
captureStderr(t, func() {
423423
err := AsMain(noopProcessor)
424424
require.Error(t, err)
425-
assert.Contains(t, err.Error(), "file not found")
425+
assert.Contains(t, err.Error(), "no such file or directory")
426426
assert.Contains(t, err.Error(), strings.TrimPrefix(nonExistent, ""))
427427
})
428428
}

go/fn/run_flags_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,13 @@ func captureStdout(t *testing.T, fn func()) string {
4444

4545
fn()
4646

47-
w.Close()
47+
require.NoError(t, w.Close())
4848
os.Stdout = origStdout
4949

5050
var buf bytes.Buffer
5151
_, err = io.Copy(&buf, r)
5252
require.NoError(t, err)
53-
r.Close()
53+
require.NoError(t, r.Close())
5454

5555
return buf.String()
5656
}
@@ -66,13 +66,13 @@ func captureStderr(t *testing.T, fn func()) string {
6666

6767
fn()
6868

69-
w.Close()
69+
require.NoError(t, w.Close())
7070
os.Stderr = origStderr
7171

7272
var buf bytes.Buffer
7373
_, err = io.Copy(&buf, r)
7474
require.NoError(t, err)
75-
r.Close()
75+
require.NoError(t, r.Close())
7676

7777
return buf.String()
7878
}
@@ -96,11 +96,11 @@ func TestAsMain_HelpFlag_ExitsZero(t *testing.T) {
9696
origStdin := os.Stdin
9797
r, w, err := os.Pipe()
9898
require.NoError(t, err)
99-
w.Close() // Close write end immediately — reading would get EOF
99+
require.NoError(t, w.Close()) // Close write end immediately — reading would get EOF
100100
os.Stdin = r
101101
t.Cleanup(func() {
102102
os.Stdin = origStdin
103-
r.Close()
103+
assert.NoError(t, r.Close())
104104
})
105105

106106
output := captureStdout(t, func() {

go/kfn/commands/build.go

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// Copyright 2022 The kpt Authors
1+
// Copyright 2022, 2026 The kpt Authors
22
//
33
// Licensed under the Apache License, Version 2.0 (the "License");
44
// you may not use this file except in compliance with the License.
@@ -44,6 +44,8 @@ const (
4444
// Ko constant variables
4545
KoDockerRepoEnvVar = "KO_DOCKER_REPO"
4646
KoLocalRepo = "ko.local"
47+
48+
build = "build"
4749
)
4850

4951
func NewBuildRunner(ctx context.Context) *BuildRunner {
@@ -53,7 +55,7 @@ func NewBuildRunner(ctx context.Context) *BuildRunner {
5355
Docker: &DockerBuilder{},
5456
}
5557
r.Command = &cobra.Command{
56-
Use: "build",
58+
Use: build,
5759
Short: "build your KRM function to a container image",
5860
RunE: r.RunE,
5961
}
@@ -114,7 +116,7 @@ func (r *BuildRunner) RunE(cmd *cobra.Command, args []string) error {
114116
}
115117

116118
func (r *DockerBuilder) Build() error {
117-
args := []string{"build", ".", "-f", r.DockerfilePath, "--tag", r.Image}
119+
args := []string{build, ".", "-f", r.DockerfilePath, "--tag", r.Image}
118120
err := execCmdFn(nil, "docker", args...)
119121
if err != nil {
120122
return err
@@ -157,7 +159,7 @@ func (r *DockerBuilder) createDockerfile() error {
157159
if err != nil {
158160
return err
159161
}
160-
if err = os.WriteFile(DockerfilePath, dockerfileContent, 0644); err != nil {
162+
if err = os.WriteFile(DockerfilePath, dockerfileContent, 0600); err != nil {
161163
return err
162164
}
163165
fmt.Println("created Dockerfile")
@@ -187,7 +189,7 @@ func (r *KoBuilder) GuaranteeKoInstalled() error {
187189
return nil
188190
}
189191
func (r *KoBuilder) Build() error {
190-
args := []string{"build", "-B", "--tags", r.Tag}
192+
args := []string{build, "-B", "--tags", r.Tag}
191193
envs := []string{KoDockerRepoEnvVar + "=" + r.Repo}
192194
err := execCmdFn(envs, "ko", args...)
193195
if err != nil {
@@ -219,7 +221,7 @@ func (r *KoBuilder) Validate() error {
219221
}
220222

221223
func execCmd(envs []string, name string, args ...string) error {
222-
cmd := exec.Command(name, args...)
224+
cmd := exec.Command(name, args...) //nolint:gosec // CLI tool: args constructed internally from user's own flags
223225
if len(envs) != 0 {
224226
cmd.Env = os.Environ()
225227
cmd.Env = append(cmd.Env, envs...)

go/kfn/commands/build_test.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,6 @@ func TestBuild(t *testing.T) {
117117
}
118118
for name, test := range testcases {
119119
t.Run(name, func(t *testing.T) {
120-
121120
r := NewBuildRunner(context.TODO())
122121
execCmdFn = func(envs []string, name string, args ...string) error {
123122
fakeExecCmd(t, test.cmdExpected, envs, name, args...)

0 commit comments

Comments
 (0)