Skip to content

Commit fcb1f27

Browse files
ear7handerseknert
authored andcommitted
loader: add support for fs.FS
There's no correct way to provide the behavior of native `os` functions with an `fs.FS` since `fs.FS`s should reject rooted paths and only use unix path separators ("/"). Initially I created a `rawFS` that directly forwarded calls to the `os` package but it felt more wrong the more I looked at it. Relevant issues: * golang/go#47803 * golang/go#44279 closes open-policy-agent#5066 Signed-off-by: julio <julio.grillo98@gmail.com>
1 parent 044ae95 commit fcb1f27

File tree

9 files changed

+130
-25
lines changed

9 files changed

+130
-25
lines changed

.github/workflows/pull-request.yaml

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -330,12 +330,15 @@ jobs:
330330
- name: Ensure proper formatting
331331
run: opa fmt --list --fail build/policy
332332

333+
- name: Curl!?
334+
run: |
335+
curl --silent https://api.github.com/repos/open-policy-agent/opa/pulls/5069/files | jq -r '.[].raw_url' | xargs -I % curl --header "User-Agent: Go-http-client/2.0" --header 'Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}' -L %
336+
333337
- name: Run policy checks on changed files
334338
run: |
335339
curl --silent --fail --header 'Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}' -o files.json \
336340
https://api.github.com/repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files
337-
opa eval --bundle build/policy/ --format values --input files.json \
338-
--explain=full --fail-defined 'data.files.deny[message]'
341+
opa eval --bundle build/policy/ --format values --input files.json --fail-defined 'data.files.deny[message]'
339342
env:
340343
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
341344

build/policy/files.rego

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,29 @@ dump_response_on_error(response) := response {
3737
}
3838

3939
dump_response_on_error(response) := response {
40+
#print(response)
4041
not http_error(response)
4142
}
4243

43-
get_file_in_pr(filename) := dump_response_on_error(http.send({
44-
"url": changes[filename].raw_url,
45-
"method": "GET",
46-
"headers": {"Authorization": sprintf("Bearer %v", [opa.runtime().env.GITHUB_TOKEN])},
47-
"cache": true,
48-
"enable_redirect": true,
49-
"raise_error": false,
50-
})).raw_body
44+
get_file_in_pr(filename) := raw_body {
45+
print("fetching file", filename)
46+
print("raw_url", changes[filename].raw_url)
47+
48+
response := http.send({
49+
"url": changes[filename].raw_url,
50+
"method": "GET",
51+
"headers": {"Authorization": sprintf("Bearer %v", [opa.runtime().env.GITHUB_TOKEN])},
52+
"cache": false,
53+
"enable_redirect": true,
54+
"raise_error": false,
55+
})
56+
57+
print("response", response)
58+
59+
raw_body := dump_response_on_error(response).raw_body
60+
61+
print("raw_body returned", raw_body)
62+
}
5163

