Skip to content

Commit

Permalink
Fix #364
Browse files Browse the repository at this point in the history
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?
  • Loading branch information
huguesalary committed Oct 3, 2024
1 parent cf22fe3 commit a858e4e
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 7 deletions.
2 changes: 1 addition & 1 deletion cmd/timoni/bundle_apply.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func runBundleApplyCmd(cmd *cobra.Command, _ []string) error {
return describeErr(workspace, "failed to parse bundle", err)
}

v, err := bm.Build()
v, err := bm.Build(workspace)
if err != nil {
return describeErr(tmpDir, "failed to build bundle", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/timoni/bundle_build.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func runBundleBuildCmd(cmd *cobra.Command, _ []string) error {
return describeErr(tmpDir, "failed to parse bundle", err)
}

v, err := bm.Build()
v, err := bm.Build(tmpDir)
if err != nil {
return describeErr(tmpDir, "failed to build bundle", err)
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/timoni/bundle_vet.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func runBundleVetCmd(cmd *cobra.Command, args []string) error {
return describeErr(workspace, "failed to parse bundle", err)
}

v, err := bm.Build()
v, err := bm.Build(workspace)
if err != nil {
return describeErr(workspace, "failed to build bundle", err)
}
Expand Down
10 changes: 6 additions & 4 deletions internal/engine/bundle_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import (
type BundleBuilder struct {
ctx *cue.Context
files []string
workspaceFiles map[string][]string
mapSourceToOrigin map[string]string
injector *RuntimeInjector
}
Expand All @@ -63,6 +64,7 @@ func NewBundleBuilder(ctx *cue.Context, files []string) *BundleBuilder {
b := &BundleBuilder{
ctx: ctx,
files: files,
workspaceFiles: make(map[string][]string),
mapSourceToOrigin: make(map[string]string, len(files)),
injector: NewRuntimeInjector(ctx),
}
Expand Down Expand Up @@ -116,26 +118,26 @@ func (b *BundleBuilder) InitWorkspace(workspace string, runtimeValues map[string
files = append(files, dstFile)
}

schemaFile := filepath.Join(workspace, fmt.Sprintf("%v.schema.cue", len(b.files)+1))
schemaFile := filepath.Join(workspace, fmt.Sprintf("%v.schema.cue", len(b.workspaceFiles[workspace])+1))
files = append(files, schemaFile)
if err := os.WriteFile(schemaFile, []byte(apiv1.BundleSchema), os.ModePerm); err != nil {
return err
}

b.files = files
b.workspaceFiles[workspace] = files
return nil
}

// Build builds a CUE instance for the specified files and returns the CUE value.
// A workspace must be initialised with InitWorkspace before calling this function.
func (b *BundleBuilder) Build() (cue.Value, error) {
func (b *BundleBuilder) Build(workspace string) (cue.Value, error) {
var value cue.Value
cfg := &load.Config{
Package: "_",
DataFiles: true,
}

ix := load.Instances(b.files, cfg)
ix := load.Instances(b.workspaceFiles[workspace], cfg)
if len(ix) == 0 {
return value, fmt.Errorf("no instances found")
}
Expand Down

0 comments on commit a858e4e

Please sign in to comment.