From c8bcbcd6298cfb65fd2bf4407e5fc7b478e6f5a1 Mon Sep 17 00:00:00 2001 From: Aditya Menon Date: Fri, 21 Nov 2025 01:32:54 +0100 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20Fix=20four=20critical=20issues:?= =?UTF-8?q?=20environment=20merging,=20kubeVersion=20detection,=20lookup()?= =?UTF-8?q?=20with=20kustomize,=20and=20Helm=204=20color=20flags=20(#2276)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: deep merge environments from multiple bases (#2273) Problem: When using multiple base helmfiles, environment values were being completely replaced instead of deep-merged due to mergo.WithOverride introduced in PR #2228. Solution: - Created mergeEnvironments() function for proper deep merging - Manually merge environment Values and Secrets slices before struct merge - Preserves all environment values from both base and current helmfile Testing: - Added TestEnvironmentMergingWithBases with two scenarios: 1. Multiple bases with overlapping environment values 2. Environment values with array merging Fixes #2273 Signed-off-by: Aditya Menon * fix: auto-detect Kubernetes version for helm-diff (#2275) Problem: When helmfile runs helm-diff without specifying kubeVersion, helm-diff falls back to v1.20.0. This causes chart compatibility checks to fail for charts requiring newer Kubernetes versions (e.g., kubeVersion: ">=1.25.0"). Root Cause: - flagsForDiff() was not passing kubeVersion to helm-diff plugin - Without --kube-version flag, helm-diff uses default v1.20.0 Solution: - Created pkg/cluster package with DetectServerVersion() function - Auto-detect cluster version using k8s.io/client-go discovery API - Pass detected version to helm-diff via --kube-version flag - Priority: helmfile.yaml kubeVersion > auto-detected version - Works with both Helm 3 and Helm 4 Implementation: - pkg/cluster/version.go: Cluster version detection - pkg/app/app.go: detectKubeVersion() helper used in diff() and apply() - pkg/state/state.go: Added DetectedKubeVersion field to DiffOpts - Integrated into flagsForDiff() with proper precedence Testing: - Unit tests for cluster version detection - Unit tests for kubeVersion precedence logic - Integration test with chart requiring Kubernetes >=1.25.0 - Tests verify upgrade scenario (critical failure case from issue) - Validated with both Helm 3 and Helm 4 Fixes #2275 Signed-off-by: Aditya Menon * fix: enable lookup() function with strategicMergePatches (#2271) Problem: When using strategicMergePatches (kustomize), Helm's lookup() function stops working. Charts like Grafana use lookup() to preserve existing resource values (e.g., PVC volumeName), which get lost when using patches. Root Cause: - Chartify runs "helm template" to render charts before applying patches - By default, "helm template" runs client-side without cluster access - The lookup() function requires cluster connectivity to query resources - Without cluster access, lookup() returns empty values Solution: - Pass --dry-run=server to helm template when using kustomize patches - This enables cluster connectivity for lookup() while keeping client-side rendering - Only applied to commands requiring cluster access (diff, apply, sync, etc.) - Offline commands (template, lint, build) remain cluster-independent Implementation: - Modified processChartification() to accept helmfileCommand parameter - Added switch-based logic to determine cluster requirement per command - Conditionally set chartifyOpts.TemplateArgs = "--dry-run=server" - Safe default: unknown commands assume cluster access Command Behavior: - helmfile diff/apply/sync: Uses --dry-run=server, lookup() works - helmfile template/lint/build: No cluster requirement, works offline - Charts without lookup(): Unaffected - Charts with lookup() + cluster: Lookup values preserved correctly Testing: - Integration test with ConfigMap using lookup() to preserve values - Verifies lookup works with strategicMergePatches - Tests both with and without cluster access - Validates offline template command still works Fixes #2271 Signed-off-by: Aditya Menon * fix: remove unnecessary error return from mergeEnvironments The mergeEnvironments function always returns nil, making the error return value unnecessary. This fixes the unparam linter warning. - Changed function signature to not return error - Updated call site to not handle error - All tests still pass Signed-off-by: Aditya Menon * fix: handle nil Environments map in mergeEnvironments Fixes panic when base helmfile has nil Environments map. Initialize the destination map if nil before merging to prevent "assignment to entry in nil map" panic. - Added nil check in mergeEnvironments to return early - Initialize layers[0].Environments before merge if nil - Fixes TestVisitDesiredStatesWithReleasesFiltered_Issue1008_MissingNonDefaultEnvInBase The panic occurred when a base helmfile didn't define any environments but a subsequent layer did. Now we properly initialize an empty map to merge into. Signed-off-by: Aditya Menon * test: disable kubeVersion auto-detection in unit tests Add DisableKubeVersionAutoDetection field to App struct to prevent unit tests from connecting to real Kubernetes clusters during testing. The kubeVersion auto-detection feature (issue #2275) was causing unit tests to fail because: 1. Tests use mock helm implementations without real cluster access 2. Auto-detection was connecting to local minikube cluster (v1.34.0) 3. Test expectations didn't include --kube-version flag in diff keys Solution: - Add DisableKubeVersionAutoDetection bool field to App struct - Check this flag in detectKubeVersion() before attempting detection - Set flag to true in all pkg/app/*_test.go files This ensures unit tests remain isolated and don't depend on external cluster state while preserving auto-detection for production use. Signed-off-by: Aditya Menon * chore: upgrade helm-diff plugin to v3.14.1 Update helm-diff plugin from v3.14.0 to v3.14.1 across all environments: - Dockerfiles (main, debian-stable-slim, ubuntu) - CI workflow matrix configurations - Integration test default version This ensures consistency across development, testing, and production environments. Signed-off-by: Aditya Menon * test: fix table formatting and improve E2E test infrastructure This commit addresses multiple test failures and improves the testing infrastructure for better reliability and maintainability. Table Formatting Fixes: - Added trimTrailingWhitespace() helper function to remove trailing whitespace from table output in both FormatAsTable() and printDAG() - Fixes TestList and TestDAG failures caused by tabwriter padding empty columns with trailing spaces - Updated golden file for table output test to match new behavior E2E Test Infrastructure Improvements: - Implemented dynamic port allocation for Docker registry tests to prevent port conflicts (replaced hardcoded port 5000/5001) - Added getFreePort() function using kernel-allocated unused ports - Added waitForRegistry() function with proper health check polling of Docker Registry /v2/ endpoint (replaces sleep hack) - Added prepareInputFile() function to handle port substitution and path resolution when copying helmfile configs to temp directories - Extracted setupLocalDockerRegistry() helper to reduce cognitive complexity from 111 to ≤110 (gocognit threshold) - Added port normalization in test output to replace dynamic ports with $REGISTRY_PORT placeholder for deterministic comparisons Test Configuration Updates: - Updated OCI chart tests to use dynamic port allocation via $REGISTRY_PORT placeholder in helmfile configs - Converted relative chart paths to absolute paths when input files are copied to temp directories (fixes path resolution issues) - Left postrenderer paths as relative since they're resolved from working directory (works for both Helm 3 and Helm 4) Golden File Updates: - Updated all OCI-related test expected outputs to use $REGISTRY_PORT placeholder instead of hardcoded ports - Removed trailing whitespace from issue_493 test expected output - Updated postrenderer test outputs to reflect chart path normalization Test Cleanup: - Removed unused fakeInit struct and CheckHelmPlugins() call from snapshot tests (not needed for template/fetch/list commands) - Removed unused imports (app, helmexec packages) Technical Details: - Port allocation uses net.Listen with port 0 for kernel assignment - Registry health check polls with 500ms intervals and 30s timeout - Chart paths: ../../charts/* → absolute paths (input file moves to temp) - Postrenderer paths: remain relative (resolved from working directory) - OCI cache paths normalized: oci__localhost_PORT → oci__localhost_$REGISTRY_PORT All originally failing tests now pass: - TestList ✓ - TestDAG ✓ - TestHelmfileTemplateWithBuildCommand (all OCI tests) ✓ - TestFormatAsTable ✓ Fixes three test failures reported in issue. Signed-off-by: Aditya Menon * fix(test): convert postrenderer paths to absolute for Helm 3 Helm 3 resolves postrenderer script paths relative to the helmfile location. When the input file is copied to a temp directory for port substitution, relative postrenderer paths break. Solution: - Added postrenderersDir parameter to prepareInputFile() - Convert ../../postrenderers/* to absolute paths for Helm 3 only - Use existing isHelm4() function to detect Helm version - Helm 4 extracts plugin names from paths, so works with relative This fixes the postrenderer test failure in CI where Helm 3 could not find the postrenderer script at the relative path. Fixes: Error: unable to find binary at ../../postrenderers/add-cm2.bash Signed-off-by: Aditya Menon * fix(test): remove remaining hardcoded port 5001 in OCI tests Updated 4 remaining OCI chart tests that still had hardcoded port 5001: - oci_chart_pull - oci_chart_pull_once - oci_chart_pull_once2 - oci_chart_pull_direct Changes: - config.yaml: Removed hardcoded port, use dynamic allocation - input.yaml.gotmpl: Replaced localhost:5001 with localhost:$REGISTRY_PORT This ensures all OCI chart tests use dynamic port allocation to prevent port conflicts during parallel test execution. Signed-off-by: Aditya Menon * fix: prevent helm-diff from normalizing server-side defaults Problem: The suppress-output-line-regex integration test was failing because helm-diff was reporting "has changed, but diff is empty after suppression" for Service resources when it should have shown ipFamilyPolicy and ipFamilies fields being removed. Root Cause: When auto-detected kubeVersion (e.g., 1.34.0) is passed to helm-diff via --kube-version flag, helm-diff normalizes server-side defaults. This makes fields like ipFamilyPolicy and ipFamilies appear unchanged, even though they don't exist in the chart template and will be removed by the upgrade. After applying suppressOutputLineRegex patterns, only label changes remained (helm.sh/chart and app.kubernetes.io/version). These were correctly suppressed, leaving an empty diff - hence the "diff is empty after suppression" message. Solution: Added a new configuration option 'disableAutoDetectedKubeVersionForDiff' to allow disabling auto-detected kubeVersion being passed to helm-diff. This prevents helm-diff from normalizing server-side defaults when needed. Default behavior: Pass auto-detected kubeVersion (fixes issue #2275, existing behavior) Opt-out behavior: Set flag to true to only use explicit kubeVersion from helmfile.yaml helmDefaults: disableAutoDetectedKubeVersionForDiff: true # false by default releases: - name: myrelease disableAutoDetectedKubeVersionForDiff: true # override per-release Implementation: - Added DisableAutoDetectedKubeVersionForDiff field to HelmSpec and ReleaseSpec - Updated flagsForDiff() to check this flag before passing kubeVersion - Default (false): pass auto-detected kubeVersion (fixes issue #2275) - Opt-out (true): only pass explicit kubeVersion from helmfile.yaml - Updated suppress-output-line-regex test to disable auto-detected kubeVersion This approach: - Maintains backward compatibility (default passes auto-detected kubeVersion) - Fixes issue #2275 for charts requiring newer Kubernetes versions - Allows users to opt-out when server-side normalization causes issues - Fixes suppress-output-line-regex test regression Signed-off-by: Aditya Menon * test: update hash values in TestGenerateID after adding DisableAutoDetectedKubeVersionForDiff field The hash values in TestGenerateID needed to be updated because adding the DisableAutoDetectedKubeVersionForDiff field to ReleaseSpec changed the structure's hash representation. This is expected behavior as generateValuesID() hashes the entire ReleaseSpec structure. Updated all expected hash values to match the new values: - baseline: foo-values-66f7fd6f7b - different bytes content: foo-values-6664979cd7 - different map content: foo-values-78897dfd49 - different chart: foo-values-64b7846cb7 - different name: bar-values-576cb7ddc7 - specific ns: myns-foo-values-6c567f54c Signed-off-by: Aditya Menon * fix: address PR review comments and resolve issue #2280 This commit addresses all review comments from GitHub Copilot and resolves issue #2280 regarding --color flag conflict with Helm 4. Changes: 1. Fixed documentation in pkg/cluster/version.go - Updated function comment to reflect error return behavior - Corrected version format example and comment 2. Added complete command categorization in pkg/state/state.go - Added all helmfile commands to cluster access switch statement - Properly categorized 15+ commands based on cluster requirements - Added clarifying comments for command groups 3. Resolved issue #2280: --color flag conflict with Helm 4 - In Helm 4, --color expects a value (never/auto/always) - Converts --color to --color=always for Helm 4 - Converts --no-color to --color=never for Helm 4 - Prevents Helm from consuming next argument as color value - Added comprehensive unit tests - Added integration test (Helm 4 only) Issue #2280 Details: When running helmfile diff with --color and --context flags on Helm 4, the --color flag would consume --context as its value, resulting in: "invalid color mode '--context': must be one of: never, auto, always" The fix detects Helm 4 and converts boolean color flags to the format Helm 4 expects, preventing the argument consumption issue. Fixes #2280 Signed-off-by: Aditya Menon * fix: correct kubeVersion precedence comment in test The comment incorrectly stated that state.KubeVersion takes precedence over paramKubeVersion, but the actual implementation (getKubeVersion in state.go:3354-3364) shows the correct order is: 1. paramKubeVersion (auto-detected from cluster) 2. release.KubeVersion (per-release override) 3. state.KubeVersion (helmfile.yaml global setting) Updated the comment to match the implementation and the test cases. Signed-off-by: Aditya Menon * fix: resolve Helm 4 --color flag conflict (issue #2280) This commit resolves issue #2280 where the --color flag causes Helm 4 to consume the next argument, resulting in errors like: "invalid color mode '--context': must be one of: never, auto, always" Root Cause: In Helm 4, the --color flag is parsed by the Helm binary before being passed to plugins like helm-diff. This causes Helm to interpret the next argument (e.g., --context) as the value for --color. Solution: Remove --color and --no-color flags from helm-diff commands when using Helm 4, and instead use the HELM_DIFF_COLOR environment variable. The helm-diff plugin supports HELM_DIFF_COLOR=[true|false] as an alternative to the --color/--no-color flags. Changes: 1. Added filterColorFlagsForHelm4() function in pkg/helmexec/exec.go - Removes --color and --no-color flags from flags slice - Sets HELM_DIFF_COLOR=true for --color - Sets HELM_DIFF_COLOR=false for --no-color 2. Modified DiffRelease() to call filterColorFlagsForHelm4() on Helm 4 3. Added comprehensive unit tests in pkg/helmexec/exec_test.go - Test_DiffRelease_ColorFlagHelm4: Verifies flags are filtered - Test_FilterColorFlagsForHelm4: Tests all flag combinations 4. Added integration test in test/integration/test-cases/issue-2280.sh - Tests the exact scenario from issue #2280 - Verifies --color and --context flags work together - Helm 4 only test (skipped on Helm 3) Fixes #2280 Signed-off-by: Aditya Menon * refactor: apply Copilot code review nitpicks This commit addresses minor code quality improvements suggested by GitHub Copilot's automated review. Changes: 1. pkg/app/formatters.go - Optimize trimTrailingWhitespace() - Only modify lines that actually have trailing whitespace - Avoids unnecessary string allocations for clean lines - Performance optimization for table formatting 2. test/e2e/template/helmfile/snapshot_test.go - Use 0600 permissions for temporary input files (was 0644) - Improves security by making temp files owner-only read/write - Prevents potential exposure of sensitive test data - Improve error messages in getFreePort() - Wrap errors with context using fmt.Errorf("%w") - Better error debugging when port allocation fails - Add retry logic to setupLocalDockerRegistry() - Handles race condition where port gets taken between allocation and use - Retries up to 3 times with new ports on "address already in use" errors - Fails fast on other Docker errors for better test diagnostics All tests passing. These are non-functional improvements that enhance code quality, performance, security, and test reliability. Signed-off-by: Aditya Menon * docs: improve code comments based on Copilot feedback This commit addresses documentation nitpicks from GitHub Copilot's automated review to improve code clarity and maintainability. Changes: 1. pkg/app/app.go - Clarify detectKubeVersion() return conditions - Updated comment to explicitly list all three cases when empty string is returned: kubeVersion already set, auto-detection disabled, or detection fails - Improves function documentation clarity 2. test/e2e/template/helmfile/snapshot_test.go - Added reference to retry logic in getFreePort() comment - Points callers to setupLocalDockerRegistry() for proper race condition handling example - Better guidance for future code maintainers 3. pkg/state/state.go - Explain patches check rationale - Added comment explaining why --dry-run=server is only enabled when patches are used - Clarifies that this is a conservative approach to minimize unnecessary cluster connections - Documents primary use case (Grafana chart with PVC preservation) All changes are documentation-only with no functional impact. All tests passing. Signed-off-by: Aditya Menon * refactor: enable lookup() for all cluster commands and add defensive check This commit addresses two Copilot review suggestions to improve code robustness and functionality. Changes: 1. pkg/state/state.go - Remove patches requirement for lookup() - Previously only enabled --dry-run=server when patches were present - Now enables it for ALL cluster-requiring commands - Rationale: lookup() function can be used without patches - Improves compatibility with charts using lookup() standalone - Trade-off: Slightly more cluster connections vs broader support 2. pkg/helmexec/exec.go - Add defensive check for HELM_DIFF_COLOR - Only set environment variable if not already present - Makes code more defensive for future implementation changes - Note: Changes behavior from "last wins" to "first wins" - In practice, env map is freshly created so check is precautionary 3. pkg/helmexec/exec_test.go - Update test expectations - Changed test case to reflect "first wins" behavior - Updated test name and comment for clarity Breaking behavior change: - When both --color and --no-color are present, the FIRST flag now wins instead of the LAST flag - This deviates from standard CLI conventions where later flags override earlier ones - However, this is unlikely to affect real usage as users rarely specify conflicting flags All tests passing. Signed-off-by: Aditya Menon --------- Signed-off-by: Aditya Menon --- .github/workflows/ci.yaml | 12 +- Dockerfile | 2 +- Dockerfile.debian-stable-slim | 2 +- Dockerfile.ubuntu | 2 +- go.mod | 2 +- pkg/app/app.go | 49 ++- pkg/app/app_apply_hooks_test.go | 9 +- pkg/app/app_apply_nokubectx_test.go | 11 +- pkg/app/app_apply_test.go | 11 +- pkg/app/app_diff_test.go | 22 +- pkg/app/app_gethelm_test.go | 11 +- pkg/app/app_lint_test.go | 11 +- pkg/app/app_list_test.go | 82 ++-- pkg/app/app_parallel_test.go | 30 +- pkg/app/app_sync_test.go | 11 +- pkg/app/app_template_test.go | 33 +- pkg/app/app_test.go | 406 ++++++++++-------- pkg/app/dag_test.go | 17 +- pkg/app/destroy_nokubectx_test.go | 11 +- pkg/app/destroy_test.go | 11 +- pkg/app/diff_nokubectx_test.go | 11 +- pkg/app/diff_test.go | 11 +- pkg/app/formatters.go | 18 +- pkg/app/init.go | 2 +- pkg/app/testdata/formatters/tableoutput | 4 +- pkg/cluster/version.go | 53 +++ pkg/cluster/version_test.go | 36 ++ pkg/helmexec/exec.go | 37 ++ pkg/helmexec/exec_test.go | 134 ++++++ pkg/state/create.go | 70 +++ pkg/state/create_test.go | 151 +++++++ pkg/state/state.go | 82 +++- pkg/state/state_kubeversion_test.go | 75 ++++ pkg/state/temp_test.go | 12 +- test/e2e/template/helmfile/snapshot_test.go | 218 +++++++--- .../issue_473_oci_chart_url_fetch/config.yaml | 2 +- .../input.yaml.gotmpl | 2 +- .../issue_473_oci_chart_url_fetch/output.yaml | 2 +- .../output.yaml | 4 +- .../snapshot/oci_chart_pull/config.yaml | 2 +- .../snapshot/oci_chart_pull/input.yaml.gotmpl | 2 +- .../snapshot/oci_chart_pull/output.yaml | 2 +- .../oci_chart_pull_direct/config.yaml | 2 +- .../oci_chart_pull_direct/input.yaml.gotmpl | 4 +- .../oci_chart_pull_direct/output.yaml | 6 +- .../snapshot/oci_chart_pull_once/config.yaml | 2 +- .../oci_chart_pull_once/input.yaml.gotmpl | 2 +- .../snapshot/oci_chart_pull_once/output.yaml | 4 +- .../snapshot/oci_chart_pull_once2/config.yaml | 2 +- .../oci_chart_pull_once2/input.yaml.gotmpl | 2 +- .../snapshot/oci_chart_pull_once2/output.yaml | 2 +- .../testdata/snapshot/oci_need/config.yaml | 2 +- .../snapshot/oci_need/input.yaml.gotmpl | 2 +- .../testdata/snapshot/oci_need/output.yaml | 2 +- .../snapshot/postrenderer/config.yaml | 2 +- .../snapshot/postrenderer/input.yaml.gotmpl | 2 +- .../snapshot/postrenderer/output-helm4.yaml | 6 +- .../snapshot/postrenderer/output.yaml | 6 +- test/integration/run.sh | 2 +- test/integration/test-cases/issue-2271.sh | 61 +++ .../issue-2271/input/dns-patch.yaml | 14 + .../input/helmfile-no-kustomize.yaml | 6 + .../test-cases/issue-2271/input/helmfile.yaml | 8 + .../issue-2271/input/test-chart/Chart.yaml | 6 + .../input/test-chart/templates/configmap.yaml | 16 + .../test-chart/templates/deployment.yaml | 20 + test/integration/test-cases/issue-2275.sh | 89 ++++ .../test-cases/issue-2275/input/helmfile.yaml | 5 + .../issue-2275/input/test-chart/Chart.yaml | 9 + .../test-chart/templates/deployment.yaml | 19 + test/integration/test-cases/issue-2280.sh | 80 ++++ .../issue-2280/input/chart/Chart.yaml | 5 + .../input/chart/templates/configmap.yaml | 6 + .../issue-2280/input/chart/values.yaml | 1 + .../test-cases/issue-2280/input/helmfile.yaml | 6 + .../input/helmfile.yaml.gotmpl | 3 + 76 files changed, 1637 insertions(+), 442 deletions(-) create mode 100644 pkg/cluster/version.go create mode 100644 pkg/cluster/version_test.go create mode 100644 pkg/state/state_kubeversion_test.go create mode 100755 test/integration/test-cases/issue-2271.sh create mode 100644 test/integration/test-cases/issue-2271/input/dns-patch.yaml create mode 100644 test/integration/test-cases/issue-2271/input/helmfile-no-kustomize.yaml create mode 100644 test/integration/test-cases/issue-2271/input/helmfile.yaml create mode 100644 test/integration/test-cases/issue-2271/input/test-chart/Chart.yaml create mode 100644 test/integration/test-cases/issue-2271/input/test-chart/templates/configmap.yaml create mode 100644 test/integration/test-cases/issue-2271/input/test-chart/templates/deployment.yaml create mode 100755 test/integration/test-cases/issue-2275.sh create mode 100644 test/integration/test-cases/issue-2275/input/helmfile.yaml create mode 100644 test/integration/test-cases/issue-2275/input/test-chart/Chart.yaml create mode 100644 test/integration/test-cases/issue-2275/input/test-chart/templates/deployment.yaml create mode 100644 test/integration/test-cases/issue-2280.sh create mode 100644 test/integration/test-cases/issue-2280/input/chart/Chart.yaml create mode 100644 test/integration/test-cases/issue-2280/input/chart/templates/configmap.yaml create mode 100644 test/integration/test-cases/issue-2280/input/chart/values.yaml create mode 100644 test/integration/test-cases/issue-2280/input/helmfile.yaml diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index b796eefb..a983d962 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -96,35 +96,35 @@ jobs: - helm-version: v3.18.6 kustomize-version: v5.8.0 plugin-secrets-version: 4.7.0 - plugin-diff-version: 3.14.0 + plugin-diff-version: 3.14.1 extra-helmfile-flags: '' # In case you need to test some optional helmfile features, # enable it via extra-helmfile-flags below. - helm-version: v3.18.6 kustomize-version: v5.8.0 plugin-secrets-version: 4.7.0 - plugin-diff-version: 3.14.0 + plugin-diff-version: 3.14.1 extra-helmfile-flags: '--enable-live-output' - helm-version: v3.19.2 kustomize-version: v5.8.0 plugin-secrets-version: 4.7.0 - plugin-diff-version: 3.14.0 + plugin-diff-version: 3.14.1 extra-helmfile-flags: '' - helm-version: v3.19.2 kustomize-version: v5.8.0 plugin-secrets-version: 4.7.0 - plugin-diff-version: 3.14.0 + plugin-diff-version: 3.14.1 extra-helmfile-flags: '--enable-live-output' # Helmfile now supports both Helm 3.x and Helm 4.x - helm-version: v4.0.0 kustomize-version: v5.8.0 plugin-secrets-version: 4.7.0 - plugin-diff-version: 3.14.0 + plugin-diff-version: 3.14.1 extra-helmfile-flags: '' - helm-version: v4.0.0 kustomize-version: v5.8.0 plugin-secrets-version: 4.7.0 - plugin-diff-version: 3.14.0 + plugin-diff-version: 3.14.1 extra-helmfile-flags: '--enable-live-output' steps: - uses: actions/checkout@v5 diff --git a/Dockerfile b/Dockerfile index c6ea7654..c6f80a99 100644 --- a/Dockerfile +++ b/Dockerfile @@ -93,7 +93,7 @@ RUN set -x && \ [ "$(age --version)" = "${AGE_VERSION}" ] && \ [ "$(age-keygen --version)" = "${AGE_VERSION}" ] -RUN helm plugin install https://github.com/databus23/helm-diff --version v3.14.0 --verify=false && \ +RUN helm plugin install https://github.com/databus23/helm-diff --version v3.14.1 --verify=false && \ helm plugin install https://github.com/jkroepke/helm-secrets/releases/download/v4.7.0/helm-secrets.tgz --verify=false && \ helm plugin install https://github.com/jkroepke/helm-secrets/releases/download/v4.7.0/helm-secrets-getter.tgz --verify=false && \ helm plugin install https://github.com/jkroepke/helm-secrets/releases/download/v4.7.0/helm-secrets-post-renderer.tgz --verify=false && \ diff --git a/Dockerfile.debian-stable-slim b/Dockerfile.debian-stable-slim index b07b4a28..f5a9d1b5 100644 --- a/Dockerfile.debian-stable-slim +++ b/Dockerfile.debian-stable-slim @@ -102,7 +102,7 @@ RUN set -x && \ [ "$(age --version)" = "${AGE_VERSION}" ] && \ [ "$(age-keygen --version)" = "${AGE_VERSION}" ] -RUN helm plugin install https://github.com/databus23/helm-diff --version v3.14.0 --verify=false && \ +RUN helm plugin install https://github.com/databus23/helm-diff --version v3.14.1 --verify=false && \ helm plugin install https://github.com/jkroepke/helm-secrets/releases/download/v4.7.0/helm-secrets.tgz --verify=false && \ helm plugin install https://github.com/jkroepke/helm-secrets/releases/download/v4.7.0/helm-secrets-getter.tgz --verify=false && \ helm plugin install https://github.com/jkroepke/helm-secrets/releases/download/v4.7.0/helm-secrets-post-renderer.tgz --verify=false && \ diff --git a/Dockerfile.ubuntu b/Dockerfile.ubuntu index e4bde36c..fbb5a9b2 100644 --- a/Dockerfile.ubuntu +++ b/Dockerfile.ubuntu @@ -102,7 +102,7 @@ RUN set -x && \ [ "$(age --version)" = "${AGE_VERSION}" ] && \ [ "$(age-keygen --version)" = "${AGE_VERSION}" ] -RUN helm plugin install https://github.com/databus23/helm-diff --version v3.14.0 --verify=false && \ +RUN helm plugin install https://github.com/databus23/helm-diff --version v3.14.1 --verify=false && \ helm plugin install https://github.com/jkroepke/helm-secrets/releases/download/v4.7.0/helm-secrets.tgz --verify=false && \ helm plugin install https://github.com/jkroepke/helm-secrets/releases/download/v4.7.0/helm-secrets-getter.tgz --verify=false && \ helm plugin install https://github.com/jkroepke/helm-secrets/releases/download/v4.7.0/helm-secrets-post-renderer.tgz --verify=false && \ diff --git a/go.mod b/go.mod index ca2730a7..c5a5a1ef 100644 --- a/go.mod +++ b/go.mod @@ -34,6 +34,7 @@ require ( helm.sh/helm/v3 v3.19.2 helm.sh/helm/v4 v4.0.0 k8s.io/apimachinery v0.34.2 + k8s.io/client-go v0.34.1 ) require ( @@ -326,7 +327,6 @@ require ( k8s.io/apiextensions-apiserver v0.34.1 // indirect k8s.io/apiserver v0.34.1 // indirect k8s.io/cli-runtime v0.34.1 // indirect - k8s.io/client-go v0.34.1 // indirect k8s.io/component-base v0.34.1 // indirect k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20250710124328-f3f2b991d03b // indirect diff --git a/pkg/app/app.go b/pkg/app/app.go index f6ee7cc8..6dffe8ed 100644 --- a/pkg/app/app.go +++ b/pkg/app/app.go @@ -15,6 +15,7 @@ import ( "go.uber.org/zap" "github.com/helmfile/helmfile/pkg/argparser" + "github.com/helmfile/helmfile/pkg/cluster" "github.com/helmfile/helmfile/pkg/envvar" "github.com/helmfile/helmfile/pkg/filesystem" "github.com/helmfile/helmfile/pkg/helmexec" @@ -28,14 +29,15 @@ var Cancel goContext.CancelFunc // App is the main application object. type App struct { - OverrideKubeContext string - OverrideHelmBinary string - OverrideKustomizeBinary string - EnableLiveOutput bool - StripArgsValuesOnExitError bool - DisableForceUpdate bool - EnforcePluginVerification bool - HelmOCIPlainHTTP bool + OverrideKubeContext string + OverrideHelmBinary string + OverrideKustomizeBinary string + EnableLiveOutput bool + StripArgsValuesOnExitError bool + DisableForceUpdate bool + EnforcePluginVerification bool + HelmOCIPlainHTTP bool + DisableKubeVersionAutoDetection bool Logger *zap.SugaredLogger Kubeconfig string @@ -1193,7 +1195,7 @@ func printDAG(batches [][]state.Release) string { _ = w.Flush() - return buf.String() + return trimTrailingWhitespace(buf.String()) } // nolint: unparam @@ -1556,6 +1558,8 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { // helm must be 2.11+ and helm-diff should be provided `--detailed-exitcode` in order for `helmfile apply` to work properly detailedExitCode := true + detectedKubeVersion := a.detectKubeVersion(st) + diffOpts := &state.DiffOpts{ Color: c.Color(), NoColor: c.NoColor(), @@ -1572,6 +1576,7 @@ func (a *App) apply(r *Run, c ApplyConfigProvider) (bool, bool, []error) { SkipSchemaValidation: c.SkipSchemaValidation(), SuppressOutputLineRegex: c.SuppressOutputLineRegex(), TakeOwnership: c.TakeOwnership(), + DetectedKubeVersion: detectedKubeVersion, } infoMsg, releasesToBeUpdated, releasesToBeDeleted, errs := r.diff(false, detailedExitCode, c, diffOpts) @@ -1789,6 +1794,29 @@ Do you really want to delete? return true, errs } +// detectKubeVersion auto-detects the Kubernetes cluster version if not specified in helmfile.yaml. +// This prevents helm-diff from falling back to v1.20.0 (issue #2275). +// Returns empty string when kubeVersion is already set in helmfile.yaml (not needed), +// when auto-detection is disabled, or if detection fails. +func (a *App) detectKubeVersion(st *state.HelmState) string { + if st.KubeVersion != "" { + return "" + } + + // Allow tests to disable auto-detection to avoid connecting to real clusters + if a.DisableKubeVersionAutoDetection { + return "" + } + + version, err := cluster.DetectServerVersion(a.Kubeconfig, a.OverrideKubeContext) + if err != nil { + // If detection fails, we silently continue - helm-diff will handle it + return "" + } + + return version +} + func (a *App) diff(r *Run, c DiffConfigProvider) (*string, bool, bool, []error) { var ( infoMsg *string @@ -1802,6 +1830,8 @@ func (a *App) diff(r *Run, c DiffConfigProvider) (*string, bool, bool, []error) helm.SetExtraArgs(GetArgs(c.Args(), r.state)...) + detectedKubeVersion := a.detectKubeVersion(st) + opts := &state.DiffOpts{ Context: c.Context(), Output: c.DiffOutput(), @@ -1817,6 +1847,7 @@ func (a *App) diff(r *Run, c DiffConfigProvider) (*string, bool, bool, []error) SkipSchemaValidation: c.SkipSchemaValidation(), SuppressOutputLineRegex: c.SuppressOutputLineRegex(), TakeOwnership: c.TakeOwnership(), + DetectedKubeVersion: detectedKubeVersion, } filtered := &Run{ diff --git a/pkg/app/app_apply_hooks_test.go b/pkg/app/app_apply_hooks_test.go index 0b8e5fc2..0bce491e 100644 --- a/pkg/app/app_apply_hooks_test.go +++ b/pkg/app/app_apply_hooks_test.go @@ -61,10 +61,11 @@ func TestApply_hooks(t *testing.T) { } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", "default"): helm, }, diff --git a/pkg/app/app_apply_nokubectx_test.go b/pkg/app/app_apply_nokubectx_test.go index e3246e4e..d4377be6 100644 --- a/pkg/app/app_apply_nokubectx_test.go +++ b/pkg/app/app_apply_nokubectx_test.go @@ -61,11 +61,12 @@ func TestApply_3(t *testing.T) { } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: filesystem.DefaultFileSystem(), - OverrideKubeContext: "", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: filesystem.DefaultFileSystem(), + OverrideKubeContext: "", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", ""): helm, }, diff --git a/pkg/app/app_apply_test.go b/pkg/app/app_apply_test.go index 8529954c..2f235f1a 100644 --- a/pkg/app/app_apply_test.go +++ b/pkg/app/app_apply_test.go @@ -61,11 +61,12 @@ func TestApply_2(t *testing.T) { } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: filesystem.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: filesystem.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", "default"): helm, }, diff --git a/pkg/app/app_diff_test.go b/pkg/app/app_diff_test.go index 9729af28..5b9e8d0b 100644 --- a/pkg/app/app_diff_test.go +++ b/pkg/app/app_diff_test.go @@ -101,11 +101,12 @@ releases: } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: filesystem.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: filesystem.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", "default"): helm, }, @@ -331,11 +332,12 @@ func TestDiffWithInstalled(t *testing.T) { } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: filesystem.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: filesystem.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", "default"): helm, }, diff --git a/pkg/app/app_gethelm_test.go b/pkg/app/app_gethelm_test.go index 3703e0ad..2a6ef4fb 100644 --- a/pkg/app/app_gethelm_test.go +++ b/pkg/app/app_gethelm_test.go @@ -29,11 +29,12 @@ func TestGetHelmWithEmptyDefaultHelmBinary(t *testing.T) { logger := newAppTestLogger() app := &App{ - OverrideHelmBinary: "", - OverrideKubeContext: "", - Logger: logger, - Env: "default", - ctx: goContext.Background(), + OverrideHelmBinary: "", + OverrideKubeContext: "", + DisableKubeVersionAutoDetection: true, + Logger: logger, + Env: "default", + ctx: goContext.Background(), } // This should NOT fail because app.getHelm() defaults empty DefaultHelmBinary to "helm" diff --git a/pkg/app/app_lint_test.go b/pkg/app/app_lint_test.go index c8dca9dd..f9b10836 100644 --- a/pkg/app/app_lint_test.go +++ b/pkg/app/app_lint_test.go @@ -103,11 +103,12 @@ releases: } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", "default"): helm, }, diff --git a/pkg/app/app_list_test.go b/pkg/app/app_list_test.go index edda7d33..f8084682 100644 --- a/pkg/app/app_list_test.go +++ b/pkg/app/app_list_test.go @@ -44,17 +44,17 @@ environments: --- releases: - name: logging - chart: incubator/raw + chart: incubator/raw namespace: kube-system - name: kubernetes-external-secrets - chart: incubator/raw + chart: incubator/raw namespace: kube-system needs: - kube-system/logging - name: external-secrets - chart: incubator/raw + chart: incubator/raw namespace: default labels: app: test @@ -62,7 +62,7 @@ releases: - kube-system/kubernetes-external-secrets - name: my-release - chart: incubator/raw + chart: incubator/raw namespace: default labels: app: test @@ -72,17 +72,17 @@ releases: # Disabled releases are treated as missing - name: disabled - chart: incubator/raw + chart: incubator/raw namespace: kube-system installed: false - name: test2 - chart: incubator/raw + chart: incubator/raw needs: - kube-system/disabled - name: test3 - chart: incubator/raw + chart: incubator/raw needs: - test2 `, @@ -111,18 +111,19 @@ releases: "/path/to/helmfile.d/helmfile_3.yaml": ` releases: - name: global - chart: incubator/raw + chart: incubator/raw namespace: kube-system `, } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: tc.environment, - Logger: logger, - valsRuntime: valsRuntime, + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: tc.environment, + Logger: logger, + valsRuntime: valsRuntime, }, files) expectNoCallsToHelm(app) @@ -160,15 +161,15 @@ releases: check(t, testcase{ environment: "default", expected: `NAME NAMESPACE ENABLED INSTALLED LABELS CHART VERSION -test2 true true chart:raw,name:test2,namespace: incubator/raw -test3 true true chart:raw,name:test3,namespace: incubator/raw -external-secrets default true true app:test,chart:raw,name:external-secrets,namespace:default incubator/raw -my-release default true true app:test,chart:raw,name:my-release,namespace:default incubator/raw -disabled kube-system true false chart:raw,name:disabled,namespace:kube-system incubator/raw -global kube-system true true chart:raw,name:global,namespace:kube-system incubator/raw -kubernetes-external-secrets kube-system true true chart:raw,name:kubernetes-external-secrets,namespace:kube-system incubator/raw -logging kube-system true true chart:raw,name:logging,namespace:kube-system incubator/raw -cache my-app true true app:test,chart:redis,name:cache,namespace:my-app bitnami/redis 17.0.7 +test2 true true chart:raw,name:test2,namespace: incubator/raw +test3 true true chart:raw,name:test3,namespace: incubator/raw +external-secrets default true true app:test,chart:raw,name:external-secrets,namespace:default incubator/raw +my-release default true true app:test,chart:raw,name:my-release,namespace:default incubator/raw +disabled kube-system true false chart:raw,name:disabled,namespace:kube-system incubator/raw +global kube-system true true chart:raw,name:global,namespace:kube-system incubator/raw +kubernetes-external-secrets kube-system true true chart:raw,name:kubernetes-external-secrets,namespace:kube-system incubator/raw +logging kube-system true true chart:raw,name:logging,namespace:kube-system incubator/raw +cache my-app true true app:test,chart:redis,name:cache,namespace:my-app bitnami/redis 17.0.7 database my-app true true chart:postgres,name:database,namespace:my-app bitnami/postgres 11.6.22 `, }, cfg) @@ -186,8 +187,8 @@ database my-app true true chart:postgres,name:da environment: "development", selectors: []string{"app=test"}, expected: `NAME NAMESPACE ENABLED INSTALLED LABELS CHART VERSION -external-secrets default true true app:test,chart:raw,name:external-secrets,namespace:default incubator/raw -my-release default true true app:test,chart:raw,name:my-release,namespace:default incubator/raw +external-secrets default true true app:test,chart:raw,name:external-secrets,namespace:default incubator/raw +my-release default true true app:test,chart:raw,name:my-release,namespace:default incubator/raw `, }, cfg) }) @@ -196,7 +197,7 @@ my-release default true true app:test,chart:raw,name:my-release, check(t, testcase{ environment: "test", expected: `NAME NAMESPACE ENABLED INSTALLED LABELS CHART VERSION -cache my-app true true app:test,chart:redis,name:cache,namespace:my-app bitnami/redis 17.0.7 +cache my-app true true app:test,chart:redis,name:cache,namespace:my-app bitnami/redis 17.0.7 database my-app true true chart:postgres,name:database,namespace:my-app bitnami/postgres 11.6.22 `, }, cfg) @@ -207,14 +208,14 @@ database my-app true true chart:postgres,name:database,namespace:my-a environment: "shared", // 'global' release has no environments, so is still excluded expected: `NAME NAMESPACE ENABLED INSTALLED LABELS CHART VERSION -test2 true true chart:raw,name:test2,namespace: incubator/raw -test3 true true chart:raw,name:test3,namespace: incubator/raw -external-secrets default true true app:test,chart:raw,name:external-secrets,namespace:default incubator/raw -my-release default true true app:test,chart:raw,name:my-release,namespace:default incubator/raw -disabled kube-system true false chart:raw,name:disabled,namespace:kube-system incubator/raw -kubernetes-external-secrets kube-system true true chart:raw,name:kubernetes-external-secrets,namespace:kube-system incubator/raw -logging kube-system true true chart:raw,name:logging,namespace:kube-system incubator/raw -cache my-app true true app:test,chart:redis,name:cache,namespace:my-app bitnami/redis 17.0.7 +test2 true true chart:raw,name:test2,namespace: incubator/raw +test3 true true chart:raw,name:test3,namespace: incubator/raw +external-secrets default true true app:test,chart:raw,name:external-secrets,namespace:default incubator/raw +my-release default true true app:test,chart:raw,name:my-release,namespace:default incubator/raw +disabled kube-system true false chart:raw,name:disabled,namespace:kube-system incubator/raw +kubernetes-external-secrets kube-system true true chart:raw,name:kubernetes-external-secrets,namespace:kube-system incubator/raw +logging kube-system true true chart:raw,name:logging,namespace:kube-system incubator/raw +cache my-app true true app:test,chart:redis,name:cache,namespace:my-app bitnami/redis 17.0.7 database my-app true true chart:postgres,name:database,namespace:my-app bitnami/postgres 11.6.22 `, }, cfg) @@ -270,12 +271,13 @@ releases: logger := helmexec.NewLogger(syncWriter, "debug") app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, - Namespace: "testNamespace", + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + Namespace: "testNamespace", }, files) expectNoCallsToHelm(app) diff --git a/pkg/app/app_parallel_test.go b/pkg/app/app_parallel_test.go index 7b59ad2b..8cc6b9dc 100644 --- a/pkg/app/app_parallel_test.go +++ b/pkg/app/app_parallel_test.go @@ -49,13 +49,14 @@ releases: } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, - valsRuntime: valsRuntime, - FileOrDir: "/path/to/helmfile.d", + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + valsRuntime: valsRuntime, + FileOrDir: "/path/to/helmfile.d", }, files) expectNoCallsToHelm(app) @@ -116,13 +117,14 @@ releases: } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, - valsRuntime: valsRuntime, - FileOrDir: "/path/to/helmfile.d", + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + valsRuntime: valsRuntime, + FileOrDir: "/path/to/helmfile.d", }, files) expectNoCallsToHelm(app) diff --git a/pkg/app/app_sync_test.go b/pkg/app/app_sync_test.go index 0836cd1d..948e8c06 100644 --- a/pkg/app/app_sync_test.go +++ b/pkg/app/app_sync_test.go @@ -59,11 +59,12 @@ func TestSync(t *testing.T) { } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", "default"): helm, }, diff --git a/pkg/app/app_template_test.go b/pkg/app/app_template_test.go index d13f643c..780249cc 100644 --- a/pkg/app/app_template_test.go +++ b/pkg/app/app_template_test.go @@ -106,11 +106,12 @@ releases: } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: &ffs.FileSystem{Glob: filepath.Glob}, - OverrideKubeContext: "default", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: &ffs.FileSystem{Glob: filepath.Glob}, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", "default"): helm, }, @@ -377,11 +378,12 @@ releases: } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: &ffs.FileSystem{Glob: filepath.Glob}, - OverrideKubeContext: "default", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: &ffs.FileSystem{Glob: filepath.Glob}, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", "default"): helm, }, @@ -478,11 +480,12 @@ releases: } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: &ffs.FileSystem{Glob: filepath.Glob}, - OverrideKubeContext: "default", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: &ffs.FileSystem{Glob: filepath.Glob}, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", "default"): helm, }, diff --git a/pkg/app/app_test.go b/pkg/app/app_test.go index 4f3a6f30..1987c64d 100644 --- a/pkg/app/app_test.go +++ b/pkg/app/app_test.go @@ -92,12 +92,13 @@ releases: fs := testhelper.NewTestFs(files) fs.GlobFixtures["/path/to/helmfile.d/a*.yaml"] = []string{"/path/to/helmfile.d/a2.yaml", "/path/to/helmfile.d/a1.yaml"} app := &App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Env: "default", - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Env: "default", + FileOrDir: "helmfile.yaml", } expectNoCallsToHelm(app) @@ -150,12 +151,13 @@ BAZ: 4 fs := testhelper.NewTestFs(files) fs.GlobFixtures["/path/to/env.*.yaml"] = []string{"/path/to/env.2.yaml", "/path/to/env.1.yaml"} app := &App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Env: "default", - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Env: "default", + FileOrDir: "helmfile.yaml", } expectNoCallsToHelm(app) @@ -193,12 +195,13 @@ releases: } fs := testhelper.NewTestFs(files) app := &App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Env: "default", - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Env: "default", + FileOrDir: "helmfile.yaml", } expectNoCallsToHelm(app) @@ -279,12 +282,13 @@ func TestUpdateStrategyParamValidation(t *testing.T) { for idx, c := range cases { fs := testhelper.NewTestFs(c.files) app := &App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Env: "default", - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Env: "default", + FileOrDir: "helmfile.yaml", } expectNoCallsToHelm(app) @@ -331,11 +335,12 @@ releases: } fs := testhelper.NewTestFs(files) app := &App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Env: "test", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Env: "test", } expectNoCallsToHelm(app) @@ -383,12 +388,13 @@ releases: } fs := testhelper.NewTestFs(files) app := &App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Env: "default", - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Env: "default", + FileOrDir: "helmfile.yaml", } expectNoCallsToHelm(app) @@ -449,13 +455,14 @@ releases: fs := testhelper.NewTestFs(files) fs.GlobFixtures["/path/to/helmfile.d/a*.yaml"] = []string{"/path/to/helmfile.d/a2.yaml", "/path/to/helmfile.d/a1.yaml"} app := &App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Selectors: []string{fmt.Sprintf("name=%s", testcase.name)}, - Namespace: "", - Env: "default", - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Selectors: []string{fmt.Sprintf("name=%s", testcase.name)}, + Namespace: "", + Env: "default", + FileOrDir: "helmfile.yaml", } expectNoCallsToHelm(app) @@ -506,13 +513,14 @@ releases: for _, testcase := range testcases { app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Selectors: []string{}, - Env: testcase.name, - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Selectors: []string{}, + Env: testcase.name, + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) @@ -620,13 +628,14 @@ releases: } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: helmexec.NewLogger(&ctxLogger{label: testcase.label}, "debug"), - Namespace: "", - Selectors: []string{testcase.label}, - Env: "default", - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: helmexec.NewLogger(&ctxLogger{label: testcase.label}, "debug"), + Namespace: "", + Selectors: []string{testcase.label}, + Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) @@ -861,13 +870,14 @@ func runFilterSubHelmFilesTests(testcases []struct { } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Selectors: []string{testcase.label}, - Env: "default", - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Selectors: []string{testcase.label}, + Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) @@ -943,13 +953,14 @@ ns: INLINE_NS } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Selectors: []string{}, - Env: "default", - FileOrDir: "/path/to/helmfile.yaml.gotmpl", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Selectors: []string{}, + Env: "default", + FileOrDir: "/path/to/helmfile.yaml.gotmpl", }, files) expectNoCallsToHelm(app) @@ -1047,13 +1058,14 @@ releases: return false, []error{} } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Selectors: []string{}, - Env: "default", - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Selectors: []string{}, + Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) @@ -1111,15 +1123,16 @@ bar: "bar1" return false, []error{} } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Selectors: []string{}, - Env: "default", - ValuesFiles: []string{"overrides.yaml"}, - Set: map[string]any{"bar": "bar2", "baz": "baz1"}, - FileOrDir: "helmfile.yaml.gotmpl", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Selectors: []string{}, + Env: "default", + ValuesFiles: []string{"overrides.yaml"}, + Set: map[string]any{"bar": "bar2", "baz": "baz1"}, + FileOrDir: "helmfile.yaml.gotmpl", }, files) expectNoCallsToHelm(app) @@ -1232,15 +1245,16 @@ x: return false, []error{} } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Selectors: []string{}, - Env: testcase.env, - ValuesFiles: []string{"overrides.yaml"}, - Set: map[string]any{"x": map[string]any{"hoge": "hoge_set", "fuga": "fuga_set"}}, - FileOrDir: "helmfile.yaml.gotmpl", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Selectors: []string{}, + Env: testcase.env, + ValuesFiles: []string{"overrides.yaml"}, + Set: map[string]any{"x": map[string]any{"hoge": "hoge_set", "fuga": "fuga_set"}}, + FileOrDir: "helmfile.yaml.gotmpl", }, files) expectNoCallsToHelm(app) @@ -1285,13 +1299,14 @@ releases: return false, []error{} } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Env: "default", - Selectors: []string{}, - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Env: "default", + Selectors: []string{}, + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) @@ -1341,13 +1356,14 @@ releases: return false, []error{} } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Selectors: []string{}, - Env: "default", - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Selectors: []string{}, + Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelm(app) @@ -1391,12 +1407,13 @@ releases: return false, []error{} } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Env: "default", - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelmVersion(app) @@ -1433,12 +1450,13 @@ releases: return false, []error{} } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Env: "default", - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelmVersion(app) @@ -1479,12 +1497,13 @@ releases: return false, []error{} } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Namespace: "", - Env: "default", - FileOrDir: "helmfile.yaml", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Namespace: "", + Env: "default", + FileOrDir: "helmfile.yaml", }, files) expectNoCallsToHelmVersion(app) @@ -1524,11 +1543,12 @@ func TestLoadDesiredStateFromYaml_DuplicateReleaseName(t *testing.T) { } fs := ffs.FromFileSystem(ffs.FileSystem{ReadFile: readFile}) app := &App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - fs: fs, - Env: "default", - Logger: newAppTestLogger(), + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + fs: fs, + Env: "default", + Logger: newAppTestLogger(), } expectNoCallsToHelm(app) @@ -1582,11 +1602,12 @@ helmDefaults: `, }) app := &App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - fs: testFs.ToFileSystem(), - Env: "default", - Logger: newAppTestLogger(), + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + fs: testFs.ToFileSystem(), + Env: "default", + Logger: newAppTestLogger(), } app.remote = remote.NewRemote(app.Logger, "", app.fs) @@ -2753,11 +2774,12 @@ releases: } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", "default"): helm, }, @@ -2826,10 +2848,11 @@ releases: } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", "default"): helm, }, @@ -3909,11 +3932,12 @@ releases: } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", "default"): helm, }, @@ -4028,11 +4052,12 @@ releases: t.Helper() app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", "default"): helm, }, @@ -4084,12 +4109,13 @@ releases: logger := helmexec.NewLogger(syncWriter, "debug") app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, - Namespace: "testNamespace", + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + Namespace: "testNamespace", }, files) expectNoCallsToHelm(app) @@ -4134,12 +4160,13 @@ releases: logger := helmexec.NewLogger(syncWriter, "debug") app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, - Namespace: "testNamespace", + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + Namespace: "testNamespace", }, files) expectNoCallsToHelm(app) @@ -4197,12 +4224,13 @@ releases: logger := helmexec.NewLogger(syncWriter, "debug") app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, - Namespace: "testNamespace", + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, + Namespace: "testNamespace", }, files) expectNoCallsToHelm(app) @@ -4214,10 +4242,10 @@ releases: assert.NoError(t, err) expected := `NAME NAMESPACE ENABLED INSTALLED LABELS CHART VERSION -myrelease1 testNamespace true false chart:mychart1,common:label,id:myrelease1,name:myrelease1,namespace:testNamespace mychart1 -myrelease2 testNamespace false true chart:mychart1,common:label,name:myrelease2,namespace:testNamespace mychart1 -myrelease3 testNamespace true true chart:mychart1,name:myrelease3,namespace:testNamespace mychart1 -myrelease4 testNamespace true true chart:mychart1,id:myrelease1,name:myrelease4,namespace:testNamespace mychart1 +myrelease1 testNamespace true false chart:mychart1,common:label,id:myrelease1,name:myrelease1,namespace:testNamespace mychart1 +myrelease2 testNamespace false true chart:mychart1,common:label,name:myrelease2,namespace:testNamespace mychart1 +myrelease3 testNamespace true true chart:mychart1,name:myrelease3,namespace:testNamespace mychart1 +myrelease4 testNamespace true true chart:mychart1,id:myrelease1,name:myrelease4,namespace:testNamespace mychart1 ` assert.Equal(t, expected, out) @@ -4252,11 +4280,12 @@ releases: {Name: "name", Value: "val"}} app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Env: "default", - FileOrDir: "helmfile.yaml.gotmpl", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Env: "default", + FileOrDir: "helmfile.yaml.gotmpl", }, files) expectNoCallsToHelm(app) @@ -4324,11 +4353,12 @@ releases: {Name: "name", Value: "val"}} app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - OverrideKubeContext: "default", - Logger: newAppTestLogger(), - Env: "default", - FileOrDir: "helmfile.yaml.gotmpl", + OverrideHelmBinary: DefaultHelmBinary, + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Logger: newAppTestLogger(), + Env: "default", + FileOrDir: "helmfile.yaml.gotmpl", }, files) expectNoCallsToHelm(app) diff --git a/pkg/app/dag_test.go b/pkg/app/dag_test.go index 599e9ad0..29a057ea 100644 --- a/pkg/app/dag_test.go +++ b/pkg/app/dag_test.go @@ -84,12 +84,13 @@ releases: } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: tc.environment, - Logger: logger, - valsRuntime: valsRuntime, + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: tc.environment, + Logger: logger, + valsRuntime: valsRuntime, }, files) expectNoCallsToHelm(app) @@ -127,8 +128,8 @@ releases: check(t, testcase{ environment: "default", expected: `GROUP RELEASE DEPENDENCIES -1 default/kube-system/logging -1 default/kube-system/disabled +1 default/kube-system/logging +1 default/kube-system/disabled 2 default/kube-system/kubernetes-external-secrets default/kube-system/logging 2 default//test2 default/kube-system/disabled 3 default/default/external-secrets default/kube-system/kubernetes-external-secrets diff --git a/pkg/app/destroy_nokubectx_test.go b/pkg/app/destroy_nokubectx_test.go index 828ec56c..279621c4 100644 --- a/pkg/app/destroy_nokubectx_test.go +++ b/pkg/app/destroy_nokubectx_test.go @@ -54,11 +54,12 @@ func TestDestroy_2(t *testing.T) { } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", ""): helm, }, diff --git a/pkg/app/destroy_test.go b/pkg/app/destroy_test.go index 39349ff5..45908f02 100644 --- a/pkg/app/destroy_test.go +++ b/pkg/app/destroy_test.go @@ -131,11 +131,12 @@ func TestDestroy(t *testing.T) { } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "default", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "default", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", "default"): helm, }, diff --git a/pkg/app/diff_nokubectx_test.go b/pkg/app/diff_nokubectx_test.go index b6624542..64aaf81e 100644 --- a/pkg/app/diff_nokubectx_test.go +++ b/pkg/app/diff_nokubectx_test.go @@ -806,11 +806,12 @@ releases: } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: "", - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: "", + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", ""): helm, }, diff --git a/pkg/app/diff_test.go b/pkg/app/diff_test.go index b813dbdd..3693bb3c 100644 --- a/pkg/app/diff_test.go +++ b/pkg/app/diff_test.go @@ -1170,11 +1170,12 @@ releases: } app := appWithFs(&App{ - OverrideHelmBinary: DefaultHelmBinary, - fs: ffs.DefaultFileSystem(), - OverrideKubeContext: overrideKubeContext, - Env: "default", - Logger: logger, + OverrideHelmBinary: DefaultHelmBinary, + fs: ffs.DefaultFileSystem(), + OverrideKubeContext: overrideKubeContext, + DisableKubeVersionAutoDetection: true, + Env: "default", + Logger: logger, helms: map[helmKey]helmexec.Interface{ createHelmKey("helm", overrideKubeContext): helm, }, diff --git a/pkg/app/formatters.go b/pkg/app/formatters.go index 7ec9d886..9716e495 100644 --- a/pkg/app/formatters.go +++ b/pkg/app/formatters.go @@ -3,10 +3,25 @@ package app import ( "encoding/json" "fmt" + "strings" "github.com/gosuri/uitable" ) +// trimTrailingWhitespace removes trailing whitespace from each line in the input string. +// This ensures consistent output formatting by removing spaces and tabs that table +// formatting libraries may add to pad empty columns. +func trimTrailingWhitespace(s string) string { + lines := strings.Split(s, "\n") + for i, line := range lines { + // Only modify lines that actually have trailing whitespace + if trimmed := strings.TrimRight(line, " \t"); trimmed != line { + lines[i] = trimmed + } + } + return strings.Join(lines, "\n") +} + func FormatAsTable(releases []*HelmRelease) error { table := uitable.New() table.AddRow("NAME", "NAMESPACE", "ENABLED", "INSTALLED", "LABELS", "CHART", "VERSION") @@ -15,7 +30,8 @@ func FormatAsTable(releases []*HelmRelease) error { table.AddRow(r.Name, r.Namespace, fmt.Sprintf("%t", r.Enabled), fmt.Sprintf("%t", r.Installed), r.Labels, r.Chart, r.Version) } - fmt.Println(table.String()) + output := trimTrailingWhitespace(table.String()) + fmt.Println(output) return nil } diff --git a/pkg/app/init.go b/pkg/app/init.go index c0e7a015..8f31a24c 100644 --- a/pkg/app/init.go +++ b/pkg/app/init.go @@ -19,7 +19,7 @@ import ( const ( HelmRequiredVersion = "v3.18.6" // Minimum required version (supports Helm 3.x and 4.x) - HelmDiffRecommendedVersion = "v3.14.0" + HelmDiffRecommendedVersion = "v3.14.1" HelmRecommendedVersion = "v4.0.0" // Recommended to use latest Helm 4 HelmSecretsRecommendedVersion = "v4.7.0" // v4.7.0+ works with both Helm 3 (single plugin) and Helm 4 (split plugin architecture) HelmGitRecommendedVersion = "v1.3.0" diff --git a/pkg/app/testdata/formatters/tableoutput b/pkg/app/testdata/formatters/tableoutput index 4630d29a..76da0860 100644 --- a/pkg/app/testdata/formatters/tableoutput +++ b/pkg/app/testdata/formatters/tableoutput @@ -1,3 +1,3 @@ NAME NAMESPACE ENABLED INSTALLED LABELS CHART VERSION -test test true true test test test -test1 test2 false false test1 test1 test1 +test test true true test test test +test1 test2 false false test1 test1 test1 diff --git a/pkg/cluster/version.go b/pkg/cluster/version.go new file mode 100644 index 00000000..23d74c55 --- /dev/null +++ b/pkg/cluster/version.go @@ -0,0 +1,53 @@ +package cluster + +import ( + "fmt" + + "k8s.io/client-go/discovery" + "k8s.io/client-go/tools/clientcmd" +) + +// DetectServerVersion detects the Kubernetes server version by connecting to the cluster. +// It returns the version in the format "major.minor.patch" (e.g., "1.34.1"). +// Returns an error if detection fails. +func DetectServerVersion(kubeconfig, context string) (string, error) { + // Build the kubeconfig + loadingRules := clientcmd.NewDefaultClientConfigLoadingRules() + if kubeconfig != "" { + loadingRules.ExplicitPath = kubeconfig + } + + configOverrides := &clientcmd.ConfigOverrides{} + if context != "" { + configOverrides.CurrentContext = context + } + + config, err := clientcmd.NewNonInteractiveDeferredLoadingClientConfig( + loadingRules, + configOverrides, + ).ClientConfig() + if err != nil { + return "", fmt.Errorf("failed to load kubeconfig: %w", err) + } + + // Create discovery client + discoveryClient, err := discovery.NewDiscoveryClientForConfig(config) + if err != nil { + return "", fmt.Errorf("failed to create discovery client: %w", err) + } + + // Get server version + serverVersion, err := discoveryClient.ServerVersion() + if err != nil { + return "", fmt.Errorf("failed to get server version: %w", err) + } + + // ServerVersion.GitVersion includes "v" prefix (e.g., "v1.34.1") + // Strip the "v" prefix to match Helm's --kube-version format (e.g., "1.34.1") + version := serverVersion.GitVersion + if len(version) > 0 && version[0] == 'v' { + version = version[1:] + } + + return version, nil +} diff --git a/pkg/cluster/version_test.go b/pkg/cluster/version_test.go new file mode 100644 index 00000000..cd6ba644 --- /dev/null +++ b/pkg/cluster/version_test.go @@ -0,0 +1,36 @@ +package cluster + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestDetectServerVersion_Integration tests the cluster version detection +// against a real Kubernetes cluster (if available). +// This test will be skipped if no cluster is accessible. +func TestDetectServerVersion_Integration(t *testing.T) { + // Try to detect version with default kubeconfig + version, err := DetectServerVersion("", "") + + if err != nil { + t.Skipf("Skipping test - no accessible Kubernetes cluster: %v", err) + return + } + + // If we got a version, verify it's in a valid format + require.NotEmpty(t, version, "Version should not be empty") + require.NotContains(t, version, "v", "Version should not have 'v' prefix") + + // Version should look like "1.xx.y" format + require.Regexp(t, `^\d+\.\d+\.\d+`, version, "Version should match semver format") +} + +// TestDetectServerVersion_InvalidConfig tests error handling +func TestDetectServerVersion_InvalidConfig(t *testing.T) { + // Try with a non-existent kubeconfig file + _, err := DetectServerVersion("/non/existent/path/kubeconfig", "") + + require.Error(t, err, "Should return error for invalid kubeconfig") + require.Contains(t, err.Error(), "failed to load kubeconfig", "Error should mention kubeconfig loading") +} diff --git a/pkg/helmexec/exec.go b/pkg/helmexec/exec.go index 90c7b98a..b2644541 100644 --- a/pkg/helmexec/exec.go +++ b/pkg/helmexec/exec.go @@ -669,6 +669,12 @@ func (helm *execer) DiffRelease(context HelmContext, name, chart, namespace stri overrideEnableLiveOutput = &enableLiveOutput } + // Issue #2280: In Helm 4, the --color flag is parsed by Helm before reaching the plugin, + // causing it to consume the next argument. Remove color flags and use HELM_DIFF_COLOR env var. + if helm.IsHelm4() { + flags = helm.filterColorFlagsForHelm4(flags, env) + } + out, err := helm.exec(append(append(preArgs, "diff", "upgrade", "--allow-unreleased", name, chart), flags...), env, overrideEnableLiveOutput) // Do our best to write STDOUT only when diff existed // Unfortunately, this works only when you run helmfile with `--detailed-exitcode` @@ -693,6 +699,37 @@ func (helm *execer) DiffRelease(context HelmContext, name, chart, namespace stri return err } +// filterColorFlagsForHelm4 removes --color and --no-color flags from the flags slice +// and sets the HELM_DIFF_COLOR environment variable instead. +// In Helm 4, the --color flag is parsed by Helm itself before reaching the helm-diff plugin, +// causing Helm to consume the next argument as the color value (issue #2280). +// The helm-diff plugin supports HELM_DIFF_COLOR=[true|false] env var as an alternative. +func (helm *execer) filterColorFlagsForHelm4(flags []string, env map[string]string) []string { + filtered := make([]string, 0, len(flags)) + + for _, flag := range flags { + switch flag { + case "--color": + // Use environment variable instead of flag for Helm 4 + // Only set if not already present (defensive check) + if _, exists := env["HELM_DIFF_COLOR"]; !exists { + env["HELM_DIFF_COLOR"] = "true" + } + case "--no-color": + // Use environment variable instead of flag for Helm 4 + // Only set if not already present (defensive check) + if _, exists := env["HELM_DIFF_COLOR"]; !exists { + env["HELM_DIFF_COLOR"] = "false" + } + default: + // Keep all other flags unchanged + filtered = append(filtered, flag) + } + } + + return filtered +} + func (helm *execer) Lint(name, chart string, flags ...string) error { helm.logger.Infof("Linting release=%v, chart=%v", name, chart) out, err := helm.exec(append([]string{"lint", chart}, flags...), map[string]string{}, nil) diff --git a/pkg/helmexec/exec_test.go b/pkg/helmexec/exec_test.go index 0400265a..154f810f 100644 --- a/pkg/helmexec/exec_test.go +++ b/pkg/helmexec/exec_test.go @@ -700,6 +700,140 @@ exec: helm --kubeconfig config --kube-context dev diff upgrade --allow-unrelease } } +func Test_DiffRelease_ColorFlagHelm4(t *testing.T) { + // Test that --color and --no-color flags are removed and HELM_DIFF_COLOR env var is set for Helm 4 + var buffer bytes.Buffer + logger := NewLogger(&buffer, "debug") + + // MockExecer creates a Helm 4 execer by default (returns v4.0.0) + helm, err := MockExecer(logger, "config", "dev") + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + // Verify it's Helm 4 + if !helm.IsHelm4() { + t.Errorf("expected Helm 4, got version: %v", helm.GetVersion()) + } + + // Test with --color flag + buffer.Reset() + err = helm.DiffRelease(HelmContext{}, "release", "chart", "default", false, "--color", "--context", "3") + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + // The --color flag should be removed and --context should remain + expected := `Comparing release=release, chart=chart, namespace=default + +exec: helm --kubeconfig config --kube-context dev diff upgrade --allow-unreleased release chart --context 3 +` + actual := buffer.String() + if actual != expected { + t.Errorf("helmexec.DiffRelease() with --color\nactual = %v\nexpect = %v", actual, expected) + } + + // Verify --color flag was removed + if strings.Contains(actual, "--color") { + t.Errorf("--color flag should have been removed, but got: %v", actual) + } + + // Test with --no-color flag + buffer.Reset() + err = helm.DiffRelease(HelmContext{}, "release", "chart", "default", false, "--no-color", "--context", "3") + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + // The --no-color flag should be removed and --context should remain + expected = `Comparing release=release, chart=chart, namespace=default + +exec: helm --kubeconfig config --kube-context dev diff upgrade --allow-unreleased release chart --context 3 +` + actual = buffer.String() + if actual != expected { + t.Errorf("helmexec.DiffRelease() with --no-color\nactual = %v\nexpect = %v", actual, expected) + } + + // Verify --no-color flag was removed + if strings.Contains(actual, "--no-color") { + t.Errorf("--no-color flag should have been removed, but got: %v", actual) + } +} + +func Test_FilterColorFlagsForHelm4(t *testing.T) { + var buffer bytes.Buffer + logger := NewLogger(&buffer, "debug") + helm, err := MockExecer(logger, "config", "dev") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + tests := []struct { + name string + inputFlags []string + expectedFlags []string + expectedEnvKey string + expectedEnvVal string + }{ + { + name: "color flag", + inputFlags: []string{"--color", "--context", "3"}, + expectedFlags: []string{"--context", "3"}, + expectedEnvKey: "HELM_DIFF_COLOR", + expectedEnvVal: "true", + }, + { + name: "no-color flag", + inputFlags: []string{"--no-color", "--context", "3"}, + expectedFlags: []string{"--context", "3"}, + expectedEnvKey: "HELM_DIFF_COLOR", + expectedEnvVal: "false", + }, + { + name: "no color flags", + inputFlags: []string{"--context", "3", "--detailed-exitcode"}, + expectedFlags: []string{"--context", "3", "--detailed-exitcode"}, + expectedEnvKey: "", + expectedEnvVal: "", + }, + { + name: "color flag with other flags", + inputFlags: []string{"--detailed-exitcode", "--color", "--suppress", "secret"}, + expectedFlags: []string{"--detailed-exitcode", "--suppress", "secret"}, + expectedEnvKey: "HELM_DIFF_COLOR", + expectedEnvVal: "true", + }, + { + name: "both color and no-color flags (first wins with defensive check)", + inputFlags: []string{"--color", "--no-color", "--context", "3"}, + expectedFlags: []string{"--context", "3"}, + expectedEnvKey: "HELM_DIFF_COLOR", + expectedEnvVal: "true", // Changed: first flag wins due to defensive check + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + env := make(map[string]string) + actualFlags := helm.filterColorFlagsForHelm4(tt.inputFlags, env) + + if !reflect.DeepEqual(actualFlags, tt.expectedFlags) { + t.Errorf("filterColorFlagsForHelm4() flags\nactual = %v\nexpect = %v", actualFlags, tt.expectedFlags) + } + + if tt.expectedEnvKey != "" { + if env[tt.expectedEnvKey] != tt.expectedEnvVal { + t.Errorf("filterColorFlagsForHelm4() env[%v]\nactual = %v\nexpect = %v", + tt.expectedEnvKey, env[tt.expectedEnvKey], tt.expectedEnvVal) + } + } else if len(env) > 0 { + t.Errorf("filterColorFlagsForHelm4() expected no env vars, but got: %v", env) + } + }) + } +} + func Test_DeleteRelease(t *testing.T) { var buffer bytes.Buffer logger := NewLogger(&buffer, "debug") diff --git a/pkg/state/create.go b/pkg/state/create.go index cd30d731..7f99253f 100644 --- a/pkg/state/create.go +++ b/pkg/state/create.go @@ -217,6 +217,60 @@ func (c *StateCreator) ParseAndLoad(content []byte, baseDir, file string, envNam return state, nil } +// mergeEnvironments deeply merges environment specifications from src into dst. +// Unlike mergo.WithOverride which replaces entire EnvironmentSpec values, this function +// properly merges the Values slices from both environments. +func mergeEnvironments(dst, src map[string]EnvironmentSpec) { + // If dst is nil, there's nothing to merge into + if dst == nil { + return + } + + for envName, srcEnv := range src { + if dstEnv, exists := dst[envName]; exists { + // Environment exists in both - merge the Values slices + mergedValues := append([]any{}, dstEnv.Values...) + mergedValues = append(mergedValues, srcEnv.Values...) + + // Merge Secrets slices + mergedSecrets := append([]string{}, dstEnv.Secrets...) + mergedSecrets = append(mergedSecrets, srcEnv.Secrets...) + + // Create merged environment + merged := EnvironmentSpec{ + Values: mergedValues, + Secrets: mergedSecrets, + } + + // Override KubeContext if src has it + if srcEnv.KubeContext != "" { + merged.KubeContext = srcEnv.KubeContext + } else { + merged.KubeContext = dstEnv.KubeContext + } + + // Override MissingFileHandler if src has it + if srcEnv.MissingFileHandler != nil { + merged.MissingFileHandler = srcEnv.MissingFileHandler + } else { + merged.MissingFileHandler = dstEnv.MissingFileHandler + } + + // Override MissingFileHandlerConfig if src has it + if srcEnv.MissingFileHandlerConfig != nil { + merged.MissingFileHandlerConfig = srcEnv.MissingFileHandlerConfig + } else { + merged.MissingFileHandlerConfig = dstEnv.MissingFileHandlerConfig + } + + dst[envName] = merged + } else { + // Environment only exists in src - just copy it + dst[envName] = srcEnv + } + } +} + func (c *StateCreator) loadBases(envValues, overrodeEnv *environment.Environment, st *HelmState, baseDir string) (*HelmState, error) { var newOverrodeEnv *environment.Environment if overrodeEnv != nil { @@ -234,9 +288,25 @@ func (c *StateCreator) loadBases(envValues, overrodeEnv *environment.Environment layers = append(layers, st) for i := 1; i < len(layers); i++ { + // Initialize Environments map if nil to avoid panic in mergeEnvironments + if layers[0].Environments == nil { + layers[0].Environments = make(map[string]EnvironmentSpec) + } + + // Manually merge environments to ensure deep merging of environment values + mergeEnvironments(layers[0].Environments, layers[i].Environments) + + // Clear the Environments from the source before mergo to avoid override + tmpEnvs := layers[i].Environments + layers[i].Environments = nil + + // Now merge the rest of the fields if err := mergo.Merge(layers[0], layers[i], mergo.WithAppendSlice, mergo.WithOverride); err != nil { return nil, err } + + // Restore the Environments back to the source layer (in case it's used later) + layers[i].Environments = tmpEnvs } return layers[0], nil diff --git a/pkg/state/create_test.go b/pkg/state/create_test.go index e8394458..0fff0173 100644 --- a/pkg/state/create_test.go +++ b/pkg/state/create_test.go @@ -728,3 +728,154 @@ bases: }) } } + +// TestEnvironmentMergingWithBases tests that environment values from multiple bases +// are properly merged rather than replaced. This is a regression test for issue #2273. +func TestEnvironmentMergingWithBases(t *testing.T) { + tests := []struct { + name string + files map[string]string + mainFile string + environment string + expectedError bool + checkValues func(t *testing.T, state *HelmState) + }{ + { + name: "environment values should merge from multiple bases", + files: map[string]string{ + "/path/one.yaml": `environments: + sandbox: + values: + - example: + enabled: true +`, + "/path/two.yaml": `environments: + sandbox: {} +`, + "/path/helmfile.yaml": `bases: +- one.yaml +- two.yaml +--- +repositories: + - name: examples + url: https://helm.github.io/examples +releases: + - name: example + chart: examples/hello-world +`, + }, + mainFile: "/path/helmfile.yaml", + environment: "sandbox", + checkValues: func(t *testing.T, state *HelmState) { + // Check that the environment has the values from the first base + envSpec, ok := state.Environments["sandbox"] + require.True(t, ok, "sandbox environment should exist") + require.NotNil(t, envSpec.Values, "environment values should not be nil") + require.Greater(t, len(envSpec.Values), 0, "environment should have values from first base") + + // Check that RenderedValues has the example.enabled value + require.NotNil(t, state.RenderedValues, "rendered values should not be nil") + exampleVal, ok := state.RenderedValues["example"] + require.True(t, ok, "example key should exist in rendered values") + exampleMap, ok := exampleVal.(map[string]any) + require.True(t, ok, "example should be a map") + enabled, ok := exampleMap["enabled"] + require.True(t, ok, "enabled key should exist") + require.Equal(t, true, enabled, "enabled should be true") + }, + }, + { + name: "environment values should merge when second base adds values", + files: map[string]string{ + "/path/one.yaml": `environments: + sandbox: + values: + - example: + enabled: true +`, + "/path/two.yaml": `environments: + sandbox: + values: + - another: + setting: value +`, + "/path/helmfile.yaml": `bases: +- one.yaml +- two.yaml +--- +repositories: + - name: examples + url: https://helm.github.io/examples +releases: + - name: example + chart: examples/hello-world +`, + }, + mainFile: "/path/helmfile.yaml", + environment: "sandbox", + checkValues: func(t *testing.T, state *HelmState) { + // Check that both values from both bases are present + require.NotNil(t, state.RenderedValues, "rendered values should not be nil") + + exampleVal, ok := state.RenderedValues["example"] + require.True(t, ok, "example key should exist in rendered values") + exampleMap, ok := exampleVal.(map[string]any) + require.True(t, ok, "example should be a map") + enabled, ok := exampleMap["enabled"] + require.True(t, ok, "enabled key should exist") + require.Equal(t, true, enabled, "enabled should be true") + + anotherVal, ok := state.RenderedValues["another"] + require.True(t, ok, "another key should exist in rendered values") + anotherMap, ok := anotherVal.(map[string]any) + require.True(t, ok, "another should be a map") + setting, ok := anotherMap["setting"] + require.True(t, ok, "setting key should exist") + require.Equal(t, "value", setting, "setting should be 'value'") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + creator := &StateCreator{ + logger: logger, + fs: &filesystem.FileSystem{ + ReadFile: func(filename string) ([]byte, error) { + content, ok := tt.files[filename] + if !ok { + return nil, fmt.Errorf("file not found: %s", filename) + } + return []byte(content), nil + }, + }, + valsRuntime: valsRuntime, + Strict: true, + } + creator.LoadFile = func(inheritedEnv, overrodeEnv *environment.Environment, baseDir, file string, evaluateBases bool) (*HelmState, error) { + path := filepath.Join(baseDir, file) + content, ok := tt.files[path] + if !ok { + return nil, fmt.Errorf("file not found: %s", path) + } + return creator.ParseAndLoad([]byte(content), filepath.Dir(path), path, tt.environment, true, evaluateBases, inheritedEnv, overrodeEnv) + } + + yamlContent, ok := tt.files[tt.mainFile] + if !ok { + t.Fatalf("no file named %q registered", tt.mainFile) + } + + state, err := creator.ParseAndLoad([]byte(yamlContent), filepath.Dir(tt.mainFile), tt.mainFile, tt.environment, true, true, nil, nil) + if tt.expectedError { + require.Error(t, err, "expected an error but got none") + return + } + require.NoError(t, err, "unexpected error: %v", err) + + if tt.checkValues != nil { + tt.checkValues(t, state) + } + }) + } +} diff --git a/pkg/state/state.go b/pkg/state/state.go index 89d627f3..c1941350 100644 --- a/pkg/state/state.go +++ b/pkg/state/state.go @@ -201,6 +201,11 @@ type HelmSpec struct { Cascade *string `yaml:"cascade,omitempty"` // SuppressOutputLineRegex is a list of regexes to suppress output lines SuppressOutputLineRegex []string `yaml:"suppressOutputLineRegex,omitempty"` + // DisableAutoDetectedKubeVersionForDiff controls whether auto-detected kubeVersion should be passed + // to helm diff. When false (default), auto-detected kubeVersion is passed to fix issue #2275. + // Set to true to only pass explicit kubeVersion from helmfile.yaml, preventing helm-diff from + // normalizing server-side defaults which could hide real changes (e.g., ipFamilyPolicy, ipFamilies). + DisableAutoDetectedKubeVersionForDiff *bool `yaml:"disableAutoDetectedKubeVersionForDiff,omitempty"` DisableValidation *bool `yaml:"disableValidation,omitempty"` DisableOpenAPIValidation *bool `yaml:"disableOpenAPIValidation,omitempty"` @@ -414,6 +419,9 @@ type ReleaseSpec struct { // SuppressOutputLineRegex is a list of regexes to suppress output lines SuppressOutputLineRegex []string `yaml:"suppressOutputLineRegex,omitempty"` + // DisableAutoDetectedKubeVersionForDiff controls whether auto-detected kubeVersion should be passed + // to helm diff for this release. See HelmSpec.DisableAutoDetectedKubeVersionForDiff for details. + DisableAutoDetectedKubeVersionForDiff *bool `yaml:"disableAutoDetectedKubeVersionForDiff,omitempty"` // Inherit is used to inherit a release template from a release or another release template Inherit Inherits `yaml:"inherit,omitempty"` @@ -1318,7 +1326,7 @@ type PrepareChartKey struct { // // If exists, it will also patch resources by json patches, strategic-merge patches, and injectors. // processChartification handles the chartification process -func (st *HelmState) processChartification(chartification *Chartify, release *ReleaseSpec, chartPath string, opts ChartPrepareOptions, skipDeps bool) (string, bool, error) { +func (st *HelmState) processChartification(chartification *Chartify, release *ReleaseSpec, chartPath string, opts ChartPrepareOptions, skipDeps bool, helmfileCommand string) (string, bool, error) { c := chartify.New( chartify.HelmBin(st.DefaultHelmBinary), chartify.KustomizeBin(st.DefaultKustomizeBinary), @@ -1360,6 +1368,36 @@ func (st *HelmState) processChartification(chartification *Chartify, release *Re } chartifyOpts.SetFlags = append(chartifyOpts.SetFlags, flags...) + // Enable cluster connectivity for lookup functions when using kustomize patches + // Issue #2271: helm template runs client-side by default, causing lookup() to return empty values + // Pass --dry-run=server to enable cluster access for lookup while still using client-side rendering + // Only do this for operations that already require cluster access + var requiresCluster bool + switch helmfileCommand { + case "diff", "apply", "sync", "destroy", "delete", "test", "status": + // Commands that interact with the cluster + requiresCluster = true + case "template", "lint", "build", "pull", "fetch", "write-values", "list", "show-dag", "deps", "repos", "cache", "init", "completion", "help", "version": + // Commands that work locally without cluster access + requiresCluster = false + default: + // For unknown commands, assume cluster access (safer default) + requiresCluster = true + } + + // Enable --dry-run=server for cluster-requiring commands to support lookup() function + // Issue #2271: helm template runs client-side by default, causing lookup() to return empty values + // The lookup() function can be used with or without patches, so we enable cluster access + // for all cluster-requiring operations (diff, apply, sync, etc.) but not for offline + // commands (template, lint, build, etc.) + if requiresCluster { + if chartifyOpts.TemplateArgs == "" { + chartifyOpts.TemplateArgs = "--dry-run=server" + } else if !strings.Contains(chartifyOpts.TemplateArgs, "--dry-run") { + chartifyOpts.TemplateArgs += " --dry-run=server" + } + } + out, err := c.Chartify(release.Name, chartPath, chartify.WithChartifyOpts(chartifyOpts)) if err != nil { return "", false, err @@ -1482,7 +1520,7 @@ func (st *HelmState) prepareChartForRelease(release *ReleaseSpec, helm helmexec. skipDeps := (!isLocal && !chartFetchedByGoGetter) || skipDepsGlobal || skipDepsRelease || skipDepsDefault if chartification != nil && helmfileCommand != "pull" { - chartPath, buildDeps, err = st.processChartification(chartification, release, chartPath, opts, skipDeps) + chartPath, buildDeps, err = st.processChartification(chartification, release, chartPath, opts, skipDeps, helmfileCommand) if err != nil { return &chartPrepareResult{err: err} } @@ -2207,6 +2245,9 @@ type DiffOpts struct { SuppressOutputLineRegex []string SkipSchemaValidation bool TakeOwnership bool + // DetectedKubeVersion is the Kubernetes version detected from the cluster. + // This is used when kubeVersion is not specified in helmfile.yaml + DetectedKubeVersion string } func (o *DiffOpts) Apply(opts *DiffOpts) { @@ -3109,11 +3150,38 @@ func (st *HelmState) flagsForDiff(helm helmexec.Interface, release *ReleaseSpec, flags = append(flags, "--disable-validation") } - // TODO: - // `helm diff` has `--kube-version` flag from v3.5.0, but only respected when `helm diff upgrade --disable-validation`. - // `helm template --validate` and `helm upgrade --dry-run` ignore `--kube-version` flag. - // For the moment, not specifying kubeVersion. - flags = st.appendApiVersionsFlags(flags, release, "") + // Determine which kubeVersion to pass to helm diff based on configuration. + // By default (disableAutoDetectedKubeVersionForDiff=false), pass auto-detected kubeVersion + // to helm-diff. This fixes issue #2275 where charts requiring newer Kubernetes versions + // would fail because helm-diff defaults to v1.20.0. + // + // If disableAutoDetectedKubeVersionForDiff=true, only pass explicit kubeVersion from + // helmfile.yaml. This prevents helm-diff from normalizing server-side defaults which + // could hide real changes (e.g., ipFamilyPolicy, ipFamilies). Use this when server-side + // normalization causes issues with diff output. + disableAutoDetected := false + if release.DisableAutoDetectedKubeVersionForDiff != nil { + disableAutoDetected = *release.DisableAutoDetectedKubeVersionForDiff + } else if st.HelmDefaults.DisableAutoDetectedKubeVersionForDiff != nil { + disableAutoDetected = *st.HelmDefaults.DisableAutoDetectedKubeVersionForDiff + } + + kubeVersionForDiff := "" + if disableAutoDetected { + // Only pass explicit kubeVersion from helmfile.yaml + if release.KubeVersion != "" { + kubeVersionForDiff = release.KubeVersion + } else if st.KubeVersion != "" { + kubeVersionForDiff = st.KubeVersion + } + } else { + // Pass auto-detected version (default behavior) + kubeVersionForDiff = "" + if opt != nil && opt.DetectedKubeVersion != "" { + kubeVersionForDiff = opt.DetectedKubeVersion + } + } + flags = st.appendApiVersionsFlags(flags, release, kubeVersionForDiff) flags = st.appendConnectionFlags(flags, release) flags = st.appendChartDownloadFlags(flags, release) diff --git a/pkg/state/state_kubeversion_test.go b/pkg/state/state_kubeversion_test.go new file mode 100644 index 00000000..f338ee20 --- /dev/null +++ b/pkg/state/state_kubeversion_test.go @@ -0,0 +1,75 @@ +package state + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// TestAppendApiVersionsFlags_KubeVersion tests that kubeVersion is properly +// passed to helm diff. This is a regression test for issue #2275. +// Priority: 1) paramKubeVersion (auto-detected), 2) release.KubeVersion, 3) state.KubeVersion (helmfile.yaml) +func TestAppendApiVersionsFlags_KubeVersion(t *testing.T) { + tests := []struct { + name string + stateKubeVersion string // kubeVersion from HelmState (helmfile.yaml) + paramKubeVersion string // kubeVersion parameter passed to appendApiVersionsFlags + expectedVersion string // which version should be in the flags + }{ + { + name: "state kubeVersion should be used when param is empty", + stateKubeVersion: "1.34.0", + paramKubeVersion: "", + expectedVersion: "1.34.0", + }, + { + name: "param kubeVersion takes precedence over state", + stateKubeVersion: "1.34.0", + paramKubeVersion: "1.30.0", + expectedVersion: "1.30.0", + }, + { + name: "no version when both are empty", + stateKubeVersion: "", + paramKubeVersion: "", + expectedVersion: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + state := &HelmState{ + ReleaseSetSpec: ReleaseSetSpec{ + KubeVersion: tt.stateKubeVersion, + }, + } + + release := &ReleaseSpec{ + Name: "test-release", + Chart: "test/chart", + } + + result := state.appendApiVersionsFlags([]string{}, release, tt.paramKubeVersion) + + if tt.expectedVersion != "" { + // Should have --kube-version flag + foundKubeVersion := false + for i := 0; i < len(result)-1; i++ { + if result[i] == "--kube-version" { + require.Equal(t, tt.expectedVersion, result[i+1], + "kube-version value should match expected") + foundKubeVersion = true + break + } + } + require.True(t, foundKubeVersion, "Should have --kube-version flag in result") + } else { + // Should NOT have --kube-version flag + for i := 0; i < len(result); i++ { + require.NotEqual(t, "--kube-version", result[i], + "Should not have --kube-version flag when nothing is set") + } + } + }) + } +} diff --git a/pkg/state/temp_test.go b/pkg/state/temp_test.go index 7e082d51..811fd084 100644 --- a/pkg/state/temp_test.go +++ b/pkg/state/temp_test.go @@ -38,39 +38,39 @@ func TestGenerateID(t *testing.T) { run(testcase{ subject: "baseline", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"}, - want: "foo-values-67dc97cbcb", + want: "foo-values-66f7fd6f7b", }) run(testcase{ subject: "different bytes content", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"}, data: []byte(`{"k":"v"}`), - want: "foo-values-75d7c4758c", + want: "foo-values-6664979cd7", }) run(testcase{ subject: "different map content", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw"}, data: map[string]any{"k": "v"}, - want: "foo-values-685f8cf685", + want: "foo-values-78897dfd49", }) run(testcase{ subject: "different chart", release: ReleaseSpec{Name: "foo", Chart: "stable/envoy"}, - want: "foo-values-75597d9c57", + want: "foo-values-64b7846cb7", }) run(testcase{ subject: "different name", release: ReleaseSpec{Name: "bar", Chart: "incubator/raw"}, - want: "bar-values-7b77df65ff", + want: "bar-values-576cb7ddc7", }) run(testcase{ subject: "specific ns", release: ReleaseSpec{Name: "foo", Chart: "incubator/raw", Namespace: "myns"}, - want: "myns-foo-values-85f979545c", + want: "myns-foo-values-6c567f54c", }) for id, n := range ids { diff --git a/test/e2e/template/helmfile/snapshot_test.go b/test/e2e/template/helmfile/snapshot_test.go index 3bbdec26..eeb58341 100644 --- a/test/e2e/template/helmfile/snapshot_test.go +++ b/test/e2e/template/helmfile/snapshot_test.go @@ -5,6 +5,8 @@ import ( "context" "fmt" "io" + "net" + "net/http" "os" "os/exec" "path/filepath" @@ -18,9 +20,7 @@ import ( "github.com/helmfile/chartify/helmtesting" "github.com/stretchr/testify/require" - "github.com/helmfile/helmfile/pkg/app" "github.com/helmfile/helmfile/pkg/envvar" - "github.com/helmfile/helmfile/pkg/helmexec" "github.com/helmfile/helmfile/pkg/yaml" ) @@ -46,10 +46,160 @@ type Config struct { HelmfileArgs []string `yaml:"helmfileArgs"` } -type fakeInit struct{} +// getFreePort asks the kernel for a free open port that is ready to use. +// This has a small race condition between the time we get the port and when we use it, +// but it's the standard approach for dynamic port allocation in tests. +// Callers should implement retry logic to handle this race condition - see setupLocalDockerRegistry(). +func getFreePort() (int, error) { + addr, err := net.ResolveTCPAddr("tcp", "localhost:0") + if err != nil { + return 0, fmt.Errorf("failed to resolve TCP address: %w", err) + } -func (f fakeInit) Force() bool { - return true + l, err := net.ListenTCP("tcp", addr) + if err != nil { + return 0, fmt.Errorf("failed to listen on TCP port: %w", err) + } + defer l.Close() + + return l.Addr().(*net.TCPAddr).Port, nil +} + +// waitForRegistry polls the Docker registry health endpoint until it's ready +// or the timeout is reached. Docker Registry v2 exposes /v2/ which returns +// 200 OK when the registry is healthy and ready to accept requests. +func waitForRegistry(t *testing.T, port int, timeout time.Duration) error { + t.Helper() + + endpoint := fmt.Sprintf("http://localhost:%d/v2/", port) + client := &http.Client{Timeout: 2 * time.Second} + deadline := time.Now().Add(timeout) + + for time.Now().Before(deadline) { + resp, err := client.Get(endpoint) + if err == nil { + resp.Body.Close() + if resp.StatusCode == http.StatusOK { + t.Logf("Registry at port %d is ready", port) + return nil + } + } + time.Sleep(500 * time.Millisecond) + } + + return fmt.Errorf("registry at port %d did not become ready within %v", port, timeout) +} + +// prepareInputFile substitutes $REGISTRY_PORT placeholder in the input file +// with the actual allocated port for Docker registry tests. It also converts +// relative chart paths to absolute paths since the input file is copied to a +// temp directory. +func prepareInputFile(t *testing.T, originalFile, tmpDir string, hostPort int, chartsDir, postrenderersDir string) string { + t.Helper() + + inputContent, err := os.ReadFile(originalFile) + require.NoError(t, err, "Failed to read input file") + + // Replace $REGISTRY_PORT placeholder with actual port + inputStr := string(inputContent) + inputStr = strings.ReplaceAll(inputStr, "$REGISTRY_PORT", fmt.Sprintf("%d", hostPort)) + + // Convert relative chart paths to absolute paths + // This is necessary because the input file is copied to a temp directory, + // breaking relative paths like ../../charts/raw-0.1.0 + inputStr = strings.ReplaceAll(inputStr, "../../charts/", chartsDir+"/") + + // Convert relative postrenderer paths to absolute paths for Helm 3 only + // Helm 3 resolves postrenderer paths relative to the helmfile location. + // When the input file is copied to a temp directory, relative paths break. + // Helm 4 extracts the plugin name from the path, so it works with relative paths. + if !isHelm4(t) && postrenderersDir != "" { + inputStr = strings.ReplaceAll(inputStr, "../../postrenderers/", postrenderersDir+"/") + } + + // Write to temporary file with restrictive permissions (owner read/write only) + tmpInputFile := filepath.Join(tmpDir, "input.yaml.gotmpl") + err = os.WriteFile(tmpInputFile, []byte(inputStr), 0600) + require.NoError(t, err, "Failed to write temporary input file") + + return tmpInputFile +} + +// setupLocalDockerRegistry sets up a local Docker registry for OCI chart testing. +// It dynamically allocates a port if not configured, starts the registry container, +// and pushes test charts to it. Returns the allocated port. +func setupLocalDockerRegistry(t *testing.T, config Config, name, defaultChartsDir string) int { + t.Helper() + + containerName := strings.Join([]string{"helmfile_docker_registry", name}, "_") + + hostPort := config.LocalDockerRegistry.Port + if hostPort <= 0 { + // Dynamically allocate an unused port to avoid conflicts + // Retry up to 3 times in case of race condition where port gets taken + // between getFreePort() and docker run + const maxRetries = 3 + var err error + for attempt := 1; attempt <= maxRetries; attempt++ { + hostPort, err = getFreePort() + require.NoError(t, err, "Failed to get free port for Docker registry") + t.Logf("Attempt %d: Allocated dynamic port %d for Docker registry in test %s", attempt, hostPort, name) + + // Try to start Docker with this port + cmd := exec.Command("docker", "run", "--rm", "-d", "-p", fmt.Sprintf("%d:5000", hostPort), "--name", containerName, "registry:2") + output, err := cmd.CombinedOutput() + if err == nil { + // Success! Docker started successfully + t.Cleanup(func() { + execDocker(t, "stop", containerName) + }) + break + } + + // Check if error is due to port conflict + if strings.Contains(string(output), "address already in use") { + if attempt < maxRetries { + t.Logf("Port %d was taken (race condition), retrying with new port...", hostPort) + continue + } + t.Fatalf("Failed to start Docker registry after %d attempts due to port conflicts", maxRetries) + } + + // Other error - fail immediately + t.Fatalf("Failed to start Docker registry: %s\nOutput: %s", err, string(output)) + } + } else { + // Use configured port + execDocker(t, "run", "--rm", "-d", "-p", fmt.Sprintf("%d:5000", hostPort), "--name", containerName, "registry:2") + t.Cleanup(func() { + execDocker(t, "stop", containerName) + }) + } + + // Wait for registry to be ready by polling its health endpoint + err := waitForRegistry(t, hostPort, 30*time.Second) + require.NoError(t, err, "Registry failed to become ready") + + // We helm-package and helm-push every test chart saved in the ./testdata/charts directory + // to the local registry, so that they can be accessed by helmfile and helm invoked while testing. + chartDir := config.LocalDockerRegistry.ChartDir + if chartDir == "" { + chartDir = defaultChartsDir + } + charts, err := os.ReadDir(chartDir) + require.NoError(t, err) + + for _, c := range charts { + chartPath := filepath.Join(chartDir, c.Name()) + if !c.IsDir() { + t.Fatalf("%s is not a directory", c) + } + tgzFile := execHelmPackage(t, chartPath) + _, err := execHelmPush(t, tgzFile, fmt.Sprintf("oci://localhost:%d/myrepo", hostPort)) + require.NoError(t, err, "Unable to run helm push to local registry: %v", err) + } + + return hostPort } func TestHelmfileTemplateWithBuildCommand(t *testing.T) { @@ -67,17 +217,6 @@ func testHelmfileTemplateWithBuildCommand(t *testing.T, GoYamlV3 bool) { localChartPortSets := make(map[int]struct{}) - logger := helmexec.NewLogger(os.Stderr, "info") - runner := &helmexec.ShellRunner{ - Logger: logger, - Ctx: context.TODO(), - } - - c := fakeInit{} - helmfileInit := app.NewHelmfileInit("helm", c, logger, runner) - err := helmfileInit.CheckHelmPlugins() - require.NoError(t, err) - _, filename, _, _ := goruntime.Caller(0) projectRoot := filepath.Join(filepath.Dir(filename), "..", "..", "..", "..") helmfileBin := filepath.Join(projectRoot, "helmfile") @@ -164,40 +303,9 @@ func testHelmfileTemplateWithBuildCommand(t *testing.T, GoYamlV3 bool) { // If localDockerRegistry.enabled is set to `true`, // run the docker registry v2 and push the test charts to the registry // so that it can be accessed by helm and helmfile as a oci registry based chart repository. + var hostPort int if config.LocalDockerRegistry.Enabled { - containerName := strings.Join([]string{"helmfile_docker_registry", name}, "_") - - hostPort := config.LocalDockerRegistry.Port - if hostPort <= 0 { - hostPort = 5000 - } - - execDocker(t, "run", "--rm", "-d", "-p", fmt.Sprintf("%d:5000", hostPort), "--name", containerName, "registry:2") - t.Cleanup(func() { - execDocker(t, "stop", containerName) - }) - - // FIXME: this is a hack to wait for registry to be up and running - // please replace with proper wait for registry - time.Sleep(5 * time.Second) - - // We helm-package and helm-push every test chart saved in the ./testdata/charts directory - // to the local registry, so that they can be accessed by helmfile and helm invoked while testing. - if config.LocalDockerRegistry.ChartDir == "" { - config.LocalDockerRegistry.ChartDir = defaultChartsDir - } - charts, err := os.ReadDir(config.LocalDockerRegistry.ChartDir) - require.NoError(t, err) - - for _, c := range charts { - chartPath := filepath.Join(config.LocalDockerRegistry.ChartDir, c.Name()) - if !c.IsDir() { - t.Fatalf("%s is not a directory", c) - } - tgzFile := execHelmPackage(t, chartPath) - _, err := execHelmPush(t, tgzFile, fmt.Sprintf("oci://localhost:%d/myrepo", hostPort)) - require.NoError(t, err, "Unable to run helm push to local registry: %v", err) - } + hostPort = setupLocalDockerRegistry(t, config, name, defaultChartsDir) } tmpDir := t.TempDir() @@ -230,6 +338,14 @@ func testHelmfileTemplateWithBuildCommand(t *testing.T, GoYamlV3 bool) { } inputFile := filepath.Join(testdataDir, name, "input.yaml.gotmpl") + + // If using dynamic Docker registry port, substitute $REGISTRY_PORT in input file + if config.LocalDockerRegistry.Enabled { + chartsDir := filepath.Join(wd, defaultChartsDir) + postrenderersDir := filepath.Join(wd, "testdata/postrenderers") + inputFile = prepareInputFile(t, inputFile, tmpDir, hostPort, chartsDir, postrenderersDir) + } + outputFile := "" if GoYamlV3 { outputFile = filepath.Join(testdataDir, name, "gopkg.in-yaml.v3-output.yaml") @@ -280,6 +396,10 @@ func testHelmfileTemplateWithBuildCommand(t *testing.T, GoYamlV3 bool) { gotStr = helmShortVersionRegex.ReplaceAllString(gotStr, `$$HelmVersion`) if config.LocalDockerRegistry.Enabled { + // Normalize the dynamic port to $REGISTRY_PORT placeholder for test comparison + gotStr = strings.ReplaceAll(gotStr, fmt.Sprintf("localhost:%d", hostPort), "localhost:$REGISTRY_PORT") + gotStr = strings.ReplaceAll(gotStr, fmt.Sprintf("oci__localhost_%d", hostPort), "oci__localhost_$REGISTRY_PORT") + sc := bufio.NewScanner(strings.NewReader(gotStr)) for sc.Scan() { if !strings.HasPrefix(sc.Text(), "Templating ") { diff --git a/test/e2e/template/helmfile/testdata/snapshot/issue_473_oci_chart_url_fetch/config.yaml b/test/e2e/template/helmfile/testdata/snapshot/issue_473_oci_chart_url_fetch/config.yaml index 20aefc41..470200fe 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/issue_473_oci_chart_url_fetch/config.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/issue_473_oci_chart_url_fetch/config.yaml @@ -1,6 +1,6 @@ localDockerRegistry: enabled: true - port: 5000 + # Port is not specified, will be dynamically allocated to avoid conflicts chartifyTempDir: temp2 helmfileArgs: - fetch diff --git a/test/e2e/template/helmfile/testdata/snapshot/issue_473_oci_chart_url_fetch/input.yaml.gotmpl b/test/e2e/template/helmfile/testdata/snapshot/issue_473_oci_chart_url_fetch/input.yaml.gotmpl index c534fb4f..d87ee3cb 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/issue_473_oci_chart_url_fetch/input.yaml.gotmpl +++ b/test/e2e/template/helmfile/testdata/snapshot/issue_473_oci_chart_url_fetch/input.yaml.gotmpl @@ -1,6 +1,6 @@ releases: - name: foo - chart: oci://localhost:5000/myrepo/raw + chart: oci://localhost:$REGISTRY_PORT/myrepo/raw version: 0.1.0 values: - templates: diff --git a/test/e2e/template/helmfile/testdata/snapshot/issue_473_oci_chart_url_fetch/output.yaml b/test/e2e/template/helmfile/testdata/snapshot/issue_473_oci_chart_url_fetch/output.yaml index d1d56924..3cf0c358 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/issue_473_oci_chart_url_fetch/output.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/issue_473_oci_chart_url_fetch/output.yaml @@ -1 +1 @@ -Pulling localhost:5000/myrepo/raw:0.1.0 +Pulling localhost:$REGISTRY_PORT/myrepo/raw:0.1.0 diff --git a/test/e2e/template/helmfile/testdata/snapshot/issue_493_template_yaml_anchors_merge/output.yaml b/test/e2e/template/helmfile/testdata/snapshot/issue_493_template_yaml_anchors_merge/output.yaml index 9c7b08f7..7bf5bb6e 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/issue_493_template_yaml_anchors_merge/output.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/issue_493_template_yaml_anchors_merge/output.yaml @@ -1,3 +1,3 @@ NAME NAMESPACE ENABLED INSTALLED LABELS CHART VERSION -release1 myNamespace true true app:myapp,chart:test,group:myGroup,name:release1,namespace:myNamespace,project:myProject test -release2 myNamespace true true app:myapp,chart:test,group:myGroup,name:release2,namespace:myNamespace,project:myProject test +release1 myNamespace true true app:myapp,chart:test,group:myGroup,name:release1,namespace:myNamespace,project:myProject test +release2 myNamespace true true app:myapp,chart:test,group:myGroup,name:release2,namespace:myNamespace,project:myProject test diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull/config.yaml b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull/config.yaml index c9145492..49e44448 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull/config.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull/config.yaml @@ -1,6 +1,6 @@ localDockerRegistry: enabled: true - port: 5001 + # Port is not specified, will be dynamically allocated to avoid conflicts chartifyTempDir: temp2 helmfileArgs: - template diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull/input.yaml.gotmpl b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull/input.yaml.gotmpl index 194400a6..0536275b 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull/input.yaml.gotmpl +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull/input.yaml.gotmpl @@ -1,6 +1,6 @@ repositories: - name: myrepo - url: localhost:5001/myrepo + url: localhost:$REGISTRY_PORT/myrepo oci: true releases: diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull/output.yaml b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull/output.yaml index 197bd4c4..90d91214 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull/output.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull/output.yaml @@ -1,4 +1,4 @@ -Pulling localhost:5001/myrepo/raw:0.1.0 +Pulling localhost:$REGISTRY_PORT/myrepo/raw:0.1.0 Templating release=foo, chart=$HELMFILE_CACHE_HOME/myrepo/raw/0.1.0/raw --- # Source: raw/templates/resources.yaml diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_direct/config.yaml b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_direct/config.yaml index c9145492..49e44448 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_direct/config.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_direct/config.yaml @@ -1,6 +1,6 @@ localDockerRegistry: enabled: true - port: 5001 + # Port is not specified, will be dynamically allocated to avoid conflicts chartifyTempDir: temp2 helmfileArgs: - template diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_direct/input.yaml.gotmpl b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_direct/input.yaml.gotmpl index f3e1f31f..8a919b53 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_direct/input.yaml.gotmpl +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_direct/input.yaml.gotmpl @@ -1,6 +1,6 @@ releases: - name: foo - chart: oci://localhost:5001/myrepo/raw + chart: oci://localhost:$REGISTRY_PORT/myrepo/raw version: 0.1.0 values: &oci_chart_pull_direct - templates: @@ -16,7 +16,7 @@ releases: values: {{`{{ .Release.Name }}`}} - name: bar - chart: oci://localhost:5001/myrepo/raw + chart: oci://localhost:$REGISTRY_PORT/myrepo/raw version: 0.1.0 namespace: ns2 values: *oci_chart_pull_direct diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_direct/output.yaml b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_direct/output.yaml index 313a9209..213d8876 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_direct/output.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_direct/output.yaml @@ -1,5 +1,5 @@ -Pulling localhost:5001/myrepo/raw:0.1.0 -Templating release=foo, chart=$HELMFILE_CACHE_HOME/oci__localhost_5001/myrepo/raw/0.1.0/raw +Pulling localhost:$REGISTRY_PORT/myrepo/raw:0.1.0 +Templating release=foo, chart=$HELMFILE_CACHE_HOME/oci__localhost_$REGISTRY_PORT/myrepo/raw/0.1.0/raw --- # Source: raw/templates/resources.yaml apiVersion: v1 @@ -12,7 +12,7 @@ metadata: data: values: foo -Templating release=bar, chart=$HELMFILE_CACHE_HOME/oci__localhost_5001/myrepo/raw/0.1.0/raw +Templating release=bar, chart=$HELMFILE_CACHE_HOME/oci__localhost_$REGISTRY_PORT/myrepo/raw/0.1.0/raw --- # Source: raw/templates/resources.yaml apiVersion: v1 diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once/config.yaml b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once/config.yaml index f0bb6728..0c7b7780 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once/config.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once/config.yaml @@ -1,7 +1,7 @@ # Templating two versions of the same chart with only one pulling of each version localDockerRegistry: enabled: true - port: 5001 + # Port is not specified, will be dynamically allocated to avoid conflicts chartifyTempDir: temp3 helmfileArgs: # Prevent releases from racing and randomizing the log diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once/input.yaml.gotmpl b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once/input.yaml.gotmpl index 394cc82d..f88336df 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once/input.yaml.gotmpl +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once/input.yaml.gotmpl @@ -1,6 +1,6 @@ repositories: - name: myrepo - url: localhost:5001/myrepo + url: localhost:$REGISTRY_PORT/myrepo oci: true releases: diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once/output.yaml b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once/output.yaml index b865783e..3a41ca8e 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once/output.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once/output.yaml @@ -1,5 +1,5 @@ -Pulling localhost:5001/myrepo/raw:0.1.0 -Pulling localhost:5001/myrepo/raw:0.0.1 +Pulling localhost:$REGISTRY_PORT/myrepo/raw:0.1.0 +Pulling localhost:$REGISTRY_PORT/myrepo/raw:0.0.1 Templating release=foo, chart=$HELMFILE_CACHE_HOME/myrepo/raw/0.1.0/raw --- # Source: raw/templates/resources.yaml diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once2/config.yaml b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once2/config.yaml index bd0820e0..9c0ad782 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once2/config.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once2/config.yaml @@ -1,7 +1,7 @@ # Templating few releases with the same chart\version and only one pulling localDockerRegistry: enabled: true - port: 5001 + # Port is not specified, will be dynamically allocated to avoid conflicts chartifyTempDir: temp3 helmfileArgs: - template diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once2/input.yaml.gotmpl b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once2/input.yaml.gotmpl index 9aea4b81..84965bd1 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once2/input.yaml.gotmpl +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once2/input.yaml.gotmpl @@ -1,6 +1,6 @@ repositories: - name: myrepo - url: localhost:5001/myrepo + url: localhost:$REGISTRY_PORT/myrepo oci: true releases: diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once2/output.yaml b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once2/output.yaml index b94a5cef..6d277d59 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once2/output.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_chart_pull_once2/output.yaml @@ -1,4 +1,4 @@ -Pulling localhost:5001/myrepo/raw:0.1.0 +Pulling localhost:$REGISTRY_PORT/myrepo/raw:0.1.0 Templating release=release-0, chart=$HELMFILE_CACHE_HOME/myrepo/raw/0.1.0/raw --- # Source: raw/templates/resources.yaml diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_need/config.yaml b/test/e2e/template/helmfile/testdata/snapshot/oci_need/config.yaml index c3a26f4a..33f724b3 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_need/config.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_need/config.yaml @@ -1,6 +1,6 @@ localDockerRegistry: enabled: true - port: 5002 + # Port is not specified, will be dynamically allocated to avoid conflicts chartifyTempDir: temp1 helmfileArgs: - template diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_need/input.yaml.gotmpl b/test/e2e/template/helmfile/testdata/snapshot/oci_need/input.yaml.gotmpl index 1e63d06b..b13cdd41 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_need/input.yaml.gotmpl +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_need/input.yaml.gotmpl @@ -23,5 +23,5 @@ releases: bar: BAR dependencies: - alias: dep - chart: oci://localhost:5002/myrepo/raw + chart: oci://localhost:$REGISTRY_PORT/myrepo/raw version: 0.1.0 diff --git a/test/e2e/template/helmfile/testdata/snapshot/oci_need/output.yaml b/test/e2e/template/helmfile/testdata/snapshot/oci_need/output.yaml index baf64c9d..3c5796df 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/oci_need/output.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/oci_need/output.yaml @@ -1,6 +1,6 @@ Building dependency release=foo, chart=$WD/temp1/foo Saving 1 charts -Downloading raw from repo oci://localhost:5002/myrepo +Downloading raw from repo oci://localhost:$REGISTRY_PORT/myrepo Deleting outdated charts Templating release=foo, chart=$WD/temp1/foo diff --git a/test/e2e/template/helmfile/testdata/snapshot/postrenderer/config.yaml b/test/e2e/template/helmfile/testdata/snapshot/postrenderer/config.yaml index deb6629a..33f724b3 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/postrenderer/config.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/postrenderer/config.yaml @@ -1,6 +1,6 @@ localDockerRegistry: enabled: true - port: 5001 + # Port is not specified, will be dynamically allocated to avoid conflicts chartifyTempDir: temp1 helmfileArgs: - template diff --git a/test/e2e/template/helmfile/testdata/snapshot/postrenderer/input.yaml.gotmpl b/test/e2e/template/helmfile/testdata/snapshot/postrenderer/input.yaml.gotmpl index a317ee0e..4aea39d7 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/postrenderer/input.yaml.gotmpl +++ b/test/e2e/template/helmfile/testdata/snapshot/postrenderer/input.yaml.gotmpl @@ -39,5 +39,5 @@ releases: baz: BAZ dependencies: - alias: dep - chart: oci://localhost:5001/myrepo/raw + chart: oci://localhost:$REGISTRY_PORT/myrepo/raw version: 0.1.0 diff --git a/test/e2e/template/helmfile/testdata/snapshot/postrenderer/output-helm4.yaml b/test/e2e/template/helmfile/testdata/snapshot/postrenderer/output-helm4.yaml index 92c27143..1c75e976 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/postrenderer/output-helm4.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/postrenderer/output-helm4.yaml @@ -1,10 +1,10 @@ -Building dependency release=foo, chart=../../charts/raw-0.1.0 +Building dependency release=foo, chart=$WD/testdata/charts/raw-0.1.0 Building dependency release=baz, chart=$WD/temp1/baz Saving 1 charts -Downloading raw from repo oci://localhost:5001/myrepo +Downloading raw from repo oci://localhost:$REGISTRY_PORT/myrepo Deleting outdated charts -Templating release=foo, chart=../../charts/raw-0.1.0 +Templating release=foo, chart=$WD/testdata/charts/raw-0.1.0 --- # Source: generated-by-postrender-1.yaml apiVersion: v1 diff --git a/test/e2e/template/helmfile/testdata/snapshot/postrenderer/output.yaml b/test/e2e/template/helmfile/testdata/snapshot/postrenderer/output.yaml index f71b03aa..d661ef51 100644 --- a/test/e2e/template/helmfile/testdata/snapshot/postrenderer/output.yaml +++ b/test/e2e/template/helmfile/testdata/snapshot/postrenderer/output.yaml @@ -1,10 +1,10 @@ -Building dependency release=foo, chart=../../charts/raw-0.1.0 +Building dependency release=foo, chart=$WD/testdata/charts/raw-0.1.0 Building dependency release=baz, chart=$WD/temp1/baz Saving 1 charts -Downloading raw from repo oci://localhost:5001/myrepo +Downloading raw from repo oci://localhost:$REGISTRY_PORT/myrepo Deleting outdated charts -Templating release=foo, chart=../../charts/raw-0.1.0 +Templating release=foo, chart=$WD/testdata/charts/raw-0.1.0 --- # Source: raw/templates/resources.yaml apiVersion: v1 diff --git a/test/integration/run.sh b/test/integration/run.sh index e93cd120..fb733d67 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -27,7 +27,7 @@ export HELM_DATA_HOME="${helm_dir}/data" export HELM_HOME="${HELM_DATA_HOME}" export HELM_PLUGINS="${HELM_DATA_HOME}/plugins" export HELM_CONFIG_HOME="${helm_dir}/config" -HELM_DIFF_VERSION="${HELM_DIFF_VERSION:-3.14.0}" +HELM_DIFF_VERSION="${HELM_DIFF_VERSION:-3.14.1}" HELM_GIT_VERSION="${HELM_GIT_VERSION:-1.4.1}" HELM_SECRETS_VERSION="${HELM_SECRETS_VERSION:-4.7.0}" export GNUPGHOME="${PWD}/${dir}/.gnupg" diff --git a/test/integration/test-cases/issue-2271.sh b/test/integration/test-cases/issue-2271.sh new file mode 100755 index 00000000..660af0c1 --- /dev/null +++ b/test/integration/test-cases/issue-2271.sh @@ -0,0 +1,61 @@ +#!/usr/bin/env bash + +# Test for issue #2271: lookup function should work with strategicMergePatches +# Without this fix, helm template runs client-side and lookup() returns empty values + +issue_2271_input_dir="${cases_dir}/issue-2271/input" +issue_2271_tmp_dir=$(mktemp -d) + +cd "${issue_2271_input_dir}" + +test_start "issue-2271: lookup function with strategicMergePatches" + +# Test 1: Install chart without kustomize patches +info "Installing chart without kustomize patches" +${helmfile} -f helmfile-no-kustomize.yaml apply --suppress-diff > "${issue_2271_tmp_dir}/test-2271-install.txt" 2>&1 +code=$? + +if [ $code -ne 0 ]; then + cat "${issue_2271_tmp_dir}/test-2271-install.txt" + rm -rf "${issue_2271_tmp_dir}" + fail "Failed to install chart" +fi + +info "Chart installed successfully" + +# Test 2: Modify ConfigMap value manually to simulate an upgrade scenario +info "Modifying ConfigMap value to test lookup preservation" +${kubectl} patch configmap test-release-2271-config --type merge -p '{"data":{"preserved-value":"test-preserved-value"}}' > /dev/null 2>&1 + +# Verify the value was changed +current_value=$(${kubectl} get configmap test-release-2271-config -o jsonpath='{.data.preserved-value}') +if [ "$current_value" != "test-preserved-value" ]; then + rm -rf "${issue_2271_tmp_dir}" + fail "Failed to update ConfigMap value. Got: $current_value" +fi + +info "ConfigMap value updated to: $current_value" + +# Test 3: Diff with strategicMergePatches should preserve the lookup value +info "Testing diff with strategicMergePatches - lookup should preserve value" + +${helmfile} -f helmfile.yaml diff > "${issue_2271_tmp_dir}/test-2271-diff.txt" 2>&1 +code=$? + +# Check if the diff contains the preserved value (not "initial-value") +if grep -q "preserved-value.*test-preserved-value" "${issue_2271_tmp_dir}/test-2271-diff.txt"; then + info "SUCCESS: lookup function preserved the value with kustomize patches" +elif grep -q "preserved-value.*initial-value" "${issue_2271_tmp_dir}/test-2271-diff.txt"; then + cat "${issue_2271_tmp_dir}/test-2271-diff.txt" + rm -rf "${issue_2271_tmp_dir}" + fail "Issue #2271 regression: lookup function returned empty value with kustomize" +else + # No diff for ConfigMap means value is perfectly preserved + info "SUCCESS: No ConfigMap changes detected (value perfectly preserved)" +fi + +# Cleanup +${helm} uninstall test-release-2271 --namespace default 2>/dev/null || true +rm -rf "${issue_2271_tmp_dir}" + +test_pass "issue-2271: lookup function with strategicMergePatches" diff --git a/test/integration/test-cases/issue-2271/input/dns-patch.yaml b/test/integration/test-cases/issue-2271/input/dns-patch.yaml new file mode 100644 index 00000000..2f51c26e --- /dev/null +++ b/test/integration/test-cases/issue-2271/input/dns-patch.yaml @@ -0,0 +1,14 @@ +# Kustomize strategic merge patch to set DNS ndots +# This simulates the grafana-dns-ndots.yaml.gotmpl from the issue +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-release-2271-app + namespace: default +spec: + template: + spec: + dnsConfig: + options: + - name: ndots + value: "1" diff --git a/test/integration/test-cases/issue-2271/input/helmfile-no-kustomize.yaml b/test/integration/test-cases/issue-2271/input/helmfile-no-kustomize.yaml new file mode 100644 index 00000000..04c1881e --- /dev/null +++ b/test/integration/test-cases/issue-2271/input/helmfile-no-kustomize.yaml @@ -0,0 +1,6 @@ +releases: + - name: test-release-2271 + namespace: default + chart: ./test-chart + installed: true + # No strategicMergePatches - lookup function should work diff --git a/test/integration/test-cases/issue-2271/input/helmfile.yaml b/test/integration/test-cases/issue-2271/input/helmfile.yaml new file mode 100644 index 00000000..c6e40009 --- /dev/null +++ b/test/integration/test-cases/issue-2271/input/helmfile.yaml @@ -0,0 +1,8 @@ +releases: + - name: test-release-2271 + namespace: default + chart: ./test-chart + installed: true + # Using strategicMergePatches causes lookup function to not execute + strategicMergePatches: + - dns-patch.yaml diff --git a/test/integration/test-cases/issue-2271/input/test-chart/Chart.yaml b/test/integration/test-cases/issue-2271/input/test-chart/Chart.yaml new file mode 100644 index 00000000..10a401d2 --- /dev/null +++ b/test/integration/test-cases/issue-2271/input/test-chart/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +name: test-chart-issue-2271 +description: Test chart for issue #2271 - lookup function with kustomize +type: application +version: 1.0.0 +appVersion: "1.0" diff --git a/test/integration/test-cases/issue-2271/input/test-chart/templates/configmap.yaml b/test/integration/test-cases/issue-2271/input/test-chart/templates/configmap.yaml new file mode 100644 index 00000000..42f47b57 --- /dev/null +++ b/test/integration/test-cases/issue-2271/input/test-chart/templates/configmap.yaml @@ -0,0 +1,16 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: {{ .Release.Name }}-config + namespace: {{ .Release.Namespace }} +data: + # Use lookup to preserve existing value if ConfigMap already exists + # This simulates the Grafana PVC volumeName preservation use case + {{- $existing := lookup "v1" "ConfigMap" .Release.Namespace (printf "%s-config" .Release.Name) }} + {{- if $existing }} + preserved-value: {{ index $existing.data "preserved-value" | default "initial-value" | quote }} + {{- else }} + preserved-value: "initial-value" + {{- end }} + # This value can change on upgrades + current-version: {{ .Chart.Version | quote }} diff --git a/test/integration/test-cases/issue-2271/input/test-chart/templates/deployment.yaml b/test/integration/test-cases/issue-2271/input/test-chart/templates/deployment.yaml new file mode 100644 index 00000000..6699d56a --- /dev/null +++ b/test/integration/test-cases/issue-2271/input/test-chart/templates/deployment.yaml @@ -0,0 +1,20 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{ .Release.Name }}-app + namespace: {{ .Release.Namespace }} +spec: + replicas: 1 + selector: + matchLabels: + app: {{ .Release.Name }} + template: + metadata: + labels: + app: {{ .Release.Name }} + spec: + containers: + - name: nginx + image: nginx:latest + ports: + - containerPort: 80 diff --git a/test/integration/test-cases/issue-2275.sh b/test/integration/test-cases/issue-2275.sh new file mode 100755 index 00000000..230552b1 --- /dev/null +++ b/test/integration/test-cases/issue-2275.sh @@ -0,0 +1,89 @@ +#!/usr/bin/env bash + +# Test for issue #2275: helmfile should detect cluster version and pass to helm-diff +# Without this fix, helm-diff falls back to v1.20.0 and fails for charts requiring newer versions + +issue_2275_input_dir="${cases_dir}/issue-2275/input" +issue_2275_tmp_dir=$(mktemp -d) + +cd "${issue_2275_input_dir}" + +test_start "issue-2275: auto-detect kubernetes version for helm-diff" + +info "Testing helmfile apply with chart requiring Kubernetes >=1.25.0" +info "Expected: Success with auto-detected cluster version" + +# Test 1: Apply should succeed with auto-detected cluster version +${helmfile} apply --skip-diff-on-install --suppress-diff > "${issue_2275_tmp_dir}/test-2275-output.txt" 2>&1 +code=$? + +if [ $code -ne 0 ]; then + if grep -q "incompatible with Kubernetes v1.20.0" "${issue_2275_tmp_dir}/test-2275-output.txt"; then + cat "${issue_2275_tmp_dir}/test-2275-output.txt" + rm -rf "${issue_2275_tmp_dir}" + fail "Issue #2275 regression: helm-diff fell back to v1.20.0" + else + cat "${issue_2275_tmp_dir}/test-2275-output.txt" + rm -rf "${issue_2275_tmp_dir}" + fail "Unexpected error during apply" + fi +fi + +info "Chart installed successfully with auto-detected version" + +# Test 2: Diff should work with auto-detected version +info "Testing helmfile diff with auto-detected cluster version" + +${helmfile} diff > "${issue_2275_tmp_dir}/test-2275-diff-output.txt" 2>&1 +code=$? + +if [ $code -ne 0 ] && [ $code -ne 2 ]; then + if grep -q "incompatible with Kubernetes v1.20.0" "${issue_2275_tmp_dir}/test-2275-diff-output.txt"; then + cat "${issue_2275_tmp_dir}/test-2275-diff-output.txt" + rm -rf "${issue_2275_tmp_dir}" + fail "Issue #2275 regression in diff: helm-diff fell back to v1.20.0" + else + cat "${issue_2275_tmp_dir}/test-2275-diff-output.txt" + rm -rf "${issue_2275_tmp_dir}" + fail "Unexpected error during diff" + fi +fi + +info "Diff succeeded with auto-detected version" + +# Test 3: Second apply (upgrade scenario) - this is the critical test case from issue #2275 +# The first apply worked with --skip-diff-on-install, but second apply would fail without the fix +info "Testing second helmfile apply (upgrade scenario) - critical test for issue #2275" +info "Modifying chart to trigger an actual upgrade..." + +# Update chart version to trigger an upgrade +sed -i.bak 's/version: 1.0.0/version: 1.0.1/' test-chart/Chart.yaml + +info "Running helmfile apply to upgrade chart (this will run diff)" +info "This would fail with 'incompatible with Kubernetes v1.20.0' before the fix" + +${helmfile} apply --suppress-diff > "${issue_2275_tmp_dir}/test-2275-apply2-output.txt" 2>&1 +code=$? + +# Restore original chart version +mv test-chart/Chart.yaml.bak test-chart/Chart.yaml + +if [ $code -ne 0 ]; then + if grep -q "incompatible with Kubernetes v1.20.0" "${issue_2275_tmp_dir}/test-2275-apply2-output.txt"; then + cat "${issue_2275_tmp_dir}/test-2275-apply2-output.txt" + rm -rf "${issue_2275_tmp_dir}" + fail "Issue #2275 regression: upgrade failed - helm-diff fell back to v1.20.0" + else + cat "${issue_2275_tmp_dir}/test-2275-apply2-output.txt" + rm -rf "${issue_2275_tmp_dir}" + fail "Unexpected error during upgrade" + fi +fi + +info "Upgrade succeeded with auto-detected version" + +# Cleanup +${helm} uninstall test-release-2275 --namespace default 2>/dev/null || true +rm -rf "${issue_2275_tmp_dir}" + +test_pass "issue-2275: auto-detect kubernetes version for helm-diff" diff --git a/test/integration/test-cases/issue-2275/input/helmfile.yaml b/test/integration/test-cases/issue-2275/input/helmfile.yaml new file mode 100644 index 00000000..5262b1e9 --- /dev/null +++ b/test/integration/test-cases/issue-2275/input/helmfile.yaml @@ -0,0 +1,5 @@ +releases: + - name: test-release-2275 + namespace: default + chart: ./test-chart + installed: true diff --git a/test/integration/test-cases/issue-2275/input/test-chart/Chart.yaml b/test/integration/test-cases/issue-2275/input/test-chart/Chart.yaml new file mode 100644 index 00000000..b9092b26 --- /dev/null +++ b/test/integration/test-cases/issue-2275/input/test-chart/Chart.yaml @@ -0,0 +1,9 @@ +apiVersion: v2 +name: test-chart-issue-2275 +description: Test chart for issue #2275 +type: application +version: 1.0.0 +appVersion: "1.0" +# This chart requires Kubernetes 1.25 or higher +# Without the fix, helm-diff uses v1.20.0 and fails +kubeVersion: ">=1.25.0" diff --git a/test/integration/test-cases/issue-2275/input/test-chart/templates/deployment.yaml b/test/integration/test-cases/issue-2275/input/test-chart/templates/deployment.yaml new file mode 100644 index 00000000..463d14e5 --- /dev/null +++ b/test/integration/test-cases/issue-2275/input/test-chart/templates/deployment.yaml @@ -0,0 +1,19 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-app +spec: + replicas: 1 + selector: + matchLabels: + app: test-app + template: + metadata: + labels: + app: test-app + spec: + containers: + - name: nginx + image: nginx:latest + ports: + - containerPort: 80 diff --git a/test/integration/test-cases/issue-2280.sh b/test/integration/test-cases/issue-2280.sh new file mode 100644 index 00000000..8af819ce --- /dev/null +++ b/test/integration/test-cases/issue-2280.sh @@ -0,0 +1,80 @@ +#!/usr/bin/env bash + +# Test for issue #2280: --color flag conflict with Helm 4 +# In Helm 4, the --color flag is parsed by Helm before reaching the helm-diff plugin +# The fix removes --color/--no-color flags and uses HELM_DIFF_COLOR env var instead + +# Only run this test on Helm 4 +if [ "${HELMFILE_HELM4}" != "1" ]; then + info "Skipping issue-2280 test (Helm 4 only)" + return 0 +fi + +issue_2280_input_dir="${cases_dir}/issue-2280/input" +issue_2280_tmp_dir=$(mktemp -d) + +cd "${issue_2280_input_dir}" + +test_start "issue-2280: --color flag with Helm 4" + +# Test 1: Install the chart first +info "Installing chart for issue #2280 test" +${helmfile} -f helmfile.yaml apply --suppress-diff > "${issue_2280_tmp_dir}/install.txt" 2>&1 +code=$? + +if [ $code -ne 0 ]; then + cat "${issue_2280_tmp_dir}/install.txt" + rm -rf "${issue_2280_tmp_dir}" + fail "Failed to install chart" +fi + +info "Chart installed successfully" + +# Test 2: Run diff with --color and --context flags +# This is the exact scenario from issue #2280 +# Before the fix, --color flag would be parsed by Helm 4 before reaching helm-diff plugin, +# consuming --context as its value, resulting in error: invalid color mode "--context" +# After the fix, --color is removed and HELM_DIFF_COLOR env var is set instead +info "Running diff with --color and --context flags" + +${helmfile} -f helmfile.yaml diff --color --context 3 > "${issue_2280_tmp_dir}/diff-color.txt" 2>&1 +code=$? + +# Check for the error from issue #2280 +if grep -q "invalid color mode" "${issue_2280_tmp_dir}/diff-color.txt"; then + cat "${issue_2280_tmp_dir}/diff-color.txt" + rm -rf "${issue_2280_tmp_dir}" + fail "Issue #2280 regression: --color flag consumed --context argument" +fi + +# diff command should succeed (exit code 0 or 2 with --detailed-exitcode) +if [ $code -ne 0 ]; then + # Check if it's a diff-related error (not the color mode error) + if ! grep -q "Comparing release" "${issue_2280_tmp_dir}/diff-color.txt"; then + cat "${issue_2280_tmp_dir}/diff-color.txt" + rm -rf "${issue_2280_tmp_dir}" + fail "Diff command failed unexpectedly" + fi +fi + +info "SUCCESS: --color flag did not interfere with --context flag" + +# Test 3: Also test with --no-color +info "Running diff with --no-color and --context flags" + +${helmfile} -f helmfile.yaml diff --no-color --context 3 > "${issue_2280_tmp_dir}/diff-no-color.txt" 2>&1 +code=$? + +if grep -q "invalid color mode" "${issue_2280_tmp_dir}/diff-no-color.txt"; then + cat "${issue_2280_tmp_dir}/diff-no-color.txt" + rm -rf "${issue_2280_tmp_dir}" + fail "Issue #2280 regression: --no-color flag consumed --context argument" +fi + +info "SUCCESS: --no-color flag did not interfere with --context flag" + +# Cleanup +${helm} uninstall test-release-2280 --namespace default 2>/dev/null || true +rm -rf "${issue_2280_tmp_dir}" + +test_pass "issue-2280: --color flag with Helm 4" diff --git a/test/integration/test-cases/issue-2280/input/chart/Chart.yaml b/test/integration/test-cases/issue-2280/input/chart/Chart.yaml new file mode 100644 index 00000000..c30671cf --- /dev/null +++ b/test/integration/test-cases/issue-2280/input/chart/Chart.yaml @@ -0,0 +1,5 @@ +apiVersion: v2 +name: test-chart-2280 +description: Test chart for issue #2280 +version: 0.1.0 +appVersion: "1.0" diff --git a/test/integration/test-cases/issue-2280/input/chart/templates/configmap.yaml b/test/integration/test-cases/issue-2280/input/chart/templates/configmap.yaml new file mode 100644 index 00000000..3344b726 --- /dev/null +++ b/test/integration/test-cases/issue-2280/input/chart/templates/configmap.yaml @@ -0,0 +1,6 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: {{ .Release.Name }}-config +data: + message: {{ .Values.message | quote }} diff --git a/test/integration/test-cases/issue-2280/input/chart/values.yaml b/test/integration/test-cases/issue-2280/input/chart/values.yaml new file mode 100644 index 00000000..21277d71 --- /dev/null +++ b/test/integration/test-cases/issue-2280/input/chart/values.yaml @@ -0,0 +1 @@ +message: "default message" diff --git a/test/integration/test-cases/issue-2280/input/helmfile.yaml b/test/integration/test-cases/issue-2280/input/helmfile.yaml new file mode 100644 index 00000000..385f934b --- /dev/null +++ b/test/integration/test-cases/issue-2280/input/helmfile.yaml @@ -0,0 +1,6 @@ +releases: + - name: test-release-2280 + namespace: default + chart: ./chart + values: + - message: "test message" diff --git a/test/integration/test-cases/suppress-output-line-regex/input/helmfile.yaml.gotmpl b/test/integration/test-cases/suppress-output-line-regex/input/helmfile.yaml.gotmpl index 634a5f76..d08fee3c 100644 --- a/test/integration/test-cases/suppress-output-line-regex/input/helmfile.yaml.gotmpl +++ b/test/integration/test-cases/suppress-output-line-regex/input/helmfile.yaml.gotmpl @@ -2,6 +2,9 @@ helmDefaults: suppressOutputLineRegex: - "helm.sh/chart" - "app.kubernetes.io/version" + # Disable auto-detected kubeVersion to prevent helm-diff from normalizing server-side defaults + # which would hide changes like ipFamilyPolicy and ipFamilies being removed + disableAutoDetectedKubeVersionForDiff: true repositories: - name: ingress-nginx