Extends builds.kpack.io with a new optional field spec.builder.ref (buildpacks-community#1857)

rashedkvm · web-flow · commit da640cc195ad · 2025-04-28T16:52:08.000Z
Extends builds.kpack.io with a new optional field spec.builder.ref. This allows build resources to reference resource and cluster builders directly. Few cosmetic cleanup. Added unit test for the new field validation and implementation

Signed-off-by: Rashed Kamal &lt;rashed.kamal@broadcom.com&gt;
diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
@@ -7187,6 +7187,10 @@
           "x-kubernetes-list-type": "",
           "x-kubernetes-patch-merge-key": "name",
           "x-kubernetes-patch-strategy": "merge"
+        },
+        "ref": {
+          "description": "Ref reference to an existing Builder/ClusterBuilder",
+          "$ref": "#/definitions/io.k8s.api.core.v1.ObjectReference"
         }
       }
     },
diff --git a/cmd/controller/main.go b/cmd/controller/main.go
@@ -169,6 +169,7 @@ func main() {
 		MaximumPlatformApiVersion: maxPlatformApi,
 		InjectedSidecarSupport:    featureFlags.InjectedSidecarSupport,
 		SSHTrustUnknownHost:       cfg.SshTrustUnknownHosts,
+		KpackClient:               client,
 	}
 
 	gitResolver := git.NewResolver(k8sClient, cfg.SshTrustUnknownHosts)
diff --git a/hack/local.sh b/hack/local.sh
@@ -13,8 +13,9 @@ Builds and generates a deployment yaml for kpack. This only builds linux images.
 
 Prerequisites:
 - pack or ko installed
-- kubectl installed
+- kubectl installed (recommended)
 - docker login to your registry
