From 54848c10497f3bb275c00ff82370f3a90214d042 Mon Sep 17 00:00:00 2001 From: hmilkovi Date: Sat, 15 Nov 2025 23:21:35 +0100 Subject: [PATCH] fix: support helm v4 beside v3 and remove -c flag for helm version as it does nothing features. --- .github/workflows/apidiff.yml | 2 +- .github/workflows/go.yml | 122 +++++++++++------- .github/workflows/release.yaml | 2 +- Makefile | 8 +- .../builtins/HelmChartInflationGenerator.go | 8 +- api/internal/builtins/PatchTransformer.go | 5 +- api/internal/loader/fileloader_test.go | 2 +- .../helmchartinflationgenerator_test.go | 1 - api/krusty/multiplepatch_test.go | 106 +++++++++++++++ hack/for-each-module.sh | 12 +- .../HelmChartInflationGenerator.go | 10 +- .../patchtransformer/PatchTransformer.go | 5 +- 12 files changed, 218 insertions(+), 65 deletions(-) diff --git a/.github/workflows/apidiff.yml b/.github/workflows/apidiff.yml index 21c60c353..f8d843222 100644 --- a/.github/workflows/apidiff.yml +++ b/.github/workflows/apidiff.yml @@ -13,7 +13,7 @@ jobs: if: (github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository) steps: - name: Clone the code - uses: actions/checkout@v5 + uses: actions/checkout@v6 with: fetch-depth: 0 - name: Setup Go diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 16edb3384..26a78c1a2 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -2,14 +2,15 @@ name: Go on: push: - branches: [ master ] + branches: [master] pull_request: - branches: [ master ] + branches: [master] permissions: contents: read jobs: + ## TODO: conditional-changes checker is not working conditional-changes: runs-on: ubuntu-latest permissions: @@ -17,7 +18,7 @@ jobs: outputs: doc: ${{ steps.filter.outputs.doc }} steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v6 - uses: dorny/paths-filter@v3 id: filter with: @@ -27,18 +28,21 @@ jobs: check-modules: name: check-synced-go-modules - needs: conditional-changes + # needs: conditional-changes # if: needs.conditional-changes.outputs.doc == 'false' runs-on: [ubuntu-latest] steps: - name: Check out code into the Go module directory - uses: actions/checkout@v5 + uses: actions/checkout@v6 with: fetch-depth: 0 - name: Set up Go 1.x uses: actions/setup-go@v6 with: go-version-file: go.work + cache: true + cache-dependency-path: | + **/go.sum id: go - name: sync go modules run: make workspace-sync @@ -47,84 +51,108 @@ jobs: lint: name: Lint - needs: conditional-changes + # needs: conditional-changes # if: needs.conditional-changes.outputs.doc == 'false' runs-on: [ubuntu-latest] steps: - name: Check out code into the Go module directory - uses: actions/checkout@v5 + uses: actions/checkout@v6 with: fetch-depth: 0 - name: Set up Go 1.x uses: actions/setup-go@v6 with: go-version-file: go.work + cache: true + cache-dependency-path: | + **/go.sum id: go - name: Lint run: make lint - name: Verify boilerplate run: make check-license - test-linux: + ## Test all modules without plugins and released modules + test-non-released-modules: name: Test Linux - needs: conditional-changes + # needs: conditional-changes # if: needs.conditional-changes.outputs.doc == 'false' runs-on: [ubuntu-latest] steps: - name: Check out code into the Go module directory - uses: actions/checkout@v5 + uses: actions/checkout@v6 - name: Set up Go 1.x uses: actions/setup-go@v6 with: go-version-file: go.work + cache: true + cache-dependency-path: | + **/go.sum id: go - - name: Test all modules - run: make test-unit-non-plugin + - name: Test all modules without plugins and released modules + run: make test-unit-non-plugin-and-non-released env: KUSTOMIZE_DOCKER_E2E: true - test-macos: - name: Test MacOS - needs: conditional-changes + test-modules: + name: Test ${{ matrix.os }} - ${{ matrix.module }} + # needs: conditional-changes # if: needs.conditional-changes.outputs.doc == 'false' - runs-on: [macos-latest] + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, macos-latest, windows-latest] + module: + - kyaml + - cmd/config + - api + - kustomize + include: + - module: kyaml + test-cmd: go test -race -v -cover ./... + - module: cmd/config + test-cmd: go test -v -cover ./... + - module: api + test-cmd: go test -v -cover ./... -ldflags "-X sigs.k8s.io/kustomize/api/provenance.buildDate=2023-01-31T23:38:41Z -X sigs.k8s.io/kustomize/api/provenance.version=(test)" + - module: kustomize + test-cmd: go test -v -cover ./... + - os: ubuntu-latest + docker-e2e: true + - os: macos-latest + docker-e2e: false + - os: windows-latest + docker-e2e: false + env: + KUSTOMIZE_DOCKER_E2E: ${{ matrix.docker-e2e }} steps: - name: Check out code into the Go module directory - uses: actions/checkout@v5 + uses: actions/checkout@v6 - name: Set up Go 1.x uses: actions/setup-go@v6 with: go-version-file: go.work + cache: true + cache-dependency-path: | + **/go.sum id: go - - name: Test all modules - run: make test-unit-non-plugin - env: - KUSTOMIZE_DOCKER_E2E: false # docker not installed on mac + - name: Test ${{ matrix.module }} + run: ${{ matrix.test-cmd }} + # TODO (#4001): replace specific modules above with this once Windows tests are passing. + if: ${{ !(matrix.os == 'windows-latest' && (matrix.module == 'api' || matrix.module == 'kustomize')) }} + working-directory: ./${{ matrix.module }} - test-windows: - name: Test Windows - needs: conditional-changes - # if: needs.conditional-changes.outputs.doc == 'false' - runs-on: [windows-latest] + # Aggregation matrix tests from test-modules for branch protection rules + test-modules-summary: + name: Test Summary + runs-on: ubuntu-latest + needs: test-modules + if: always() steps: - - name: Check out code into the Go module directory - uses: actions/checkout@v5 - - name: Set up Go 1.x - uses: actions/setup-go@v6 - with: - go-version-file: go.work - id: go - - name: Test kyaml - run: go test -cover ./... - working-directory: ./kyaml - - name: Test cmd/config - run: go test -cover ./... - working-directory: ./cmd/config - env: - KUSTOMIZE_DOCKER_E2E: false # docker on windows not working well yet - - # TODO (#4001): replace specific modules above with this once Windows tests are passing. - #- name: Test all modules - # run: make test-unit-non-plugin - # env: - # KUSTOMIZE_DOCKER_E2E: false # docker on windows not working well yet + - name: Check test results + run: | + if [[ "${{ needs.test-modules.result }}" != "success" ]]; then + echo "Some tests failed or were cancelled" + exit 1 + fi + echo "All tests passed successfully" diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index d2fb3d617..bf44d0581 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Check out code into the Go module directory - uses: actions/checkout@v5 + uses: actions/checkout@v6 with: fetch-depth: 0 - name: Set up Go 1.x diff --git a/Makefile b/Makefile index 6030267ab..2b9dfd3c1 100644 --- a/Makefile +++ b/Makefile @@ -136,11 +136,15 @@ test-unit-all: \ test-unit-non-plugin \ test-unit-kustomize-plugins -# This target is used by our Github Actions CI to run unit tests for all non-plugin modules in multiple GOOS environments. .PHONY: test-unit-non-plugin test-unit-non-plugin: ./hack/for-each-module.sh "make test" "./plugin/*" 20 +# This target is used by our Github Actions CI to run unit tests for all non-plugin and non-released modules in multiple GOOS environments. +.PHONY: test-unit-non-plugin-and-non-released +test-unit-non-plugin-and-non-released: + ./hack/for-each-module.sh "make test" "./plugin/*|./kyaml/go.mod|./cmd/config/go.mod|./api/go.mod|./kustomize/go.mod" 16 + .PHONY: build-non-plugin-all build-non-plugin-all: ./hack/for-each-module.sh "make build" "./plugin/*" 20 @@ -183,7 +187,7 @@ test-examples-kustomize-against-latest-release: $(MYGOBIN)/mdrip workspace-sync: go work sync ./hack/doGoMod.sh tidy - + # --- Cleanup targets --- .PHONY: clean clean: clean-kustomize-external-go-plugin uninstall-tools diff --git a/api/internal/builtins/HelmChartInflationGenerator.go b/api/internal/builtins/HelmChartInflationGenerator.go index 7a148f81e..aed01081a 100644 --- a/api/internal/builtins/HelmChartInflationGenerator.go +++ b/api/internal/builtins/HelmChartInflationGenerator.go @@ -367,9 +367,9 @@ func (p *HelmChartInflationGeneratorPlugin) markHelmGeneratedResources(rm resmap return nil } -// checkHelmVersion will return an error if the helm version is not V3 +// checkHelmVersion will return an error if the helm version is not V3 or V4 func (p *HelmChartInflationGeneratorPlugin) checkHelmVersion() error { - stdout, err := p.runHelmCommand([]string{"version", "-c", "--short"}) + stdout, err := p.runHelmCommand([]string{"version", "--short"}) if err != nil { return err } @@ -385,8 +385,8 @@ func (p *HelmChartInflationGeneratorPlugin) checkHelmVersion() error { v = v[1:] } majorVersion := strings.Split(v, ".")[0] - if majorVersion != "3" { - return fmt.Errorf("this plugin requires helm V3 but got v%s", v) + if majorVersion != "3" && majorVersion != "4" { + return fmt.Errorf("this plugin requires helm V3 or V4 but got v%s", v) } return nil } diff --git a/api/internal/builtins/PatchTransformer.go b/api/internal/builtins/PatchTransformer.go index 278d10093..442291d11 100644 --- a/api/internal/builtins/PatchTransformer.go +++ b/api/internal/builtins/PatchTransformer.go @@ -89,7 +89,10 @@ func (p *PatchTransformerPlugin) Transform(m resmap.ResMap) error { if p.smPatches != nil { return p.transformStrategicMerge(m) } - return p.transformJson6902(m) + if p.jsonPatches != nil { + return p.transformJson6902(m) + } + return nil } // transformStrategicMerge applies each loaded strategic merge patch diff --git a/api/internal/loader/fileloader_test.go b/api/internal/loader/fileloader_test.go index d84ecbfb1..17e8fbfe5 100644 --- a/api/internal/loader/fileloader_test.go +++ b/api/internal/loader/fileloader_test.go @@ -686,5 +686,5 @@ func TestLoaderHTTPMalformedURL(t *testing.T) { RestrictionRootOnly, MakeFakeFs([]testData{}), filesys.Separator) _, err := l1.Load(malformedURL) require.Error(err) - require.Equal("HTTP Error: status code 500 (Internal Server Error)", err.Error()) + require.Equal("HTTP Error: status code 400 (Bad Request)", err.Error()) } diff --git a/api/krusty/helmchartinflationgenerator_test.go b/api/krusty/helmchartinflationgenerator_test.go index 1e54738e2..4a0d8bb96 100644 --- a/api/krusty/helmchartinflationgenerator_test.go +++ b/api/krusty/helmchartinflationgenerator_test.go @@ -1174,7 +1174,6 @@ kind: Service metadata: annotations: helm-namespace: helm-ns - internal.config.kubernetes.io/helm-generated: "true" name: test-service namespace: helm-ns `) diff --git a/api/krusty/multiplepatch_test.go b/api/krusty/multiplepatch_test.go index b2d24c5c7..5293ba31d 100644 --- a/api/krusty/multiplepatch_test.go +++ b/api/krusty/multiplepatch_test.go @@ -4,6 +4,7 @@ package krusty_test import ( + "strings" "testing" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" @@ -1766,3 +1767,108 @@ metadata: name: fluentd-sa-abc `) } + +// TestEmptyPatchFilesShouldBeIgnored verifies that empty patch files are ignored. +// Tests three cases: +// 1. Completely empty file +// 2. File with only comments +// 3. File with whitespace and comments +func TestEmptyPatchFilesShouldBeIgnored(t *testing.T) { + th := kusttest_test.MakeHarness(t) + + // Write a basic resource + th.WriteF("deployment.yaml", ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx +spec: + template: + spec: + containers: + - name: nginx + image: nginx +`) + + // Create empty patch files of different types + th.WriteF("empty.yaml", ``) + th.WriteF("comments-only.yaml", ` +# This is a comment +# Another comment +`) + th.WriteF("whitespace.yaml", ` + +# Comments with whitespace + + # Indented comment + +`) + + // Reference empty patches in kustomization + th.WriteK(".", ` +resources: +- deployment.yaml +patches: +- path: empty.yaml +- path: comments-only.yaml +- path: whitespace.yaml +`) + + // Empty patches should be ignored, output should be unchanged + m := th.Run(".", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: nginx +spec: + template: + spec: + containers: + - image: nginx + name: nginx +`) +} + +// TestEmptyPatchesStrategicMergeFails verifies that empty patch files are +// handled correctly with the deprecated patchesStrategicMerge field +func TestEmptyPatchesStrategicMergeFails(t *testing.T) { + th := kusttest_test.MakeHarness(t) + + // Create a basic resource + th.WriteF("resource.yaml", ` +apiVersion: v1 +kind: ConfigMap +metadata: + name: dummy +data: + dummy: value +`) + + // Create an empty patch file + th.WriteF("empty-patch.yaml", ``) + + // Create a patch file with only comments + th.WriteF("comments-patch.yaml", ` +# This is just a comment +# Another comment +`) + + // Create kustomization using patchesStrategicMerge + th.WriteK(".", ` +resources: +- resource.yaml +patchesStrategicMerge: +- empty-patch.yaml +- comments-patch.yaml +`) + + // This fails with message + err := th.RunWithErr(".", th.MakeDefaultOptions()) + if err == nil { + t.Fatalf("expected error for empty patchesStrategicMerge files but got none") + } + if !strings.Contains(err.Error(), "patch appears to be empty") { + t.Fatalf("unexpected error: %v", err) + } +} diff --git a/hack/for-each-module.sh b/hack/for-each-module.sh index 5981a68e2..3f15289ff 100755 --- a/hack/for-each-module.sh +++ b/hack/for-each-module.sh @@ -22,8 +22,18 @@ seen=() KUSTOMIZE_ROOT=$(pwd) export KUSTOMIZE_ROOT +# Build find command with multiple -not -path options +find_cmd="find . -name go.mod -not -path \"./site/*\"" +if [[ -n "$skip_pattern" ]]; then + # Split skip_pattern by | and add -not -path for each + IFS='|' read -ra PATTERNS <<< "$skip_pattern" + for pattern in "${PATTERNS[@]}"; do + find_cmd+=" -not -path \"$pattern\"" + done +fi + # verify all modules pass validation -for i in $(find . -name go.mod -not -path "./site/*" -not -path "$skip_pattern"); do +for i in $(eval "$find_cmd"); do pushd . cd $(dirname "$i"); diff --git a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go index 36a0c6a39..105bf9b94 100644 --- a/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go +++ b/plugin/builtin/helmchartinflationgenerator/HelmChartInflationGenerator.go @@ -2,7 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 // Helm chart inflation generator. -// Uses helm V3 to generate k8s YAML from a helm chart. +// Uses helm V3 or V4 to generate k8s YAML from a helm chart. //go:generate pluginator package main @@ -375,9 +375,9 @@ func (p *plugin) markHelmGeneratedResources(rm resmap.ResMap) error { return nil } -// checkHelmVersion will return an error if the helm version is not V3 +// checkHelmVersion will return an error if the helm version is not V3 or V4 func (p *plugin) checkHelmVersion() error { - stdout, err := p.runHelmCommand([]string{"version", "-c", "--short"}) + stdout, err := p.runHelmCommand([]string{"version", "--short"}) if err != nil { return err } @@ -393,8 +393,8 @@ func (p *plugin) checkHelmVersion() error { v = v[1:] } majorVersion := strings.Split(v, ".")[0] - if majorVersion != "3" { - return fmt.Errorf("this plugin requires helm V3 but got v%s", v) + if majorVersion != "3" && majorVersion != "4" { + return fmt.Errorf("this plugin requires helm V3 or V4 but got v%s", v) } return nil } diff --git a/plugin/builtin/patchtransformer/PatchTransformer.go b/plugin/builtin/patchtransformer/PatchTransformer.go index 77e5439b0..4a2be54bf 100644 --- a/plugin/builtin/patchtransformer/PatchTransformer.go +++ b/plugin/builtin/patchtransformer/PatchTransformer.go @@ -94,7 +94,10 @@ func (p *plugin) Transform(m resmap.ResMap) error { if p.smPatches != nil { return p.transformStrategicMerge(m) } - return p.transformJson6902(m) + if p.jsonPatches != nil { + return p.transformJson6902(m) + } + return nil } // transformStrategicMerge applies each loaded strategic merge patch