API changes

- Make Build.Spec.Source optional - Rename Build.Spec.Source.GitSource to Git (go-type only) - Rename Build.Spec.Source.LocalSource to Local (go-type only) - Rename BuildRun.Spec.Source.LocalSource to Local (go-type only) - Remove undocumented CONTEXT_DIR parameter - Declare Build.Spec.Source.Type as required - Declare BuildRun.Spec.Source.Type as required
shipwright-io · Feb 23, 2024 · 18de56a · 18de56a
1 parent 6ee1cd8
commit 18de56a
Show file tree

Hide file tree

Showing 25 changed files with 253 additions and 245 deletions.
diff --git a/Makefile b/Makefile
@@ -198,7 +198,7 @@ test-unit-ginkgo: ginkgo
 		-compilers=2 \
 		-race \
 		-trace \
-		cmd/... \
+		xcmd/... \
 		pkg/...
 
 # Based on https://github.com/kubernetes/community/blob/master/contributors/devel/sig-testing/integration-tests.md

diff --git a/pkg/apis/build/v1beta1/build_conversion.go b/pkg/apis/build/v1beta1/build_conversion.go
@@ -54,14 +54,6 @@ func (src *Build) ConvertTo(ctx context.Context, obj *unstructured.Unstructured)
 		alphaBuild.ObjectMeta.Annotations[v1alpha1.AnnotationBuildRunDeletion] = strconv.FormatBool(*src.Spec.Retention.AtBuildDeletion)
 	}
 
