fix: Preserve CRD location when patching charts

[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 := strings.Replace(f, tempDir+string(filepath.Separator), "", 1)
    relPath = filepath.ToSlash(relPath)
    if strings.HasPrefix(relPath, "templates/crds/") {
        crdsFromTemplates = true
        r.Logf("Detected CRDs in templates/ directory - will preserve location")
        break
    }
}

// Preserve original location to maintain chart author's intent
if crdsFromTemplates {
    crdsDir = filepath.Join(tempDir, "templates", "crds")  // ✅ Keep original
    r.Logf("Preserving CRDs in templates/crds/ (original location)")
} else if r.IsHelm3() || r.IsHelm4() {
    crdsDir = filepath.Join(tempDir, "crds")  // Standard location
    r.Logf("Placing CRDs in crds/ (standard Helm 3/4 location)")
}

Changes

patch.go

• Lines 48-62: Added CRD source location detection logic

  • Checks if any generated files are in templates/crds/ subdirectory
  • Uses filepath.ToSlash() for cross-platform compatibility
  • Logs detection result for debugging

• Lines 342-368: Modified CRD placement logic

  • Preserves templates/crds/ location when detected
  • Falls back to standard crds/ location for Helm 3/4
  • Maintains Helm 2 compatibility
  • Added logging for transparency

• Comments: Added detailed explanations referencing issue #2291

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 in templates/

2. TestPatch_CRDLocationDetection: Detection logic validation

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

3. TestPatch_KEDA_RealWorld: Real-world KEDA scenario

   • ✅ Mimics actual KEDA chart structure (6 CRDs in templates/crds/)
   • ✅ 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 Results

All tests pass and cover:

• ✅ CRD location preservation logic
• ✅ Path normalization (Windows/Unix)
• ✅ Edge cases (no CRDs, mixed resources)
• ✅ Real-world chart structures (KEDA)

Verification

A complete reproduction test case demonstrating the issue is available with:

• Automated verification script showing CRD relocation bug
• KEDA chart structure analysis (v2.18.1)
• Before/after comparison of chart structure
• Complete technical documentation with code references

Run verification:

# See /tmp/keda-issue-test/README.md for full reproduction steps
cd /tmp/keda-issue-test
./verify-issue.sh

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
• All existing functionality maintained

Impact

Affected Charts

Any chart with CRDs in templates/ subdirectories:

• KEDA (confirmed affected - Use of strategicMergePatches causes KEDA chart to uninstall its CRDs helmfile#2291)
• Community charts using conditional CRD installation
• Charts requiring CRD updates during upgrades
• Charts using template features in CRDs

Benefits

• ✅ Preserves chart author's intent and design
• ✅ Fixes helm diff showing incorrect CRD deletions
• ✅ Maintains upgrade capability for template-based CRDs
• ✅ Supports conditional CRD installation patterns
• ✅ Prevents accidental CRD management behavior changes

Example: KEDA Chart

Before this fix:

helm template + strategicMergePatches
  → CRDs moved from templates/crds/ to crds/
  → helm diff shows 6 CRDs being removed ❌
  → Patches on Deployments unexpectedly affect CRD management

After this fix:

helm template + strategicMergePatches
  → CRDs preserved in templates/crds/
  → helm diff shows only intended Deployment changes ✅
  → CRD management behavior unchanged

Related Issues

• Use of strategicMergePatches causes KEDA chart to uninstall its CRDs helmfile#2291 - Original issue report with detailed user impact
• This fix enables strategicMergePatches to work correctly with KEDA and similar charts

Review Guide

Key areas for reviewers:

1. Detection Logic (patch.go:48-62)

   • Correctly identifies files in templates/crds/ subdirectory
   • Handles path separators cross-platform
   • Early detection before processing

2. Preservation Logic (patch.go:342-368)

   • Places CRDs in correct location based on source
   • Maintains backward compatibility
   • Clear logging for debugging

3. Test Coverage (patch_test.go)

   • Comprehensive scenario coverage
   • Real-world KEDA structure validation
   • Cross-platform compatibility

4. Documentation

   • Clear comments explaining the "why"
   • References to original issue
   • Helm behavior differences documented

---

Ready for review ✅

This fix resolves a critical issue where patches unintentionally change CRD management behavior, making strategicMergePatches safe to use with charts like KEDA.

[aditmeno]

@yxxhero PR is ready for review

[zhaque44]

@yxxhero @mumoshu are we sure we want this change?

[yxxhero]

@aditmeno LGTM.