Merge pull request #337 from osscontainertools/mz334-cache-lookahead-refactoring-part-2

mz334: move resolveCrossStageCommands forwards

Author: mzihlmann

pkg/dockerfile/dockerfile.go

@@ -253,7 +253,7 @@ func saveStage(index int, stages []instructions.Stage) bool {
 // ResolveCrossStageCommands resolves any calls to previous stages with names to indices
 // Ex. --from=secondStage should be --from=1 for easier processing later on
 // As third party library lowers stage name in FROM instruction, this function resolves stage case insensitively.
-func ResolveCrossStageCommands(cmds []instructions.Command, stageNameToIdx map[string]int) {
+func resolveCrossStageCommands(cmds []instructions.Command, stageNameToIdx map[string]int) {
 	for _, cmd := range cmds {
 		switch c := cmd.(type) {
 		case *instructions.CopyCommand:
@@ -331,7 +331,7 @@ func MakeKanikoStages(opts *config.KanikoOptions, stages []instructions.Stage, m
 			return nil, fmt.Errorf("failed to parse ONBUILD instructions: %w", err)
 		}
 		stage.Commands = append(cmds, stage.Commands...)
-
+		resolveCrossStageCommands(stage.Commands, stageByName)
 		if opts.SkipUnusedStages {
 			if baseImageStoredLocally {
 				stagesDependencies[baseImageIndex]++
@@ -340,16 +340,7 @@ func MakeKanikoStages(opts *config.KanikoOptions, stages []instructions.Stage, m
 				switch cmd := c.(type) {
 				case *instructions.CopyCommand:
 					if copyFromIndex, err := strconv.Atoi(cmd.From); err == nil {
-						// numeric reference `COPY --from=0`
 						copyDependencies[copyFromIndex]++
-					} else {
-						// named reference `COPY --from=base`
-						if copyFromIndex, ok := stageByName[strings.ToLower(cmd.From)]; ok {
-							// There can be references that appear as non-existing stages
-							// ie. `COPY --from=debian` would try refer to `debian` as stage
-							// before falling back to `debian` as a docker image.
-							copyDependencies[copyFromIndex]++
-						}
 					}
 				}
 			}

pkg/dockerfile/dockerfile_test.go

@@ -235,7 +235,7 @@ func Test_GetOnBuildInstructions(t *testing.T) {
 			if len(cmds) != len(test.expCommands) {
 				t.Fatalf("Expected %d commands, got %d", len(test.expCommands), len(cmds))
 			}
-			ResolveCrossStageCommands(cmds, test.stageToIdx)
+			resolveCrossStageCommands(cmds, test.stageToIdx)
 
 			for i, cmd := range cmds {
 				if reflect.TypeOf(cmd) != reflect.TypeOf(test.expCommands[i]) {

pkg/executor/build.go

@@ -1084,8 +1084,6 @@ func fetchExtraStages(stages []config.KanikoStage, opts *config.KanikoOptions) e
 	t := timing.Start("Fetching Extra Stages")
 	defer timing.DefaultRun.Stop(t)
 
-	var names []string
-
 	for _, s := range stages {
 		for _, cmd := range s.Commands {
 			c, ok := cmd.(*instructions.CopyCommand)
@@ -1094,14 +1092,13 @@ func fetchExtraStages(stages []config.KanikoStage, opts *config.KanikoOptions) e
 			}
 
 			// FROMs at this point are guaranteed to be either an integer referring to a previous stage,
-			// the name of a previous stage, or a name of a remote image.
+			// or a name of a remote image.
 
-			// If it is an integer stage index, validate that it is actually a previous index
-			if fromIndex, err := strconv.Atoi(c.From); err == nil && s.Index > fromIndex && fromIndex >= 0 {
-				continue
-			}
-			// Check if the name is the alias of a previous stage
-			if fromPreviousStage(c, names) {
+			if fromIndex, err := strconv.Atoi(c.From); err == nil {
+				// If it is an integer stage index, validate that it is actually a previous index
+				if s.Index <= fromIndex || fromIndex < 0 {
+					return fmt.Errorf("%s refers to invalid stage: %d", c.String(), fromIndex)
+				}
 				continue
 			}
 
@@ -1118,23 +1115,10 @@ func fetchExtraStages(stages []config.KanikoStage, opts *config.KanikoOptions) e
 				return err
 			}
 		}
-		// Store the name of the current stage in the list with names, if applicable.
-		if s.Name != "" {
-			names = append(names, s.Name)
-		}
 	}
 	return nil
 }
 
-func fromPreviousStage(copyCommand *instructions.CopyCommand, previousStageNames []string) bool {
-	for _, previousStageName := range previousStageNames {
-		if previousStageName == copyCommand.From {
-			return true
-		}
-	}
-	return false
-}
-
 func extractImageToDependencyDir(name string, image v1.Image) error {
 	t := timing.Start("Extracting Image to Dependency Dir")
 	defer timing.DefaultRun.Stop(t)
@@ -1214,7 +1198,6 @@ func ResolveCrossStageInstructions(stages []config.KanikoStage) map[string]int {
 		if stage.Name != "" {
 			nameToIndex[stage.Name] = stage.Index
 		}
-		dockerfile.ResolveCrossStageCommands(stage.Commands, nameToIndex)
 	}
 
 	logrus.Debugf("Built stage name to index map: %v", nameToIndex)