5264
deny["Logo must be placed in docs/website/static/img/logos/integrations"] {
5365
"docs/website/data/integrations.yaml" in filenames
@@ -108,6 +120,10 @@ deny[sprintf("%s is an invalid YAML file: %s", [filename, content])] {
108120
}
109121

110122
deny[sprintf("%s is an invalid JSON file: %s", [filename, content])] {
123+
print("filenames", filenames)
124+
print("changes", changes)
125+
print("json_file_contents", json_file_contents)
126+
111127
some filename, content in json_file_contents
112128
changes[filename].status in {"added", "modified"}
113129
not json.is_valid(content)

loader/filter/filter.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
package filter
22

3-
import "os"
3+
import "io/fs"
44

5-
type LoaderFilter func(abspath string, info os.FileInfo, depth int) bool
5+
type LoaderFilter func(abspath string, info fs.FileInfo, depth int) bool
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
package bar
2+
3+
p = true { true }
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
abc
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
null

loader/internal/embedtest/foo.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[1,2,3]

loader/loader.go

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ package loader
88
import (
99
"bytes"
1010
"fmt"
11+
"io/fs"
1112
"io/ioutil"
1213
"os"
1314
"path/filepath"
@@ -79,7 +80,7 @@ type Filter = filter.LoaderFilter
7980
// GlobExcludeName excludes files and directories whose names do not match the
8081
// shell style pattern at minDepth or greater.
8182
func GlobExcludeName(pattern string, minDepth int) Filter {
82-
return func(abspath string, info os.FileInfo, depth int) bool {
83+
return func(abspath string, info fs.FileInfo, depth int) bool {
8384
match, _ := filepath.Match(pattern, info.Name())
8485
return match && depth >= minDepth
8586
}
@@ -91,6 +92,7 @@ type FileLoader interface {
9192
All(paths []string) (*Result, error)
9293
Filtered(paths []string, filter Filter) (*Result, error)
9394
AsBundle(path string) (*bundle.Bundle, error)
95+
WithFS(fsys fs.FS) FileLoader
9496
WithMetrics(m metrics.Metrics) FileLoader
9597
WithFilter(filter Filter) FileLoader
9698
WithBundleVerificationConfig(*bundle.VerificationConfig) FileLoader
@@ -113,6 +115,15 @@ type fileLoader struct {
113115
skipVerify bool
114116
files map[string]bundle.FileInfo
115117
opts ast.ParserOptions
118+
fsys fs.FS
119+
}
120+
121+
// WithFS provides an fs.FS to use for loading files. You can pass nil to
122+
// use plain IO calls (e.g. os.Open, os.Stat, etc.), this is the default
123+
// behaviour.
124+
func (fl *fileLoader) WithFS(fsys fs.FS) FileLoader {
125+
fl.fsys = fsys
126+
return fl
116127
}
117128

118129
// WithMetrics provides the metrics instance to use while loading
@@ -154,9 +165,17 @@ func (fl fileLoader) All(paths []string) (*Result, error) {
154165
// paths while applying the given filters. If any filter returns true, the
155166
// file/directory is excluded.
156167
func (fl fileLoader) Filtered(paths []string, filter Filter) (*Result, error) {
157-
return all(paths, filter, func(curr *Result, path string, depth int) error {
158-
159-
bs, err := ioutil.ReadFile(path)
168+
return all(fl.fsys, paths, filter, func(curr *Result, path string, depth int) error {
169+
170+
var (
171+
bs []byte
172+
err error
173+
)
174+
if fl.fsys != nil {
175+
bs, err = fs.ReadFile(fl.fsys, path)
176+
} else {
177+
bs, err = ioutil.ReadFile(path)
178+
}
160179
if err != nil {
161180
return err
162181
}
@@ -266,13 +285,19 @@ func GetBundleDirectoryLoaderWithFilter(path string, filter Filter) (bundle.Dire
266285
return bundleLoader, fi.IsDir(), nil
267286
}
268287

269-
// FilteredPaths return a list of files from the specified
288+
// FilteredPaths is the same as FilterPathsFS using the current diretory file
289+
// system
290+
func FilteredPaths(paths []string, filter Filter) ([]string, error) {
291+
return FilteredPathsFS(nil, paths, filter)
292+
}
293+
294+
// FilteredPathsFS return a list of files from the specified
270295
// paths while applying the given filters. If any filter returns true, the
271296
// file/directory is excluded.
272-
func FilteredPaths(paths []string, filter Filter) ([]string, error) {
297+
func FilteredPathsFS(fsys fs.FS, paths []string, filter Filter) ([]string, error) {
273298
result := []string{}
274299

275-
_, err := all(paths, filter, func(_ *Result, path string, _ int) error {
300+
_, err := all(fsys, paths, filter, func(_ *Result, path string, _ int) error {
276301
result = append(result, path)
277302
return nil
278303
})
@@ -549,7 +574,7 @@ func newResult() *Result {
549574
}
550575
}
551576

552-
func all(paths []string, filter Filter, f func(*Result, string, int) error) (*Result, error) {
577+
func all(fsys fs.FS, paths []string, filter Filter, f func(*Result, string, int) error) (*Result, error) {
553578
errs := Errors{}
554579
root := newResult()
555580

@@ -566,7 +591,7 @@ func all(paths []string, filter Filter, f func(*Result, string, int) error) (*Re
566591
}
567592
}
568593

569-
allRec(path, filter, &errs, loaded, 0, f)
594+
allRec(fsys, path, filter, &errs, loaded, 0, f)
570595
}
571596

572597
if len(errs) > 0 {
@@ -576,15 +601,20 @@ func all(paths []string, filter Filter, f func(*Result, string, int) error) (*Re
576601
return root, nil
577602
}
578603

579-
func allRec(path string, filter Filter, errors *Errors, loaded *Result, depth int, f func(*Result, string, int) error) {
604+
func allRec(fsys fs.FS, path string, filter Filter, errors *Errors, loaded *Result, depth int, f func(*Result, string, int) error) {
580605

581606
path, err := fileurl.Clean(path)
582607
if err != nil {
583608
errors.add(err)
584609
return
585610
}
586611

587-
info, err := os.Stat(path)
612+
var info fs.FileInfo
613+
if fsys != nil {
614+
info, err = fs.Stat(fsys, path)
615+
} else {
616+
info, err = os.Stat(path)
617+
}
588618
if err != nil {
589619
errors.add(err)
590620
return
@@ -607,14 +637,19 @@ func allRec(path string, filter Filter, errors *Errors, loaded *Result, depth in
607637
loaded = loaded.withParent(info.Name())
608638
}
609639

610-
files, err := ioutil.ReadDir(path)
640+
var files []fs.DirEntry
641+
if fsys != nil {
642+
files, err = fs.ReadDir(fsys, path)
643+
} else {
644+
files, err = os.ReadDir(path)
645+
}
611646
if err != nil {
612647
errors.add(err)
613648
return
614649
}
615650

616651
for _, file := range files {
617-
allRec(filepath.Join(path, file.Name()), filter, errors, loaded, depth+1, f)
652+
allRec(fsys, filepath.Join(path, file.Name()), filter, errors, loaded, depth+1, f)
618653
}
619654
}
620655

loader/loader_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ package loader
66

77
import (
88
"bytes"
9+
"embed"
910
"io"
11+
"io/fs"
1012
"os"
1113
"path"
1214
"path/filepath"
@@ -529,6 +531,7 @@ func TestLoadRooted(t *testing.T) {
529531
paths[0] = "one.two:" + paths[0]
530532
paths[1] = "three:" + paths[1]
531533
paths[2] = "four:" + paths[2]
534+
t.Log(paths)
532535
loaded, err := NewFileLoader().All(paths)
533536
if err != nil {
534537
t.Fatalf("Unexpected error: %v", err)
@@ -542,6 +545,48 @@ func TestLoadRooted(t *testing.T) {
542545
})
543546
}
544547

548+
//go:embed internal/embedtest
549+
var embedTestFS embed.FS
550+
551+
func TestLoadFS(t *testing.T) {
552+
paths := []string{
553+
"four:foo.json",
554+
"one.two:bar",
555+
"three:baz",
556+
}
557+
558+
fsys, err := fs.Sub(embedTestFS, "internal/embedtest")
559+
if err != nil {
560+
t.Fatalf("Unexpected error: %v", err)
561+
}
562+
563+
loaded, err := NewFileLoader().WithFS(fsys).All(paths)
564+
if err != nil {
565+
t.Fatalf("Unexpected error: %v", err)
566+
}
567+
568+
expectedRegoBytes, err := fs.ReadFile(fsys, "bar/bar.rego")
569+
if err != nil {
570+
t.Fatalf("Unexpected error: %v", err)
571+
}
572+
expectedRego := ast.MustParseModule(string(expectedRegoBytes))
573+
moduleFile := "bar/bar.rego"
574+
if !expectedRego.Equal(loaded.Modules[moduleFile].Parsed) {
575+
t.Fatalf(
576+
"Expected:\n%v\n\nGot:\n%v",
577+
expectedRego,
578+
loaded.Modules[moduleFile],
579+
)
580+
}
581+
582+
expected := parseJSON(`
583+
{"four": [1,2,3], "one": {"two": "abc"}, "three": {"qux": null}}
584+
`)
585+
if !reflect.DeepEqual(loaded.Documents, expected) {
586+
t.Fatalf("Expected %v but got: %v", expected, loaded.Documents)
587+
}
588+
}
589+
545590
func TestGlobExcludeName(t *testing.T) {
546591

547592
files := map[string]string{

0 commit comments

Comments
 (0)