Skip to content
This repository was archived by the owner on Aug 14, 2020. It is now read-only.

Commit 3fe7af7

Browse files
sgottisgotti
committed
acirenderer: check recursive dependencies.
This patch checks and returns an error when a recursive dependency is detected. A recursive dependency is something like: A --- A or A --- B --- A Having the same image referenced by different branches isn't a recursion: A --- B --- C \-- C
1 parent 80ab01d commit 3fe7af7

File tree

2 files changed

+309
-0
lines changed

2 files changed

+309
-0
lines changed

‎pkg/acirenderer/acirenderer_test.go

+295
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,301 @@ func TestGetUpperPWLM(t *testing.T) {
223223
}
224224
}
225225

226+
// Test dependency recursion.
227+
// This tests A --- A
228+
// If not checked it will cause an infinite recursion.
229+
func TestRecursiveDep1(t *testing.T) {
230+
dir, err := ioutil.TempDir("", tstprefix)
231+
if err != nil {
232+
t.Fatalf("error creating tempdir: %v", err)
233+
}
234+
defer os.RemoveAll(dir)
235+
ds := NewTestStore()
236+
237+
// A
238+
imj := `
239+
{
240+
"acKind": "ImageManifest",
241+
"acVersion": "0.1.1",
242+
"name": "example.com/test01"
243+
}
244+
`
245+
246+
// Make A depend on itself
247+
imj, err = addDependencies(imj,
248+
types.Dependency{
249+
ImageName: "example.com/test01",
250+
},
251+
)
252+
if err != nil {
253+
t.Fatalf("unexpected error: %v", err)
254+
}
255+
256+
entries := []*testTarEntry{
257+
{
258+
contents: imj,
259+
header: &tar.Header{
260+
Name: "manifest",
261+
Size: int64(len(imj)),
262+
},
263+
},
264+
// An empty dir
265+
{
266+
header: &tar.Header{
267+
Name: "rootfs/a",
268+
Typeflag: tar.TypeDir,
269+
},
270+
},
271+
}
272+
273+
key1, err := newTestACI(entries, dir, ds)
274+
275+
expectedFiles := []*fileInfo{
276+
&fileInfo{path: "manifest", typeflag: tar.TypeReg},
277+
&fileInfo{path: "rootfs/a", typeflag: tar.TypeDir},
278+
}
279+
280+
err = checkRenderACI("example.com/test01", expectedFiles, ds)
281+
if err == nil {
282+
t.Fatalf("expected recursion error")
283+
} else {
284+
expectedErr := fmt.Errorf("recursion error, image with key %s already referenced by a parent image", key1)
285+
if err.Error() != expectedErr.Error() {
286+
t.Fatalf("expected error: %v, got: %v", expectedErr, err)
287+
}
288+
}
289+
}
290+
291+
// Test dependency recursion.
292+
// This tests A --- B --- A
293+
// If not checked it will cause an infinite recursion.
294+
func TestRecursiveDep2(t *testing.T) {
295+
dir, err := ioutil.TempDir("", tstprefix)
296+
if err != nil {
297+
t.Fatalf("error creating tempdir: %v", err)
298+
}
299+
defer os.RemoveAll(dir)
300+
ds := NewTestStore()
301+
302+
// B
303+
imj := `
304+
{
305+
"acKind": "ImageManifest",
306+
"acVersion": "0.1.1",
307+
"name": "example.com/test01"
308+
}
309+
`
310+
311+
// Make B depend on A
312+
imj, err = addDependencies(imj,
313+
types.Dependency{
314+
ImageName: "example.com/test02",
315+
},
316+
)
317+
if err != nil {
318+
t.Fatalf("unexpected error: %v", err)
319+
}
320+
321+
entries := []*testTarEntry{
322+
{
323+
contents: imj,
324+
header: &tar.Header{
325+
Name: "manifest",
326+
Size: int64(len(imj)),
327+
},
328+
},
329+
// An empty dir
330+
{
331+
header: &tar.Header{
332+
Name: "rootfs/a",
333+
Typeflag: tar.TypeDir,
334+
},
335+
},
336+
}
337+
338+
key1, err := newTestACI(entries, dir, ds)
339+
if err != nil {
340+
t.Fatalf("unexpected error: %v", err)
341+
}
342+
343+
// A
344+
imj = `
345+
{
346+
"acKind": "ImageManifest",
347+
"acVersion": "0.1.1",
348+
"name": "example.com/test02"
349+
}
350+
`
351+
352+
k1, _ := types.NewHash(key1)
353+
imj, err = addDependencies(imj,
354+
types.Dependency{
355+
ImageName: "example.com/test01",
356+
ImageID: k1},
357+
)
358+
if err != nil {
359+
t.Fatalf("unexpected error: %v", err)
360+
}
361+
362+
entries = []*testTarEntry{
363+
{
364+
contents: imj,
365+
header: &tar.Header{
366+
Name: "manifest",
367+
Size: int64(len(imj)),
368+
},
369+
},
370+
}
371+
372+
key2, err := newTestACI(entries, dir, ds)
373+
if err != nil {
374+
t.Fatalf("unexpected error: %v", err)
375+
}
376+
377+
expectedFiles := []*fileInfo{
378+
&fileInfo{path: "manifest", typeflag: tar.TypeReg},
379+
&fileInfo{path: "rootfs/a", typeflag: tar.TypeDir},
380+
}
381+
382+
err = checkRenderACI("example.com/test02", expectedFiles, ds)
383+
if err == nil {
384+
t.Fatalf("expected recursion error")
385+
} else {
386+
expectedErr := fmt.Errorf("recursion error, image with key %s already referenced by a parent image", key2)
387+
if err.Error() != expectedErr.Error() {
388+
t.Fatalf("expected error: %v, got: %v", expectedErr, err)
389+
}
390+
}
391+
}
392+
393+
// Having an image referenced inside different branches is not a recursion problem
394+
// This tests this dep tree:
395+
// A --- B --- C
396+
// \-- C
397+
func TestNonRecursiveDep(t *testing.T) {
398+
dir, err := ioutil.TempDir("", tstprefix)
399+
if err != nil {
400+
t.Fatalf("error creating tempdir: %v", err)
401+
}
402+
defer os.RemoveAll(dir)
403+
ds := NewTestStore()
404+
405+
// C
406+
imj := `
407+
{
408+
"acKind": "ImageManifest",
409+
"acVersion": "0.1.1",
410+
"name": "example.com/test01"
411+
}
412+
`
413+
414+
entries := []*testTarEntry{
415+
{
416+
contents: imj,
417+
header: &tar.Header{
418+
Name: "manifest",
419+
Size: int64(len(imj)),
420+
},
421+
},
422+
// An empty dir
423+
{
424+
header: &tar.Header{
425+
Name: "rootfs/a",
426+
Typeflag: tar.TypeDir,
427+
},
428+
},
429+
}
430+
431+
key1, err := newTestACI(entries, dir, ds)
432+
if err != nil {
433+
t.Fatalf("unexpected error: %v", err)
434+
}
435+
436+
// B
437+
imj = `
438+
{
439+
"acKind": "ImageManifest",
440+
"acVersion": "0.1.1",
441+
"name": "example.com/test02"
442+
}
443+
`
444+
445+
k1, _ := types.NewHash(key1)
446+
imj, err = addDependencies(imj,
447+
// C
448+
types.Dependency{
449+
ImageName: "example.com/test01",
450+
ImageID: k1},
451+
)
452+
if err != nil {
453+
t.Fatalf("unexpected error: %v", err)
454+
}
455+
456+
entries = []*testTarEntry{
457+
{
458+
contents: imj,
459+
header: &tar.Header{
460+
Name: "manifest",
461+
Size: int64(len(imj)),
462+
},
463+
},
464+
}
465+
466+
key2, err := newTestACI(entries, dir, ds)
467+
if err != nil {
468+
t.Fatalf("unexpected error: %v", err)
469+
}
470+
471+
// A
472+
imj = `
473+
{
474+
"acKind": "ImageManifest",
475+
"acVersion": "0.1.1",
476+
"name": "example.com/test03"
477+
}
478+
`
479+
480+
k2, _ := types.NewHash(key2)
481+
imj, err = addDependencies(imj,
482+
// B
483+
types.Dependency{
484+
ImageName: "example.com/test02",
485+
ImageID: k2},
486+
// C
487+
types.Dependency{
488+
ImageName: "example.com/test01",
489+
ImageID: k1},
490+
)
491+
if err != nil {
492+
t.Fatalf("unexpected error: %v", err)
493+
}
494+
495+
entries = []*testTarEntry{
496+
{
497+
contents: imj,
498+
header: &tar.Header{
499+
Name: "manifest",
500+
Size: int64(len(imj)),
501+
},
502+
},
503+
}
504+
505+
_, err = newTestACI(entries, dir, ds)
506+
if err != nil {
507+
t.Fatalf("unexpected error: %v", err)
508+
}
509+
510+
expectedFiles := []*fileInfo{
511+
&fileInfo{path: "manifest", typeflag: tar.TypeReg},
512+
&fileInfo{path: "rootfs/a", typeflag: tar.TypeDir},
513+
}
514+
515+
err = checkRenderACI("example.com/test03", expectedFiles, ds)
516+
if err != nil {
517+
t.Fatalf("unexpected error: %v", err)
518+
}
519+
}
520+
226521
// Test an image with 1 dep. The parent provides a dir not provided by the image.
227522
func TestDirFromParent(t *testing.T) {
228523
dir, err := ioutil.TempDir("", tstprefix)

‎pkg/acirenderer/resolve.go

+14
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ package acirenderer
1616

1717
import (
1818
"container/list"
19+
"fmt"
1920

2021
"github.com/appc/spec/schema/types"
2122
)
@@ -75,6 +76,19 @@ func createDepList(key string, ap ACIRegistry) (Images, error) {
7576
if err != nil {
7677
return nil, err
7778
}
79+
// Check that an upper image is not the same as this dependency (same key)
80+
// Walk up the current branch and check that this depKey is not already in this branch
81+
curlevel := img.Level + 1
82+
for tel := el; tel != nil; tel = tel.Prev() {
83+
timg := tel.Value.(Image)
84+
if timg.Level < curlevel {
85+
if depKey == timg.Key {
86+
return nil, fmt.Errorf("recursion error, image with key %s already referenced by a parent image", depKey)
87+
}
88+
}
89+
curlevel = timg.Level
90+
}
91+
7892
depimg = Image{Im: im, Key: depKey, Level: img.Level + 1}
7993
imgsl.InsertAfter(depimg, el)
8094
}

0 commit comments

Comments
 (0)