fix: Preserve CRD location when patching charts (#2291)

[aditmeno]

Summary

Fixes helmfile/helmfile#2291

This PR fixes an issue where strategicMergePatches causes CRDs from templates/crds/ to be relocated to the root crds/ directory, changing how Helm manages them and causing helm diff to show CRDs being removed.

Problem

When using strategicMergePatches with charts like KEDA that place CRDs in templates/crds/:

1. Before patching: CRDs exist in templates/crds/ (managed as regular resources)
2. After chartify processing: CRDs relocated to root crds/ directory (Helm special handling)
3. Result: helm diff shows CRDs being removed, even though patches don't reference CRDs

Why This Matters

Chart authors intentionally use templates/crds/ instead of root crds/ for:

• Conditional rendering: {{- if .Values.crds.install }}
• Template features: .Release.Namespace, labels, annotations
• Upgrade capability: CRDs that need updates during helm upgrade

Helm's Different Treatment

Aspect	templates/crds/	crds/
Templating	✅ Go templates work	❌ Plain YAML only
On upgrade	✅ Updated	❌ Immutable (install-only)
On uninstall	✅ Deleted	❌ Orphaned

Root Cause

In patch.go:342-364, CRDs were unconditionally relocated:

if r.IsHelm3() || r.IsHelm4() {
    crdsDir = filepath.Join(tempDir, "crds")  // ❌ Always moves to crds/
}

Solution

Detect if CRDs originally came from templates/crds/ and preserve that location:

// Detect if CRDs originally came from templates/ directory
crdsFromTemplates := false
for _, f := range generatedManifestFiles {
    relPath := filepath.ToSlash(strings.TrimPrefix(f, tempDir))
    if strings.HasPrefix(relPath, "templates/crds/") {
        crdsFromTemplates = true
        break
    }
}

// Preserve original location
if crdsFromTemplates {
    crdsDir = filepath.Join(tempDir, "templates", "crds")  // ✅ Keep original location
} else if r.IsHelm3() || r.IsHelm4() {
    crdsDir = filepath.Join(tempDir, "crds")  // Standard location
}

Changes

patch.go

• Added CRD source location detection (lines 48-62)
• Modified CRD placement logic to preserve original location (lines 342-368)
• Added logging to indicate when CRDs are preserved vs. standard placement
• Cross-platform path handling with filepath.ToSlash()

patch_test.go (new file)

Added comprehensive test coverage:

1. TestPatch_PreserveCRDLocation: End-to-end scenarios

   • CRDs from templates/crds/ stay in templates/crds/
   • CRDs from root crds/ go to crds/
   • Mixed resources without CRDs

2. TestPatch_CRDLocationDetection: Detection logic validation

   • Correct detection of templates/crds/ files
   • No false positives for other paths
   • Cross-platform path handling

3. TestPatch_KEDA_RealWorld: Real-world KEDA scenario

   • Mimics actual KEDA chart structure
   • Validates the fix for issue #2291

Testing

# Run new tests
go test -v -run TestPatch_PreserveCRDLocation
go test -v -run TestPatch_CRDLocationDetection
go test -v -run TestPatch_KEDA_RealWorld

# Run all tests
go test -v ./...

Test Coverage

• ✅ CRDs from templates/crds/ preserved in original location
• ✅ CRDs from root crds/ use standard Helm 3/4 location
• ✅ No CRDs scenario (no regression)
• ✅ KEDA real-world structure (6 CRDs in templates/crds/)
• ✅ Cross-platform path handling (Windows/Unix)

Verification

A complete reproduction test case is available at /tmp/keda-issue-test/ with:

• Automated verification script (verify-issue.sh)
• KEDA chart structure analysis
• Before/after comparison
• Complete documentation

Backward Compatibility

✅ No breaking changes

• Charts with CRDs in root crds/ → same behavior as before
• Charts with CRDs in templates/crds/ → now correctly preserved
• Charts without CRDs → no impact

Impact

Affected Charts

Any chart with CRDs in templates/ subdirectories:

• KEDA (confirmed affected)
• Community charts using conditional CRD installation
• Charts requiring CRD updates during upgrades

Benefits

• ✅ Preserves chart author's intent
• ✅ Fixes helm diff showing incorrect CRD deletions
• ✅ Maintains upgrade capability for template-based CRDs
• ✅ Supports conditional CRD installation

Related Issues

• Use of strategicMergePatches causes KEDA chart to uninstall its CRDs helmfile/helmfile#2291 - Original issue report
• See /tmp/keda-issue-test/SUMMARY.md for complete analysis
• See /tmp/keda-issue-test/ANALYSIS.md for technical deep-dive

Reviewers

Key areas to review:

1. Detection logic (patch.go:48-62): Correctly identifies templates/crds/ files
2. Preservation logic (patch.go:342-368): Places CRDs in correct location
3. Tests (patch_test.go): Comprehensive coverage of scenarios
4. Cross-platform compatibility: Path handling with filepath.ToSlash()

---

Ready for review ✅

[aditmeno]

Closing this PR as it was created to the upstream repository instead: helmfile#160