Skip to content

Commit c7ced70

Browse files
leodidoona-agent
andcommitted
fix(sbom): use determineDockerExportMode for consistent export mode detection
The SBOM code had a simplified 3-layer precedence for determining exportToCache, while buildDocker uses a 5-layer precedence via determineDockerExportMode(). This caused SBOM to make different decisions than the build about whether to use OCI layout or Docker daemon. Bug scenario: Package has exportToCache=true, user sets env var to false. Build correctly uses Docker daemon, but SBOM tried to scan image.tar which doesn't exist. Fix: Call determineDockerExportMode() in SBOM code to ensure both build and SBOM use the same precedence logic. Added integration test that reproduces the bug and verifies the fix. Co-authored-by: Ona <[email protected]>
1 parent af13065 commit c7ced70

File tree

2 files changed

+258
-44
lines changed

2 files changed

+258
-44
lines changed

pkg/leeway/build_integration_test.go

Lines changed: 248 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -269,29 +269,29 @@ CMD ["echo", "test"]`
269269
WithLocalCache(localCache),
270270
WithDontTest(true),
271271
)
272-
272+
273273
// Handle expected errors (e.g., push failures without credentials)
274274
if tt.expectError {
275275
if err == nil {
276276
t.Fatal("Expected build to fail but it succeeded")
277277
}
278-
278+
279279
// Validate error matches expected pattern
280280
if tt.expectErrorMatch != "" {
281281
matched, regexErr := regexp.MatchString(tt.expectErrorMatch, err.Error())
282282
if regexErr != nil {
283283
t.Fatalf("Invalid error regex pattern: %v", regexErr)
284284
}
285285
if !matched {
286-
t.Fatalf("Error doesn't match expected pattern.\nExpected pattern: %s\nActual error: %v",
286+
t.Fatalf("Error doesn't match expected pattern.\nExpected pattern: %s\nActual error: %v",
287287
tt.expectErrorMatch, err)
288288
}
289-
t.Logf("Build failed as expected with error matching pattern '%s': %v",
289+
t.Logf("Build failed as expected with error matching pattern '%s': %v",
290290
tt.expectErrorMatch, err)
291291
} else {
292292
t.Logf("Build failed as expected: %v", err)
293293
}
294-
294+
295295
// For legacy push test, we expect it to fail at push step
296296
// The detailed Docker error (e.g., "push access denied", "authorization failed")
297297
// is logged but wrapped in a generic "build failed" error.
@@ -300,7 +300,7 @@ CMD ["echo", "test"]`
300300
// Skip further validation for this test case
301301
return
302302
}
303-
303+
304304
if err != nil {
305305
t.Fatalf("Build failed: %v", err)
306306
}
@@ -328,10 +328,10 @@ CMD ["echo", "test"]`
328328
// Normalize paths for comparison (remove leading ./)
329329
normalizedActual := strings.TrimPrefix(actualFile, "./")
330330
normalizedExpected := strings.TrimPrefix(expectedFile, "./")
331-
332-
if filepath.Base(normalizedActual) == normalizedExpected ||
333-
normalizedActual == normalizedExpected ||
334-
strings.HasPrefix(normalizedActual, normalizedExpected+"/") {
331+
332+
if filepath.Base(normalizedActual) == normalizedExpected ||
333+
normalizedActual == normalizedExpected ||
334+
strings.HasPrefix(normalizedActual, normalizedExpected+"/") {
335335
found = true
336336
break
337337
}
@@ -557,7 +557,7 @@ CMD ["cat", "/test-file.txt"]`
557557

558558
// Step 4: Extract image.tar from cache and load into Docker
559559
t.Log("Step 4: Extracting image.tar and loading into Docker")
560-
560+
561561
// First, remove the image if it exists
562562
exec.Command("docker", "rmi", "-f", testImage).Run()
563563

