Skip to content

Handle Helm API output which uses non-OS specific path package#42

Open
HarrisonWAffel wants to merge 1 commit intorancher-archives:mainfrom
HarrisonWAffel:handle-path-output
Open

Handle Helm API output which uses non-OS specific path package#42
HarrisonWAffel wants to merge 1 commit intorancher-archives:mainfrom
HarrisonWAffel:handle-path-output

Conversation

@HarrisonWAffel
Copy link
Copy Markdown

@HarrisonWAffel HarrisonWAffel commented Oct 24, 2023

While running chart tests on windows, I noticed that hull would error out with the following log:

panic: runtime error: index out of range [1] with length 1 [recovered]
	panic: runtime error: index out of range [1] with length 1

goroutine 7 [running]:
testing.tRunner.func1.2({0x244fca0, 0xc0002f9ec0})
	C:/Program Files/Go/src/testing/testing.go:1545 +0x238
testing.tRunner.func1()
	C:/Program Files/Go/src/testing/testing.go:1548 +0x397
panic({0x244fca0?, 0xc0002f9ec0?})
	C:/Program Files/Go/src/runtime/panic.go:914 +0x21f
github.com/rancher/hull/pkg/chart.(*chart).RenderTemplate(0xc0002b37d0, 0xc000410fc0?)
	C:/Users/adminuser/go/pkg/mod/github.com/rancher/hull@v0.0.0-20230424152137-627ef5347afd/pkg/chart/chart.go:84 +0x7ff
github.com/rancher/hull/pkg/test.(*Suite).Run.func1(0xc0001d1a00)
	C:/Users/adminuser/go/pkg/mod/github.com/rancher/hull@v0.0.0-20230424152137-627ef5347afd/pkg/test/suite.go:124 +0x78
testing.tRunner(0xc0001d1a00, 0xc00049cdc0)
	C:/Program Files/Go/src/testing/testing.go:1595 +0xff

After investigating, this issue seems to stem from how the Helm API returns charts after calling c.Render within Hulls RenderTemplate function.

As the Helm API recurses through the chart files, it builds out the file paths using the path library. Unfortunately, the path library does not take into account OS specific file separators, only path/filepath does.

Hull will use this output to further render only the yaml files so that they may be tested. However, Hull is (properly) using the filepath.Seperator constant to split template names from their paths to identify the yaml files. By splitting the value using the filepath.Seperator constant, as well as the immediate indexing into the resulting slice, we run into the above error.

The fix proposed in this PR is to no longer use the filepath.Seperator constant, as Helm will always return linux path separators. This results in Hull tests working as expected on Windows, and does not impact linux/macos. Changing this in Helm itself would likely be a major breaking change, as this logic has existed for ~7 years.

@HarrisonWAffel HarrisonWAffel changed the title handle Hull API output which relies on non-OS specific path package Handle Hull API output which relies on non-OS specific path package Oct 24, 2023
@HarrisonWAffel HarrisonWAffel changed the title Handle Hull API output which relies on non-OS specific path package Handle Helm API output uses non-OS specific path package Oct 24, 2023
@HarrisonWAffel HarrisonWAffel changed the title Handle Helm API output uses non-OS specific path package Handle Helm API output which uses non-OS specific path package Oct 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant