fix: normalize template paths for Windows compatibility

[lexfrei]

Summary

Fixes #10 — template paths with Windows backslashes don't match helm engine map keys.

Changes:

• Add NormalizeTemplatePath() function using filepath.ToSlash() to convert OS-specific separators to forward slash
• Normalize paths in all code paths including error fallbacks
• Remove broken tests in pkg/engine/helm/engine_test.go that referenced non-existent ClientProvider
• Add platform-specific tests (Windows-only via build tag)

Root Cause

filepath.Join() produces OS-specific paths (backslash on Windows), but helm engine's Render() returns map keys with forward slashes regardless of OS. This mismatch causes "template not found" errors on Windows.

Test Plan

• go build ./... passes
• go test ./pkg/engine/... passes
• gofmt -l clean
• Cross-compile check: GOOS=windows go build ./pkg/engine/

Summary by CodeRabbit

• Bug Fixes

  • Improved error handling and messaging for template parsing and in-place file writes.
  • Hardened path resolution to avoid escaping project root and to fall back to normalized paths; path separators normalized for cross-platform compatibility.

• Refactor

  • Streamlined template rendering flow and removed obsolete metadata-generation code.

• Tests

  • Added cross-platform path-normalization tests; simplified rendering tests by removing external client-dependent scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Adds NormalizeTemplatePath to standardize template paths to forward slashes, applies it throughout rendering and CLI modeline/path resolution, removes the generateModeline function, and updates error handling for modeline parsing and in-place file writes.

Changes

Cohort / File(s)	Summary
Path Normalization Core pkg/engine/engine.go	Adds public NormalizeTemplatePath(string) string using filepath.ToSlash. Uses path.Join(chartName, NormalizeTemplatePath(templateFile)) when building template keys for Helm rendering.
Path Normalization Tests pkg/engine/engine_test.go, pkg/engine/engine_windows_test.go	Adds table-driven tests validating normalization across UNIX, Windows, nested, empty, and trailing-slash cases; Windows file is build-tag guarded.
Template Command Updates pkg/commands/template.go	Removes generateModeline and encoding/json usage. Replaces path fallbacks with engine.NormalizeTemplatePath, normalizes modeline and rendering paths, changes error wrapping to %w, and adds error handling for in-place WriteFile failures. Ensures path separators are forward slashes for Helm compatibility.
Helm Engine Test Cleanup pkg/engine/helm/engine_test.go	Deletes tests and helpers that exercised a Kubernetes dynamic client (TestRenderWithClientProvider, TestRenderWithClientProvider_error, testClientProvider, makeUnstructured), simplifying tests to pure Helm rendering.

Sequence Diagram(s)

sequenceDiagram autonumber participant User participant CLI as Template Command participant Engine participant Normalize as NormalizeTemplatePath participant Helm as Helm Engine User->>CLI: talm template -t "templates\controlplane.yaml" CLI->>Engine: Render(templatePath, options) Engine->>Normalize: NormalizeTemplatePath(templatePath) Normalize-->>Engine: "templates/controlplane.yaml" Engine->>Helm: Render(chartName + "/" + normalizedPath) Helm-->>Engine: renderedOutput / renderError Engine-->>CLI: renderedOutput or wrapped error (includes normalized path) CLI-->>User: write output or error (in-place WriteFile error wrapped) 

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: normalizing template paths for Windows compatibility, which is the primary fix addressed across all modified files.
Linked Issues check	✅ Passed	The PR successfully addresses issue #10 by implementing NormalizeTemplatePath to convert OS-specific path separators to forward slashes, ensuring template lookup works consistently across platforms.
Out of Scope Changes check	✅ Passed	The PR includes removal of broken ClientProvider tests and addition of platform-specific tests, all directly supporting the path normalization fix; no unrelated changes detected.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Summary of Changes

Hello @lexfrei, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances cross-platform compatibility, particularly for Windows users, by resolving an inconsistency in how template paths are handled. The core problem stemmed from the Helm engine expecting forward slashes for template keys, while filepath.Join() on Windows would produce backslashes. The solution introduces a dedicated path normalization function, ensuring all template paths are standardized to forward slashes before being passed to the Helm engine, thereby eliminating 'template not found' errors and improving the reliability of template rendering on Windows.

Highlights

