git validation is disabled by default

docs/build.md

@@ -86,17 +86,17 @@ 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
 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

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)

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:

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"

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))
 		})
 	})