fix: validate dependencies after download to prevent cache inconsistency

leodido · ona-agent · leodido · commit a19656c45a10 · 2025-12-05T11:08:27.000+01:00
When a package is downloaded but one of its dependencies fails to
download (e.g., network error, SLSA verification failure), the cached
package becomes unusable because its dependencies are missing.

This causes build failures that cannot recover because:
1. Package A is in cache, so A.build() returns early
2. A.buildDependencies() is never called
3. Missing dependency B is never built
4. Build fails with PkgNotBuiltErr{B}

Fix: After the download phase, validate that all cached packages have
their required dependencies available. If a dependency is missing and
won't be built, remove the package from cache and mark it for rebuild.

The validation respects package types:
- Go/Yarn packages: check all transitive dependencies
- Generic/Docker packages: check only direct dependencies
- Ephemeral dependencies are skipped (always rebuilt)

A dependency is considered available if:
- It's in the local cache, OR
- It will be built (PackageNotBuiltYet), OR
- It will be downloaded (PackageInRemoteCache)

Co-authored-by: Ona &lt;no-reply@ona.com&gt;
diff --git a/pkg/leeway/build.go b/pkg/leeway/build.go
@@ -707,6 +707,31 @@ func Build(pkg *Package, opts ...BuildOption) (err error) {
 		}
 	}
 
+	// Validate that cached packages have all their dependencies available.
+	// This prevents build failures when a package is cached but a dependency failed to download.
+	// If a cached package has missing dependencies, remove it from cache and mark for rebuild.
+	for _, p := range allpkg {
+		status := pkgstatus[p]
+		if status != PackageDownloaded && status != PackageBuilt {
+			// Only validate packages that are in the local cache
+			continue
+		}
+
+		if !validateDependenciesAvailable(p, ctx.LocalCache, pkgstatus) {
+			log.WithField("package", p.FullName()).Warn("Cached package has missing dependencies, will rebuild")
+
+			// Remove the package from local cache
+			if path, exists := ctx.LocalCache.Location(p); exists {
+				if err := os.Remove(path); err != nil {
+					log.WithError(err).WithField("package", p.FullName()).Warn("Failed to remove package from cache")
+				}
+			}
+
+			// Mark for rebuild
+			pkgstatus[p] = PackageNotBuiltYet
+		}
+	}
+
 	ctx.Reporter.BuildStarted(pkg, pkgstatus)
 	defer func(err *error) {
 		ctx.Reporter.BuildFinished(pkg, *err)
@@ -1303,6 +1328,58 @@ func (p *Package) packagesToDownload(inLocalCache map[*Package]struct{}, inRemot
 	}
 }
 
