local repo by samueltaripin · Pull Request #510 · open-edge-platform/os-image-composer

samueltaripin · 2026-04-08T01:51:15Z

Merge Checklist

All boxes should be checked before merging the PR

[ / ] The changes in the PR have been built and tested
[ / ] Documentation has been updated to reflect the changes (or no doc update needed)
[ / ] Ready to merge

Description

Includes local repo

Any Newly Introduced Dependencies

How Has This Been Tested?

Signed-off-by: Mah, Yock Gen <yock.gen.mah@intel.com>

Copilot

Pull request overview

Adds support for using local filesystem directories as package repositories (via a new path field) by generating/serving temporary DEB/RPM repositories over HTTP, and updates schema + tests to validate the new template shape.

Changes:

Introduces a temporary HTTP file server helper to serve local repositories.
Adds path support to the image template schema/config and wires local repo package discovery into DEB/RPM download flows.
Expands unit tests across providers, config/schema validation, and package utility helpers.

Reviewed changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated 14 comments.

Show a summary per file

File	Description
internal/utils/network/httpserver.go	New helper to serve a repository directory over HTTP with shutdown support.
internal/utils/network/httpserver_test.go	Tests for the temporary HTTP server behavior.
internal/provider/ubuntu/ubuntu.go	Adds `dpkg-scanpackages` host dependency needed for local DEB repo metadata generation.
internal/provider/ubuntu/ubuntu_test.go	Adds tests for repo list building and error wrapping/host dependency behaviors.
internal/provider/rcd/rcd_test.go	Adds tests for PostProcess cleanup and host dependency behaviors (and related mocks).
internal/provider/elxr/elxr.go	Adds `dpkg-scanpackages` host dependency for local DEB repo metadata generation.
internal/provider/elxr/elxr_test.go	Adds tests for PostProcess cleanup and host dependency behaviors (and related mocks).
internal/ospackage/rpmutils/verify.go	Adds special-casing for `[trusted=yes]` during RPM verification.
internal/ospackage/rpmutils/helper.go	Adds RPM `CreateTemporaryRepository` to build + serve a local RPM repo.
internal/ospackage/rpmutils/helper_test.go	Adds tests for local RPM repo creation helpers and local user package handling.
internal/ospackage/rpmutils/download.go	Adds `Path` field + local repo package ingestion (`LocalUserPackages`) into RPM download flow.
internal/ospackage/debutils/zip_test.go	Adds tests for XZ decompression and dispatcher behavior.
internal/ospackage/debutils/resolver_internal_test.go	Adds tests for internal resolver/helper functions (glob/filter/url join).
internal/ospackage/debutils/helper.go	Adds DEB `CreateTemporaryRepository` to build + serve a local Debian repo (Packages/Release).
internal/ospackage/debutils/helper_test.go	Adds tests for DEB temp repo creation and local user package handling.
internal/ospackage/debutils/download.go	Adds `Path` field and local repo package ingestion (`LocalUserPackages`) into DEB download flow.
internal/image/imageos/imageos_test.go	Adds additional error-path tests for initramfs/sysfs/ESP/verity helper logic.
internal/config/version/version_test.go	Adds tests asserting default build metadata values.
internal/config/validate/validate_test.go	Adds schema validation test for path-based repositories with `[trusted=yes]`.
internal/config/schema/os-image-template.schema.json	Extends schema to allow `path` repositories and enforces `url` XOR `path`.
internal/config/schema/embed_test.go	Adds tests ensuring embedded schemas are present and valid JSON with expected metadata.
internal/config/config.go	Adds `path` to `PackageRepository` and a `ValidatePackageRepository` helper.
internal/config/config_test.go	Adds YAML parsing test for path-based repositories.
internal/config/apt_sources_test.go	Adds tests ensuring path-only repos don’t generate apt sources; adds nil-input test for normalization.
image-templates/ubuntu24-x86_64-edge-raw.yml	Adds commented example for a local path-based repo.

Copilot · 2026-04-13T01:54:23Z

internal/ospackage/rpmutils/helper.go

