From cbca249bfe6b207230f106593f164a4bb583d862 Mon Sep 17 00:00:00 2001 From: Xiu Juan Xiang Date: Wed, 17 Mar 2021 14:48:42 +0800 Subject: [PATCH] git validation is disabled by default --- docs/build.md | 6 ++--- pkg/reconciler/build/build_test.go | 32 ++++++++++++++++++++++++++- pkg/validate/sourceurl.go | 4 ++-- test/build_samples.go | 3 +++ test/integration/build_to_git_test.go | 22 +++++++++--------- 5 files changed, 51 insertions(+), 16 deletions(-) diff --git a/docs/build.md b/docs/build.md index 1a9cb493f..65fcc29d0 100644 --- a/docs/build.md +++ b/docs/build.md @@ -86,9 +86,9 @@ A `Build` resource can specify a Git source, together with other parameters like - `source.revision` - An specific revision to select from the source repository, this can be a commit or branch name. If not defined, it will fallback to the git repository default branch. - `source.contextDir` - For repositories where the source code is not located at the root folder, you can specify this path here. Currently, only supported by `buildah`, `kaniko` and `buildpacks` build strategies. -By default, the Build controller will validate that the Git repository exists. If the validation is not desired, users can define the `build.shipwright.io/verify.repository` annotation with `false`. For example: +By default, the Build controller won't validate that the Git repository exists. If the validation is desired, users can define the `build.shipwright.io/verify.repository` annotation with `true` explicitly. For example: -Example of a `Build` with the **build.shipwright.io/verify.repository** annotation, in order to disable the `spec.source.url` validation. +Example of a `Build` with the **build.shipwright.io/verify.repository** annotation, in order to enable the `spec.source.url` validation. ```yaml apiVersion: shipwright.io/v1alpha1 @@ -96,7 +96,7 @@ kind: Build metadata: name: buildah-golang-build annotations: - build.shipwright.io/verify.repository: "false" + build.shipwright.io/verify.repository: "true" spec: source: url: https://github.com/shipwright-io/sample-go diff --git a/pkg/reconciler/build/build_test.go b/pkg/reconciler/build/build_test.go index 16893f5ff..2ca1113a2 100644 --- a/pkg/reconciler/build/build_test.go +++ b/pkg/reconciler/build/build_test.go @@ -373,7 +373,9 @@ var _ = Describe("Reconcile Build", func() { // validate file protocol It("fails when source URL is invalid", func() { buildSample.Spec.Source.URL = "foobar" - + buildSample.SetAnnotations(map[string]string{ + build.AnnotationBuildVerifyRepository: "true", + }) statusCall := ctl.StubFunc(corev1.ConditionFalse, build.RemoteRepositoryUnreachable, "invalid source url") statusWriter.UpdateCalls(statusCall) @@ -385,6 +387,9 @@ var _ = Describe("Reconcile Build", func() { // validate https protocol It("fails when public source URL is unreachable", func() { buildSample.Spec.Source.URL = "https://github.com/shipwright-io/sample-go-fake" + buildSample.SetAnnotations(map[string]string{ + build.AnnotationBuildVerifyRepository: "true", + }) statusCall := ctl.StubFunc(corev1.ConditionFalse, build.RemoteRepositoryUnreachable, "remote repository unreachable") statusWriter.UpdateCalls(statusCall) @@ -394,6 +399,31 @@ var _ = Describe("Reconcile Build", func() { Expect(statusWriter.UpdateCallCount()).To(Equal(1)) }) + // skip validation because of empty sourceURL annotation + It("succeed when source URL is invalid because source annotation is empty", func() { + buildSample.Spec.Source.URL = "foobar" + + // Fake some client Get calls and ensure we populate all + // different resources we could get during reconciliation + client.GetCalls(func(context context.Context, nn types.NamespacedName, object runtime.Object) error { + switch object := object.(type) { + case *build.Build: + buildSample.DeepCopyInto(object) + case *build.ClusterBuildStrategy: + clusterBuildStrategySample.DeepCopyInto(object) + } + return nil + }) + + statusCall := ctl.StubFunc(corev1.ConditionTrue, build.SucceedStatus, build.AllValidationsSucceeded) + statusWriter.UpdateCalls(statusCall) + + result, err := reconciler.Reconcile(request) + Expect(err).ToNot(HaveOccurred()) + Expect(statusWriter.UpdateCallCount()).To(Equal(1)) + Expect(reconcile.Result{}).To(Equal(result)) + }) + // skip validation because of false sourceURL annotation It("succeed when source URL is invalid because source annotation is false", func() { buildSample = ctl.BuildWithClusterBuildStrategyAndFalseSourceAnnotation(buildName, namespace, buildStrategyName) diff --git a/pkg/validate/sourceurl.go b/pkg/validate/sourceurl.go index c651f8372..2335f7051 100644 --- a/pkg/validate/sourceurl.go +++ b/pkg/validate/sourceurl.go @@ -27,13 +27,13 @@ type SourceURLRef struct { func (s SourceURLRef) ValidatePath(ctx context.Context) error { if s.Build.Spec.Source.SecretRef == nil { switch s.Build.GetAnnotations()[build.AnnotationBuildVerifyRepository] { - case "", "true": + case "true": err := git.ValidateGitURLExists(s.Build.Spec.Source.URL) if err != nil { s.MarkBuildStatus(s.Build, build.RemoteRepositoryUnreachable, err.Error()) } return err - case "false": + case "", "false": ctxlog.Info(ctx, fmt.Sprintf("the annotation %s is set to %s, nothing to do", build.AnnotationBuildVerifyRepository, s.Build.GetAnnotations()[build.AnnotationBuildVerifyRepository])) return nil default: diff --git a/test/build_samples.go b/test/build_samples.go index b86cdf8c5..080464bea 100644 --- a/test/build_samples.go +++ b/test/build_samples.go @@ -218,6 +218,9 @@ spec: const BuildCBSWithWrongURL = ` apiVersion: shipwright.io/v1alpha1 kind: Build +metadata: + annotations: + build.shipwright.io/verify.repository: "true" spec: source: url: "https://github.foobar.com/sbose78/taxi" diff --git a/test/integration/build_to_git_test.go b/test/integration/build_to_git_test.go index 28074911d..fca57f582 100644 --- a/test/integration/build_to_git_test.go +++ b/test/integration/build_to_git_test.go @@ -60,7 +60,7 @@ var _ = Describe("Integration tests Build and referenced Source url", func() { }) Context("when the build source url protocol is a fake http without the verify annotation", func() { - It("should validate source url by default", func() { + It("should not validate source url by default", func() { // populate Build related vars buildName := BUILD + tb.Namespace @@ -75,11 +75,12 @@ var _ = Describe("Integration tests Build and referenced Source url", func() { Expect(tb.CreateBuild(buildObject)).To(BeNil()) // wait until the Build finish the validation - buildObject, err := tb.GetBuildTillRegistration(buildName, corev1.ConditionFalse) + buildObject, err := tb.GetBuildTillRegistration(buildName, corev1.ConditionTrue) Expect(err).To(BeNil()) - Expect(buildObject.Status.Registered).To(Equal(corev1.ConditionFalse)) - Expect(buildObject.Status.Reason).To(Equal(v1alpha1.RemoteRepositoryUnreachable)) - Expect(buildObject.Status.Message).To(Equal("remote repository unreachable")) + // skip validation due to empty annotation + Expect(buildObject.Status.Registered).To(Equal(corev1.ConditionTrue)) + Expect(buildObject.Status.Reason).To(Equal(v1alpha1.SucceedStatus)) + Expect(buildObject.Status.Message).To(Equal(v1alpha1.AllValidationsSucceeded)) }) }) @@ -110,7 +111,7 @@ var _ = Describe("Integration tests Build and referenced Source url", func() { }) Context("when the build source url protocol is a fake https without the verify annotation", func() { - It("should validate source url by default", func() { + It("should not validate source url by default", func() { // populate Build related vars buildName := BUILD + tb.Namespace @@ -125,11 +126,12 @@ var _ = Describe("Integration tests Build and referenced Source url", func() { Expect(tb.CreateBuild(buildObject)).To(BeNil()) // wait until the Build finish the validation - buildObject, err := tb.GetBuildTillRegistration(buildName, corev1.ConditionFalse) + buildObject, err := tb.GetBuildTillRegistration(buildName, corev1.ConditionTrue) Expect(err).To(BeNil()) - Expect(buildObject.Status.Registered).To(Equal(corev1.ConditionFalse)) - Expect(buildObject.Status.Reason).To(Equal(v1alpha1.RemoteRepositoryUnreachable)) - Expect(buildObject.Status.Message).To(Equal("remote repository unreachable")) + // skip validation due to empty annotation + Expect(buildObject.Status.Registered).To(Equal(corev1.ConditionTrue)) + Expect(buildObject.Status.Reason).To(Equal(v1alpha1.SucceedStatus)) + Expect(buildObject.Status.Message).To(Equal(v1alpha1.AllValidationsSucceeded)) }) })