Skip to content

Commit df0addf

Browse files
yogisinhaashutosh-narkar
authored andcommitted
Changed the LoadPaths function.
Squashing all the commits for following issue: Fix for the issue when OPA doesnot load tarball on cmd line as a bundle. Fixes #5879 Signed-off-by: Yogesh Sinha <[email protected]>
1 parent dea7c10 commit df0addf

File tree

2 files changed

+264
-3
lines changed

2 files changed

+264
-3
lines changed

internal/runtime/init/init.go

+26-3
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,16 @@ func LoadPaths(paths []string,
127127
caps = ast.CapabilitiesForThisVersion()
128128
}
129129

130+
// tar.gz files are automatically loaded as bundles
131+
var likelyBundles, nonBundlePaths []string
132+
if !asBundle {
133+
likelyBundles, nonBundlePaths = splitByTarGzExt(paths)
134+
paths = likelyBundles
135+
}
136+
130137
var result LoadPathsResult
131138
var err error
132-
133-
if asBundle {
139+
if asBundle || len(likelyBundles) > 0 {
134140
result.Bundles = make(map[string]*bundle.Bundle, len(paths))
135141
for _, path := range paths {
136142
result.Bundles[path], err = loader.NewFileLoader().
@@ -145,14 +151,18 @@ func LoadPaths(paths []string,
145151
return nil, err
146152
}
147153
}
154+
}
155+
156+
if len(nonBundlePaths) == 0 {
148157
return &result, nil
149158
}
150159

151160
files, err := loader.NewFileLoader().
152161
WithFS(fsys).
153162
WithProcessAnnotation(processAnnotations).
154163
WithCapabilities(caps).
155-
Filtered(paths, filter)
164+
Filtered(nonBundlePaths, filter)
165+
156166
if err != nil {
157167
return nil, err
158168
}
@@ -162,6 +172,19 @@ func LoadPaths(paths []string,
162172
return &result, nil
163173
}
164174

175+
// splitByTarGzExt splits the paths in 2 groups. Ones with .tar.gz and another with
176+
// non .tar.gz extensions.
177+
func splitByTarGzExt(paths []string) (targzs []string, nonTargzs []string) {
178+
for _, path := range paths {
179+
if strings.HasSuffix(path, ".tar.gz") {
180+
targzs = append(targzs, path)
181+
} else {
182+
nonTargzs = append(nonTargzs, path)
183+
}
184+
}
185+
return
186+
}
187+
165188
// WalkPaths reads data and policy from the given paths and returns a set of bundle directory loaders
166189
// or descriptors that contain information about files.
167190
func WalkPaths(paths []string, filter loader.Filter, asBundle bool) (*WalkPathsResult, error) {

internal/runtime/init/init_test.go

+238
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
package init
66

77
import (
8+
"bytes"
89
"context"
10+
"encoding/json"
911
"io"
1012
"io/fs"
1113
"os"
@@ -14,6 +16,9 @@ import (
1416
"strings"
1517
"testing"
1618

19+
"github.com/open-policy-agent/opa/ast"
20+
"github.com/open-policy-agent/opa/bundle"
21+
"github.com/open-policy-agent/opa/internal/file/archive"
1722
"github.com/open-policy-agent/opa/loader"
1823
"github.com/open-policy-agent/opa/storage"
1924
inmem "github.com/open-policy-agent/opa/storage/inmem/test"
@@ -183,6 +188,239 @@ p = true { 1 = 2 }`
183188
}
184189
}
185190

191+
func TestLoadTarGzsInBundleAndNonBundleMode(t *testing.T) {
192+
193+
type bundleInfo struct {
194+
fileName string
195+
files [][2]string
196+
expBundle bundle.Bundle
197+
}
198+
199+
bundle1TarGz := bundleInfo{
200+
fileName: "bundle1.tar.gz",
201+
files: [][2]string{
202+
{"/a/data.json", `{"foo": "bar1", "x": {"y": {"z": [1]}}}`},
203+
{"/a/.manifest", `{"roots": ["a"]}`},
204+
},
205+
expBundle: bundle.Bundle{
206+
Manifest: bundle.Manifest{
207+
Roots: &[]string{"a"},
208+
},
209+
Data: map[string]interface{}{
210+
"a": map[string]interface{}{
211+
"foo": "bar1",
212+
"x": map[string]interface{}{
213+
"y": map[string]interface{}{
214+
"z": []interface{}{json.Number("1")},
215+
},
216+
},
217+
},
218+
},
219+
},
220+
}
221+
222+
bundle2TarGz := bundleInfo{
223+
fileName: "bundle2.tar.gz",
224+
files: [][2]string{
225+
{"/b/data.json", `{"foo": "bar2", "x": {"y": {"z": [1]}}}`},
226+
{"/b/.manifest", `{"roots": ["b"]}`},
227+
},
228+
expBundle: bundle.Bundle{
229+
Manifest: bundle.Manifest{
230+
Roots: &[]string{"b"},
231+
},
232+
Data: map[string]interface{}{
233+
"b": map[string]interface{}{
234+
"foo": "bar2",
235+
"x": map[string]interface{}{
236+
"y": map[string]interface{}{
237+
"z": []interface{}{json.Number("1")},
238+
},
239+
},
240+
},
241+
},
242+
},
243+
}
244+
245+
bundle1Folder := map[string]string{
246+
"/bundle1/a/data.json": `{"foo1": "bar2", "x": {"y": {"z": [2]}}}`,
247+
"/bundle1/a/.manifest": `{"roots": ["a"]}`,
248+
"/bundle1/a/foo.rego": `package a.b.y`,
249+
}
250+
251+
modulePath := "/bundle1/a/foo.rego"
252+
module := `package a.b.y`
253+
254+
bundle1FolderInfo := bundleInfo{
255+
fileName: "bundle1",
256+
expBundle: bundle.Bundle{
257+
Manifest: bundle.Manifest{
258+
Roots: &[]string{"a"},
259+
},
260+
Data: map[string]interface{}{
261+
"a": map[string]interface{}{
262+
"foo1": "bar2",
263+
"x": map[string]interface{}{
264+
"y": map[string]interface{}{
265+
"z": []interface{}{json.Number("2")},
266+
},
267+
},
268+
},
269+
},
270+
Modules: []bundle.ModuleFile{
271+
{
272+
URL: modulePath,
273+
Path: modulePath,
274+
Parsed: ast.MustParseModule(module),
275+
Raw: []byte(module),
276+
},
277+
},
278+
},
279+
}
280+
281+
tests := []struct {
282+
note string
283+
bundleInfoTC []bundleInfo
284+
folderContent map[string]string
285+
expectedBundles int
286+
expectedModules int
287+
asBundle bool
288+
}{
289+
{
290+
note: "load multiple bundles. one tar.gz and one folder. Bundle mode is true",
291+
folderContent: bundle1Folder,
292+
bundleInfoTC: []bundleInfo{
293+
bundle1TarGz,
294+
bundle1FolderInfo,
295+
},
296+
expectedBundles: 2,
297+
expectedModules: 0,
298+
asBundle: true,
299+
},
300+
{
301+
note: "load multiple bundles. one tar.gz and one folder. Bundle mode is false",
302+
folderContent: bundle1Folder,
303+
bundleInfoTC: []bundleInfo{
304+
bundle1TarGz,
305+
bundle1FolderInfo,
306+
},
307+
expectedBundles: 1,
308+
expectedModules: 1,
309+
asBundle: false,
310+
},
311+
{
312+
note: "load multiple bundles. two tar.gz and one folder. Bundle mode is true",
313+
folderContent: bundle1Folder,
314+
bundleInfoTC: []bundleInfo{
315+
bundle1TarGz,
316+
bundle2TarGz,
317+
bundle1FolderInfo,
318+
},
319+
expectedBundles: 3,
320+
expectedModules: 0,
321+
asBundle: true,
322+
},
323+
{
324+
note: "load multiple bundles. two tar.gz and one folder. Bundle mode is false",
325+
folderContent: bundle1Folder,
326+
bundleInfoTC: []bundleInfo{
327+
bundle1TarGz,
328+
bundle2TarGz,
329+
bundle1FolderInfo,
330+
},
331+
expectedBundles: 2,
332+
expectedModules: 1,
333+
asBundle: false,
334+
},
335+
{
336+
note: "load just one folder. Bundle mode is true",
337+
folderContent: bundle1Folder,
338+
bundleInfoTC: []bundleInfo{
339+
bundle1FolderInfo,
340+
},
341+
expectedBundles: 1,
342+
expectedModules: 0,
343+
asBundle: true,
344+
},
345+
{
346+
note: "load just one folder. Bundle mode is false",
347+
folderContent: bundle1Folder,
348+
bundleInfoTC: []bundleInfo{
349+
bundle1FolderInfo,
350+
},
351+
expectedBundles: 0,
352+
expectedModules: 1,
353+
asBundle: false,
354+
},
355+
}
356+
357+
for _, tc := range tests {
358+
t.Run(tc.note, func(t *testing.T) {
359+
360+
test.WithTempFS(tc.folderContent, func(rootDir string) {
361+
paths := []string{}
362+
for _, bdlInfo := range tc.bundleInfoTC {
363+
if strings.HasSuffix(bdlInfo.fileName, ".tar.gz") {
364+
365+
// Create the tar gz files temporarily
366+
buf := archive.MustWriteTarGz(bdlInfo.files)
367+
bundleFile := filepath.Join(rootDir, bdlInfo.fileName)
368+
out, err := os.Create(bundleFile)
369+
if err != nil {
370+
t.Fatalf("Unexpected error: %v", err)
371+
}
372+
_, err = out.Write(buf.Bytes())
373+
if err != nil {
374+
t.Fatalf("Unexpected error: %v", err)
375+
}
376+
}
377+
378+
paths = append(paths, filepath.Join(rootDir, bdlInfo.fileName))
379+
}
380+
381+
loaded, err := LoadPaths(paths, nil, tc.asBundle, nil, true, false, nil, nil)
382+
if err != nil {
383+
t.Fatal("Failed LoadPaths ", err)
384+
}
385+
if tc.expectedBundles != len(loaded.Bundles) {
386+
t.Fatalf("Expected %d bundles, got %d", tc.expectedBundles, len(loaded.Bundles))
387+
}
388+
if tc.expectedModules != len(loaded.Files.Modules) {
389+
t.Fatalf("Expected %d modules, got %d", tc.expectedModules, len(loaded.Files.Modules))
390+
}
391+
392+
// Testing the content
393+
for path, actual := range loaded.Bundles {
394+
for _, bdlInfo := range tc.bundleInfoTC {
395+
if strings.HasSuffix(path, bdlInfo.fileName) {
396+
var buf bytes.Buffer
397+
if err := bundle.NewWriter(&buf).Write(bdlInfo.expBundle); err != nil {
398+
t.Fatal(err)
399+
}
400+
401+
expected, err := bundle.NewReader(&buf).Read()
402+
if err != nil {
403+
t.Fatal(err)
404+
}
405+
406+
// adjusting the URL and Path due to /tmp/ path
407+
if len(bdlInfo.expBundle.Modules) > 0 {
408+
expected.Modules[0].URL = rootDir + expected.Modules[0].URL
409+
expected.Modules[0].Path = rootDir + expected.Modules[0].Path
410+
}
411+
412+
if !expected.Equal(*actual) {
413+
t.Fatalf("\nExpected: %+v\nGot: %+v", expected, actual)
414+
}
415+
}
416+
}
417+
}
418+
})
419+
})
420+
}
421+
422+
}
423+
186424
func TestWalkPaths(t *testing.T) {
187425
files := map[string]string{
188426
"/bundle1/a/data.json": `{"foo": "bar1", "x": {"y": {"z": [1]}}}`,

0 commit comments

Comments
 (0)