Skip to content
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

fix: check public key for signed packages during zarf package pull #3347

Merged
merged 4 commits into from
Dec 18, 2024

Conversation

AustinAbro321
Copy link
Contributor

@AustinAbro321 AustinAbro321 commented Dec 18, 2024

Description

zarf package pull ignores the --key when pulling signed packages

Related Issue

Fixes #3346

Checklist before merging

@AustinAbro321 AustinAbro321 requested review from a team as code owners December 18, 2024 19:14
Copy link

netlify bot commented Dec 18, 2024

Deploy Preview for zarf-docs canceled.

Name Link
🔨 Latest commit 8e3a966
🔍 Latest deploy log https://app.netlify.com/sites/zarf-docs/deploys/6763265f4df19d0008679f0c

Signed-off-by: Austin Abro <[email protected]>
Copy link

codecov bot commented Dec 18, 2024

Codecov Report

Attention: Patch coverage is 63.63636% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/internal/packager2/pull.go 70.00% 2 Missing and 1 partial ⚠️
src/cmd/package.go 0.00% 1 Missing ⚠️
Files with missing lines Coverage Δ
src/cmd/package.go 0.00% <0.00%> (ø)
src/internal/packager2/pull.go 28.28% <70.00%> (+1.82%) ⬆️

Copy link
Contributor Author

@AustinAbro321 AustinAbro321 Dec 18, 2024

Choose a reason for hiding this comment

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

refactored this test to create and publish the package to an in memory registry, this way we don't rely on external private keys or external infrastructure. Added a test to ensure pulling the package without a public key fails and verified that it does fail on main.

@AustinAbro321 AustinAbro321 changed the title fix: pull did not check public key for signed packages fix: check public key for signed packages during zarf package pull Dec 18, 2024
Signed-off-by: Austin Abro <[email protected]>
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

func (suite *PullInspectTestSuite) TearDownSuite() {
local := fmt.Sprintf("zarf-package-dos-games-%s-1.0.0.tar.zst", e2e.Arch)
e2e.CleanFiles(suite.T(), local)
suite.Reference.Registry = testutil.SetupInMemoryRegistry(testutil.TestContext(suite.T()), suite.T(), 31888)
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL testutil.SetupInMemoryRegistry

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's very useful for unit tests as well, once the create refactor is merged, we should be able to make tests for publish and pull with unit tests

@AustinAbro321 AustinAbro321 added this pull request to the merge queue Dec 18, 2024
Merged via the queue into main with commit 7ab4398 Dec 18, 2024
26 checks passed
@AustinAbro321 AustinAbro321 deleted the fix-pull-to-use-public-key branch December 18, 2024 20:38
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.

zarf package pull does not respect --key flag
2 participants