From b054fbdbb6dde81c62f7d4e6fd784ef279a927e0 Mon Sep 17 00:00:00 2001 From: Aurel Canciu Date: Mon, 6 Feb 2023 17:14:52 +0100 Subject: [PATCH] Prevent panic when cloning empty git repository This covers the edge case in which a user creates a GitRepository CR referencing an empty Git repository. Currently, the controller will panic in this situation since the returned commit pointer is nil. Signed-off-by: Aurel Canciu --- api/v1beta2/gitrepository_types.go | 4 ++ controllers/gitrepository_controller.go | 8 +++ controllers/gitrepository_controller_test.go | 59 ++++++++++++++++++++ 3 files changed, 71 insertions(+) diff --git a/api/v1beta2/gitrepository_types.go b/api/v1beta2/gitrepository_types.go index f85191e87..552e578da 100644 --- a/api/v1beta2/gitrepository_types.go +++ b/api/v1beta2/gitrepository_types.go @@ -258,6 +258,10 @@ const ( // GitOperationFailedReason signals that a Git operation (e.g. clone, // checkout, etc.) failed. GitOperationFailedReason string = "GitOperationFailed" + + // GitEmptyRepositoryReason signals that a referenced Git repository is empty + // (only has an initial branch without any commits). + GitEmptyRepositoryReason string = "GitEmptyRepository" ) // GetConditions returns the status conditions of the object. diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index 8854e6227..718b31065 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -537,6 +537,14 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, sp *patch if err != nil { return sreconcile.ResultEmpty, err } + if c == nil { + e := serror.NewGeneric( + fmt.Errorf("git repository is empty"), + sourcev1.GitEmptyRepositoryReason, + ) + conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) + return sreconcile.ResultEmpty, e + } // Assign the commit to the shared commit reference. *commit = *c diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index 0db3b856a..d63d2f2cb 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -220,6 +220,65 @@ func TestGitRepositoryReconciler_Reconcile(t *testing.T) { testSuspendedObjectDeleteWithArtifact(ctx, g, obj) } +func TestGitRepositoryReconciler_reconcileSource_emptyRepository(t *testing.T) { + g := NewWithT(t) + + server, err := gittestserver.NewTempGitServer() + g.Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(server.Root()) + server.AutoCreate() + g.Expect(server.StartHTTP()).To(Succeed()) + defer server.StopHTTP() + + obj := &sourcev1.GitRepository{ + ObjectMeta: metav1.ObjectMeta{ + GenerateName: "empty-", + Generation: 1, + }, + Spec: sourcev1.GitRepositorySpec{ + Interval: metav1.Duration{Duration: interval}, + Timeout: &metav1.Duration{Duration: timeout}, + URL: server.HTTPAddress() + "/test.git", + }, + } + + fs := memfs.New() + localRepo, err := gogit.Init(memory.NewStorage(), fs) + g.Expect(err).NotTo(HaveOccurred()) + + builder := fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()) + + r := &GitRepositoryReconciler{ + Client: builder.Build(), + EventRecorder: record.NewFakeRecorder(32), + Storage: testStorage, + patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), + } + + tmpDir := t.TempDir() + + head, _ := localRepo.Head() + g.Expect(head).To(BeNil()) + + g.Expect(r.Client.Create(context.TODO(), obj)).ToNot(HaveOccurred()) + defer func() { + g.Expect(r.Client.Delete(context.TODO(), obj)).ToNot(HaveOccurred()) + }() + + var commit git.Commit + var includes artifactSet + sp := patch.NewSerialPatcher(obj, r.Client) + + got, err := r.reconcileSource(context.TODO(), sp, obj, &commit, &includes, tmpDir) + assertConditions := []metav1.Condition{ + *conditions.TrueCondition(sourcev1.FetchFailedCondition, sourcev1.GitEmptyRepositoryReason, "git repository is empty"), + } + g.Expect(obj.Status.Conditions).To(conditions.MatchConditions(assertConditions)) + g.Expect(err).To(HaveOccurred()) + g.Expect(got).To(Equal(sreconcile.ResultEmpty)) + g.Expect(commit).ToNot(BeNil()) +} + func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) { type options struct { username string