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 6, 2023
1 parent ae3a81e commit b054fbd
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 0 deletions.
4 changes: 4 additions & 0 deletions api/v1beta2/gitrepository_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
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"),
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

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

0 comments on commit b054fbd

Please sign in to comment.