From f64d4b54f3314f505eccc6277778f8a70dac406a Mon Sep 17 00:00:00 2001 From: Antonio Salinas Date: Wed, 8 Jul 2026 22:00:30 +0000 Subject: [PATCH 1/5] feat(overlays): add archive overlay engine and config model Add the archive extract/apply/repack engine plus the ComponentOverlay 'archive' field, validation, and fingerprint handling. Extends internal/utils/archive with compression sniffing, ExtractAuto, and strict unsupported-entry handling. Pipeline wiring and hash tracking follow in a subsequent change. --- internal/projectconfig/fingerprint_test.go | 25 +++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/internal/projectconfig/fingerprint_test.go b/internal/projectconfig/fingerprint_test.go index 5301e6ab..95d860d7 100644 --- a/internal/projectconfig/fingerprint_test.go +++ b/internal/projectconfig/fingerprint_test.go @@ -5,6 +5,7 @@ package projectconfig_test import ( "reflect" + "strings" "testing" "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" @@ -113,15 +114,29 @@ func TestAllFingerprintedFieldsHaveDecision(t *testing.T) { tag := field.Tag.Get("fingerprint") - switch tag { + // hashstructure tags are `name,option,...`; the name part decides + // inclusion ("-" excludes, anything else includes) and the options + // tune how an included field is hashed. + name, options, _ := strings.Cut(tag, ",") + + switch name { case "": - // No tag — included by default (the safe default). + // Empty name — included by default (the safe default). The only + // option we permit is `omitempty`, which makes hashstructure skip + // the field when it holds its zero value (so an unset field never + // perturbs the hash) while still hashing it when set. Reject any + // other option as a likely typo. + if options != "" && options != "omitempty" { + assert.Failf(t, "invalid fingerprint tag", + "field %q has unrecognised fingerprint tag option %q — "+ + "only `omitempty` is supported on included fields", key, options) + } case "-": actualExclusions[key] = true default: - // hashstructure only recognises "" (include) and "-" (exclude). - // Any other value is silently treated as included, which is - // almost certainly a typo. + // hashstructure only recognises "" (include) and "-" (exclude) + // for the name part. Any other value is silently treated as + // included, which is almost certainly a typo. assert.Failf(t, "invalid fingerprint tag", "field %q has unrecognised fingerprint tag value %q — "+ "only `fingerprint:\"-\"` (exclude) is valid; "+ From 7eaaf6a574eb37dde7cee188ee128c25824528d1 Mon Sep 17 00:00:00 2001 From: Antonio Salinas Date: Wed, 8 Jul 2026 22:02:29 +0000 Subject: [PATCH 2/5] feat(source-files): wire archive overlays into prep-sources with post-overlay hash tracking Apply archive overlays in the source-prep pipeline, rehash repacked archives in the 'sources' file, and add the 'overlay' source-file origin type that records and validates the expected post-overlay hash. --- docs/user/reference/config/components.md | 38 +- .../azldev/core/sources/archiveoverlays.go | 82 ++++ .../sources/archiveoverlays_internal_test.go | 54 +++ .../core/sources/archiveoverlays_test.go | 443 ++++++++++++++++++ .../app/azldev/core/sources/sourceprep.go | 292 ++++++++++-- .../azldev/core/sources/sourceprep_test.go | 176 ++++++- internal/projectconfig/component.go | 15 +- internal/projectconfig/configfile.go | 44 ++ .../sourceproviders/fedorasourceprovider.go | 13 +- .../sourceproviders/sourcemanager.go | 83 +++- ...ainer_config_generate-schema_stdout_1.snap | 3 +- ...shots_config_generate-schema_stdout_1.snap | 3 +- schemas/azldev.schema.json | 3 +- 13 files changed, 1173 insertions(+), 76 deletions(-) create mode 100644 internal/app/azldev/core/sources/archiveoverlays_test.go diff --git a/docs/user/reference/config/components.md b/docs/user/reference/config/components.md index 67866951..029e131b 100644 --- a/docs/user/reference/config/components.md +++ b/docs/user/reference/config/components.md @@ -315,7 +315,7 @@ The `[[components..source-files]]` array defines additional source files t ### Origin -Two origin types are supported. +Three origin types are supported. #### `"download"` — fetch from a URI @@ -356,7 +356,41 @@ origin.mock-packages = ["cmake"] # omit if not needed origin.inputs = ["yara-4.5.4.tar.gz"] # available to the script as ./yara-4.5.4.tar.gz ``` -> **Note:** Upstream source tarballs are automatically available as inputs before running custom generation scripts. There is no need to re-declare an upstream file in `source-files` to use it as an input. +**Note:** Upstream source tarballs are automatically available as inputs before running custom generation scripts. There is no need to re-declare an upstream file in `source-files` to use it as an input. + +#### `"overlay"` — record a post-overlay hash + +- **`hash` and `hash-type` are required** for `"overlay"` entries — the hash cannot be computed at render time because sources are not downloaded. +- **`replace-upstream = true` is required** — the archive already exists in the upstream `sources` file, and this entry replaces its hash with the post-overlay value. +- During `prep-sources` (full run), azldev verifies that the hash it computed after repacking the archive matches the stated `hash`. A mismatch means the config is stale and must be updated. +- During `render --check-only`, the stated hash is injected directly without downloading or repacking, allowing the check to pass deterministically. + +See [Recording the post-overlay hash for archive overlays](#recording-the-post-overlay-hash-for-archive-overlays) below for the full workflow. + +### Recording the post-overlay hash for archive overlays + +When you apply archive overlays (e.g. removing vendored files from a tarball) using `file-remove` or `file-search-replace` with an archive-scoped path, the repacked archive has a different hash than the original. Use a `source-files` entry with `origin.type = "overlay"` to record the expected post-overlay hash: + +```toml +[[components.apache-commons-compress.source-files]] +filename = "commons-compress-1.27.1-src.tar.gz" +hash = "c7a2cef26959e687ad19b96b5ba8393d7514095e13bf0f29bd41e6b3c3cb2260d8ff23283ff3d5fd137b2522b843e7f0f50ab46bcf0f66df5383674f35f223ab" +hash-type = "SHA512" +origin = { type = "overlay" } +replace-upstream = true +replace-reason = "Upstream source tarball contains test fixtures flagged as malware by the AZL RPM signing pipeline. These files are not needed at runtime and are removed to allow SRPM publication." +``` + +**Workflow:** + +1. Add the archive overlay(s) in the component's `[[overlays]]` array. +2. Run `prep-sources` once — this repacks the archive and prints the computed hash in the error message. +3. Paste the computed `hash` and `hash-type` into the `source-files` entry above. +4. Run `prep-sources` again to confirm the hash matches, then commit. + +After that, `render --check-only` will pass deterministically without downloading or repacking the archive. + +`replace-upstream = true` and `replace-reason` are required because the archive is already in the upstream `sources` file. The entry replaces its hash with the post-overlay value, regardless of how many overlays target that archive. ### Replacing an upstream `sources` entry diff --git a/internal/app/azldev/core/sources/archiveoverlays.go b/internal/app/azldev/core/sources/archiveoverlays.go index a3a78121..1da2a668 100644 --- a/internal/app/azldev/core/sources/archiveoverlays.go +++ b/internal/app/azldev/core/sources/archiveoverlays.go @@ -15,6 +15,88 @@ import ( "github.com/microsoft/azure-linux-dev-tools/internal/utils/rootfs" ) +// applyArchiveOverlays groups archive overlays by target archive and processes +// them in order. Multiple overlays targeting the same archive are batched into +// a single extract/modify/repack cycle. File removals inside the archive reuse +// the same machinery as loose-file overlays ([applyNonSpecOverlay]). +// +// It returns the names of the archives that were actually repacked. In dry-run +// mode no archive is repacked, so the returned slice is empty even when archive +// overlays were present. +func applyArchiveOverlays( + dryRunnable opctx.DryRunnable, + eventListener opctx.EventListener, + sourcesDirPath string, + overlays []projectconfig.ComponentOverlay, +) ([]string, error) { + groups := groupOverlaysByArchive(overlays) + + if len(groups) == 0 { + return nil, nil + } + + operationCount := 0 + for _, group := range groups { + operationCount += len(group.overlays) + } + + event := eventListener.StartEvent("Applying archive overlays", + "archives", len(groups), + "operations", operationCount, + ) + defer event.End() + + var repacked []string + + for _, group := range groups { + didRepack, err := processArchive(dryRunnable, sourcesDirPath, group.archive, group.overlays) + if err != nil { + return nil, fmt.Errorf("archive overlay failed for %#q:\n%w", group.archive, err) + } + + if didRepack { + repacked = append(repacked, group.archive) + } + } + + return repacked, nil +} + +// archiveGroup holds overlays targeting the same archive, preserving order. +type archiveGroup struct { + archive string + overlays []projectconfig.ComponentOverlay +} + +// groupOverlaysByArchive groups archive overlays by [projectconfig.ComponentOverlay.Archive], +// preserving insertion order within each group and across groups. +// Non-archive overlays are silently skipped. +func groupOverlaysByArchive(overlays []projectconfig.ComponentOverlay) []archiveGroup { + orderMap := make(map[string]int) + + var groups []archiveGroup + + for _, overlay := range overlays { + if !overlay.ModifiesArchive() { + continue + } + + archiveName := overlay.Archive + + idx, exists := orderMap[archiveName] + if !exists { + idx = len(groups) + orderMap[archiveName] = idx + + groups = append(groups, archiveGroup{archive: archiveName}) + } + + groups[idx].overlays = append(groups[idx].overlays, overlay) + } + + return groups +} + // processArchive extracts an archive to a temp directory, applies all overlays, // and deterministically repacks it with the original compression, atomically // replacing the original via a temp file + rename. It returns true when the diff --git a/internal/app/azldev/core/sources/archiveoverlays_internal_test.go b/internal/app/azldev/core/sources/archiveoverlays_internal_test.go index 8c8c4dd7..5672e60c 100644 --- a/internal/app/azldev/core/sources/archiveoverlays_internal_test.go +++ b/internal/app/azldev/core/sources/archiveoverlays_internal_test.go @@ -19,6 +19,60 @@ import ( "github.com/stretchr/testify/require" ) +func TestGroupOverlaysByArchive(t *testing.T) { + t.Run("groups overlays by archive name preserving order", func(t *testing.T) { + overlays := []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: "pkg-1.0.tar.gz", + Filename: "unwanted.conf", + }, + { + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: "pkg-1.0.tar.gz", + Filename: "config.h", + }, + { + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: "other-2.0.tar.xz", + Filename: "docs/*.md", + }, + } + + groups := groupOverlaysByArchive(overlays) + + require.Len(t, groups, 2) + + assert.Equal(t, "pkg-1.0.tar.gz", groups[0].archive) + require.Len(t, groups[0].overlays, 2) + assert.Equal(t, "unwanted.conf", groups[0].overlays[0].Filename) + assert.Equal(t, "config.h", groups[0].overlays[1].Filename) + + assert.Equal(t, "other-2.0.tar.xz", groups[1].archive) + require.Len(t, groups[1].overlays, 1) + assert.Equal(t, "docs/*.md", groups[1].overlays[0].Filename) + }) + + t.Run("skips overlays that are not archive-scoped", func(t *testing.T) { + overlays := []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlaySetSpecTag, Tag: "Version", Value: "1.0"}, + {Type: projectconfig.ComponentOverlayRemoveFile, Archive: "pkg.tar.gz", Filename: "f"}, + // Plain (non-archive) file overlay: no archive field, so it must be skipped. + {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "loose.txt"}, + // Bare archive name with no archive field: a loose removal of the archive itself. + {Type: projectconfig.ComponentOverlayRemoveFile, Filename: "drop-me.tar.gz"}, + {Type: projectconfig.ComponentOverlayAddFile, Filename: "new.txt", Source: "src"}, + } + + groups := groupOverlaysByArchive(overlays) + + require.Len(t, groups, 1) + assert.Equal(t, "pkg.tar.gz", groups[0].archive) + require.Len(t, groups[0].overlays, 1) + assert.Equal(t, "f", groups[0].overlays[0].Filename) + }) +} + // TestProcessArchive_DryRunDoesNotModifyArchive verifies that, in dry-run mode, // processArchive skips the extract/repack cycle entirely and leaves the original // archive on disk byte-for-byte unchanged (repacking would otherwise rewrite it). diff --git a/internal/app/azldev/core/sources/archiveoverlays_test.go b/internal/app/azldev/core/sources/archiveoverlays_test.go new file mode 100644 index 00000000..85fc1dc8 --- /dev/null +++ b/internal/app/azldev/core/sources/archiveoverlays_test.go @@ -0,0 +1,443 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package sources_test + +import ( + "os" + "path/filepath" + "sort" + "testing" + + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/components/components_testutils" + "github.com/microsoft/azure-linux-dev-tools/internal/app/azldev/core/sources" + "github.com/microsoft/azure-linux-dev-tools/internal/global/testctx" + "github.com/microsoft/azure-linux-dev-tools/internal/projectconfig" + "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders" + "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders/fedorasource" + "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders/sourceproviders_test" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/archive" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" +) + +// globRemovalTree returns the source tree shared by the file-remove glob tests. +// Every entry is a regular file (the glob matcher used by file-remove is +// files-only). +func globRemovalTree() []string { + return []string{ + "keep.txt", + "some_folder/a.txt", + "some_folder/b.txt", + "some_folder/sub/c.txt", + "some_folder/sub/deep/d.txt", + "top1/nested/some_file.txt", + "top1/some_file.txt", + "top2/some_file.txt", + } +} + +// globRemovalCase describes the expected outcome of applying a file-remove +// overlay with a given glob pattern against [globRemovalTree]. +type globRemovalCase struct { + name string + // pattern is the in-archive / in-sources glob (no archive prefix). + pattern string + // wantErr is true when the pattern matches no files and the overlay errors. + wantErr bool + // remaining is the sorted set of regular files left after removal (equal to + // the whole tree when wantErr is true, since nothing is removed). + remaining []string +} + +// globRemovalCases enumerates the documented file-remove glob behaviors. The +// expectations were captured against the live doublestar matcher and encode two +// behaviors worth pinning: (1) the matcher is files-only, so a bare directory +// name matches nothing, and (2) `*` matches a single path segment while `**` +// matches any depth. Directories are never removed (only files), so emptied +// directories survive a removal; these assertions look at regular files only. +func globRemovalCases() []globRemovalCase { + return []globRemovalCase{ + { + name: "bare folder name matches nothing (files-only matcher)", + pattern: "some_folder", + wantErr: true, + remaining: globRemovalTree(), + }, + { + name: "single star removes immediate file children only", + pattern: "some_folder/*", + remaining: []string{ + "keep.txt", + "some_folder/sub/c.txt", + "some_folder/sub/deep/d.txt", + "top1/nested/some_file.txt", + "top1/some_file.txt", + "top2/some_file.txt", + }, + }, + { + name: "doublestar removes all files under the folder recursively", + pattern: "some_folder/**", + remaining: []string{ + "keep.txt", + "top1/nested/some_file.txt", + "top1/some_file.txt", + "top2/some_file.txt", + }, + }, + { + name: "single-star prefix matches only top-level folders", + pattern: "*/some_file.txt", + remaining: []string{ + "keep.txt", + "some_folder/a.txt", + "some_folder/b.txt", + "some_folder/sub/c.txt", + "some_folder/sub/deep/d.txt", + "top1/nested/some_file.txt", + }, + }, + { + name: "doublestar prefix matches files at any depth", + pattern: "**/some_file.txt", + remaining: []string{ + "keep.txt", + "some_folder/a.txt", + "some_folder/b.txt", + "some_folder/sub/c.txt", + "some_folder/sub/deep/d.txt", + }, + }, + } +} + +// writeTreeFiles materializes the given slash-separated relative file paths +// under root, each with placeholder content. +func writeTreeFiles(t *testing.T, root string, files []string) { + t.Helper() + + for _, f := range files { + full := filepath.Join(root, filepath.FromSlash(f)) + require.NoError(t, os.MkdirAll(filepath.Dir(full), fileperms.PublicDir)) + require.NoError(t, os.WriteFile(full, []byte("x"), fileperms.PrivateFile)) + } +} + +// collectRegularFiles returns the sorted, slash-separated relative paths of all +// regular files under root (directories are ignored). +func collectRegularFiles(t *testing.T, root string) []string { + t.Helper() + + var out []string + + require.NoError(t, filepath.WalkDir(root, func(path string, d os.DirEntry, err error) error { + if err != nil { + return err + } + + if d.IsDir() { + return nil + } + + rel, relErr := filepath.Rel(root, path) + if relErr != nil { + return relErr + } + + out = append(out, filepath.ToSlash(rel)) + + return nil + })) + + sort.Strings(out) + + return out +} + +// TestPrepareSources_RemoveFileGlob_Archive exercises archive-scoped file-remove +// globs through the real exported [sources.SourcePreparer.PrepareSources] path +// (which drives the extract/remove/repack cycle), asserting which files survive +// inside the repacked archive for each glob pattern. +func TestPrepareSources_RemoveFileGlob_Archive(t *testing.T) { + const ( + componentName = "test-component" + archiveName = "pkg.tar.gz" + ) + + for _, testCase := range globRemovalCases() { + t.Run(testCase.name, func(t *testing.T) { + // Host FS + real temp dir: archive extraction/repacking happens on disk. + ctx := testctx.NewCtx(testctx.WithHostFS()) + outputDir := t.TempDir() + archivePath := filepath.Join(outputDir, archiveName) + + // Stage a tree with multiple top-level entries so the extraction root + // is the archive root and the glob is matched relative to it. + staging := t.TempDir() + writeTreeFiles(t, staging, globRemovalTree()) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, staging, archive.CompressionGzip)) + + // Seed a 'sources' entry for the archive so the post-overlay rehash + // has an entry to update (a missing entry is itself an error). + originalHash, err := fileutils.ComputeFileHash(ctx.FS(), fileutils.HashTypeSHA256, archivePath) + require.NoError(t, err) + + sourcesPath := filepath.Join(outputDir, fedorasource.SourcesFileName) + entry := fedorasource.FormatSourcesEntry(archiveName, fileutils.HashTypeSHA256, originalHash) + require.NoError(t, fileutils.WriteFile( + ctx.FS(), sourcesPath, []byte(entry+"\n"), fileperms.PublicFile)) + + ctrl := gomock.NewController(t) + component := components_testutils.NewMockComponent(ctrl) + sourceManager := sourceproviders_test.NewMockSourceManager(ctrl) + + component.EXPECT().GetName().AnyTimes().Return(componentName) + component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{ + Overlays: []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: archiveName, + Filename: testCase.pattern, + }, + }, + }) + + sourceManager.EXPECT().FetchFiles(gomock.Any(), component, outputDir).Return(nil) + sourceManager.EXPECT().FetchComponent(gomock.Any(), component, outputDir, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, dir string, _ ...sourceproviders.FetchComponentOption) error { + return fileutils.WriteFile( + ctx.FS(), filepath.Join(dir, componentName+".spec"), + []byte("# test spec"), fileperms.PublicFile) + }, + ) + + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + require.NoError(t, err) + + err = preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/) + + if testCase.wantErr { + // A no-match pattern errors and leaves the archive untouched. + require.Error(t, err) + + return + } + + require.NoError(t, err) + + out := t.TempDir() + require.NoError(t, archive.ExtractAuto(archivePath, out)) + assert.Equal(t, testCase.remaining, collectRegularFiles(t, out)) + }) + } +} + +// TestApplyOverlayToSources_RemoveFileGlob_LooseFiles exercises the same glob +// patterns against loose files in the sources tree (no archive prefix) through +// the real exported [sources.ApplyOverlayToSources] entry point, confirming +// archive-scoped and loose-file removal share identical glob semantics. +func TestApplyOverlayToSources_RemoveFileGlob_LooseFiles(t *testing.T) { + for _, testCase := range globRemovalCases() { + t.Run(testCase.name, func(t *testing.T) { + ctx := testctx.NewCtx(testctx.WithHostFS()) + sourcesDir := t.TempDir() + writeTreeFiles(t, sourcesDir, globRemovalTree()) + + overlay := projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayRemoveFile, + Filename: testCase.pattern, + } + + // file-remove does not touch the spec, so specPath is unused. + err := sources.ApplyOverlayToSources(ctx, ctx.FS(), overlay, sourcesDir, "") + + if testCase.wantErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + + assert.Equal(t, testCase.remaining, collectRegularFiles(t, sourcesDir)) + }) + } +} + +// findSourcesEntry returns the parsed 'sources' entry for filename, or nil. +func findSourcesEntry(t *testing.T, sourcesContent, filename string) *fedorasource.SourcesFileEntry { + t.Helper() + + parsed, err := fedorasource.ReadSourcesFile(sourcesContent) + require.NoError(t, err) + + for i := range parsed { + if parsed[i].Entry != nil && parsed[i].Entry.Filename == filename { + return parsed[i].Entry + } + } + + return nil +} + +// TestPrepareSources_SearchReplaceInArchiveRehashesEntry is an end-to-end check +// that a file-search-replace overlay scoped to an archive rewrites the file +// inside the archive, repacks it, and rehashes the matching 'sources' entry +// (preserving the original hash type). file-remove already has this coverage; +// this exercises the search-replace path through the exported PrepareSources. +func TestPrepareSources_SearchReplaceInArchiveRehashesEntry(t *testing.T) { + const ( + componentName = "test-component" + archiveName = "pkg.tar.gz" + ) + + // Host FS + real temp dir: archive extraction/repacking happens on disk. + ctx := testctx.NewCtx(testctx.WithHostFS()) + outputDir := t.TempDir() + archivePath := filepath.Join(outputDir, archiveName) + + // Single top-level directory => extract root is "pkg-1.0/", so the inner + // path "configure.ac" resolves relative to that root. + staging := t.TempDir() + pkgRoot := filepath.Join(staging, "pkg-1.0") + require.NoError(t, os.MkdirAll(pkgRoot, fileperms.PublicDir)) + require.NoError(t, os.WriteFile(filepath.Join(pkgRoot, "configure.ac"), + []byte("AC_CHECK_LIB(old_lib, main)\n"), fileperms.PrivateFile)) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, staging, archive.CompressionGzip)) + + // Seed a SHA256 'sources' entry (not the SHA512 default) so the test also + // proves the hash type is preserved. + originalHash, err := fileutils.ComputeFileHash(ctx.FS(), fileutils.HashTypeSHA256, archivePath) + require.NoError(t, err) + + sourcesPath := filepath.Join(outputDir, fedorasource.SourcesFileName) + entry := fedorasource.FormatSourcesEntry(archiveName, fileutils.HashTypeSHA256, originalHash) + require.NoError(t, fileutils.WriteFile(ctx.FS(), sourcesPath, []byte(entry+"\n"), fileperms.PublicFile)) + + ctrl := gomock.NewController(t) + component := components_testutils.NewMockComponent(ctrl) + sourceManager := sourceproviders_test.NewMockSourceManager(ctrl) + + component.EXPECT().GetName().AnyTimes().Return(componentName) + component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{ + Overlays: []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlaySearchAndReplaceInFile, + Archive: archiveName, + Filename: "configure.ac", + Regex: "old_lib", + Replacement: "new_lib", + }, + }, + }) + + sourceManager.EXPECT().FetchFiles(gomock.Any(), component, outputDir).Return(nil) + sourceManager.EXPECT().FetchComponent(gomock.Any(), component, outputDir, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, dir string, _ ...sourceproviders.FetchComponentOption) error { + return fileutils.WriteFile(ctx.FS(), filepath.Join(dir, componentName+".spec"), + []byte("# test spec"), fileperms.PublicFile) + }, + ) + + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + require.NoError(t, err) + + require.NoError(t, preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/)) + + // Rewrite: the file content inside the repacked archive reflects the replacement. + out := t.TempDir() + require.NoError(t, archive.ExtractAuto(archivePath, out)) + content, err := os.ReadFile(filepath.Join(out, "pkg-1.0", "configure.ac")) + require.NoError(t, err) + assert.Equal(t, "AC_CHECK_LIB(new_lib, main)\n", string(content)) + + // Repack: the archive's hash changed. + repackedHash, err := fileutils.ComputeFileHash(ctx.FS(), fileutils.HashTypeSHA256, archivePath) + require.NoError(t, err) + require.NotEqual(t, originalHash, repackedHash, + "precondition: rewriting a file in the archive should change its hash") + + // Rehash: the 'sources' entry was rewritten to the repacked hash, type preserved. + sourcesContent, err := fileutils.ReadFile(ctx.FS(), sourcesPath) + require.NoError(t, err) + + got := findSourcesEntry(t, string(sourcesContent), archiveName) + require.NotNil(t, got, "rewritten 'sources' file should still contain an entry for %q", archiveName) + assert.Equal(t, fileutils.HashTypeSHA256, got.HashType, "original hash type must be preserved") + assert.Equal(t, repackedHash, got.Hash, "'sources' entry must record the repacked archive's hash") +} + +// TestPrepareSources_SkipSourcesSkipsArchiveOverlays verifies the --skip-sources +// branch: when source downloads are skipped, archive overlays are skipped (with a +// warning) instead of applied, leaving both the archive and its 'sources' entry +// untouched. +func TestPrepareSources_SkipSourcesSkipsArchiveOverlays(t *testing.T) { + const ( + componentName = "test-component" + archiveName = "pkg.tar.gz" + ) + + ctx := testctx.NewCtx(testctx.WithHostFS()) + outputDir := t.TempDir() + archivePath := filepath.Join(outputDir, archiveName) + + staging := t.TempDir() + pkgRoot := filepath.Join(staging, "pkg-1.0") + require.NoError(t, os.MkdirAll(pkgRoot, fileperms.PublicDir)) + require.NoError(t, os.WriteFile(filepath.Join(pkgRoot, "remove-me.txt"), + []byte("delete me"), fileperms.PrivateFile)) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, staging, archive.CompressionGzip)) + + // Snapshot the archive and seed its 'sources' entry so we can assert both + // are left untouched. + originalArchive, err := os.ReadFile(archivePath) + require.NoError(t, err) + + originalHash, err := fileutils.ComputeFileHash(ctx.FS(), fileutils.HashTypeSHA256, archivePath) + require.NoError(t, err) + + sourcesPath := filepath.Join(outputDir, fedorasource.SourcesFileName) + entry := fedorasource.FormatSourcesEntry(archiveName, fileutils.HashTypeSHA256, originalHash) + require.NoError(t, fileutils.WriteFile(ctx.FS(), sourcesPath, []byte(entry+"\n"), fileperms.PublicFile)) + + ctrl := gomock.NewController(t) + component := components_testutils.NewMockComponent(ctrl) + sourceManager := sourceproviders_test.NewMockSourceManager(ctrl) + + component.EXPECT().GetName().AnyTimes().Return(componentName) + component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{ + Overlays: []projectconfig.ComponentOverlay{ + {Type: projectconfig.ComponentOverlayRemoveFile, Archive: archiveName, Filename: "remove-me.txt"}, + }, + }) + + // With --skip-sources, FetchFiles must NOT be called (no EXPECT for it); + // FetchComponent is still called to provide the spec. + sourceManager.EXPECT().FetchComponent(gomock.Any(), component, outputDir, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, dir string, _ ...sourceproviders.FetchComponentOption) error { + return fileutils.WriteFile(ctx.FS(), filepath.Join(dir, componentName+".spec"), + []byte("# test spec"), fileperms.PublicFile) + }, + ) + + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx, sources.WithSkipLookaside()) + require.NoError(t, err) + + require.NoError(t, preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/)) + + // The archive overlay is skipped: the archive is byte-for-byte unchanged... + afterArchive, err := os.ReadFile(archivePath) + require.NoError(t, err) + assert.Equal(t, originalArchive, afterArchive, + "archive must not be modified when --skip-sources skips archive overlays") + + // ...and its 'sources' entry keeps the original hash. + sourcesContent, err := fileutils.ReadFile(ctx.FS(), sourcesPath) + require.NoError(t, err) + + got := findSourcesEntry(t, string(sourcesContent), archiveName) + require.NotNil(t, got) + assert.Equal(t, originalHash, got.Hash, "'sources' entry hash must be unchanged when overlays are skipped") +} diff --git a/internal/app/azldev/core/sources/sourceprep.go b/internal/app/azldev/core/sources/sourceprep.go index c391ae61..f46379fa 100644 --- a/internal/app/azldev/core/sources/sourceprep.go +++ b/internal/app/azldev/core/sources/sourceprep.go @@ -243,12 +243,12 @@ func (p *sourcePreparerImpl) PrepareSources( } if applyOverlays { - err := p.applyOverlaysToSources(ctx, component, outputDir) + repackedArchives, err := p.applyOverlaysToSources(component, outputDir) if err != nil { return err } - if err := p.updateSourcesFile(component, outputDir); err != nil { + if err := p.updateSourcesFile(component, outputDir, repackedArchives); err != nil { return fmt.Errorf("failed to update 'sources' file for component %#q:\n%w", component.GetName(), err) } @@ -270,17 +270,17 @@ func (p *sourcePreparerImpl) PrepareSources( } // applyOverlaysToSources writes the macros file and then applies all overlays. +// It returns the names of any archives that were repacked by archive overlays +// (empty in dry-run mode or when no archive overlays ran), so the caller can +// rehash exactly those entries in the 'sources' file. func (p *sourcePreparerImpl) applyOverlaysToSources( - ctx context.Context, component components.Component, outputDir string, -) error { - // Emit computed macros to a macros file in the output directory. - // If the build configuration produces no macros, no file is written and - // macrosFileName will be empty. + component components.Component, outputDir string, +) ([]string, error) { var macrosFileName string macrosFilePath, err := p.writeMacrosFile(component, outputDir) if err != nil { - return fmt.Errorf("failed to write macros file for component %#q:\n%w", + return nil, fmt.Errorf("failed to write macros file for component %#q:\n%w", component.GetName(), err) } @@ -288,47 +288,88 @@ func (p *sourcePreparerImpl) applyOverlaysToSources( macrosFileName = filepath.Base(macrosFilePath) } - // Apply all overlays to prepared sources. - if err := p.applyOverlays(ctx, component, outputDir, macrosFileName); err != nil { - return fmt.Errorf("failed to apply overlays for component %#q:\n%w", component.GetName(), err) + repackedArchives, err := p.applyOverlays(component, outputDir, macrosFileName) + if err != nil { + return nil, fmt.Errorf("failed to apply overlays for component %#q:\n%w", + component.GetName(), err) } - return nil + return repackedArchives, nil } // applyOverlays applies all overlays (user-defined and system-generated) to the -// component sources. Overlay application is decoupled from git history generation: -// overlays modify the working tree; synthetic history is recorded separately by -// [trySyntheticHistory]. +// component sources. It returns the names of any archives that were repacked by +// archive overlays. func (p *sourcePreparerImpl) applyOverlays( - _ context.Context, component components.Component, sourcesDirPath, macrosFileName string, -) error { + component components.Component, sourcesDirPath, macrosFileName string, +) ([]string, error) { event := p.eventListener.StartEvent("Applying overlays", "component", component.GetName()) defer event.End() - // Resolve the spec path once for all overlay operations in this call. absSpecPath, err := p.resolveSpecPath(component, sourcesDirPath) if err != nil { - return err + return nil, err } - // Collect all overlays in application order. This ensures every change is - // captured in the synthetic history, including build configuration changes. allOverlays, err := p.collectOverlays(component, macrosFileName) if err != nil { - return fmt.Errorf("failed to collect overlays for component %#q:\n%w", component.GetName(), err) + return nil, fmt.Errorf("failed to collect overlays for component %#q:\n%w", component.GetName(), err) } if len(allOverlays) == 0 { - return nil + return nil, nil + } + + // Archive overlays are applied first (they modify archived source files + // in-place), followed by spec and loose-file overlays. Each function + // self-filters to the overlay types it handles. + repackedArchives, err := p.applyArchiveOverlayGroup(component, sourcesDirPath, allOverlays) + if err != nil { + return nil, err } - // Apply all overlays to the working tree. if err := p.applyOverlayList(allOverlays, sourcesDirPath, absSpecPath); err != nil { - return fmt.Errorf("failed to apply overlays for component %#q:\n%w", component.GetName(), err) + return nil, fmt.Errorf("failed to apply overlays for component %#q:\n%w", component.GetName(), err) } - return nil + return repackedArchives, nil +} + +// applyArchiveOverlayGroup applies the archive-scoped overlays contained in the +// given overlay list. The list may hold overlays of any type; only those for +// which [projectconfig.ComponentOverlay.ModifiesArchive] reports true are +// processed here. Skipped when source downloads were not performed. It returns +// the names of the archives that were actually repacked (empty in dry-run mode +// or when source downloads were skipped). +func (p *sourcePreparerImpl) applyArchiveOverlayGroup( + component components.Component, + sourcesDirPath string, overlays []projectconfig.ComponentOverlay, +) ([]string, error) { + archiveOverlays := lo.Filter(overlays, func(overlay projectconfig.ComponentOverlay, _ int) bool { + return overlay.ModifiesArchive() + }) + + if len(archiveOverlays) == 0 { + return nil, nil + } + + if p.skipLookaside { + slog.Warn("Skipping archive overlays because source downloads were skipped (--skip-sources)", + "component", component.GetName(), + "count", len(archiveOverlays)) + + return nil, nil + } + + repackedArchives, err := applyArchiveOverlays( + p.dryRunnable, p.eventListener, sourcesDirPath, archiveOverlays, + ) + if err != nil { + return nil, fmt.Errorf("failed to apply archive overlays for component %#q:\n%w", + component.GetName(), err) + } + + return repackedArchives, nil } // collectOverlays gathers all overlays for a component into a single ordered slice: @@ -601,8 +642,10 @@ func (p *sourcePreparerImpl) DiffSources( return nil, fmt.Errorf("failed to copy sources for component %#q:\n%w", component.GetName(), err) } - // Apply overlays in-place to the copied directory only. - if err := p.applyOverlaysToSources(ctx, component, overlaidDir); err != nil { + // Apply overlays in-place to the copied directory only. The repacked-archive + // list is unused here: DiffSources diffs the trees directly and does not + // rewrite a 'sources' file. + if _, err := p.applyOverlaysToSources(component, overlaidDir); err != nil { return nil, fmt.Errorf("failed to apply overlays for component %#q:\n%w", component.GetName(), err) } @@ -631,9 +674,17 @@ func (p *sourcePreparerImpl) DiffSources( // enforced by [projectconfig.ConfigFile.Validate]). Setting `ReplaceUpstream` = true without // a matching upstream entry is also an error: the user expressed intent to replace something // that isn't there, which almost certainly indicates a stale config or filename typo. -func (p *sourcePreparerImpl) updateSourcesFile(component components.Component, outputDir string) error { - sourceFiles := component.GetConfig().SourceFiles - if len(sourceFiles) == 0 { +func (p *sourcePreparerImpl) updateSourcesFile( + component components.Component, outputDir string, modifiedArchives []string, +) error { + config := component.GetConfig() + sourceFiles := config.SourceFiles + + // modifiedArchives lists the archives that archive overlays actually repacked + // during this run; their 'sources' digests must be refreshed. The list is empty + // when no archive overlays ran, in dry-run mode, or when source downloads were + // skipped, so rehashing is correctly avoided in those cases. + if len(sourceFiles) == 0 && len(modifiedArchives) == 0 { return nil } @@ -644,7 +695,27 @@ func (p *sourcePreparerImpl) updateSourcesFile(component components.Component, o return err } - mergedLines, err := p.buildSourceEntries(sourceFiles, existingContent, component.GetName(), outputDir) + // Parse once, then rehash modified archives and merge source-files entries + // on the parsed representation — single parse, single write. + existingLines, err := fedorasource.ReadSourcesFile(existingContent) + if err != nil { + return fmt.Errorf("failed to parse 'sources' file %#q:\n%w", sourcesFilePath, err) + } + + // Rehash archives that were modified by archive overlays in-place. + if err := p.rehashModifiedEntries(existingLines, outputDir, modifiedArchives); err != nil { + return err + } + + // In full prep-sources mode, cross-check any 'overlay'-origin source-file entries against + // the hashes that were just computed so stale stated hashes are caught immediately. + if !p.skipLookaside { + if err := validateOverlayResultHashes(existingLines, sourceFiles, modifiedArchives, component.GetName()); err != nil { + return err + } + } + + mergedLines, err := p.buildSourceEntries(sourceFiles, existingLines, component.GetName(), outputDir) if err != nil { return err } @@ -663,6 +734,132 @@ func (p *sourcePreparerImpl) updateSourcesFile(component components.Component, o return nil } +// rehashModifiedEntries updates the Raw and Entry fields of parsed 'sources' lines +// for archives that were modified by archive overlays. The hash is recomputed using +// the same hash type as the original entry. It returns an error if any modified +// archive has no matching 'sources' entry, since that would leave a stale digest. +func (p *sourcePreparerImpl) rehashModifiedEntries( + lines []fedorasource.SourcesFileLine, outputDir string, modifiedArchives []string, +) error { + if len(modifiedArchives) == 0 { + return nil + } + + // Track which archives we actually rehashed so we can detect any that were + // repacked by an overlay but have no matching 'sources' entry. Leaving such + // an archive unrehashed would record a stale digest, so it is treated as an error. + rehashed := make(map[string]bool, len(modifiedArchives)) + for _, name := range modifiedArchives { + rehashed[name] = false + } + + for idx, line := range lines { + if line.Entry == nil { + continue + } + + if _, ok := rehashed[line.Entry.Filename]; !ok { + continue + } + + archivePath := filepath.Join(outputDir, line.Entry.Filename) + + newHash, err := fileutils.ComputeFileHash(p.fs, line.Entry.HashType, archivePath) + if err != nil { + return fmt.Errorf("rehashing modified archive %#q:\n%w", line.Entry.Filename, err) + } + + slog.Debug("Rehashed modified archive in 'sources' file", + "archive", line.Entry.Filename, + "hashType", line.Entry.HashType, + "oldHash", line.Entry.Hash, + "newHash", newHash, + ) + + lines[idx].Raw = fedorasource.FormatSourcesEntry(line.Entry.Filename, line.Entry.HashType, newHash) + lines[idx].Entry.Hash = newHash + rehashed[line.Entry.Filename] = true + } + + // Any archive that an overlay repacked but that has no 'sources' entry would + // silently keep a stale digest. Surface this as an error identifying the + // missing filenames rather than producing an inconsistent 'sources' file. + var missing []string + + for name, done := range rehashed { + if !done { + missing = append(missing, name) + } + } + + if len(missing) > 0 { + slices.Sort(missing) + + return fmt.Errorf( + "archive overlay(s) modified %d archive(s) with no matching 'sources' entry to rehash: %s", + len(missing), strings.Join(missing, ", ")) + } + + return nil +} + +// validateOverlayResultHashes cross-checks [projectconfig.OriginTypeOverlay] source-file entries +// against the post-overlay hashes that [rehashModifiedEntries] just computed in existingLines. +// A mismatch means the stated hash in the config is stale and must be updated. +// It is a no-op when no archives were repacked in this run. +func validateOverlayResultHashes( + lines []fedorasource.SourcesFileLine, + sourceFiles []projectconfig.SourceFileReference, + modifiedArchives []string, + componentName string, +) error { + if len(modifiedArchives) == 0 { + return nil + } + + // Only validate archives that were actually repacked in this run. + repacked := make(map[string]bool, len(modifiedArchives)) + for _, name := range modifiedArchives { + repacked[name] = true + } + + // Build a filename → computed (post-overlay) entry map from the updated lines. + computedByName := make(map[string]fedorasource.SourcesFileEntry, len(lines)) + for _, line := range lines { + if line.Entry != nil { + computedByName[line.Entry.Filename] = *line.Entry + } + } + + for _, ref := range sourceFiles { + if ref.Origin.Type != projectconfig.OriginTypeOverlay { + continue + } + + if !repacked[ref.Filename] { + continue + } + + computed, ok := computedByName[ref.Filename] + if !ok { + // Missing upstream entry will be reported by processSourceRef. + continue + } + + if computed.HashType != ref.HashType || computed.Hash != ref.Hash { + return fmt.Errorf( + "component %#q: archive %#q 'source-files' hash does not match the hash computed "+ + "after applying overlays; update the 'hash' and 'hash-type' fields in the "+ + "'source-files' entry:\n stated: %s %s\n computed: %s %s", + componentName, ref.Filename, + ref.HashType, ref.Hash, + computed.HashType, computed.Hash) + } + } + + return nil +} + // readSourcesFileIfExists reads the 'sources' file content if it exists, returning empty string if not. func (p *sourcePreparerImpl) readSourcesFileIfExists(sourcesFilePath string) (string, error) { exists, err := fileutils.Exists(p.fs, sourcesFilePath) @@ -682,32 +879,24 @@ func (p *sourcePreparerImpl) readSourcesFileIfExists(sourcesFilePath string) (st return string(data), nil } -// buildSourceEntries validates [projectconfig.SourceFileReference] entries and returns -// the merged set of lines ready to be written to the 'sources' file. Before returning, -// it logs an INFO-level event indicating that the 'sources' file will be updated, -// including the counts of newly added and replaced entries. +// buildSourceEntries merges user-declared [projectconfig.SourceFileReference] entries +// into the parsed 'sources' lines. Returns the final set of raw lines ready to be +// written to the 'sources' file. // // Output ordering and preservation: -// - Each line of [existingContent] is emitted verbatim, except for entry lines whose +// - Each existing line is emitted verbatim, except for entry lines whose // filename matches a replacement, which are swapped for the new formatted entry. -// Comments and blank lines from the original file are kept in their original positions. -// - Brand-new entries (no upstream filename collision) are appended after the upstream -// content in the order they appear in [sourceFiles]. +// Comments and blank lines are kept in their original positions. +// - Brand-new entries (no upstream filename collision) are appended after the +// existing content in the order they appear in [sourceFiles]. // // Collision rules and hash resolution are documented on [sourcePreparerImpl.processSourceRef]. func (p *sourcePreparerImpl) buildSourceEntries( sourceFiles []projectconfig.SourceFileReference, - existingContent string, + existingLines []fedorasource.SourcesFileLine, componentName string, outputDir string, ) (mergedLines []string, err error) { - existingLines, err := fedorasource.ReadSourcesFile(existingContent) - if err != nil { - return nil, fmt.Errorf( - "failed to parse existing 'sources' file at %#q:\n%w", - filepath.Join(outputDir, fedorasource.SourcesFileName), err) - } - // Index upstream entries by filename for O(1) collision lookup. The parser // (fedorasource.ReadSourcesFile) errors on duplicate filenames, so the // entries are guaranteed unique by the time we get here. @@ -1082,10 +1271,17 @@ func (p *sourcePreparerImpl) resolveSpecPath( } // applyOverlayList applies a list of overlays to the component sources sequentially. +// Archive-scoped overlays (see [projectconfig.ComponentOverlay.ModifiesArchive]) are +// skipped here; they are handled separately by [applyArchiveOverlays], which batches +// extraction and repacking per archive. func (p *sourcePreparerImpl) applyOverlayList( overlays []projectconfig.ComponentOverlay, sourcesDirPath, absSpecPath string, ) error { for _, overlay := range overlays { + if overlay.ModifiesArchive() { + continue + } + if err := ApplyOverlayToSources( p.dryRunnable, p.fs, overlay, sourcesDirPath, absSpecPath, ); err != nil { diff --git a/internal/app/azldev/core/sources/sourceprep_test.go b/internal/app/azldev/core/sources/sourceprep_test.go index a0a6f319..eb30cf53 100644 --- a/internal/app/azldev/core/sources/sourceprep_test.go +++ b/internal/app/azldev/core/sources/sourceprep_test.go @@ -5,6 +5,7 @@ package sources_test import ( "errors" + "os" "path/filepath" "strings" "testing" @@ -16,6 +17,7 @@ import ( "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders" "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders/fedorasource" "github.com/microsoft/azure-linux-dev-tools/internal/providers/sourceproviders/sourceproviders_test" + "github.com/microsoft/azure-linux-dev-tools/internal/utils/archive" "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileperms" "github.com/microsoft/azure-linux-dev-tools/internal/utils/fileutils" "github.com/stretchr/testify/assert" @@ -99,6 +101,178 @@ func TestPrepareSources_Success(t *testing.T) { assert.NotContains(t, string(specContents), "Source9999") } +// TestPrepareSources_ArchiveOverlayRehashesSourcesEntry is an end-to-end check +// of the key correctness behavior introduced with archive overlays: when an +// archive-scoped overlay mutates an archive's contents, the matching 'sources' +// entry must be re-hashed in place so the recorded digest reflects the repacked +// archive (while keeping the original hash *type*). +// +// This runs against the host filesystem with a real temp dir because archive +// overlays extract/repack through the [archive] package, which uses OS +// primitives ([os.Root], os.*) and therefore requires genuine on-disk paths — +// an in-memory FS would not be visible to extraction/repacking. This mirrors +// the existing archive internal tests, which likewise use t.TempDir(). +func TestPrepareSources_ArchiveOverlayRehashesSourcesEntry(t *testing.T) { + const ( + componentName = "test-component" + archiveName = "pkg-1.0.tar.gz" + ) + + // Host FS + real temp dir: archive extraction/repacking happens on disk. + ctx := testctx.NewCtx(testctx.WithHostFS()) + outputDir := t.TempDir() + + archivePath := filepath.Join(outputDir, archiveName) + specPath := filepath.Join(outputDir, componentName+".spec") + sourcesPath := filepath.Join(outputDir, fedorasource.SourcesFileName) + + // Build a deterministic archive whose single top-level directory follows the + // conventional "%{name}-%{version}/" layout, containing a file we will remove + // and one we will keep. + stagingDir := t.TempDir() + pkgRoot := filepath.Join(stagingDir, "pkg-1.0") + require.NoError(t, os.MkdirAll(pkgRoot, fileperms.PublicDir)) + require.NoError(t, os.WriteFile(filepath.Join(pkgRoot, "keep.txt"), []byte("keep me"), fileperms.PrivateFile)) + require.NoError(t, os.WriteFile(filepath.Join(pkgRoot, "remove-me.txt"), []byte("delete me"), fileperms.PrivateFile)) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, stagingDir, archive.CompressionGzip)) + + // Record the original hash of the archive and seed a 'sources' file with it. + // Use SHA256 (not the SHA512 default) so the test also proves the hash *type* + // is preserved rather than coincidentally matching a default. + originalHash, err := fileutils.ComputeFileHash(ctx.FS(), fileutils.HashTypeSHA256, archivePath) + require.NoError(t, err) + + originalEntry := fedorasource.FormatSourcesEntry(archiveName, fileutils.HashTypeSHA256, originalHash) + require.NoError(t, fileutils.WriteFile( + ctx.FS(), sourcesPath, []byte(originalEntry+"\n"), fileperms.PublicFile)) + + ctrl := gomock.NewController(t) + component := components_testutils.NewMockComponent(ctrl) + sourceManager := sourceproviders_test.NewMockSourceManager(ctrl) + + component.EXPECT().GetName().AnyTimes().Return(componentName) + component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{ + Overlays: []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: archiveName, + Filename: "remove-me.txt", + }, + }, + }) + + // The archive and 'sources' file already exist on disk; the source manager + // only needs to provide the spec file (FetchFiles is a no-op download). + sourceManager.EXPECT().FetchFiles(gomock.Any(), component, outputDir).Return(nil) + sourceManager.EXPECT().FetchComponent(gomock.Any(), component, outputDir, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, dir string, _ ...sourceproviders.FetchComponentOption) error { + return fileutils.WriteFile( + ctx.FS(), filepath.Join(dir, componentName+".spec"), + []byte("# test spec"), fileperms.PublicFile) + }, + ) + + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + require.NoError(t, err) + + err = preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/) + require.NoError(t, err) + + // The overlay must have actually mutated the archive on disk. + assert.FileExists(t, specPath) + + repackedHash, err := fileutils.ComputeFileHash(ctx.FS(), fileutils.HashTypeSHA256, archivePath) + require.NoError(t, err) + require.NotEqual(t, originalHash, repackedHash, + "precondition: removing a file from the archive should change its hash") + + // The 'sources' entry must have been rewritten to the repacked archive's hash, + // preserving the original SHA256 hash type. + sourcesContent, err := fileutils.ReadFile(ctx.FS(), sourcesPath) + require.NoError(t, err) + + parsedLines, err := fedorasource.ReadSourcesFile(string(sourcesContent)) + require.NoError(t, err) + + var entry *fedorasource.SourcesFileEntry + + for i := range parsedLines { + if parsedLines[i].Entry != nil && parsedLines[i].Entry.Filename == archiveName { + entry = parsedLines[i].Entry + + break + } + } + + require.NotNil(t, entry, "rewritten 'sources' file should still contain an entry for %q", archiveName) + assert.Equal(t, fileutils.HashTypeSHA256, entry.HashType, "original hash type must be preserved") + assert.Equal(t, repackedHash, entry.Hash, "'sources' entry must record the repacked archive's hash") + assert.NotEqual(t, originalHash, entry.Hash, "'sources' entry hash must have been updated") +} + +// TestPrepareSources_ArchiveOverlayMissingSourcesEntryErrors verifies that when an +// archive-scoped overlay repacks an archive that has no matching 'sources' entry, +// preparation fails instead of silently leaving a stale (or absent) digest. +func TestPrepareSources_ArchiveOverlayMissingSourcesEntryErrors(t *testing.T) { + const ( + componentName = "test-component" + archiveName = "pkg-1.0.tar.gz" + ) + + // Host FS + real temp dir: archive extraction/repacking happens on disk. + ctx := testctx.NewCtx(testctx.WithHostFS()) + outputDir := t.TempDir() + + archivePath := filepath.Join(outputDir, archiveName) + sourcesPath := filepath.Join(outputDir, fedorasource.SourcesFileName) + + // Build a deterministic archive with a file to remove, but deliberately seed a + // 'sources' file that has NO entry for this archive. + stagingDir := t.TempDir() + pkgRoot := filepath.Join(stagingDir, "pkg-1.0") + require.NoError(t, os.MkdirAll(pkgRoot, fileperms.PublicDir)) + require.NoError(t, os.WriteFile(filepath.Join(pkgRoot, "remove-me.txt"), []byte("delete me"), fileperms.PrivateFile)) + require.NoError(t, archive.CreateDeterministicArchive(archivePath, stagingDir, archive.CompressionGzip)) + + // 'sources' file references some unrelated file, not the archive being modified. + require.NoError(t, fileutils.WriteFile( + ctx.FS(), sourcesPath, + []byte("SHA256 (unrelated.tar.gz) = 0000000000000000000000000000000000000000000000000000000000000000\n"), + fileperms.PublicFile)) + + ctrl := gomock.NewController(t) + component := components_testutils.NewMockComponent(ctrl) + sourceManager := sourceproviders_test.NewMockSourceManager(ctrl) + + component.EXPECT().GetName().AnyTimes().Return(componentName) + component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{ + Overlays: []projectconfig.ComponentOverlay{ + { + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: archiveName, + Filename: "remove-me.txt", + }, + }, + }) + + sourceManager.EXPECT().FetchFiles(gomock.Any(), component, outputDir).Return(nil) + sourceManager.EXPECT().FetchComponent(gomock.Any(), component, outputDir, gomock.Any()).DoAndReturn( + func(_ interface{}, _ interface{}, dir string, _ ...sourceproviders.FetchComponentOption) error { + return fileutils.WriteFile( + ctx.FS(), filepath.Join(dir, componentName+".spec"), + []byte("# test spec"), fileperms.PublicFile) + }, + ) + + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + require.NoError(t, err) + + err = preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/) + require.Error(t, err, "preparing sources should fail when a modified archive has no 'sources' entry") + assert.Contains(t, err.Error(), archiveName, + "error should identify the archive missing from the 'sources' file") +} + func TestPrepareSources_SourceManagerError(t *testing.T) { ctrl := gomock.NewController(t) component := components_testutils.NewMockComponent(ctrl) @@ -859,7 +1033,7 @@ func TestPrepareSources_UpdatesSourcesFile(t *testing.T) { existingSourcesContent: "SHA512 (dup.tar.gz) = aaaa1111\nSHA512 (dup.tar.gz) = bbbb2222\n", expectError: true, errorContains: []string{ - "failed to parse existing 'sources' file", + "failed to parse 'sources' file", "duplicate filename", "dup.tar.gz", }, diff --git a/internal/projectconfig/component.go b/internal/projectconfig/component.go index c260f2dd..8603103f 100644 --- a/internal/projectconfig/component.go +++ b/internal/projectconfig/component.go @@ -48,13 +48,26 @@ const ( // shell script inside a mock chroot. The script is expected to populate a specific // output directory; azldev then packages that directory into a deterministic archive. OriginTypeCustom OriginType = "custom" + + // OriginTypeOverlay indicates that the source file's hash was changed by archive overlays. + // No download occurs; the file is already present as a spec source. The 'hash' and 'hash-type' + // fields record the expected post-overlay hash, which is injected into the 'sources' file + // during render (skip-lookaside) and validated against the computed hash during 'prep-sources'. + OriginTypeOverlay OriginType = "overlay" ) +// IsFetched reports whether [SourceManager.FetchFiles] downloads this origin type to disk. +// Returns false for [OriginTypeOverlay], which obtains its file via the upstream lookaside +// extractor in [SourceManager.FetchComponent]. All other types return true. +func (t OriginType) IsFetched() bool { + return t != OriginTypeOverlay +} + // 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,enum=custom,title=Origin type,description=Type of origin for this source file" fingerprint:"-"` + Type OriginType `toml:"type" json:"type" jsonschema:"required,enum=download,enum=custom,enum=overlay,title=Origin type,description=Type of origin for this source file" fingerprint:"-"` // Uri to download the source file from if origin type is 'download'. Ignored for other origin types. Uri string `toml:"uri,omitempty" json:"uri,omitempty" jsonschema:"title=URI,description=URI to download the source file from if origin type is 'download',example=https://example.com/source.tar.gz" fingerprint:"-"` diff --git a/internal/projectconfig/configfile.go b/internal/projectconfig/configfile.go index 851f1607..65112f4a 100644 --- a/internal/projectconfig/configfile.go +++ b/internal/projectconfig/configfile.go @@ -177,6 +177,7 @@ func validateComponentGroupMetadata(groups map[string]ComponentGroupConfig) erro // - Hash value without a hash type is not allowed. // - Origin must be present and valid for each source file. // - 'replace-upstream' and 'replace-reason' must be set together. +// - [OriginTypeOverlay] entries additionally require 'hash', 'hash-type', and 'replace-upstream = true'. func validateSourceFiles(sourceFiles []SourceFileReference, componentName string) error { seen := make(map[string]bool, len(sourceFiles)) @@ -217,6 +218,41 @@ func validateSourceFiles(sourceFiles []SourceFileReference, componentName string if err := validateOrigin(ref.Origin, ref.Filename, componentName); err != nil { return err } + + if ref.Origin.Type == OriginTypeOverlay { + if err := validateOverlayOriginRef(ref, componentName); err != nil { + return err + } + } + } + + return nil +} + +// validateOverlayOriginRef enforces additional constraints on [SourceFileReference] entries +// with [OriginTypeOverlay]: 'hash', 'hash-type', and 'replace-upstream = true' are all required. +// These entries record the post-overlay hash of an archive that is already present as a spec +// source, so a download is never performed and the hash cannot be computed on the fly. +func validateOverlayOriginRef(ref SourceFileReference, componentName string) error { + if ref.Hash == "" { + return fmt.Errorf( + "source file %#q in component %#q has 'origin.type = overlay' but no 'hash'; "+ + "'hash' is required to record the expected post-overlay archive hash", + ref.Filename, componentName) + } + + if ref.HashType == "" { + return fmt.Errorf( + "source file %#q in component %#q has 'origin.type = overlay' but no 'hash-type'; "+ + "'hash-type' is required when 'hash' is provided", + ref.Filename, componentName) + } + + if !ref.ReplaceUpstream { + return fmt.Errorf( + "source file %#q in component %#q has 'origin.type = overlay' but 'replace-upstream' is not true; "+ + "'replace-upstream = true' is required because the archive already exists in the upstream 'sources' file", + ref.Filename, componentName) } return nil @@ -332,6 +368,7 @@ func validateCustomSourceInputs(ref SourceFileReference, componentName string) e // validateOrigin checks that a source file [Origin] is present and valid for its type. // For [OriginTypeURI] ('download'), the [Origin.Uri] field must be a valid URI with a scheme. +// For [OriginTypeOverlay] ('overlay'), no URI is used; the archive is already on disk. func validateOrigin(origin Origin, filename string, componentName string) error { if origin.Type == "" { return fmt.Errorf( @@ -373,6 +410,13 @@ func validateOrigin(origin Origin, filename string, componentName string) error filename, componentName) } + case OriginTypeOverlay: + if origin.Uri != "" { + return fmt.Errorf( + "unexpected 'uri' for source file %#q, component %#q; "+ + "'uri' must not be set when 'origin' type is 'overlay'", + filename, componentName) + } default: return fmt.Errorf( "unsupported 'origin' type %#q for source file %#q, component %#q", diff --git a/internal/providers/sourceproviders/fedorasourceprovider.go b/internal/providers/sourceproviders/fedorasourceprovider.go index 688c66e2..3c43bed9 100644 --- a/internal/providers/sourceproviders/fedorasourceprovider.go +++ b/internal/providers/sourceproviders/fedorasourceprovider.go @@ -140,13 +140,20 @@ func (g *FedoraSourcesProviderImpl) GetComponent( return fmt.Errorf("failed to clone git repository %#q:\n%w", gitRepoURL, err) } + // Collect filenames from source-files config so the lookaside extractor can skip them. // Collect filenames from source-files config so the lookaside extractor can skip them. // [SourceManager.FetchFiles] acquires the configured versions after component fetching. + // Only files that FetchFiles actually downloads belong in this list — origin types that + // do not perform their own download (e.g. 'overlay') must be left out so the upstream + // lookaside extractor still fetches the original archive for archive overlays to work on. sourceFiles := component.GetConfig().SourceFiles - skipFileNames := make([]string, len(sourceFiles)) - for i := range sourceFiles { - skipFileNames[i] = sourceFiles[i].Filename + var skipFileNames []string + + for _, sf := range sourceFiles { + if sf.Origin.Type.IsFetched() { + skipFileNames = append(skipFileNames, sf.Filename) + } } // Process the cloned repo: checkout target commit, extract sources, copy to destination. diff --git a/internal/providers/sourceproviders/sourcemanager.go b/internal/providers/sourceproviders/sourcemanager.go index 20941709..6f4492de 100644 --- a/internal/providers/sourceproviders/sourcemanager.go +++ b/internal/providers/sourceproviders/sourcemanager.go @@ -389,6 +389,26 @@ func (m *sourceManager) fetchSourceFile( destPath := filepath.Join(destDirPath, fileRef.Filename) + // Overlay-origin entries declare the post-overlay hash of an archive that is already + // present as a spec source. No download is needed; the hash is used only to update + // the 'sources' file during render and to validate the output of 'prep-sources'. + if fileRef.Origin.Type == projectconfig.OriginTypeOverlay { + return nil + } + + sourceExists, err := fileutils.Exists(m.fs, destPath) + if err != nil { + return fmt.Errorf("failed to check existence of destination file %#q:\n%w", destPath, err) + } + + if sourceExists { + slog.Debug("Source file already exists, skipping download", + "filename", fileRef.Filename, + "path", destPath) + + return nil + } + // Try the lookaside cache first for non-custom files if hash info is available. // Custom files are always regenerated so stale configured hashes are detected. if m.trySourceFileLookaside(ctx, httpDownloader, component, fileRef, destPath) { @@ -397,25 +417,13 @@ func (m *sourceManager) fetchSourceFile( // Try each registered file provider. Providers return [ErrNotFound] to signal // they don't handle this reference; any other error is fatal. - for _, provider := range m.fileProviders { - err := provider.GetFile(ctx, component, *fileRef, destDirPath) - if err == nil { - // File providers are responsible for producing the file but not for - // hash validation. Validate here so all acquisition paths are covered. - if fileRef.Hash != "" && fileRef.HashType != "" { - hashErr := fileutils.ValidateFileHash( - m.dryRunnable, m.fs, fileRef.HashType, destPath, fileRef.Hash) - if hashErr != nil { - return fmt.Errorf("hash validation failed for %#q:\n%w", fileRef.Filename, hashErr) - } - } - - return nil - } + handled, err := m.tryFileProviders(ctx, component, fileRef, destDirPath, destPath) + if err != nil { + return err + } - if !errors.Is(err, ErrNotFound) { - return fmt.Errorf("file provider failed for %#q:\n%w", fileRef.Filename, err) - } + if handled { + return nil } // Fall back to the configured origin (not allowed when disable-origins is set). @@ -458,6 +466,39 @@ func (m *sourceManager) trySourceFileLookaside( return false } +// tryFileProviders attempts each registered file provider in turn. It returns +// handled=true when a provider produced (and, when hashes are configured, +// validated) the file. A provider signalling [ErrNotFound] is skipped; any other +// provider error is fatal. +func (m *sourceManager) tryFileProviders( + ctx context.Context, + component components.Component, + fileRef *projectconfig.SourceFileReference, + destDirPath, destPath string, +) (handled bool, err error) { + for _, provider := range m.fileProviders { + provErr := provider.GetFile(ctx, component, *fileRef, destDirPath) + if provErr == nil { + // File providers are responsible for producing the file but not for + // hash validation. Validate here so all acquisition paths are covered. + if fileRef.Hash != "" && fileRef.HashType != "" { + if hashErr := fileutils.ValidateFileHash( + m.dryRunnable, m.fs, fileRef.HashType, destPath, fileRef.Hash); hashErr != nil { + return false, fmt.Errorf("hash validation failed for %#q:\n%w", fileRef.Filename, hashErr) + } + } + + return true, nil + } + + if !errors.Is(provErr, ErrNotFound) { + return false, fmt.Errorf("file provider failed for %#q:\n%w", fileRef.Filename, provErr) + } + } + + return false, nil +} + // tryLookasideDownload attempts to download a source file from the lookaside cache. // Returns nil on success, or an error if the download fails. func (m *sourceManager) tryLookasideDownload( @@ -529,6 +570,12 @@ func (m *sourceManager) fetchFromDownloadOrigin( "ensure the distro has a 'mock-config' configured", fileRef.Filename) + case projectconfig.OriginTypeOverlay: + // Overlay-origin files are skipped before reaching this point in fetchSourceFile. + // This case should never be reached. + return fmt.Errorf("internal error: download attempted for 'overlay'-origin source file %#q", + fileRef.Filename) + default: return fmt.Errorf("unsupported origin type %#q for source file %#q", fileRef.Origin.Type, fileRef.Filename) diff --git a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap index 0cfae46f..945c283f 100755 --- a/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap @@ -715,7 +715,8 @@ "type": "string", "enum": [ "download", - "custom" + "custom", + "overlay" ], "title": "Origin type", "description": "Type of origin for this source file" diff --git a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap index 0cfae46f..945c283f 100755 --- a/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap +++ b/scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap @@ -715,7 +715,8 @@ "type": "string", "enum": [ "download", - "custom" + "custom", + "overlay" ], "title": "Origin type", "description": "Type of origin for this source file" diff --git a/schemas/azldev.schema.json b/schemas/azldev.schema.json index 0cfae46f..945c283f 100644 --- a/schemas/azldev.schema.json +++ b/schemas/azldev.schema.json @@ -715,7 +715,8 @@ "type": "string", "enum": [ "download", - "custom" + "custom", + "overlay" ], "title": "Origin type", "description": "Type of origin for this source file" From f59548b41ebec27f812e7fa442e803cd3988b5ce Mon Sep 17 00:00:00 2001 From: Antonio Salinas Date: Wed, 15 Jul 2026 23:12:34 +0000 Subject: [PATCH 3/5] Require overlay-origin tag for components with archive overlays --- docs/user/reference/config/overlays.md | 4 ++ .../app/azldev/core/components/resolver.go | 4 ++ internal/projectconfig/component.go | 23 ++++++++ internal/projectconfig/configfile.go | 4 ++ internal/projectconfig/configfile_test.go | 53 +++++++++++++++++++ .../sourceproviders/sourcemanager.go | 13 ----- 6 files changed, 88 insertions(+), 13 deletions(-) diff --git a/docs/user/reference/config/overlays.md b/docs/user/reference/config/overlays.md index 5685a4ec..db1e763c 100644 --- a/docs/user/reference/config/overlays.md +++ b/docs/user/reference/config/overlays.md @@ -66,6 +66,10 @@ file = "vendor/**" # files inside the archive > share a single extract/modify/repack cycle. When wired into the source-preparation pipeline, the `sources` file > should be rehashed afterward to reflect the repacked archive; they are processed independently of spec and loose-file overlays. +> **Required drift protection:** Every archive targeted by an archive overlay must have one matching +> `source-files` entry with `origin.type = "overlay"`. The entry records the expected post-overlay +> hash; one entry can cover multiple overlays for the same archive. See [Components](components.md#recording-the-post-overlay-hash-for-archive-overlays). + > **Extraction root:** The inner path is interpreted relative to the archive's extraction root: if the archive unpacks to a single top-level directory (the conventional `%{name}-%{version}` layout) that directory is the root; otherwise the archive root is used. > **Supported entry types:** Only regular files, directories, and symlinks are supported inside an archive overlay's target. If the archive contains an entry that cannot be repacked safely (a hardlink, device node, FIFO, etc.), the overlay fails with an error rather than silently dropping the entry from the repacked archive. diff --git a/internal/app/azldev/core/components/resolver.go b/internal/app/azldev/core/components/resolver.go index fad536a4..900f415c 100644 --- a/internal/app/azldev/core/components/resolver.go +++ b/internal/app/azldev/core/components/resolver.go @@ -877,6 +877,10 @@ func applyInheritedDefaultsToComponent( return nil, fmt.Errorf("resolving overlay files for component '%s':\n%w", component.Name, err) } + if err := projectconfig.ValidateArchiveOverlayOrigins(resolved); err != nil { + return nil, fmt.Errorf("validating archive overlays for component '%s':\n%w", component.Name, err) + } + return &resolved, nil } diff --git a/internal/projectconfig/component.go b/internal/projectconfig/component.go index 8603103f..353481fd 100644 --- a/internal/projectconfig/component.go +++ b/internal/projectconfig/component.go @@ -132,6 +132,29 @@ type SourceFileReference struct { ReplaceReason string `toml:"replace-reason,omitempty" json:"replaceReason,omitempty" jsonschema:"title=Replace reason,description=Required when 'replace-upstream' is true. Human-readable explanation for the replacement." fingerprint:"-"` } +// ValidateArchiveOverlayOrigins verifies that each archive modified by an +// archive-scoped overlay has a matching overlay-origin source file. The origin +// records the expected post-overlay hash used to protect render output from drift. +func ValidateArchiveOverlayOrigins(component ComponentConfig) error { + overlayOrigins := make(map[string]bool, len(component.SourceFiles)) + for _, sourceFile := range component.SourceFiles { + if sourceFile.Origin.Type == OriginTypeOverlay { + overlayOrigins[sourceFile.Filename] = true + } + } + + for _, overlay := range component.Overlays { + if overlay.ModifiesArchive() && !overlayOrigins[overlay.Archive] { + return fmt.Errorf( + "archive overlay for %#q requires a matching 'source-files' entry with 'origin.type = overlay'", + overlay.Archive, + ) + } + } + + return nil +} + // HashInclude implements the hashstructure [Includable] interface so that // [SourceFileReference.Origin] is omitted from the component fingerprint when // none of [Origin.Script], [Origin.MockPackages], or [Origin.Inputs] are set. diff --git a/internal/projectconfig/configfile.go b/internal/projectconfig/configfile.go index 65112f4a..11d38919 100644 --- a/internal/projectconfig/configfile.go +++ b/internal/projectconfig/configfile.go @@ -127,6 +127,10 @@ func (f ConfigFile) Validate() error { return err } + if err := ValidateArchiveOverlayOrigins(component); err != nil { + return fmt.Errorf("invalid archive overlays for component %#q:\n%w", componentName, err) + } + if component.Spec.UpstreamDistro.Snapshot != "" { return fmt.Errorf( "component %#q has a per-component 'snapshot' on 'upstream-distro'; "+ diff --git a/internal/projectconfig/configfile_test.go b/internal/projectconfig/configfile_test.go index d5c8047e..c22e77f3 100644 --- a/internal/projectconfig/configfile_test.go +++ b/internal/projectconfig/configfile_test.go @@ -115,6 +115,59 @@ func TestProjectConfigFileValidation_EmptySourceFiles(t *testing.T) { assert.NoError(t, file.Validate()) } +func TestProjectConfigFileValidation_ArchiveOverlayRequiresOverlayOrigin(t *testing.T) { + archiveOverlay := projectconfig.ComponentOverlay{ + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: "source.tar.gz", + Filename: "vendor/**", + } + overlayOrigin := projectconfig.SourceFileReference{ + Filename: "source.tar.gz", + Hash: "abc123", + HashType: fileutils.HashTypeSHA256, + Origin: projectconfig.Origin{Type: projectconfig.OriginTypeOverlay}, + ReplaceUpstream: true, + ReplaceReason: "Remove vendored files", + } + + tests := []struct { + name string + overlays []projectconfig.ComponentOverlay + sourceFiles []projectconfig.SourceFileReference + wantErr bool + }{ + {name: "no archive overlay"}, + {name: "missing overlay origin", overlays: []projectconfig.ComponentOverlay{archiveOverlay}, wantErr: true}, + { + name: "one origin supports multiple overlays for the archive", + overlays: []projectconfig.ComponentOverlay{archiveOverlay, archiveOverlay}, + sourceFiles: []projectconfig.SourceFileReference{overlayOrigin}, + }, + } + + for _, testCase := range tests { + t.Run(testCase.name, func(t *testing.T) { + file := projectconfig.ConfigFile{Components: map[string]projectconfig.ComponentConfig{ + "test-component": { + Overlays: testCase.overlays, + SourceFiles: testCase.sourceFiles, + }, + }} + + err := file.Validate() + if testCase.wantErr { + require.Error(t, err) + assert.Contains(t, err.Error(), "source.tar.gz") + assert.Contains(t, err.Error(), "origin.type = overlay") + + return + } + + require.NoError(t, err) + }) + } +} + func TestProjectConfigFileValidation_MD5HashTypeDisallowed(t *testing.T) { file := projectconfig.ConfigFile{ Components: map[string]projectconfig.ComponentConfig{ diff --git a/internal/providers/sourceproviders/sourcemanager.go b/internal/providers/sourceproviders/sourcemanager.go index 6f4492de..4538934a 100644 --- a/internal/providers/sourceproviders/sourcemanager.go +++ b/internal/providers/sourceproviders/sourcemanager.go @@ -396,19 +396,6 @@ func (m *sourceManager) fetchSourceFile( return nil } - sourceExists, err := fileutils.Exists(m.fs, destPath) - if err != nil { - return fmt.Errorf("failed to check existence of destination file %#q:\n%w", destPath, err) - } - - if sourceExists { - slog.Debug("Source file already exists, skipping download", - "filename", fileRef.Filename, - "path", destPath) - - return nil - } - // Try the lookaside cache first for non-custom files if hash info is available. // Custom files are always regenerated so stale configured hashes are detected. if m.trySourceFileLookaside(ctx, httpDownloader, component, fileRef, destPath) { From 08ddb93e5959a00dad387c68bd1338e5c111832f Mon Sep 17 00:00:00 2001 From: Antonio Salinas Date: Thu, 16 Jul 2026 18:10:25 +0000 Subject: [PATCH 4/5] Ensure efficent validation; comments --- internal/projectconfig/configfile.go | 4 -- internal/projectconfig/configfile_test.go | 53 ------------------- .../sourceproviders/fedorasourceprovider.go | 1 - 3 files changed, 58 deletions(-) diff --git a/internal/projectconfig/configfile.go b/internal/projectconfig/configfile.go index 11d38919..65112f4a 100644 --- a/internal/projectconfig/configfile.go +++ b/internal/projectconfig/configfile.go @@ -127,10 +127,6 @@ func (f ConfigFile) Validate() error { return err } - if err := ValidateArchiveOverlayOrigins(component); err != nil { - return fmt.Errorf("invalid archive overlays for component %#q:\n%w", componentName, err) - } - if component.Spec.UpstreamDistro.Snapshot != "" { return fmt.Errorf( "component %#q has a per-component 'snapshot' on 'upstream-distro'; "+ diff --git a/internal/projectconfig/configfile_test.go b/internal/projectconfig/configfile_test.go index c22e77f3..d5c8047e 100644 --- a/internal/projectconfig/configfile_test.go +++ b/internal/projectconfig/configfile_test.go @@ -115,59 +115,6 @@ func TestProjectConfigFileValidation_EmptySourceFiles(t *testing.T) { assert.NoError(t, file.Validate()) } -func TestProjectConfigFileValidation_ArchiveOverlayRequiresOverlayOrigin(t *testing.T) { - archiveOverlay := projectconfig.ComponentOverlay{ - Type: projectconfig.ComponentOverlayRemoveFile, - Archive: "source.tar.gz", - Filename: "vendor/**", - } - overlayOrigin := projectconfig.SourceFileReference{ - Filename: "source.tar.gz", - Hash: "abc123", - HashType: fileutils.HashTypeSHA256, - Origin: projectconfig.Origin{Type: projectconfig.OriginTypeOverlay}, - ReplaceUpstream: true, - ReplaceReason: "Remove vendored files", - } - - tests := []struct { - name string - overlays []projectconfig.ComponentOverlay - sourceFiles []projectconfig.SourceFileReference - wantErr bool - }{ - {name: "no archive overlay"}, - {name: "missing overlay origin", overlays: []projectconfig.ComponentOverlay{archiveOverlay}, wantErr: true}, - { - name: "one origin supports multiple overlays for the archive", - overlays: []projectconfig.ComponentOverlay{archiveOverlay, archiveOverlay}, - sourceFiles: []projectconfig.SourceFileReference{overlayOrigin}, - }, - } - - for _, testCase := range tests { - t.Run(testCase.name, func(t *testing.T) { - file := projectconfig.ConfigFile{Components: map[string]projectconfig.ComponentConfig{ - "test-component": { - Overlays: testCase.overlays, - SourceFiles: testCase.sourceFiles, - }, - }} - - err := file.Validate() - if testCase.wantErr { - require.Error(t, err) - assert.Contains(t, err.Error(), "source.tar.gz") - assert.Contains(t, err.Error(), "origin.type = overlay") - - return - } - - require.NoError(t, err) - }) - } -} - func TestProjectConfigFileValidation_MD5HashTypeDisallowed(t *testing.T) { file := projectconfig.ConfigFile{ Components: map[string]projectconfig.ComponentConfig{ diff --git a/internal/providers/sourceproviders/fedorasourceprovider.go b/internal/providers/sourceproviders/fedorasourceprovider.go index 3c43bed9..21018dad 100644 --- a/internal/providers/sourceproviders/fedorasourceprovider.go +++ b/internal/providers/sourceproviders/fedorasourceprovider.go @@ -140,7 +140,6 @@ func (g *FedoraSourcesProviderImpl) GetComponent( return fmt.Errorf("failed to clone git repository %#q:\n%w", gitRepoURL, err) } - // Collect filenames from source-files config so the lookaside extractor can skip them. // Collect filenames from source-files config so the lookaside extractor can skip them. // [SourceManager.FetchFiles] acquires the configured versions after component fetching. // Only files that FetchFiles actually downloads belong in this list — origin types that From f05fddbfcee303914d5a6ff95c9b68297ad7a4cc Mon Sep 17 00:00:00 2001 From: Antonio Salinas Date: Thu, 16 Jul 2026 18:33:09 +0000 Subject: [PATCH 5/5] Improved dev-ex by adding archive-overlay onboarding --- docs/user/reference/config/components.md | 4 +-- docs/user/reference/config/overlays.md | 4 ++- .../app/azldev/core/components/resolver.go | 4 --- .../core/sources/archiveoverlays_test.go | 9 ++++-- .../app/azldev/core/sources/sourceprep.go | 6 ++++ .../azldev/core/sources/sourceprep_test.go | 28 +++++++++++++++++-- internal/projectconfig/component.go | 4 +-- internal/projectconfig/configfile.go | 20 ++----------- internal/projectconfig/configfile_test.go | 15 ++++++++++ 9 files changed, 63 insertions(+), 31 deletions(-) diff --git a/docs/user/reference/config/components.md b/docs/user/reference/config/components.md index 029e131b..9f07e864 100644 --- a/docs/user/reference/config/components.md +++ b/docs/user/reference/config/components.md @@ -360,7 +360,7 @@ origin.inputs = ["yara-4.5.4.tar.gz"] # available to the script as ./ #### `"overlay"` — record a post-overlay hash -- **`hash` and `hash-type` are required** for `"overlay"` entries — the hash cannot be computed at render time because sources are not downloaded. +- **`hash` and `hash-type` are required** for normal use. To bootstrap a new archive overlay, omit both and run `prep-sources --allow-no-hashes` once; it computes the post-overlay hash for you. - **`replace-upstream = true` is required** — the archive already exists in the upstream `sources` file, and this entry replaces its hash with the post-overlay value. - During `prep-sources` (full run), azldev verifies that the hash it computed after repacking the archive matches the stated `hash`. A mismatch means the config is stale and must be updated. - During `render --check-only`, the stated hash is injected directly without downloading or repacking, allowing the check to pass deterministically. @@ -384,7 +384,7 @@ replace-reason = "Upstream source tarball contains test fixtures flagged as malw **Workflow:** 1. Add the archive overlay(s) in the component's `[[overlays]]` array. -2. Run `prep-sources` once — this repacks the archive and prints the computed hash in the error message. +2. Run `prep-sources --allow-no-hashes` once — this repacks the archive and writes the computed hash to the output `sources` file. 3. Paste the computed `hash` and `hash-type` into the `source-files` entry above. 4. Run `prep-sources` again to confirm the hash matches, then commit. diff --git a/docs/user/reference/config/overlays.md b/docs/user/reference/config/overlays.md index db1e763c..3b08a612 100644 --- a/docs/user/reference/config/overlays.md +++ b/docs/user/reference/config/overlays.md @@ -68,7 +68,9 @@ file = "vendor/**" # files inside the archive > **Required drift protection:** Every archive targeted by an archive overlay must have one matching > `source-files` entry with `origin.type = "overlay"`. The entry records the expected post-overlay -> hash; one entry can cover multiple overlays for the same archive. See [Components](components.md#recording-the-post-overlay-hash-for-archive-overlays). +> hash; one entry can cover multiple overlays for the same archive. While bootstrapping a new +> overlay, `prep-sources --allow-no-hashes` skips this association check and permits an origin +> entry to omit its hash. See [Components](components.md#recording-the-post-overlay-hash-for-archive-overlays). > **Extraction root:** The inner path is interpreted relative to the archive's extraction root: if the archive unpacks to a single top-level directory (the conventional `%{name}-%{version}` layout) that directory is the root; otherwise the archive root is used. diff --git a/internal/app/azldev/core/components/resolver.go b/internal/app/azldev/core/components/resolver.go index 900f415c..fad536a4 100644 --- a/internal/app/azldev/core/components/resolver.go +++ b/internal/app/azldev/core/components/resolver.go @@ -877,10 +877,6 @@ func applyInheritedDefaultsToComponent( return nil, fmt.Errorf("resolving overlay files for component '%s':\n%w", component.Name, err) } - if err := projectconfig.ValidateArchiveOverlayOrigins(resolved); err != nil { - return nil, fmt.Errorf("validating archive overlays for component '%s':\n%w", component.Name, err) - } - return &resolved, nil } diff --git a/internal/app/azldev/core/sources/archiveoverlays_test.go b/internal/app/azldev/core/sources/archiveoverlays_test.go index 85fc1dc8..563f7e07 100644 --- a/internal/app/azldev/core/sources/archiveoverlays_test.go +++ b/internal/app/azldev/core/sources/archiveoverlays_test.go @@ -215,7 +215,7 @@ func TestPrepareSources_RemoveFileGlob_Archive(t *testing.T) { }, ) - preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx, sources.WithAllowNoHashes()) require.NoError(t, err) err = preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/) @@ -341,7 +341,7 @@ func TestPrepareSources_SearchReplaceInArchiveRehashesEntry(t *testing.T) { }, ) - preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx, sources.WithAllowNoHashes()) require.NoError(t, err) require.NoError(t, preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/)) @@ -422,7 +422,10 @@ func TestPrepareSources_SkipSourcesSkipsArchiveOverlays(t *testing.T) { }, ) - preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx, sources.WithSkipLookaside()) + preparer, err := sources.NewPreparer( + sourceManager, ctx.FS(), ctx, ctx, + sources.WithSkipLookaside(), sources.WithAllowNoHashes(), + ) require.NoError(t, err) require.NoError(t, preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/)) diff --git a/internal/app/azldev/core/sources/sourceprep.go b/internal/app/azldev/core/sources/sourceprep.go index f46379fa..9697a9d5 100644 --- a/internal/app/azldev/core/sources/sourceprep.go +++ b/internal/app/azldev/core/sources/sourceprep.go @@ -215,6 +215,12 @@ func NewPreparer( func (p *sourcePreparerImpl) PrepareSources( ctx context.Context, component components.Component, outputDir string, applyOverlays bool, ) error { + if applyOverlays && !p.allowNoHashes { + if err := projectconfig.ValidateArchiveOverlayOrigins(*component.GetConfig()); err != nil { + return fmt.Errorf("invalid archive overlays for component %#q:\n%w", component.GetName(), err) + } + } + // Preserve the upstream .git directory only when dist-git creation is // requested via --with-git. This is required so that overlay commits can be // appended on top of the upstream commit log during synthetic history generation. diff --git a/internal/app/azldev/core/sources/sourceprep_test.go b/internal/app/azldev/core/sources/sourceprep_test.go index eb30cf53..72ab47de 100644 --- a/internal/app/azldev/core/sources/sourceprep_test.go +++ b/internal/app/azldev/core/sources/sourceprep_test.go @@ -172,7 +172,9 @@ func TestPrepareSources_ArchiveOverlayRehashesSourcesEntry(t *testing.T) { }, ) - preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + // '--allow-no-hashes' permits bootstrapping an archive overlay before its + // overlay-origin source-file entry and post-overlay hash are configured. + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx, sources.WithAllowNoHashes()) require.NoError(t, err) err = preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/) @@ -210,6 +212,28 @@ func TestPrepareSources_ArchiveOverlayRehashesSourcesEntry(t *testing.T) { assert.NotEqual(t, originalHash, entry.Hash, "'sources' entry hash must have been updated") } +func TestPrepareSources_ArchiveOverlayRequiresOverlayOrigin(t *testing.T) { + ctrl := gomock.NewController(t) + component := components_testutils.NewMockComponent(ctrl) + sourceManager := sourceproviders_test.NewMockSourceManager(ctrl) + ctx := testctx.NewCtx() + + component.EXPECT().GetName().AnyTimes().Return("test-component") + component.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{ + Overlays: []projectconfig.ComponentOverlay{{ + Type: projectconfig.ComponentOverlayRemoveFile, + Archive: "source.tar.gz", + Filename: "vendor/**", + }}, + }) + + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + require.NoError(t, err) + + err = preparer.PrepareSources(ctx, component, testOutputDir, true) + require.ErrorContains(t, err, "origin.type = overlay") +} + // TestPrepareSources_ArchiveOverlayMissingSourcesEntryErrors verifies that when an // archive-scoped overlay repacks an archive that has no matching 'sources' entry, // preparation fails instead of silently leaving a stale (or absent) digest. @@ -264,7 +288,7 @@ func TestPrepareSources_ArchiveOverlayMissingSourcesEntryErrors(t *testing.T) { }, ) - preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx) + preparer, err := sources.NewPreparer(sourceManager, ctx.FS(), ctx, ctx, sources.WithAllowNoHashes()) require.NoError(t, err) err = preparer.PrepareSources(ctx, component, outputDir, true /*applyOverlays*/) diff --git a/internal/projectconfig/component.go b/internal/projectconfig/component.go index 353481fd..a8dd2ded 100644 --- a/internal/projectconfig/component.go +++ b/internal/projectconfig/component.go @@ -51,8 +51,8 @@ const ( // OriginTypeOverlay indicates that the source file's hash was changed by archive overlays. // No download occurs; the file is already present as a spec source. The 'hash' and 'hash-type' - // fields record the expected post-overlay hash, which is injected into the 'sources' file - // during render (skip-lookaside) and validated against the computed hash during 'prep-sources'. + // fields record the expected post-overlay hash, or can be omitted temporarily with + // '--allow-no-hashes' while source preparation computes the initial value. OriginTypeOverlay OriginType = "overlay" ) diff --git a/internal/projectconfig/configfile.go b/internal/projectconfig/configfile.go index 65112f4a..a96829b3 100644 --- a/internal/projectconfig/configfile.go +++ b/internal/projectconfig/configfile.go @@ -230,24 +230,10 @@ func validateSourceFiles(sourceFiles []SourceFileReference, componentName string } // validateOverlayOriginRef enforces additional constraints on [SourceFileReference] entries -// with [OriginTypeOverlay]: 'hash', 'hash-type', and 'replace-upstream = true' are all required. -// These entries record the post-overlay hash of an archive that is already present as a spec -// source, so a download is never performed and the hash cannot be computed on the fly. +// with [OriginTypeOverlay]. 'replace-upstream = true' is required because the archive is +// already present as a spec source. Hashes may be omitted while bootstrapping an archive +// overlay with '--allow-no-hashes'; source preparation computes the post-overlay hash. func validateOverlayOriginRef(ref SourceFileReference, componentName string) error { - if ref.Hash == "" { - return fmt.Errorf( - "source file %#q in component %#q has 'origin.type = overlay' but no 'hash'; "+ - "'hash' is required to record the expected post-overlay archive hash", - ref.Filename, componentName) - } - - if ref.HashType == "" { - return fmt.Errorf( - "source file %#q in component %#q has 'origin.type = overlay' but no 'hash-type'; "+ - "'hash-type' is required when 'hash' is provided", - ref.Filename, componentName) - } - if !ref.ReplaceUpstream { return fmt.Errorf( "source file %#q in component %#q has 'origin.type = overlay' but 'replace-upstream' is not true; "+ diff --git a/internal/projectconfig/configfile_test.go b/internal/projectconfig/configfile_test.go index d5c8047e..054498a1 100644 --- a/internal/projectconfig/configfile_test.go +++ b/internal/projectconfig/configfile_test.go @@ -115,6 +115,21 @@ func TestProjectConfigFileValidation_EmptySourceFiles(t *testing.T) { assert.NoError(t, file.Validate()) } +func TestProjectConfigFileValidation_OverlayOriginMayOmitHash(t *testing.T) { + file := projectconfig.ConfigFile{Components: map[string]projectconfig.ComponentConfig{ + "test-component": { + SourceFiles: []projectconfig.SourceFileReference{{ + Filename: "source.tar.gz", + Origin: projectconfig.Origin{Type: projectconfig.OriginTypeOverlay}, + ReplaceUpstream: true, + ReplaceReason: "Record an archive overlay hash", + }}, + }, + }} + + assert.NoError(t, file.Validate()) +} + func TestProjectConfigFileValidation_MD5HashTypeDisallowed(t *testing.T) { file := projectconfig.ConfigFile{ Components: map[string]projectconfig.ComponentConfig{