Skip to content

Commit

Permalink
Prevent panic when cloning empty git repository
Browse files Browse the repository at this point in the history
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 <aurelcanciu@gmail.com>
  • Loading branch information
relu committed Feb 7, 2023
1 parent ae3a81e commit 14a4a5e
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 0 deletions.
8 changes: 8 additions & 0 deletions controllers/gitrepository_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
50 changes: 50 additions & 0 deletions controllers/gitrepository_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 14a4a5e

Please sign in to comment.