Implement required artifact resolution in jib builder (#4997)

* Implement required artifact resolution in jib builder

* Update pkg/skaffold/build/jib/jib.go

Co-authored-by: Brian de Alwis <[email protected]>

Co-authored-by: Brian de Alwis <[email protected]>

Author: gsquared94

pkg/skaffold/build/gcb/jib.go

@@ -41,15 +41,15 @@ func (b *Builder) jibBuildSpec(artifact *latest.Artifact, tag string) (cloudbuil
 			Steps: []*cloudbuild.BuildStep{{
 				Name:       b.MavenImage,
 				Entrypoint: "sh",
-				Args:       fixHome("mvn", jib.GenerateMavenBuildArgs("build", tag, artifact.JibArtifact, b.skipTests, b.cfg.GetInsecureRegistries())),
+				Args:       fixHome("mvn", jib.GenerateMavenBuildArgs("build", tag, artifact.JibArtifact, b.skipTests, true, artifact.Dependencies, b.artifactStore, b.cfg.GetInsecureRegistries())),
 			}},
 		}, nil
 	case jib.JibGradle:
 		return cloudbuild.Build{
 			Steps: []*cloudbuild.BuildStep{{
 				Name:       b.GradleImage,
 				Entrypoint: "sh",
-				Args:       fixHome("gradle", jib.GenerateGradleBuildArgs("jib", tag, artifact.JibArtifact, b.skipTests, b.cfg.GetInsecureRegistries())),
+				Args:       fixHome("gradle", jib.GenerateGradleBuildArgs("jib", tag, artifact.JibArtifact, b.skipTests, true, artifact.Dependencies, b.artifactStore, b.cfg.GetInsecureRegistries())),
 			}},
 		}, nil
 	default:

pkg/skaffold/build/jib/build.go

@@ -34,15 +34,15 @@ func (b *Builder) Build(ctx context.Context, out io.Writer, artifact *latest.Art
 	switch t {
 	case JibMaven:
 		if b.pushImages {
-			return b.buildJibMavenToRegistry(ctx, out, artifact.Workspace, artifact.JibArtifact, tag)
+			return b.buildJibMavenToRegistry(ctx, out, artifact.Workspace, artifact.JibArtifact, artifact.Dependencies, tag)
 		}
-		return b.buildJibMavenToDocker(ctx, out, artifact.Workspace, artifact.JibArtifact, tag)
+		return b.buildJibMavenToDocker(ctx, out, artifact.Workspace, artifact.JibArtifact, artifact.Dependencies, tag)
 
 	case JibGradle:
 		if b.pushImages {
-			return b.buildJibGradleToRegistry(ctx, out, artifact.Workspace, artifact.JibArtifact, tag)
+			return b.buildJibGradleToRegistry(ctx, out, artifact.Workspace, artifact.JibArtifact, artifact.Dependencies, tag)
 		}
-		return b.buildJibGradleToDocker(ctx, out, artifact.Workspace, artifact.JibArtifact, tag)
+		return b.buildJibGradleToDocker(ctx, out, artifact.Workspace, artifact.JibArtifact, artifact.Dependencies, tag)
 
 	default:
 		return "", fmt.Errorf("unable to determine Jib builder type for %s", artifact.Workspace)

pkg/skaffold/build/jib/gradle.go

@@ -42,17 +42,17 @@ const MinimumJibGradleVersionForSync = "2.0.0"
 // GradleCommand stores Gradle executable and wrapper name
 var GradleCommand = util.CommandWrapper{Executable: "gradle", Wrapper: "gradlew"}
 
-func (b *Builder) buildJibGradleToDocker(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibArtifact, tag string) (string, error) {
-	args := GenerateGradleBuildArgs("jibDockerBuild", tag, artifact, b.skipTests, b.cfg.GetInsecureRegistries())
+func (b *Builder) buildJibGradleToDocker(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibArtifact, deps []*latest.ArtifactDependency, tag string) (string, error) {
+	args := GenerateGradleBuildArgs("jibDockerBuild", tag, artifact, b.skipTests, b.pushImages, deps, b.artifacts, b.cfg.GetInsecureRegistries())
 	if err := b.runGradleCommand(ctx, out, workspace, args); err != nil {
 		return "", err
 	}
 
 	return b.localDocker.ImageID(ctx, tag)
 }
 
-func (b *Builder) buildJibGradleToRegistry(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibArtifact, tag string) (string, error) {
-	args := GenerateGradleBuildArgs("jib", tag, artifact, b.skipTests, b.cfg.GetInsecureRegistries())
+func (b *Builder) buildJibGradleToRegistry(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibArtifact, deps []*latest.ArtifactDependency, tag string) (string, error) {
+	args := GenerateGradleBuildArgs("jib", tag, artifact, b.skipTests, b.pushImages, deps, b.artifacts, b.cfg.GetInsecureRegistries())
 	if err := b.runGradleCommand(ctx, out, workspace, args); err != nil {
 		return "", err
 	}
@@ -97,14 +97,14 @@ func getSyncMapCommandGradle(ctx context.Context, workspace string, a *latest.Ji
 }
 
 // GenerateGradleBuildArgs generates the arguments to Gradle for building the project as an image.
-func GenerateGradleBuildArgs(task string, imageName string, a *latest.JibArtifact, skipTests bool, insecureRegistries map[string]bool) []string {
+func GenerateGradleBuildArgs(task string, imageName string, a *latest.JibArtifact, skipTests, pushImages bool, deps []*latest.ArtifactDependency, r ArtifactResolver, insecureRegistries map[string]bool) []string {
 	args := gradleBuildArgsFunc(task, a, skipTests, true, MinimumJibGradleVersion)
 	if insecure, err := isOnInsecureRegistry(imageName, insecureRegistries); err == nil && insecure {
 		// jib doesn't support marking specific registries as insecure
 		args = append(args, "-Djib.allowInsecureRegistries=true")
 	}
-	if a.BaseImage != "" {
-		args = append(args, fmt.Sprintf("-Djib.from.image=%s", a.BaseImage))
+	if baseImg, found := baseImageArg(a, r, deps, pushImages); found {
+		args = append(args, baseImg)
 	}
 	args = append(args, "--image="+imageName)
 	return args

pkg/skaffold/build/jib/gradle_test.go

@@ -82,7 +82,7 @@ func TestBuildJibGradleToDocker(t *testing.T) {
 			api := (&testutil.FakeAPIClient{}).Add("img:tag", "imageID")
 			localDocker := fakeLocalDaemon(api)
 
-			builder := NewArtifactBuilder(localDocker, &mockConfig{}, false, false)
+			builder := NewArtifactBuilder(localDocker, &mockConfig{}, false, false, nil)
 			result, err := builder.Build(context.Background(), ioutil.Discard, &latest.Artifact{
 				ArtifactType: latest.ArtifactType{
 					JibArtifact: test.artifact,
@@ -153,7 +153,7 @@ func TestBuildJibGradleToRegistry(t *testing.T) {
 			})
 			localDocker := fakeLocalDaemon(&testutil.FakeAPIClient{})
 
-			builder := NewArtifactBuilder(localDocker, &mockConfig{}, true, false)
+			builder := NewArtifactBuilder(localDocker, &mockConfig{}, true, false, nil)
 			result, err := builder.Build(context.Background(), ioutil.Discard, &latest.Artifact{
 				ArtifactType: latest.ArtifactType{
 					JibArtifact: test.artifact,
@@ -341,23 +341,59 @@ func TestGenerateGradleBuildArgs(t *testing.T) {
 	tests := []struct {
 		description        string
 		in                 latest.JibArtifact
+		deps               []*latest.ArtifactDependency
 		image              string
 		skipTests          bool
+		pushImages         bool
+		r                  ArtifactResolver
 		insecureRegistries map[string]bool
 		out                []string
 	}{
-		{"single module", latest.JibArtifact{}, "image", false, nil, []string{"fake-gradleBuildArgs-for-testTask", "--image=image"}},
-		{"single module without tests", latest.JibArtifact{}, "image", true, nil, []string{"fake-gradleBuildArgs-for-testTask-skipTests", "--image=image"}},
-		{"multi module", latest.JibArtifact{Project: "project"}, "image", false, nil, []string{"fake-gradleBuildArgs-for-project-for-testTask", "--image=image"}},
-		{"multi module without tests", latest.JibArtifact{Project: "project"}, "image", true, nil, []string{"fake-gradleBuildArgs-for-project-for-testTask-skipTests", "--image=image"}},
-		{"multi module without tests with insecure registries", latest.JibArtifact{Project: "project"}, "registry.tld/image", true, map[string]bool{"registry.tld": true}, []string{"fake-gradleBuildArgs-for-project-for-testTask-skipTests", "-Djib.allowInsecureRegistries=true", "--image=registry.tld/image"}},
-		{"single module with custom base image", latest.JibArtifact{BaseImage: "docker://busybox"}, "image", false, nil, []string{"fake-gradleBuildArgs-for-testTask", "-Djib.from.image=docker://busybox", "--image=image"}},
-		{"multi module with custom base image", latest.JibArtifact{Project: "project", BaseImage: "docker://busybox"}, "image", false, nil, []string{"fake-gradleBuildArgs-for-project-for-testTask", "-Djib.from.image=docker://busybox", "--image=image"}},
+		{description: "single module", image: "image", out: []string{"fake-gradleBuildArgs-for-testTask", "--image=image"}},
+		{description: "single module without tests", image: "image", skipTests: true, out: []string{"fake-gradleBuildArgs-for-testTask-skipTests", "--image=image"}},
+		{description: "multi module", in: latest.JibArtifact{Project: "project"}, image: "image", out: []string{"fake-gradleBuildArgs-for-project-for-testTask", "--image=image"}},
+		{description: "multi module without tests", in: latest.JibArtifact{Project: "project"}, image: "image", skipTests: true, out: []string{"fake-gradleBuildArgs-for-project-for-testTask-skipTests", "--image=image"}},
+		{description: "multi module without tests with insecure registries", in: latest.JibArtifact{Project: "project"}, image: "registry.tld/image", skipTests: true, insecureRegistries: map[string]bool{"registry.tld": true}, out: []string{"fake-gradleBuildArgs-for-project-for-testTask-skipTests", "-Djib.allowInsecureRegistries=true", "--image=registry.tld/image"}},
+		{description: "single module with custom base image", in: latest.JibArtifact{BaseImage: "docker://busybox"}, image: "image", out: []string{"fake-gradleBuildArgs-for-testTask", "-Djib.from.image=docker://busybox", "--image=image"}},
+		{description: "multi module with custom base image", in: latest.JibArtifact{Project: "project", BaseImage: "docker://busybox"}, image: "image", out: []string{"fake-gradleBuildArgs-for-project-for-testTask", "-Djib.from.image=docker://busybox", "--image=image"}},
+		{
+			description: "single module with local base image from required artifacts",
+			in:          latest.JibArtifact{BaseImage: "alias"},
+			image:       "image",
+			deps:        []*latest.ArtifactDependency{{ImageName: "img", Alias: "alias"}},
+			r:           mockArtifactResolver{m: map[string]string{"img": "img:tag"}},
+			out:         []string{"fake-gradleBuildArgs-for-testTask", "-Djib.from.image=docker://img:tag", "--image=image"},
+		},
+		{
+			description: "multi module with local base image from required artifacts",
+			in:          latest.JibArtifact{Project: "project", BaseImage: "alias"},
+			image:       "image",
+			deps:        []*latest.ArtifactDependency{{ImageName: "img", Alias: "alias"}},
+			r:           mockArtifactResolver{m: map[string]string{"img": "img:tag"}},
+			out:         []string{"fake-gradleBuildArgs-for-project-for-testTask", "-Djib.from.image=docker://img:tag", "--image=image"},
+		}, {
+			description: "single module with remote base image from required artifacts",
+			in:          latest.JibArtifact{BaseImage: "alias"},
+			image:       "image",
+			pushImages:  true,
+			deps:        []*latest.ArtifactDependency{{ImageName: "img", Alias: "alias"}},
+			r:           mockArtifactResolver{m: map[string]string{"img": "img:tag"}},
+			out:         []string{"fake-gradleBuildArgs-for-testTask", "-Djib.from.image=img:tag", "--image=image"},
+		},
+		{
+			description: "multi module with remote base image from required artifacts",
+			in:          latest.JibArtifact{Project: "project", BaseImage: "alias"},
+			image:       "image",
+			pushImages:  true,
+			deps:        []*latest.ArtifactDependency{{ImageName: "img", Alias: "alias"}},
+			r:           mockArtifactResolver{m: map[string]string{"img": "img:tag"}},
+			out:         []string{"fake-gradleBuildArgs-for-project-for-testTask", "-Djib.from.image=img:tag", "--image=image"},
+		},
 	}
 	for _, test := range tests {
 		testutil.Run(t, test.description, func(t *testutil.T) {
 			t.Override(&gradleBuildArgsFunc, getGradleBuildArgsFuncFake(t, MinimumJibGradleVersion))
-			command := GenerateGradleBuildArgs("testTask", test.image, &test.in, test.skipTests, test.insecureRegistries)
+			command := GenerateGradleBuildArgs("testTask", test.image, &test.in, test.skipTests, test.pushImages, test.deps, test.r, test.insecureRegistries)
 			t.CheckDeepEqual(test.out, command)
 		})
 	}
@@ -485,3 +521,12 @@ type mockConfig struct {
 }
 
 func (c *mockConfig) GetInsecureRegistries() map[string]bool { return nil }
+
+type mockArtifactResolver struct {
+	m map[string]string
+}
+
+func (r mockArtifactResolver) GetImageTag(imageName string) (string, bool) {
+	val, found := r.m[imageName]
+	return val, found
+}

pkg/skaffold/build/jib/jib.go

@@ -326,3 +326,26 @@ func isOnInsecureRegistry(image string, insecureRegistries map[string]bool) (boo
 
 	return docker.IsInsecure(ref, insecureRegistries), nil
 }
+
+// baseImageArg formats the base image as a build argument. It also replaces the provided base image with an image from the required artifacts if specified.
+func baseImageArg(a *latest.JibArtifact, r ArtifactResolver, deps []*latest.ArtifactDependency, pushImages bool) (string, bool) {
+	if a.BaseImage == "" {
+		return "", false
+	}
+	for _, d := range deps {
+		if a.BaseImage != d.Alias {
+			continue
+		}
+		img, found := r.GetImageTag(d.ImageName)
+		if !found {
+			logrus.Fatalf("failed to resolve build result for required artifact %q", d.ImageName)
+		}
+		if pushImages {
+			// pull image from the registry (prefix `registry://` is optional)
+			return fmt.Sprintf("-Djib.from.image=%s", img), true
+		}
+		// must use `docker://` prefix to retrieve image from the local docker daemon
+		return fmt.Sprintf("-Djib.from.image=docker://%s", img), true
+	}
+	return fmt.Sprintf("-Djib.from.image=%s", a.BaseImage), true
+}

pkg/skaffold/build/jib/maven.go

@@ -42,17 +42,17 @@ const MinimumJibMavenVersionForSync = "2.0.0"
 // MavenCommand stores Maven executable and wrapper name
 var MavenCommand = util.CommandWrapper{Executable: "mvn", Wrapper: "mvnw"}
 
-func (b *Builder) buildJibMavenToDocker(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibArtifact, tag string) (string, error) {
-	args := GenerateMavenBuildArgs("dockerBuild", tag, artifact, b.skipTests, b.cfg.GetInsecureRegistries())
+func (b *Builder) buildJibMavenToDocker(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibArtifact, deps []*latest.ArtifactDependency, tag string) (string, error) {
+	args := GenerateMavenBuildArgs("dockerBuild", tag, artifact, b.skipTests, b.pushImages, deps, b.artifacts, b.cfg.GetInsecureRegistries())
 	if err := b.runMavenCommand(ctx, out, workspace, args); err != nil {
 		return "", err
 	}
 
 	return b.localDocker.ImageID(ctx, tag)
 }
 
-func (b *Builder) buildJibMavenToRegistry(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibArtifact, tag string) (string, error) {
-	args := GenerateMavenBuildArgs("build", tag, artifact, b.skipTests, b.cfg.GetInsecureRegistries())
+func (b *Builder) buildJibMavenToRegistry(ctx context.Context, out io.Writer, workspace string, artifact *latest.JibArtifact, deps []*latest.ArtifactDependency, tag string) (string, error) {
+	args := GenerateMavenBuildArgs("build", tag, artifact, b.skipTests, b.pushImages, deps, b.artifacts, b.cfg.GetInsecureRegistries())
 	if err := b.runMavenCommand(ctx, out, workspace, args); err != nil {
 		return "", err
 	}
@@ -98,14 +98,14 @@ func getSyncMapCommandMaven(ctx context.Context, workspace string, a *latest.Jib
 }
 
 // GenerateMavenBuildArgs generates the arguments to Maven for building the project as an image.
-func GenerateMavenBuildArgs(goal string, imageName string, a *latest.JibArtifact, skipTests bool, insecureRegistries map[string]bool) []string {
+func GenerateMavenBuildArgs(goal string, imageName string, a *latest.JibArtifact, skipTests, pushImages bool, deps []*latest.ArtifactDependency, r ArtifactResolver, insecureRegistries map[string]bool) []string {
 	args := mavenBuildArgsFunc(goal, a, skipTests, true, MinimumJibMavenVersion)
 	if insecure, err := isOnInsecureRegistry(imageName, insecureRegistries); err == nil && insecure {
 		// jib doesn't support marking specific registries as insecure
 		args = append(args, "-Djib.allowInsecureRegistries=true")
 	}
-	if a.BaseImage != "" {
-		args = append(args, fmt.Sprintf("-Djib.from.image=%s", a.BaseImage))
+	if baseImg, found := baseImageArg(a, r, deps, pushImages); found {
+		args = append(args, baseImg)
 	}
 	args = append(args, "-Dimage="+imageName)