Skip to content

fix: add check during image discovery to make sure images are valid#3234

Merged
brandtkeller merged 8 commits intozarf-dev:mainfrom
a1994sc:fix-regex-for-dev-find
Feb 19, 2025
Merged

fix: add check during image discovery to make sure images are valid#3234
brandtkeller merged 8 commits intozarf-dev:mainfrom
a1994sc:fix-regex-for-dev-find

Conversation

@a1994sc
Copy link
Copy Markdown
Contributor

@a1994sc a1994sc commented Nov 12, 2024 •
edited
Loading

Description

When using zarf to discover images with the Ironbank chart kyverno-policies, the restrict-image-registries policy, (simple version included in tests), has an entry of:

  • registry.dso.mil/*

This is a false positive and cases cosign looks up to fail as that is not a valid image.

Checklist before merging

@netlify
Copy link
Copy Markdown

netlify Bot commented Nov 12, 2024 •
edited
Loading

✅ Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit f811a85
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/6733c4caa3301a0008950240

@netlify
Copy link
Copy Markdown

netlify Bot commented Nov 12, 2024 •
edited
Loading

✅ Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit 0b2f368
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/67b21f3682261b0008eac692

@a1994sc a1994sc marked this pull request as ready for review November 12, 2024 21:18
@a1994sc a1994sc requested review from a team as code owners November 12, 2024 21:18
@a1994sc
Copy link
Copy Markdown
Contributor Author

a1994sc commented Nov 20, 2024

Related to #3253

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 13, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Files with missing lines Coverage Δ
src/pkg/packager/prepare.go 53.84% <100.00%> (+0.38%) ⬆️
src/pkg/packager/regex.go 100.00% <100.00%> (ø)

brandtkeller
brandtkeller reviewed Feb 14, 2025
View reviewed changes
Comment thread src/pkg/packager/prepare.go Outdated
@a1994sc
feat(regex): added regex class
b2b355c 
Signed-off-by: Allen Conlon <software@conlon.dev>
@a1994sc a1994sc force-pushed the fix-regex-for-dev-find branch from 06ee1fd to b2b355c Compare February 16, 2025 17:16
@a1994sc
Copy link
Copy Markdown
Contributor Author

a1994sc commented Feb 18, 2025

Any reason those two test are stuck?

@brandtkeller
Copy link
Copy Markdown
Member

brandtkeller commented Feb 18, 2025

Any reason those two test are stuck?

GitHub reports "unexpected error" - so I'm going to give those a retry real quick.

@a1994sc
Copy link
Copy Markdown
Contributor Author

a1994sc commented Feb 18, 2025

Any reason those two test are stuck?

GitHub reports "unexpected error" - so I'm going to give those a retry real quick.

Ah, thank you very much!!

brandtkeller
brandtkeller approved these changes Feb 18, 2025
View reviewed changes
Copy link
Copy Markdown
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.

This definitely improves the baseline. I'll let another maintainer weigh-in if there is any patterns or constraints we want to follow with regards to the regex logic - otherwise LGTM.

@a1994sc
Copy link
Copy Markdown
Contributor Author

a1994sc commented Feb 18, 2025

Thanks!! I was a little worried that because you all are migrating to package2 it might have needed to be put on hold. Though very happy to help an awesome project!

mkcp
mkcp approved these changes Feb 19, 2025
View reviewed changes
Copy link
Copy Markdown
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. Good test case and defense for the logic. Matching the license and incl. attribution makes accepting this easy. Nice job

@a1994sc
Copy link
Copy Markdown
Contributor Author

a1994sc commented Feb 19, 2025

I would be happy to help with the effort to port over to the package2 implementation, whenever that works starts

@brandtkeller brandtkeller added this pull request to the merge queue Feb 19, 2025
Merged via the queue into zarf-dev:main with commit a8377be Feb 19, 2025
@a1994sc
Copy link
Copy Markdown
Contributor Author

a1994sc commented Feb 19, 2025

Thanks Team!!

nevinaragam pushed a commit to nevinaragam/zarf that referenced this pull request May 20, 2025
@a1994sc @nevinaragam
fix: add check during image discovery to make sure images are valid (z…
654761c 
…arf-dev#3234)

Signed-off-by: Allen Conlon <software@conlon.dev>
Signed-off-by: NevinAragam <nevin.aragam@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mkcp mkcp mkcp approved these changes

@brandtkeller brandtkeller brandtkeller approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@a1994sc @brandtkeller @mkcp