Merge pull request #332 from gitpod-io/wv/fix-cached-package-dependency-validation

fix: validate all transitive dependencies for cached packages

Author: WVerlaek

pkg/leeway/build.go

@@ -1470,17 +1470,19 @@ func (p *Package) packagesToDownload(inLocalCache map[*Package]struct{}, inRemot
 // 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()
-	}
+	// Always check ALL transitive dependencies for cached packages.
+	// This is necessary because a cached package might be used by a Go/Yarn package
+	// that needs all transitive dependencies available. If we only check direct
+	// dependencies for Generic/Docker packages, a Go package consuming them would
+	// fail during prep when it tries to access a missing transitive dependency.
+	//
+	// Example: GoPackage X -> GenericPackage A -> DockerPackage B
+	// If A is cached but B is not, and we only check A's direct deps (B),
+	// we'd correctly invalidate A. But if the chain is:
+	// GoPackage X -> GenericPackage A -> GenericPackage B -> DockerPackage C
+	// And A is cached, B is cached, but C is not, we need to check transitively
+	// to ensure C is available, otherwise X's build will fail.
+	deps := p.GetTransitiveDependencies()
 
 	for _, dep := range deps {
 		if dep.Ephemeral {

pkg/leeway/build_validate_deps_test.go

@@ -216,24 +216,48 @@ func TestValidateDependenciesAvailable(t *testing.T) {
 			expectedResult: true,
 		},
 		{
-			name: "Docker package only checks direct dependencies",
+			name: "Docker package checks all transitive 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 := newTestPackage("test:pkg", DockerPackage)
 				pkg.dependencies = []*Package{depA}
 
 				pkgstatus := map[*Package]PackageBuildStatus{
 					pkg:  PackageDownloaded,
 					depA: PackageBuilt,
-					// depB has no status - but Docker doesn't need it
+					// depB has no status - validation should fail because
+					// cached packages need all transitive deps available
 				}
 				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
+				// depB is NOT in cache - validation should fail
+				return pkg, pkgstatus, cache
+			},
+			expectedResult: false, // Changed: now checks transitive deps
+		},
+		{
+			name: "Docker 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", DockerPackage)
+				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,