Skip to content

Commit a45d5b0

Browse files
leodidoona-agent
andcommitted
fix: correct git timestamp retrieval and integration test issues
Fix critical bug where git commands were running without a working directory set, causing 'exit status 128' failures. Also fix 7 issues in TestDockerPackage_OCILayout_Determinism_Integration. Git Timestamp Fix: - Move getGitCommitTimestamp from sbom.go to gitinfo.go - Rename to GetCommitTimestamp (exported, follows codebase patterns) - Accept GitInfo directly (contains commit hash and working directory) - Ensure git commands run in correct repository directory - Improve error handling and messages Integration Test Fixes: 1. Use FindWorkspace instead of undefined Load function 2. Correct build method signature (buildctx parameter) 3. Fix package name format (use FullName()) 4. Initialize buildContext properly (avoid nil pointer) 5. Initialize ConsoleReporter (avoid nil pointer) 6. Set git user config (required for commits) 7. Use deterministic git timestamps (GIT_AUTHOR_DATE/GIT_COMMITTER_DATE) Impact: - Fixes CI failures when building Docker packages - Enables integration test to run successfully - Verifies OCI layout determinism (same checksum across builds) - Improves code organization and maintainability Verification: - Unit tests pass - Integration test passes with deterministic checksums - No backward compatibility issues Co-authored-by: Ona <no-reply@ona.com>
1 parent 6bc7552 commit a45d5b0

File tree

5 files changed

+129
-62
lines changed

5 files changed

+129
-62
lines changed

pkg/leeway/build.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2437,11 +2437,11 @@ func (p *Package) getDeterministicMtime() (int64, error) {
24372437
"Building from source tarballs without git metadata will cause cache inconsistencies")
24382438
}
24392439

2440-
timestamp, err := getGitCommitTimestamp(context.Background(), commit)
2440+
timestamp, err := GetCommitTimestamp(context.Background(), p.C.Git())
24412441
if err != nil {
2442-
return 0, fmt.Errorf("failed to get deterministic timestamp for tar mtime (commit: %s): %w. "+
2442+
return 0, fmt.Errorf("failed to get deterministic timestamp for tar mtime: %w. "+
24432443
"Ensure git is available and the repository is not a shallow clone, or set SOURCE_DATE_EPOCH environment variable",
2444-
commit, err)
2444+
err)
24452445
}
24462446
return timestamp.Unix(), nil
24472447
}

pkg/leeway/build_integration_test.go

Lines changed: 46 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -590,42 +590,64 @@ CMD ["cat", "/build-time.txt"]
590590
t.Fatal(err)
591591
}
592592

593+
gitConfigName := exec.Command("git", "config", "user.name", "Test User")
594+
gitConfigName.Dir = wsDir
595+
if err := gitConfigName.Run(); err != nil {
596+
t.Fatal(err)
597+
}
598+
599+
gitConfigEmail := exec.Command("git", "config", "user.email", "test@example.com")
600+
gitConfigEmail.Dir = wsDir
601+
if err := gitConfigEmail.Run(); err != nil {
602+
t.Fatal(err)
603+
}
604+
593605
gitAdd := exec.Command("git", "add", ".")
594606
gitAdd.Dir = wsDir
595607
if err := gitAdd.Run(); err != nil {
596608
t.Fatal(err)
597609
}
598610

611+
// Use fixed timestamp for deterministic git commit
612+
// This ensures the commit timestamp is the same across test runs
599613
gitCommit := exec.Command("git", "commit", "-m", "initial")
600614
gitCommit.Dir = wsDir
615+
gitCommit.Env = append(os.Environ(),
616+
"GIT_AUTHOR_DATE=2021-01-01T00:00:00Z",
617+
"GIT_COMMITTER_DATE=2021-01-01T00:00:00Z",
618+
)
601619
if err := gitCommit.Run(); err != nil {
602620
t.Fatal(err)
603621
}
604622

