Fix previous-image flag for analyzer and creator (#897)

Signed-off-by: Ujjwal Goyal <importujjwal@gmail.com>

build.go

@@ -160,10 +160,8 @@ type BuildOptions struct {
 	// User's group id used to build the image
 	GroupID int
 
-	// Slice of two images
-	// 1. A previous image to set to a particular tag reference, digest reference, or image ID;
-	// 2. The image to refer to
-	Images []string
+	// A previous image to set to a particular tag reference, digest reference, or (when performing a daemon build) image ID;
+	PreviousImage string
 }
 
 // ProxyConfig specifies proxy setting to be set as environment variables in a container.
@@ -314,7 +312,7 @@ func (c *Client) Build(ctx context.Context, opts BuildOptions) error {
 		CacheImage:         opts.CacheImage,
 		Workspace:          opts.Workspace,
 		GID:                opts.GroupID,
-		Images:             opts.Images,
+		PreviousImage:      opts.PreviousImage,
 	}
 
 	lifecycleVersion := ephemeralBuilder.LifecycleDescriptor().Info.Version

internal/build/lifecycle_execution.go

@@ -188,6 +188,10 @@ func (l *LifecycleExecution) Create(ctx context.Context, publish bool, dockerHos
 		flags = append(flags, "-gid", strconv.Itoa(l.opts.GID))
 	}
 
+	if l.opts.PreviousImage != "" {
+		flags = append(flags, "-previous-image", l.opts.PreviousImage)
+	}
+
 	processType := determineDefaultProcessType(l.platformAPI, l.opts.DefaultProcessType)
 	if processType != "" {
 		flags = append(flags, "-process-type", processType)
@@ -322,11 +326,8 @@ func (l *LifecycleExecution) newAnalyze(repoName, networkMode string, publish bo
 		flagsOpt = WithFlags("-gid", strconv.Itoa(l.opts.GID))
 	}
 
-	if l.opts.Images != nil {
-		if len(l.opts.Images) != 2 {
-			return nil, errors.New("'-previous-image' takes two arguments, a previous image and a reference image")
-		}
-		l.opts.LifecycleImage = l.opts.Images[0]
+	if l.opts.PreviousImage != "" {
+		l.opts.LifecycleImage = l.opts.PreviousImage
 	}
 
 	if publish {

internal/build/lifecycle_execution_test.go

@@ -798,6 +798,24 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
 				})
 			})
 		})
+
+		when("-previous-image is used", func() {
+			it("passes previous-image to creator", func() {
+				lifecycle := newTestLifecycleExec(t, true, func(options *build.LifecycleOptions) {
+					options.PreviousImage = "previous-image"
+				})
+				fakePhaseFactory := fakes.NewFakePhaseFactory()
+				err := lifecycle.Create(context.Background(), false, "", false, "test", "test", "test", fakeBuildCache, fakeLaunchCache, []string{}, []string{}, fakePhaseFactory)
+				h.AssertNil(t, err)
+
+				lastCallIndex := len(fakePhaseFactory.NewCalledWithProvider) - 1
+				h.AssertNotEq(t, lastCallIndex, -1)
+
+				configProvider := fakePhaseFactory.NewCalledWithProvider[lastCallIndex]
+				h.AssertEq(t, configProvider.Name(), "creator")
+				h.AssertIncludeAllExpectedPatterns(t, configProvider.ContainerConfig().Cmd, []string{"-previous-image", "previous-image"})
+			})
+		})
 	})
 
 	when("#Detect", func() {
@@ -1266,7 +1284,7 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
 			})
 		})
 