@@ -585,19 +585,19 @@ CMD ["cat", "/test-file.txt"]`
585585
if err := os.MkdirAll(ociDir, 0755); err != nil {
586586
t.Fatal(err)
587587
}
588-
588+
589589
extractOCICmd := exec.Command("tar", "-xf", imageTarPath, "-C", ociDir)
590590
if output, err := extractOCICmd.CombinedOutput(); err != nil {
591591
t.Fatalf("Failed to extract OCI layout: %v\nOutput: %s", err, string(output))
592592
}
593-
593+
594594
// Try skopeo first, fall back to crane, then fail with helpful message
595595
var loadCmd *exec.Cmd
596596
var toolUsed string
597-
597+
598598
if _, err := exec.LookPath("skopeo"); err == nil {
599599
// Use skopeo to load OCI layout directory
600-
loadCmd = exec.Command("skopeo", "copy",
600+
loadCmd = exec.Command("skopeo", "copy",
601601
fmt.Sprintf("oci:%s", ociDir),
602602
fmt.Sprintf("docker-daemon:%s", testImage))
603603
toolUsed = "skopeo"
@@ -611,7 +611,7 @@ CMD ["cat", "/test-file.txt"]`
611611
" apt-get install skopeo # or\n" +
612612
" go install github.com/google/go-containerregistry/cmd/crane@latest")
613613
}
614-
614+
615615
loadOutput, err := loadCmd.CombinedOutput()
616616
if err != nil {
617617
t.Fatalf("Failed to load OCI image using %s: %v\nOutput: %s", toolUsed, err, string(loadOutput))
@@ -620,15 +620,15 @@ CMD ["cat", "/test-file.txt"]`
620620

621621
// Step 5: Verify the loaded image works
622622
t.Log("Step 5: Verifying loaded image works")
623-
623+
624624
// Get the digest of the loaded image
625625
inspectCmd := exec.Command("docker", "inspect", "--format={{index .Id}}", testImage)
626626
inspectOutput, err := inspectCmd.Output()
627627
if err != nil {
628628
t.Fatalf("Failed to inspect loaded image: %v", err)
629629
}
630630
loadedDigest := strings.TrimSpace(string(inspectOutput))
631-
631+
632632
t.Logf("Loaded image digest: %s", loadedDigest)
633633
t.Logf("Original metadata digest: %s", metadata.Digest)
634634

@@ -1243,7 +1243,6 @@ RUN echo "test content" > /test.txt
12431243
}
12441244
}
12451245

1246-
12471246
// TestDockerPackage_OCIExtraction_NoImage_Integration reproduces and verifies the fix for
12481247
// the bug where container extraction fails with "No such image" when exportToCache=true.
12491248
//
@@ -1706,7 +1705,6 @@ CMD ["echo", "test"]`
17061705
}
17071706
}
17081707

