dockerfile: validate order when linking stages

Dockerfile stages always needed to be in order (you can not refer
to a stage that is defined after your current command), but the
behavior for incorrect order was undefined instead of consistently
erroring.

Previously copying from a stage defined later would usually cause
an error about a missing source file that could have been quite
confusing. When the source path was "/" then the behavior could have
been worse and resulted in COPY passing without error, without any
files actually being copied (or only images from base image being
copied).

This validates the order when on COPY and mount dispatch, returns
proper error with correct source code location.

Signed-off-by: Tonis Tiigi <[email protected]>

frontend/dockerfile/dockerfile2llb/convert.go

@@ -492,6 +492,8 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS
 		if !isReachable(target, d) || d.noinit {
 			continue
 		}
+		// mark as initialized, used to determine states that have not been dispatched yet
+		d.noinit = true
 
 		if d.base != nil {
 			d.state = d.base.state
@@ -651,6 +653,7 @@ func toCommand(ic instructions.Command, allDispatchStates *dispatchStates) (comm
 						deps:         make(map[*dispatchState]instructions.Command),
 						paths:        make(map[string]struct{}),
 						unregistered: true,
+						noinit:       true,
 					}
 				}
 			} else {
@@ -779,7 +782,11 @@ func dispatch(d *dispatchState, cmd command, opt dispatchOpt) error {
 	case *instructions.CopyCommand:
 		l := opt.buildContext
 		if len(cmd.sources) != 0 {
-			l = cmd.sources[0].state
+			src := cmd.sources[0]
+			if !src.noinit {
+				return errors.Errorf("cannot copy from stage %q, it needs to be defined before current stage %q", c.From, d.stageName)
+			}
+			l = src.state
 		}
 		err = dispatchCopy(d, copyConfig{
 			params:       c.SourcesAndDest,

frontend/dockerfile/dockerfile2llb/convert_runmount.go

@@ -33,6 +33,7 @@ func detectRunMount(cmd *command, allDispatchStates *dispatchStates) bool {
 					deps:         make(map[*dispatchState]instructions.Command),
 					paths:        make(map[string]struct{}),
 					unregistered: true,
+					noinit:       true,
 				}
 			}
 			sources[i] = stn
@@ -65,12 +66,27 @@ func dispatchRunMounts(d *dispatchState, c *instructions.RunCommand, sources []*
 	mounts := instructions.GetMounts(c)
 
 	for i, mount := range mounts {
+		target := mount.Target
+		if !filepath.IsAbs(filepath.Clean(mount.Target)) {
+			dir, err := d.state.GetDir(context.TODO())
+			if err != nil {
+				return nil, err
+			}
+			target = filepath.Join("/", dir, mount.Target)
+		}
+		if target == "/" {
+			return nil, errors.Errorf("invalid mount target %q", target)
+		}
 		if mount.From == "" && mount.Type == instructions.MountTypeCache {
 			mount.From = emptyImageName
 		}
 		st := opt.buildContext
 		if mount.From != "" {
-			st = sources[i].state
+			src := sources[i]
+			st = src.state
+			if !src.noinit {
+				return nil, errors.Errorf("cannot mount from stage %q to %q, stage needs to be defined before current command", mount.From, target)
+			}
 		}
 		var mountOpts []llb.MountOption
 		if mount.Type == instructions.MountTypeTmpfs {
@@ -113,17 +129,7 @@ func dispatchRunMounts(d *dispatchState, c *instructions.RunCommand, sources []*
 			}
 			mountOpts = append(mountOpts, llb.AsPersistentCacheDir(opt.cacheIDNamespace+"/"+mount.CacheID, sharing))
 		}
-		target := mount.Target
-		if !filepath.IsAbs(filepath.Clean(mount.Target)) {
-			dir, err := d.state.GetDir(context.TODO())
-			if err != nil {
-				return nil, err
-			}
-			target = filepath.Join("/", dir, mount.Target)
-		}
-		if target == "/" {
-			return nil, errors.Errorf("invalid mount target %q", target)
-		}
+
 		if src := path.Join("/", mount.Source); src != "/" {
 			mountOpts = append(mountOpts, llb.SourcePath(src))
 		} else {

frontend/dockerfile/dockerfile_test.go

@@ -156,6 +156,7 @@ var allTests = integration.TestFuncs(
 	testWorkdirUser,
 	testWorkdirExists,
 	testWorkdirCopyIgnoreRelative,
+	testOutOfOrderStage,
 	testCopyFollowAllSymlinks,
 	testDockerfileAddChownExpand,
 	testSourceDateEpochWithoutExporter,
@@ -953,6 +954,42 @@ RUN [ "$(stat -c "%U %G" /mydir)" == "user user" ]
 	require.NoError(t, err)
 }
 
+func testOutOfOrderStage(t *testing.T, sb integration.Sandbox) {
+	integration.SkipOnPlatform(t, "windows")
+	f := getFrontend(t, sb)
+
+	for _, src := range []string{"/", "/d2"} {
+		dockerfile := []byte(fmt.Sprintf(`
+FROM busybox AS target
+COPY --from=build %s /out
+
+FROM alpine AS build
+COPY /Dockerfile /d2
+
+FROM target
+`, src))
+
+		dir := integration.Tmpdir(
+			t,
+			fstest.CreateFile("Dockerfile", dockerfile, 0600),
+		)
+
+		c, err := client.New(sb.Context(), sb.Address())
+		require.NoError(t, err)
+		defer c.Close()
+
+		_, err = f.Solve(sb.Context(), c, client.SolveOpt{
+			LocalDirs: map[string]string{
+				dockerui.DefaultLocalNameDockerfile: dir,
+				dockerui.DefaultLocalNameContext:    dir,
+			},
+		}, nil)
+		require.Error(t, err)
+		require.Contains(t, err.Error(), "cannot copy from stage")
+		require.Contains(t, err.Error(), "needs to be defined before current stage")
+	}
+}
+
 func testWorkdirCopyIgnoreRelative(t *testing.T, sb integration.Sandbox) {
 	integration.SkipOnPlatform(t, "windows")
 	f := getFrontend(t, sb)