From c3f2045ff9aeda6f0213ab46ecbd73fdd69bf5ce Mon Sep 17 00:00:00 2001 From: encalada Date: Fri, 13 Oct 2023 16:35:27 +0200 Subject: [PATCH 1/7] Ensure conversion for local source takes place - Introduce missing localType object in spec.source for Build - Introduce new spec.source object for BuildRun - Ensure conversion for spec.sources in Build or spec.sources in BuildRun can convert to the spec.source for beta --- pkg/apis/build/v1alpha1/source.go | 10 ++++ pkg/apis/build/v1beta1/build_conversion.go | 57 +++++++++++++------ pkg/apis/build/v1beta1/buildrun_conversion.go | 25 ++++++++ pkg/apis/build/v1beta1/buildrun_types.go | 6 ++ pkg/apis/build/v1beta1/source.go | 20 +++++++ 5 files changed, 101 insertions(+), 17 deletions(-) diff --git a/pkg/apis/build/v1alpha1/source.go b/pkg/apis/build/v1alpha1/source.go index 089a95cc8..8528529ce 100644 --- a/pkg/apis/build/v1alpha1/source.go +++ b/pkg/apis/build/v1alpha1/source.go @@ -65,3 +65,13 @@ type Source struct { // +optional Credentials *corev1.LocalObjectReference `json:"credentials,omitempty"` } + +// IsLocalCopyType tells if we have an entry of the type local +func IsLocalCopyType(sources []BuildSource) (int, bool) { + for i, bs := range sources { + if bs.Type == LocalCopy { + return i, true + } + } + return -1, false +} diff --git a/pkg/apis/build/v1beta1/build_conversion.go b/pkg/apis/build/v1beta1/build_conversion.go index 11ceb95ea..f1e0f297e 100644 --- a/pkg/apis/build/v1beta1/build_conversion.go +++ b/pkg/apis/build/v1beta1/build_conversion.go @@ -106,23 +106,35 @@ func (src *Build) ConvertFrom(ctx context.Context, obj *unstructured.Unstructure func (dest *BuildSpec) ConvertFrom(orig *v1alpha1.BuildSpec) error { // Handle BuildSpec Source specSource := Source{} - if orig.Source.BundleContainer != nil { - specSource.Type = OCIArtifactType - specSource.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 + + // 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 len(orig.Sources) > 0 && isLocal { + specSource.Type = LocalType + specSource.LocalSource = &Local{ + Name: orig.Sources[index].Name, + Timeout: orig.Sources[index].Timeout, } } else { - specSource.Type = GitType - specSource.GitSource = &Git{ - URL: orig.Source.URL, - Revision: orig.Source.Revision, - } - if orig.Source.Credentials != nil { - specSource.GitSource.CloneSecret = &orig.Source.Credentials.Name + if orig.Source.BundleContainer != nil { + specSource.Type = OCIArtifactType + specSource.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 { + specSource.Type = GitType + specSource.GitSource = &Git{ + URL: orig.Source.URL, + Revision: orig.Source.Revision, + } + if orig.Source.Credentials != nil { + specSource.GitSource.CloneSecret = &orig.Source.Credentials.Name + } } } specSource.ContextDir = orig.Source.ContextDir @@ -213,8 +225,19 @@ func (dest *BuildSpec) ConvertFrom(orig *v1alpha1.BuildSpec) error { } func (dest *BuildSpec) ConvertTo(bs *v1alpha1.BuildSpec) error { - // Handle BuildSpec Source - bs.Source = getAlphaBuildSource(*dest) + // Handle BuildSpec Sources or Source + if dest.Source.Type == LocalType { + bs.Sources = []v1alpha1.BuildSource{} + if dest.Source.LocalSource != nil { + bs.Sources = append(bs.Sources, v1alpha1.BuildSource{ + Name: dest.Source.LocalSource.Name, + Type: v1alpha1.LocalCopy, + Timeout: dest.Source.LocalSource.Timeout, + }) + } + } else { + bs.Source = getAlphaBuildSource(*dest) + } // Handle BuildSpec Trigger if dest.Trigger != nil { diff --git a/pkg/apis/build/v1beta1/buildrun_conversion.go b/pkg/apis/build/v1beta1/buildrun_conversion.go index d55b7dad9..bdde2a86b 100644 --- a/pkg/apis/build/v1beta1/buildrun_conversion.go +++ b/pkg/apis/build/v1beta1/buildrun_conversion.go @@ -42,6 +42,18 @@ func (src *BuildRun) ConvertTo(ctx context.Context, obj *unstructured.Unstructur } } + // BuildRunSpec Sources + if src.Spec.Source != nil { + alphaBuildRun.Spec.Sources = []v1alpha1.BuildSource{} + if src.Spec.Source.Type == LocalType { + alphaBuildRun.Spec.Sources = append(alphaBuildRun.Spec.Sources, v1alpha1.BuildSource{ + Name: src.Spec.Source.LocalSource.Name, + Type: v1alpha1.LocalCopy, + Timeout: src.Spec.Source.LocalSource.Timeout, + }) + } + } + // BuildRunSpec ServiceAccount // With the deprecation of serviceAccount.Generate, serviceAccount is set to ".generate" to have the SA created on fly. if src.Spec.ServiceAccount != nil && *src.Spec.ServiceAccount == ".generate" { @@ -191,6 +203,19 @@ func (dest *BuildRunSpec) ConvertFrom(orig *v1alpha1.BuildRunSpec) error { dest.Build.Name = orig.BuildRef.Name } + // 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 len(orig.Sources) > 0 && isLocal { + dest.Source = &BuildRunSource{ + Type: LocalType, + LocalSource: &Local{ + Name: orig.Sources[index].Name, + Timeout: orig.Sources[index].Timeout, + }, + } + } + if orig.ServiceAccount != nil { dest.ServiceAccount = orig.ServiceAccount.Name } diff --git a/pkg/apis/build/v1beta1/buildrun_types.go b/pkg/apis/build/v1beta1/buildrun_types.go index af06728f7..087ae7e8d 100644 --- a/pkg/apis/build/v1beta1/buildrun_types.go +++ b/pkg/apis/build/v1beta1/buildrun_types.go @@ -39,6 +39,12 @@ type BuildRunSpec struct { // +optional Build *ReferencedBuild `json:"build,omitempty"` + // Source refers to the location where the source code is, + // this could only be a local source + // + // +optional + Source *BuildRunSource `json:"source,omitempty"` + // ServiceAccount refers to the kubernetes serviceaccount // which is used for resource control. // Default serviceaccount will be set if it is empty diff --git a/pkg/apis/build/v1beta1/source.go b/pkg/apis/build/v1beta1/source.go index ee1bcfa72..dad731678 100644 --- a/pkg/apis/build/v1beta1/source.go +++ b/pkg/apis/build/v1beta1/source.go @@ -36,6 +36,9 @@ type Local struct { // // +optional Timeout *metav1.Duration `json:"timeout,omitempty"` + + // Name of the local step + Name string `json:"name,omitempty"` } // Git describes the git repository to pull @@ -101,4 +104,21 @@ type Source struct { // // +optional GitSource *Git `json:"git,omitempty"` + + // LocalSource + // + // +optional + LocalSource *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 + // + LocalSource *Local `json:"local,omitempty"` } From 412e4b279726d3fc70696aa88de3b4d9ab93b89f Mon Sep 17 00:00:00 2001 From: encalada Date: Wed, 18 Oct 2023 11:09:57 +0200 Subject: [PATCH 2/7] Add conversion webhook unit test for local - Validate conversion from beta to alpha, alpha to beta, for the Build CR's. - Validate conversion from beta to alpha, alpha to beta, for the BuildRun CR's. --- pkg/webhook/conversion/converter_test.go | 262 +++++++++++++++++++++++ 1 file changed, 262 insertions(+) diff --git a/pkg/webhook/conversion/converter_test.go b/pkg/webhook/conversion/converter_test.go index f9d378038..fdb3dfd81 100644 --- a/pkg/webhook/conversion/converter_test.go +++ b/pkg/webhook/conversion/converter_test.go @@ -61,6 +61,72 @@ var _ = Describe("ConvertCRD", func() { Context("for a Build CR from v1beta1 to v1alpha1", func() { var desiredAPIVersion = "shipwright.io/v1alpha1" + It("converts for spec source Local type", func() { + // Create the yaml in v1beta1 + buildTemplate := `kind: ConversionReview +apiVersion: %s +request: + uid: 0000-0000-0000-0000 + desiredAPIVersion: %s + objects: + - apiVersion: shipwright.io/v1beta1 + kind: Build + metadata: + name: buildkit-build + spec: + source: + type: Local + local: + timeout: 1m + name: foobar_local + strategy: + name: %s + kind: %s +` + o := fmt.Sprintf(buildTemplate, apiVersion, + desiredAPIVersion, strategyName, strategyKind) + + // Invoke the /convert webhook endpoint + conversionReview, err := getConversionReview(o) + Expect(err).To(BeNil()) + Expect(conversionReview.Response.Result.Status).To(Equal(v1.StatusSuccess)) + + convertedObj, err := ToUnstructured(conversionReview) + Expect(err).To(BeNil()) + + build, err := toV1Alpha1BuildObject(convertedObj) + Expect(err).To(BeNil()) + + // Prepare our desired v1alpha1 Build + desiredBuild := v1alpha1.Build{ + TypeMeta: v1.TypeMeta{ + APIVersion: "shipwright.io/v1alpha1", + Kind: "Build", + }, + ObjectMeta: v1.ObjectMeta{ + Name: "buildkit-build", + }, + Spec: v1alpha1.BuildSpec{ + Source: v1alpha1.Source{}, + Sources: []v1alpha1.BuildSource{ + { + Name: "foobar_local", + Type: v1alpha1.LocalCopy, + Timeout: &v1.Duration{ + Duration: 1 * time.Minute, + }, + }, + }, + Strategy: v1alpha1.Strategy{ + Name: strategyName, + Kind: (*v1alpha1.BuildStrategyKind)(&strategyKind), + }, + }, + } + + // Use ComparableTo and assert the whole object + Expect(build).To(BeComparableTo(desiredBuild)) + }) It("converts for spec source OCIArtifacts type, strategy and triggers", func() { branchMain := "main" branchDev := "develop" @@ -401,6 +467,72 @@ request: Context("for a Build CR from v1alpha1 to v1beta1", func() { var desiredAPIVersion = "shipwright.io/v1beta1" + It("converts for spec sources local to source local", func() { + // Create the yaml in v1alpha1 + // When source and sources are present, if sources with local type + // exists, we will convert to Local type Source, and ignore the source url + buildTemplate := `kind: ConversionReview +apiVersion: %s +request: + uid: 0000-0000-0000-0000 + desiredAPIVersion: %s + objects: + - apiVersion: shipwright.io/v1alpha1 + kind: Build + metadata: + name: buildkit-build + spec: + source: + url: fake_url + sources: + - name: foobar_local + type: LocalCopy + timeout: 1m + - name: foobar_local_two + type: LocalCopy + timeout: 1m +` + o := fmt.Sprintf(buildTemplate, apiVersion, + desiredAPIVersion) + + // Invoke the /convert webhook endpoint + conversionReview, err := getConversionReview(o) + Expect(err).To(BeNil()) + Expect(conversionReview.Response.Result.Status).To(Equal(v1.StatusSuccess)) + + convertedObj, err := ToUnstructured(conversionReview) + Expect(err).To(BeNil()) + + build, err := toV1Beta1BuildObject(convertedObj) + Expect(err).To(BeNil()) + + // Prepare our desired v1beta1 Build + desiredBuild := v1beta1.Build{ + TypeMeta: v1.TypeMeta{ + APIVersion: "shipwright.io/v1beta1", + Kind: "Build", + }, + ObjectMeta: v1.ObjectMeta{ + Name: "buildkit-build", + }, + Spec: v1beta1.BuildSpec{ + Source: v1beta1.Source{ + Type: v1beta1.LocalType, + LocalSource: &v1beta1.Local{ + Name: "foobar_local", + Timeout: &v1.Duration{ + Duration: 1 * time.Minute, + }, + }, + }, + }, + } + + // Use ComparableTo and assert the whole object + Expect(build).To(BeComparableTo(desiredBuild)) + + }) + It("converts for spec bundleContainer source type, triggers and output", func() { pruneOption := "Never" branchMain := "main" @@ -638,6 +770,71 @@ request: Context("for a BuildRun CR from v1beta1 to v1alpha1", func() { var desiredAPIVersion = "shipwright.io/v1alpha1" + It("converts for spec source", func() { + // Create the yaml in v1beta1 + buildTemplate := `kind: ConversionReview +apiVersion: %s +request: + uid: 0000-0000-0000-0000 + desiredAPIVersion: %s + objects: + - apiVersion: shipwright.io/v1beta1 + kind: BuildRun + metadata: + name: buildkit-run + spec: + build: + name: a_build + source: + type: Local + local: + name: foobar_local + timeout: 1m +` + o := fmt.Sprintf(buildTemplate, apiVersion, + desiredAPIVersion) + + // Invoke the /convert webhook endpoint + conversionReview, err := getConversionReview(o) + Expect(err).To(BeNil()) + Expect(conversionReview.Response.Result.Status).To(Equal(v1.StatusSuccess)) + + convertedObj, err := ToUnstructured(conversionReview) + Expect(err).To(BeNil()) + + buildRun, err := toV1Alpha1BuildRunObject(convertedObj) + Expect(err).To(BeNil()) + + // Prepare our desired v1alpha1 BuildRun + desiredBuildRun := v1alpha1.BuildRun{ + ObjectMeta: v1.ObjectMeta{ + Name: "buildkit-run", + }, + TypeMeta: v1.TypeMeta{ + APIVersion: "shipwright.io/v1alpha1", + Kind: "BuildRun", + }, + Spec: v1alpha1.BuildRunSpec{ + BuildRef: &v1alpha1.BuildRef{ + Name: "a_build", + }, + Sources: []v1alpha1.BuildSource{ + { + Name: "foobar_local", + Type: v1alpha1.LocalCopy, + Timeout: &v1.Duration{ + Duration: 1 * time.Minute, + }, + }, + }, + ServiceAccount: &v1alpha1.ServiceAccount{}, + }, + } + + // Use ComparableTo and assert the whole object + Expect(buildRun).To(BeComparableTo(desiredBuildRun)) + }) + It("converts for spec Build spec", func() { pruneOption := "AfterPull" sa := "foobar" @@ -835,6 +1032,71 @@ request: }) Context("for a BuildRun CR from v1alpha1 to v1beta1", func() { var desiredAPIVersion = "shipwright.io/v1beta1" + + It("converts for spec source", func() { + // Create the yaml in v1alpha1 + buildTemplate := `kind: ConversionReview +apiVersion: %s +request: + uid: 0000-0000-0000-0000 + desiredAPIVersion: %s + objects: + - apiVersion: shipwright.io/v1alpha1 + kind: BuildRun + metadata: + name: buildkit-run + spec: + buildRef: + name: a_build + sources: + - name: foobar_local + type: LocalCopy + timeout: 1m +` + o := fmt.Sprintf(buildTemplate, apiVersion, + desiredAPIVersion) + + // Invoke the /convert webhook endpoint + conversionReview, err := getConversionReview(o) + Expect(err).To(BeNil()) + Expect(conversionReview.Response.Result.Status).To(Equal(v1.StatusSuccess)) + + convertedObj, err := ToUnstructured(conversionReview) + Expect(err).To(BeNil()) + + buildRun, err := toV1Beta1BuildRunObject(convertedObj) + Expect(err).To(BeNil()) + + // Prepare our desired v1alpha1 BuildRun + desiredBuildRun := v1beta1.BuildRun{ + ObjectMeta: v1.ObjectMeta{ + Name: "buildkit-run", + }, + TypeMeta: v1.TypeMeta{ + APIVersion: "shipwright.io/v1beta1", + Kind: "BuildRun", + }, + Spec: v1beta1.BuildRunSpec{ + Build: &v1beta1.ReferencedBuild{ + Name: "a_build", + }, + Source: &v1beta1.BuildRunSource{ + Type: v1beta1.LocalType, + LocalSource: &v1beta1.Local{ + Name: "foobar_local", + Timeout: &v1.Duration{ + Duration: 1 * time.Minute, + }, + }, + }, + Output: &v1beta1.Image{}, + }, + } + + // Use ComparableTo and assert the whole object + Expect(buildRun).To(BeComparableTo(desiredBuildRun)) + }) + It("converts for spec Build buildref", func() { // Create the yaml in v1alpha1 buildTemplate := `kind: ConversionReview From e22ca65d8d84a25cb556f5b1c2c0eeefd3df2df6 Mon Sep 17 00:00:00 2001 From: encalada Date: Wed, 18 Oct 2023 14:10:18 +0200 Subject: [PATCH 3/7] add docs on br source --- docs/buildrun.md | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/docs/buildrun.md b/docs/buildrun.md index f79a9955b..31e2d1972 100644 --- a/docs/buildrun.md +++ b/docs/buildrun.md @@ -111,6 +111,27 @@ spec: image: foo/bar:latest ``` +### Defining the Build Source + +BuildRun's support the specification of a Local type source. This is useful for working on development +mode, without forcing a user to commit/push changes to their related version control system. For more information please +refer to [SHIP 0016 - enabling local source code](https://github.com/shipwright-io/community/blob/main/ships/0016-enable-local-source-code-support.md). + +```yaml +apiVersion: shipwright.io/v1beta1 +kind: BuildRun +metadata: + name: local-buildrun +spec: + build: + name: a-build + source: + type: Local + local: + name: local-source + timeout: 3m +``` + ### Defining ParamValues A `BuildRun` resource can define _paramValues_ for parameters specified in the build strategy. If a value has been provided for a parameter with the same name in the `Build` already, then the value from the `BuildRun` will have precedence. From 9ef85060959e02dfd9839c22aa7c5b13669a2916 Mon Sep 17 00:00:00 2001 From: encalada Date: Wed, 18 Oct 2023 15:48:52 +0200 Subject: [PATCH 4/7] Make the buildrun.spec.build a mandatory field Due to seeing panics reported in issues, when this field for v1beta1 CR's was not set by human error. --- pkg/apis/build/v1beta1/buildrun_conversion.go | 6 +++--- pkg/apis/build/v1beta1/buildrun_types.go | 8 ++++---- pkg/webhook/conversion/converter_test.go | 4 ++-- test/e2e/v1beta1/common_suite_test.go | 2 +- test/e2e/v1beta1/validators_test.go | 2 +- test/v1beta1_samples/catalog.go | 16 ++++++++-------- 6 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/apis/build/v1beta1/buildrun_conversion.go b/pkg/apis/build/v1beta1/buildrun_conversion.go index bdde2a86b..26bd9dfca 100644 --- a/pkg/apis/build/v1beta1/buildrun_conversion.go +++ b/pkg/apis/build/v1beta1/buildrun_conversion.go @@ -36,9 +36,9 @@ func (src *BuildRun) ConvertTo(ctx context.Context, obj *unstructured.Unstructur return err } alphaBuildRun.Spec.BuildSpec = &newBuildSpec - } else { + } else if src.Spec.Build.Name != nil { alphaBuildRun.Spec.BuildRef = &v1alpha1.BuildRef{ - Name: src.Spec.Build.Name, + Name: *src.Spec.Build.Name, } } @@ -200,7 +200,7 @@ func (dest *BuildRunSpec) ConvertFrom(orig *v1alpha1.BuildRunSpec) error { } } if orig.BuildRef != nil { - dest.Build.Name = orig.BuildRef.Name + dest.Build.Name = &orig.BuildRef.Name } // only interested on spec.sources as long as an item of the list diff --git a/pkg/apis/build/v1beta1/buildrun_types.go b/pkg/apis/build/v1beta1/buildrun_types.go index 087ae7e8d..87eed134f 100644 --- a/pkg/apis/build/v1beta1/buildrun_types.go +++ b/pkg/apis/build/v1beta1/buildrun_types.go @@ -29,15 +29,15 @@ type ReferencedBuild struct { // Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names // // +optional - Name string `json:"name,omitempty"` + Name *string `json:"name,omitempty"` } // BuildRunSpec defines the desired state of BuildRun type BuildRunSpec struct { // Build refers to an embedded build specification + // This field is mandatory // - // +optional - Build *ReferencedBuild `json:"build,omitempty"` + Build *ReferencedBuild `json:"build"` // Source refers to the location where the source code is, // this could only be a local source @@ -380,7 +380,7 @@ func (brs *BuildRunStatus) SetCondition(condition *Condition) { // build resource or an embedded build specification func (buildrunSpec *BuildRunSpec) BuildName() string { if buildrunSpec.Build != nil { - return buildrunSpec.Build.Name + return *buildrunSpec.Build.Name } // Only BuildRuns with a ReferencedBuild can actually return a proper Build name diff --git a/pkg/webhook/conversion/converter_test.go b/pkg/webhook/conversion/converter_test.go index fdb3dfd81..ae9c39193 100644 --- a/pkg/webhook/conversion/converter_test.go +++ b/pkg/webhook/conversion/converter_test.go @@ -1078,7 +1078,7 @@ request: }, Spec: v1beta1.BuildRunSpec{ Build: &v1beta1.ReferencedBuild{ - Name: "a_build", + Name: pointer.String("a_build"), }, Source: &v1beta1.BuildRunSource{ Type: v1beta1.LocalType, @@ -1162,7 +1162,7 @@ request: }, Spec: v1beta1.BuildRunSpec{ Build: &v1beta1.ReferencedBuild{ - Name: "a_build", + Name: pointer.String("a_build"), }, ServiceAccount: &sa, Timeout: &v1.Duration{ diff --git a/test/e2e/v1beta1/common_suite_test.go b/test/e2e/v1beta1/common_suite_test.go index 2cb4c1638..24b396d08 100644 --- a/test/e2e/v1beta1/common_suite_test.go +++ b/test/e2e/v1beta1/common_suite_test.go @@ -283,7 +283,7 @@ func (b *buildRunPrototype) ForBuild(build *buildv1beta1.Build) *buildRunPrototy if b.buildRun.Spec.Build == nil { b.buildRun.Spec.Build = &buildv1beta1.ReferencedBuild{} } - b.buildRun.Spec.Build.Name = build.Name + b.buildRun.Spec.Build.Name = &build.Name b.buildRun.ObjectMeta.Namespace = build.Namespace return b } diff --git a/test/e2e/v1beta1/validators_test.go b/test/e2e/v1beta1/validators_test.go index 34cc8e99d..46eb6f9a8 100644 --- a/test/e2e/v1beta1/validators_test.go +++ b/test/e2e/v1beta1/validators_test.go @@ -331,7 +331,7 @@ func buildRunTestData(ns string, identifier string, filePath string) (*buildv1be buildRun.SetNamespace(ns) buildRun.SetName(identifier) - buildRun.Spec.Build.Name = identifier + buildRun.Spec.Build.Name = &identifier serviceAccountName := os.Getenv(EnvVarServiceAccountName) if serviceAccountName == "generated" { diff --git a/test/v1beta1_samples/catalog.go b/test/v1beta1_samples/catalog.go index 029aa2b25..7cf2defe8 100644 --- a/test/v1beta1_samples/catalog.go +++ b/test/v1beta1_samples/catalog.go @@ -785,7 +785,7 @@ func (c *Catalog) DefaultBuildRun(buildRunName string, buildName string) *build. }, Spec: build.BuildRunSpec{ Build: &build.ReferencedBuild{ - Name: buildName, + Name: &buildName, }, }, Status: build.BuildRunStatus{ @@ -830,7 +830,7 @@ func (c *Catalog) BuildRunWithBuildSnapshot(buildRunName string, buildName strin }, Spec: build.BuildRunSpec{ Build: &build.ReferencedBuild{ - Name: buildName, + Name: &buildName, }, }, } @@ -855,7 +855,7 @@ func (c *Catalog) BuildRunWithExistingOwnerReferences(buildRunName string, build }, Spec: build.BuildRunSpec{ Build: &build.ReferencedBuild{ - Name: buildName, + Name: &buildName, }, }, } @@ -871,7 +871,7 @@ func (c *Catalog) BuildRunWithFakeNamespace(buildRunName string, buildName strin }, Spec: build.BuildRunSpec{ Build: &build.ReferencedBuild{ - Name: buildName, + Name: &buildName, }, }, } @@ -960,7 +960,7 @@ func (c *Catalog) BuildRunWithSA(buildRunName string, buildName string, saName s }, Spec: build.BuildRunSpec{ Build: &build.ReferencedBuild{ - Name: buildName, + Name: &buildName, }, ServiceAccount: &saName, }, @@ -976,7 +976,7 @@ func (c *Catalog) BuildRunWithoutSA(buildRunName string, buildName string) *buil }, Spec: build.BuildRunSpec{ Build: &build.ReferencedBuild{ - Name: buildName, + Name: &buildName, }, ServiceAccount: nil, }, @@ -992,7 +992,7 @@ func (c *Catalog) BuildRunWithSAGenerate(buildRunName string, buildName string) }, Spec: build.BuildRunSpec{ Build: &build.ReferencedBuild{ - Name: buildName, + Name: &buildName, }, ServiceAccount: pointer.String(".generate"), }, @@ -1054,7 +1054,7 @@ func (c *Catalog) LoadBRWithNameAndRef(name string, buildName string, d []byte) return nil, err } b.Name = name - b.Spec.Build.Name = buildName + b.Spec.Build.Name = &buildName return b, nil } From f5b8ed439a10156c42cb6e47193717c79ce55186 Mon Sep 17 00:00:00 2001 From: encalada Date: Wed, 18 Oct 2023 16:43:36 +0200 Subject: [PATCH 5/7] Update CRDs and deepcopy via make generate --- deploy/crds/shipwright.io_buildruns.yaml | 47 ++++++++++++++++++- deploy/crds/shipwright.io_builds.yaml | 11 +++++ .../build/v1beta1/zz_generated.deepcopy.go | 36 ++++++++++++++ 3 files changed, 93 insertions(+), 1 deletion(-) diff --git a/deploy/crds/shipwright.io_buildruns.yaml b/deploy/crds/shipwright.io_buildruns.yaml index 3787885b9..d9278535a 100644 --- a/deploy/crds/shipwright.io_buildruns.yaml +++ b/deploy/crds/shipwright.io_buildruns.yaml @@ -6388,7 +6388,8 @@ spec: description: BuildRunSpec defines the desired state of BuildRun properties: build: - description: Build refers to an embedded build specification + description: Build refers to an embedded build specification This + field is mandatory properties: name: description: 'Name of the referent; More info: http://kubernetes.io/docs/user-guide/identifiers#names' @@ -6703,6 +6704,17 @@ spec: description: URL describes the URL of the Git repository. type: string type: object + local: + description: LocalSource + properties: + name: + description: Name of the local step + type: string + timeout: + description: Timeout how long the BuildSource execution + must take. + type: string + type: object ociArtifact: description: OCIArtifact properties: @@ -8786,6 +8798,26 @@ spec: which is used for resource control. Default serviceaccount will be set if it is empty type: string + source: + description: Source refers to the location where the source code is, + this could only be a local source + properties: + local: + description: LocalSource + properties: + name: + description: Name of the local step + type: string + timeout: + description: Timeout how long the BuildSource execution must + take. + type: string + type: object + type: + description: Type is the BuildRunSource qualifier, the type of + the data-source. Only LocalType is supported. + type: string + type: object state: description: State is used for canceling a buildrun (and maybe more later on). @@ -10354,6 +10386,8 @@ spec: - name type: object type: array + required: + - build type: object status: description: BuildRunStatus defines the observed state of BuildRun @@ -10662,6 +10696,17 @@ spec: description: URL describes the URL of the Git repository. type: string type: object + local: + description: LocalSource + properties: + name: + description: Name of the local step + type: string + timeout: + description: Timeout how long the BuildSource execution + must take. + type: string + type: object ociArtifact: description: OCIArtifact properties: diff --git a/deploy/crds/shipwright.io_builds.yaml b/deploy/crds/shipwright.io_builds.yaml index 8da54648a..a08601278 100644 --- a/deploy/crds/shipwright.io_builds.yaml +++ b/deploy/crds/shipwright.io_builds.yaml @@ -2479,6 +2479,17 @@ spec: description: URL describes the URL of the Git repository. type: string type: object + local: + description: LocalSource + properties: + name: + description: Name of the local step + type: string + timeout: + description: Timeout how long the BuildSource execution must + take. + type: string + type: object ociArtifact: description: OCIArtifact properties: diff --git a/pkg/apis/build/v1beta1/zz_generated.deepcopy.go b/pkg/apis/build/v1beta1/zz_generated.deepcopy.go index 47062397a..842578b49 100644 --- a/pkg/apis/build/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/build/v1beta1/zz_generated.deepcopy.go @@ -204,6 +204,27 @@ func (in *BuildRunRetention) DeepCopy() *BuildRunRetention { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BuildRunSource) DeepCopyInto(out *BuildRunSource) { + *out = *in + if in.LocalSource != nil { + in, out := &in.LocalSource, &out.LocalSource + *out = new(Local) + (*in).DeepCopyInto(*out) + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BuildRunSource. +func (in *BuildRunSource) DeepCopy() *BuildRunSource { + if in == nil { + return nil + } + out := new(BuildRunSource) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BuildRunSpec) DeepCopyInto(out *BuildRunSpec) { *out = *in @@ -212,6 +233,11 @@ func (in *BuildRunSpec) DeepCopyInto(out *BuildRunSpec) { *out = new(ReferencedBuild) (*in).DeepCopyInto(*out) } + if in.Source != nil { + in, out := &in.Source, &out.Source + *out = new(BuildRunSource) + (*in).DeepCopyInto(*out) + } if in.ServiceAccount != nil { in, out := &in.ServiceAccount, &out.ServiceAccount *out = new(string) @@ -984,6 +1010,11 @@ func (in *ReferencedBuild) DeepCopyInto(out *ReferencedBuild) { *out = new(BuildSpec) (*in).DeepCopyInto(*out) } + if in.Name != nil { + in, out := &in.Name, &out.Name + *out = new(string) + **out = **in + } return } @@ -1046,6 +1077,11 @@ func (in *Source) DeepCopyInto(out *Source) { *out = new(Git) (*in).DeepCopyInto(*out) } + if in.LocalSource != nil { + in, out := &in.LocalSource, &out.LocalSource + *out = new(Local) + (*in).DeepCopyInto(*out) + } return } From ac9c985c6a6abdb39f9f20259961fef0399312c4 Mon Sep 17 00:00:00 2001 From: Sascha Schwarze Date: Wed, 18 Oct 2023 20:53:02 +0200 Subject: [PATCH 6/7] Fixes for Beta API and conversion --- pkg/apis/build/v1beta1/build_conversion.go | 17 ++++++-------- pkg/apis/build/v1beta1/buildrun_conversion.go | 23 ++++++++----------- pkg/apis/build/v1beta1/buildrun_types.go | 4 ++-- .../build/v1beta1/zz_generated.deepcopy.go | 6 +---- pkg/webhook/conversion/converter_test.go | 4 ++-- test/e2e/v1beta1/common_suite_test.go | 6 ----- test/v1beta1_samples/catalog.go | 14 +++++------ 7 files changed, 28 insertions(+), 46 deletions(-) diff --git a/pkg/apis/build/v1beta1/build_conversion.go b/pkg/apis/build/v1beta1/build_conversion.go index f1e0f297e..c246cd528 100644 --- a/pkg/apis/build/v1beta1/build_conversion.go +++ b/pkg/apis/build/v1beta1/build_conversion.go @@ -110,7 +110,7 @@ func (dest *BuildSpec) ConvertFrom(orig *v1alpha1.BuildSpec) error { // 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 len(orig.Sources) > 0 && isLocal { + if isLocal { specSource.Type = LocalType specSource.LocalSource = &Local{ Name: orig.Sources[index].Name, @@ -226,15 +226,12 @@ 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 { - bs.Sources = []v1alpha1.BuildSource{} - if dest.Source.LocalSource != nil { - bs.Sources = append(bs.Sources, v1alpha1.BuildSource{ - Name: dest.Source.LocalSource.Name, - Type: v1alpha1.LocalCopy, - Timeout: dest.Source.LocalSource.Timeout, - }) - } + if dest.Source.Type == LocalType && dest.Source.LocalSource != nil { + bs.Sources = append(bs.Sources, v1alpha1.BuildSource{ + Name: dest.Source.LocalSource.Name, + Type: v1alpha1.LocalCopy, + Timeout: dest.Source.LocalSource.Timeout, + }) } else { bs.Source = getAlphaBuildSource(*dest) } diff --git a/pkg/apis/build/v1beta1/buildrun_conversion.go b/pkg/apis/build/v1beta1/buildrun_conversion.go index 26bd9dfca..4d1e213b7 100644 --- a/pkg/apis/build/v1beta1/buildrun_conversion.go +++ b/pkg/apis/build/v1beta1/buildrun_conversion.go @@ -43,15 +43,12 @@ func (src *BuildRun) ConvertTo(ctx context.Context, obj *unstructured.Unstructur } // BuildRunSpec Sources - if src.Spec.Source != nil { - alphaBuildRun.Spec.Sources = []v1alpha1.BuildSource{} - if src.Spec.Source.Type == LocalType { - alphaBuildRun.Spec.Sources = append(alphaBuildRun.Spec.Sources, v1alpha1.BuildSource{ - Name: src.Spec.Source.LocalSource.Name, - Type: v1alpha1.LocalCopy, - Timeout: src.Spec.Source.LocalSource.Timeout, - }) - } + if src.Spec.Source != nil && src.Spec.Source.Type == LocalType && src.Spec.Source.LocalSource != nil { + alphaBuildRun.Spec.Sources = append(alphaBuildRun.Spec.Sources, v1alpha1.BuildSource{ + Name: src.Spec.Source.LocalSource.Name, + Type: v1alpha1.LocalCopy, + Timeout: src.Spec.Source.LocalSource.Timeout, + }) } // BuildRunSpec ServiceAccount @@ -193,11 +190,9 @@ func (src *BuildRun) ConvertFrom(ctx context.Context, obj *unstructured.Unstruct func (dest *BuildRunSpec) ConvertFrom(orig *v1alpha1.BuildRunSpec) error { // BuildRunSpec BuildSpec - dest.Build = &ReferencedBuild{} if orig.BuildSpec != nil { - if dest.Build.Build != nil { - dest.Build.Build.ConvertFrom(orig.BuildSpec) - } + dest.Build.Build = &BuildSpec{} + dest.Build.Build.ConvertFrom(orig.BuildSpec) } if orig.BuildRef != nil { dest.Build.Name = &orig.BuildRef.Name @@ -206,7 +201,7 @@ func (dest *BuildRunSpec) ConvertFrom(orig *v1alpha1.BuildRunSpec) error { // 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 len(orig.Sources) > 0 && isLocal { + if isLocal { dest.Source = &BuildRunSource{ Type: LocalType, LocalSource: &Local{ diff --git a/pkg/apis/build/v1beta1/buildrun_types.go b/pkg/apis/build/v1beta1/buildrun_types.go index 87eed134f..2880a4213 100644 --- a/pkg/apis/build/v1beta1/buildrun_types.go +++ b/pkg/apis/build/v1beta1/buildrun_types.go @@ -37,7 +37,7 @@ type BuildRunSpec struct { // Build refers to an embedded build specification // This field is mandatory // - Build *ReferencedBuild `json:"build"` + Build ReferencedBuild `json:"build"` // Source refers to the location where the source code is, // this could only be a local source @@ -379,7 +379,7 @@ func (brs *BuildRunStatus) SetCondition(condition *Condition) { // BuildName returns the name of the associated build, which can be a referenced // build resource or an embedded build specification func (buildrunSpec *BuildRunSpec) BuildName() string { - if buildrunSpec.Build != nil { + if buildrunSpec.Build.Name != nil { return *buildrunSpec.Build.Name } diff --git a/pkg/apis/build/v1beta1/zz_generated.deepcopy.go b/pkg/apis/build/v1beta1/zz_generated.deepcopy.go index 842578b49..f7660573f 100644 --- a/pkg/apis/build/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/build/v1beta1/zz_generated.deepcopy.go @@ -228,11 +228,7 @@ func (in *BuildRunSource) DeepCopy() *BuildRunSource { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BuildRunSpec) DeepCopyInto(out *BuildRunSpec) { *out = *in - if in.Build != nil { - in, out := &in.Build, &out.Build - *out = new(ReferencedBuild) - (*in).DeepCopyInto(*out) - } + in.Build.DeepCopyInto(&out.Build) if in.Source != nil { in, out := &in.Source, &out.Source *out = new(BuildRunSource) diff --git a/pkg/webhook/conversion/converter_test.go b/pkg/webhook/conversion/converter_test.go index ae9c39193..a6967bb84 100644 --- a/pkg/webhook/conversion/converter_test.go +++ b/pkg/webhook/conversion/converter_test.go @@ -1077,7 +1077,7 @@ request: Kind: "BuildRun", }, Spec: v1beta1.BuildRunSpec{ - Build: &v1beta1.ReferencedBuild{ + Build: v1beta1.ReferencedBuild{ Name: pointer.String("a_build"), }, Source: &v1beta1.BuildRunSource{ @@ -1161,7 +1161,7 @@ request: Kind: "BuildRun", }, Spec: v1beta1.BuildRunSpec{ - Build: &v1beta1.ReferencedBuild{ + Build: v1beta1.ReferencedBuild{ Name: pointer.String("a_build"), }, ServiceAccount: &sa, diff --git a/test/e2e/v1beta1/common_suite_test.go b/test/e2e/v1beta1/common_suite_test.go index 24b396d08..a898b3ede 100644 --- a/test/e2e/v1beta1/common_suite_test.go +++ b/test/e2e/v1beta1/common_suite_test.go @@ -280,18 +280,12 @@ func (b *buildRunPrototype) Namespace(namespace string) *buildRunPrototype { } func (b *buildRunPrototype) ForBuild(build *buildv1beta1.Build) *buildRunPrototype { - if b.buildRun.Spec.Build == nil { - b.buildRun.Spec.Build = &buildv1beta1.ReferencedBuild{} - } b.buildRun.Spec.Build.Name = &build.Name b.buildRun.ObjectMeta.Namespace = build.Namespace return b } func (b *buildRunPrototype) WithBuildSpec(buildSpec *buildv1beta1.BuildSpec) *buildRunPrototype { - if b.buildRun.Spec.Build == nil { - b.buildRun.Spec.Build = &buildv1beta1.ReferencedBuild{} - } b.buildRun.Spec.Build.Build = buildSpec return b } diff --git a/test/v1beta1_samples/catalog.go b/test/v1beta1_samples/catalog.go index 7cf2defe8..177291b74 100644 --- a/test/v1beta1_samples/catalog.go +++ b/test/v1beta1_samples/catalog.go @@ -784,7 +784,7 @@ func (c *Catalog) DefaultBuildRun(buildRunName string, buildName string) *build. Name: buildRunName, }, Spec: build.BuildRunSpec{ - Build: &build.ReferencedBuild{ + Build: build.ReferencedBuild{ Name: &buildName, }, }, @@ -829,7 +829,7 @@ func (c *Catalog) BuildRunWithBuildSnapshot(buildRunName string, buildName strin }, }, Spec: build.BuildRunSpec{ - Build: &build.ReferencedBuild{ + Build: build.ReferencedBuild{ Name: &buildName, }, }, @@ -854,7 +854,7 @@ func (c *Catalog) BuildRunWithExistingOwnerReferences(buildRunName string, build OwnerReferences: []metav1.OwnerReference{fakeOwnerRef}, }, Spec: build.BuildRunSpec{ - Build: &build.ReferencedBuild{ + Build: build.ReferencedBuild{ Name: &buildName, }, }, @@ -870,7 +870,7 @@ func (c *Catalog) BuildRunWithFakeNamespace(buildRunName string, buildName strin Namespace: "foobarns", }, Spec: build.BuildRunSpec{ - Build: &build.ReferencedBuild{ + Build: build.ReferencedBuild{ Name: &buildName, }, }, @@ -959,7 +959,7 @@ func (c *Catalog) BuildRunWithSA(buildRunName string, buildName string, saName s Name: buildRunName, }, Spec: build.BuildRunSpec{ - Build: &build.ReferencedBuild{ + Build: build.ReferencedBuild{ Name: &buildName, }, ServiceAccount: &saName, @@ -975,7 +975,7 @@ func (c *Catalog) BuildRunWithoutSA(buildRunName string, buildName string) *buil Name: buildRunName, }, Spec: build.BuildRunSpec{ - Build: &build.ReferencedBuild{ + Build: build.ReferencedBuild{ Name: &buildName, }, ServiceAccount: nil, @@ -991,7 +991,7 @@ func (c *Catalog) BuildRunWithSAGenerate(buildRunName string, buildName string) Name: buildRunName, }, Spec: build.BuildRunSpec{ - Build: &build.ReferencedBuild{ + Build: build.ReferencedBuild{ Name: &buildName, }, ServiceAccount: pointer.String(".generate"), From 7ce6db18195b7ec3b056bb554932dd09df768204 Mon Sep 17 00:00:00 2001 From: Sascha Schwarze Date: Wed, 18 Oct 2023 21:02:06 +0200 Subject: [PATCH 7/7] Fix handling of legacy parameters --- pkg/apis/build/v1beta1/buildstrategy_conversion.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/apis/build/v1beta1/buildstrategy_conversion.go b/pkg/apis/build/v1beta1/buildstrategy_conversion.go index cd3155a8a..1b0715faf 100644 --- a/pkg/apis/build/v1beta1/buildstrategy_conversion.go +++ b/pkg/apis/build/v1beta1/buildstrategy_conversion.go @@ -114,13 +114,13 @@ func (src *BuildStrategySpec) ConvertTo(bs *v1alpha1.BuildStrategySpec) { } for argIndex, arg := range buildStep.Args { - if strings.Contains(arg, "$(params.dockerfile)") { + if strings.Contains(arg, "$(params.builder-image)") { buildStep.Args[argIndex] = strings.ReplaceAll(arg, "$(params.builder-image)", "$(build.builder.image)") } } for envIndex, env := range buildStep.Env { - if strings.Contains(env.Value, "$(params.dockerfile)") { + if strings.Contains(env.Value, "$(params.builder-image)") { buildStep.Env[envIndex].Value = strings.ReplaceAll(env.Value, "$(params.builder-image)", "$(build.builder.image)") } } @@ -212,7 +212,7 @@ func (src *BuildStrategySpec) ConvertFrom(bs v1alpha1.BuildStrategySpec) { } if strings.Contains(arg, "$(build.builder.image)") { usesBuilderImage = true - step.Command[argIndex] = strings.ReplaceAll(arg, "$(build.builder.image)", "$(params.builder-image)") + step.Args[argIndex] = strings.ReplaceAll(arg, "$(build.builder.image)", "$(params.builder-image)") } } @@ -227,7 +227,7 @@ func (src *BuildStrategySpec) ConvertFrom(bs v1alpha1.BuildStrategySpec) { } if strings.Contains(env.Value, "$(build.builder.image)") { usesBuilderImage = true - step.Command[envIndex] = strings.ReplaceAll(env.Value, "$(build.builder.image)", "$(params.builder-image)") + step.Env[envIndex].Value = strings.ReplaceAll(env.Value, "$(build.builder.image)", "$(params.builder-image)") } }