[bundle] Fix local module refs for multi-cluster

[huguesalary]

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

• The func (b *BundleBuilder) InitWorkspace(workspace string, runtimeValues map[string]string) error function in
  internal/engine/bundle_builder.go
• The BundleBuilder.files field
• The BundleBuilder.mapSourceToOrigin field
• The func (b *BundleBuilder) getInstanceUrl(v cue.Value) string
  function in internal/engine/bundle_builder.go

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:

• b.files == ["/path/to/a/bundle.cue"]

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

• b.files == ["/tmp/workspace1/bundle.cue"]
• b.mapSourceToOrigin["/tmp/workspace1/bundle.cue"]="/path/to/a/bundle.cue"

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

• b.files == ["/tmp/workspace2/bundle.cue"]
• b.mapSourceToOrigin["/tmp/workspace2/bundle.cue"]="/tmp/workspace1/bundle.cue"

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]

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

[stefanprodan]

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.

[stefanprodan]

We can't use CWD here, the relative path is to the bundle file (what v.Pos().Filename() does) instead of the working dir of the CLI. You can see the e2e tests failing for:

timoni bundle build -f hack/tests/nginx_bundle.cue --runtime-from-env

ERR module not found at path /home/runner/work/blueprints/starter

[huguesalary]

We can't use CWD here, the relative path is to the bundle file (what v.Pos().Filename() does) instead of the working dir of the CLI.

Ha ok, I understand the logic better now, thanks.

This raises the issue that when the bundle file comes from Stdin (-f -), the v.Pos().Filename() becomes the temporary directory of the file Stdin was written to. Which makes the getInstanceUrl function fail again, since the path is now wrong.

I'm also unclear what will happen when someone specifies both a bundle via Stdin and a file (-f- -f bundle.cue).

I'll look into that.

[stefanprodan]

I think we can error out if the bundle comes from stdin and contains local refs.

[huguesalary]

Sounds good.

I removed the cwd code and added 4 new tests:

• bundle apply with file from stdin (-f-) and module: url: file://./relative/path/to/module
• bundle apply with file from stdin (-f-) and module: url: file:///absolute/path/to/module
• bundle apply with file from concrete file (-f /path/to/bundle.cue) and module: url: file://./relative/path/to/module
• bundle apply with file from concrete file (-f /path/to/bundle.cue) and module: url: file:///absolute/path/to/module

How's that?

edit: also fixed the RuntimeBuilder. Happy to make a different PR if preferred.

[stefanprodan]

LGTM

Thanks @huguesalary 🥇