Skip to content

Commit 9bdb882

Browse files
xiujuan95qu1queee
xiujuan95
authored and
qu1queee
committed
Add main logic, unit test and integration test for build controller validates spec.source.url
refine the main logic of build validates sourceURL return validate error and update error only return updateError when validate sourceURL failed rename verify sourceURL annotation and make verifySourceURL by default update test cases again revert verifySourceURL verify sourceURL by default add unit test for validateSourceURL return a same error when list remote repo failed output different errors and update tests Signed-off-by: Enrique Encalada <encalada@de.ibm.com>
1 parent 65bbdb3 commit 9bdb882

File tree

10 files changed

+626
-13
lines changed

10 files changed

+626
-13
lines changed

‎pkg/apis/build/v1alpha1/build_types.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ const (
2222
// AnnotationBuildRefSecret is an annotation that tells the Build Controller to reconcile on
2323
// events of the secret only if is referenced by a Build in the same namespace
2424
AnnotationBuildRefSecret = "build.build.dev/referenced.secret"
25+
26+
// AnnotationBuildVerifyRepository tells the Build Controller to check a remote repository. If the annotation is not set
27+
// or has a value of 'true', the controller triggers the validation. A value of 'false' means the controller
28+
// will bypass checking the remote repository.
29+
AnnotationBuildVerifyRepository = "build.build.dev/verify.repository"
2530
)
2631

2732
// BuildSpec defines the desired state of Build

‎pkg/controller/build/build_controller.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"github.com/shipwright-io/build/pkg/config"
3333
"github.com/shipwright-io/build/pkg/controller/utils"
3434
"github.com/shipwright-io/build/pkg/ctxlog"
35+
"github.com/shipwright-io/build/pkg/git"
3536
buildmetrics "github.com/shipwright-io/build/pkg/metrics"
3637
)
3738