+	// Create temporary repository directory
+	tempRepoPath := filepath.Join("/tmp", fmt.Sprintf("rpmrepo_%s_%d", repoName, time.Now().Unix()))
+	if err := os.MkdirAll(tempRepoPath, 0755); err != nil {
+		return "", "", nil, fmt.Errorf("failed to create temporary repository directory: %w", err)
+	}


The temporary repo directory name is predictable (/tmp/..._<unix seconds>) and is created with MkdirAll. In a multi-user environment this is vulnerable to pre-creation/symlink attacks and can collide under concurrency. Prefer os.MkdirTemp (or similar) to create a unique, securely-created directory under the OS temp dir.

Copilot · 2026-04-13T01:54:23Z

internal/ospackage/debutils/helper.go

+	tempRepoPath := filepath.Join("/tmp", fmt.Sprintf("debrepo_%s_%d", repoName, time.Now().Unix()))
+	if err := os.MkdirAll(tempRepoPath, 0755); err != nil {


The temporary repo directory name is predictable (/tmp/..._<unix seconds>) and is created with MkdirAll. In a multi-user environment this is vulnerable to pre-creation/symlink attacks and can collide under concurrency. Prefer os.MkdirTemp (or similar) to create a unique, securely-created directory under the OS temp dir.

Suggested change

tempRepoPath := filepath.Join("/tmp", fmt.Sprintf("debrepo_%s_%d", repoName, time.Now().Unix()))

if err := os.MkdirAll(tempRepoPath, 0755); err != nil {

safeRepoName := strings.NewReplacer(" ", "_", string(os.PathSeparator), "_").Replace(repoName)

tempRepoPath, err := os.MkdirTemp("", "debrepo_"+safeRepoName+"_")

if err != nil {

Copilot · 2026-04-13T01:54:24Z

internal/utils/network/httpserver.go

+	// Validate repository path exists
+	if _, err := os.Stat(repoPath); os.IsNotExist(err) {
+		return "", nil, fmt.Errorf("repository directory does not exist: %s", repoPath)
+	} else if err != nil {
+		return "", nil, fmt.Errorf("failed to access repository directory: %w", err)
+	}
+


repoPath is only checked for existence, but not that it is a directory. If a file path is provided, http.Dir(repoPath) won’t behave as intended and the error message (“repository directory…”) becomes misleading. Consider verifying Stat().IsDir() and returning a clear error when the path is not a directory.

Suggested change

// Validate repository path exists

if _, err := os.Stat(repoPath); os.IsNotExist(err) {

return "", nil, fmt.Errorf("repository directory does not exist: %s", repoPath)

} else if err != nil {

return "", nil, fmt.Errorf("failed to access repository directory: %w", err)

}

// Validate repository path exists and is a directory

repoInfo, err := os.Stat(repoPath)

if os.IsNotExist(err) {

return "", nil, fmt.Errorf("repository directory does not exist: %s", repoPath)

} else if err != nil {

return "", nil, fmt.Errorf("failed to access repository directory: %w", err)

}

if !repoInfo.IsDir() {

return "", nil, fmt.Errorf("repository path is not a directory: %s", repoPath)

}

Copilot · 2026-04-13T01:54:24Z

internal/utils/network/httpserver.go

+	listener, err := net.Listen("tcp", ":0")
+	if err != nil {
+		return "", nil, fmt.Errorf("failed to find available port: %w", err)
+	}
+	port := listener.Addr().(*net.TCPAddr).Port


Selecting a port by listening on :0 and then closing the listener introduces a race: another process can claim the port before ListenAndServe re-binds. Prefer creating the listener on 127.0.0.1:0 and starting the server via server.Serve(listener) so the same bound socket is used.

Copilot · 2026-04-13T01:54:24Z

internal/utils/network/httpserver.go

+	server := &http.Server{
+		Addr:    ":" + strconv.Itoa(port),
+		Handler: mux,


Addr: ":<port>" binds on all interfaces, which can expose the served repository to the network even though the returned URL is localhost. Bind explicitly to 127.0.0.1 (or localhost) to keep this temporary server local-only.

Copilot · 2026-04-13T01:54:25Z

internal/ospackage/rpmutils/download.go

+
+// In LocalUserPackages(), before the main processing loop:
+// for _, rpItx := range localRepo {
+//     if rpItx.Path == "" {
+//         continue
+//     }
+//
+//     // Check if it's a proper repository
+//     repoMetaDataPath := filepath.Join(rpItx.Path, "repodata/repomd.xml")
+//     if _, err := os.Stat(repoMetaDataPath); os.IsNotExist(err) {
+//         // Not a proper repo - need to create one
+//         tempRepoPath, _, cleanup, err := CreateTemporaryRepository(rpItx.Path, rpItx.Name)
+//         if err != nil {
+//             return nil, fmt.Errorf("failed to create temporary repository: %w", err)
+//         }
+//         // Store cleanup function for later use if needed
+//         _ = cleanup
+//         // Update the path to point to the new temp repo
+//         rpItx.Path = tempRepoPath
+//     }
+//
+//     // Continue with existing repo processing...
+// }


This commented-out pseudo-code block appears to be leftover implementation notes and makes the file harder to maintain. Please remove it now that LocalUserPackages() is implemented.

Suggested change

// In LocalUserPackages(), before the main processing loop:

// for _, rpItx := range localRepo {

// if rpItx.Path == "" {

// continue

// }

//

// // Check if it's a proper repository

// repoMetaDataPath := filepath.Join(rpItx.Path, "repodata/repomd.xml")

// if _, err := os.Stat(repoMetaDataPath); os.IsNotExist(err) {

// // Not a proper repo - need to create one

// tempRepoPath, _, cleanup, err := CreateTemporaryRepository(rpItx.Path, rpItx.Name)

// if err != nil {

// return nil, fmt.Errorf("failed to create temporary repository: %w", err)

// }

// // Store cleanup function for later use if needed

// _ = cleanup

// // Update the path to point to the new temp repo

// rpItx.Path = tempRepoPath

// }

//

// // Continue with existing repo processing...

// }

Copilot · 2026-04-13T01:54:26Z

internal/ospackage/rpmutils/verify.go

+	log := logger.Logger()
+	//ignore verification if trusted=yes
+	if pubkeyPath == "[trusted=yes]" {
+		log.Infof("Repository marked (%s) as [trusted=yes], skipping Release file signature verification", rpmPath)
+		return nil


[trusted=yes] handling here is currently ineffective: createTempGPGKeyFiles() skips entries equal to [trusted=yes], so VerifyAll() will never pass that value into verifyWithGoRpm(). If the intent is to allow skipping RPM signature verification, handle it explicitly in Validate() (e.g., detect the sentinel and bypass verification) and update the log message to refer to RPM signature verification (not “Release file”).

Copilot · 2026-04-13T01:54:26Z

internal/config/config.go

+
+// ValidatePackageRepository validates that either URL or Path is provided
+func (pr *PackageRepository) ValidatePackageRepository() error {
+	if pr.URL == "" && pr.Path == "" {
+		return fmt.Errorf("repository '%s': either 'url' or 'path' must be provided", pr.Codename)
+	}
+	if pr.URL != "" && pr.Path != "" {
+		return fmt.Errorf("repository '%s': cannot specify both 'url' and 'path', choose one", pr.Codename)
+	}
+	return nil
+}


ValidatePackageRepository is added but isn’t called anywhere in the codebase, so the URL-vs-Path constraint is not enforced at runtime unless schema validation is always run beforehand. Either wire this into template load/validation (e.g., validate repositories during LoadTemplate), or remove the unused method to avoid dead code.

Suggested change

// ValidatePackageRepository validates that either URL or Path is provided

func (pr *PackageRepository) ValidatePackageRepository() error {

if pr.URL == "" && pr.Path == "" {

return fmt.Errorf("repository '%s': either 'url' or 'path' must be provided", pr.Codename)

}

if pr.URL != "" && pr.Path != "" {

return fmt.Errorf("repository '%s': cannot specify both 'url' and 'path', choose one", pr.Codename)

}

return nil

}

Copilot · 2026-04-13T01:54:26Z

internal/ospackage/debutils/helper_test.go

+// TestCreateTemporaryRepositorySuccess tests CreateTemporaryRepository with valid DEB files
+func TestCreateTemporaryRepositorySuccess(t *testing.T) {


Despite the name, this test does not exercise or validate the success path. Rename it to reflect what it actually verifies, or create the expected metadata files on disk so the true success path is tested.

Suggested change

// TestCreateTemporaryRepositorySuccess tests CreateTemporaryRepository with valid DEB files

func TestCreateTemporaryRepositorySuccess(t *testing.T) {

// TestCreateTemporaryRepositoryFailsWhenMetadataMissing tests that

// CreateTemporaryRepository returns an error when mocked commands do not

// create the repository metadata files the function validates.

func TestCreateTemporaryRepositoryFailsWhenMetadataMissing(t *testing.T) {

Copilot · 2026-04-13T01:54:26Z

internal/ospackage/debutils/helper_test.go

+	// Test that the repository paths would be different (from the temp directory structure)
+	// Even though the function fails, the initial path creation should use unique names
+	t.Log("This test verifies unique temporary directory naming with mocked commands")
+}


This test doesn’t assert anything about directory uniqueness (it only logs). Consider capturing and comparing the returned repoPath values to ensure they differ, or remove the test if it can’t make a meaningful assertion with the current mocking approach.

yockgen and others added 19 commits March 19, 2026 13:50

Adding local package repositories interface

e154f57

Signed-off-by: Mah, Yock Gen <yock.gen.mah@intel.com>

Merge branch 'main' into pg-dev-localrepo

723ff90

Adding create temp local repo interfaces

4b95e78

Signed-off-by: Mah, Yock Gen <yock.gen.mah@intel.com>

Adding rpm repo creation and web repo functions

31ad515

Signed-off-by: Mah, Yock Gen <yock.gen.mah@intel.com>

Merge branch 'main' into pg-dev-localrepo

7be172d

Enabling local repo creation tool for rpm

14e316e

Signed-off-by: Mah, Yock Gen <yock.gen.mah@intel.com>

Adding debian local repo creation functions

82a1573

Signed-off-by: Mah, Yock Gen <yock.gen.mah@intel.com>

fix local repo

2dd7176

Merge branch 'main' into pg-dev-localrepo

2136f2f

Merge branch 'main' into pg-dev-localrepo

823d964

fix schema

fe8ef68

Merge branch 'main' into pg-dev-localrepo

70a7ab3

remove not required host dependency

c10ef12

increase unit test coverage

faae574

increase unit test coverage

68138f3

create unit test for schema

1558e8f

increase unit test for weak path

3196133

increase unit test coverage

b82fd29

Merge branch 'main' into pg-dev-localrepo

87994f0

samueltaripin marked this pull request as ready for review April 13, 2026 01:46

samueltaripin requested a review from a team as a code owner April 13, 2026 01:46

Copilot AI review requested due to automatic review settings April 13, 2026 01:46

Copilot started reviewing on behalf of samueltaripin April 13, 2026 01:46 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

fix local repo for rpm

7a69a6a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

local repo#510

local repo#510
samueltaripin wants to merge 20 commits intomainfrom
pg-dev-localrepo

samueltaripin commented Apr 8, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Copilot AI Apr 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		tempRepoPath := filepath.Join("/tmp", fmt.Sprintf("debrepo_%s_%d", repoName, time.Now().Unix()))
		if err := os.MkdirAll(tempRepoPath, 0755); err != nil {

		// TestCreateTemporaryRepositorySuccess tests CreateTemporaryRepository with valid DEB files
		func TestCreateTemporaryRepositorySuccess(t *testing.T) {

-// TestCreateTemporaryRepositorySuccess tests CreateTemporaryRepository with valid DEB files
-func TestCreateTemporaryRepositorySuccess(t *testing.T) {
+// TestCreateTemporaryRepositoryFailsWhenMetadataMissing tests that
+// CreateTemporaryRepository returns an error when mocked commands do not
+// create the repository metadata files the function validates.
+func TestCreateTemporaryRepositoryFailsWhenMetadataMissing(t *testing.T) {

Conversation

samueltaripin commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge Checklist

Description

Any Newly Introduced Dependencies

How Has This Been Tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

samueltaripin commented Apr 8, 2026 •

edited

Loading