+- kbld and kapp (https://carvel.dev/#install)
 
 OPTIONS
   --help                          -h  prints the command usage
@@ -102,7 +103,7 @@ function main() {
 
  if "${apply}"; then
   echo "Applying $output to cluster"
-  kubectl apply -f $output
+  kapp deploy -a kpack -f $output -y
  fi
 
 }
diff --git a/pkg/apis/build/v1alpha1/build_validation_test.go b/pkg/apis/build/v1alpha1/build_validation_test.go
@@ -80,7 +80,7 @@ func testBuildValidation(t *testing.T, when spec.G, it spec.S) {
 
 		it("missing builder name", func() {
 			build.Spec.Builder.Image = ""
-			assertValidationError(build, apis.ErrMissingField("image").ViaField("spec", "builder"))
+			assertValidationError(build, apis.ErrMissingField("image", "ref").ViaField("spec", "builder"))
 		})
 
 		it("invalid builder name", func() {
@@ -217,7 +217,7 @@ func testBuildValidation(t *testing.T, when spec.G, it spec.S) {
 			build.Spec.Builder.Image = ""
 			assertValidationError(build,
 				apis.ErrMissingField("tags").ViaField("spec").
-					Also(apis.ErrMissingField("image").ViaField("spec", "builder")))
+					Also(apis.ErrMissingField("image", "ref").ViaField("spec", "builder")))
 		})
 
 		it("validates spec is immutable", func() {
diff --git a/pkg/apis/build/v1alpha2/build_pod.go b/pkg/apis/build/v1alpha2/build_pod.go
@@ -104,11 +104,12 @@ type BuildContext struct {
 }
 
 type BuildPodBuilderConfig struct {
-	StackID      string
-	RunImage     string
-	Uid          int64
-	Gid          int64
-	PlatformAPIs []string
+	StackID       string
+	RunImage      string
+	Uid           int64
+	Gid           int64
+	PlatformAPIs  []string
+	ResolvedImage string
 }
 
 var (
@@ -240,7 +241,7 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor
 
 	analyzeContainer := corev1.Container{
 		Name:      AnalyzeContainerName,
-		Image:     b.Spec.Builder.Image,
+		Image:     buildContext.BuildPodBuilderConfig.ResolvedImage,
 		Command:   []string{AnalyzeCommand},
 		Resources: b.Spec.Resources,
 		Args: args(
@@ -282,7 +283,7 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor
 
 	detectContainer := corev1.Container{
 		Name:      DetectContainerName,
-		Image:     b.Spec.Builder.Image,
+		Image:     buildContext.BuildPodBuilderConfig.ResolvedImage,
 		Command:   []string{DetectCommand},
 		Resources: b.Spec.Resources,
 		Args: []string{
@@ -432,7 +433,7 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor
 				step(
 					corev1.Container{
 						Name:            RestoreContainerName,
-						Image:           b.Spec.Builder.Image,
+						Image:           buildContext.BuildPodBuilderConfig.ResolvedImage,
 						Command:         []string{RestoreCommand},
 						Resources:       b.Spec.Resources,
 						SecurityContext: containerSecurityContext(),
@@ -456,7 +457,7 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor
 				step(
 					corev1.Container{
 						Name:            BuildContainerName,
-						Image:           b.Spec.Builder.Image,
+						Image:           buildContext.BuildPodBuilderConfig.ResolvedImage,
 						Command:         []string{BuildCommand},
 						Resources:       b.Spec.Resources,
 						SecurityContext: containerSecurityContext(),
@@ -481,7 +482,7 @@ func (b *Build) BuildPod(images BuildPodImages, buildContext BuildContext) (*cor
 				step(
 					corev1.Container{
 						Name:            ExportContainerName,
-						Image:           b.Spec.Builder.Image,
+						Image:           buildContext.BuildPodBuilderConfig.ResolvedImage,
 						Command:         []string{ExportCommand},
 						Resources:       b.Spec.Resources,
 						SecurityContext: containerSecurityContext(),
@@ -919,7 +920,7 @@ func (b *Build) setupSecretVolumesAndArgs(secrets []corev1.Secret, filter func(s
 			annotatedUrl := secret.Annotations[BlobSecretAnnotationPrefix]
 			args = append(args, fmt.Sprintf("-blob=%s=%s", secret.Name, annotatedUrl))
 		default:
-			//ignoring secret
+			// ignoring secret
 			continue
 		}
 
diff --git a/pkg/apis/build/v1alpha2/build_pod_test.go b/pkg/apis/build/v1alpha2/build_pod_test.go
@@ -249,11 +249,12 @@ func testBuildPod(t *testing.T, when spec.G, it spec.S) {
 
 	buildContext := buildapi.BuildContext{
 		BuildPodBuilderConfig: buildapi.BuildPodBuilderConfig{
-			StackID:      "com.builder.stack.io",
-			RunImage:     "builderregistry.io/run",
-			Uid:          2000,
-			Gid:          3000,
-			PlatformAPIs: []string{"0.7", "0.8", "0.9"},
+			StackID:       "com.builder.stack.io",
+			RunImage:      "builderregistry.io/run",
+			Uid:           2000,
+			Gid:           3000,
+			PlatformAPIs:  []string{"0.7", "0.8", "0.9"},
+			ResolvedImage: builderImage,
 		},
 		Secrets:  secrets,
 		Bindings: serviceBindings,
diff --git a/pkg/apis/build/v1alpha2/build_validation_test.go b/pkg/apis/build/v1alpha2/build_validation_test.go
@@ -38,6 +38,27 @@ func testBuildValidation(t *testing.T, when spec.G, it spec.S) {
 			},
 		},
 	}
+	buildUsingRef := &Build{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "buildWithRef-name",
+		},
+		Spec: BuildSpec{
+			Tags: []string{"some/image"},
+			Builder: corev1alpha1.BuildBuilderSpec{
+				Ref: &corev1.ObjectReference{
+					Name: "default",
+					Kind: "ClusterBuilder",
+				},
+			},
+			ServiceAccountName: "some/service-account",
+			Source: corev1alpha1.SourceConfig{
+				Git: &corev1alpha1.Git{
+					URL:      "http://github.com/repo",
+					Revision: "master",
+				},
+			},
+		},
+	}
 	when("Default", func() {
 		it("does not modify already set fields", func() {
 			oldBuild := build.DeepCopy()
@@ -81,14 +102,32 @@ func testBuildValidation(t *testing.T, when spec.G, it spec.S) {
 
 		it("missing builder name", func() {
 			build.Spec.Builder.Image = ""
-			assertValidationError(build, context.TODO(), apis.ErrMissingField("image").ViaField("spec", "builder"))
+			assertValidationError(build, context.TODO(), apis.ErrMissingField("image", "ref").ViaField("spec", "builder"))
 		})
 
 		it("invalid builder name", func() {
 			build.Spec.Builder.Image = "foo.ioo/builder-but-not-a-builder@sha256:alksdifhjalsouidfh"
 			assertValidationError(build, context.TODO(), apis.ErrInvalidValue("foo.ioo/builder-but-not-a-builder@sha256:alksdifhjalsouidfh", "image").ViaField("spec", "builder"))
 		})
 
+		it("valid cluster builder by ref", func() {
+			assert.Nil(t, buildUsingRef.Validate(context.TODO()))
+		})
+
+		it("both image and builder by ref present", func() {
+			buildUsingRef.Spec.Builder.Image = "builder/bionic-builder@sha256:e431a4f94fb84854fd081da62762192f36fd093fdfb85ad3bc009b9309524e2d"
+			assertValidationError(buildUsingRef, context.TODO(), apis.ErrMultipleOneOf("image", "ref").ViaField("spec", "builder"))
+		})
+
+		it("validate namespaced builder by ref", func() {
+			buildUsingRef.Spec.Builder.Ref = &corev1.ObjectReference{
+				Kind:      "Builder",
+				Name:      "namespaced-builder",
+				Namespace: "default",
+			}
+			assert.Nil(t, buildUsingRef.Validate(context.TODO()))
+		})
+
 		it("multiple sources", func() {
 			build.Spec.Source.Git = &corev1alpha1.Git{
 				URL:      "http://github.com/repo",
@@ -331,7 +370,7 @@ func testBuildValidation(t *testing.T, when spec.G, it spec.S) {
 			build.Spec.Builder.Image = ""
 			assertValidationError(build, context.TODO(),
 				apis.ErrMissingField("tags").ViaField("spec").
-					Also(apis.ErrMissingField("image").ViaField("spec", "builder")))
+					Also(apis.ErrMissingField("image", "ref").ViaField("spec", "builder")))
 		})
 
 		it("validates spec is immutable", func() {
diff --git a/pkg/apis/build/v1alpha2/cluster_store_conversion_test.go b/pkg/apis/build/v1alpha2/cluster_store_conversion_test.go
@@ -24,7 +24,7 @@ func testClusterStoreConversion(t *testing.T, when spec.G, it spec.S) {
 				Annotations: map[string]string{"some-key": "some-value"},
 			},
 			Spec: ClusterStoreSpec{
-				Sources: []corev1alpha1.ImageSource{{"some-image"}, {"another-image"}},
+				Sources: []corev1alpha1.ImageSource{{Image: "some-image"}, {Image: "another-image"}},
 				ServiceAccountRef: &corev1.ObjectReference{
 					Namespace: "some-namespace",
 					Name:      "some-service-account",
@@ -65,7 +65,7 @@ func testClusterStoreConversion(t *testing.T, when spec.G, it spec.S) {
 				},
 			},
 			Spec: v1alpha1.ClusterStoreSpec{
-				Sources: []corev1alpha1.ImageSource{{"some-image"}, {"another-image"}},
+				Sources: []corev1alpha1.ImageSource{{Image: "some-image"}, {Image: "another-image"}},
 			},
 			Status: v1alpha1.ClusterStoreStatus{
 				Status: corev1alpha1.Status{},
diff --git a/pkg/apis/core/v1alpha1/build_types.go b/pkg/apis/core/v1alpha1/build_types.go
@@ -19,6 +19,9 @@ type BuildBuilderSpec struct {
 	// +patchStrategy=merge
 	// +listType
 	ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty" patchStrategy:"merge" patchMergeKey:"name" protobuf:"bytes,15,rep,name=imagePullSecrets"`
+
+	// Ref reference to an existing Builder/ClusterBuilder
+	Ref *corev1.ObjectReference `json:"ref,omitempty"`
 }
 
 // +k8s:openapi-gen=true
diff --git a/pkg/apis/core/v1alpha1/build_validation.go b/pkg/apis/core/v1alpha1/build_validation.go
@@ -5,13 +5,48 @@ import (
 	"fmt"
 	"regexp"
 
-	"knative.dev/pkg/apis"
-
+	"github.com/google/go-containerregistry/pkg/name"
 	"github.com/pivotal/kpack/pkg/apis/validate"
+	corev1 "k8s.io/api/core/v1"
+	"knative.dev/pkg/apis"
 )
 
 func (bbs *BuildBuilderSpec) Validate(ctx context.Context) *apis.FieldError {
-	return validate.Image(bbs.Image)
+	return validateBuilderOrImagePresent(bbs).
+		Also(validateBuilderRefAndImageMutuallyExclusive(bbs)).
+		Also(validateBuilderRef(bbs.Ref).ViaField("ref")).
+		Also(validateImage(bbs.Image))
+}
+
+func validateImage(value string) *apis.FieldError {
+	if value != "" {
+		_, err := name.ParseReference(value, name.WeakValidation)
+		if err != nil {
+			return apis.ErrInvalidValue(value, "image")
+		}
+	}
+
+	return nil
+}
+
+func validateBuilderRef(ref *corev1.ObjectReference) *apis.FieldError {
+	if ref != nil {
+		return validate.FieldNotEmpty(ref.Name, "name").Also(validate.FieldNotEmpty(ref.Kind, "kind"))
+	}
+	return nil
+}
+
+func validateBuilderRefAndImageMutuallyExclusive(bbs *BuildBuilderSpec) *apis.FieldError {
+	if bbs.Image != "" && bbs.Ref != nil {
+		return apis.ErrMultipleOneOf("image", "ref")
+	}
+	return nil
+}
+func validateBuilderOrImagePresent(bbs *BuildBuilderSpec) *apis.FieldError {
+	if bbs.Image == "" && bbs.Ref == nil {
+		return apis.ErrMissingField("image", "ref")
+	}
+	return nil
 }
 
 func (bs CNBBindings) Validate(ctx context.Context) *apis.FieldError {
diff --git a/pkg/apis/core/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/core/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/buildpod/generator.go b/pkg/buildpod/generator.go
diff --git a/pkg/buildpod/generator_test.go b/pkg/buildpod/generator_test.go
diff --git a/pkg/dockercreds/k8sdockercreds/k8s_keychain_test.go b/pkg/dockercreds/k8sdockercreds/k8s_keychain_test.go
diff --git a/pkg/openapi/generated.openapi.go b/pkg/openapi/generated.openapi.go

Original file line number	Diff line number	Diff line change
`@@ -7187,6 +7187,10 @@`
`7187`	`7187`	`"x-kubernetes-list-type": "",`
`7188`	`7188`	`"x-kubernetes-patch-merge-key": "name",`
`7189`	`7189`	`"x-kubernetes-patch-strategy": "merge"`
	`7190`	`+ },`
	`7191`	`+ "ref": {`
	`7192`	`+ "description": "Ref reference to an existing Builder/ClusterBuilder",`
	`7193`	`+ "$ref": "#/definitions/io.k8s.api.core.v1.ObjectReference"`
`7190`	`7194`	`}`
`7191`	`7195`	`}`
`7192`	`7196`	`},`
Original file line number	Diff line number	Diff line change
`@@ -169,6 +169,7 @@ func main() {`
`169`	`169`	`MaximumPlatformApiVersion: maxPlatformApi,`
`170`	`170`	`InjectedSidecarSupport: featureFlags.InjectedSidecarSupport,`
`171`	`171`	`SSHTrustUnknownHost: cfg.SshTrustUnknownHosts,`
	`172`	`+ KpackClient: client,`
`172`	`173`	`}`
`173`	`174`
`174`	`175`	`gitResolver := git.NewResolver(k8sClient, cfg.SshTrustUnknownHosts)`
Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,9 @@ type BuildBuilderSpec struct {`
`19`	`19`	`// +patchStrategy=merge`
`20`	`20`	`// +listType`
`21`	`21`	ImagePullSecrets []corev1.LocalObjectReference `json:"imagePullSecrets,omitempty" patchStrategy:"merge" patchMergeKey:"name" protobuf:"bytes,15,rep,name=imagePullSecrets"`
	`22`	`+`
	`23`	`+ // Ref reference to an existing Builder/ClusterBuilder`
	`24`	+ Ref *corev1.ObjectReference `json:"ref,omitempty"`
`22`	`25`	`}`
`23`	`26`
`24`	`27`	`// +k8s:openapi-gen=true`