diff --git a/go.mod b/go.mod index 61e7ab62..96c0d2e0 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,7 @@ require ( github.com/gosuri/uitable v0.0.4 github.com/hashicorp/go-getter v1.8.3 github.com/hashicorp/hcl/v2 v2.24.0 - github.com/helmfile/chartify v0.26.0 + github.com/helmfile/chartify v0.26.1 github.com/helmfile/vals v0.42.6 github.com/spf13/cobra v1.10.1 github.com/spf13/pflag v1.0.10 diff --git a/go.sum b/go.sum index fcc5dc27..2b20d953 100644 --- a/go.sum +++ b/go.sum @@ -481,8 +481,8 @@ github.com/hashicorp/jsonapi v1.4.3-0.20250220162346-81a76b606f3e h1:xwy/1T0cxHW github.com/hashicorp/jsonapi v1.4.3-0.20250220162346-81a76b606f3e/go.mod h1:kWfdn49yCjQvbpnvY1dxxAuAFzISwrrMDQOcu6NsFoM= github.com/hashicorp/vault/api v1.22.0 h1:+HYFquE35/B74fHoIeXlZIP2YADVboaPjaSicHEZiH0= github.com/hashicorp/vault/api v1.22.0/go.mod h1:IUZA2cDvr4Ok3+NtK2Oq/r+lJeXkeCrHRmqdyWfpmGM= -github.com/helmfile/chartify v0.26.0 h1:uG1sThH7MGhyuevTqnwi70+7SHh+IpLSd2SnBVGYmZo= -github.com/helmfile/chartify v0.26.0/go.mod h1:e4Ym+XfSIPdqG3KL8lwkSrvQzrRKTEQKyF1/8BoFpVA= +github.com/helmfile/chartify v0.26.1 h1:HzVs7qbas5HB/vyIJCo6vgpnPX+hLnhx51Bl352Jj0A= +github.com/helmfile/chartify v0.26.1/go.mod h1:e4Ym+XfSIPdqG3KL8lwkSrvQzrRKTEQKyF1/8BoFpVA= github.com/helmfile/vals v0.42.6 h1:ephnaYL1F9F96iUL5fVjeOi50wqNYvG4QhCpuiklwwg= github.com/helmfile/vals v0.42.6/go.mod h1:ngPOAKsECRSSUQcE1Pdu4LbMr4Nrvy2b7Ut1d+oyJxg= github.com/hinshun/vt10x v0.0.0-20220119200601-820417d04eec h1:qv2VnGeEQHchGaZ/u7lxST/RaJw+cv273q79D81Xbog= diff --git a/test/integration/run.sh b/test/integration/run.sh index da635768..41e07b64 100755 --- a/test/integration/run.sh +++ b/test/integration/run.sh @@ -116,6 +116,7 @@ ${kubectl} create namespace ${test_ns} || fail "Could not create namespace ${tes . ${dir}/test-cases/state-values-set-cli-args-in-environments.sh . ${dir}/test-cases/issue-2281-array-merge.sh . ${dir}/test-cases/issue-2247.sh +. ${dir}/test-cases/issue-2291.sh # ALL DONE ----------------------------------------------------------------------------------------------------------- diff --git a/test/integration/test-cases/issue-2247.sh b/test/integration/test-cases/issue-2247.sh index bbc8d439..8cb34368 100755 --- a/test/integration/test-cases/issue-2247.sh +++ b/test/integration/test-cases/issue-2247.sh @@ -65,6 +65,7 @@ info "SUCCESS: OCI charts without version do not trigger validation error" if ! command -v docker &> /dev/null; then info "Skipping registry tests (Docker not available)" rm -rf "${issue_2247_tmp_dir}" + trap - EXIT test_pass "issue-2247: OCI charts without version (validation tests only)" return 0 fi @@ -73,6 +74,7 @@ fi if ! docker info &> /dev/null; then info "Skipping registry tests (Docker daemon not running)" rm -rf "${issue_2247_tmp_dir}" + trap - EXIT test_pass "issue-2247: OCI charts without version (validation tests only)" return 0 fi @@ -105,6 +107,7 @@ if [ $? -ne 0 ]; then cat "${issue_2247_tmp_dir}/registry-start.log" warn "Failed to start Docker registry - skipping registry tests" rm -rf "${issue_2247_tmp_dir}" + trap - EXIT test_pass "issue-2247: OCI charts without version (validation tests only)" return 0 fi @@ -125,6 +128,7 @@ done if [ $attempt -eq $max_attempts ]; then warn "Registry did not become ready in time - skipping registry tests" cleanup_registry + trap - EXIT test_pass "issue-2247: OCI charts without version (validation tests only)" return 0 fi @@ -138,6 +142,7 @@ if [ $? -ne 0 ]; then cat "${issue_2247_tmp_dir}/package.log" warn "Failed to package chart - skipping registry tests" cleanup_registry + trap - EXIT test_pass "issue-2247: OCI charts without version (validation tests only)" return 0 fi @@ -145,12 +150,14 @@ set -e # Re-enable exit on error after successful package info "Pushing chart version 1.0.0 to local registry" set +e # Disable exit on error to handle failures gracefully -${helm} push "${issue_2247_tmp_dir}/test-chart-2247-1.0.0.tgz" oci://localhost:${registry_port} > "${issue_2247_tmp_dir}/push.log" 2>&1 +# Use --plain-http for localhost registry (HTTP instead of HTTPS) +${helm} push "${issue_2247_tmp_dir}/test-chart-2247-1.0.0.tgz" oci://localhost:${registry_port} --plain-http > "${issue_2247_tmp_dir}/push.log" 2>&1 if [ $? -ne 0 ]; then set -e # Re-enable before cleanup cat "${issue_2247_tmp_dir}/push.log" warn "Failed to push chart to registry - skipping registry tests" cleanup_registry + trap - EXIT test_pass "issue-2247: OCI charts without version (validation tests only)" return 0 fi @@ -162,7 +169,7 @@ cp -r "${issue_2247_chart_dir}" "${issue_2247_tmp_dir}/chart-v2" sed -i.bak 's/version: 1.0.0/version: 2.0.0/' "${issue_2247_tmp_dir}/chart-v2/Chart.yaml" set +e # Disable exit on error for package/push operations ${helm} package "${issue_2247_tmp_dir}/chart-v2" -d "${issue_2247_tmp_dir}" > "${issue_2247_tmp_dir}/package-v2.log" 2>&1 -${helm} push "${issue_2247_tmp_dir}/test-chart-2247-2.0.0.tgz" oci://localhost:${registry_port} > "${issue_2247_tmp_dir}/push-v2.log" 2>&1 +${helm} push "${issue_2247_tmp_dir}/test-chart-2247-2.0.0.tgz" oci://localhost:${registry_port} --plain-http > "${issue_2247_tmp_dir}/push-v2.log" 2>&1 set -e # Re-enable exit on error info "Successfully pushed chart versions 1.0.0 and 2.0.0" @@ -270,4 +277,6 @@ else fi # All tests passed! +# Remove the EXIT trap to avoid interfering with subsequent tests +trap - EXIT test_pass "issue-2247: OCI charts without version (all tests including registry)" diff --git a/test/integration/test-cases/issue-2291.sh b/test/integration/test-cases/issue-2291.sh new file mode 100755 index 00000000..a8457761 --- /dev/null +++ b/test/integration/test-cases/issue-2291.sh @@ -0,0 +1,100 @@ +#!/usr/bin/env bash + +# Test for issue #2291: strategicMergePatches should NOT cause CRDs to be removed +# Issue: https://github.com/helmfile/helmfile/issues/2291 +# +# Problem: When using strategicMergePatches, chartify was relocating CRDs from +# templates/crds/ to the root crds/ directory, changing how Helm manages them. +# This caused helm diff to incorrectly show CRDs as being removed. +# +# Fix: Chartify now preserves the original CRD location (templates/crds/) + +issue_2291_input_dir="${cases_dir}/issue-2291/input" +issue_2291_tmp_dir=$(mktemp -d) + +# Cleanup function to ensure resources are removed even if test fails +cleanup_issue_2291() { + ${helm} uninstall test-release-2291 --namespace ${test_ns} 2>/dev/null || true + ${kubectl} delete crd testresources.test.io 2>/dev/null || true + rm -rf "${issue_2291_tmp_dir}" +} +trap cleanup_issue_2291 EXIT + +test_start "issue-2291: CRDs preserved with strategicMergePatches" + +# Test 1: Template the chart to verify CRDs are included +info "Step 1: Templating chart to verify CRD structure" +${helmfile} -f "${issue_2291_input_dir}/helmfile.yaml" template > "${issue_2291_tmp_dir}/templated.yaml" 2>&1 +code=$? + +if [ $code -ne 0 ]; then + cat "${issue_2291_tmp_dir}/templated.yaml" + fail "Failed to template chart" +fi + +# Verify CRD is in the templated output +if ! grep -q "kind: CustomResourceDefinition" "${issue_2291_tmp_dir}/templated.yaml"; then + cat "${issue_2291_tmp_dir}/templated.yaml" + fail "CRD not found in templated output" +fi + +info "✓ CRD found in templated output" + +# Verify the CRD name +if ! grep -q "name: testresources.test.io" "${issue_2291_tmp_dir}/templated.yaml"; then + cat "${issue_2291_tmp_dir}/templated.yaml" + fail "Expected CRD 'testresources.test.io' not found" +fi + +info "✓ CRD testresources.test.io found" + +# Test 2: Apply the chart with strategicMergePatches +info "Step 2: Applying chart with strategicMergePatches" +${helmfile} -f "${issue_2291_input_dir}/helmfile.yaml" apply --suppress-diff > "${issue_2291_tmp_dir}/apply.txt" 2>&1 +code=$? + +if [ $code -ne 0 ]; then + cat "${issue_2291_tmp_dir}/apply.txt" + fail "Failed to apply chart" +fi + +info "✓ Chart applied successfully" + +# Test 3: Verify CRD was created +info "Step 3: Verifying CRD was installed" +if ! ${kubectl} get crd testresources.test.io > /dev/null 2>&1; then + fail "CRD testresources.test.io was not installed" +fi + +info "✓ CRD testresources.test.io is installed" + +# Test 4: Run diff - should show NO changes (especially NO CRD removal) +info "Step 4: Running diff - should show no CRD removal" +${helmfile} -f "${issue_2291_input_dir}/helmfile.yaml" diff > "${issue_2291_tmp_dir}/diff.txt" 2>&1 + +# Check if diff shows CRD being removed (the bug we're fixing). +# Note: Exit code is not checked since helmfile diff returns 2 when differences exist. +if grep -q "testresources.test.io.*will be deleted" "${issue_2291_tmp_dir}/diff.txt" || \ + grep -q "testresources.test.io.*removed" "${issue_2291_tmp_dir}/diff.txt" || \ + grep -q -- "- kind: CustomResourceDefinition" "${issue_2291_tmp_dir}/diff.txt"; then + cat "${issue_2291_tmp_dir}/diff.txt" + fail "BUG DETECTED: helm diff shows CRD being removed (issue #2291 regression)" +fi + +info "✓ CRD is NOT marked for removal" + +# Test 5: Verify Deployment has the DNS patch applied +info "Step 5: Verifying strategic merge patch was applied to Deployment" +if ! ${kubectl} get deployment test-app -o yaml | grep -q "ndots"; then + fail "DNS patch was not applied to Deployment" +fi + +info "✓ Strategic merge patch applied successfully" + +# Cleanup is handled by the trap, but we do it explicitly here too +# and then remove the trap before test_pass to avoid double cleanup +info "Cleaning up" +cleanup_issue_2291 +trap - EXIT + +test_pass "issue-2291: CRDs preserved with strategicMergePatches" diff --git a/test/integration/test-cases/issue-2291/input/dns-patch.yaml b/test/integration/test-cases/issue-2291/input/dns-patch.yaml new file mode 100644 index 00000000..c41e5e61 --- /dev/null +++ b/test/integration/test-cases/issue-2291/input/dns-patch.yaml @@ -0,0 +1,14 @@ +# Strategic merge patch to modify Deployment +# This patch should NOT cause CRDs to be removed +--- +apiVersion: apps/v1 +kind: Deployment +metadata: + name: test-app +spec: + template: + spec: + dnsConfig: + options: + - name: ndots + value: "1" diff --git a/test/integration/test-cases/issue-2291/input/helmfile.yaml b/test/integration/test-cases/issue-2291/input/helmfile.yaml new file mode 100644 index 00000000..97231e19 --- /dev/null +++ b/test/integration/test-cases/issue-2291/input/helmfile.yaml @@ -0,0 +1,8 @@ +releases: + - name: test-release-2291 + chart: ./test-chart + installed: true + # Strategic merge patches should NOT cause CRDs to be removed + # Issue: https://github.com/helmfile/helmfile/issues/2291 + strategicMergePatches: + - dns-patch.yaml diff --git a/test/integration/test-cases/issue-2291/input/test-chart/Chart.yaml b/test/integration/test-cases/issue-2291/input/test-chart/Chart.yaml new file mode 100644 index 00000000..938d16ed --- /dev/null +++ b/test/integration/test-cases/issue-2291/input/test-chart/Chart.yaml @@ -0,0 +1,6 @@ +apiVersion: v2 +name: test-chart-issue-2291 +description: Test chart for issue #2291 - CRD location preservation +type: application +version: 1.0.0 +appVersion: "1.0.0" diff --git a/test/integration/test-cases/issue-2291/input/test-chart/templates/crds/crd-testresources.yaml b/test/integration/test-cases/issue-2291/input/test-chart/templates/crds/crd-testresources.yaml new file mode 100644 index 00000000..3fa3a585 --- /dev/null +++ b/test/integration/test-cases/issue-2291/input/test-chart/templates/crds/crd-testresources.yaml @@ -0,0 +1,29 @@ +{{- if .Values.crds.install }} +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + name: testresources.test.io + labels: + app: {{ .Chart.Name }} +spec: + group: test.io + names: + kind: TestResource + listKind: TestResourceList + plural: testresources + singular: testresource + scope: Namespaced + versions: + - name: v1 + served: true + storage: true + schema: + openAPIV3Schema: + type: object + properties: + spec: + type: object + properties: + replicas: + type: integer +{{- end }} diff --git a/test/integration/test-cases/issue-2291/input/test-chart/templates/deployment.yaml b/test/integration/test-cases/issue-2291/input/test-chart/templates/deployment.yaml new file mode 100644 index 00000000..19e5954f --- /dev/null +++ b/test/integration/test-cases/issue-2291/input/test-chart/templates/deployment.yaml @@ -0,0 +1,21 @@ +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{ .Values.deployment.name }} + labels: + app: {{ .Chart.Name }} +spec: + replicas: {{ .Values.deployment.replicas }} + selector: + matchLabels: + app: {{ .Chart.Name }} + template: + metadata: + labels: + app: {{ .Chart.Name }} + spec: + containers: + - name: app + image: nginx:latest + ports: + - containerPort: 80 diff --git a/test/integration/test-cases/issue-2291/input/test-chart/values.yaml b/test/integration/test-cases/issue-2291/input/test-chart/values.yaml new file mode 100644 index 00000000..80682d08 --- /dev/null +++ b/test/integration/test-cases/issue-2291/input/test-chart/values.yaml @@ -0,0 +1,6 @@ +crds: + install: true + +deployment: + name: test-app + replicas: 1 diff --git a/test/integration/test-cases/suppress-output-line-regex/output/diff-helm4 b/test/integration/test-cases/suppress-output-line-regex/output/diff-helm4 index 944faa16..e455c7de 100644 --- a/test/integration/test-cases/suppress-output-line-regex/output/diff-helm4 +++ b/test/integration/test-cases/suppress-output-line-regex/output/diff-helm4 @@ -1,5 +1,5 @@ Comparing release=ingress-nginx, chart=ingress-nginx/ingress-nginx, namespace=helmfile-tests -helmfile-tests, ingress-nginx, ClusterRole (rbac.authorization.k8s.io) has changed: +helmfile-tests, ingress-nginx, ClusterRole (rbac.authorization.k8s.io) has changed, but diff is empty after suppression. helmfile-tests, ingress-nginx, ClusterRoleBinding (rbac.authorization.k8s.io) has changed: # Source: ingress-nginx/templates/clusterrolebinding.yaml apiVersion: rbac.authorization.k8s.io/v1 @@ -134,8 +134,8 @@ helmfile-tests, ingress-nginx, RoleBinding (rbac.authorization.k8s.io) has chang name: ingress-nginx - namespace: "helmfile-tests" + namespace: helmfile-tests -helmfile-tests, ingress-nginx, ServiceAccount (v1) has changed: -helmfile-tests, ingress-nginx-admission, ClusterRole (rbac.authorization.k8s.io) has changed: +helmfile-tests, ingress-nginx, ServiceAccount (v1) has changed, but diff is empty after suppression. +helmfile-tests, ingress-nginx-admission, ClusterRole (rbac.authorization.k8s.io) has changed, but diff is empty after suppression. helmfile-tests, ingress-nginx-admission, ClusterRoleBinding (rbac.authorization.k8s.io) has changed: # Source: ingress-nginx/templates/admission-webhooks/job-patch/clusterrolebinding.yaml apiVersion: rbac.authorization.k8s.io/v1 @@ -210,7 +210,7 @@ helmfile-tests, ingress-nginx-admission, RoleBinding (rbac.authorization.k8s.io) name: ingress-nginx-admission - namespace: "helmfile-tests" + namespace: helmfile-tests -helmfile-tests, ingress-nginx-admission, ServiceAccount (v1) has changed: +helmfile-tests, ingress-nginx-admission, ServiceAccount (v1) has changed, but diff is empty after suppression. helmfile-tests, ingress-nginx-admission, ValidatingWebhookConfiguration (admissionregistration.k8s.io) has changed: # Source: ingress-nginx/templates/admission-webhooks/validating-webhook.yaml # before changing this value, check the required kubernetes version @@ -371,7 +371,7 @@ helmfile-tests, ingress-nginx-admission-patch, Job (batch) has changed: - fsGroup: 2000 - runAsNonRoot: true - runAsUser: 2000 -helmfile-tests, ingress-nginx-controller, ConfigMap (v1) has changed: +helmfile-tests, ingress-nginx-controller, ConfigMap (v1) has changed, but diff is empty after suppression. helmfile-tests, ingress-nginx-controller, Deployment (apps) has changed: # Source: ingress-nginx/templates/controller-deployment.yaml apiVersion: apps/v1 @@ -497,8 +497,41 @@ helmfile-tests, ingress-nginx-controller, Deployment (apps) has changed: secret: secretName: ingress-nginx-admission helmfile-tests, ingress-nginx-controller, Service (v1) has changed: -helmfile-tests, ingress-nginx-controller-admission, Service (v1) has changed: -helmfile-tests, nginx, IngressClass (networking.k8s.io) has changed: + # Source: ingress-nginx/templates/controller-service.yaml + apiVersion: v1 + kind: Service + metadata: + annotations: + labels: + app.kubernetes.io/name: ingress-nginx + app.kubernetes.io/instance: ingress-nginx + app.kubernetes.io/part-of: ingress-nginx + app.kubernetes.io/managed-by: Helm + app.kubernetes.io/component: controller + name: ingress-nginx-controller + namespace: helmfile-tests + spec: + type: LoadBalancer +- ipFamilyPolicy: SingleStack +- ipFamilies: +- - IPv4 + ports: + - name: http + port: 80 + protocol: TCP + targetPort: http + appProtocol: http + - name: https + port: 443 + protocol: TCP + targetPort: https + appProtocol: https + selector: + app.kubernetes.io/name: ingress-nginx + app.kubernetes.io/instance: ingress-nginx + app.kubernetes.io/component: controller +helmfile-tests, ingress-nginx-controller-admission, Service (v1) has changed, but diff is empty after suppression. +helmfile-tests, nginx, IngressClass (networking.k8s.io) has changed, but diff is empty after suppression. helmfile-tests, ingress-nginx-admission, NetworkPolicy (networking.k8s.io) has been removed: - # Source: ingress-nginx/templates/admission-webhooks/job-patch/networkpolicy.yaml - apiVersion: networking.k8s.io/v1