fix(source preparation): Source Files Download Ordering (#501)

Author: Antonio Salinas

internal/app/azldev/cmds/component/build.go

@@ -181,7 +181,14 @@ func BuildComponent(
 	workDirFactory *workdir.Factory,
 	options *ComponentBuildOptions,
 ) (results ComponentBuildResults, err error) {
-	sourceManager, err := sourceproviders.NewSourceManager(env)
+	// Resolve the effective distro for this component before creating the source manager.
+	distro, err := sourceproviders.ResolveDistro(env, component)
+	if err != nil {
+		return ComponentBuildResults{},
+			fmt.Errorf("failed to resolve distro for component %q:\n%w", component.GetName(), err)
+	}
+
+	sourceManager, err := sourceproviders.NewSourceManager(env, distro)
 	if err != nil {
 		return ComponentBuildResults{},
 			fmt.Errorf("failed to setup source retrieval manager for component %q:\n%w", component.GetName(), err)

internal/app/azldev/cmds/component/diffsources.go

@@ -86,7 +86,13 @@ func DiffComponentSources(env *azldev.Env, options *DiffSourcesOptions) (interfa
 	event := env.StartEvent("Diffing sources", "component", component.GetName())
 	defer event.End()
 
-	sourceManager, err := sourceproviders.NewSourceManager(env)
+	// Resolve the effective distro for this component before creating the source manager.
+	distro, err := sourceproviders.ResolveDistro(env, component)
+	if err != nil {
+		return nil, fmt.Errorf("failed to resolve distro for component %q:\n%w", component.GetName(), err)
+	}
+
+	sourceManager, err := sourceproviders.NewSourceManager(env, distro)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create source manager:\n%w", err)
 	}

internal/app/azldev/cmds/component/preparesources.go

@@ -97,8 +97,14 @@ func PrepareComponentSources(env *azldev.Env, options *PrepareSourcesOptions) er
 
 	var sourceManager sourceproviders.SourceManager
 
+	// Resolve the effective distro for this component before creating the source manager.
+	distro, err := sourceproviders.ResolveDistro(env, component)
+	if err != nil {
+		return fmt.Errorf("failed to resolve distro for component %#q:\n%w", component.GetName(), err)
+	}
+
 	// Create source manager to handle all source fetching, both local and upstream.
-	sourceManager, err = sourceproviders.NewSourceManager(env)
+	sourceManager, err = sourceproviders.NewSourceManager(env, distro)
 	if err != nil {
 		return fmt.Errorf("failed to create source manager:\n%w", err)
 	}

internal/projectconfig/component.go

@@ -38,6 +38,7 @@ const (
 )
 
 // Origin describes where a source file comes from and how to retrieve it.
+// When omitted from a source file reference, the file will be resolved via the lookaside cache.
 type Origin struct {
 	// Type indicates how the source file should be acquired.
 	Type OriginType `toml:"type" json:"type" jsonschema:"required,enum=download,title=Origin type,description=Type of origin for this source file"`
@@ -59,8 +60,8 @@ type SourceFileReference struct {
 	// Type of hash used by Hash (e.g., "sha256", "sha512").
 	HashType fileutils.HashType `toml:"hash-type,omitempty" json:"hashType,omitempty"`
 
-	// Type of origin for this source file (e.g., URI, custom).
-	Origin Origin `toml:"origin" json:"origin"`
+	// Origin for this source file. When omitted, the file is resolved via the lookaside cache.
+	Origin Origin `toml:"origin,omitempty" json:"origin,omitempty"`
 }
 
 // Defines a component group. Component groups are logical groupings of components (see [ComponentConfig]).

internal/projectconfig/distro.go

@@ -53,6 +53,9 @@ type DistroDefinition struct {
 	// Published artifact information
 	PackageRepositories []PackageRepository `toml:"repos,omitempty" json:"repos,omitempty" jsonschema:"title=Package Repositories,description=List of package repository definitions"`
 
+	// When true, source file downloads will not fall back to configured origins if the lookaside cache fails.
+	DisableOrigins bool `toml:"disable-origins,omitempty" json:"disableOrigins,omitempty" jsonschema:"title=Disable origins,description=When true only allow source files from the lookaside cache and do not fall back to configured origins"`
+
 	// Versions: maps version => definition
 	Versions map[string]DistroVersionDefinition `toml:"versions,omitempty" json:"versions,omitempty" jsonschema:"title=Versions,description=Mapping of distro version definitions"`
 }

internal/providers/sourceproviders/fedorasource/fedorasource.go

@@ -22,8 +22,12 @@ import (
 
 type FedoraSourceDownloader interface {
 	// ExtractSourcesFromRepo processes a git repository by downloading any required
-	// lookaside cache files into the repository directory.
-	ExtractSourcesFromRepo(ctx context.Context, repoDir string, packageName string, lookasideBaseURI string) error
+	// lookaside cache files into the repository directory. Files whose names appear
+	// in skipFilenames are not downloaded (e.g., files already fetched separately).
+	ExtractSourcesFromRepo(
+		ctx context.Context, repoDir string, packageName string,
+		lookasideBaseURI string, skipFilenames []string,
+	) error
 }
 
 // FedoraSourceDownloaderImpl is an implementation of GitRepoExtractor.
@@ -107,7 +111,7 @@ func NewFedoraRepoExtractorImpl(
 // ExtractSourcesFromRepo processes the git repository by downloading any required
 // lookaside cache files into the repository directory.
 func (g *FedoraSourceDownloaderImpl) ExtractSourcesFromRepo(
-	ctx context.Context, repoDir string, packageName string, lookasideBaseURI string,
+	ctx context.Context, repoDir string, packageName string, lookasideBaseURI string, skipFileNames []string,
 ) error {
 	if repoDir == "" {
 		return errors.New("repository directory cannot be empty")
@@ -117,10 +121,6 @@ func (g *FedoraSourceDownloaderImpl) ExtractSourcesFromRepo(
 		return errors.New("lookaside base URI cannot be empty")
 	}
 
-	if err := verifyFedoraLookasideBaseURI(lookasideBaseURI); err != nil {
-		return err
-	}
-
 	repoDirExists, err := fileutils.Exists(g.fileSystem, repoDir)
 	if err != nil {
 		return fmt.Errorf("failed to check if repository directory exists at %#q:\n%w", repoDir, err)
@@ -152,7 +152,12 @@ func (g *FedoraSourceDownloaderImpl) ExtractSourcesFromRepo(
 		return fmt.Errorf("failed to parse sources file at %#q:\n%w", sourcesFilePath, err)
 	}
 
-	err = g.downloadAndVerifySources(ctx, sourceFiles, repoDir)
+	skipSet := make(map[string]bool, len(skipFileNames))
+	for _, name := range skipFileNames {
+		skipSet[name] = true
+	}
+
+	err = g.downloadAndVerifySources(ctx, sourceFiles, repoDir, skipSet)
 	if err != nil {
 		return fmt.Errorf("failed to download sources:\n%w", err)
 	}
@@ -164,9 +169,17 @@ func (g *FedoraSourceDownloaderImpl) downloadAndVerifySources(
 	ctx context.Context,
 	sourceFiles []sourceFileInfo,
 	repoDir string,
+	skipSet map[string]bool,
 ) error {
 	sourcesTotal := len(sourceFiles)
 	for sourceIndex, sourceFile := range sourceFiles {
+		if skipSet[sourceFile.fileName] {
+			slog.Debug("File already provided, skipping lookaside download",
+				"fileName", sourceFile.fileName)
+
+			continue
+		}
+
 		destFilePath := filepath.Join(repoDir, sourceFile.fileName)
 
 		exists, err := fileutils.Exists(g.fileSystem, destFilePath)
@@ -268,11 +281,10 @@ func parseSourcesFile(content string, packageName string, lookasideBaseURI strin
 			hashType = "MD5"
 		}
 
-		sourceURI := lookasideBaseURI
-		sourceURI = strings.ReplaceAll(sourceURI, "$pkg", packageName)
-		sourceURI = strings.ReplaceAll(sourceURI, "$filename", fileName)
-		sourceURI = strings.ReplaceAll(sourceURI, "$hashtype", strings.ToLower(hashType))
-		sourceURI = strings.ReplaceAll(sourceURI, "$hash", hash)
+		sourceURI, err := BuildLookasideURL(lookasideBaseURI, packageName, fileName, hashType, hash)
+		if err != nil {
+			return nil, fmt.Errorf("failed to build lookaside URL at line %d:\n%w", lineNum+1, err)
+		}
 
 		sourceFiles = append(sourceFiles, sourceFileInfo{
 			fileName:     fileName,
@@ -285,14 +297,42 @@ func parseSourcesFile(content string, packageName string, lookasideBaseURI strin
 	return sourceFiles, nil
 }
 
-func verifyFedoraLookasideBaseURI(lookasideBaseURI string) error {
-	// Check for placeholder variables in the lookaside URI
-	requiredPlaceholders := []string{"$pkg", "$filename", "$hashtype", "$hash"}
-	for _, placeholder := range requiredPlaceholders {
-		if !strings.Contains(lookasideBaseURI, placeholder) {
-			return fmt.Errorf("lookaside base URI is missing required placeholder: %s", placeholder)
+// Lookaside URI template placeholders supported by [BuildLookasideURL].
+const (
+	// PlaceholderPkg is replaced with the package name.
+	PlaceholderPkg = "$pkg"
+	// PlaceholderFilename is replaced with the source file name.
+	PlaceholderFilename = "$filename"
+	// PlaceholderHashType is replaced with the lowercase hash algorithm (e.g., "sha512").
+	PlaceholderHashType = "$hashtype"
+	// PlaceholderHash is replaced with the hash value.
+	PlaceholderHash = "$hash"
+)
+
+// BuildLookasideURL constructs a lookaside cache URL by substituting placeholders in the
+// URI template with the provided values. Supported placeholders are [PlaceholderPkg],
+// [PlaceholderFilename], [PlaceholderHashType], and [PlaceholderHash].
+// Placeholders not present in the template are simply ignored.
+//
+// Returns an error if any of the provided values contain a placeholder string, as this
+// would cause ambiguous substitution results depending on replacement order.
+func BuildLookasideURL(template, packageName, fileName, hashType, hash string) (string, error) {
+	// allPlaceholders lists all supported lookaside URI template placeholders.
+	allPlaceholders := []string{PlaceholderPkg, PlaceholderFilename, PlaceholderHashType, PlaceholderHash}
+
+	for _, v := range []string{packageName, fileName, hashType, hash} {
+		for _, p := range allPlaceholders {
+			if strings.Contains(v, p) {
+				return "", fmt.Errorf("value %#q contains placeholder %s, which would cause ambiguous substitution", v, p)
+			}
 		}
 	}
 
-	return nil
+	uri := template
+	uri = strings.ReplaceAll(uri, PlaceholderPkg, packageName)
+	uri = strings.ReplaceAll(uri, PlaceholderFilename, fileName)
+	uri = strings.ReplaceAll(uri, PlaceholderHashType, strings.ToLower(hashType))
+	uri = strings.ReplaceAll(uri, PlaceholderHash, hash)
+
+	return uri, nil
 }

internal/providers/sourceproviders/fedorasource/fedorasource_test.go

@@ -110,7 +110,7 @@ func TestExtractSourcesFromRepo(t *testing.T) {
 		return afero.WriteFile(ctx.FS(), destPath, []byte("test patch content"), testFilePerms)
 	})
 
-	err = extractor.ExtractSourcesFromRepo(context.Background(), testRepoDir, testPackageName, testLookasideURI)
+	err = extractor.ExtractSourcesFromRepo(context.Background(), testRepoDir, testPackageName, testLookasideURI, nil)
 	require.NoError(t, err)
 }
 
@@ -123,13 +123,13 @@ func TestExtractSourcesFromRepoValidation(t *testing.T) {
 	require.NoError(t, err)
 
 	t.Run("empty repo dir", func(t *testing.T) {
-		err := extractor.ExtractSourcesFromRepo(context.Background(), "", testPackageName, testLookasideURI)
+		err := extractor.ExtractSourcesFromRepo(context.Background(), "", testPackageName, testLookasideURI, nil)
 		require.Error(t, err)
 		assert.Contains(t, err.Error(), "repository directory cannot be empty")
 	})
 
 	t.Run("empty lookaside URI", func(t *testing.T) {
-		err := extractor.ExtractSourcesFromRepo(context.Background(), testRepoDir, testPackageName, "")
+		err := extractor.ExtractSourcesFromRepo(context.Background(), testRepoDir, testPackageName, "", nil)
 		require.Error(t, err)
 		assert.Contains(t, err.Error(), "lookaside base URI cannot be empty")
 	})
@@ -138,7 +138,9 @@ func TestExtractSourcesFromRepoValidation(t *testing.T) {
 		require.NoError(t, ctx.FS().MkdirAll(testEmptyRepoDir, 0o755))
 
 		// Missing sources file is valid - it means no external sources to download
-		err := extractor.ExtractSourcesFromRepo(context.Background(), testEmptyRepoDir, testPackageName, testLookasideURI)
+		err := extractor.ExtractSourcesFromRepo(
+			context.Background(), testEmptyRepoDir, testPackageName, testLookasideURI, nil,
+		)
 		require.NoError(t, err)
 	})
 }
@@ -158,7 +160,7 @@ func TestExtractSourcesFromRepoDownloadFailure(t *testing.T) {
 	mockDownloader.EXPECT().Download(gomock.Any(), gomock.Any(), gomock.Any()).
 		Return(downloadErr)
 
-	err = extractor.ExtractSourcesFromRepo(context.Background(), testRepoDir, testPackageName, testLookasideURI)
+	err = extractor.ExtractSourcesFromRepo(context.Background(), testRepoDir, testPackageName, testLookasideURI, nil)
 	require.Error(t, err)
 	require.ErrorIs(t, err, downloadErr)
 	assert.Contains(t, err.Error(), "failed to download sources")
@@ -182,7 +184,7 @@ func TestExtractSourcesFromRepoHashMismatch(t *testing.T) {
 			return afero.WriteFile(ctx.FS(), destPath, []byte("wrong content"), testFilePerms)
 		})
 
-	err = extractor.ExtractSourcesFromRepo(context.Background(), testRepoDir, testPackageName, testLookasideURI)
+	err = extractor.ExtractSourcesFromRepo(context.Background(), testRepoDir, testPackageName, testLookasideURI, nil)
 	require.Error(t, err)
 	assert.Contains(t, err.Error(), "hash mismatch")
 }
@@ -267,28 +269,75 @@ func TestParseSourcesFile(t *testing.T) {
 	})
 }
 
-func TestVerifyFedoraLookasideBaseURI(t *testing.T) {
-	t.Run("valid URI with all placeholders", func(t *testing.T) {
-		err := verifyFedoraLookasideBaseURI("https://example.com/$hashtype/$hash/$pkg/$filename")
-		require.NoError(t, err)
-	})
-
-	t.Run("missing placeholder returns error", func(t *testing.T) {
-		tests := []struct {
-			uri     string
-			missing string
-		}{
-			{"https://example.com/$hash/$pkg/$filename", "$hashtype"},
-			{"https://example.com/$hashtype/$hash/$filename", "$pkg"},
-			{"https://example.com/$hashtype/$hash/$pkg/", "$filename"},
-		}
-
-		for _, tc := range tests {
-			t.Run(tc.missing, func(t *testing.T) {
-				err := verifyFedoraLookasideBaseURI(tc.uri)
-				require.Error(t, err, "URI %q should fail for missing %s", tc.uri, tc.missing)
-				assert.Contains(t, err.Error(), tc.missing)
-			})
-		}
-	})
+func TestBuildLookasideURL(t *testing.T) {
+	tests := []struct {
+		name          string
+		template      string
+		pkg           string
+		filename      string
+		hashType      string
+		hash          string
+		expected      string
+		expectedError string
+	}{
+		{
+			name:     "standard template",
+			template: "https://example.com/repo/pkgs/$pkg/$filename/$hashtype/$hash/$filename",
+			pkg:      "my-pkg",
+			filename: "source.tar.gz",
+			hashType: "SHA512",
+			hash:     "abc123",
+			expected: "https://example.com/repo/pkgs/my-pkg/source.tar.gz/sha512/abc123/source.tar.gz",
+		},
+		{
+			name:     "different placeholder order",
+			template: "https://example.com/$hashtype/$hash/$pkg/$filename",
+			pkg:      "test-pkg",
+			filename: "file.tar.gz",
+			hashType: "SHA256",
+			hash:     "def456",
+			expected: "https://example.com/sha256/def456/test-pkg/file.tar.gz",
+		},
+		{
+			name:     "template without filename placeholder",
+			template: "https://example.com/$pkg/$hashtype/$hash",
+			pkg:      "my-pkg",
+			filename: "source.tar.gz",
+			hashType: "SHA512",
+			hash:     "abc123",
+			expected: "https://example.com/my-pkg/sha512/abc123",
+		},
+		{
+			name:          "packageName containing placeholder",
+			template:      "https://example.com/$pkg/$filename/$hashtype/$hash",
+			pkg:           "evil-$filename-pkg",
+			filename:      "source.tar.gz",
+			hashType:      "SHA512",
+			hash:          "abc123",
+			expectedError: "ambiguous substitution",
+		},
+		{
+			name:          "fileName containing placeholder",
+			template:      "https://example.com/$pkg/$filename/$hashtype/$hash",
+			pkg:           "my-pkg",
+			filename:      "$hash-source.tar.gz",
+			hashType:      "SHA512",
+			hash:          "abc123",
+			expectedError: "ambiguous substitution",
+		},
+	}
+
+	for _, testCase := range tests {
+		t.Run(testCase.name, func(t *testing.T) {
+			result, err := BuildLookasideURL(
+				testCase.template, testCase.pkg, testCase.filename, testCase.hashType, testCase.hash,
+			)
+			if testCase.expectedError != "" {
+				assert.ErrorContains(t, err, testCase.expectedError)
+			} else {
+				require.NoError(t, err)
+				assert.Equal(t, testCase.expected, result)
+			}
+		})
+	}
 }