@@ -240,6 +241,17 @@ func (r *ReconcileBuild) Reconcile(request reconcile.Request) (reconcile.Result,
240241
b.Status.Registered = corev1.ConditionFalse
241242
b.Status.Reason = succeedStatus
242243

244+
// Validate if remote repository exists
245+
if b.Spec.Source.SecretRef == nil {
246+
if err := r.validateSourceURL(ctx, b, b.Spec.Source.URL); err != nil {
247+
b.Status.Reason = err.Error()
248+
if updateErr := r.client.Status().Update(ctx, b); updateErr != nil {
249+
return reconcile.Result{}, updateErr
250+
}
251+
return reconcile.Result{}, nil
252+
}
253+
}
254+
243255
// Validate if the referenced secrets exist in the namespace
244256
var secretNames []string
245257
if b.Spec.Output.SecretRef != nil && b.Spec.Output.SecretRef.Name != "" {
@@ -459,6 +471,21 @@ func (r *ReconcileBuild) validateBuildRunOwnerReferences(ctx context.Context, b
459471
return nil
460472
}
461473

474+
// validateSourceURL returns error message if remote repository doesn't exist
475+
func (r *ReconcileBuild) validateSourceURL(ctx context.Context, b *build.Build, sourceURL string) error {
476+
switch b.GetAnnotations()[build.AnnotationBuildVerifyRepository] {
477+
case "", "true":
478+
return git.ValidateGitURLExists(sourceURL)
479+
case "false":
480+
ctxlog.Info(ctx, fmt.Sprintf("the annotation %s is set to %s, nothing to do", build.AnnotationBuildVerifyRepository, b.GetAnnotations()[build.AnnotationBuildVerifyRepository]))
481+
return nil
482+
default:
483+
var annoErr = fmt.Errorf("the annotation %s was not properly defined, supported values are true or false", build.AnnotationBuildVerifyRepository)
484+
ctxlog.Error(ctx, annoErr, namespace, b.Namespace, name, b.Name)
485+
return annoErr
486+
}
487+
}
488+
462489
// validateOwnerReferences returns an index value if a Build is owning a reference or -1 if this is not the case
463490
func (r *ReconcileBuild) validateBuildOwnerReference(references []metav1.OwnerReference, build *build.Build) int {
464491
for i, ownerRef := range references {

‎pkg/controller/build/build_controller_test.go‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -511,5 +511,115 @@ var _ = Describe("Reconcile Build", func() {
511511
Expect(reconcile.Result{}).To(Equal(result))
512512
})
513513
})
514+
515+
Context("when source URL is specified", func() {
516+
// validate file protocol
517+
It("fails when source URL is invalid", func() {
518+
buildSample.Spec.Source.URL = "foobar"
519+
520+
// Fake some client LIST calls and ensure we populate all
521+
// different resources we could get during reconciliation
522+
client.ListCalls(func(context context.Context, object runtime.Object, _ ...crc.ListOption) error {
523+
switch object := object.(type) {
524+
case *corev1.SecretList:
525+
list := ctl.FakeSecretList()
526+
list.DeepCopyInto(object)
527+
case *build.ClusterBuildStrategyList:
528+
list := ctl.ClusterBuildStrategyList(buildStrategyName)
529+
list.DeepCopyInto(object)
530+
}
531+
return nil
532+
})
533+
534+
statusCall := ctl.StubFunc(corev1.ConditionFalse, "invalid source url")
535+
statusWriter.UpdateCalls(statusCall)
536+
537+
_, err := reconciler.Reconcile(request)
538+
Expect(err).To(BeNil())
539+
Expect(statusWriter.UpdateCallCount()).To(Equal(1))
540+
})
541+
542+
// validate https protocol
543+
It("fails when public source URL is unreachable", func() {
544+
buildSample.Spec.Source.URL = "https://github.com/sbose78/taxi-fake"
545+
546+
// Fake some client LIST calls and ensure we populate all
547+
// different resources we could get during reconciliation
548+
client.ListCalls(func(context context.Context, object runtime.Object, _ ...crc.ListOption) error {
549+
switch object := object.(type) {
550+
case *corev1.SecretList:
551+
list := ctl.FakeSecretList()
552+
list.DeepCopyInto(object)
553+
case *build.ClusterBuildStrategyList:
554+
list := ctl.ClusterBuildStrategyList(buildStrategyName)
555+
list.DeepCopyInto(object)
556+
}
557+
return nil
558+
})
559+
560+
statusCall := ctl.StubFunc(corev1.ConditionFalse, "remote repository unreachable")
561+
statusWriter.UpdateCalls(statusCall)
562+
563+
_, err := reconciler.Reconcile(request)
564+
Expect(err).To(BeNil())
565+
Expect(statusWriter.UpdateCallCount()).To(Equal(1))
566+
})
567+
568+
// skip validation because of false sourceURL annotation
569+
It("succeed when source URL is invalid because source annotation is false", func() {
570+
buildSample = ctl.BuildWithClusterBuildStrategyAndFalseSourceAnnotation(buildName, namespace, buildStrategyName)
571+
572+
// Fake some client LIST calls and ensure we populate all
573+
// different resources we could get during reconciliation
574+
client.ListCalls(func(context context.Context, object runtime.Object, _ ...crc.ListOption) error {
575+
switch object := object.(type) {
576+
case *corev1.SecretList:
577+
list := ctl.FakeSecretList()
578+
list.DeepCopyInto(object)
579+
case *build.ClusterBuildStrategyList:
580+
list := ctl.ClusterBuildStrategyList(buildStrategyName)
581+
list.DeepCopyInto(object)
582+
}
583+
return nil
584+
})
585+
586+
statusCall := ctl.StubFunc(corev1.ConditionTrue, "Succeeded")
587+
statusWriter.UpdateCalls(statusCall)
588+
589+
result, err := reconciler.Reconcile(request)
590+
Expect(err).ToNot(HaveOccurred())
591+
Expect(statusWriter.UpdateCallCount()).To(Equal(1))
592+
Expect(reconcile.Result{}).To(Equal(result))
593+
})
594+
595+
// skip validation because build references a sourceURL secret
596+
It("succeed when source URL is fake private URL because build reference a sourceURL secret", func() {
597+
buildSample := ctl.BuildWithClusterBuildStrategyAndSourceSecret(buildName, namespace, buildStrategyName)
598+
buildSample.Spec.Source.URL = "https://github.yourco.com/org/build-fake"
599+
buildSample.Spec.Source.SecretRef.Name = registrySecret
600+
601+
// Fake some client LIST calls and ensure we populate all
602+
// different resources we could get during reconciliation
603+
client.ListCalls(func(context context.Context, object runtime.Object, _ ...crc.ListOption) error {
604+
switch object := object.(type) {
605+
case *corev1.SecretList:
606+
list := ctl.SecretList(registrySecret)
607+
list.DeepCopyInto(object)
608+
case *build.ClusterBuildStrategyList:
609+
list := ctl.ClusterBuildStrategyList(buildStrategyName)
610+
list.DeepCopyInto(object)
611+
}
612+
return nil
613+
})
614+
615+
statusCall := ctl.StubFunc(corev1.ConditionTrue, "Succeeded")
616+
statusWriter.UpdateCalls(statusCall)
617+
618+
result, err := reconciler.Reconcile(request)
619+
Expect(err).ToNot(HaveOccurred())
620+
Expect(statusWriter.UpdateCallCount()).To(Equal(1))
621+
Expect(reconcile.Result{}).To(Equal(result))
622+
})
623+
})
514624
})
515625
})

