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 attest with --key #2551

Merged
merged 7 commits into from
Jan 31, 2024
Merged

Fix attest with --key #2551

merged 7 commits into from
Jan 31, 2024

Conversation

willmurphyscode
Copy link
Contributor

@willmurphyscode willmurphyscode commented Jan 26, 2024

Pull request adding a new testing pattern for CLIs, based on work over in clio: anchore/clio#37

Some feedback on this PR might inform anchore/clio#37, and this is meant to be merged after anchore/clio#37.

Fixes #2544 #2590

Testing

wagoodman and others added 6 commits January 25, 2024 11:05
Signed-off-by: Will Murphy <[email protected]>
Signed-off-by: Will Murphy <[email protected]>
Signed-off-by: Will Murphy <[email protected]>
Signed-off-by: Will Murphy <[email protected]>
@willmurphyscode willmurphyscode changed the title Fix attest with key Fix attest with --key Jan 29, 2024
Copy link
Contributor

@spiffcs spiffcs left a comment

Choose a reason for hiding this comment

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

The setup looks correct given the changes in the shared CLI library. Did it work locally with an import/replace statement? I think the go mod in this PR isn't setup to pull the module from the branch so it's erroring saying no such module exists

@willmurphyscode
Copy link
Contributor Author

@spiffcs - that's correct - a go.work file was used locally to test this. Once anchore/clio#37 is merged, I'll update this PR. It just makes sense to review them together, since it would be hard to see how the assertion and test application would be used in the clio PR.

Signed-off-by: Will Murphy <[email protected]>
@willmurphyscode willmurphyscode marked this pull request as ready for review January 31, 2024 20:40
@willmurphyscode willmurphyscode merged commit bbddac1 into main Jan 31, 2024
11 checks passed
@willmurphyscode willmurphyscode deleted the fix-attest-with-key branch January 31, 2024 21:39
@kzantow kzantow linked an issue Feb 15, 2024 that may be closed by this pull request
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
Fix passing "--key" to the attest command. Additionally, pull in an update to
the clio CLI library to permit unit testing that flags and env vars are parsed
to the correct field on command options structs. This testing strategy was
needed here because testing attestation in an end to end test requires a
prohibitive amount of setup.

---------

Signed-off-by: Alex Goodman <[email protected]>
Signed-off-by: Will Murphy <[email protected]>
Co-authored-by: Alex Goodman <[email protected]>
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.

syft attest command missing --key flag unknown flag: --key when use syft attest --key [KEY]
3 participants