-	// convert OCIArtifact to Bundle
-	if src.Spec.Source.OCIArtifact != nil {
-		alphaBuild.Spec.Source.BundleContainer = &v1alpha1.BundleContainer{
-			Image: src.Spec.Source.OCIArtifact.Image,
-			Prune: (*v1alpha1.PruneOption)(src.Spec.Source.OCIArtifact.Prune),
-		}
-	}
-
 	mapito, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&alphaBuild)
 	if err != nil {
 		ctxlog.Error(ctx, err, "failed structuring the newObject")
@@ -111,40 +103,44 @@ func (src *Build) ConvertFrom(ctx context.Context, obj *unstructured.Unstructure
 
 func (dest *BuildSpec) ConvertFrom(orig *v1alpha1.BuildSpec) error {
 	// Handle BuildSpec Source
-	specSource := Source{}
 
 	// only interested on spec.sources as long as an item of the list
 	// is of the type LocalCopy. Otherwise, we move into bundle or git types.
 	index, isLocal := v1alpha1.IsLocalCopyType(orig.Sources)
 	if isLocal {
-		specSource.Type = LocalType
-		specSource.LocalSource = &Local{
-			Name:    orig.Sources[index].Name,
-			Timeout: orig.Sources[index].Timeout,
+		dest.Source = &Source{
+			Type: LocalType,
+			Local: &Local{
+				Name:    orig.Sources[index].Name,
+				Timeout: orig.Sources[index].Timeout,
+			},
+			ContextDir: orig.Source.ContextDir,
 		}
-	} else {
-		if orig.Source.BundleContainer != nil {
-			specSource.Type = OCIArtifactType
-			specSource.OCIArtifact = &OCIArtifact{
+	} else if orig.Source.BundleContainer != nil {
+		dest.Source = &Source{
+			Type: OCIArtifactType,
+			OCIArtifact: &OCIArtifact{
 				Image: orig.Source.BundleContainer.Image,
 				Prune: (*PruneOption)(orig.Source.BundleContainer.Prune),
-			}
-			if orig.Source.Credentials != nil {
-				specSource.OCIArtifact.PullSecret = &orig.Source.Credentials.Name
-			}
-		} else if orig.Source.URL != nil {
-			specSource.Type = GitType
-			specSource.GitSource = &Git{
+			},
+			ContextDir: orig.Source.ContextDir,
+		}
+		if orig.Source.Credentials != nil {
+			dest.Source.OCIArtifact.PullSecret = &orig.Source.Credentials.Name
+		}
+	} else if orig.Source.URL != nil {
+		dest.Source = &Source{
+			Type: GitType,
+			Git: &Git{
 				URL:      *orig.Source.URL,
 				Revision: orig.Source.Revision,
-			}
-			if orig.Source.Credentials != nil {
-				specSource.GitSource.CloneSecret = &orig.Source.Credentials.Name
-			}
+			},
+			ContextDir: orig.Source.ContextDir,
+		}
+		if orig.Source.Credentials != nil {
+			dest.Source.Git.CloneSecret = &orig.Source.Credentials.Name
 		}
 	}
-	specSource.ContextDir = orig.Source.ContextDir
-	dest.Source = specSource
 
 	// Handle BuildSpec Triggers
 	if orig.Trigger != nil {
@@ -232,11 +228,11 @@ func (dest *BuildSpec) ConvertFrom(orig *v1alpha1.BuildSpec) error {
 
 func (dest *BuildSpec) ConvertTo(bs *v1alpha1.BuildSpec) error {
 	// Handle BuildSpec Sources or Source
-	if dest.Source.Type == LocalType && dest.Source.LocalSource != nil {
+	if dest.Source != nil && dest.Source.Type == LocalType && dest.Source.Local != nil {
 		bs.Sources = append(bs.Sources, v1alpha1.BuildSource{
-			Name:    dest.Source.LocalSource.Name,
+			Name:    dest.Source.Local.Name,
 			Type:    v1alpha1.LocalCopy,
-			Timeout: dest.Source.LocalSource.Timeout,
+			Timeout: dest.Source.Local.Timeout,
 		})
 	} else {
 		bs.Source = getAlphaBuildSource(*dest)
@@ -419,6 +415,11 @@ func convertToBetaTriggers(orig *v1alpha1.TriggerWhen) TriggerWhen {
 
 func getAlphaBuildSource(src BuildSpec) v1alpha1.Source {
 	source := v1alpha1.Source{}
+
+	if src.Source == nil {
+		return source
+	}
+
 	var credentials corev1.LocalObjectReference
 	var revision *string
 
@@ -434,14 +435,14 @@ func getAlphaBuildSource(src BuildSpec) v1alpha1.Source {
 			Prune: (*v1alpha1.PruneOption)(src.Source.OCIArtifact.Prune),
 		}
 	default:
-		if src.Source.GitSource != nil && src.Source.GitSource.CloneSecret != nil {
+		if src.Source.Git != nil && src.Source.Git.CloneSecret != nil {
 			credentials = corev1.LocalObjectReference{
-				Name: *src.Source.GitSource.CloneSecret,
+				Name: *src.Source.Git.CloneSecret,
 			}
 		}
-		if src.Source.GitSource != nil {
-			source.URL = &src.Source.GitSource.URL
-			revision = src.Source.GitSource.Revision
+		if src.Source.Git != nil {
+			source.URL = &src.Source.Git.URL
+			revision = src.Source.Git.Revision
 		}
 
 	}

diff --git a/pkg/apis/build/v1beta1/build_types.go b/pkg/apis/build/v1beta1/build_types.go
@@ -110,7 +110,7 @@ type BuildSpec struct {
 	// Source refers to the location where the source code is,
 	// this could be a git repository, a local source or an oci
 	// artifact
-	Source Source `json:"source"`
+	Source *Source `json:"source"`
 
 	// Trigger defines the scenarios where a new build should be triggered.
 	//
@@ -278,14 +278,18 @@ func init() {
 
 // GetSourceCredentials returns the secret name for a Build Source
 func (b Build) GetSourceCredentials() *string {
+	if b.Spec.Source == nil {
+		return nil
+	}
+
 	switch b.Spec.Source.Type {
 	case OCIArtifactType:
 		if b.Spec.Source.OCIArtifact != nil && b.Spec.Source.OCIArtifact.PullSecret != nil {
 			return b.Spec.Source.OCIArtifact.PullSecret
 		}
 	default:
-		if b.Spec.Source.GitSource != nil && b.Spec.Source.GitSource.CloneSecret != nil {
-			return b.Spec.Source.GitSource.CloneSecret
+		if b.Spec.Source.Git != nil && b.Spec.Source.Git.CloneSecret != nil {
+			return b.Spec.Source.Git.CloneSecret
 		}
 	}
 	return nil

diff --git a/pkg/apis/build/v1beta1/buildrun_conversion.go b/pkg/apis/build/v1beta1/buildrun_conversion.go
@@ -43,11 +43,11 @@ func (src *BuildRun) ConvertTo(ctx context.Context, obj *unstructured.Unstructur
 	}
 
 	// BuildRunSpec Sources
-	if src.Spec.Source != nil && src.Spec.Source.Type == LocalType && src.Spec.Source.LocalSource != nil {
+	if src.Spec.Source != nil && src.Spec.Source.Type == LocalType && src.Spec.Source.Local != nil {
 		alphaBuildRun.Spec.Sources = append(alphaBuildRun.Spec.Sources, v1alpha1.BuildSource{
-			Name:    src.Spec.Source.LocalSource.Name,
+			Name:    src.Spec.Source.Local.Name,
 			Type:    v1alpha1.LocalCopy,
-			Timeout: src.Spec.Source.LocalSource.Timeout,
+			Timeout: src.Spec.Source.Local.Timeout,
 		})
 	}
 
@@ -268,7 +268,7 @@ func (dest *BuildRunSpec) ConvertFrom(orig *v1alpha1.BuildRunSpec) error {
 	if isLocal {
 		dest.Source = &BuildRunSource{
 			Type: LocalType,
-			LocalSource: &Local{
+			Local: &Local{
 				Name:    orig.Sources[index].Name,
 				Timeout: orig.Sources[index].Timeout,
 			},

diff --git a/pkg/apis/build/v1beta1/source.go b/pkg/apis/build/v1beta1/source.go
@@ -84,9 +84,7 @@ type OCIArtifact struct {
 // Source describes the Git source repository to fetch.
 type Source struct {
 	// Type is the BuildSource qualifier, the type of the data-source.
-	//
-	// +optional
-	Type BuildSourceType `json:"type,omitempty"`
+	Type BuildSourceType `json:"type"`
 
 	// ContextDir is a path to subfolder in the repo. Optional.
 	//
@@ -98,25 +96,24 @@ type Source struct {
 	// +optional
 	OCIArtifact *OCIArtifact `json:"ociArtifact,omitempty"`
 
-	// GitSource
+	// Git
 	//
 	// +optional
-	GitSource *Git `json:"git,omitempty"`
+	Git *Git `json:"git,omitempty"`
 
-	// LocalSource
+	// Local
 	//
 	// +optional
-	LocalSource *Local `json:"local,omitempty"`
+	Local *Local `json:"local,omitempty"`
 }
 
 // BuildRunSource describes the local source to use
 type BuildRunSource struct {
 	// Type is the BuildRunSource qualifier, the type of the data-source.
 	// Only LocalType is supported.
 	//
-	// +optional
-	Type BuildSourceType `json:"type,omitempty"`
-	// LocalSource
+	Type BuildSourceType `json:"type"`
+	// Local
 	//
-	LocalSource *Local `json:"local,omitempty"`
+	Local *Local `json:"local,omitempty"`
 }
diff --git a/pkg/apis/build/v1beta1/zz_generated.deepcopy.go b/pkg/apis/build/v1beta1/zz_generated.deepcopy.go
diff --git a/pkg/reconciler/build/build_test.go b/pkg/reconciler/build/build_test.go
@@ -83,7 +83,7 @@ var _ = Describe("Reconcile Build", func() {
 	Describe("Reconcile", func() {
 		Context("when source secret is specified", func() {
 			It("fails when the secret does not exist", func() {
-				buildSample.Spec.Source.GitSource.CloneSecret = pointer.String("non-existing")
+				buildSample.Spec.Source.Git.CloneSecret = pointer.String("non-existing")
 
 				buildSample.Spec.Output.PushSecret = nil
 
@@ -96,7 +96,7 @@ var _ = Describe("Reconcile Build", func() {
 			})
 
 			It("succeeds when the secret exists foobar", func() {
-				buildSample.Spec.Source.GitSource.CloneSecret = pointer.String("existing")
+				buildSample.Spec.Source.Git.CloneSecret = pointer.String("existing")
 				buildSample.Spec.Output.PushSecret = nil
 
 				// Fake some client Get calls and ensure we populate all
@@ -162,7 +162,7 @@ var _ = Describe("Reconcile Build", func() {
 
 		Context("when source secret and output secret are specified", func() {
 			It("fails when both secrets do not exist", func() {
-				buildSample.Spec.Source.GitSource.CloneSecret = pointer.String("non-existing-source")
+				buildSample.Spec.Source.Git.CloneSecret = pointer.String("non-existing-source")
 				buildSample.Spec.Output.PushSecret = pointer.String("non-existing-output")
 
 				statusCall := ctl.StubFunc(corev1.ConditionFalse, build.MultipleSecretRefNotFound, "missing secrets are non-existing-output,non-existing-source")
@@ -312,7 +312,7 @@ var _ = Describe("Reconcile Build", func() {
 		Context("when source URL is specified", func() {
 			// validate file protocol
 			It("fails when source URL is invalid", func() {
-				buildSample.Spec.Source.GitSource.URL = "foobar"
+				buildSample.Spec.Source.Git.URL = "foobar"
 				buildSample.SetAnnotations(map[string]string{
 					build.AnnotationBuildVerifyRepository: "true",
 				})
@@ -326,7 +326,7 @@ var _ = Describe("Reconcile Build", func() {
 
 			// validate https protocol
 			It("fails when public source URL is unreachable", func() {
-				buildSample.Spec.Source.GitSource.URL = "https://github.com/shipwright-io/sample-go-fake"
+				buildSample.Spec.Source.Git.URL = "https://github.com/shipwright-io/sample-go-fake"
 				buildSample.SetAnnotations(map[string]string{
 					build.AnnotationBuildVerifyRepository: "true",
 				})
@@ -341,7 +341,7 @@ var _ = Describe("Reconcile Build", func() {
 
 			// skip validation because of empty sourceURL annotation
 			It("succeed when source URL is invalid because source annotation is empty", func() {
-				buildSample.Spec.Source.GitSource.URL = "foobar"
+				buildSample.Spec.Source.Git.URL = "foobar"
 
 				// Fake some client Get calls and ensure we populate all
 				// different resources we could get during reconciliation
@@ -392,8 +392,8 @@ var _ = Describe("Reconcile Build", func() {
 			// skip validation because build references a sourceURL secret
 			It("succeed when source URL is fake private URL because build reference a sourceURL secret", func() {
 				buildSample := ctl.BuildWithClusterBuildStrategyAndSourceSecret(buildName, namespace, buildStrategyName)
-				buildSample.Spec.Source.GitSource.URL = "https://github.yourco.com/org/build-fake"
-				buildSample.Spec.Source.GitSource.CloneSecret = pointer.String(registrySecret)
+				buildSample.Spec.Source.Git.URL = "https://github.yourco.com/org/build-fake"
+				buildSample.Spec.Source.Git.CloneSecret = pointer.String(registrySecret)
 
 				// Fake some client Get calls and ensure we populate all
 				// different resources we could get during reconciliation
@@ -420,7 +420,7 @@ var _ = Describe("Reconcile Build", func() {
 
 		Context("when environment variables are specified", func() {
 			JustBeforeEach(func() {
-				buildSample.Spec.Source.GitSource.CloneSecret = pointer.String("existing")
+				buildSample.Spec.Source.Git.CloneSecret = pointer.String("existing")
 				buildSample.Spec.Output.PushSecret = nil
 
 				// Fake some client Get calls and ensure we populate all

diff --git a/pkg/reconciler/buildrun/buildrun_test.go b/pkg/reconciler/buildrun/buildrun_test.go
@@ -1431,9 +1431,9 @@ var _ = Describe("Reconcile BuildRun", func() {
 						Spec: build.BuildRunSpec{
 							Build: build.ReferencedBuild{
 								Build: &build.BuildSpec{
-									Source: build.Source{
+									Source: &build.Source{
 										Type: build.GitType,
-										GitSource: &build.Git{
+										Git: &build.Git{
 											URL: "https://github.com/shipwright-io/sample-go.git",
 										},
 										ContextDir: pointer.String("source-build"),

diff --git a/pkg/reconciler/buildrun/resources/credentials_test.go b/pkg/reconciler/buildrun/resources/credentials_test.go
@@ -37,9 +37,9 @@ var _ = Describe("Credentials", func() {
 		BeforeEach(func() {
 			build = &buildv1beta1.Build{
 				Spec: buildv1beta1.BuildSpec{
-					Source: buildv1beta1.Source{
+					Source: &buildv1beta1.Source{
 						Type: buildv1beta1.GitType,
-						GitSource: &buildv1beta1.Git{
+						Git: &buildv1beta1.Git{
 							URL:         "a/b/c",
 							CloneSecret: pointer.String("secret_a"),
 						},
@@ -83,9 +83,9 @@ var _ = Describe("Credentials", func() {
 		BeforeEach(func() {
 			build = &buildv1beta1.Build{
 				Spec: buildv1beta1.BuildSpec{
-					Source: buildv1beta1.Source{
+					Source: &buildv1beta1.Source{
 						Type: buildv1beta1.GitType,
-						GitSource: &buildv1beta1.Git{
+						Git: &buildv1beta1.Git{
 							URL: "a/b/c",
 						},
 					},
@@ -121,9 +121,9 @@ var _ = Describe("Credentials", func() {
 		BeforeEach(func() {
 			build = &buildv1beta1.Build{
 				Spec: buildv1beta1.BuildSpec{
-					Source: buildv1beta1.Source{
+					Source: &buildv1beta1.Source{
 						Type: buildv1beta1.GitType,
-						GitSource: &buildv1beta1.Git{
+						Git: &buildv1beta1.Git{
 							URL:         "a/b/c",
 							CloneSecret: nil,
 						},