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 c8fdc36
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 0 deletions.
12 changes: 12 additions & 0 deletions controllers/gitrepository_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ import (
"github.com/fluxcd/source-controller/internal/util"
)

// EmptyGitRepositoryReason signals that a referenced Git repository is empty
// (only has an initial branch without any commits).
const EmptyGitRepositoryReason string = "EmptyGitRepository"

// gitRepositoryReadyCondition contains the information required to summarize a
// v1beta2.GitRepository Ready Condition.
var gitRepositoryReadyCondition = summarize.Conditions{
Expand Down Expand Up @@ -537,6 +541,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"),
EmptyGitRepositoryReason,
)
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, EmptyGitRepositoryReason, "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 c8fdc36

Please sign in to comment.