-		// getting false pass
+		// doesn't cover edge case where --publish is used
 		when("-previous-image is used", func() {
 			var (
 				lifecycle        *build.LifecycleExecution
@@ -1275,43 +1293,23 @@ func testLifecycleExecution(t *testing.T, when spec.G, it spec.S) {
 			fakePhase := &fakes.FakePhase{}
 			fakePhaseFactory = fakes.NewFakePhaseFactory(fakes.WhichReturnsForNew(fakePhase))
 
-			// doesn't cover edge case where --publish is used
-
-			when("2 images are provided as arguments", func() {
-				it("passes <previous-image> to analyzer", func() {
-					lifecycle = newTestLifecycleExec(t, true, func(options *build.LifecycleOptions) {
-						options.Images = []string{"previous-image", "image"}
-					})
-					// fakePhaseFactory := fakes.NewFakePhaseFactory()
-
-					err := lifecycle.Analyze(context.Background(), "test", "test", false, "", false, fakeCache, fakePhaseFactory)
-					h.AssertNil(t, err)
-
-					lastCallIndex := len(fakePhaseFactory.NewCalledWithProvider) - 1
-					h.AssertNotEq(t, lastCallIndex, -1)
-
-					configProvider := fakePhaseFactory.NewCalledWithProvider[lastCallIndex]
-					h.AssertEq(t, configProvider.Name(), "analyzer")
-					h.AssertEq(t, configProvider.ContainerConfig().Image, "previous-image")
+			it.Before(func() {
+				lifecycle = newTestLifecycleExec(t, true, func(options *build.LifecycleOptions) {
+					options.PreviousImage = "previous-image"
 				})
 			})
 
-			// when("insufficient arguments are provided", func() {
-			// 	lifecycle := newTestLifecycleExec(t, true, func(options *build.LifecycleOptions) {
-			// 		options.Images = []string{}
-			// 	})
-			// 	fakePhaseFactory := fakes.NewFakePhaseFactory()
-
-			// 	err := lifecycle.Analyze(context.Background(), "test", "test", false, "", false, fakeCache, fakePhaseFactory)
-			// 	h.AssertNil(t, err)
+			it("passes previous-image to analyzer", func() {
+				err := lifecycle.Analyze(context.Background(), "test", "test", false, "", false, fakeCache, fakePhaseFactory)
+				h.AssertNil(t, err)
 
-			// 	lastCallIndex := len(fakePhaseFactory.NewCalledWithProvider) - 1
-			// 	h.AssertNotEq(t, lastCallIndex, -1)
+				lastCallIndex := len(fakePhaseFactory.NewCalledWithProvider) - 1
+				h.AssertNotEq(t, lastCallIndex, -1)
 
-			// 	configProvider := fakePhaseFactory.NewCalledWithProvider[lastCallIndex]
-			// 	h.AssertEq(t, configProvider.Name(), "analyzer")
-			// 	h.AssertEq(t, configProvider.ContainerConfig().Image, "previous-image")
-			// })
+				configProvider := fakePhaseFactory.NewCalledWithProvider[lastCallIndex]
+				h.AssertEq(t, configProvider.Name(), "analyzer")
+				h.AssertEq(t, configProvider.ContainerConfig().Image, "previous-image")
+			})
 		})
 	})
 

internal/build/lifecycle_executor.go

@@ -70,7 +70,7 @@ type LifecycleOptions struct {
 	FileFilter         func(string) bool
 	Workspace          string
 	GID                int
-	Images             []string
+	PreviousImage      string
 }
 
 func NewLifecycleExecutor(logger logging.Logger, docker client.CommonAPIClient) *LifecycleExecutor {

internal/commands/build.go

@@ -42,7 +42,7 @@ type BuildFlags struct {
 	AdditionalTags     []string
 	Workspace          string
 	GID                int
-	Images             []string
+	PreviousImage      string
 }
 
 // Build an image from source code
@@ -153,7 +153,7 @@ func Build(logger logging.Logger, cfg config.Config, packClient PackClient) *cob
 				Workspace:                flags.Workspace,
 				LifecycleImage:           lifecycleImage,
 				GroupID:                  gid,
-				Images:                   flags.Images,
+				PreviousImage:            flags.PreviousImage,
 			}); err != nil {
 				return errors.Wrap(err, "failed to build")
 			}
@@ -193,7 +193,7 @@ This option may set DOCKER_HOST environment variable for the build container if
 	cmd.Flags().StringArrayVar(&buildFlags.Volumes, "volume", nil, "Mount host volume into the build container, in the form '<host path>:<target path>[:<options>]'.\n- 'host path': Name of the volume or absolute directory path to mount.\n- 'target path': The path where the file or directory is available in the container.\n- 'options' (default \"ro\"): An optional comma separated list of mount options.\n    - \"ro\", volume contents are read-only.\n    - \"rw\", volume contents are readable and writeable.\n    - \"volume-opt=<key>=<value>\", can be specified more than once, takes a key-value pair consisting of the option name and its value."+multiValueHelp("volume"))
 	cmd.Flags().StringVar(&buildFlags.Workspace, "workspace", "", "Location at which to mount the app dir in the build image")
 	cmd.Flags().IntVar(&buildFlags.GID, "gid", 0, `Override GID of user's group in the stack's build and run images. The provided value must be a positive number`)
-	cmd.Flags().StringArrayVar(&buildFlags.Images, "previous-image", nil, "Set previous image to a particular tag reference, digest reference, or image ID, in the form '<previous-image> <image>'")
+	cmd.Flags().StringVar(&buildFlags.PreviousImage, "previous-image", "", "Set previous image to a particular tag reference, digest reference, or (when performing a daemon build) image ID")
 }
 
 func validateBuildFlags(flags *BuildFlags, cfg config.Config, packClient PackClient, logger logging.Logger) error {