diff --git a/Dockerfile b/Dockerfile index 5f89b20b..9ffb3647 100644 --- a/Dockerfile +++ b/Dockerfile @@ -9,9 +9,10 @@ ENV ASSETS_DIR=/usr/local/bin \ USER_NAME=codebase-operator \ USER_UID=1001 -RUN apk add --no-cache ca-certificates=20261020-r0 \ - openssh-client==10.0_p1-r10 \ - git==2.49.1-r0 +# Alpine keeps only the latest build of a package per branch: pin minors (~=), not builds, and bump them with the FROM branch. +# apk upgrade also brings the base image's CA bundle up to date. +RUN apk upgrade --no-cache && \ + apk add --no-cache git~=2.49 openssh-client~=10.0 RUN adduser -h ${HOME} -s /bin/ash -D -u ${USER_UID} codebase-operator diff --git a/pkg/git/cli.go b/pkg/git/cli.go index d47b90fc..a0d09ab4 100644 --- a/pkg/git/cli.go +++ b/pkg/git/cli.go @@ -36,6 +36,9 @@ const ( hostKeyChangedMsg = "REMOTE HOST IDENTIFICATION HAS CHANGED" hostKeyRevokedMsg = "REVOKED HOST KEY DETECTED" + // git push stderr text for a remote without atomic push, matched by substring. Re-check on git upgrades. + atomicUnsupportedMsg = "does not support --atomic" + // Network stall bounds for remote operations, in seconds. httpStallSeconds = 600 sshConnectSecs = 30 @@ -74,10 +77,14 @@ type gitConfig struct{ key, value string } // baseConfig applies to every git invocation. // - gc.auto=0, maintenance.auto=false: no detached repack that outlives the call. // - pack.threads=1: one pack thread. +// - pack.windowMemory=100m, core.bigFileThreshold=64m: pack-objects caps the memory of its delta window, +// and blobs over the threshold skip the delta search and are streamed. var baseConfig = []gitConfig{ {"gc.auto", "0"}, {"maintenance.auto", "false"}, {"pack.threads", "1"}, + {"pack.windowMemory", "100m"}, + {"core.bigFileThreshold", "64m"}, } // defaultAllowedProtocols is GIT_ALLOW_PROTOCOL for remote operations. @@ -241,6 +248,8 @@ func emptyDirectory(path string) error { } // Push pushes refspecs to origin without forcing. Without refspecs it pushes all branches. +// The push is atomic when the remote supports it: a rejected ref then leaves every ref unchanged. +// Otherwise it is a plain push, which can update some refs while the remote rejects others. func (p *CLIGitProvider) Push(ctx context.Context, directory string, refspecs ...string) error { log := ctrl.LoggerFrom(ctx).WithValues("directory", directory) log.Info("Pushing changes with git CLI") @@ -254,9 +263,20 @@ func (p *CLIGitProvider) Push(ctx context.Context, directory string, refspecs .. return fmt.Errorf("failed to push: %w", err) } - args := append([]string{"-C", directory, "push", "--", "origin"}, refspecs...) + push := func(options ...string) error { + args := append(append([]string{"-C", directory, "push"}, options...), "--", "origin") + + return p.runRemote(ctx, origin, append(args, refspecs...)...) + } + + err = push("--atomic") + if err != nil && strings.Contains(err.Error(), atomicUnsupportedMsg) { + log.Info("Remote does not support atomic push, pushing without it") - if err = p.runRemote(ctx, origin, args...); err != nil { + err = push() + } + + if err != nil { return fmt.Errorf("failed to push: %w", remoteErr(err, origin)) } diff --git a/pkg/git/cli_behavior_test.go b/pkg/git/cli_behavior_test.go index 8d28e74e..35254330 100644 --- a/pkg/git/cli_behavior_test.go +++ b/pkg/git/cli_behavior_test.go @@ -591,6 +591,51 @@ func TestCLIGitProvider_Push_DefaultsToAllBranches(t *testing.T) { assert.Equal(t, gitRun(t, "-C", dst, "rev-parse", "dev"), gitRun(t, "-C", bare, "rev-parse", "dev")) } +func TestCLIGitProvider_Push_Atomic(t *testing.T) { + requireBinary(t, "git") + + setCADefaults(t, nil, nil) + + ctx := context.Background() + root, bare := newBareRemote(t, "dev") + dst := filepath.Join(root, "work") + p := NewCLIGitProvider(Config{}) + + require.NoError(t, p.Clone(ctx, "file://"+bare, dst)) + + // dev is rewritten on the remote, so the push of the local dev is rejected. + seedCommit(t, bare, "remote dev", "+refs/heads/master:refs/heads/dev") + + remoteMain := gitRun(t, "-C", bare, "rev-parse", "main") + + require.NoError(t, os.WriteFile(filepath.Join(dst, "b.txt"), []byte("b"), 0o600)) + require.NoError(t, p.Commit(ctx, dst, "add b")) + + require.ErrorContains(t, p.Push(ctx, dst), "atomic push failed") + assert.Equal(t, remoteMain, gitRun(t, "-C", bare, "rev-parse", "main"), "a rejected ref must leave main unchanged") +} + +func TestCLIGitProvider_Push_RemoteWithoutAtomic(t *testing.T) { + requireBinary(t, "git") + + setCADefaults(t, nil, nil) + + ctx := context.Background() + root, bare := newBareRemote(t) + dst := filepath.Join(root, "work") + p := NewCLIGitProvider(Config{}) + + gitRun(t, "-C", bare, "config", "receive.advertiseAtomic", "false") + require.NoError(t, p.Clone(ctx, "file://"+bare, dst)) + require.NoError(t, os.WriteFile(filepath.Join(dst, "b.txt"), []byte("b"), 0o600)) + require.NoError(t, p.Commit(ctx, dst, "add b")) + gitRun(t, "-C", dst, "tag", "v1") + require.NoError(t, p.Push(ctx, dst, RefSpecPushAllBranches, RefSpecPushAllTags)) + + assert.Equal(t, gitRun(t, "-C", dst, "rev-parse", "main"), gitRun(t, "-C", bare, "rev-parse", "main")) + assert.Equal(t, "v1", gitRun(t, "-C", bare, "tag")) +} + func TestCLIGitProvider_exec_KillRemovesLocks(t *testing.T) { oldGrace, oldDrain := terminateGrace, pipeDrainGrace terminateGrace, pipeDrainGrace = 300*time.Millisecond, time.Second diff --git a/pkg/git/cli_test.go b/pkg/git/cli_test.go index 8a37318c..7c71d37e 100644 --- a/pkg/git/cli_test.go +++ b/pkg/git/cli_test.go @@ -150,9 +150,11 @@ func TestBaseEnv_Config(t *testing.T) { assert.Equal(t, os.DevNull, m["GIT_CONFIG_GLOBAL"]) assert.Equal(t, "1", m["GIT_CONFIG_NOSYSTEM"]) assert.Equal(t, map[string]string{ - "gc.auto": "0", - "maintenance.auto": "false", - "pack.threads": "1", + "gc.auto": "0", + "maintenance.auto": "false", + "pack.threads": "1", + "pack.windowMemory": "100m", + "core.bigFileThreshold": "64m", }, gitConfigMap(t, gitEnv())) } diff --git a/pkg/git/git.go b/pkg/git/git.go index 436ec365..7bee2570 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -11,7 +11,7 @@ type Git interface { // Commit commits changes in the working directory. Commit(ctx context.Context, directory, message string, ops ...CommitOps) error - // Push pushes changes to the remote repository. + // Push pushes changes to the remote repository, atomically when the remote supports it. // refspecs: optional refspecs (e.g., RefSpecPushAllBranches, RefSpecPushAllTags). Push(ctx context.Context, directory string, refspecs ...string) error