1709-
17101708
// TestDockerPackage_SBOM_EnvVar_Integration verifies SBOM generation respects
17111709
// LEEWAY_DOCKER_EXPORT_TO_CACHE environment variable when package config doesn't
17121710
// explicitly set exportToCache.
@@ -1958,3 +1956,233 @@ CMD ["echo", "test"]`
19581956
t.Logf("✅ SBOM generation correctly respects LEEWAY_DOCKER_EXPORT_TO_CACHE environment variable")
19591957
}
19601958
}
1959+
1960+
// TestDockerPackage_SBOM_UserEnvOverridesPackageConfig_Integration verifies that
1961+
// user-set environment variable overrides package config for SBOM generation.
1962+
//
1963+
// This tests the precedence hierarchy:
1964+
// 1. CLI flag (highest)
1965+
// 2. User environment variable (set before workspace loading) <-- This should override package config
1966+
// 3. Package config (exportToCache in BUILD.yaml)
1967+
// 4. Workspace default
1968+
// 5. Global default (lowest)
1969+
//
1970+
// Bug scenario: Package has exportToCache=true, but user sets LEEWAY_DOCKER_EXPORT_TO_CACHE=false.
1971+
// Build correctly uses Docker daemon (no OCI), but SBOM incorrectly tries to scan image.tar.
1972+
func TestDockerPackage_SBOM_UserEnvOverridesPackageConfig_Integration(t *testing.T) {
1973+
if testing.Short() {
1974+
t.Skip("Skipping integration test in short mode")
1975+
}
1976+
1977+
// Ensure Docker is available
1978+
if err := exec.Command("docker", "version").Run(); err != nil {
1979+
t.Skip("Docker not available, skipping integration test")
1980+
}
1981+
1982+
// Create temporary workspace
1983+
tmpDir := t.TempDir()
1984+
1985+
// Initialize git repository
1986+
{
1987+
gitInit := exec.Command("git", "init")
1988+
gitInit.Dir = tmpDir
1989+
gitInit.Env = append(os.Environ(), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null")
1990+
if err := gitInit.Run(); err != nil {
1991+
t.Fatalf("Failed to initialize git repository: %v", err)
1992+
}
1993+
1994+
gitConfigName := exec.Command("git", "config", "user.name", "Test User")
1995+
gitConfigName.Dir = tmpDir
1996+
gitConfigName.Env = append(os.Environ(), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null")
1997+
if err := gitConfigName.Run(); err != nil {
1998+
t.Fatalf("Failed to configure git user.name: %v", err)
1999+
}
2000+
2001+
gitConfigEmail := exec.Command("git", "config", "user.email", "[email protected]")
2002+
gitConfigEmail.Dir = tmpDir
2003+
gitConfigEmail.Env = append(os.Environ(), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null")
2004+
if err := gitConfigEmail.Run(); err != nil {
2005+
t.Fatalf("Failed to configure git user.email: %v", err)
2006+
}
2007+
}
2008+
2009+
// Create WORKSPACE.yaml with SBOM enabled
2010+
workspaceYAML := `defaultTarget: "app:docker"
2011+
sbom:
2012+
enabled: true
2013+
scanVulnerabilities: false`
2014+
workspacePath := filepath.Join(tmpDir, "WORKSPACE.yaml")
2015+
if err := os.WriteFile(workspacePath, []byte(workspaceYAML), 0644); err != nil {
2016+
t.Fatal(err)
2017+
}
2018+
2019+
// Create component directory
2020+
appDir := filepath.Join(tmpDir, "app")
2021+
if err := os.MkdirAll(appDir, 0755); err != nil {
2022+
t.Fatal(err)
2023+
}
2024+
2025+
// Create a simple Dockerfile
2026+
dockerfile := `FROM alpine:latest
2027+
RUN apk add --no-cache curl
2028+
LABEL test="sbom-override-test"
2029+
CMD ["echo", "test"]`
2030+
2031+
dockerfilePath := filepath.Join(appDir, "Dockerfile")
2032+
if err := os.WriteFile(dockerfilePath, []byte(dockerfile), 0644); err != nil {
2033+
t.Fatal(err)
2034+
}
2035+
2036+
// Create BUILD.yaml WITH exportToCache=true
2037+
// This is the key: package wants OCI export, but user will override via env var
2038+
buildYAML := `packages:
2039+
- name: docker
2040+
type: docker
2041+
config:
2042+
dockerfile: Dockerfile
2043+
exportToCache: true`
2044+
2045+
buildPath := filepath.Join(appDir, "BUILD.yaml")
2046+
if err := os.WriteFile(buildPath, []byte(buildYAML), 0644); err != nil {
2047+
t.Fatal(err)
2048+
}
2049+
2050+
// Create initial git commit
2051+
gitAdd := exec.Command("git", "add", ".")
2052+
gitAdd.Dir = tmpDir
2053+
gitAdd.Env = append(os.Environ(), "GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_SYSTEM=/dev/null")
2054+
if err := gitAdd.Run(); err != nil {
2055+
t.Fatalf("Failed to git add: %v", err)
2056+
}
2057+
2058+
gitCommit := exec.Command("git", "commit", "-m", "initial")
2059+
gitCommit.Dir = tmpDir
2060+
gitCommit.Env = append(os.Environ(),
2061+
"GIT_CONFIG_GLOBAL=/dev/null",
2062+
"GIT_CONFIG_SYSTEM=/dev/null",
2063+
"GIT_AUTHOR_DATE=2021-01-01T00:00:00Z",
2064+
"GIT_COMMITTER_DATE=2021-01-01T00:00:00Z",
2065+
)
2066+
if err := gitCommit.Run(); err != nil {
2067+
t.Fatalf("Failed to git commit: %v", err)
2068+
}
2069+
2070+
// Load workspace
2071+
workspace, err := FindWorkspace(tmpDir, Arguments{}, "", "")
2072+
if err != nil {
2073+
t.Fatal(err)
2074+
}
2075+
2076+
// Verify SBOM is enabled
2077+
if !workspace.SBOM.Enabled {
2078+
t.Fatal("SBOM should be enabled in workspace")
2079+
}
2080+
2081+
// Get the package and verify it has exportToCache=true
2082+
pkg, ok := workspace.Packages["app:docker"]
2083+
if !ok {
2084+
t.Fatal("package app:docker not found")
2085+
}
2086+
2087+
dockerCfg, ok := pkg.Config.(DockerPkgConfig)
2088+
if !ok {
2089+
t.Fatal("package should have Docker config")
2090+
}
2091+
if dockerCfg.ExportToCache == nil || !*dockerCfg.ExportToCache {
2092+
t.Fatal("package config should have exportToCache=true")
2093+
}
2094+
2095+
t.Log("Package has exportToCache=true in config")
2096+
2097+
// Create build context with user env var override set to FALSE
2098+
// This simulates: user explicitly sets LEEWAY_DOCKER_EXPORT_TO_CACHE=false
2099+
// which should override the package config's exportToCache=true
2100+
cacheDir := filepath.Join(tmpDir, ".cache")
2101+
cache, err := local.NewFilesystemCache(cacheDir)
2102+
if err != nil {
2103+
t.Fatal(err)
2104+
}
2105+
2106+
buildCtx, err := newBuildContext(buildOptions{
2107+
LocalCache: cache,
2108+
// Simulate user explicitly setting env var to false BEFORE workspace loading
2109+
// This is Layer 2 in the precedence hierarchy and should override Layer 3 (package config)
2110+
DockerExportEnvSet: true,
2111+
DockerExportEnvValue: false,
2112+
Reporter: NewConsoleReporter(),
2113+
})
2114+
if err != nil {
2115+
t.Fatal(err)
2116+
}
2117+
2118+
t.Log("Build context has DockerExportEnvSet=true, DockerExportEnvValue=false (user override)")
2119+
2120+
// Build the package
2121+
// With the fix: Build uses Docker daemon (no OCI) because user env overrides package config
2122+
// SBOM should also use Docker daemon
2123+
// Without the fix: Build uses Docker daemon, but SBOM tries to use OCI (image.tar) -> fails
2124+
err = pkg.build(buildCtx)
2125+
if err != nil {
2126+
t.Fatalf("Build failed: %v\n\nThis failure likely means SBOM tried to scan image.tar "+
2127+
"which doesn't exist because the build correctly used Docker daemon "+
2128+
"(user env var override). The SBOM code needs to use determineDockerExportMode().", err)
2129+
}
2130+
2131+
t.Log("✅ Build succeeded - SBOM correctly respected user env var override")
2132+
2133+
// Verify SBOM files were created
2134+
cacheLoc, exists := cache.Location(pkg)
2135+
if !exists {
2136+
t.Fatal("Package not found in cache")
2137+
}
2138+
2139+
sbomFormats := []string{
2140+
"sbom.cdx.json",
2141+
"sbom.spdx.json",
2142+
"sbom.json",
2143+
}
2144+
2145+
foundSBOMs := make(map[string]bool)
2146+
2147+
f, err := os.Open(cacheLoc)
2148+
if err != nil {
2149+
t.Fatalf("Failed to open cache file: %v", err)
2150+
}
2151+
defer f.Close()
2152+
2153+
gzin, err := gzip.NewReader(f)
2154+
if err != nil {
2155+
t.Fatalf("Failed to create gzip reader: %v", err)
2156+
}
2157+
defer gzin.Close()
2158+
2159+
tarin := tar.NewReader(gzin)
2160+
for {
2161+
hdr, err := tarin.Next()
2162+
if errors.Is(err, io.EOF) {
2163+
break
2164+
}
2165+
if err != nil {
2166+
t.Fatalf("Failed to read tar: %v", err)
2167+
}
2168+
2169+
filename := filepath.Base(hdr.Name)
2170+
for _, sbomFile := range sbomFormats {
2171+
if filename == sbomFile {
2172+
foundSBOMs[sbomFile] = true
2173+
t.Logf("✅ Found SBOM file: %s", sbomFile)
2174+
}
2175+
}
2176+
}
2177+
2178+
for _, sbomFile := range sbomFormats {
2179+
if !foundSBOMs[sbomFile] {
2180+
t.Errorf("❌ SBOM file %s not found in cache", sbomFile)
2181+
}
2182+
}
2183+
2184+
if len(foundSBOMs) == len(sbomFormats) {
2185+
t.Log("✅ All SBOM formats generated successfully")
2186+
t.Log("✅ SBOM generation correctly respects user env var override of package config")
2187+
}
2188+
}

0 commit comments

Comments
 (0)