From 1d381f2b086a9bd53d7a9d0d9991aaf68f76ad13 Mon Sep 17 00:00:00 2001 From: Aravindhan Ayyanathan Date: Thu, 11 Jun 2026 14:38:18 +0100 Subject: [PATCH 1/3] enforce lint in CI Signed-off-by: Aravindhan Ayyanathan --- go/Makefile | 2 +- go/fn/internal/docs/render.go | 14 +++++++------- go/fn/run.go | 7 ++++--- go/fn/run_filemode_property_test.go | 4 ++-- go/fn/run_flags_test.go | 12 ++++++------ 5 files changed, 20 insertions(+), 19 deletions(-) diff --git a/go/Makefile b/go/Makefile index 4e8e0876..98c8cf40 100644 --- a/go/Makefile +++ b/go/Makefile @@ -25,7 +25,7 @@ lint: install-golangci-lint lint-modules lint-modules: $(MODULES) @for f in $(^D); do \ (cd $$f; echo "Checking golangci-lint $$f"; \ - $(GOBIN)/golangci-lint run ./...); \ + $(GOBIN)/golangci-lint run ./...) || exit 1; \ done .PHONY: test diff --git a/go/fn/internal/docs/render.go b/go/fn/internal/docs/render.go index e6c17839..76b7bc14 100644 --- a/go/fn/internal/docs/render.go +++ b/go/fn/internal/docs/render.go @@ -1,4 +1,4 @@ -// Copyright 2025 The kpt Authors +// Copyright 2025-2026 The kpt Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -42,26 +42,26 @@ type DocOutput struct { // "no documentation available" message. func RenderHelp(w io.Writer, sections Sections, meta Metadata) { if sections.Short == "" && sections.Long == "" && sections.Examples == "" && isMetadataEmpty(meta) { - fmt.Fprint(w, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n") + _, _ = fmt.Fprint(w, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n") return } if sections.Short != "" { - fmt.Fprintf(w, "%s\n", sections.Short) + _, _ = fmt.Fprintf(w, "%s\n", sections.Short) } if sections.Long != "" { if sections.Short != "" { - fmt.Fprint(w, "\n") + _, _ = fmt.Fprint(w, "\n") } - fmt.Fprintf(w, "%s\n", sections.Long) + _, _ = fmt.Fprintf(w, "%s\n", sections.Long) } if sections.Examples != "" { if sections.Short != "" || sections.Long != "" { - fmt.Fprint(w, "\n") + _, _ = fmt.Fprint(w, "\n") } - fmt.Fprintf(w, "Examples:\n%s\n", sections.Examples) + _, _ = fmt.Fprintf(w, "Examples:\n%s\n", sections.Examples) } } diff --git a/go/fn/run.go b/go/fn/run.go index 0f3cfd7d..54d7e6c0 100644 --- a/go/fn/run.go +++ b/go/fn/run.go @@ -18,6 +18,7 @@ import ( "fmt" "io" "os" + "path/filepath" "slices" "strings" @@ -141,7 +142,7 @@ func AsMain(input any, opts ...Option) error { // handleHelp renders help text to STDOUT based on registered docs. func handleHelp(cfg *mainConfig) error { if cfg.readme == nil && cfg.metadata == nil { - fmt.Fprint(os.Stdout, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n") + _, _ = fmt.Fprint(os.Stdout, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n") return nil } @@ -159,7 +160,7 @@ func handleHelp(cfg *mainConfig) error { // handleDoc renders JSON documentation to STDOUT based on registered docs. func handleDoc(cfg *mainConfig) error { if cfg.readme == nil && cfg.metadata == nil { - fmt.Fprint(os.Stdout, "{}") + _, _ = fmt.Fprint(os.Stdout, "{}") return nil } @@ -183,7 +184,7 @@ func readFilesAsResourceList(paths []string) (*ResourceList, error) { FunctionConfig: NewEmptyKubeObject(), } for _, path := range paths { - data, err := os.ReadFile(path) + data, err := os.ReadFile(filepath.Clean(path)) if err != nil { if os.IsNotExist(err) { return nil, fmt.Errorf("file not found: %s", path) diff --git a/go/fn/run_filemode_property_test.go b/go/fn/run_filemode_property_test.go index 30b5b148..c0e112f3 100644 --- a/go/fn/run_filemode_property_test.go +++ b/go/fn/run_filemode_property_test.go @@ -38,7 +38,7 @@ func genKRMResource() *rapid.Generator[string] { for i := range numEntries { key := rapid.StringMatching(`[a-z][a-z0-9]{1,8}`).Draw(t, fmt.Sprintf("key%d", i)) value := rapid.StringMatching(`[a-zA-Z0-9]{1,15}`).Draw(t, fmt.Sprintf("value%d", i)) - dataLines.WriteString(fmt.Sprintf(" %s: %s\n", key, value)) + fmt.Fprintf(&dataLines, " %s: %s\n", key, value) } return fmt.Sprintf(`apiVersion: v1 kind: ConfigMap @@ -65,7 +65,7 @@ func TestProperty6_FileModeEquivalence(t *testing.T) { if err != nil { t.Fatalf("failed to create temp dir: %v", err) } - defer os.RemoveAll(tmpDir) + defer func() { _ = os.RemoveAll(tmpDir) }() var filePaths []string for i, res := range resources { diff --git a/go/fn/run_flags_test.go b/go/fn/run_flags_test.go index 578fc176..af2d1a03 100644 --- a/go/fn/run_flags_test.go +++ b/go/fn/run_flags_test.go @@ -44,13 +44,13 @@ func captureStdout(t *testing.T, fn func()) string { fn() - w.Close() + _ = w.Close() os.Stdout = origStdout var buf bytes.Buffer _, err = io.Copy(&buf, r) require.NoError(t, err) - r.Close() + _ = r.Close() return buf.String() } @@ -66,13 +66,13 @@ func captureStderr(t *testing.T, fn func()) string { fn() - w.Close() + _ = w.Close() os.Stderr = origStderr var buf bytes.Buffer _, err = io.Copy(&buf, r) require.NoError(t, err) - r.Close() + _ = r.Close() return buf.String() } @@ -96,11 +96,11 @@ func TestAsMain_HelpFlag_ExitsZero(t *testing.T) { origStdin := os.Stdin r, w, err := os.Pipe() require.NoError(t, err) - w.Close() // Close write end immediately — reading would get EOF + _ = w.Close() // Close write end immediately — reading would get EOF os.Stdin = r t.Cleanup(func() { os.Stdin = origStdin - r.Close() + _ = r.Close() }) output := captureStdout(t, func() { From 6397b6354b91d7e47598a2fee12d644346e6bc51 Mon Sep 17 00:00:00 2001 From: Aravindhan Ayyanathan Date: Thu, 11 Jun 2026 15:03:45 +0100 Subject: [PATCH 2/3] Address copilot review comments Signed-off-by: Aravindhan Ayyanathan --- go/fn/internal/docs/render.go | 29 +++++++++++++++++++++-------- go/fn/internal/docs/render_test.go | 16 ++++++++++++---- go/fn/run.go | 21 +++++++++------------ go/fn/run_filemode_property_test.go | 10 ++++++++-- go/fn/run_filemode_test.go | 6 +++--- go/fn/run_flags_test.go | 12 ++++++------ 6 files changed, 59 insertions(+), 35 deletions(-) diff --git a/go/fn/internal/docs/render.go b/go/fn/internal/docs/render.go index 76b7bc14..e5e856ac 100644 --- a/go/fn/internal/docs/render.go +++ b/go/fn/internal/docs/render.go @@ -40,29 +40,42 @@ type DocOutput struct { // RenderHelp writes formatted help text to w. // If sections are empty and metadata is zero-value, writes a minimal // "no documentation available" message. -func RenderHelp(w io.Writer, sections Sections, meta Metadata) { +// Returns the first write error encountered. +func RenderHelp(w io.Writer, sections Sections, meta Metadata) error { if sections.Short == "" && sections.Long == "" && sections.Examples == "" && isMetadataEmpty(meta) { - _, _ = fmt.Fprint(w, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n") - return + _, err := fmt.Fprint(w, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n") + return err } if sections.Short != "" { - _, _ = fmt.Fprintf(w, "%s\n", sections.Short) + if _, err := fmt.Fprintf(w, "%s\n", sections.Short); err != nil { + return err + } } if sections.Long != "" { if sections.Short != "" { - _, _ = fmt.Fprint(w, "\n") + if _, err := fmt.Fprint(w, "\n"); err != nil { + return err + } + } + if _, err := fmt.Fprintf(w, "%s\n", sections.Long); err != nil { + return err } - _, _ = fmt.Fprintf(w, "%s\n", sections.Long) } if sections.Examples != "" { if sections.Short != "" || sections.Long != "" { - _, _ = fmt.Fprint(w, "\n") + if _, err := fmt.Fprint(w, "\n"); err != nil { + return err + } + } + if _, err := fmt.Fprintf(w, "Examples:\n%s\n", sections.Examples); err != nil { + return err } - _, _ = fmt.Fprintf(w, "Examples:\n%s\n", sections.Examples) } + + return nil } // isMetadataEmpty reports whether all fields of meta are zero-value. diff --git a/go/fn/internal/docs/render_test.go b/go/fn/internal/docs/render_test.go index 12143dc7..15229dda 100644 --- a/go/fn/internal/docs/render_test.go +++ b/go/fn/internal/docs/render_test.go @@ -84,7 +84,9 @@ func TestProperty4_HelpOutputExcludesCobraBoilerplate(t *testing.T) { // Render help output. var buf bytes.Buffer - RenderHelp(&buf, sections, meta) + if err := RenderHelp(&buf, sections, meta); err != nil { + t.Errorf("RenderHelp failed: %v", err) + } output := buf.String() // Assert that the help output does NOT contain cobra-style boilerplate. @@ -218,7 +220,9 @@ func TestProperty3_HelpOutputContainsParsedSections(t *testing.T) { // Render help output. var buf bytes.Buffer - RenderHelp(&buf, sections, Metadata{}) + if err := RenderHelp(&buf, sections, Metadata{}); err != nil { + t.Errorf("RenderHelp failed: %v", err) + } output := buf.String() // Assert that the help output contains each parsed section. @@ -250,7 +254,9 @@ func TestRenderHelp_FullSectionsAndMetadata(t *testing.T) { } var buf bytes.Buffer - RenderHelp(&buf, sections, meta) + if err := RenderHelp(&buf, sections, meta); err != nil { + t.Errorf("RenderHelp failed: %v", err) + } output := buf.String() // Verify output contains the Short description. @@ -283,7 +289,9 @@ func TestRenderHelp_EmptySections(t *testing.T) { meta := Metadata{} var buf bytes.Buffer - RenderHelp(&buf, sections, meta) + if err := RenderHelp(&buf, sections, meta); err != nil { + t.Errorf("RenderHelp failed: %v", err) + } output := buf.String() expected := "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n" diff --git a/go/fn/run.go b/go/fn/run.go index 54d7e6c0..676b4d55 100644 --- a/go/fn/run.go +++ b/go/fn/run.go @@ -142,8 +142,8 @@ func AsMain(input any, opts ...Option) error { // handleHelp renders help text to STDOUT based on registered docs. func handleHelp(cfg *mainConfig) error { if cfg.readme == nil && cfg.metadata == nil { - _, _ = fmt.Fprint(os.Stdout, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n") - return nil + _, err := fmt.Fprint(os.Stdout, "No documentation available. Pass fn.WithDocs to fn.AsMain to enable --help.\n") + return err } sections := docs.ParseMarkers(cfg.readme) @@ -153,15 +153,14 @@ func handleHelp(cfg *mainConfig) error { meta = docs.Metadata{} } - docs.RenderHelp(os.Stdout, sections, meta) - return nil + return docs.RenderHelp(os.Stdout, sections, meta) } // handleDoc renders JSON documentation to STDOUT based on registered docs. func handleDoc(cfg *mainConfig) error { if cfg.readme == nil && cfg.metadata == nil { - _, _ = fmt.Fprint(os.Stdout, "{}") - return nil + _, err := fmt.Fprint(os.Stdout, "{}") + return err } sections := docs.ParseMarkers(cfg.readme) @@ -184,12 +183,10 @@ func readFilesAsResourceList(paths []string) (*ResourceList, error) { FunctionConfig: NewEmptyKubeObject(), } for _, path := range paths { - data, err := os.ReadFile(filepath.Clean(path)) + cleanPath := filepath.Clean(path) + data, err := os.ReadFile(cleanPath) if err != nil { - if os.IsNotExist(err) { - return nil, fmt.Errorf("file not found: %s", path) - } - return nil, fmt.Errorf("failed to read file %s: %v", path, err) + return nil, fmt.Errorf("file %s: %w", path, err) } // Empty files are valid — proceed with no items from this file. if len(strings.TrimSpace(string(data))) == 0 { @@ -197,7 +194,7 @@ func readFilesAsResourceList(paths []string) (*ResourceList, error) { } objects, err := ParseKubeObjects(data) if err != nil { - return nil, fmt.Errorf("failed to parse KRM resources from %s: %v", path, err) + return nil, fmt.Errorf("failed to parse KRM resources from %s: %w", path, err) } for _, obj := range objects { rl.Items = append(rl.Items, obj) diff --git a/go/fn/run_filemode_property_test.go b/go/fn/run_filemode_property_test.go index c0e112f3..8db42cbb 100644 --- a/go/fn/run_filemode_property_test.go +++ b/go/fn/run_filemode_property_test.go @@ -38,7 +38,9 @@ func genKRMResource() *rapid.Generator[string] { for i := range numEntries { key := rapid.StringMatching(`[a-z][a-z0-9]{1,8}`).Draw(t, fmt.Sprintf("key%d", i)) value := rapid.StringMatching(`[a-zA-Z0-9]{1,15}`).Draw(t, fmt.Sprintf("value%d", i)) - fmt.Fprintf(&dataLines, " %s: %s\n", key, value) + if _, err := fmt.Fprintf(&dataLines, " %s: %s\n", key, value); err != nil { + t.Errorf("failed to write data line: %v", err) + } } return fmt.Sprintf(`apiVersion: v1 kind: ConfigMap @@ -65,7 +67,11 @@ func TestProperty6_FileModeEquivalence(t *testing.T) { if err != nil { t.Fatalf("failed to create temp dir: %v", err) } - defer func() { _ = os.RemoveAll(tmpDir) }() + defer func() { + if err := os.RemoveAll(tmpDir); err != nil { + t.Errorf("failed to remove temp dir %s: %v", tmpDir, err) + } + }() var filePaths []string for i, res := range resources { diff --git a/go/fn/run_filemode_test.go b/go/fn/run_filemode_test.go index 4d6dac62..68173b2b 100644 --- a/go/fn/run_filemode_test.go +++ b/go/fn/run_filemode_test.go @@ -108,7 +108,7 @@ func TestFileMode_NonExistentFile(t *testing.T) { err := AsMain(noopProcessor) require.Error(t, err, "non-existent file should return an error") - assert.Contains(t, err.Error(), "file not found") + assert.Contains(t, err.Error(), "no such file or directory") assert.Contains(t, err.Error(), nonExistentPath, "error should include the file path") } @@ -244,7 +244,7 @@ func TestReadFilesAsResourceList_NonExistentFile(t *testing.T) { rl, err := readFilesAsResourceList([]string{nonExistentPath}) require.Error(t, err) assert.Nil(t, rl) - assert.Contains(t, err.Error(), "file not found") + assert.Contains(t, err.Error(), "no such file or directory") assert.Contains(t, err.Error(), nonExistentPath) } @@ -422,7 +422,7 @@ func TestFileMode_NonExistentAmongValid(t *testing.T) { captureStderr(t, func() { err := AsMain(noopProcessor) require.Error(t, err) - assert.Contains(t, err.Error(), "file not found") + assert.Contains(t, err.Error(), "no such file or directory") assert.Contains(t, err.Error(), strings.TrimPrefix(nonExistent, "")) }) } diff --git a/go/fn/run_flags_test.go b/go/fn/run_flags_test.go index af2d1a03..3f27dae5 100644 --- a/go/fn/run_flags_test.go +++ b/go/fn/run_flags_test.go @@ -44,13 +44,13 @@ func captureStdout(t *testing.T, fn func()) string { fn() - _ = w.Close() + require.NoError(t, w.Close()) os.Stdout = origStdout var buf bytes.Buffer _, err = io.Copy(&buf, r) require.NoError(t, err) - _ = r.Close() + require.NoError(t, r.Close()) return buf.String() } @@ -66,13 +66,13 @@ func captureStderr(t *testing.T, fn func()) string { fn() - _ = w.Close() + require.NoError(t, w.Close()) os.Stderr = origStderr var buf bytes.Buffer _, err = io.Copy(&buf, r) require.NoError(t, err) - _ = r.Close() + require.NoError(t, r.Close()) return buf.String() } @@ -96,11 +96,11 @@ func TestAsMain_HelpFlag_ExitsZero(t *testing.T) { origStdin := os.Stdin r, w, err := os.Pipe() require.NoError(t, err) - _ = w.Close() // Close write end immediately — reading would get EOF + require.NoError(t, w.Close()) // Close write end immediately — reading would get EOF os.Stdin = r t.Cleanup(func() { os.Stdin = origStdin - _ = r.Close() + assert.NoError(t, r.Close()) }) output := captureStdout(t, func() { From d2a1186ff6a5c30558f7e32808232548aef3b9fe Mon Sep 17 00:00:00 2001 From: Aravindhan Ayyanathan Date: Thu, 11 Jun 2026 18:14:31 +0100 Subject: [PATCH 3/3] Unify golangci-lint setup and fix kfn lint errors Signed-off-by: Aravindhan Ayyanathan --- go/{fn => }/.golangci.yml | 0 go/Makefile | 32 +++++++++++++++++++------------- go/kfn/commands/build.go | 14 ++++++++------ go/kfn/commands/build_test.go | 1 - go/kfn/commands/init.go | 4 ++-- 5 files changed, 29 insertions(+), 22 deletions(-) rename go/{fn => }/.golangci.yml (100%) diff --git a/go/fn/.golangci.yml b/go/.golangci.yml similarity index 100% rename from go/fn/.golangci.yml rename to go/.golangci.yml diff --git a/go/Makefile b/go/Makefile index 98c8cf40..afe365b5 100644 --- a/go/Makefile +++ b/go/Makefile @@ -1,9 +1,22 @@ +# Copyright 2026 The kpt Authors +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +GOLANGCI_LINT_VERSION ?= 2.12.2 + .PHONY: all all: fix vet fmt test lint -GOPATH := $(shell go env GOPATH) -GOBIN := $(shell go env GOPATH)/bin -OUT_DIR := .out MODULES = $(shell find . -name 'go.mod' -print) .PHONY: fix @@ -14,18 +27,11 @@ fix: $(MODULES) fmt: $(MODULES) @for f in $(^D); do (cd $$f; echo "Formatting $$f"; go fmt ./...); done -.PHONY: install-golangci-lint -install-golangci-lint: - go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@latest - .PHONY: lint -lint: install-golangci-lint lint-modules - -.PHONY: lint-modules -lint-modules: $(MODULES) +lint: $(MODULES) @for f in $(^D); do \ - (cd $$f; echo "Checking golangci-lint $$f"; \ - $(GOBIN)/golangci-lint run ./...) || exit 1; \ + (cd $$f; echo "Linting $$f"; \ + go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v$(GOLANGCI_LINT_VERSION) run ./...) || exit 1; \ done .PHONY: test diff --git a/go/kfn/commands/build.go b/go/kfn/commands/build.go index cf1db970..9addc0e9 100644 --- a/go/kfn/commands/build.go +++ b/go/kfn/commands/build.go @@ -1,4 +1,4 @@ -// Copyright 2022 The kpt Authors +// Copyright 2022, 2026 The kpt Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -44,6 +44,8 @@ const ( // Ko constant variables KoDockerRepoEnvVar = "KO_DOCKER_REPO" KoLocalRepo = "ko.local" + + build = "build" ) func NewBuildRunner(ctx context.Context) *BuildRunner { @@ -53,7 +55,7 @@ func NewBuildRunner(ctx context.Context) *BuildRunner { Docker: &DockerBuilder{}, } r.Command = &cobra.Command{ - Use: "build", + Use: build, Short: "build your KRM function to a container image", RunE: r.RunE, } @@ -114,7 +116,7 @@ func (r *BuildRunner) RunE(cmd *cobra.Command, args []string) error { } func (r *DockerBuilder) Build() error { - args := []string{"build", ".", "-f", r.DockerfilePath, "--tag", r.Image} + args := []string{build, ".", "-f", r.DockerfilePath, "--tag", r.Image} err := execCmdFn(nil, "docker", args...) if err != nil { return err @@ -157,7 +159,7 @@ func (r *DockerBuilder) createDockerfile() error { if err != nil { return err } - if err = os.WriteFile(DockerfilePath, dockerfileContent, 0644); err != nil { + if err = os.WriteFile(DockerfilePath, dockerfileContent, 0600); err != nil { return err } fmt.Println("created Dockerfile") @@ -187,7 +189,7 @@ func (r *KoBuilder) GuaranteeKoInstalled() error { return nil } func (r *KoBuilder) Build() error { - args := []string{"build", "-B", "--tags", r.Tag} + args := []string{build, "-B", "--tags", r.Tag} envs := []string{KoDockerRepoEnvVar + "=" + r.Repo} err := execCmdFn(envs, "ko", args...) if err != nil { @@ -219,7 +221,7 @@ func (r *KoBuilder) Validate() error { } func execCmd(envs []string, name string, args ...string) error { - cmd := exec.Command(name, args...) + cmd := exec.Command(name, args...) //nolint:gosec // CLI tool: args constructed internally from user's own flags if len(envs) != 0 { cmd.Env = os.Environ() cmd.Env = append(cmd.Env, envs...) diff --git a/go/kfn/commands/build_test.go b/go/kfn/commands/build_test.go index 1fb127b1..75b5af8f 100644 --- a/go/kfn/commands/build_test.go +++ b/go/kfn/commands/build_test.go @@ -117,7 +117,6 @@ func TestBuild(t *testing.T) { } for name, test := range testcases { t.Run(name, func(t *testing.T) { - r := NewBuildRunner(context.TODO()) execCmdFn = func(envs []string, name string, args ...string) error { fakeExecCmd(t, test.cmdExpected, envs, name, args...) diff --git a/go/kfn/commands/init.go b/go/kfn/commands/init.go index 856a5c98..d15ae6a6 100644 --- a/go/kfn/commands/init.go +++ b/go/kfn/commands/init.go @@ -1,4 +1,4 @@ -// Copyright 2022 The kpt Authors +// Copyright 2022, 2026 The kpt Authors // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -74,7 +74,7 @@ func (r *InitRunner) RunE(cmd *cobra.Command, args []string) error { } func (r *InitRunner) GetFnPackage() error { - cmd := exec.Command("kpt", "pkg", "get", r.FnPkgPath, r.FnName) + cmd := exec.Command("kpt", "pkg", "get", r.FnPkgPath, r.FnName) //nolint:gosec // CLI tool: args from user's own flags var out, errout bytes.Buffer cmd.Stdout = &out