Skip to content

Commit

Permalink
Add ability to load bundles from an arbitrary filesystem
Browse files Browse the repository at this point in the history
Support OPA Client SDK programs loading bundles from an arbitraty filesystem, such as an in-memory filesystem, which unlocks additional uses that include compiling a bundle to an intermediate representation from a client program rather than the OPA command line.

Fixes #5833

bundle: Add filesystem support
Soften constraint in `Equal` method to support bundle comparison for rootless filesystems, eg treat "/file" and "file" as equal for both URLs and Paths
Add `WithPathFormat` for `DirectoryLoader` builders to centralise logic for how paths are returned during file traversal, ie in `NextFile`
Add support for specifiying the root directory for `dirLoaderFS`

compile: Add filesystem support
Add `WithFS` builder helper to pass into `initload.LoadPaths` to load bundles from a filesystem

internal/runtime/init: Add filesystem support
Pass newly supplied `fsys fs.FS` parameter in `LoadPaths` into file loader builder

loader: Add filesystem support
Add new `GetBundleDirectLoaderFS` which can load bundles from the supplied filesystem

runtime: Add filesystem support
Pass-through nil parameter as `fsys fs.FS` parameter into `initLoad.LoadPaths` (OPA servers/repls are not in scope for loading from filesystem)

util/test: Add in-memory filesystem support
Add new `WithTestFS` helper to allow tests that currently use `WithTempFS` to choose between a disk-based or memory-based filesystem - now used throughout `compile_test`

Signed-off-by: Kieran Othen <kieran.othen@mac.com>
  • Loading branch information
