Skip to content

Commit

Permalink
bundle: Use full system path for bundle dir
Browse files Browse the repository at this point in the history
When reading a bundle from a directory we should be using the full
system path for the module files. This helps greatly with reducing
complexity of trying to read error messages. It also makes the
integration with tools like VSCode work better as they can provide
links from the console output file paths to the file in question.

This will not affect bundles loaded from tarballs.

Fixes: #1796
Signed-off-by: Patrick East <east.patrick@gmail.com>
  • Loading branch information
patrick-east committed Dec 18, 2019
1 parent 2699974 commit 3d8389e
Show file tree
Hide file tree
Showing 4 changed files with 57 additions and 7 deletions.
26 changes: 21 additions & 5 deletions bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ type Reader struct {
loader DirectoryLoader
includeManifestInData bool
metrics metrics.Metrics
baseDir string
}

// NewReader returns a new Reader which is configured for reading tarballs.
Expand Down Expand Up @@ -157,6 +158,13 @@ func (r *Reader) WithMetrics(m metrics.Metrics) *Reader {
return r
}

// WithBaseDir sets a base directory for file paths of loaded Rego
// modules. This will *NOT* affect the loaded path of data files.
func (r *Reader) WithBaseDir(dir string) *Reader {
r.baseDir = dir
return r
}

// Read returns a new Bundle loaded from the reader.
func (r *Reader) Read() (Bundle, error) {

Expand Down Expand Up @@ -186,18 +194,19 @@ func (r *Reader) Read() (Bundle, error) {
path := filepath.ToSlash(f.Path())

if strings.HasSuffix(path, RegoExt) {
fullPath := r.fullPath(path)
r.metrics.Timer(metrics.RegoModuleParse).Start()
module, err := ast.ParseModule(path, buf.String())
module, err := ast.ParseModule(fullPath, buf.String())
r.metrics.Timer(metrics.RegoModuleParse).Stop()
if err != nil {
return bundle, err
}
if module == nil {
return bundle, fmt.Errorf("module '%s' is empty", path)
return bundle, fmt.Errorf("module '%s' is empty", fullPath)
}

mf := ModuleFile{
Path: path,
Path: fullPath,
Raw: buf.Bytes(),
Parsed: module,
}
Expand All @@ -211,7 +220,7 @@ func (r *Reader) Read() (Bundle, error) {
r.metrics.Timer(metrics.RegoDataParse).Stop()

if err != nil {
return bundle, errors.Wrapf(err, "bundle load failed on %v", path)
return bundle, errors.Wrapf(err, "bundle load failed on %v", r.fullPath(path))
}

if err := insertValue(&bundle, path, value); err != nil {
Expand All @@ -227,7 +236,7 @@ func (r *Reader) Read() (Bundle, error) {
r.metrics.Timer(metrics.RegoDataParse).Stop()

if err != nil {
return bundle, errors.Wrapf(err, "bundle load failed on %v", path)
return bundle, errors.Wrapf(err, "bundle load failed on %v", r.fullPath(path))
}

if err := insertValue(&bundle, path, value); err != nil {
Expand Down Expand Up @@ -268,6 +277,13 @@ func (r *Reader) Read() (Bundle, error) {
return bundle, nil
}

func (r *Reader) fullPath(path string) string {
if r.baseDir != "" {
path = filepath.Join(r.baseDir, path)
}
return path
}

// Write serializes the Bundle and writes it to w.
func Write(w io.Writer, bundle Bundle) error {
gw := gzip.NewWriter(w)
Expand Down
20 changes: 18 additions & 2 deletions bundle/bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"bytes"
"compress/gzip"
"encoding/json"
"path/filepath"
"strings"
"testing"

Expand All @@ -16,7 +17,14 @@ import (
)

func TestRead(t *testing.T) {
testReadBundle(t, "")
}

func TestReadWithBaseDir(t *testing.T) {
testReadBundle(t, "/foo/bar")
}

func testReadBundle(t *testing.T, baseDir string) {
files := [][2]string{
{"/a/b/c/data.json", "[1,2,3]"},
{"/a/b/d/data.json", "true"},
Expand All @@ -26,13 +34,21 @@ func TestRead(t *testing.T) {
}

buf := archive.MustWriteTarGz(files)
bundle, err := NewReader(buf).Read()
loader := NewTarballLoader(buf)
br := NewCustomReader(loader).WithBaseDir(baseDir)

bundle, err := br.Read()
if err != nil {
t.Fatal(err)
}

module := `package example`

modulePath := "/example/example.rego"
if baseDir != "" {
modulePath = filepath.Join(baseDir, modulePath)
}

exp := Bundle{
Data: map[string]interface{}{
"a": map[string]interface{}{
Expand All @@ -51,7 +67,7 @@ func TestRead(t *testing.T) {
},
Modules: []ModuleFile{
{
Path: "/example/example.rego",
Path: modulePath,
Parsed: ast.MustParseModule(module),
Raw: []byte(module),
},
Expand Down
8 changes: 8 additions & 0 deletions loader/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,10 +167,18 @@ func (fl fileLoader) AsBundle(path string) (*bundle.Bundle, error) {
}

br := bundle.NewCustomReader(bundleLoader).WithMetrics(fl.metrics)

// For bundle directories add the full path in front of module file names
// to simplify debugging.
if fi.IsDir() {
br.WithBaseDir(path)
}

b, err := br.Read()
if err != nil {
err = errors.Wrap(err, fmt.Sprintf("bundle %s", path))
}

return &b, err
}

Expand Down
10 changes: 10 additions & 0 deletions loader/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,16 @@ func TestAsBundleWithDir(t *testing.T) {
t.Fatalf("expected 2 modules, got %d", len(b.Modules))
}

expectedModulePaths := map[string]struct{}{
filepath.Join(rootDir, "foo/policy.rego"): {},
filepath.Join(rootDir, "base.rego"): {},
}
for _, mf := range b.Modules {
if _, found := expectedModulePaths[mf.Path]; !found {
t.Errorf("Unexpected module file with path %s in bundle modules", mf.Path)
}
}

expectedData := util.MustUnmarshalJSON([]byte(`{"foo": [1,2,3]}`))
if !reflect.DeepEqual(b.Data, expectedData) {
t.Fatalf("expected data %+v, got %+v", expectedData, b.Data)
Expand Down

0 comments on commit 3d8389e

Please sign in to comment.