+// validateDependenciesAvailable checks if all required dependencies of a package are available.
+// A dependency is considered available if it's in the local cache OR will be built (PackageNotBuiltYet).
+// Returns true if all dependencies are available, false otherwise.
+//
+// This validation ensures cache consistency: a package should only remain in cache
+// if all its dependencies are also available. This prevents build failures when
+// a package is cached but one of its dependencies failed to download.
+func validateDependenciesAvailable(p *Package, localCache cache.LocalCache, pkgstatus map[*Package]PackageBuildStatus) bool {
+	var deps []*Package
+	switch p.Type {
+	case YarnPackage, GoPackage:
+		// Go and Yarn packages need all transitive dependencies
+		deps = p.GetTransitiveDependencies()
+	case GenericPackage, DockerPackage:
+		// Generic and Docker packages only need direct dependencies
+		deps = p.GetDependencies()
+	default:
+		deps = p.GetDependencies()
+	}
+
+	for _, dep := range deps {
+		if dep.Ephemeral {
+			// Ephemeral packages are always rebuilt, skip validation
+			continue
+		}
+
+		_, inCache := localCache.Location(dep)
+		status := pkgstatus[dep]
+
+		// Dependency is available if:
+		// 1. It's in the local cache (PackageBuilt or PackageDownloaded), OR
+		// 2. It will be built locally (PackageNotBuiltYet), OR
+		// 3. It will be downloaded (PackageInRemoteCache)
+		depAvailable := inCache ||
+			status == PackageNotBuiltYet ||
+			status == PackageInRemoteCache ||
+			status == PackageBuilt ||
+			status == PackageDownloaded
+
+		if !depAvailable {
+			log.WithFields(log.Fields{
+				"package":    p.FullName(),
+				"dependency": dep.FullName(),
+				"depStatus":  status,
+				"inCache":    inCache,
+			}).Debug("Dependency not available for cached package")
+			return false
+		}
+	}
+	return true
+}
+
 type PackageBuildPhase string
 
 const (
diff --git a/pkg/leeway/build_validate_deps_test.go b/pkg/leeway/build_validate_deps_test.go
@@ -0,0 +1,251 @@
+package leeway
+
+import (
+	"testing"
+
+	"github.com/gitpod-io/leeway/pkg/leeway/cache"
+	"github.com/stretchr/testify/require"
+)
+
+// mockLocalCache implements cache.LocalCache for testing
+type mockLocalCache struct {
+	packages map[string]string // fullName -> path
+}
+
+func newMockLocalCache() *mockLocalCache {
+	return &mockLocalCache{
+		packages: make(map[string]string),
+	}
+}
+
+func (m *mockLocalCache) Location(pkg cache.Package) (string, bool) {
+	path, exists := m.packages[pkg.FullName()]
+	return path, exists
+}
+
+func (m *mockLocalCache) addPackage(fullName, path string) {
+	m.packages[fullName] = path
+}
+
+// newTestPackage creates a test package with the given name and type
+func newTestPackage(name string, pkgType PackageType) *Package {
+	pkg := &Package{
+		fullNameOverride: name,
+		dependencies:     []*Package{},
+	}
+	pkg.PackageInternal.Type = pkgType
+	return pkg
+}
+
+func TestValidateDependenciesAvailable(t *testing.T) {
+	tests := []struct {
+		name           string
+		setupPackages  func() (*Package, map[*Package]PackageBuildStatus, *mockLocalCache)
+		expectedResult bool
+	}{
+		{
+			name: "package with no dependencies",
+			setupPackages: func() (*Package, map[*Package]PackageBuildStatus, *mockLocalCache) {
+				pkg := newTestPackage("test:pkg", GenericPackage)
+				pkgstatus := map[*Package]PackageBuildStatus{
+					pkg: PackageDownloaded,
+				}
+				cache := newMockLocalCache()
+				cache.addPackage("test:pkg", "/cache/test-pkg.tar.gz")
+				return pkg, pkgstatus, cache
+			},
+			expectedResult: true,
+		},
+		{
+			name: "package with all dependencies in cache",
+			setupPackages: func() (*Package, map[*Package]PackageBuildStatus, *mockLocalCache) {
+				depA := newTestPackage("test:dep-a", GenericPackage)
+				pkg := newTestPackage("test:pkg", GenericPackage)
+				pkg.dependencies = []*Package{depA}
+
+				pkgstatus := map[*Package]PackageBuildStatus{
+					pkg:  PackageDownloaded,
+					depA: PackageBuilt,
+				}
+				cache := newMockLocalCache()
+				cache.addPackage("test:pkg", "/cache/test-pkg.tar.gz")
+				cache.addPackage("test:dep-a", "/cache/test-dep-a.tar.gz")
+				return pkg, pkgstatus, cache
+			},
+			expectedResult: true,
+		},
+		{
+			name: "package with dependency marked for build",
+			setupPackages: func() (*Package, map[*Package]PackageBuildStatus, *mockLocalCache) {
+				depA := newTestPackage("test:dep-a", GenericPackage)
+				pkg := newTestPackage("test:pkg", GenericPackage)
+				pkg.dependencies = []*Package{depA}
+
+				pkgstatus := map[*Package]PackageBuildStatus{
+					pkg:  PackageDownloaded,
+					depA: PackageNotBuiltYet, // Will be built
+				}
+				cache := newMockLocalCache()
+				cache.addPackage("test:pkg", "/cache/test-pkg.tar.gz")
+				// depA is NOT in cache but will be built
+				return pkg, pkgstatus, cache
+			},
+			expectedResult: true,
+		},
+		{
+			name: "package with dependency in remote cache",
+			setupPackages: func() (*Package, map[*Package]PackageBuildStatus, *mockLocalCache) {
+				depA := newTestPackage("test:dep-a", GenericPackage)
+				pkg := newTestPackage("test:pkg", GenericPackage)
+				pkg.dependencies = []*Package{depA}
+
+				pkgstatus := map[*Package]PackageBuildStatus{
+					pkg:  PackageDownloaded,
+					depA: PackageInRemoteCache, // Will be downloaded
+				}
+				cache := newMockLocalCache()
+				cache.addPackage("test:pkg", "/cache/test-pkg.tar.gz")
+				return pkg, pkgstatus, cache
+			},
+			expectedResult: true,
+		},
+		{
+			name: "package with missing dependency - not in cache, unknown status",
+			setupPackages: func() (*Package, map[*Package]PackageBuildStatus, *mockLocalCache) {
+				depA := newTestPackage("test:dep-a", GenericPackage)
+				pkg := newTestPackage("test:pkg", GenericPackage)
+				pkg.dependencies = []*Package{depA}
+
+				pkgstatus := map[*Package]PackageBuildStatus{
+					pkg: PackageDownloaded,
+					// depA has no status entry
+				}
+				cache := newMockLocalCache()
+				cache.addPackage("test:pkg", "/cache/test-pkg.tar.gz")
+				// depA is NOT in cache
+				return pkg, pkgstatus, cache
+			},
+			expectedResult: false,
+		},
+		{
+			name: "package with ephemeral dependency - should be skipped",
+			setupPackages: func() (*Package, map[*Package]PackageBuildStatus, *mockLocalCache) {
+				depA := newTestPackage("test:dep-a", GenericPackage)
+				depA.Ephemeral = true // Ephemeral packages are always rebuilt
+
+				pkg := newTestPackage("test:pkg", GenericPackage)
+				pkg.dependencies = []*Package{depA}
+
+				pkgstatus := map[*Package]PackageBuildStatus{
+					pkg: PackageDownloaded,
+					// depA has no status - but it's ephemeral so should be skipped
+				}
+				cache := newMockLocalCache()
+				cache.addPackage("test:pkg", "/cache/test-pkg.tar.gz")
+				return pkg, pkgstatus, cache
+			},
+			expectedResult: true,
+		},
+		{
+			name: "Go package with transitive dependency missing",
+			setupPackages: func() (*Package, map[*Package]PackageBuildStatus, *mockLocalCache) {
+				depB := newTestPackage("test:dep-b", GenericPackage)
+				depA := newTestPackage("test:dep-a", GenericPackage)
+				depA.dependencies = []*Package{depB}
+
+				pkg := newTestPackage("test:pkg", GoPackage) // Go packages need transitive deps
+				pkg.dependencies = []*Package{depA}
+
+				pkgstatus := map[*Package]PackageBuildStatus{
+					pkg:  PackageDownloaded,
+					depA: PackageBuilt,
+					// depB has no status - missing transitive dependency
+				}
+				cache := newMockLocalCache()
+				cache.addPackage("test:pkg", "/cache/test-pkg.tar.gz")
+				cache.addPackage("test:dep-a", "/cache/test-dep-a.tar.gz")
+				// depB is NOT in cache
+				return pkg, pkgstatus, cache
+			},
+			expectedResult: false,
+		},
+		{
+			name: "Go package with all transitive dependencies available",
+			setupPackages: func() (*Package, map[*Package]PackageBuildStatus, *mockLocalCache) {
+				depB := newTestPackage("test:dep-b", GenericPackage)
+				depA := newTestPackage("test:dep-a", GenericPackage)
+				depA.dependencies = []*Package{depB}
+
+				pkg := newTestPackage("test:pkg", GoPackage)
+				pkg.dependencies = []*Package{depA}
+
+				pkgstatus := map[*Package]PackageBuildStatus{
+					pkg:  PackageDownloaded,
+					depA: PackageBuilt,
+					depB: PackageBuilt,
+				}
+				cache := newMockLocalCache()
+				cache.addPackage("test:pkg", "/cache/test-pkg.tar.gz")
+				cache.addPackage("test:dep-a", "/cache/test-dep-a.tar.gz")
+				cache.addPackage("test:dep-b", "/cache/test-dep-b.tar.gz")
+				return pkg, pkgstatus, cache
+			},
+			expectedResult: true,
+		},
+		{
+			name: "Docker package only checks direct dependencies",
+			setupPackages: func() (*Package, map[*Package]PackageBuildStatus, *mockLocalCache) {
+				depB := newTestPackage("test:dep-b", GenericPackage)
+				depA := newTestPackage("test:dep-a", GenericPackage)
+				depA.dependencies = []*Package{depB}
+
+				pkg := newTestPackage("test:pkg", DockerPackage) // Docker only needs direct deps
+				pkg.dependencies = []*Package{depA}
+
+				pkgstatus := map[*Package]PackageBuildStatus{
+					pkg:  PackageDownloaded,
+					depA: PackageBuilt,
+					// depB has no status - but Docker doesn't need it
+				}
+				cache := newMockLocalCache()
+				cache.addPackage("test:pkg", "/cache/test-pkg.tar.gz")
+				cache.addPackage("test:dep-a", "/cache/test-dep-a.tar.gz")
+				// depB is NOT in cache - but Docker doesn't check transitive deps
+				return pkg, pkgstatus, cache
+			},
+			expectedResult: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			pkg, pkgstatus, cache := tt.setupPackages()
+			result := validateDependenciesAvailable(pkg, cache, pkgstatus)
+			require.Equal(t, tt.expectedResult, result)
+		})
+	}
+}
+
+func TestValidateDependenciesAvailable_YarnPackage(t *testing.T) {
+	// Yarn packages should check transitive dependencies like Go packages
+	depB := newTestPackage("test:dep-b", GenericPackage)
+	depA := newTestPackage("test:dep-a", GenericPackage)
+	depA.dependencies = []*Package{depB}
+
+	pkg := newTestPackage("test:pkg", YarnPackage)
+	pkg.dependencies = []*Package{depA}
+
+	pkgstatus := map[*Package]PackageBuildStatus{
+		pkg:  PackageDownloaded,
+		depA: PackageBuilt,
+		// depB missing
+	}
+
+	cache := newMockLocalCache()
+	cache.addPackage("test:pkg", "/cache/test-pkg.tar.gz")
+	cache.addPackage("test:dep-a", "/cache/test-dep-a.tar.gz")
+
+	// Should fail because Yarn needs transitive deps
+	result := validateDependenciesAvailable(pkg, cache, pkgstatus)
+	require.False(t, result, "Yarn package should fail validation when transitive dependency is missing")
+}
