From 14a4a5eed60f42639f96896d158bb03e76662ceb 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 --- controllers/gitrepository_controller.go | 8 ++++ controllers/gitrepository_controller_test.go | 50 ++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index 8854e6227..a47207c19 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"), + "EmptyGitRepository", + ) + 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..dff5a4a64 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -220,6 +220,56 @@ 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", + }, + } + + builder := fakeclient.NewClientBuilder().WithScheme(testEnv.GetScheme()) + + r := &GitRepositoryReconciler{ + Client: builder.Build(), + EventRecorder: record.NewFakeRecorder(32), + Storage: testStorage, + patchOptions: getPatchOptions(gitRepositoryReadyCondition.Owned, "sc"), + } + + 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, t.TempDir()) + assertConditions := []metav1.Condition{ + *conditions.TrueCondition(sourcev1.FetchFailedCondition, "EmptyGitRepository", "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