From 54058c808b46f91f9327a933e0c601ad9f256681 Mon Sep 17 00:00:00 2001 From: encalada Date: Wed, 14 Feb 2024 16:05:22 +0100 Subject: [PATCH] Add changes based on code review Multiple items, most important ones: - Reintroduce source validation for Build - Reintroduce test case when retention is removed from Build - Fix Build Update predicate for AtBuildDeletion --- pkg/reconciler/build/build.go | 1 + pkg/reconciler/build/controller.go | 33 ++++++---- pkg/reconciler/buildrun/buildrun.go | 1 + pkg/reconciler/buildrun/buildrun_test.go | 3 +- .../buildrun/resources/credentials.go | 12 ++-- pkg/reconciler/buildrun/resources/results.go | 10 ++- .../buildrun/resources/results_test.go | 1 + .../buildrun/resources/service_accounts.go | 3 + .../resources/service_accounts_test.go | 2 +- pkg/reconciler/buildrun/resources/sources.go | 6 +- .../buildrun/resources/sources/bundle.go | 3 + .../buildrun/resources/sources/git.go | 4 ++ pkg/validate/ownerreferences.go | 50 +++++++-------- pkg/validate/secrets.go | 2 +- pkg/validate/sources.go | 60 ++++++++++++++++++ pkg/validate/sources_test.go | 63 +++++++++++++++++++ pkg/validate/sourceurl.go | 2 +- pkg/validate/validate.go | 4 +- test/e2e/v1beta1/common_suite_test.go | 2 + test/e2e/v1beta1/e2e_one_off_builds_test.go | 2 - test/integration/build_to_buildruns_test.go | 36 +++++++++++ test/utils/v1beta1/builds.go | 10 +++ test/v1beta1_samples/buildrun_samples.go | 1 + 23 files changed, 250 insertions(+), 61 deletions(-) create mode 100644 pkg/validate/sources.go create mode 100644 pkg/validate/sources_test.go diff --git a/pkg/reconciler/build/build.go b/pkg/reconciler/build/build.go index 88f61ea479..ea4d246ea9 100644 --- a/pkg/reconciler/build/build.go +++ b/pkg/reconciler/build/build.go @@ -28,6 +28,7 @@ var validationTypes = [...]string{ validate.SourceURL, validate.Secrets, validate.Strategies, + validate.Source, validate.BuildName, validate.Envs, validate.Triggers, diff --git a/pkg/reconciler/build/controller.go b/pkg/reconciler/build/controller.go index c0537de43e..ec673d9ceb 100644 --- a/pkg/reconciler/build/controller.go +++ b/pkg/reconciler/build/controller.go @@ -68,18 +68,27 @@ func add(ctx context.Context, mgr manager.Manager, r reconcile.Reconciler, maxCo oldBuildRetention := o.Spec.Retention newBuildRetention := n.Spec.Retention - if o.Spec.Retention != nil && n.Spec.Retention != nil { - if !reflect.DeepEqual(oldBuildRetention, newBuildRetention) { - if o.Spec.Retention.AtBuildDeletion != n.Spec.Retention.AtBuildDeletion { - ctxlog.Debug( - ctx, - "updating predicated passed, the build retention AtBuildDeletion was modified.", - namespace, - n.GetNamespace(), - name, - n.GetName(), - ) - buildAtBuildDeletion = true + logAndEnableDeletion := func() { + ctxlog.Debug( + ctx, + "updating predicated passed, the build retention AtBuildDeletion was modified.", + namespace, + n.GetNamespace(), + name, + n.GetName(), + ) + buildAtBuildDeletion = true + } + + if !reflect.DeepEqual(oldBuildRetention, newBuildRetention) { + switch { + case o.Spec.Retention == nil && n.Spec.Retention != nil: + if n.Spec.Retention.AtBuildDeletion != nil { + logAndEnableDeletion() + } + case o.Spec.Retention != nil && n.Spec.Retention == nil: + if o.Spec.Retention.AtBuildDeletion != nil { + logAndEnableDeletion() } } } diff --git a/pkg/reconciler/buildrun/buildrun.go b/pkg/reconciler/buildrun/buildrun.go index d4f247494a..de8a675e48 100644 --- a/pkg/reconciler/buildrun/buildrun.go +++ b/pkg/reconciler/buildrun/buildrun.go @@ -157,6 +157,7 @@ func (r *ReconcileBuildRun) Reconcile(ctx context.Context, request reconcile.Req validate.NewSourceURL(r.client, build), validate.NewCredentials(r.client, build), validate.NewStrategies(r.client, build), + validate.NewSourceRef(build), validate.NewBuildName(build), validate.NewEnv(build), ) diff --git a/pkg/reconciler/buildrun/buildrun_test.go b/pkg/reconciler/buildrun/buildrun_test.go index 68ec0ce9a6..113b4ef77b 100644 --- a/pkg/reconciler/buildrun/buildrun_test.go +++ b/pkg/reconciler/buildrun/buildrun_test.go @@ -1323,7 +1323,7 @@ var _ = Describe("Reconcile BuildRun", func() { }) }) - It("should mark BuildRun as invalid if BuildRef and BuildSpec are used", func() { + It("should mark BuildRun as invalid if Build name and BuildSpec are used", func() { buildRunSample = &build.BuildRun{ ObjectMeta: metav1.ObjectMeta{ Name: buildRunName, @@ -1432,6 +1432,7 @@ var _ = Describe("Reconcile BuildRun", func() { Build: build.ReferencedBuild{ Build: &build.BuildSpec{ Source: build.Source{ + Type: build.GitType, GitSource: &build.Git{ URL: "https://github.com/shipwright-io/sample-go.git", }, diff --git a/pkg/reconciler/buildrun/resources/credentials.go b/pkg/reconciler/buildrun/resources/credentials.go index 960c6eba36..495154b410 100644 --- a/pkg/reconciler/buildrun/resources/credentials.go +++ b/pkg/reconciler/buildrun/resources/credentials.go @@ -22,31 +22,31 @@ func ApplyCredentials(ctx context.Context, build *buildv1beta1.Build, buildRun * // if output is overridden by buildrun, and if this override has credentials, // it should be added to the sa if buildRun.Spec.Output != nil && buildRun.Spec.Output.PushSecret != nil { - modified = updateServiceAccountIfSecretNotLinked(ctx, buildRun.Spec.Output.PushSecret, serviceAccount) || modified + modified = updateServiceAccountIfSecretNotLinked(ctx, *buildRun.Spec.Output.PushSecret, serviceAccount) || modified } else { // otherwise, if buildrun does not override the output credentials, // we should use the ones provided by the build if build.Spec.Output.PushSecret != nil { - modified = updateServiceAccountIfSecretNotLinked(ctx, build.Spec.Output.PushSecret, serviceAccount) || modified + modified = updateServiceAccountIfSecretNotLinked(ctx, *build.Spec.Output.PushSecret, serviceAccount) || modified } } return modified } -func updateServiceAccountIfSecretNotLinked(ctx context.Context, sourceSecret *string, serviceAccount *corev1.ServiceAccount) bool { +func updateServiceAccountIfSecretNotLinked(ctx context.Context, sourceSecret string, serviceAccount *corev1.ServiceAccount) bool { isSecretPresent := false for _, credentialSecret := range serviceAccount.Secrets { - if credentialSecret.Name == *sourceSecret { + if credentialSecret.Name == sourceSecret { isSecretPresent = true break } } if !isSecretPresent { - ctxlog.Debug(ctx, "adding secret to serviceAccount", "secret", *sourceSecret, "serviceAccount", serviceAccount.Name) + ctxlog.Debug(ctx, "adding secret to serviceAccount", "secret", sourceSecret, "serviceAccount", serviceAccount.Name) serviceAccount.Secrets = append(serviceAccount.Secrets, corev1.ObjectReference{ - Name: *sourceSecret, + Name: sourceSecret, }) return true } diff --git a/pkg/reconciler/buildrun/resources/results.go b/pkg/reconciler/buildrun/resources/results.go index 1f2775117b..d6d60dae91 100644 --- a/pkg/reconciler/buildrun/resources/results.go +++ b/pkg/reconciler/buildrun/resources/results.go @@ -29,20 +29,18 @@ func UpdateBuildRunUsingTaskResults( taskRunResult []pipelineapi.TaskRunResult, request reconcile.Request, ) { - // Initializing source result - buildRun.Status.Source = &build.SourceResult{} - // Set source results updateBuildRunStatusWithSourceResult(buildRun, taskRunResult) - // Initializing output result - buildRun.Status.Output = &build.Output{} - // Set output results updateBuildRunStatusWithOutputResult(ctx, buildRun, taskRunResult, request) } func updateBuildRunStatusWithOutputResult(ctx context.Context, buildRun *build.BuildRun, taskRunResult []pipelineapi.TaskRunResult, request reconcile.Request) { + if buildRun.Status.Output == nil { + buildRun.Status.Output = &build.Output{} + } + for _, result := range taskRunResult { switch result.Name { case generateOutputResultName(imageDigestResult): diff --git a/pkg/reconciler/buildrun/resources/results_test.go b/pkg/reconciler/buildrun/resources/results_test.go index 6085e83643..85e56098cc 100644 --- a/pkg/reconciler/buildrun/resources/results_test.go +++ b/pkg/reconciler/buildrun/resources/results_test.go @@ -176,6 +176,7 @@ var _ = Describe("TaskRun results to BuildRun", func() { resources.UpdateBuildRunUsingTaskResults(ctx, br, tr.Status.Results, taskRunRequest) Expect(br.Status.Source).ToNot(Equal(nil)) + Expect(br.Status.Source.Git).ToNot(Equal(nil)) Expect(br.Status.Source.Git.CommitSha).To(Equal(commitSha)) Expect(br.Status.Source.Git.CommitAuthor).To(Equal("foo bar")) Expect(br.Status.Output.Digest).To(Equal(imageDigest)) diff --git a/pkg/reconciler/buildrun/resources/service_accounts.go b/pkg/reconciler/buildrun/resources/service_accounts.go index b27ec6301c..d49f308258 100644 --- a/pkg/reconciler/buildrun/resources/service_accounts.go +++ b/pkg/reconciler/buildrun/resources/service_accounts.go @@ -88,6 +88,9 @@ func GenerateSA(ctx context.Context, client client.Client, build *buildv1beta1.B // DeleteServiceAccount deletes the service account of a completed BuildRun if the service account // was generated func DeleteServiceAccount(ctx context.Context, client client.Client, completedBuildRun *buildv1beta1.BuildRun) error { + if !IsGeneratedServiceAccountUsed(completedBuildRun) { + return nil + } serviceAccount := &corev1.ServiceAccount{} serviceAccount.Name = GetGeneratedServiceAccountName(completedBuildRun) serviceAccount.Namespace = completedBuildRun.Namespace diff --git a/pkg/reconciler/buildrun/resources/service_accounts_test.go b/pkg/reconciler/buildrun/resources/service_accounts_test.go index 2534fa3968..eb271f9e3c 100644 --- a/pkg/reconciler/buildrun/resources/service_accounts_test.go +++ b/pkg/reconciler/buildrun/resources/service_accounts_test.go @@ -128,7 +128,7 @@ var _ = Describe("Operating service accounts", func() { }) }) - Context("Retrieving autogenerated service accounts when the spec.serviceAccount .generated value is used", func() { + Context("Retrieving autogenerated service accounts when the spec.serviceAccount .generate value is used", func() { It("should provide a generated sa name", func() { Expect(resources.GetGeneratedServiceAccountName(buildRunSample)).To(Equal(buildRunSample.Name)) diff --git a/pkg/reconciler/buildrun/resources/sources.go b/pkg/reconciler/buildrun/resources/sources.go index e1d5572c35..415cbb593e 100644 --- a/pkg/reconciler/buildrun/resources/sources.go +++ b/pkg/reconciler/buildrun/resources/sources.go @@ -71,8 +71,10 @@ func AmendTaskSpecWithSources( sources.AppendBundleStep(cfg, taskSpec, build.Spec.Source.OCIArtifact, defaultSourceName) } case buildv1beta1.GitType: - appendSourceTimestampResult(taskSpec) - sources.AppendGitStep(cfg, taskSpec, *build.Spec.Source.GitSource, defaultSourceName) + if build.Spec.Source.GitSource != nil { + appendSourceTimestampResult(taskSpec) + sources.AppendGitStep(cfg, taskSpec, *build.Spec.Source.GitSource, defaultSourceName) + } } } } diff --git a/pkg/reconciler/buildrun/resources/sources/bundle.go b/pkg/reconciler/buildrun/resources/sources/bundle.go index 6c6e62d486..ec39f1ab69 100644 --- a/pkg/reconciler/buildrun/resources/sources/bundle.go +++ b/pkg/reconciler/buildrun/resources/sources/bundle.go @@ -75,6 +75,9 @@ func AppendBundleResult(buildRun *build.BuildRun, name string, results []pipelin imageDigest := FindResultValue(results, name, "image-digest") if strings.TrimSpace(imageDigest) != "" { + if buildRun.Status.Source == nil { + buildRun.Status.Source = &build.SourceResult{} + } buildRun.Status.Source.OciArtifact = &build.OciArtifactSourceResult{ Digest: imageDigest, } diff --git a/pkg/reconciler/buildrun/resources/sources/git.go b/pkg/reconciler/buildrun/resources/sources/git.go index 6eef3591d0..67c91b7ccc 100644 --- a/pkg/reconciler/buildrun/resources/sources/git.go +++ b/pkg/reconciler/buildrun/resources/sources/git.go @@ -11,6 +11,7 @@ import ( corev1 "k8s.io/api/core/v1" "github.com/shipwright-io/build/pkg/apis/build/v1beta1" + build "github.com/shipwright-io/build/pkg/apis/build/v1beta1" buildv1beta1 "github.com/shipwright-io/build/pkg/apis/build/v1beta1" "github.com/shipwright-io/build/pkg/config" @@ -115,6 +116,9 @@ func AppendGitResult(buildRun *buildv1beta1.BuildRun, name string, results []pip branchName := FindResultValue(results, name, branchName) if strings.TrimSpace(commitAuthor) != "" || strings.TrimSpace(commitSha) != "" || strings.TrimSpace(branchName) != "" { + if buildRun.Status.Source == nil { + buildRun.Status.Source = &build.SourceResult{} + } buildRun.Status.Source.Git = &v1beta1.GitSourceResult{ CommitAuthor: commitAuthor, CommitSha: commitSha, diff --git a/pkg/validate/ownerreferences.go b/pkg/validate/ownerreferences.go index ea94a4c0fb..27d38f2954 100644 --- a/pkg/validate/ownerreferences.go +++ b/pkg/validate/ownerreferences.go @@ -35,40 +35,34 @@ func (o OwnerRef) ValidatePath(ctx context.Context) error { return err } - if o.Build.Spec.Retention != nil && o.Build.Spec.Retention.AtBuildDeletion != nil { - switch *o.Build.Spec.Retention.AtBuildDeletion { - case true: - // if the buildRun does not have an ownerreference to the Build, lets add it. - for i := range buildRunList.Items { - buildRun := buildRunList.Items[i] + if o.Build.Spec.Retention != nil && o.Build.Spec.Retention.AtBuildDeletion != nil && *o.Build.Spec.Retention.AtBuildDeletion { + // if the buildRun does not have an ownerreference to the Build, lets add it. + for i := range buildRunList.Items { + buildRun := buildRunList.Items[i] - if index := o.validateBuildOwnerReference(buildRun.OwnerReferences); index == -1 { - if err := controllerutil.SetControllerReference(o.Build, &buildRun, o.Scheme); err != nil { - o.Build.Status.Reason = build.BuildReasonPtr(build.SetOwnerReferenceFailed) - o.Build.Status.Message = pointer.String(fmt.Sprintf("unexpected error when trying to set the ownerreference: %v", err)) - } - if err = o.Client.Update(ctx, &buildRun); err != nil { - return err - } - ctxlog.Info(ctx, fmt.Sprintf("successfully updated BuildRun %s", buildRun.Name), namespace, buildRun.Namespace, name, buildRun.Name) + if index := o.validateBuildOwnerReference(buildRun.OwnerReferences); index == -1 { + if err := controllerutil.SetControllerReference(o.Build, &buildRun, o.Scheme); err != nil { + o.Build.Status.Reason = build.BuildReasonPtr(build.SetOwnerReferenceFailed) + o.Build.Status.Message = pointer.String(fmt.Sprintf("unexpected error when trying to set the ownerreference: %v", err)) } + if err = o.Client.Update(ctx, &buildRun); err != nil { + return err + } + ctxlog.Info(ctx, fmt.Sprintf("successfully updated BuildRun %s", buildRun.Name), namespace, buildRun.Namespace, name, buildRun.Name) } - case false: - // if the buildRun have an ownerreference to the Build, lets remove it - for i := range buildRunList.Items { - buildRun := buildRunList.Items[i] + } + } else { + // if the buildRun have an ownerreference to the Build, lets remove it + for i := range buildRunList.Items { + buildRun := buildRunList.Items[i] - if index := o.validateBuildOwnerReference(buildRun.OwnerReferences); index != -1 { - buildRun.OwnerReferences = removeOwnerReferenceByIndex(buildRun.OwnerReferences, index) - if err := o.Client.Update(ctx, &buildRun); err != nil { - return err - } - ctxlog.Info(ctx, fmt.Sprintf("successfully updated BuildRun %s", buildRun.Name), namespace, buildRun.Namespace, name, buildRun.Name) + if index := o.validateBuildOwnerReference(buildRun.OwnerReferences); index != -1 { + buildRun.OwnerReferences = removeOwnerReferenceByIndex(buildRun.OwnerReferences, index) + if err := o.Client.Update(ctx, &buildRun); err != nil { + return err } + ctxlog.Info(ctx, fmt.Sprintf("successfully updated BuildRun %s", buildRun.Name), namespace, buildRun.Namespace, name, buildRun.Name) } - default: - ctxlog.Info(ctx, fmt.Sprintln("the build retention AtBuildDeletion was not properly defined"), namespace, o.Build.Namespace, name, o.Build.Name) - return fmt.Errorf("the build retention AtBuildDeletion was not properly defined") } } return nil diff --git a/pkg/validate/secrets.go b/pkg/validate/secrets.go index 26ed154f2e..0043049663 100644 --- a/pkg/validate/secrets.go +++ b/pkg/validate/secrets.go @@ -61,7 +61,7 @@ func (s Credentials) ValidatePath(ctx context.Context) error { func (s Credentials) buildCredentialserences() map[string]build.BuildReason { // Validate if the referenced secrets exist in the namespace secretRefMap := map[string]build.BuildReason{} - if s.Build.Spec.Output.PushSecret != nil && *s.Build.Spec.Output.PushSecret != "" { + if s.Build.Spec.Output.PushSecret != nil { secretRefMap[*s.Build.Spec.Output.PushSecret] = build.SpecOutputSecretRefNotFound } diff --git a/pkg/validate/sources.go b/pkg/validate/sources.go new file mode 100644 index 0000000000..f33e3fe750 --- /dev/null +++ b/pkg/validate/sources.go @@ -0,0 +1,60 @@ +// Copyright The Shipwright Contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package validate + +import ( + "context" + "fmt" + + build "github.com/shipwright-io/build/pkg/apis/build/v1beta1" +) + +// SourcesRef implements RuntimeRef interface to add validations for `build.spec.source`. +type SourceRef struct { + Build *build.Build // build instance for analysis +} + +// 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 + } + + return nil +} + +// validateSourceEntry inspect informed entry, probes all required attributes. +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 { + return nil + } + + switch source.Type { + case "Git": + if source.GitSource == nil { + return fmt.Errorf("type does not match the source") + } + case "OCI": + if source.OCIArtifact == nil { + return fmt.Errorf("type does not match the source") + } + case "Local": + if source.LocalSource == nil { + return fmt.Errorf("type does not match the source") + } + case "": + return fmt.Errorf("type definition is missing") + } + return nil +} + +// NewSourcesRef instantiate a new SourcesRef passing the build object pointer along. +func NewSourceRef(b *build.Build) *SourceRef { + return &SourceRef{Build: b} +} diff --git a/pkg/validate/sources_test.go b/pkg/validate/sources_test.go new file mode 100644 index 0000000000..d2914eb8d3 --- /dev/null +++ b/pkg/validate/sources_test.go @@ -0,0 +1,63 @@ +// Copyright The Shipwright Contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package validate_test + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + build "github.com/shipwright-io/build/pkg/apis/build/v1beta1" + "github.com/shipwright-io/build/pkg/validate" +) + +var _ = Describe("SourcesRef", func() { + Context("ValidatePath", func() { + It("should successfully validate an empty source", func() { + srcRef := validate.NewSourceRef(&build.Build{}) + + Expect(srcRef.ValidatePath(context.TODO())).To(BeNil()) + }) + + 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{}, + }, + }, + }) + + Expect(srcRef.ValidatePath(context.TODO())).To(BeNil()) + }) + + 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{}, + }, + }, + }) + + Expect(srcRef.ValidatePath(context.TODO())).To(HaveOccurred()) + }) + + 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{}, + }, + }, + }) + + Expect(srcRef.ValidatePath(context.TODO())).To(HaveOccurred()) + }) + }) +}) diff --git a/pkg/validate/sourceurl.go b/pkg/validate/sourceurl.go index 570defef69..888f889f1a 100644 --- a/pkg/validate/sourceurl.go +++ b/pkg/validate/sourceurl.go @@ -33,7 +33,7 @@ func NewSourceURL(client client.Client, build *build.Build) *SourceURLRef { 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 && gitSource.URL != "" { + if gitSource.CloneSecret == nil { switch s.Build.GetAnnotations()[build.AnnotationBuildVerifyRepository] { case "true": if err := git.ValidateGitURLExists(ctx, gitSource.URL); err != nil { diff --git a/pkg/validate/validate.go b/pkg/validate/validate.go index 169d835cc5..3e854c8f6c 100644 --- a/pkg/validate/validate.go +++ b/pkg/validate/validate.go @@ -23,7 +23,7 @@ const ( // SourceURL for validating the source URL in Build objects SourceURL = "sourceurl" // Sources for validating `spec.sources` entries - Sources = "sources" + Source = "source" // BuildName for validating `metadata.name` entry BuildName = "buildname" // Envs for validating `spec.env` entries @@ -63,6 +63,8 @@ func NewValidation( return &SourceURLRef{Build: build, Client: client}, nil case OwnerReferences: return &OwnerRef{Build: build, Client: client, Scheme: scheme}, nil + case Source: + return &SourceRef{Build: build}, nil case BuildName: return &BuildNameRef{Build: build}, nil case Envs: diff --git a/test/e2e/v1beta1/common_suite_test.go b/test/e2e/v1beta1/common_suite_test.go index 3c66eca37a..1be97295c3 100644 --- a/test/e2e/v1beta1/common_suite_test.go +++ b/test/e2e/v1beta1/common_suite_test.go @@ -84,6 +84,7 @@ func (b *buildPrototype) SourceGit(repository string) *buildPrototype { if b.build.Spec.Source.GitSource == nil { b.build.Spec.Source.GitSource = &buildv1beta1.Git{} } + b.build.Spec.Source.Type = buildv1beta1.GitType b.build.Spec.Source.GitSource.URL = repository b.build.Spec.Source.OCIArtifact = nil return b @@ -94,6 +95,7 @@ func (b *buildPrototype) SourceBundle(image string) *buildPrototype { b.build.Spec.Source.OCIArtifact = &buildv1beta1.OCIArtifact{} } b.build.Spec.Source.Type = buildv1beta1.OCIArtifactType + b.build.Spec.Source.Type = buildv1beta1.OCIArtifactType b.build.Spec.Source.OCIArtifact.Image = image return b } diff --git a/test/e2e/v1beta1/e2e_one_off_builds_test.go b/test/e2e/v1beta1/e2e_one_off_builds_test.go index e88e028ce7..fc85ca21c8 100644 --- a/test/e2e/v1beta1/e2e_one_off_builds_test.go +++ b/test/e2e/v1beta1/e2e_one_off_builds_test.go @@ -69,7 +69,6 @@ var _ = Describe("Using One-Off Builds", func() { ClusterBuildStrategy("buildpacks-v3"). Namespace(testBuild.Namespace). Name(testID). - SourceType("Git"). SourceGit("https://github.com/shipwright-io/sample-go.git"). SourceContextDir("source-build"). OutputImage(outputImage.String()). @@ -89,7 +88,6 @@ var _ = Describe("Using One-Off Builds", func() { ClusterBuildStrategy("buildah-shipwright-managed-push"). Namespace(testBuild.Namespace). Name(testID). - SourceType("Git"). SourceGit("https://github.com/shipwright-io/sample-go.git"). SourceContextDir("docker-build"). Dockerfile("Dockerfile"). diff --git a/test/integration/build_to_buildruns_test.go b/test/integration/build_to_buildruns_test.go index fa26a191b6..1ffa0fd20a 100644 --- a/test/integration/build_to_buildruns_test.go +++ b/test/integration/build_to_buildruns_test.go @@ -411,6 +411,42 @@ var _ = Describe("Integration tests Build and BuildRuns", func() { Expect(ownerReferenceNames(br.OwnerReferences)).ShouldNot(ContainElement(buildObject.Name)) }) + + It("does not deletes the buildrun if retention atBuildDeletion is removed", func() { + + Expect(tb.CreateBuild(buildObject)).To(BeNil()) + + buildObject, err = tb.GetBuildTillValidation(buildObject.Name) + Expect(err).To(BeNil()) + + autoDeleteBuildRun, err := tb.Catalog.LoadBRWithNameAndRef( + BUILDRUN+tb.Namespace, + BUILD+tb.Namespace, + []byte(test.MinimalBuildRun), + ) + Expect(err).To(BeNil()) + + Expect(tb.CreateBR(autoDeleteBuildRun)).To(BeNil()) + + _, err = tb.GetBRTillStartTime(autoDeleteBuildRun.Name) + Expect(err).To(BeNil()) + + buildObject.Spec.Retention = nil + err = tb.UpdateBuild(buildObject) + Expect(err).To(BeNil()) + + buildObject, err = tb.GetBuildTillValidation(buildObject.Name) + Expect(err).To(BeNil()) + + err = tb.DeleteBuild(BUILD + tb.Namespace) + Expect(err).To(BeNil()) + + br, err := tb.GetBRTillNotOwner(BUILDRUN+tb.Namespace, buildObject.Name) + Expect(err).To(BeNil()) + Expect(ownerReferenceNames(br.OwnerReferences)).ShouldNot(ContainElement(buildObject.Name)) + + }) + It("does delete the buildrun after several modifications of the annotation", func() { Expect(tb.CreateBuild(buildObject)).To(BeNil()) diff --git a/test/utils/v1beta1/builds.go b/test/utils/v1beta1/builds.go index e6e5704b4c..a3809b6c6d 100644 --- a/test/utils/v1beta1/builds.go +++ b/test/utils/v1beta1/builds.go @@ -27,6 +27,16 @@ func (t *TestBuild) CreateBuild(build *v1beta1.Build) error { return err } +// UpdateBR updates a BuildRun on the current test namespace +func (t *TestBuild) UpdateBuild(build *v1beta1.Build) error { + brInterface := t.BuildClientSet.ShipwrightV1beta1().Builds(t.Namespace) + _, err := brInterface.Update(context.TODO(), build, metav1.UpdateOptions{}) + if err != nil { + return err + } + return nil +} + // DeleteBuild deletes a Build on the desired namespace func (t *TestBuild) DeleteBuild(name string) error { bInterface := t.BuildClientSet.ShipwrightV1beta1().Builds(t.Namespace) diff --git a/test/v1beta1_samples/buildrun_samples.go b/test/v1beta1_samples/buildrun_samples.go index 0d8676400c..cb88c40e09 100644 --- a/test/v1beta1_samples/buildrun_samples.go +++ b/test/v1beta1_samples/buildrun_samples.go @@ -134,6 +134,7 @@ spec: build: spec: source: + type: Git git: url: "https://github.com/shipwright-io/sample-go" contextDir: docker-build