kjothen authored and ashutosh-narkar committed Apr 26, 2023
1 parent 1ec047c commit b65c68e
Show file tree
Hide file tree
Showing 11 changed files with 863 additions and 619 deletions.
8 changes: 6 additions & 2 deletions bundle/bundle.go
Expand Up @@ -1090,10 +1090,14 @@ func (b Bundle) Equal(other Bundle) bool {
return false
}
for i := range b.Modules {
if b.Modules[i].URL != other.Modules[i].URL {
// To support bundles built from rootless filesystems we ignore a "/" prefix
// for URLs and Paths, such that "/file" and "file" are equivalent
if strings.TrimPrefix(b.Modules[i].URL, string(filepath.Separator)) !=
strings.TrimPrefix(other.Modules[i].URL, string(filepath.Separator)) {
return false
}
if b.Modules[i].Path != other.Modules[i].Path {
if strings.TrimPrefix(b.Modules[i].Path, string(filepath.Separator)) !=
strings.TrimPrefix(other.Modules[i].Path, string(filepath.Separator)) {
return false
}
if !b.Modules[i].Parsed.Equal(other.Modules[i].Parsed) {
Expand Down
28 changes: 23 additions & 5 deletions bundle/bundle_test.go
Expand Up @@ -13,11 +13,11 @@ import (
"errors"
"fmt"
"io"

"path/filepath"
"reflect"
"strings"
"testing"
"testing/fstest"

"github.com/open-policy-agent/opa/ast"
"github.com/open-policy-agent/opa/internal/file/archive"
Expand Down Expand Up @@ -83,11 +83,15 @@ func TestManifestEqual(t *testing.T) {
}

func TestRead(t *testing.T) {
testReadBundle(t, "")
for _, useMemoryFS := range []bool{false, true} {
testReadBundle(t, "", useMemoryFS)
}
}

func TestReadWithBaseDir(t *testing.T) {
testReadBundle(t, "/foo/bar")
for _, useMemoryFS := range []bool{false, true} {
testReadBundle(t, "/foo/bar", useMemoryFS)
}
}

func TestReadWithSizeLimit(t *testing.T) {
Expand Down Expand Up @@ -162,8 +166,11 @@ func TestReadWithBundleEtag(t *testing.T) {
}
}

func testReadBundle(t *testing.T, baseDir string) {
func testReadBundle(t *testing.T, baseDir string, useMemoryFS bool) {
module := `package example`
if useMemoryFS && baseDir == "" {
baseDir = "."
}

modulePath := "/example/example.rego"
if baseDir != "" {
Expand Down Expand Up @@ -194,7 +201,18 @@ func testReadBundle(t *testing.T, baseDir string) {
}

buf := archive.MustWriteTarGz(files)
loader := NewTarballLoaderWithBaseURL(buf, baseDir)
var loader DirectoryLoader
if useMemoryFS {
fsys := make(fstest.MapFS, 1)
fsys["test.tar"] = &fstest.MapFile{Data: buf.Bytes()}
fh, err := fsys.Open("test.tar")
if err != nil {
t.Fatalf("Unexpected error: %s", err)
}
loader = NewTarballLoaderWithBaseURL(fh, baseDir)
} else {
loader = NewTarballLoaderWithBaseURL(buf, baseDir)
}
br := NewCustomReader(loader).WithBaseDir(baseDir)

bundle, err := br.Read()
Expand Down
115 changes: 80 additions & 35 deletions bundle/file.go
Expand Up @@ -7,7 +7,6 @@ import (
"fmt"
"io"
"os"
"path"
"path/filepath"
"sort"
"strings"
Expand Down Expand Up @@ -108,30 +107,37 @@ func (d *Descriptor) Close() error {
return err
}

type PathFormat int64

const (
Chrooted PathFormat = iota
SlashRooted
Passthrough
)

// DirectoryLoader defines an interface which can be used to load
// files from a directory by iterating over each one in the tree.
type DirectoryLoader interface {
// NextFile must return io.EOF if there is no next value. The returned
// descriptor should *always* be closed when no longer needed.
NextFile() (*Descriptor, error)
WithFilter(filter filter.LoaderFilter) DirectoryLoader
WithPathFormat(PathFormat) DirectoryLoader
}

type dirLoader struct {
root string
files []string
idx int
filter filter.LoaderFilter
root string
files []string
idx int
filter filter.LoaderFilter
pathFormat PathFormat
}

// NewDirectoryLoader returns a basic DirectoryLoader implementation
// that will load files from a given root directory path.
func NewDirectoryLoader(root string) DirectoryLoader {

// Normalize root directory, ex "./src/bundle" -> "src/bundle"
// We don't need an absolute path, but this makes the joined/trimmed
// paths more uniform.
func normalizeRootDirectory(root string) string {
if len(root) > 1 {
// Normalize relative directories, ex "./src/bundle" -> "src/bundle"
// We don't need an absolute path, but this makes the joined/trimmed
// paths more uniform.
if root[0] == '.' && root[1] == filepath.Separator {
if len(root) == 2 {
root = root[:1] // "./" -> "."
Expand All @@ -140,9 +146,15 @@ func NewDirectoryLoader(root string) DirectoryLoader {
}
}
}
return root
}

// NewDirectoryLoader returns a basic DirectoryLoader implementation
// that will load files from a given root directory path.
func NewDirectoryLoader(root string) DirectoryLoader {
d := dirLoader{
root: root,
root: normalizeRootDirectory(root),
pathFormat: Chrooted,
}
return &d
}
Expand All @@ -153,6 +165,36 @@ func (d *dirLoader) WithFilter(filter filter.LoaderFilter) DirectoryLoader {
return d
}

// WithPathFormat specifies how a path is formatted in a Descriptor
func (d *dirLoader) WithPathFormat(pathFormat PathFormat) DirectoryLoader {
d.pathFormat = pathFormat
return d
}

func formatPath(fileName string, root string, pathFormat PathFormat) string {
switch pathFormat {
case SlashRooted:
if !strings.HasPrefix(fileName, string(filepath.Separator)) {
return string(filepath.Separator) + fileName
}
return fileName
case Chrooted:
// Trim off the root directory and return path as if chrooted
result := strings.TrimPrefix(fileName, filepath.FromSlash(root))
if root == "." && filepath.Base(fileName) == ManifestExt {
result = fileName
}
if !strings.HasPrefix(result, string(filepath.Separator)) {
result = string(filepath.Separator) + result
}
return result
case Passthrough:
fallthrough
default:
return fileName
}
}

// NextFile iterates to the next file in the directory tree
// and returns a file Descriptor for the file.
func (d *dirLoader) NextFile() (*Descriptor, error) {
Expand Down Expand Up @@ -187,28 +229,20 @@ func (d *dirLoader) NextFile() (*Descriptor, error) {
d.idx++
fh := newLazyFile(fileName)

// Trim off the root directory and return path as if chrooted
cleanedPath := strings.TrimPrefix(fileName, filepath.FromSlash(d.root))
if d.root == "." && filepath.Base(fileName) == ManifestExt {
cleanedPath = fileName
}

if !strings.HasPrefix(cleanedPath, string(os.PathSeparator)) {
cleanedPath = string(os.PathSeparator) + cleanedPath
}

f := newDescriptor(path.Join(d.root, cleanedPath), cleanedPath, fh).withCloser(fh)
cleanedPath := formatPath(fileName, d.root, d.pathFormat)
f := newDescriptor(filepath.Join(d.root, cleanedPath), cleanedPath, fh).withCloser(fh)
return f, nil
}

type tarballLoader struct {
baseURL string
r io.Reader
tr *tar.Reader
files []file
idx int
filter filter.LoaderFilter
skipDir map[string]struct{}
baseURL string
r io.Reader
tr *tar.Reader
files []file
idx int
filter filter.LoaderFilter
skipDir map[string]struct{}
pathFormat PathFormat
}

type file struct {
Expand All @@ -221,7 +255,8 @@ type file struct {
// NewTarballLoader is deprecated. Use NewTarballLoaderWithBaseURL instead.
func NewTarballLoader(r io.Reader) DirectoryLoader {
l := tarballLoader{
r: r,
r: r,
pathFormat: Passthrough,
}
return &l
}
Expand All @@ -231,8 +266,9 @@ func NewTarballLoader(r io.Reader) DirectoryLoader {
// with the baseURL.
func NewTarballLoaderWithBaseURL(r io.Reader, baseURL string) DirectoryLoader {
l := tarballLoader{
baseURL: strings.TrimSuffix(baseURL, "/"),
r: r,
baseURL: strings.TrimSuffix(baseURL, "/"),
r: r,
pathFormat: Passthrough,
}
return &l
}
Expand All @@ -243,6 +279,12 @@ func (t *tarballLoader) WithFilter(filter filter.LoaderFilter) DirectoryLoader {
return t
}

// WithPathFormat specifies how a path is formatted in a Descriptor
func (t *tarballLoader) WithPathFormat(pathFormat PathFormat) DirectoryLoader {
t.pathFormat = pathFormat
return t
}

// NextFile iterates to the next file in the directory tree
// and returns a file Descriptor for the file.
func (t *tarballLoader) NextFile() (*Descriptor, error) {
Expand Down Expand Up @@ -329,7 +371,10 @@ func (t *tarballLoader) NextFile() (*Descriptor, error) {
f := t.files[t.idx]
t.idx++

return newDescriptor(path.Join(t.baseURL, f.name), f.name, f.reader), nil
cleanedPath := formatPath(f.name, "", t.pathFormat)
d := newDescriptor(filepath.Join(t.baseURL, cleanedPath), cleanedPath, f.reader)
return d, nil

}

// Next implements the storage.Iterator interface.
Expand Down
24 changes: 20 additions & 4 deletions bundle/filefs.go
Expand Up @@ -23,16 +23,26 @@ type dirLoaderFS struct {
files []string
idx int
filter filter.LoaderFilter
root string
pathFormat PathFormat
}

// NewFSLoader returns a basic DirectoryLoader implementation
// that will load files from a fs.FS interface
func NewFSLoader(filesystem fs.FS) (DirectoryLoader, error) {
return NewFSLoaderWithRoot(filesystem, defaultFSLoaderRoot), nil
}

// NewFSLoaderWithRoot returns a basic DirectoryLoader implementation
// that will load files from a fs.FS interface at the supplied root
func NewFSLoaderWithRoot(filesystem fs.FS, root string) DirectoryLoader {
d := dirLoaderFS{
filesystem: filesystem,
root: normalizeRootDirectory(root),
pathFormat: Chrooted,
}

return &d, nil
return &d
}

func (d *dirLoaderFS) walkDir(path string, dirEntry fs.DirEntry, err error) error {
Expand Down Expand Up @@ -67,14 +77,20 @@ func (d *dirLoaderFS) WithFilter(filter filter.LoaderFilter) DirectoryLoader {
return d
}

// WithPathFormat specifies how a path is formatted in a Descriptor
func (d *dirLoaderFS) WithPathFormat(pathFormat PathFormat) DirectoryLoader {
d.pathFormat = pathFormat
return d
}

// NextFile iterates to the next file in the directory tree
// and returns a file Descriptor for the file.
func (d *dirLoaderFS) NextFile() (*Descriptor, error) {
d.Lock()
defer d.Unlock()

if d.files == nil {
err := fs.WalkDir(d.filesystem, defaultFSLoaderRoot, d.walkDir)
err := fs.WalkDir(d.filesystem, d.root, d.walkDir)
if err != nil {
return nil, fmt.Errorf("failed to list files: %w", err)
}
Expand All @@ -94,7 +110,7 @@ func (d *dirLoaderFS) NextFile() (*Descriptor, error) {
return nil, fmt.Errorf("failed to open file %s: %w", fileName, err)
}

fileNameWithSlash := fmt.Sprintf("/%s", fileName)
f := newDescriptor(fileNameWithSlash, fileNameWithSlash, fh).withCloser(fh)
cleanedPath := formatPath(fileName, d.root, d.pathFormat)
f := newDescriptor(cleanedPath, cleanedPath, fh).withCloser(fh)
return f, nil
}
10 changes: 9 additions & 1 deletion compile/compile.go
Expand Up @@ -12,6 +12,7 @@ import (
"errors"
"fmt"
"io"
"io/fs"
"path/filepath"
"regexp"
"sort"
Expand Down Expand Up @@ -79,6 +80,7 @@ type Compiler struct {
bsc *bundle.SigningConfig // represents the key configuration used to generate a signed bundle
keyID string // represents the name of the default key used to verify a signed bundle
metadata *map[string]interface{} // represents additional data included in .manifest file
fsys fs.FS // file system to use when loading paths
}

// New returns a new compiler instance that can be invoked.
Expand Down Expand Up @@ -220,6 +222,12 @@ func (c *Compiler) WithMetadata(metadata *map[string]interface{}) *Compiler {
return c
}

// WithFS sets the file system to use when loading paths
func (c *Compiler) WithFS(fsys fs.FS) *Compiler {
c.fsys = fsys
return c
}

func addEntrypointsFromAnnotations(c *Compiler, ar []*ast.AnnotationsRef) error {
for _, ref := range ar {
var entrypoint ast.Ref
Expand Down Expand Up @@ -410,7 +418,7 @@ func (c *Compiler) initBundle() error {
// TODO(tsandall): the metrics object should passed through here so we that
// we can track read and parse times.

load, err := initload.LoadPaths(c.paths, c.filter, c.asBundle, c.bvc, false, c.useRegoAnnotationEntrypoints, c.capabilities)
load, err := initload.LoadPaths(c.paths, c.filter, c.asBundle, c.bvc, false, c.useRegoAnnotationEntrypoints, c.capabilities, c.fsys)
if err != nil {
return fmt.Errorf("load error: %w", err)
}
Expand Down

0 comments on commit b65c68e

Please sign in to comment.