Merge pull request #92 from github/override-bad-date-check

Skip 'bad date' check for some imports

Author: spraints

Makefile

@@ -90,6 +90,7 @@ go-test-integration:
 
 	# Add our compiled `spokes-receive-pack` to the PATH while running tests:
 	PATH="$(CURDIR)/bin:$(PATH)" \
+	    GIT_CONFIG_SYSTEM="$(CURDIR)/internal/integration/testdata/gitconfig" \
 	    $(GO) test $(TESTINTEGRATIONFLAGS) $(TESTSUITE) 2>&1
 
 	@echo "$(M) disabling failpoints ..."

internal/integration/integration_test.go

@@ -29,6 +29,14 @@ committer Spokes Receive Pack <spokes@receive.pack> 1234567890 +0000
 This commit object intentionally broken
 `
 
+var suiteDir string = func() string {
+	pwd, err := os.Getwd()
+	if err != nil {
+		panic(err)
+	}
+	return pwd
+}()
+
 type SpokesReceivePackTestSuite struct {
 	suite.Suite
 	localRepo, remoteRepo string
@@ -181,6 +189,28 @@ func (suite *SpokesReceivePackTestSuite) TestSpokesReceivePackHiddenRefs() {
 		"should partially fail")
 }
 
+func (suite *SpokesReceivePackTestSuite) TestBadDate() {
+	cmd := exec.Command("git", "-C", filepath.Join(suiteDir, "testdata/bad-date/sha1.git"),
+		"push", "--receive-pack=spokes-receive-pack-wrapper", suite.remoteRepo, "main")
+	out, err := cmd.CombinedOutput()
+	suite.T().Logf("$ %s\n%s", strings.Join(cmd.Args, " "), out)
+	assert.Error(suite.T(), err, "expect an error a repo with a malformed committer line in a commit")
+	assert.Contains(suite.T(), string(out), " badDate:", "should complain about a bad date")
+}
+
+func (suite *SpokesReceivePackTestSuite) TestBadDateAllowedWithOverride() {
+	cmd := exec.Command("git", "-C", filepath.Join(suiteDir, "testdata/bad-date/sha1.git"),
+		"push", "--receive-pack=spokes-receive-pack-wrapper", suite.remoteRepo, "main")
+	cmd.Env = append(os.Environ(),
+		"GIT_SOCKSTAT_VAR_is_importing=bool:true",
+		"GIT_SOCKSTAT_VAR_allow_baddate_in_import=bool:true",
+	)
+	out, err := cmd.CombinedOutput()
+	suite.T().Logf("$ %s\n%s", strings.Join(cmd.Args, " "), out)
+	assert.NoError(suite.T(), err, "expect the push to succeed")
+	assert.Contains(suite.T(), string(out), " badDate:", "should still complain about a bad date")
+}
+
 func (suite *SpokesReceivePackTestSuite) TestWithGovernor() {
 	started := make(chan any)
 	govSock, msgs, cleanup := startFakeGovernor(suite.T(), started, nil)

internal/integration/missingobjects_test.go

@@ -13,37 +13,136 @@ import (
 	"testing"
 	"time"
 
+	"github.com/github/spokes-receive-pack/internal/objectformat"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
 func TestMissingObjects(t *testing.T) {
-	var info struct {
+
+	x := setUpMissingObjectsTestRepo(t)
+	testRepo := x.TestRepo
+	info := x.Info
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+	defer cancel()
+
+	srp := startSpokesReceivePack(ctx, t, testRepo)
+
+	refs, _, err := readAdv(srp.Out)
+	require.NoError(t, err)
+	assert.Equal(t, refs, map[string]string{
+		info.Ref:    info.OldOID,
+		info.DelRef: info.OldOID,
+	})
+
+	// Send the pack that's missing a commit.
+	pack, err := os.Open("testdata/missing-objects/bad.pack")
+	require.NoError(t, err)
+	defer pack.Close()
+
+	writePushData(
+		t, srp,
+		[]refUpdate{
+			// Try to update the ref that's already there to commit C (but we won't
+			// push its parent and the remote doesn't have the parent either).
+			{info.OldOID, info.NewOID, info.Ref},
+		},
+		pack,
+	)
+
+	refStatus, unpackRes, _, err := readResult(t, srp.Out)
+	require.NoError(t, err)
+	assert.Equal(t, map[string]string{
+		info.Ref: "ng error processing packfiles: exit status 128",
+	}, refStatus)
+	assert.Equal(t, "unpack index-pack failed\n", unpackRes)
+}
+
+func TestDeleteAndUpdate(t *testing.T) {
+	const refToCreate = "refs/heads/new-branch"
+
+	x := setUpMissingObjectsTestRepo(t)
+	testRepo := x.TestRepo
+	info := x.Info
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
+	defer cancel()
+
+	srp := startSpokesReceivePack(ctx, t, testRepo)
+
+	refs, _, err := readAdv(srp.Out)
+	require.NoError(t, err)
+	assert.Equal(t, refs, map[string]string{
+		info.Ref:    info.OldOID,
+		info.DelRef: info.OldOID,
+	})
+
+	// Send the pack that's missing a commit.
+	pack, err := os.Open("testdata/missing-objects/empty.pack")
+	require.NoError(t, err)
+	defer pack.Close()
+
+	writePushData(
+		t, srp,
+		[]refUpdate{
+			// Try to create another ref with a commit that the remote already has.
+			{objectformat.NullOIDSHA1, info.OldOID, refToCreate},
+			// Try to delete a ref.
+			{info.OldOID, objectformat.NullOIDSHA1, info.DelRef},
+		},
+		pack,
+	)
+
+	refStatus, unpackRes, _, err := readResult(t, srp.Out)
+	require.NoError(t, err)
+	assert.Equal(t, map[string]string{
+		refToCreate: "ok",
+		info.DelRef: "ok",
+	}, refStatus)
+	assert.Equal(t, "unpack ok\n", unpackRes)
+}
+
+type missingObjectsTestInfo struct {
+	TestRepo string
+	Info     struct {
 		OldOID string `json:"push_from"`
 		NewOID string `json:"push_to"`
 		Ref    string `json:"ref"`
 		DelRef string `json:"extra_ref"`
 	}
+}
+
+func setUpMissingObjectsTestRepo(t *testing.T) missingObjectsTestInfo {
 	const (
-		remote      = "testdata/missing-objects/remote.git"
-		badPack     = "testdata/missing-objects/bad.pack"
-		infoFile    = "testdata/missing-objects/info.json"
-		refToCreate = "refs/heads/new-branch"
+		remote   = "testdata/missing-objects/remote.git"
+		badPack  = "testdata/missing-objects/bad.pack"
+		infoFile = "testdata/missing-objects/info.json"
 	)
 
+	var res missingObjectsTestInfo
+
 	infoJSON, err := os.ReadFile(infoFile)
 	require.NoError(t, err)
-	require.NoError(t, json.Unmarshal(infoJSON, &info))
+	require.NoError(t, json.Unmarshal(infoJSON, &res.Info))
 
 	origin, err := filepath.Abs(remote)
 	require.NoError(t, err)
 
-	testRepo := t.TempDir()
-	requireRun(t, "git", "clone", "--mirror", origin, testRepo)
+	res.TestRepo = t.TempDir()
+	requireRun(t, "git", "clone", "--mirror", origin, res.TestRepo)
 
-	ctx, cancel := context.WithTimeout(context.Background(), time.Second)
-	defer cancel()
+	return res
+}
 
+type spokesReceivePackProcess struct {
+	Cmd *exec.Cmd
+	In  io.WriteCloser
+	Out io.Reader
+	Err chan error
+}
+
+func startSpokesReceivePack(ctx context.Context, t *testing.T, testRepo string) spokesReceivePackProcess {
 	srp := exec.CommandContext(ctx, "spokes-receive-pack", ".")
 	srp.Dir = testRepo
 	srp.Env = append(os.Environ(),
@@ -59,47 +158,34 @@ func TestMissingObjects(t *testing.T) {
 
 	bufSRPOut := bufio.NewReader(srpOut)
 
-	refs, _, err := readAdv(bufSRPOut)
-	require.NoError(t, err)
-	assert.Equal(t, refs, map[string]string{
-		info.Ref:    info.OldOID,
-		info.DelRef: info.OldOID,
-	})
-
-	// Try to update the ref that's already there to commit C (but we won't
-	// push its parent and the remote doesn't have the parent either).
-	require.NoError(t, writePktlinef(srpIn,
-		"%s %s %s\x00report-status report-status-v2 side-band-64k object-format=sha1\n",
-		info.OldOID, info.NewOID, info.Ref))
+	return spokesReceivePackProcess{
+		Cmd: srp,
+		In:  srpIn,
+		Out: bufSRPOut,
+		Err: srpErr,
+	}
+}
 
-	// Try to create another ref with a commit that the remote already has.
-	require.NoError(t, writePktlinef(srpIn,
-		"%040d %s %s",
-		0, info.OldOID, refToCreate))
+type refUpdate struct {
+	OldOID, NewOID, Ref string
+}
 
-	// Try to delete another ref.
-	require.NoError(t, writePktlinef(srpIn,
-		"%s %040d %s",
-		info.OldOID, 0, info.DelRef))
+func writePushData(t *testing.T, srp spokesReceivePackProcess, updates []refUpdate, pack io.Reader) {
+	caps := "\x00report-status report-status-v2 side-band-64k object-format=sha1\n"
+	for _, up := range updates {
+		require.NoError(t, writePktlinef(srp.In,
+			"%s %s %s%s",
+			up.OldOID, up.NewOID, up.Ref,
+			caps))
+		caps = ""
+	}
 
-	_, err = srpIn.Write([]byte("0000"))
+	_, err := srp.In.Write([]byte("0000"))
 	require.NoError(t, err)
 
-	// Send the pack that's missing a commit.
-	pack, err := os.Open("testdata/missing-objects/bad.pack")
-	require.NoError(t, err)
-	if _, err := io.Copy(srpIn, pack); err != nil {
+	if _, err := io.Copy(srp.In, pack); err != nil {
 		t.Logf("error writing pack to spokes-receive-pack input: %v", err)
 	}
 
-	require.NoError(t, srpIn.Close())
-
-	refStatus, unpackRes, _, err := readResult(t, bufSRPOut)
-	require.NoError(t, err)
-	assert.Equal(t, map[string]string{
-		info.Ref:    "ng missing necessary objects",
-		info.DelRef: "ok",
-		refToCreate: "ok",
-	}, refStatus)
-	assert.Equal(t, "unpack ok\n", unpackRes)
+	require.NoError(t, srp.In.Close())
 }

internal/integration/testdata/bad-date/sha1.git/HEAD

@@ -0,0 +1 @@
+ref: refs/heads/main

internal/integration/testdata/bad-date/sha1.git/config

@@ -0,0 +1,4 @@
+[core]
+	repositoryformatversion = 0
+	filemode = true
+	bare = true

internal/integration/testdata/bad-date/sha1.git/description

@@ -0,0 +1 @@
+Unnamed repository; edit this file 'description' to name the repository.

internal/integration/testdata/bad-date/sha1.git/info/refs

@@ -0,0 +1 @@
+e1971a634e8b1e52b09eba0d21d03ec291c6b690	refs/heads/main

internal/integration/testdata/bad-date/sha1.git/objects/info/packs

@@ -0,0 +1,2 @@
+P pack-01f7c523f4ab85d86dbe8b116ab6cd69e87497f3.pack
+