605623
// Build first time
606624
cacheDir1 := filepath.Join(tmpDir, "cache1")
607-
if err := os.MkdirAll(cacheDir1, 0755); err != nil {
625+
cache1, err := local.NewFilesystemCache(cacheDir1)
626+
if err != nil {
608627
t.Fatal(err)
609628
}
610629

611-
buildCtx1 := &buildContext{
612-
LocalCache: &FilesystemCache{Location: cacheDir1},
613-
DockerExportToCache: true,
614-
DockerExportSet: true,
615-
Reporter: &ConsoleReporter{},
630+
buildCtx1, err := newBuildContext(buildOptions{
631+
LocalCache: cache1,
632+
DockerExportToCache: true,
633+
DockerExportSet: true,
634+
Reporter: NewConsoleReporter(),
635+
})
636+
if err != nil {
637+
t.Fatalf("Failed to create build context: %v", err)
616638
}
617639

618-
ws1, err := Load(wsDir)
640+
ws1, err := FindWorkspace(wsDir, Arguments{}, "", "")
619641
if err != nil {
620642
t.Fatalf("Failed to load workspace: %v", err)
621643
}
622644

623-
pkg1, exists := ws1.Packages[":test-image"]
645+
pkg1, exists := ws1.Packages["//:test-image"]
624646
if !exists {
625-
t.Fatal("Package :test-image not found")
647+
t.Fatal("Package //:test-image not found")
626648
}
627649

628-
if _, err := pkg1.build(buildCtx1); err != nil {
650+
if err := pkg1.build(buildCtx1); err != nil {
629651
t.Fatalf("First build failed: %v", err)
630652
}
631653

@@ -641,28 +663,32 @@ CMD ["cat", "/build-time.txt"]
641663

642664
// Build second time (clean cache)
643665
cacheDir2 := filepath.Join(tmpDir, "cache2")
644-
if err := os.MkdirAll(cacheDir2, 0755); err != nil {
666+
cache2, err := local.NewFilesystemCache(cacheDir2)
667+
if err != nil {
645668
t.Fatal(err)
646669
}
647670

648-
buildCtx2 := &buildContext{
649-
LocalCache: &FilesystemCache{Location: cacheDir2},
650-
DockerExportToCache: true,
651-
DockerExportSet: true,
652-
Reporter: &ConsoleReporter{},
671+
buildCtx2, err := newBuildContext(buildOptions{
672+
LocalCache: cache2,
673+
DockerExportToCache: true,
674+
DockerExportSet: true,
675+
Reporter: NewConsoleReporter(),
676+
})
677+
if err != nil {
678+
t.Fatalf("Failed to create build context: %v", err)
653679
}
654680

655-
ws2, err := Load(wsDir)
681+
ws2, err := FindWorkspace(wsDir, Arguments{}, "", "")
656682
if err != nil {
657683
t.Fatalf("Failed to load workspace: %v", err)
658684
}
659685

660-
pkg2, exists := ws2.Packages[":test-image"]
686+
pkg2, exists := ws2.Packages["//:test-image"]
661687
if !exists {
662-
t.Fatal("Package :test-image not found")
688+
t.Fatal("Package //:test-image not found")
663689
}
664690

665-
if _, err := pkg2.build(buildCtx2); err != nil {
691+
if err := pkg2.build(buildCtx2); err != nil {
666692
t.Fatalf("Second build failed: %v", err)
667693
}
668694

pkg/leeway/gitinfo.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
package leeway
22

33
import (
4+
"context"
45
"fmt"
56
"os"
67
"os/exec"
78
"path/filepath"
9+
"strconv"
810
"strings"
11+
"time"
912

1013
log "github.com/sirupsen/logrus"
1114
"golang.org/x/xerrors"
@@ -174,3 +177,53 @@ func (info *GitInfo) DirtyFiles(files []string) bool {
174177
}
175178
return false
176179
}
180+
181+
// GetCommitTimestamp returns the timestamp of a git commit.
182+
// It first checks SOURCE_DATE_EPOCH for reproducible builds, then falls back to git.
183+
// The context allows for cancellation of the git command if the build is cancelled.
184+
//
185+
// This function is used for:
186+
// - Deterministic tar mtime in package builds
187+
// - SBOM timestamp normalization
188+
// - Any operation requiring reproducible timestamps
189+
func GetCommitTimestamp(ctx context.Context, gitInfo *GitInfo) (time.Time, error) {
190+
// Try SOURCE_DATE_EPOCH first (for reproducible builds)
191+
// This takes precedence over git to allow explicit override
192+
if epoch := os.Getenv("SOURCE_DATE_EPOCH"); epoch != "" {
193+
timestamp, err := strconv.ParseInt(epoch, 10, 64)
194+
if err == nil {
195+
return time.Unix(timestamp, 0).UTC(), nil
196+
}
197+
// Log warning but continue to git fallback
198+
log.WithError(err).WithField("SOURCE_DATE_EPOCH", epoch).Warn("Invalid SOURCE_DATE_EPOCH, falling back to git commit timestamp")
199+
}
200+
201+
// Validate git information is available
202+
if gitInfo == nil {
203+
return time.Time{}, fmt.Errorf("no git information available")
204+
}
205+
206+
if gitInfo.Commit == "" {
207+
return time.Time{}, fmt.Errorf("no git commit available")
208+
}
209+
210+
// Execute git command with context support for cancellation
211+
cmd := exec.CommandContext(ctx, "git", "show", "-s", "--format=%ct", gitInfo.Commit)
212+
cmd.Dir = gitInfo.WorkingCopyLoc
213+
214+
output, err := cmd.Output()
215+
if err != nil {
216+
return time.Time{}, &GitError{
217+
Op: fmt.Sprintf("show -s --format=%%ct %s", gitInfo.Commit),
218+
Err: err,
219+
}
220+
}
221+
222+
// Parse Unix timestamp
223+
timestamp, err := strconv.ParseInt(strings.TrimSpace(string(output)), 10, 64)
224+
if err != nil {
225+
return time.Time{}, fmt.Errorf("failed to parse commit timestamp '%s': %w", strings.TrimSpace(string(output)), err)
226+
}
227+
228+
return time.Unix(timestamp, 0).UTC(), nil
229+
}

pkg/leeway/sbom.go

Lines changed: 3 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,9 @@ import (
1010
"fmt"
1111
"io"
1212
"os"
13-
"os/exec"
1413
"path/filepath"
1514
"regexp"
1615
"runtime"
17-
"strconv"
1816
"strings"
1917
"time"
2018

@@ -103,33 +101,6 @@ func GetSBOMParallelism(sbomConfig WorkspaceSBOM) int {
103101
return runtime.NumCPU()
104102
}
105103

106-
// getGitCommitTimestamp returns the timestamp of the git commit.
107-
// The context allows for cancellation of the git command if the build is cancelled.
108-
func getGitCommitTimestamp(ctx context.Context, commit string) (time.Time, error) {
109-
// Try SOURCE_DATE_EPOCH first (for reproducible builds)
110-
if epoch := os.Getenv("SOURCE_DATE_EPOCH"); epoch != "" {
111-
timestamp, err := strconv.ParseInt(epoch, 10, 64)
112-
if err == nil {
113-
return time.Unix(timestamp, 0).UTC(), nil
114-
}
115-
// Log warning but continue to git fallback
116-
log.WithError(err).WithField("SOURCE_DATE_EPOCH", epoch).Warn("Invalid SOURCE_DATE_EPOCH, falling back to git commit timestamp")
117-
}
118-
119-
// Get commit timestamp from git with context support for cancellation
120-
cmd := exec.CommandContext(ctx, "git", "show", "-s", "--format=%ct", commit)
121-
output, err := cmd.Output()
122-
if err != nil {
123-
return time.Time{}, fmt.Errorf("failed to get commit timestamp: %w", err)
124-
}
125-
126-
timestamp, err := strconv.ParseInt(strings.TrimSpace(string(output)), 10, 64)
127-
if err != nil {
128-
return time.Time{}, fmt.Errorf("failed to parse commit timestamp: %w", err)
129-
}
130-
131-
return time.Unix(timestamp, 0).UTC(), nil
132-
}
133104

134105
// generateDeterministicUUID generates a UUIDv5 from content
135106
func generateDeterministicUUID(content []byte) string {
@@ -324,11 +295,11 @@ func writeSBOM(buildctx *buildContext, p *Package, builddir string) (err error)
324295
}
325296

326297
// Normalize SBOMs after generation
327-
timestamp, err := getGitCommitTimestamp(context.Background(), p.C.Git().Commit)
298+
timestamp, err := GetCommitTimestamp(context.Background(), p.C.Git())
328299
if err != nil {
329-
return fmt.Errorf("failed to get deterministic timestamp for SBOM normalization (commit: %s): %w. "+
300+
return fmt.Errorf("failed to get deterministic timestamp for SBOM normalization: %w. "+
330301
"Ensure git is available and the repository is not a shallow clone, or set SOURCE_DATE_EPOCH environment variable",
331-
p.C.Git().Commit, err)
302+
err)
332303
}
333304

334305
// Normalize CycloneDX

pkg/leeway/sbom_normalize_test.go

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ func TestGenerateDeterministicUUID(t *testing.T) {
6767
}
6868
}
6969

70-
func TestGetGitCommitTimestamp_SourceDateEpoch(t *testing.T) {
70+
func TestGetCommitTimestamp_SourceDateEpoch(t *testing.T) {
7171
// Save original env var
7272
originalEnv := os.Getenv("SOURCE_DATE_EPOCH")
7373
defer func() {
@@ -111,7 +111,12 @@ func TestGetGitCommitTimestamp_SourceDateEpoch(t *testing.T) {
111111
}
112112

113113
// Use HEAD as commit (should exist in test environment)
114-
timestamp, err := getGitCommitTimestamp(context.Background(), "HEAD")
114+
wd, _ := os.Getwd()
115+
gitInfo := &GitInfo{
116+
Commit: "HEAD",
117+
WorkingCopyLoc: wd,
118+
}
119+
timestamp, err := GetCommitTimestamp(context.Background(), gitInfo)
115120

116121
if tt.wantErr && err == nil {
117122
t.Error("expected error, got nil")
@@ -128,11 +133,17 @@ func TestGetGitCommitTimestamp_SourceDateEpoch(t *testing.T) {
128133
}
129134
}
130135

131-
func TestGetGitCommitTimestamp_GitCommit(t *testing.T) {
136+
func TestGetCommitTimestamp_GitCommit(t *testing.T) {
132137
// This test requires being in a git repository
133138
// Use HEAD as a known commit
134139
ctx := context.Background()
135-
timestamp, err := getGitCommitTimestamp(ctx, "HEAD")
140+
wd, _ := os.Getwd()
141+
gitInfo := &GitInfo{
142+
Commit: "HEAD",
143+
WorkingCopyLoc: wd,
144+
}
145+
146+
timestamp, err := GetCommitTimestamp(ctx, gitInfo)
136147
if err != nil {
137148
t.Skipf("skipping test: not in a git repository or git not available: %v", err)
138149
}
@@ -148,7 +159,7 @@ func TestGetGitCommitTimestamp_GitCommit(t *testing.T) {
148159
}
149160

150161
// Verify deterministic: calling twice should return same result
151-
timestamp2, err := getGitCommitTimestamp(ctx, "HEAD")
162+
timestamp2, err := GetCommitTimestamp(ctx, gitInfo)
152163
if err != nil {
153164
t.Fatalf("second call failed: %v", err)
154165
}
@@ -157,12 +168,18 @@ func TestGetGitCommitTimestamp_GitCommit(t *testing.T) {
157168
}
158169
}
159170

160-
func TestGetGitCommitTimestamp_ContextCancellation(t *testing.T) {
171+
func TestGetCommitTimestamp_ContextCancellation(t *testing.T) {
161172
// Create a cancelled context
162173
ctx, cancel := context.WithCancel(context.Background())
163174
cancel() // Cancel immediately
164175

165-
_, err := getGitCommitTimestamp(ctx, "HEAD")
176+
wd, _ := os.Getwd()
177+
gitInfo := &GitInfo{
178+
Commit: "HEAD",
179+
WorkingCopyLoc: wd,
180+
}
181+
182+
_, err := GetCommitTimestamp(ctx, gitInfo)
166183
if err == nil {
167184
t.Error("expected error with cancelled context, got nil")
168185
}

0 commit comments

Comments
 (0)