From ad4ae59aa6bea400c877a747d7cffb20beccd394 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Fri, 10 May 2019 15:43:47 +0000 Subject: [PATCH 1/4] [dockerfile2llb] check for circular dependency in convert Signed-off-by: Stepan Blyshchak --- frontend/dockerfile/dockerfile2llb/convert.go | 39 +++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 05279230330d..e21e51ce1a8c 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -207,6 +207,10 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State, } } + if has, state := hasCircularDependency(allDispatchStates.states); has { + return nil, nil, fmt.Errorf("circular dependency detected on stage: %s", state.stageName) + } + eg, ctx := errgroup.WithContext(ctx) for i, d := range allDispatchStates.states { reachable := isReachable(target, d) @@ -1130,6 +1134,41 @@ func isReachable(from, to *dispatchState) (ret bool) { return false } +func hasCircularDependency(states []*dispatchState) (bool, *dispatchState) { + var visit func (state *dispatchState) bool + if states == nil { + return false, nil + } + visited := make(map[*dispatchState]struct{}) + path := make(map[*dispatchState]struct{}) + + visit = func (state *dispatchState) bool { + _, ok := visited[state] + if ok { + return false + } + visited[state] = struct{}{} + path[state] = struct{}{} + for dep := range state.deps { + _, ok = path[dep] + if ok { + return true + } + if visit(dep) { + return true + } + } + delete(path, state) + return false + } + for _, state := range states { + if visit(state) { + return true, state + } + } + return false, nil +} + func parseUser(str string) (uid uint32, gid uint32, err error) { if str == "" { return 0, 0, nil From 98a6ff4fe2abcd94d2fefc972071123bb0956497 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Sat, 11 May 2019 18:06:50 +0300 Subject: [PATCH 2/4] apply gofmt Signed-off-by: Stepan Blyshchak --- frontend/dockerfile/dockerfile2llb/convert.go | 66 +++++++++---------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index e21e51ce1a8c..a0262d231696 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -209,7 +209,7 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State, if has, state := hasCircularDependency(allDispatchStates.states); has { return nil, nil, fmt.Errorf("circular dependency detected on stage: %s", state.stageName) - } + } eg, ctx := errgroup.WithContext(ctx) for i, d := range allDispatchStates.states { @@ -1135,38 +1135,38 @@ func isReachable(from, to *dispatchState) (ret bool) { } func hasCircularDependency(states []*dispatchState) (bool, *dispatchState) { - var visit func (state *dispatchState) bool - if states == nil { - return false, nil - } - visited := make(map[*dispatchState]struct{}) - path := make(map[*dispatchState]struct{}) - - visit = func (state *dispatchState) bool { - _, ok := visited[state] - if ok { - return false - } - visited[state] = struct{}{} - path[state] = struct{}{} - for dep := range state.deps { - _, ok = path[dep] - if ok { - return true - } - if visit(dep) { - return true - } - } - delete(path, state) - return false - } - for _, state := range states { - if visit(state) { - return true, state - } - } - return false, nil + var visit func(state *dispatchState) bool + if states == nil { + return false, nil + } + visited := make(map[*dispatchState]struct{}) + path := make(map[*dispatchState]struct{}) + + visit = func(state *dispatchState) bool { + _, ok := visited[state] + if ok { + return false + } + visited[state] = struct{}{} + path[state] = struct{}{} + for dep := range state.deps { + _, ok = path[dep] + if ok { + return true + } + if visit(dep) { + return true + } + } + delete(path, state) + return false + } + for _, state := range states { + if visit(state) { + return true, state + } + } + return false, nil } func parseUser(str string) (uid uint32, gid uint32, err error) { From 981ee384065698e73e2d75c6ec9cc760aa4deeba Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Sat, 11 May 2019 18:21:15 +0300 Subject: [PATCH 3/4] [convert] move 'stageName = ""' in covert after checking for circular dependency To have stageName in error output in case one stage depends on itself Signed-off-by: Stepan Blyshchak --- frontend/dockerfile/dockerfile2llb/convert.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index a0262d231696..f368fe6b49c8 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -172,10 +172,6 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State, } } - if len(allDispatchStates.states) == 1 { - allDispatchStates.states[0].stageName = "" - } - var target *dispatchState if opt.Target == "" { target = allDispatchStates.lastTarget() @@ -211,6 +207,10 @@ func Dockerfile2LLB(ctx context.Context, dt []byte, opt ConvertOpt) (*llb.State, return nil, nil, fmt.Errorf("circular dependency detected on stage: %s", state.stageName) } + if len(allDispatchStates.states) == 1 { + allDispatchStates.states[0].stageName = "" + } + eg, ctx := errgroup.WithContext(ctx) for i, d := range allDispatchStates.states { reachable := isReachable(target, d) From ea4bb024df4d392cb7b7f01d82f2e68852bd4df8 Mon Sep 17 00:00:00 2001 From: Stepan Blyshchak Date: Sat, 11 May 2019 18:24:09 +0300 Subject: [PATCH 4/4] [convert_test] add ut for circular dependency cases in dockerfile Signed-off-by: Stepan Blyshchak --- .../dockerfile/dockerfile2llb/convert_test.go | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/frontend/dockerfile/dockerfile2llb/convert_test.go b/frontend/dockerfile/dockerfile2llb/convert_test.go index 61d63fd6c13a..c7f83d918795 100644 --- a/frontend/dockerfile/dockerfile2llb/convert_test.go +++ b/frontend/dockerfile/dockerfile2llb/convert_test.go @@ -152,3 +152,23 @@ func TestToEnvList(t *testing.T) { resutl = toEnvMap(args, env) assert.Equal(t, map[string]string{"key1": "val1", "key2": "v1"}, resutl) } + +func TestDockerfileCircularDependencies(t *testing.T) { + // single stage depends on itself + df := `FROM busybox AS stage0 +COPY --from=stage0 f1 /sub/ +` + _, _, err := Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + assert.EqualError(t, err, "circular dependency detected on stage: stage0") + + // multiple stages with circular dependency + df = `FROM busybox AS stage0 +COPY --from=stage2 f1 /sub/ +FROM busybox AS stage1 +COPY --from=stage0 f2 /sub/ +FROM busybox AS stage2 +COPY --from=stage1 f2 /sub/ +` + _, _, err = Dockerfile2LLB(appcontext.Context(), []byte(df), ConvertOpt{}) + assert.EqualError(t, err, "circular dependency detected on stage: stage0") +}