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.

Sign up for GitHub

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

Jump to bottom

feat: blob verify command #1137

Draft
wants to merge 38 commits into
base: main
Choose a base branch
from
Draft

feat: blob verify command #1137

wants to merge 38 commits into from

Conversation

Two-Hearts
Copy link
Contributor

@Two-Hearts Two-Hearts commented Jan 7, 2025 •
edited
Loading

This PR adds blob verify command with E2E tests.

@Two-Hearts
blob signing
1619d18 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
blob signing
5870423 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
update
c6da7dc 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
Merge branch 'notaryproject:main' into blobsign
c13744e
@Two-Hearts
add tests
97c3fcd 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
fix test
f0db92f 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
fix test
9cf65b8 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
fix e2e test
e8fe8c6 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
add e2e tests
cb997c7 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
add e2e tests
07d0ffb 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
fix e2e tests
cf40ba2 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
fix e2e test
1051ad4 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
add more e2e tests
cf5fecb 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
add more e2e tests
fc4dcfc 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
fix e2e test
576a286 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
add more tests
0c984a0 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
Merge branch 'notaryproject:main' into blobsign
8474d58
@Two-Hearts
update
14a1f3e 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
fix e2e test
752afe6 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
resolve conflicts
c30e199 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
update
bc8f710 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
initial commit
9ce5ac9 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
update
9e84a3f 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
fix tests
2659ff7 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts Two-Hearts marked this pull request as draft January 7, 2025 05:47
@Two-Hearts Two-Hearts changed the title feat: blob verify feat: blob verify command Jan 7, 2025
@codecov Codecov
Copy link

codecov bot commented Jan 7, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 90.57592% with 18 lines in your changes missing coverage. Please review.

Project coverage is 73.78%. Comparing base (1118964) to head (1294938).

Files with missing lines Patch % Lines
internal/ioutil/print.go 83.01% 7 Missing and 2 partials ⚠️
internal/cmd/verifier.go 78.57% 4 Missing and 2 partials ⚠️
cmd/notation/blob/verify.go 97.16% 2 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1137      +/-   ##
==========================================
+ Coverage   72.52%   73.78%   +1.26%     
==========================================
  Files          50       52       +2     
  Lines        3126     3242     +116     
==========================================
+ Hits         2267     2392     +125     
+ Misses        667      658       -9     
  Partials      192      192

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Two-Hearts
update
a1d8562 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
fix test
d6eab1b 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
fix tests
94b586a 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
fix tests
ce63a53 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
update
3602591 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
added e2e tests
eeaf02c 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
fix e2e tests
8cc2aeb 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
fix e2e tests
32b009b 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
fix e2e tests
54ecf6e 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
add more e2e tests
fa50e18 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts
add more e2e tests
07c66b4 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts Two-Hearts marked this pull request as ready for review January 10, 2025 09:03
@Two-Hearts
added more e2e tests
4c07098 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts Two-Hearts closed this Jan 14, 2025
@Two-Hearts
resolve conflicts
2bbf48c 
Signed-off-by: Patrick Zheng <[email protected]>
@Two-Hearts Two-Hearts reopened this Jan 14, 2025
@Two-Hearts
update
1294938 
Signed-off-by: Patrick Zheng <[email protected]>
shizhMSFT
shizhMSFT requested changes Jan 14, 2025
View reviewed changes
internal/ioutil/print.go
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is time to do refactoring on the formatted output.

Copy link
Contributor

Choose a reason for hiding this comment

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

@Two-Hearts @JeyJeyGao Let's have a meeting and discuss the output design.

cmd/notation/blob/verify.go
// `application/jose+json` and `application/cose` are supported.
func parseSignatureMediaType(signaturePath string) (string, error) {
signatureFileName := filepath.Base(signaturePath)
format := strings.Split(signatureFileName, ".")[1]
Copy link
Contributor

Choose a reason for hiding this comment

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

This line may panic.

@Two-Hearts Two-Hearts marked this pull request as draft January 21, 2025 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shizhMSFT shizhMSFT shizhMSFT requested changes

@gokarnm gokarnm Awaiting requested review from gokarnm gokarnm is a code owner

@JeyJeyGao JeyJeyGao Awaiting requested review from JeyJeyGao JeyJeyGao is a code owner

@NiazFK NiazFK Awaiting requested review from NiazFK NiazFK is a code owner

@priteshbandi priteshbandi Awaiting requested review from priteshbandi priteshbandi is a code owner

@rgnote rgnote Awaiting requested review from rgnote rgnote is a code owner

@toddysm toddysm Awaiting requested review from toddysm toddysm is a code owner

@vaninrao10 vaninrao10 Awaiting requested review from vaninrao10 vaninrao10 is a code owner

@yizha1 yizha1 Awaiting requested review from yizha1 yizha1 is a code owner

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Two-Hearts @shizhMSFT