feat(find-images): 4509 include archives in find image by chaospuppy · Pull Request #4551 · zarf-dev/zarf

chaospuppy · 2026-01-23T20:31:09Z

Description

This MR resolves #4577 by doing the following:

Allow schema validation to succeed by removing findings related to an empty image list under imageArchives keys. Currently this finding is only removed when calling PackageDefinition from FindImages
Add FindImagesInArchive to identify and return all images in imageArchives.path
Update unit tests

Related Issue

Fixes #4577

Checklist before merging

Test, docs, adr added or updated as needed
Contributor Guide Steps followed

netlify · 2026-01-23T20:31:16Z

✅ Deploy Preview for zarf-docs ready!

Name	Link
🔨 Latest commit	`439a463`
🔍 Latest deploy log	https://app.netlify.com/projects/zarf-docs/deploys/69ec06f511d0680008ccbe2c
😎 Deploy Preview	https://deploy-preview-4551--zarf-docs.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

chaospuppy · 2026-01-23T20:33:26Z

@AustinAbro321 when you have a moment, please take a look. I am still going to update a few unit tests.

codecov · 2026-01-27T21:04:01Z

Codecov Report

❌ Patch coverage is 56.83761% with 101 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/cmd/dev.go	0.00%	38 Missing ⚠️
src/pkg/packager/find_images.go	67.56%	16 Missing and 8 partials ⚠️
src/pkg/images/unpack.go	64.15%	12 Missing and 7 partials ⚠️
src/pkg/packager/update.go	76.27%	9 Missing and 5 partials ⚠️
src/pkg/images/common.go	40.00%	4 Missing and 2 partials ⚠️

