Skip to content

feat: add logic to add image to possible list when auth is needed #4011

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

a1994sc
Copy link
Contributor

@a1994sc a1994sc commented Jul 21, 2025

Description

Updates logic that when trying to use zarf dev find-images a package, it will silently fail if the registry, in this case registry1.dso.mil, needs auth to check it.

Related Issue

Fixes #4007

Checklist before merging

…auth, but is not available

Signed-off-by: Allen Conlon <[email protected]>
@a1994sc a1994sc marked this pull request as ready for review July 21, 2025 20:55
@a1994sc a1994sc requested review from a team as code owners July 21, 2025 20:55
Copy link

netlify bot commented Jul 21, 2025

Deploy Preview for zarf-docs ready!

Name Link
🔨 Latest commit b2c71f7
🔍 Latest deploy log https://app.netlify.com/projects/zarf-docs/deploys/68801ec004d879000825daec
😎 Deploy Preview https://deploy-preview-4011--zarf-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@@ -683,8 +683,10 @@ func (o *devFindImagesOptions) run(cmd *cobra.Command, args []string) error {

componentDefinition := "\ncomponents:\n"
for _, finding := range imagesScans {
if len(finding.Matches) > 0 {
if len(finding.Matches)+len(finding.PotentialMatches)+len(finding.CosignArtifacts) > 0 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was a slight rework because at present will render like the following:

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch - this does fix formatting

Copy link

codecov bot commented Jul 21, 2025

Codecov Report

Attention: Patch coverage is 66.66667% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/cmd/dev.go 0.00% 3 Missing ⚠️
Files with missing lines Coverage Δ
src/pkg/packager/find_images.go 56.48% <100.00%> (+2.39%) ⬆️
src/cmd/dev.go 44.55% <0.00%> (-0.15%) ⬇️

... and 3 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.

Copy link
Member

@brandtkeller brandtkeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Functions as intended with local testing - left one potential edge case for consideration otherwise looks like a great improvement.

@@ -240,6 +242,12 @@ func FindImages(ctx context.Context, packagePath string, opts FindImagesOptions)
if descriptor, err := crane.Head(image, images.WithGlobalInsecureFlag()...); err != nil {
// Test if this is a real image, if not just quiet log to debug, this is normal
l.Debug("suspected image does not appear to be valid", "error", err)
// statusCheck is find if the error has an 40x error code
// shaCheck is remove false positives of sha256:aaaaa....
if statusCheck.FindString(err.Error()) != "" && shaCheck.FindString(image) == "" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good - one additional edge case is that a dial tcp connect: connection refused error will still be silent.

For example:

./build/zarf dev find-images src/pkg/packager/testdata/find-images/fuzzy-registry-auth/ --skip-cosign -l debug

Will not return the same entries as the test case - as the logs indicate

2025-07-22 23:00:09 DBG found possible fuzzy match kind=ConfigMap value=localhost:65000/sig-storage/csi-node-driver-registrar:v2.13.0
2025-07-22 23:00:09 DBG found possible fuzzy match kind=ConfigMap value=localhost:65000/sig-storage/csi-resizer:v1.13.2
2025-07-22 23:00:09 DBG found possible fuzzy match kind=ConfigMap value=localhost:65000/sig-storage/csi-snapshotter:v8.2.1
2025-07-22 23:00:09 DBG suspected image does not appear to be valid error=Get "https://localhost:65000/v2/": dial tcp 127.0.0.1:65000: connect: connection refused; Get "http://localhost:65000/v2/": dial tcp 127.0.0.1:65000: connect: connection refused
2025-07-22 23:00:09 DBG suspected image does not appear to be valid error=Get "https://localhost:65000/v2/": dial tcp 127.0.0.1:65000: connect: connection refused; Get "http://localhost:65000/v2/": dial tcp 127.0.0.1:65000: connect: connection refused
2025-07-22 23:00:09 DBG suspected image does not appear to be valid error=Get "https://localhost:65000/v2/": dial tcp 127.0.0.1:65000: connect: connection refused; Get "http://localhost:65000/v2/": dial tcp 127.0.0.1:65000: connect: connection refused

Should this be captured as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I think that makes sense to me, especially because Zarf detects this as match without checking:

apiVersion: apps/v1
kind: Pod
metadata:
  labels:
    app: agent
spec:
  containers:
    - name: agent
      image: localhost:65000/sig-storage/csi-snapshotter:v8.2.0

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed!

@@ -683,8 +683,10 @@ func (o *devFindImagesOptions) run(cmd *cobra.Command, args []string) error {

componentDefinition := "\ncomponents:\n"
for _, finding := range imagesScans {
if len(finding.Matches) > 0 {
if len(finding.Matches)+len(finding.PotentialMatches)+len(finding.CosignArtifacts) > 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch - this does fix formatting

@a1994sc a1994sc requested a review from brandtkeller July 23, 2025 11:27
Copy link
Member

@brandtkeller brandtkeller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Contributor

@mkcp mkcp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@brandtkeller brandtkeller added this pull request to the merge queue Jul 23, 2025
Merged via the queue into zarf-dev:main with commit df5340c Jul 23, 2025
32 of 33 checks passed
@a1994sc a1994sc deleted the feature/find-images-with-auth-repo branch July 23, 2025 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dev find-images fails when not logged into registry that requires auth
3 participants