diff --git a/docs/user/reference/config/components.md b/docs/user/reference/config/components.md index 67866951..9f07e864 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 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. + +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 --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. + +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/docs/user/reference/config/overlays.md b/docs/user/reference/config/overlays.md index 5685a4ec..3b08a612 100644 --- a/docs/user/reference/config/overlays.md +++ b/docs/user/reference/config/overlays.md @@ -66,6 +66,12 @@ 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. 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. > **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/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..563f7e07 --- /dev/null +++ b/internal/app/azldev/core/sources/archiveoverlays_test.go @@ -0,0 +1,446 @@ +// 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, sources.WithAllowNoHashes()) + 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, sources.WithAllowNoHashes()) + 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(), sources.WithAllowNoHashes(), + ) + 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..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. @@ -243,12 +249,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 +276,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 +294,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 +648,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 +680,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 +701,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 +740,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 +885,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 +1277,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..72ab47de 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,202 @@ 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) + }, + ) + + // '--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*/) + 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") +} + +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. +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, sources.WithAllowNoHashes()) + 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 +1057,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..a8dd2ded 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, or can be omitted temporarily with + // '--allow-no-hashes' while source preparation computes the initial value. + 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:"-"` @@ -119,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 851f1607..a96829b3 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,27 @@ 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]. '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.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 +354,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 +396,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/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{ 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; "+ diff --git a/internal/providers/sourceproviders/fedorasourceprovider.go b/internal/providers/sourceproviders/fedorasourceprovider.go index 688c66e2..21018dad 100644 --- a/internal/providers/sourceproviders/fedorasourceprovider.go +++ b/internal/providers/sourceproviders/fedorasourceprovider.go @@ -142,11 +142,17 @@ func (g *FedoraSourcesProviderImpl) GetComponent( // 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..4538934a 100644 --- a/internal/providers/sourceproviders/sourcemanager.go +++ b/internal/providers/sourceproviders/sourcemanager.go @@ -389,6 +389,13 @@ 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 + } + // 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 +404,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 +453,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 +557,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"