Files with missing lines	Coverage Δ
src/pkg/packager/load/load.go	`43.02% <ø> (ø)`
src/pkg/images/common.go	`35.87% <40.00%> (+0.34%)`	⬆️
src/pkg/packager/update.go	`55.55% <76.27%> (+7.59%)`	⬆️
src/pkg/images/unpack.go	`59.74% <64.15%> (+5.64%)`	⬆️
src/pkg/packager/find_images.go	`57.98% <67.56%> (+2.36%)`	⬆️
src/cmd/dev.go	`44.02% <0.00%> (-1.79%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

AustinAbro321

Good start! Thanks for taking this on

AustinAbro321 · 2026-01-27T21:30:26Z

+// It returns a list of images found in the extracted OCI layout.
+func FindImagesInArchive(ctx context.Context, imageArchive, destDir string) ([]string, error) {
+	// Create a temporary directory for extraction
+	tmpdir, err := utils.MakeTempDir("")


The extraction logic in this block is mostly duplicated. Lets make a common function to de-duplicate it

Made a few changes with this in mind

AustinAbro321 · 2026-01-27T21:35:15Z

 	// SkipVersionCheck skips version requirement validation
 	SkipVersionCheck bool
+	// SkipImageArchivesImages ignores schema validation errors when imageArchives does not include an images list
+	SkipEmptyImageArchivesImages bool


I'd prefer a strategy where we create a function load.PackageDefinitionNoValidate() then also make a public function load.Validate(). Keep the load.PackageDefinition function this way validation happens by default.

I believe a public validate function could be generally useful for SDK users who might be creating packages without zarf.yaml files, additionally it'll leave more room in the future for more generic solutions if we need to skip validation for a similar reason

I'm happy to make that split still, but our failure was while validating the file itself here, rather than during the validation of v1alpha1.ZarfPackage. Therefore, it wasn't useful for us to delay validation after adding a placeholder image as we previously discussed to the v1alpha1.ZarfPackage.

I could split them and then add some work to change the zarf.ymal on disk to add an empty array there, but that didn't seem like desirable behavior.

Alternatively, I could also split out a Validate and ValidateWithoutSchemaValidation (not literally that name, but something like that) and use the later

AustinAbro321 · 2026-01-27T21:42:08Z

+				return nil, fmt.Errorf("failed to unpack image archive %s: %w", archive.Path, err)
+			}
+			for _, image := range archiveImages {
+				matchedImages[image] = true


I think matched images isn't the right construct. Maybe a new field should be added such as archiveImages. One of the goals of find-images is to be able to copy and paste the output into your zarf.yaml. Currently you'd see this output, which would result in trying to pull the image from the internet if you copied and pasted it

2026-01-27 16:38:42 INF looking up cosign artifacts for discovered images count=3 components: - name: kiwix-serve images: - docker.io/library/kiwix-data:local - ghcr.io/kiwix/kiwix-serve:3.5.0-2 - kiwix-data:local

I see, good point, I missed the desire to pull images from the image archive on disk after zarf.yaml has been updated with the output of zarf dev find-images. I'll fix that up!

@AustinAbro321 to clarify, are you hoping to see images elements expressed in a such as way as to direct Zarf to pull them from the archive, rather than from the internet?

Ah I missed your comment before asking, I think I see the answer in #4577

Just went back and clarified #4577 a bit, but just to write here as well, we should no longer include images because they're in an image archive. If they're in both a manifest we should check if the image archive contains that image, then in the output to the user structure it so it's in the image archive, and the user can simply copy and paste.

AustinAbro321 · 2026-02-02T15:20:47Z

Hello @chaospuppy, I reazlied that the behavior I proposed in #4509, didn't really fit the zarf dev find-images command. However, there is still work to be done to get image archives to behave properly for zarf dev find-images. I've created #4577 to track this work. If you're interested, I believe this PR could be adjusted for that issue. LMK if you have any questions.

If you do adjust this PR for #4577, don't worry about validation, assume the given zarf.yaml file is valid

chaospuppy · 2026-02-06T19:11:51Z

@AustinAbro321 Are you thinking about handling validation differently in the near future? Trying to get an understanding for which changes I should back out. I am also happy to keep the validation changes in, including making Validation public and creating PackageDefinitionWithoutValidation.

AustinAbro321 · 2026-02-09T13:02:38Z

@chaospuppy Very possibly handling validation in the near future yes. I suggest backing out of the validation changes in this PR

…eholder images list, add skipimagearchivesimages definition option Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

…efinitionOption for skipping imagearchives images schema findings Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

…es output Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

…dev#4554) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

…-organization group (zarf-dev#4553) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

…ev#4552) Signed-off-by: Jason Washburn <jason.washburn@gmail.com> Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

Signed-off-by: Austin Abro <austinabro321@gmail.com> Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

AustinAbro321

Good progress. When running this locally on examples/kiwix I get

components:
  - name: kiwix-serve
    images:
      - ghcr.io/kiwix/kiwix-serve:3.5.0-2
      - kiwix-data:local
  # Archive images - kiwix-serve
  - name: kiwix-serve
    imageArchives:
      - path: kiwix-data.tar
        images: []

Whereas I'd expect

  - name: kiwix-serve
    images:
      - ghcr.io/kiwix/kiwix-serve:3.5.0-2
    imageArchives:
      - path: kiwix-data.tar
        images: 
         - kiwix-data:local

The kiwix-data:local is in the wrong spot, and they are printed as different components.

AustinAbro321 · 2026-03-04T16:22:48Z

+
+type ImageArchivesScan struct {
+	ComponentName string
+	ImageArchives []ImageArchive


I think they are coupled in that if image archives were to change, we'd also want to change this block since ultimately we'd want to use the information here to update / print a package definition

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

chaospuppy · 2026-03-04T19:58:42Z

@AustinAbro321 I've resolved the duplicate components issue, thanks for spotting that. To your point about kiwix-data:local missing from imageArchives, I'm wondering how we want to handle that exactly. Since creating a tarball of an image tagged without a registry with docker save always results in an index.json io.containerd.image.name that is pre-pended with docker.io/library/, there is some ambiguity over if kiwix-data:local really does correspond to docker.io/library/kiwix-data:local found in the archive. The user could also be looking for ghcr.io/kiwix/kiwix-data:local, for example. Are do we also want to assume that images without registries in their references refer to docker.io/library images?

AustinAbro321 · 2026-03-04T20:54:48Z

So a fun thing about containers is that if the registry is not specified then it's always assumed to be docker.io. Docker was the registry that popularized containers, so they made themselves the default. Generally the community wants to move away from this. IMO it would be a positive change if the find images command started returning the full path, so instead of kiwix-data:local, it'd return docker.io/library/kiwix-data:local. This should simplify things for you as well.

You should be able to use the transform.ParseImageRef library to get the qualified name.

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

chaospuppy · 2026-03-05T17:43:46Z

@AustinAbro321 updated to return qualified names for scan Matches. I did not do the same for PotentialMatches since if I understand the use case for PotentialMatches, we wouldn't want to push an image to the zarf registry from the archive that isn't actually used.

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

…ind-image

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

…-in-find-image

AustinAbro321

Good progress, some more requests as I go through

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

…ind-image

…ind-image Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

…ind-image

AustinAbro321

Requesting an architecture change. Appreciate the iterations

…ind-image

… generation and dev Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

chaospuppy · 2026-04-25T00:38:01Z

+			allArchiveImages = append(allArchiveImages, archiveImages...)
+		}
+
+		definitionImageResults = append(definitionImageResults, result)


I considered excluding results that were only populated with ComponentName.

I think it's fine to assume findImages won't return empty scans

AustinAbro321

The architecture is shaping up, a few more comments

AustinAbro321 · 2026-04-27T13:58:59Z

+// imageArchives.
+// It returns []DefinitionImageResult
+func FindDefinitionImages(ctx context.Context, packagePath string, opts FindImagesOptions) ([]DefinitionImageResult, error) {
+	loadOpts := load.DefinitionOptions{}


You need to pass all of the relevant loadOptions here

AustinAbro321 · 2026-04-27T13:59:58Z

+			componentDefinition += fmt.Sprintf("  # Archive images - %s\n", finding.ComponentName)
+			componentDefinition += "    imageArchives:\n"


This part needs to be outside of the loop so it works for multiple image archives in the same component

AustinAbro321 · 2026-04-27T14:01:25Z

+		}
+
+		for _, archive := range component.ImageArchives {
+			archivePath := path.Join(pkgPath.BaseDir, archive.Path)


Suggested change

archivePath := path.Join(pkgPath.BaseDir, archive.Path)

archivePath := filepath.Join(pkgPath.BaseDir, archive.Path)

path is used for consistent separators, where filepath changes the separator depending on the OS so we want the latter here

AustinAbro321 · 2026-04-27T14:29:10Z

+			allArchiveImages = append(allArchiveImages, archiveImages...)
+		}
+
+		definitionImageResults = append(definitionImageResults, result)


I think it's fine to assume findImages won't return empty scans

github-project-automation Bot added this to Zarf Jan 23, 2026

chaospuppy force-pushed the 4509-include-archives-in-find-image branch from aad61df to 6d872f4 Compare January 24, 2026 00:49

chaospuppy marked this pull request as ready for review January 24, 2026 00:50

chaospuppy requested review from a team as code owners January 24, 2026 00:50

chaospuppy force-pushed the 4509-include-archives-in-find-image branch 2 times, most recently from b03b517 to f35514c Compare January 27, 2026 20:43

chaospuppy changed the title ~~4509 include archives in find image~~ feat(find-images): 4509 include archives in find image Jan 27, 2026

AustinAbro321 requested changes Jan 27, 2026

View reviewed changes

github-project-automation Bot moved this to In progress in Zarf Jan 27, 2026

chaospuppy force-pushed the 4509-include-archives-in-find-image branch from 08c339e to 9534ea6 Compare January 31, 2026 01:53

chaospuppy and others added 13 commits February 9, 2026 14:22

update help text, create PackageDefinitionWithoutValidation, add plac…

2720683

…eholder images list, add skipimagearchivesimages definition option Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

remove PackageDefinitionWithoutValidation, unexport validate(), add D…

703577b

…efinitionOption for skipping imagearchives images schema findings Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

add findimagesinarchive, unit tests, and add found images to FindImag…

2573306

…es output Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

update find_images unit tests, add test for image archive

6f07f16

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

fix linting issues

b1fdd01

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

update docs

f8b3257

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

update unit test

779c821

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

chore(deps): bump github/codeql-action from 4.31.10 to 4.31.11 (zarf-…

339febc

…dev#4554) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

docs: fix grammatical error in OnlyHasImageLayers doc comment (zarf-d…

495dc86

…ev#4552) Signed-off-by: Jason Washburn <jason.washburn@gmail.com> Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

chore: update dos games package 1.3.0 (zarf-dev#4555)

5b9cba5

Signed-off-by: Austin Abro <austinabro321@gmail.com> Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

docs: clarify data injections deprecation timeline (zarf-dev#4556)

e06baea

Signed-off-by: Austin Abro <austinabro321@gmail.com> Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

chore: update dos-games references to 1.3 (zarf-dev#4557)

30ceccb

Signed-off-by: Austin Abro <austinabro321@gmail.com> Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

chaospuppy and others added 2 commits March 3, 2026 21:28

comment cleanup

2af75dd

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

Merge branch 'main' into 4509-include-archives-in-find-image

5d98232

AustinAbro321 requested changes Mar 4, 2026

View reviewed changes

fix duplicate component printing

6550e20

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

tranform to fully qualified image references

cbecbfc

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

chaospuppy added 5 commits March 10, 2026 10:14

fix unit tests

9813f42

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

Merge remote-tracking branch 'origin' into 4509-include-archives-in-f…

5a432dd

…ind-image

more unit test fixes - remove need for fulylqualifiedmatchedimages

6b17827

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

fix golagnci-lint findings

1b120f3

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

Merge remote-tracking branch 'origin/main' into 4509-include-archives…

a1f8cd0

…-in-find-image

AustinAbro321 requested changes Mar 13, 2026

View reviewed changes

chaospuppy added 4 commits March 13, 2026 14:32

implement suggestions, clarify updateNeeded

7f4a3eb

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

Merge remote-tracking branch 'origin' into 4509-include-archives-in-f…

fdee702

…ind-image

Merge remote-tracking branch 'origin' into 4509-include-archives-in-f…

bab2700

…ind-image

Merge remote-tracking branch 'origin' into 4509-include-archives-in-f…

0bc6b45

…ind-image

chaospuppy requested a review from AustinAbro321 March 24, 2026 22:01

Merge remote-tracking branch 'origin' into 4509-include-archives-in-f…

3327eae

…ind-image Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

chaospuppy force-pushed the 4509-include-archives-in-find-image branch from 987f2e9 to 3327eae Compare April 7, 2026 17:06

chaospuppy added 2 commits April 8, 2026 16:21

fix unit tests for findimages

bd9f31e

Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

Merge remote-tracking branch 'origin' into 4509-include-archives-in-f…

808d40c

…ind-image

AustinAbro321 requested changes Apr 9, 2026

View reviewed changes

Comment thread src/pkg/packager/find_images.go Outdated

chaospuppy added 3 commits April 17, 2026 14:28

Merge remote-tracking branch 'origin' into 4509-include-archives-in-f…

8f76957

…ind-image

Merge remote-tracking branch 'origin' into 4509-include-archives-in-f…

5a10b27

…ind-image

create FindDefinitionImages, make filterimages private, update update…

439a463

… generation and dev Signed-off-by: Tim Seagren <timseagren@defenseunicorns.com>

chaospuppy force-pushed the 4509-include-archives-in-find-image branch from c67b9be to 439a463 Compare April 25, 2026 00:12

chaospuppy commented Apr 25, 2026

View reviewed changes

AustinAbro321 requested changes Apr 27, 2026

View reviewed changes

		componentDefinition += fmt.Sprintf(" # Archive images - %s\n", finding.ComponentName)
		componentDefinition += " imageArchives:\n"

	archivePath := path.Join(pkgPath.BaseDir, archive.Path)
	archivePath := filepath.Join(pkgPath.BaseDir, archive.Path)

Conversation

chaospuppy commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Checklist before merging

Uh oh!

netlify Bot commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for zarf-docs ready!

Uh oh!

chaospuppy commented Jan 23, 2026

Uh oh!

codecov Bot commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

AustinAbro321 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chaospuppy Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AustinAbro321 commented Feb 2, 2026

Uh oh!

chaospuppy commented Feb 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AustinAbro321 commented Feb 9, 2026

Uh oh!

AustinAbro321 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chaospuppy commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AustinAbro321 commented Mar 4, 2026

Uh oh!

chaospuppy commented Mar 5, 2026

Uh oh!

AustinAbro321 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AustinAbro321 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chaospuppy commented Jan 23, 2026 •

edited

Loading

netlify Bot commented Jan 23, 2026 •

edited

Loading

codecov Bot commented Jan 27, 2026 •

edited

Loading

chaospuppy Jan 27, 2026 •

edited

Loading

chaospuppy commented Feb 6, 2026 •

edited

Loading

chaospuppy commented Mar 4, 2026 •

edited

Loading