From d45fec0b837f4ab5ec6ca8675ea449c791dc2ca1 Mon Sep 17 00:00:00 2001 From: Sascha Schwarze Date: Fri, 23 Feb 2024 22:50:39 +0100 Subject: [PATCH] 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 - Add missing documentation to API fields --- deploy/crds/shipwright.io_buildruns.yaml | 35 ++++++--- deploy/crds/shipwright.io_builds.yaml | 13 ++-- pkg/apis/build/v1beta1/build_conversion.go | 77 ++++++++++--------- pkg/apis/build/v1beta1/build_types.go | 14 +++- pkg/apis/build/v1beta1/buildrun_conversion.go | 8 +- pkg/apis/build/v1beta1/source.go | 30 ++++---- .../build/v1beta1/zz_generated.deepcopy.go | 18 +++-- pkg/reconciler/build/build_test.go | 18 ++--- pkg/reconciler/buildrun/buildrun_test.go | 4 +- .../buildrun/resources/credentials_test.go | 12 +-- .../buildrun/resources/results_test.go | 10 +-- pkg/reconciler/buildrun/resources/sources.go | 21 ++--- .../buildrun/resources/sources/git.go | 4 +- pkg/reconciler/buildrun/resources/taskrun.go | 59 +------------- .../buildrun/resources/taskrun_test.go | 63 ++++++++++----- pkg/validate/sources.go | 17 ++-- pkg/validate/sources_test.go | 16 ++-- pkg/validate/sourceurl.go | 8 +- pkg/webhook/conversion/converter_test.go | 16 ++-- test/e2e/v1beta1/common_suite_test.go | 29 +++++-- test/e2e/v1beta1/common_test.go | 8 +- test/integration/build_to_git_test.go | 42 +++++----- test/integration/build_to_secrets_test.go | 16 ++-- test/integration/buildrun_status_test.go | 6 +- test/v1beta1_samples/build_samples.go | 3 +- test/v1beta1_samples/catalog.go | 24 +++--- 26 files changed, 294 insertions(+), 277 deletions(-) diff --git a/deploy/crds/shipwright.io_buildruns.yaml b/deploy/crds/shipwright.io_buildruns.yaml index f0cfdf9fd4..13454550ae 100644 --- a/deploy/crds/shipwright.io_buildruns.yaml +++ b/deploy/crds/shipwright.io_buildruns.yaml @@ -6695,7 +6695,8 @@ spec: repo. Optional. type: string git: - description: GitSource + description: Git contains the details for the source of + type Git properties: cloneSecret: description: CloneSecret references a Secret that @@ -6714,7 +6715,8 @@ spec: - url type: object local: - description: LocalSource + description: Local contains the details for the source + of type Local properties: name: description: Name of the local step @@ -6725,7 +6727,8 @@ spec: type: string type: object ociArtifact: - description: OCIArtifact + description: OCIArtifact contains the details for the + source of type OCIArtifact properties: image: description: Image reference, i.e. quay.io/org/image:tag @@ -6746,8 +6749,10 @@ spec: type: object type: description: Type is the BuildSource qualifier, the type - of the data-source. + of the source. type: string + required: + - type type: object strategy: description: Strategy references the BuildStrategy to use @@ -8543,7 +8548,6 @@ spec: type: array required: - output - - source - strategy type: object type: object @@ -8812,7 +8816,8 @@ spec: this could only be a local source properties: local: - description: LocalSource + description: Local contains the details for the source of type + Local properties: name: description: Name of the local step @@ -8824,8 +8829,10 @@ spec: type: object type: description: Type is the BuildRunSource qualifier, the type of - the data-source. Only LocalType is supported. + the source. Only Local is supported. type: string + required: + - type type: object state: description: State is used for canceling a buildrun (and maybe more @@ -10690,7 +10697,8 @@ spec: Optional. type: string git: - description: GitSource + description: Git contains the details for the source of type + Git properties: cloneSecret: description: CloneSecret references a Secret that contains @@ -10708,7 +10716,8 @@ spec: - url type: object local: - description: LocalSource + description: Local contains the details for the source of + type Local properties: name: description: Name of the local step @@ -10719,7 +10728,8 @@ spec: type: string type: object ociArtifact: - description: OCIArtifact + description: OCIArtifact contains the details for the source + of type OCIArtifact properties: image: description: Image reference, i.e. quay.io/org/image:tag @@ -10740,8 +10750,10 @@ spec: type: object type: description: Type is the BuildSource qualifier, the type of - the data-source. + the source. type: string + required: + - type type: object strategy: description: Strategy references the BuildStrategy to use to build @@ -12461,7 +12473,6 @@ spec: type: array required: - output - - source - strategy type: object completionTime: diff --git a/deploy/crds/shipwright.io_builds.yaml b/deploy/crds/shipwright.io_builds.yaml index 82bbd07b67..2003f317fa 100644 --- a/deploy/crds/shipwright.io_builds.yaml +++ b/deploy/crds/shipwright.io_builds.yaml @@ -2464,7 +2464,7 @@ spec: description: ContextDir is a path to subfolder in the repo. Optional. type: string git: - description: GitSource + description: Git contains the details for the source of type Git properties: cloneSecret: description: CloneSecret references a Secret that contains @@ -2482,7 +2482,8 @@ spec: - url type: object local: - description: LocalSource + description: Local contains the details for the source of type + Local properties: name: description: Name of the local step @@ -2493,7 +2494,8 @@ spec: type: string type: object ociArtifact: - description: OCIArtifact + description: OCIArtifact contains the details for the source of + type OCIArtifact properties: image: description: Image reference, i.e. quay.io/org/image:tag @@ -2514,8 +2516,10 @@ spec: type: object type: description: Type is the BuildSource qualifier, the type of the - data-source. + source. type: string + required: + - type type: object strategy: description: Strategy references the BuildStrategy to use to build @@ -4174,7 +4178,6 @@ spec: type: array required: - output - - source - strategy type: object status: diff --git a/pkg/apis/build/v1beta1/build_conversion.go b/pkg/apis/build/v1beta1/build_conversion.go index c39d652d23..be06a55a06 100644 --- 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 index a0e51f298b..0d233f0c50 100644 --- a/pkg/apis/build/v1beta1/build_types.go +++ b/pkg/apis/build/v1beta1/build_types.go @@ -110,7 +110,9 @@ 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"` + // + // +optional + Source *Source `json:"source"` // Trigger defines the scenarios where a new build should be triggered. // @@ -137,6 +139,7 @@ type BuildSpec struct { Timeout *metav1.Duration `json:"timeout,omitempty"` // Env contains additional environment variables that should be passed to the build container + // // +optional Env []corev1.EnvVar `json:"env,omitempty"` @@ -147,6 +150,7 @@ type BuildSpec struct { // Volumes contains volume Overrides of the BuildStrategy volumes in case those are allowed // to be overridden. Must only contain volumes that exist in the corresponding BuildStrategy + // // +optional Volumes []BuildVolume `json:"volumes,omitempty"` } @@ -278,14 +282,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 index ddcdf6aa9a..a6fc3f95df 100644 --- 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 index 9413c57486..f1f5a2fd1d 100644 --- a/pkg/apis/build/v1beta1/source.go +++ b/pkg/apis/build/v1beta1/source.go @@ -74,6 +74,7 @@ type OCIArtifact struct { // // +optional Prune *PruneOption `json:"prune,omitempty"` + // PullSecret references a Secret that contains credentials to access // the repository. // @@ -83,40 +84,39 @@ 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 is the BuildSource qualifier, the type of the source. + Type BuildSourceType `json:"type"` // ContextDir is a path to subfolder in the repo. Optional. // // +optional ContextDir *string `json:"contextDir,omitempty"` - // OCIArtifact + // OCIArtifact contains the details for the source of type OCIArtifact // // +optional OCIArtifact *OCIArtifact `json:"ociArtifact,omitempty"` - // GitSource + // Git contains the details for the source of type Git // // +optional - GitSource *Git `json:"git,omitempty"` + Git *Git `json:"git,omitempty"` - // LocalSource + // Local contains the details for the source of type 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. + // Type is the BuildRunSource qualifier, the type of the source. + // Only Local is supported. // - // +optional - Type BuildSourceType `json:"type,omitempty"` - // LocalSource + Type BuildSourceType `json:"type"` + + // Local contains the details for the source of type Local // - LocalSource *Local `json:"local,omitempty"` + // +optional + 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 index e2fa2ff9c2..b35e1ebe96 100644 --- a/pkg/apis/build/v1beta1/zz_generated.deepcopy.go +++ b/pkg/apis/build/v1beta1/zz_generated.deepcopy.go @@ -207,8 +207,8 @@ func (in *BuildRunRetention) DeepCopy() *BuildRunRetention { // 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 + if in.Local != nil { + in, out := &in.Local, &out.Local *out = new(Local) (*in).DeepCopyInto(*out) } @@ -352,7 +352,11 @@ func (in *BuildRunStatus) DeepCopy() *BuildRunStatus { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *BuildSpec) DeepCopyInto(out *BuildSpec) { *out = *in - in.Source.DeepCopyInto(&out.Source) + if in.Source != nil { + in, out := &in.Source, &out.Source + *out = new(Source) + (*in).DeepCopyInto(*out) + } if in.Trigger != nil { in, out := &in.Trigger, &out.Trigger *out = new(Trigger) @@ -1061,13 +1065,13 @@ func (in *Source) DeepCopyInto(out *Source) { *out = new(OCIArtifact) (*in).DeepCopyInto(*out) } - if in.GitSource != nil { - in, out := &in.GitSource, &out.GitSource + if in.Git != nil { + in, out := &in.Git, &out.Git *out = new(Git) (*in).DeepCopyInto(*out) } - if in.LocalSource != nil { - in, out := &in.LocalSource, &out.LocalSource + if in.Local != nil { + in, out := &in.Local, &out.Local *out = new(Local) (*in).DeepCopyInto(*out) } diff --git a/pkg/reconciler/build/build_test.go b/pkg/reconciler/build/build_test.go index aecd01c8d2..bd4a14ac15 100644 --- 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 index f05e76db6f..1745182351 100644 --- 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 index 91d29006ff..b2bdf4df8c 100644 --- 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, }, diff --git a/pkg/reconciler/buildrun/resources/results_test.go b/pkg/reconciler/buildrun/resources/results_test.go index 26bebae104..cb585aa551 100644 --- a/pkg/reconciler/buildrun/resources/results_test.go +++ b/pkg/reconciler/buildrun/resources/results_test.go @@ -50,9 +50,9 @@ var _ = Describe("TaskRun results to BuildRun", func() { It("should surface the TaskRun results emitting from default(git) source step", func() { commitSha := "0e0583421a5e4bf562ffe33f3651e16ba0c78591" br.Status.BuildSpec = &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", }, }, @@ -83,7 +83,7 @@ var _ = Describe("TaskRun results to BuildRun", func() { It("should surface the TaskRun results emitting from default(bundle) source step", func() { bundleImageDigest := "sha256:fe1b73cd25ac3f11dec752755e2" br.Status.BuildSpec = &build.BuildSpec{ - Source: build.Source{ + Source: &build.Source{ Type: build.OCIArtifactType, OCIArtifact: &build.OCIArtifact{ Image: "ghcr.io/shipwright-io/sample-go/source-bundle:latest", @@ -135,9 +135,9 @@ var _ = Describe("TaskRun results to BuildRun", func() { commitSha := "0e0583421a5e4bf562ffe33f3651e16ba0c78591" imageDigest := "sha256:fe1b73cd25ac3f11dec752755e2" br.Status.BuildSpec = &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", }, }, diff --git a/pkg/reconciler/buildrun/resources/sources.go b/pkg/reconciler/buildrun/resources/sources.go index 415cbb593e..2748e6e08c 100644 --- a/pkg/reconciler/buildrun/resources/sources.go +++ b/pkg/reconciler/buildrun/resources/sources.go @@ -31,12 +31,12 @@ func isLocalCopyBuildSource( // Note: In v1alpha1 we will append all sources across builds and buildruns, and then return // the first source of type Local. - if build.Spec.Source.Type == buildv1beta1.LocalType { - return build.Spec.Source.LocalSource + if build.Spec.Source != nil && build.Spec.Source.Type == buildv1beta1.LocalType { + return build.Spec.Source.Local } if buildRun.Spec.Source != nil && buildRun.Spec.Source.Type == buildv1beta1.LocalType { - return buildRun.Spec.Source.LocalSource + return buildRun.Spec.Source.Local } return nil @@ -61,7 +61,7 @@ func AmendTaskSpecWithSources( ) { if localCopy := isLocalCopyBuildSource(build, buildRun); localCopy != nil { sources.AppendLocalCopyStep(cfg, taskSpec, localCopy.Timeout) - } else { + } else if build.Spec.Source != nil { // create the step for spec.source, either Git or Bundle switch build.Spec.Source.Type { @@ -71,9 +71,9 @@ func AmendTaskSpecWithSources( sources.AppendBundleStep(cfg, taskSpec, build.Spec.Source.OCIArtifact, defaultSourceName) } case buildv1beta1.GitType: - if build.Spec.Source.GitSource != nil { + if build.Spec.Source.Git != nil { appendSourceTimestampResult(taskSpec) - sources.AppendGitStep(cfg, taskSpec, *build.Spec.Source.GitSource, defaultSourceName) + sources.AppendGitStep(cfg, taskSpec, *build.Spec.Source.Git, defaultSourceName) } } } @@ -82,13 +82,16 @@ func AmendTaskSpecWithSources( func updateBuildRunStatusWithSourceResult(buildrun *buildv1beta1.BuildRun, results []pipelineapi.TaskRunResult) { buildSpec := buildrun.Status.BuildSpec - // no results for HTTP sources yet + if buildSpec.Source == nil { + return + } + switch { case buildSpec.Source.Type == buildv1beta1.OCIArtifactType && buildSpec.Source.OCIArtifact != nil: sources.AppendBundleResult(buildrun, defaultSourceName, results) - case buildSpec.Source.Type == buildv1beta1.GitType && buildSpec.Source.GitSource != nil: - sources.AppendGitResult(buildrun, defaultSourceName, results) + case buildSpec.Source.Type == buildv1beta1.GitType && buildSpec.Source.Git != nil: + sources.AppendGitSourceResult(buildrun, defaultSourceName, results) } if sourceTimestamp := sources.FindResultValue(results, defaultSourceName, sourceTimestampName); strings.TrimSpace(sourceTimestamp) != "" { diff --git a/pkg/reconciler/buildrun/resources/sources/git.go b/pkg/reconciler/buildrun/resources/sources/git.go index 67c91b7ccc..b9c7b96ee3 100644 --- a/pkg/reconciler/buildrun/resources/sources/git.go +++ b/pkg/reconciler/buildrun/resources/sources/git.go @@ -109,8 +109,8 @@ func AppendGitStep( taskSpec.Steps = append(taskSpec.Steps, gitStep) } -// AppendGitResult append git source result to build run -func AppendGitResult(buildRun *buildv1beta1.BuildRun, name string, results []pipelineapi.TaskRunResult) { +// AppendGitSourceResult append git source result to build run +func AppendGitSourceResult(buildRun *buildv1beta1.BuildRun, name string, results []pipelineapi.TaskRunResult) { commitAuthor := FindResultValue(results, name, commitAuthorResult) commitSha := FindResultValue(results, name, commitSHAResult) branchName := FindResultValue(results, name, branchName) diff --git a/pkg/reconciler/buildrun/resources/taskrun.go b/pkg/reconciler/buildrun/resources/taskrun.go index 0fd3d69a78..5a4991c9a3 100644 --- a/pkg/reconciler/buildrun/resources/taskrun.go +++ b/pkg/reconciler/buildrun/resources/taskrun.go @@ -30,30 +30,8 @@ const ( paramSourceContext = "source-context" workspaceSource = "source" - - inputParamContextDir = "CONTEXT_DIR" ) -// getStringTransformations gets us MANDATORY replacements using -// a poor man's templating mechanism - TODO: Use golang templating -func getStringTransformations(fullText string) string { - - stringTransformations := map[string]string{ - // this will be removed, build strategy author should use $(params.shp-output-image) directly - "$(build.output.image)": fmt.Sprintf("$(params.%s-%s)", prefixParamsResultsVolumes, paramOutputImage), - - // this will be removed, build strategy author should use $(params.shp-source-context); it is still needed by the ko build - // strategy that mis-uses this setting to store the path to the main package; requires strategy parameter support to get rid - "$(build.source.contextDir)": fmt.Sprintf("$(inputs.params.%s)", inputParamContextDir), - } - - // Run the text through all possible replacements - for k, v := range stringTransformations { - fullText = strings.ReplaceAll(fullText, k, v) - } - return fullText -} - // GenerateTaskSpec creates Tekton TaskRun spec to be used for a build run func GenerateTaskSpec( cfg *config.Config, @@ -65,16 +43,6 @@ func GenerateTaskSpec( ) (*pipelineapi.TaskSpec, error) { generatedTaskSpec := pipelineapi.TaskSpec{ Params: []pipelineapi.ParamSpec{ - { - // CONTEXT_DIR comes from the git source specification - // in the Build object - Description: "The root of the code", - Name: inputParamContextDir, - Default: &pipelineapi.ParamValue{ - Type: pipelineapi.ParamTypeString, - StringVal: ".", - }, - }, { Name: fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, paramOutputImage), Description: "The URL of the image that the build produces", @@ -157,18 +125,6 @@ func GenerateTaskSpec( // define the steps coming from the build strategy for _, containerValue := range buildSteps { - var taskCommand []string - for _, buildStrategyCommandPart := range containerValue.Command { - taskCommand = append(taskCommand, getStringTransformations(buildStrategyCommandPart)) - } - - var taskArgs []string - for _, buildStrategyArgPart := range containerValue.Args { - taskArgs = append(taskArgs, getStringTransformations(buildStrategyArgPart)) - } - - taskImage := getStringTransformations(containerValue.Image) - // Any collision between the env vars in the Container step and those in the Build/BuildRun // will result in an error and cause a failed TaskRun stepEnv, err := env.MergeEnvVars(combinedEnvs, containerValue.Env, false) @@ -177,12 +133,12 @@ func GenerateTaskSpec( } step := pipelineapi.Step{ - Image: taskImage, + Image: containerValue.Image, ImagePullPolicy: containerValue.ImagePullPolicy, Name: containerValue.Name, VolumeMounts: containerValue.VolumeMounts, - Command: taskCommand, - Args: taskArgs, + Command: containerValue.Command, + Args: containerValue.Args, SecurityContext: containerValue.SecurityContext, WorkingDir: containerValue.WorkingDir, ComputeResources: containerValue.Resources, @@ -325,14 +281,7 @@ func GenerateTaskRun( }, } - if build.Spec.Source.ContextDir != nil { - params = append(params, pipelineapi.Param{ - Name: inputParamContextDir, - Value: pipelineapi.ParamValue{ - Type: pipelineapi.ParamTypeString, - StringVal: *build.Spec.Source.ContextDir, - }, - }) + if build.Spec.Source != nil && build.Spec.Source.ContextDir != nil { params = append(params, pipelineapi.Param{ Name: fmt.Sprintf("%s-%s", prefixParamsResultsVolumes, paramSourceContext), Value: pipelineapi.ParamValue{ diff --git a/pkg/reconciler/buildrun/resources/taskrun_test.go b/pkg/reconciler/buildrun/resources/taskrun_test.go index 5c3a02d976..f665ce0bfe 100644 --- a/pkg/reconciler/buildrun/resources/taskrun_test.go +++ b/pkg/reconciler/buildrun/resources/taskrun_test.go @@ -77,7 +77,7 @@ var _ = Describe("GenerateTaskrun", func() { Expect(got.Steps[0].Name).To(Equal("source-default")) Expect(got.Steps[0].Command[0]).To(Equal("/ko-app/git")) Expect(got.Steps[0].Args).To(Equal([]string{ - "--url", build.Spec.Source.GitSource.URL, + "--url", build.Spec.Source.Git.URL, "--target", "$(params.shp-source-root)", "--result-file-commit-sha", "$(results.shp-source-default-commit-sha.path)", "--result-file-commit-author", "$(results.shp-source-default-commit-author.path)", @@ -131,11 +131,11 @@ var _ = Describe("GenerateTaskrun", func() { Expect(got.Params).To(utils.ContainNamedElement("shp-output-image")) Expect(got.Params).To(utils.ContainNamedElement("shp-output-insecure")) - // legacy params - Expect(got.Params).ToNot(utils.ContainNamedElement("BUILDER_IMAGE")) // test build has no builder image - Expect(got.Params).To(utils.ContainNamedElement("CONTEXT_DIR")) + // legacy params have been removed + Expect(got.Params).ToNot(utils.ContainNamedElement("BUILDER_IMAGE")) + Expect(got.Params).ToNot(utils.ContainNamedElement("CONTEXT_DIR")) - Expect(len(got.Params)).To(Equal(6)) + Expect(len(got.Params)).To(Equal(5)) }) It("should contain a step to mutate the image with single mutate args", func() { @@ -382,15 +382,49 @@ var _ = Describe("GenerateTaskrun", func() { }) }) + Context("when Build and BuildRun have no source", func() { + + BeforeEach(func() { + build, err = ctl.LoadBuildYAML([]byte(test.BuildBSMinimalNoSource)) + Expect(err).ToNot(HaveOccurred()) + + buildRun, err = ctl.LoadBuildRunFromBytes([]byte(test.MinimalBuildahBuildRun)) + Expect(err).ToNot(HaveOccurred()) + + buildStrategy, err = ctl.LoadBuildStrategyFromBytes([]byte(test.MinimalBuildahBuildStrategy)) + Expect(err).ToNot(HaveOccurred()) + buildStrategy.Spec.Steps[0].ImagePullPolicy = "Always" + + expectedCommandOrArg = []string{ + "bud", "--tag=$(params.shp-output-image)", "--file=$(params.dockerfile)", "$(params.shp-source-context)", + } + }) + + JustBeforeEach(func() { + taskRun, err := resources.GenerateTaskRun(config.NewDefaultConfig(), build, buildRun, "", buildStrategy) + Expect(err).ToNot(HaveOccurred()) + got = taskRun.Spec.TaskSpec + }) + + It("should not contain a source step", func() { + sourceStepFound := false + for _, step := range got.Steps { + if strings.HasPrefix(step.Name, "source") { + sourceStepFound = true + } + } + Expect(sourceStepFound).To(BeFalse(), "Found unexpected source step") + }) + }) }) Describe("Generate the TaskRun", func() { var ( - k8sDuration30s *metav1.Duration - k8sDuration1m *metav1.Duration - namespace, contextDir, outputPath, outputPathBuildRun, serviceAccountName string - got *pipelineapi.TaskRun - err error + k8sDuration30s *metav1.Duration + k8sDuration1m *metav1.Duration + namespace, outputPath, outputPathBuildRun, serviceAccountName string + got *pipelineapi.TaskRun + err error ) BeforeEach(func() { duration, err := time.ParseDuration("30s") @@ -405,7 +439,6 @@ var _ = Describe("GenerateTaskrun", func() { } namespace = "build-test" - contextDir = "docker-build" outputPath = "image-registry.openshift-image-registry.svc:5000/example/buildpacks-app" outputPathBuildRun = "image-registry.openshift-image-registry.svc:5000/example/buildpacks-app-v2" serviceAccountName = buildpacks + "-serviceaccount" @@ -497,9 +530,6 @@ var _ = Describe("GenerateTaskrun", func() { paramOutputImageFound := false paramOutputInsecureFound := false - // legacy params - paramContextDirFound := false - for _, param := range params { switch param.Name { case "shp-source-root": @@ -518,10 +548,6 @@ var _ = Describe("GenerateTaskrun", func() { paramOutputInsecureFound = true Expect(param.Value.StringVal).To(Equal("false")) - case "CONTEXT_DIR": - paramContextDirFound = true - Expect(param.Value.StringVal).To(Equal(contextDir)) - default: Fail(fmt.Sprintf("Unexpected param found: %s", param.Name)) } @@ -531,7 +557,6 @@ var _ = Describe("GenerateTaskrun", func() { Expect(paramSourceContextFound).To(BeTrue()) Expect(paramOutputImageFound).To(BeTrue()) Expect(paramOutputInsecureFound).To(BeTrue()) - Expect(paramContextDirFound).To(BeTrue()) }) It("should ensure resource replacements happen when needed", func() { diff --git a/pkg/validate/sources.go b/pkg/validate/sources.go index 3b7161249c..593ec8c890 100644 --- a/pkg/validate/sources.go +++ b/pkg/validate/sources.go @@ -18,34 +18,33 @@ type SourceRef struct { // ValidatePath executes the validation routine, inspecting the `build.spec.source` path func (s *SourceRef) ValidatePath(_ context.Context) error { - - if err := s.validateSourceEntry(s.Build.Spec.Source); err != nil { - return err + if s.Build.Spec.Source != nil { + return s.validateSourceEntry(s.Build.Spec.Source) } return nil } // validateSourceEntry inspect informed entry, probes all required attributes. -func (s *SourceRef) validateSourceEntry(source build.Source) error { +func (s *SourceRef) validateSourceEntry(source *build.Source) error { // dont bail out if the Source object is empty, we preserve the old behaviour as in v1alpha1 - if source.Type == "" && source.GitSource == nil && - source.OCIArtifact == nil && source.LocalSource == nil { + if source.Type == "" && source.Git == nil && + source.OCIArtifact == nil && source.Local == nil { return nil } switch source.Type { case "Git": - if source.GitSource == nil || source.OCIArtifact != nil || source.LocalSource != nil { + if source.Git == nil || source.OCIArtifact != nil || source.Local != nil { return fmt.Errorf("type does not match the source") } case "OCI": - if source.OCIArtifact == nil || source.GitSource != nil || source.LocalSource != nil { + if source.OCIArtifact == nil || source.Git != nil || source.Local != nil { return fmt.Errorf("type does not match the source") } case "Local": - if source.LocalSource == nil || source.OCIArtifact != nil || source.GitSource != nil { + if source.Local == nil || source.OCIArtifact != nil || source.Git != nil { return fmt.Errorf("type does not match the source") } case "": diff --git a/pkg/validate/sources_test.go b/pkg/validate/sources_test.go index d2914eb8d3..182b0693cb 100644 --- a/pkg/validate/sources_test.go +++ b/pkg/validate/sources_test.go @@ -25,9 +25,9 @@ var _ = Describe("SourcesRef", func() { It("should successfully validate a build with source", func() { srcRef := validate.NewSourceRef(&build.Build{ Spec: build.BuildSpec{ - Source: build.Source{ - Type: "Git", - GitSource: &build.Git{}, + Source: &build.Source{ + Type: "Git", + Git: &build.Git{}, }, }, }) @@ -38,8 +38,8 @@ var _ = Describe("SourcesRef", func() { It("should fail to validate if the type is not defined", func() { srcRef := validate.NewSourceRef(&build.Build{ Spec: build.BuildSpec{ - Source: build.Source{ - GitSource: &build.Git{}, + Source: &build.Source{ + Git: &build.Git{}, }, }, }) @@ -50,9 +50,9 @@ var _ = Describe("SourcesRef", func() { It("should fail to validate if the type does not match the source git", func() { srcRef := validate.NewSourceRef(&build.Build{ Spec: build.BuildSpec{ - Source: build.Source{ - Type: "OCI", - GitSource: &build.Git{}, + Source: &build.Source{ + Type: "OCI", + Git: &build.Git{}, }, }, }) diff --git a/pkg/validate/sourceurl.go b/pkg/validate/sourceurl.go index 888f889f1a..a48ca20bb8 100644 --- a/pkg/validate/sourceurl.go +++ b/pkg/validate/sourceurl.go @@ -31,12 +31,12 @@ func NewSourceURL(client client.Client, build *build.Build) *SourceURLRef { // that the spec.source.url exists. This validation only applies // to endpoints that do not require authentication. func (s SourceURLRef) ValidatePath(ctx context.Context) error { - if s.Build.Spec.Source.Type == build.GitType { - gitSource := s.Build.Spec.Source.GitSource - if gitSource.CloneSecret == nil { + if s.Build.Spec.Source != nil && s.Build.Spec.Source.Type == build.GitType && s.Build.Spec.Source.Git != nil { + Git := s.Build.Spec.Source.Git + if Git.CloneSecret == nil { switch s.Build.GetAnnotations()[build.AnnotationBuildVerifyRepository] { case "true": - if err := git.ValidateGitURLExists(ctx, gitSource.URL); err != nil { + if err := git.ValidateGitURLExists(ctx, Git.URL); err != nil { s.MarkBuildStatus(s.Build, build.RemoteRepositoryUnreachable, err.Error()) return err } diff --git a/pkg/webhook/conversion/converter_test.go b/pkg/webhook/conversion/converter_test.go index 3ec33d8184..c264bb353f 100644 --- a/pkg/webhook/conversion/converter_test.go +++ b/pkg/webhook/conversion/converter_test.go @@ -242,7 +242,7 @@ request: // Use ComparableTo and assert the whole object Expect(build).To(BeComparableTo(desiredBuild)) }) - It("converts for spec source GitSource type, strategy, params and output", func() { + It("converts for spec source Git type, strategy, params and output", func() { // Create the yaml in v1beta1 buildTemplate := `kind: ConversionReview apiVersion: %s @@ -367,7 +367,7 @@ request: // Use ComparableTo and assert the whole object Expect(build).To(BeComparableTo(desiredBuild)) }) - It("converts for spec source GitSource type, strategy, retention and volumes", func() { + It("converts for spec source Git type, strategy, retention and volumes", func() { limit := uint(10) // Create the yaml in v1beta1 buildTemplate := `kind: ConversionReview @@ -525,9 +525,9 @@ request: Name: "buildkit-build", }, Spec: v1beta1.BuildSpec{ - Source: v1beta1.Source{ + Source: &v1beta1.Source{ Type: v1beta1.LocalType, - LocalSource: &v1beta1.Local{ + Local: &v1beta1.Local{ Name: "foobar_local", Timeout: &v1.Duration{ Duration: 1 * time.Minute, @@ -620,7 +620,7 @@ request: Name: "buildkit-build", }, Spec: v1beta1.BuildSpec{ - Source: v1beta1.Source{ + Source: &v1beta1.Source{ Type: v1beta1.OCIArtifactType, ContextDir: &ctxDir, OCIArtifact: &v1beta1.OCIArtifact{ @@ -737,10 +737,10 @@ request: Name: "buildkit-build", }, Spec: v1beta1.BuildSpec{ - Source: v1beta1.Source{ + Source: &v1beta1.Source{ Type: v1beta1.GitType, ContextDir: &ctxDir, - GitSource: &v1beta1.Git{ + Git: &v1beta1.Git{ URL: url, Revision: &revision, CloneSecret: &secretName, @@ -1239,7 +1239,7 @@ request: }, Source: &v1beta1.BuildRunSource{ Type: v1beta1.LocalType, - LocalSource: &v1beta1.Local{ + Local: &v1beta1.Local{ Name: "foobar_local", Timeout: &v1.Duration{ Duration: 1 * time.Minute, diff --git a/test/e2e/v1beta1/common_suite_test.go b/test/e2e/v1beta1/common_suite_test.go index 036ddbe746..732508a35a 100644 --- a/test/e2e/v1beta1/common_suite_test.go +++ b/test/e2e/v1beta1/common_suite_test.go @@ -65,11 +65,19 @@ func (b *buildPrototype) ClusterBuildStrategy(name string) *buildPrototype { } func (b *buildPrototype) SourceCredentials(name string) *buildPrototype { - if name != "" { - if b.build.Spec.Source.GitSource == nil { - b.build.Spec.Source.GitSource = &buildv1beta1.Git{} + if name != "" && b.build.Spec.Source != nil { + switch b.build.Spec.Source.Type { + case buildv1beta1.OCIArtifactType: + if b.build.Spec.Source.OCIArtifact == nil { + b.build.Spec.Source.OCIArtifact = &buildv1beta1.OCIArtifact{} + } + b.build.Spec.Source.OCIArtifact.PullSecret = &name + case buildv1beta1.GitType: + if b.build.Spec.Source.Git == nil { + b.build.Spec.Source.Git = &buildv1beta1.Git{} + } + b.build.Spec.Source.Git.CloneSecret = &name } - b.build.Spec.Source.GitSource.CloneSecret = &name } return b @@ -81,21 +89,28 @@ func (b *buildPrototype) SourceType(sourceType string) *buildPrototype { } func (b *buildPrototype) SourceGit(repository string) *buildPrototype { - if b.build.Spec.Source.GitSource == nil { - b.build.Spec.Source.GitSource = &buildv1beta1.Git{} + if b.build.Spec.Source == nil { + b.build.Spec.Source = &buildv1beta1.Source{} + } + if b.build.Spec.Source.Git == nil { + b.build.Spec.Source.Git = &buildv1beta1.Git{} } b.build.Spec.Source.Type = buildv1beta1.GitType - b.build.Spec.Source.GitSource.URL = repository + b.build.Spec.Source.Git.URL = repository b.build.Spec.Source.OCIArtifact = nil return b } func (b *buildPrototype) SourceBundle(image string) *buildPrototype { + if b.build.Spec.Source == nil { + b.build.Spec.Source = &buildv1beta1.Source{} + } if b.build.Spec.Source.OCIArtifact == nil { b.build.Spec.Source.OCIArtifact = &buildv1beta1.OCIArtifact{} } b.build.Spec.Source.Type = buildv1beta1.OCIArtifactType b.build.Spec.Source.OCIArtifact.Image = image + b.build.Spec.Source.Git = nil return b } diff --git a/test/e2e/v1beta1/common_test.go b/test/e2e/v1beta1/common_test.go index fd317de76b..494ec221d5 100644 --- a/test/e2e/v1beta1/common_test.go +++ b/test/e2e/v1beta1/common_test.go @@ -96,20 +96,20 @@ func amendOutputCredentials(b *buildv1beta1.Build, secretName string) { Logf("Amended object: name='%s', secret-ref='%s'", b.Name, secretName) } -// amendSourceSecretName patch Build source.Credentials with secret name. +// amendSourceSecretName patch Build source.Git.CloneSecret with secret name. func amendSourceSecretName(b *buildv1beta1.Build, secretName string) { if secretName == "" { return } - b.Spec.Source.GitSource.CloneSecret = &secretName + b.Spec.Source.Git.CloneSecret = &secretName } -// amendSourceURL patch Build source.URL with informed string. +// amendSourceURL patch Build source.Git.URL with informed string. func amendSourceURL(b *buildv1beta1.Build, sourceURL string) { if sourceURL == "" { return } - b.Spec.Source.GitSource.URL = sourceURL + b.Spec.Source.Git.URL = sourceURL } // amendBuild make changes on build object. diff --git a/test/integration/build_to_git_test.go b/test/integration/build_to_git_test.go index 5fb1f74a12..6e781d4dce 100644 --- a/test/integration/build_to_git_test.go +++ b/test/integration/build_to_git_test.go @@ -47,9 +47,9 @@ var _ = Describe("Integration tests Build and referenced Source url", func() { Expect(err).To(BeNil()) buildObject.ObjectMeta.Annotations["build.shipwright.io/verify.repository"] = "true" - buildObject.Spec.Source = v1beta1.Source{ + buildObject.Spec.Source = &v1beta1.Source{ Type: v1beta1.GitType, - GitSource: &v1beta1.Git{ + Git: &v1beta1.Git{ URL: "http://github.com/shipwright-io/sample-go", }, } @@ -77,9 +77,9 @@ var _ = Describe("Integration tests Build and referenced Source url", func() { ) Expect(err).To(BeNil()) - buildObject.Spec.Source = v1beta1.Source{ + buildObject.Spec.Source = &v1beta1.Source{ Type: v1beta1.GitType, - GitSource: &v1beta1.Git{ + Git: &v1beta1.Git{ URL: "http://github.com/shipwright-io/sample-go-fake", }, } @@ -108,9 +108,9 @@ var _ = Describe("Integration tests Build and referenced Source url", func() { Expect(err).To(BeNil()) buildObject.ObjectMeta.Annotations["build.shipwright.io/verify.repository"] = "true" - buildObject.Spec.Source = v1beta1.Source{ + buildObject.Spec.Source = &v1beta1.Source{ Type: v1beta1.GitType, - GitSource: &v1beta1.Git{ + Git: &v1beta1.Git{ URL: "https://github.com/shipwright-io/sample-go", }, } @@ -138,9 +138,9 @@ var _ = Describe("Integration tests Build and referenced Source url", func() { ) Expect(err).To(BeNil()) - buildObject.Spec.Source = v1beta1.Source{ + buildObject.Spec.Source = &v1beta1.Source{ Type: v1beta1.GitType, - GitSource: &v1beta1.Git{ + Git: &v1beta1.Git{ URL: "https://github.com/shipwright-io/sample-go-fake", }, } @@ -169,9 +169,9 @@ var _ = Describe("Integration tests Build and referenced Source url", func() { Expect(err).To(BeNil()) buildObject.ObjectMeta.Annotations["build.shipwright.io/verify.repository"] = "true" - buildObject.Spec.Source = v1beta1.Source{ + buildObject.Spec.Source = &v1beta1.Source{ Type: v1beta1.GitType, - GitSource: &v1beta1.Git{ + Git: &v1beta1.Git{ URL: "foobar", }, } @@ -201,9 +201,9 @@ var _ = Describe("Integration tests Build and referenced Source url", func() { Expect(err).To(BeNil()) buildObject.ObjectMeta.Annotations["build.shipwright.io/verify.repository"] = "false" - buildObject.Spec.Source = v1beta1.Source{ + buildObject.Spec.Source = &v1beta1.Source{ Type: v1beta1.GitType, - GitSource: &v1beta1.Git{ + Git: &v1beta1.Git{ URL: "foobar", }, } @@ -232,9 +232,9 @@ var _ = Describe("Integration tests Build and referenced Source url", func() { Expect(err).To(BeNil()) buildObject.ObjectMeta.Annotations["build.shipwright.io/verify.repository"] = "true" - buildObject.Spec.Source = v1beta1.Source{ + buildObject.Spec.Source = &v1beta1.Source{ Type: v1beta1.GitType, - GitSource: &v1beta1.Git{ + Git: &v1beta1.Git{ URL: "https://github.yourco.com/org/build-fake", }, } @@ -263,15 +263,15 @@ var _ = Describe("Integration tests Build and referenced Source url", func() { Expect(err).To(BeNil()) buildObject.ObjectMeta.Annotations["build.shipwright.io/verify.repository"] = "true" - buildObject.Spec.Source = v1beta1.Source{ + buildObject.Spec.Source = &v1beta1.Source{ Type: v1beta1.GitType, - GitSource: &v1beta1.Git{ + Git: &v1beta1.Git{ URL: "https://github.yourco.com/org/build-fake", CloneSecret: pointer.String("foobar"), }, } - sampleSecret := tb.Catalog.SecretWithAnnotation(*buildObject.Spec.Source.GitSource.CloneSecret, buildObject.Namespace) + sampleSecret := tb.Catalog.SecretWithAnnotation(*buildObject.Spec.Source.Git.CloneSecret, buildObject.Namespace) Expect(tb.CreateSecret(sampleSecret)).To(BeNil()) Expect(tb.CreateBuild(buildObject)).To(BeNil()) @@ -300,9 +300,9 @@ var _ = Describe("Integration tests Build and referenced Source url", func() { Expect(err).To(BeNil()) buildObject.ObjectMeta.Annotations["build.shipwright.io/verify.repository"] = "true" - buildObject.Spec.Source = v1beta1.Source{ + buildObject.Spec.Source = &v1beta1.Source{ Type: v1beta1.GitType, - GitSource: &v1beta1.Git{ + Git: &v1beta1.Git{ URL: "git@github.com:shipwright-io/build-fake.git", }, } @@ -333,9 +333,9 @@ var _ = Describe("Integration tests Build and referenced Source url", func() { Expect(err).To(BeNil()) buildObject.ObjectMeta.Annotations["build.shipwright.io/verify.repository"] = "true" - buildObject.Spec.Source = v1beta1.Source{ + buildObject.Spec.Source = &v1beta1.Source{ Type: v1beta1.GitType, - GitSource: &v1beta1.Git{ + Git: &v1beta1.Git{ URL: "ssh://github.com/shipwright-io/build-fake.git", }, } diff --git a/test/integration/build_to_secrets_test.go b/test/integration/build_to_secrets_test.go index 4cb0efb714..7b22646d33 100644 --- a/test/integration/build_to_secrets_test.go +++ b/test/integration/build_to_secrets_test.go @@ -222,7 +222,7 @@ var _ = Describe("Integration tests Build and referenced Secrets", func() { ) Expect(err).To(BeNil()) - sampleSecret := tb.Catalog.SecretWithAnnotation(*buildObject.Spec.Source.GitSource.CloneSecret, buildObject.Namespace) + sampleSecret := tb.Catalog.SecretWithAnnotation(*buildObject.Spec.Source.Git.CloneSecret, buildObject.Namespace) Expect(tb.CreateSecret(sampleSecret)).To(BeNil()) @@ -236,14 +236,14 @@ var _ = Describe("Integration tests Build and referenced Secrets", func() { Expect(*buildObject.Status.Message).To(Equal("all validations succeeded")) // delete a secret - Expect(tb.DeleteSecret(*buildObject.Spec.Source.GitSource.CloneSecret)).To(BeNil()) + Expect(tb.DeleteSecret(*buildObject.Spec.Source.Git.CloneSecret)).To(BeNil()) // assert that the validation happened one more time buildObject, err = tb.GetBuildTillRegistration(buildName, corev1.ConditionFalse) Expect(err).To(BeNil()) Expect(*buildObject.Status.Registered).To(Equal(corev1.ConditionFalse)) Expect(*buildObject.Status.Reason).To(Equal(v1beta1.SpecSourceSecretRefNotFound)) - Expect(*buildObject.Status.Message).To(Equal(fmt.Sprintf("referenced secret %s not found", *buildObject.Spec.Source.GitSource.CloneSecret))) + Expect(*buildObject.Status.Message).To(Equal(fmt.Sprintf("referenced secret %s not found", *buildObject.Spec.Source.Git.CloneSecret))) }) It("should validate when a missing secret is recreated", func() { @@ -265,7 +265,7 @@ var _ = Describe("Integration tests Build and referenced Secrets", func() { // Status reason sometimes returns message "there are no secrets in namespace..." // Expect(buildObject.Status.Reason).To(Equal(fmt.Sprintf("secret %s does not exist", buildObject.Spec.Source.Credentials.Name))) - sampleSecret := tb.Catalog.SecretWithAnnotation(*buildObject.Spec.Source.GitSource.CloneSecret, buildObject.Namespace) + sampleSecret := tb.Catalog.SecretWithAnnotation(*buildObject.Spec.Source.Git.CloneSecret, buildObject.Namespace) // generate resources Expect(tb.CreateSecret(sampleSecret)).To(BeNil()) @@ -299,7 +299,7 @@ var _ = Describe("Integration tests Build and referenced Secrets", func() { ) Expect(err).To(BeNil()) - specSourceSecret := tb.Catalog.SecretWithAnnotation(*firstBuildObject.Spec.Source.GitSource.CloneSecret, firstBuildObject.Namespace) + specSourceSecret := tb.Catalog.SecretWithAnnotation(*firstBuildObject.Spec.Source.Git.CloneSecret, firstBuildObject.Namespace) Expect(tb.CreateSecret(specSourceSecret)).To(BeNil()) @@ -325,14 +325,14 @@ var _ = Describe("Integration tests Build and referenced Secrets", func() { Expect(err).To(BeNil()) Expect(*o.Status.Registered).To(Equal(corev1.ConditionFalse)) Expect(*o.Status.Reason).To(Equal(v1beta1.SpecSourceSecretRefNotFound)) - Expect(*o.Status.Message).To(Equal(fmt.Sprintf("referenced secret %s not found", *firstBuildObject.Spec.Source.GitSource.CloneSecret))) + Expect(*o.Status.Message).To(Equal(fmt.Sprintf("referenced secret %s not found", *firstBuildObject.Spec.Source.Git.CloneSecret))) // assert that the validation happened one more time o, err = tb.GetBuildTillRegistration(secondBuildName, corev1.ConditionFalse) Expect(err).To(BeNil()) Expect(*o.Status.Registered).To(Equal(corev1.ConditionFalse)) Expect(*o.Status.Reason).To(Equal(v1beta1.SpecSourceSecretRefNotFound)) - Expect(*o.Status.Message).To(Equal(fmt.Sprintf("referenced secret %s not found", *secondBuildObject.Spec.Source.GitSource.CloneSecret))) + Expect(*o.Status.Message).To(Equal(fmt.Sprintf("referenced secret %s not found", *secondBuildObject.Spec.Source.Git.CloneSecret))) }) It("should validate the Builds when a missing secret is recreated", func() { // populate Build related vars @@ -365,7 +365,7 @@ var _ = Describe("Integration tests Build and referenced Secrets", func() { Expect(err).To(BeNil()) Expect(*buildObject.Status.Registered).To(Equal(corev1.ConditionFalse)) - specSourceSecret := tb.Catalog.SecretWithAnnotation(*firstBuildObject.Spec.Source.GitSource.CloneSecret, firstBuildObject.Namespace) + specSourceSecret := tb.Catalog.SecretWithAnnotation(*firstBuildObject.Spec.Source.Git.CloneSecret, firstBuildObject.Namespace) // generate resources Expect(tb.CreateSecret(specSourceSecret)).To(BeNil()) diff --git a/test/integration/buildrun_status_test.go b/test/integration/buildrun_status_test.go index 810619d5f1..529fa64b13 100644 --- a/test/integration/buildrun_status_test.go +++ b/test/integration/buildrun_status_test.go @@ -47,9 +47,9 @@ var _ = Describe("Checking BuildRun Status fields", func() { Spec: v1beta1.BuildRunSpec{ Build: v1beta1.ReferencedBuild{ Build: &v1beta1.BuildSpec{ - Source: v1beta1.Source{ + Source: &v1beta1.Source{ Type: v1beta1.GitType, - GitSource: &v1beta1.Git{ + Git: &v1beta1.Git{ URL: "https://github.com/shipwright-io/sample-go", Revision: pointer.String("v0.1.0"), }, @@ -88,7 +88,7 @@ var _ = Describe("Checking BuildRun Status fields", func() { Kind: &cbs, Name: strategy.Name, }, - Source: v1beta1.Source{ + Source: &v1beta1.Source{ Type: v1beta1.OCIArtifactType, OCIArtifact: &v1beta1.OCIArtifact{ Image: "ghcr.io/shipwright-io/sample-go/source-bundle@sha256:9a5e264c19980387b8416e0ffa7460488272fb8a6a56127c657edaa2682daab2", diff --git a/test/v1beta1_samples/build_samples.go b/test/v1beta1_samples/build_samples.go index 271f157c1f..289a33bd50 100644 --- a/test/v1beta1_samples/build_samples.go +++ b/test/v1beta1_samples/build_samples.go @@ -185,12 +185,11 @@ spec: image: image-registry.openshift-image-registry.svc:5000/example/buildpacks-app ` -// BuildBSMinimalNoSource defines a Build with a BuildStrategy without sources +// BuildBSMinimalNoSource defines a Build with a BuildStrategy without source const BuildBSMinimalNoSource = ` apiVersion: shipwright.io/v1beta1 kind: Build spec: - source: {} strategy: kind: BuildStrategy output: diff --git a/test/v1beta1_samples/catalog.go b/test/v1beta1_samples/catalog.go index 62b4994801..58a38ef402 100644 --- a/test/v1beta1_samples/catalog.go +++ b/test/v1beta1_samples/catalog.go @@ -98,8 +98,8 @@ func (c *Catalog) BuildWithClusterBuildStrategyAndFalseSourceAnnotation(name str Annotations: map[string]string{build.AnnotationBuildVerifyRepository: "false"}, }, Spec: build.BuildSpec{ - Source: build.Source{ - GitSource: &build.Git{ + Source: &build.Source{ + Git: &build.Git{ URL: "foobar", }, Type: build.GitType, @@ -124,8 +124,8 @@ func (c *Catalog) BuildWithClusterBuildStrategy(name string, ns string, strategy Namespace: ns, }, Spec: build.BuildSpec{ - Source: build.Source{ - GitSource: &build.Git{ + Source: &build.Source{ + Git: &build.Git{ URL: "https://github.com/shipwright-io/sample-go", }, Type: build.GitType, @@ -152,8 +152,8 @@ func (c *Catalog) BuildWithClusterBuildStrategyAndSourceSecret(name string, ns s Namespace: ns, }, Spec: build.BuildSpec{ - Source: build.Source{ - GitSource: &build.Git{ + Source: &build.Source{ + Git: &build.Git{ URL: "https://github.com/shipwright-io/sample-go", CloneSecret: &secret, }, @@ -179,8 +179,8 @@ func (c *Catalog) BuildWithBuildStrategy(name string, ns string, strategyName st Namespace: ns, }, Spec: build.BuildSpec{ - Source: build.Source{ - GitSource: &build.Git{ + Source: &build.Source{ + Git: &build.Git{ URL: "https://github.com/shipwright-io/sample-go", }, Type: build.GitType, @@ -201,8 +201,8 @@ func (c *Catalog) BuildWithNilBuildStrategyKind(name string, ns string, strategy Namespace: ns, }, Spec: build.BuildSpec{ - Source: build.Source{ - GitSource: &build.Git{ + Source: &build.Source{ + Git: &build.Git{ URL: "https://github.com/shipwright-io/sample-go", }, Type: build.GitType, @@ -222,8 +222,8 @@ func (c *Catalog) BuildWithOutputSecret(name string, ns string, secretName strin Namespace: ns, }, Spec: build.BuildSpec{ - Source: build.Source{ - GitSource: &build.Git{ + Source: &build.Source{ + Git: &build.Git{ URL: "https://github.com/shipwright-io/sample-go", }, Type: build.GitType,