chore: update case for find images

[a1994sc]

Description

...

Related Issue

Fixes #4022

Checklist before merging

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

[netlify]

✅ Deploy Preview for zarf-docs canceled.

Name	Link
🔨 Latest commit	2d393ec
🔍 Latest deploy log	https://app.netlify.com/projects/zarf-docs/deploys/68927fcf831c5e00081647cb

[brandtkeller]

Looking it over - this is certainly a tricky one.

The loosening of constraints to support the issue that was previously implemented is worthy of keeping - but it does introduce this side-effect.

I'm not against filtering by specific resource types - but it does leave a lot of room for many other gaps that can only be resolved by changes to source code.

A loose alternative could be creating a deny/block-list in the zarf-config.yaml that could be used to filter out additional items. Open to thoughts from others.

[brandtkeller]

@zarf-dev/maintainers any thoughts for what the behavior here might look like?

[AustinAbro321]

Thinking this over, I'd prefer reverting the functionality in #4011. Something I hadn't thought about is that even when an image doesn't exist in the registry it'll give a 401 error. This makes the registry call as is not useful, as we'll pretty much always get 401 errors for non-images.

It'll be hard to maintain logic otherwise for fuzzy images, and we want the list here to be very accurate, even for maybe images. I've seen users with flows where they auto update their package using zarf dev find-images. Additionally, users will need to login to create a package with the image anyway. I think the most we'd want to do would be to warn the user that we failed to connect to auth to the registry.

[brandtkeller]

In this case - should we opt to improve documentation to support:

1. info logs for inability to authenticate during 401's (already an error/debug log)
2. Update command docs and documentation to clarify we will only provide output if images for which the registry call returns information/successful.

There is certainly a trade-off here. I don't hold strong opinions but am curious if there is a subset that is still valuable here? IE put the more loose filtering behind a flag perhaps?

[AustinAbro321]

Yeah I think both of those steps would be good. The issue I have with a flag, is that the user should be logged into their registry anyway, assuming that they want to create the package soon after finding images. We shouldn't ask them for another action on top of that, when they already very likely need to login

[a1994sc]

Valid, I do think that my own issues came from some level of annoyance when using a new devbox. I do think that there is a weird delta from how zarf assumes that .spec.containers[].image is always valid.... however given the how many different ways images are formatted and what not, it is not an easy problem to solve for

[a1994sc]

So I reverted the logic changes from pr 4011, left in the logic that fixes the improper formatting of the zarf dev find-images:

https://github.com/zarf-dev/zarf/pull/4011/files#r2220321491

As well as the SetupInMemoryRegistryWithAuth Function

[AustinAbro321]

Appreciate you making this! I do think we should get rid of the SetupInMemoryRegistryWithAuth function since we're not using it. It's also in git history if we want to add it back.

[a1994sc]

Appreciate you making this! I do think we should get rid of the SetupInMemoryRegistryWithAuth function since we're not using it. It's also in git history if we want to add it back.

Removed that function, let me know if there are any other changes

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines	Coverage Δ
src/pkg/packager/find_images.go	55.66% <ø> (-0.83%)	⬇️

... and 4 files with indirect coverage changes

🚀 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]

LGTM, thanks!

[mkcp]

LGTM