‎pkg/git/git.go‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// Copyright The Shipwright Contributors
2+
//
3+
// SPDX-License-Identifier: Apache-2.0
4+
5+
package git
6+
7+
import (
8+
"fmt"
9+
10+
"github.com/go-git/go-git/v5/config"
11+
"github.com/go-git/go-git/v5/plumbing/transport"
12+
"github.com/go-git/go-git/v5/storage/memory"
13+
14+
gogitv5 "github.com/go-git/go-git/v5"
15+
)
16+
17+
const (
18+
defaultRemote = "origin"
19+
httpsProtocol = "https"
20+
httpProtocol = "http"
21+
fileProtocol = "file"
22+
gitProtocol = "ssh"
23+
)
24+
25+
// ValidateGitURLExists validate if a source URL exists or not
26+
// Note: We have an upcoming PR for the Build Status, where we
27+
// intend to define a single Status.Reason in the form of 'remoteRepositoryUnreachable',
28+
// where the Status.Message will contain the longer text, like 'invalid source url
29+
func ValidateGitURLExists(urlPath string) error {
30+
31+
endpoint, err := transport.NewEndpoint(urlPath)
32+
if err != nil {
33+
return err
34+
}
35+
36+
switch endpoint.Protocol {
37+
case httpsProtocol, httpProtocol:
38+
repo := gogitv5.NewRemote(memory.NewStorage(), &config.RemoteConfig{
39+
Name: defaultRemote,
40+
URLs: []string{urlPath},
41+
})
42+
// Note: When the urlPath is an valid public path, however, this path doesn't exist, func will return `authentication required`, this maybe mislead
43+
// So convert this error message to `repository not found`
44+
_, err := repo.List(&gogitv5.ListOptions{})
45+
if err != nil && err == transport.ErrAuthenticationRequired {
46+
return fmt.Errorf("remote repository unreachable")
47+
} else if err != nil {
48+
return err
49+
}
50+
case fileProtocol:
51+
return fmt.Errorf("invalid source url")
52+
case gitProtocol:
53+
return fmt.Errorf("the source url requires authentication")
54+
default:
55+
return nil
56+
}
57+
return nil
58+
}

‎pkg/git/git_suite_test.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright The Shipwright Contributors
2+
//
3+
// SPDX-License-Identifier: Apache-2.0
4+
5+
package git_test
6+
7+
import (
8+
"testing"
9+
10+
. "github.com/onsi/ginkgo"
11+
. "github.com/onsi/gomega"
12+
)
13+
14+
func TestGit(t *testing.T) {
15+
RegisterFailHandler(Fail)
16+
RunSpecs(t, "Git Suite")
17+
}

‎pkg/git/git_test.go‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Copyright The Shipwright Contributors
2+
//
3+
// SPDX-License-Identifier: Apache-2.0
4+
5+
package git_test
6+
7+
import (
8+
"errors"
9+
10+
. "github.com/onsi/ginkgo"
11+
. "github.com/onsi/ginkgo/extensions/table"
12+
. "github.com/onsi/gomega"
13+
"github.com/onsi/gomega/types"
14+
"github.com/shipwright-io/build/pkg/git"
15+
)
16+
17+
var _ = Describe("Git", func() {
18+
19+
DescribeTable("the source url validation errors",
20+
func(url string, expected types.GomegaMatcher) {
21+
Expect(git.ValidateGitURLExists(url)).To(expected)
22+
},
23+
Entry("Check remote https public repository", "https://github.com/shipwright-io/build", BeNil()),
24+
Entry("Check remote fake https public repository", "https://github.com/shipwright-io/build-fake", Equal(errors.New("remote repository unreachable"))),
25+
Entry("Check invalid repository", "foobar", Equal(errors.New("invalid source url"))),
26+
Entry("Check git repository which requires authentication", "git@github.com:shipwright-io/build-fake.git", Equal(errors.New("the source url requires authentication"))),
27+
Entry("Check ssh repository which requires authentication", "ssh://github.com/shipwright-io/build-fake", Equal(errors.New("the source url requires authentication"))),
28+
)
29+
})

‎test/build_samples.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,33 @@ spec:
214214
image: image-registry.openshift-image-registry.svc:5000/example/buildpacks-app
215215
`
216216

217+
// BuildCBSWithVerifyRepositoryAnnotation defines a Build
218+
// with the verify repository annotation key
219+
const BuildCBSWithVerifyRepositoryAnnotation = `
220+
apiVersion: build.dev/v1alpha1
221+
kind: Build
222+
metadata:
223+
annotations:
224+
build.build.dev/verify.repository: ""
225+
spec:
226+
strategy:
227+
kind: ClusterBuildStrategy
228+
output:
229+
image: image-registry.openshift-image-registry.svc:5000/example/buildpacks-app
230+
`
231+
232+
// BuildCBSWithoutVerifyRepositoryAnnotation defines a minimal
233+
// Build without source url and annotation
234+
const BuildCBSWithoutVerifyRepositoryAnnotation = `
235+
apiVersion: build.dev/v1alpha1
236+
kind: Build
237+
spec:
238+
strategy:
239+
kind: ClusterBuildStrategy
240+
output:
241+
image: image-registry.openshift-image-registry.svc:5000/example/buildpacks-app
242+
`
243+
217244
// BuildCBSWithBuildRunDeletion defines a Build with a
218245
// ClusterBuildStrategy and the annotation for automatic BuildRun
219246
// deletion

0 commit comments

Comments
 (0)