* test: add integration test for issue #2291 with all fixes Add comprehensive integration test for issue #2291 that validates CRD preservation when using strategicMergePatches with chartify. Problem: When using strategicMergePatches, chartify was relocating CRDs from templates/crds/ to root crds/ directory, changing how Helm manages them. This caused helm diff to incorrectly show CRDs as being removed, even though they were still present. Solution: Chartify now preserves the original CRD location in templates/crds/. This integration test validates the fix by: 1. Templating a chart with CRDs in templates/crds/ 2. Applying the chart with strategicMergePatches 3. Verifying CRD is installed 4. Running helm diff to ensure CRD is NOT marked for removal 5. Verifying the strategic merge patch was applied Additional fixes included in this commit: - Fixed grep command error when matching YAML deletion patterns - Updated expected test output for Helm 4 diff behavior - Fixed EXIT trap interference between test cases - Added --plain-http flag for Helm 4 OCI registry compatibility - Ensured CRD templates are valid (cluster-scoped, no namespace) - Fixed strategic merge patch namespace matching Test coverage: - CRD preservation in templates/crds/ subdirectory - Strategic merge patch application - Helm diff behavior with CRDs - Integration with chartify kustomize processing Fixes #2291 Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> * switch chartify package to upstream one Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> * implement copilot suggestion Signed-off-by: Aditya Menon <amenon@canarytechnologies.com> --------- Signed-off-by: Aditya Menon <amenon@canarytechnologies.com>
This commit is contained in:
Aditya Menon
GitHub
parent
83b4a8ffc7
commit
d40bfced56
2
go.mod
2
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
|
||||
|
|
|
|||
4
go.sum
4
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=
|
||||
|
|
|
|||
|
|
@ -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 -----------------------------------------------------------------------------------------------------------
|
||||
|
||||
|
|
|
|||
|
|
@ -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)"
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
@ -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"
|
||||
|
|
@ -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
|
||||
|
|
@ -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"
|
||||
|
|
@ -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 }}
|
||||
|
|
@ -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
|
||||
|
|
@ -0,0 +1,6 @@
|
|||
crds:
|
||||
install: true
|
||||
|
||||
deployment:
|
||||
name: test-app
|
||||
replicas: 1
|
||||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Reference in New Issue