-
Notifications
You must be signed in to change notification settings - Fork 194
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
feat: add logic to add image to possible list when auth is needed #4011
Conversation
…auth, but is not available Signed-off-by: Allen Conlon <[email protected]>
✅ Deploy Preview for zarf-docs ready!
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 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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
Signed-off-by: Allen Conlon <[email protected]>
Codecov ReportAttention: Patch coverage is
... and 3 files with indirect coverage changes 🚀 New features to boost your workflow:
|
…igured Signed-off-by: Allen Conlon <[email protected]>
There was a problem hiding this 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.
src/pkg/packager/find_images.go
Outdated
@@ -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) == "" { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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 { |
There was a problem hiding this comment.
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
Signed-off-by: Allen Conlon <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
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