stefanprodan / timoni

Timoni is a package manager for Kubernetes, powered by CUE and inspired by Helm.
https://timoni.sh
Apache License 2.0
1.58k stars 70 forks source link

[bundle] Fix local module refs for multi-cluster #429

Closed huguesalary closed 1 month ago

huguesalary commented 1 month ago

This bug is caused by the relationship between the following components:

When called, the InitWorkspace goes through the list of files stored in b.files, parses them and writes the result in new files (let's call them "result files") located under the path indicated by workspace. In parallel, b.mapSourceToOrigin maps a "result file" path to the original file the result is based on.

At the end of the function, the original list b.files is replaced with the list of "result files".

The next invocation of InitWorkspace then uses b.files again, but this time it contains the previously computed "result files" paths.

So, for a given:

calling InitWorkspace("/tmp/workspace1/"), will yield:

Then calling InitWorkspace("/tmp/workspace2/"), will yield:

The getInstanceUrl relies on the mapSourceToOrigin being accurate to propely resolve relative file:// paths.

When getInstanceUrl() is called for the first bundle, it will propely resolve ./examples/redis/ to /path/to/a/examples/redis/.

When getInstanceUrl() is called for the second bundle, it will erroneously resolve ./examples/redis/ to /tmp/workspace1/examples/redis/.

This commit fixes this.

However it is worth noting that using a relative path for a file:// URI feels strange. Should it be allowed in the first place?

Fix #364

huguesalary commented 1 month ago

This fix doesn't work if -f - is used instead of an actual path to a file.

stefanprodan commented 1 month ago

However it is worth noting that using a relative path for a file:// URI feels strange. Should it be allowed in the first place?

This is how many tools work, for example Docker Compose, so I don't find it strange but actually useful.