• Windows Compatibility Fix: Addresses a critical issue where template paths on Windows systems, using backslashes, would not correctly match the forward-slash keys used by the Helm engine, leading to 'template not found' errors.
• New Path Normalization Function: Introduced a new utility function, NormalizeTemplatePath(), which converts OS-specific path separators (like backslashes on Windows) to standard forward slashes, ensuring consistent path representation across different operating systems.
• Consistent Path Handling: Integrated the NormalizeTemplatePath() function across all relevant code paths, including error fallbacks, to ensure that all template paths are consistently normalized before being processed by the Helm engine.
• Test Suite Refinement: Removed outdated and broken tests in pkg/engine/helm/engine_test.go that referenced a non-existent ClientProvider. New platform-specific tests (Windows-only via build tag) were added to validate the path normalization logic.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request effectively addresses an issue with template path normalization on Windows by introducing a NormalizeTemplatePath function and applying it across the codebase. The changes ensure that paths use forward slashes, which is compatible with the Helm engine. The addition of platform-specific tests is a great way to ensure this fix works as expected and prevents future regressions.

My review includes a couple of suggestions to further improve the code quality. One is to address a significant block of duplicated code for path resolution, which could be refactored for better maintainability. The other is to remove a redundant path normalization call, which would make the code slightly cleaner and more efficient. Overall, this is a solid contribution that improves cross-platform compatibility.

[gemini-code-assist]

The templateFile variable, which comes from opts.TemplateFiles, already contains paths that have been normalized in the generateOutput function in pkg/commands/template.go. Therefore, the call to NormalizeTemplatePath here is redundant. You can simplify the code by removing it.

Suggested change

	requestedTemplate := path.Join(chrt.Name(), NormalizeTemplatePath(templateFile))
	requestedTemplate := path.Join(chrt.Name(), templateFile)

[lexfrei]

Keeping normalization as a defensive measure. filepath.ToSlash is idempotent (no-op on already-normalized paths), so the cost is negligible. This ensures the engine works correctly even if called from contexts that don't pre-normalize paths.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

pkg/commands/template.go (2)

223-272: Consider extracting duplicated path resolution logic.

The path resolution logic between lines 223-272 (for template rendering) and lines 298-383 (for modeline generation) is nearly identical. Both blocks handle the same edge cases: unresolvable root, absolute/relative path conversion, paths escaping root, and normalization.

Consider extracting this into a helper function like resolveTemplatePaths(templateFiles []string, rootDir string) []string to reduce duplication and maintenance burden.

🔎 Sketch of potential helper function

// resolveTemplatePaths resolves template file paths relative to rootDir, // normalizing path separators for Helm engine compatibility. func resolveTemplatePaths(templateFiles []string, rootDir string) []string { resolved := make([]string, len(templateFiles)) absRootDir, rootErr := filepath.Abs(rootDir) if rootErr != nil { for i, p := range templateFiles { resolved[i] = engine.NormalizeTemplatePath(p) } return resolved } // ... rest of resolution logic return resolved }

Also applies to: 298-383

---

313-321: Variable err is reused, shadowing the outer scope declaration.

Line 313 assigns to err which was declared at line 299. Then line 321 reassigns err again. While this works correctly, it can be confusing. Consider using distinct variable names (e.g., absErr, relErr) for clarity, similar to what's done in the earlier block (lines 238-247).

🔎 Suggested improvement

	// Resolve relative path from current working directory -absTemplatePath, err = filepath.Abs(templatePath) -if err != nil { +var absErr error +absTemplatePath, absErr = filepath.Abs(templatePath) +if absErr != nil {	// If we can't get absolute path, use original (normalized)	templatePathsForModeline[i] = engine.NormalizeTemplatePath(templatePath)	continue	}	}	// Check if the resolved path is inside root project -relPath, err := filepath.Rel(absRootDirModeline, absTemplatePath) -if err != nil { +relPath, relErr := filepath.Rel(absRootDirModeline, absTemplatePath) +if relErr != nil {	// If we can't get relative path, use original (normalized)	templatePathsForModeline[i] = engine.NormalizeTemplatePath(templatePath)	continue	}

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1433664 and 47555ff.

📒 Files selected for processing (5)

• pkg/commands/template.go
• pkg/engine/engine.go
• pkg/engine/engine_test.go
• pkg/engine/engine_windows_test.go
• pkg/engine/helm/engine_test.go

💤 Files with no reviewable changes (1)

• pkg/engine/helm/engine_test.go

🧰 Additional context used

🧬 Code graph analysis (3)

pkg/engine/engine_test.go (1)

pkg/engine/engine.go (1)

• NormalizeTemplatePath (63-65)

pkg/engine/engine_windows_test.go (1)

pkg/engine/engine.go (1)

• NormalizeTemplatePath (63-65)

pkg/commands/template.go (4)

pkg/commands/root_detection.go (1)

• ResolveSecretsPath (226-234)

pkg/commands/root.go (1)

• Config (38-73)

pkg/modeline/modeline.go (1)

• Config (12-16)

pkg/engine/engine.go (1)

• NormalizeTemplatePath (63-65)

🔇 Additional comments (7)

pkg/engine/engine_test.go (1)

19-39: Well-structured table-driven tests for path normalization.

The test cases appropriately verify that forward-slash paths pass through unchanged on Unix-like systems. This complements the Windows-specific tests in engine_windows_test.go that verify backslash conversion.

pkg/engine/engine_windows_test.go (1)

1-40: Good Windows-specific test coverage with proper build constraint.

The test cases effectively verify backslash-to-forward-slash conversion for Windows paths, including the important mixed-separator scenario. The //go:build windows constraint ensures these tests only run on Windows where filepath.ToSlash actually performs conversion.

pkg/engine/engine.go (2)

60-65: Clean implementation of path normalization.

The use of filepath.ToSlash is the correct approach for converting OS-specific path separators to forward slashes. The documentation clearly explains why this is needed for Helm engine compatibility.

---

244-245: Correct use of path.Join for Helm engine key construction.

Using path.Join (which always uses forward slashes) instead of filepath.Join (which uses OS-specific separators) is the right choice here since Helm engine map keys consistently use forward slashes. The inline comment helpfully documents this decision.

pkg/commands/template.go (3)

144-144: Good error wrapping improvement.

Using %w for error wrapping allows proper error unwrapping with errors.Is and errors.As.

---

176-178: Improved error handling for file write operations.

The explicit error message with the filename provides better debugging context when write operations fail.

---

269-270: Path normalization correctly applied to resolved paths.

The final normalization ensures forward-slash separators regardless of which resolution path was taken.

[kvaps]

LGTM

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

pkg/commands/template.go (2)

223-272: Extract duplicated path resolution logic into a helper function.

The path resolution logic (lines 223-272 and 298-384) is nearly identical, differing only in variable names to avoid shadowing. This duplication increases maintenance burden and the risk of inconsistency.

💡 Suggested refactor approach

Extract the common logic into a helper function:

// resolveAndNormalizeTemplatePaths converts template paths to normalized paths // relative to the project root, falling back to normalized originals on errors. func resolveAndNormalizeTemplatePaths(templatePaths []string, rootDir string) []string { resolved := make([]string, len(templatePaths)) absRootDir, rootErr := filepath.Abs(rootDir) if rootErr != nil { // If we can't get absolute root, normalize original paths for i, p := range templatePaths { resolved[i] = engine.NormalizeTemplatePath(p)	} return resolved	} // ... rest of the resolution logic ... return resolved }

Then call it from both locations:

// For rendering resolvedTemplateFiles := resolveAndNormalizeTemplatePaths(templateCmdFlags.templateFiles, Config.RootDir) // For modeline templatePathsForModeline := resolveAndNormalizeTemplatePaths(templateCmdFlags.templateFiles, Config.RootDir)

This eliminates ~100 lines of duplication and ensures consistent behavior.

Also applies to: 298-384

---

88-88: Typo in comment: "clinet" → "client".

🔎 Proposed fix

-// Set dummy endpoint to avoid errors on building clinet +// Set dummy endpoint to avoid errors on building client

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 47555ff and f585274.

📒 Files selected for processing (5)

• pkg/commands/template.go
• pkg/engine/engine.go
• pkg/engine/engine_test.go
• pkg/engine/engine_windows_test.go
• pkg/engine/helm/engine_test.go

💤 Files with no reviewable changes (1)

• pkg/engine/helm/engine_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• pkg/engine/engine_windows_test.go
• pkg/engine/engine_test.go
• pkg/engine/engine.go

🧰 Additional context used

🧬 Code graph analysis (1)

pkg/commands/template.go (4)

pkg/commands/root_detection.go (1)

• ResolveSecretsPath (226-234)

pkg/commands/root.go (1)

• Config (38-73)

pkg/modeline/modeline.go (1)

• Config (12-16)

pkg/engine/engine.go (1)

• NormalizeTemplatePath (64-66)

🔇 Additional comments (3)

pkg/commands/template.go (3)

144-144: LGTM: Improved error wrapping.

Using %w instead of %v enables proper error inspection with errors.Is and errors.As.

---

176-178: LGTM: Enhanced error context for file writes.

Wrapping the write error with the specific filename improves debuggability.

---

370-378: os.SameFile behavior differs across platforms.

On Unix, os.SameFile uses device and inode fields; on other systems the decision may be based on path names. If the canonical path optimization doesn't work as expected on Windows, the fallback behavior (skipping canonicalization) is acceptable and won't break functionality.