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

feat: support OCI image manifest #509

Merged
merged 42 commits into from Feb 9, 2023

Conversation

patrickzheng200
Copy link
Contributor

@patrickzheng200 patrickzheng200 commented Jan 16, 2023

This PR adds OCI image manifest support to notation CLI according to this spec.

If using OCI artifact manifest to store signatures, notation requires the registry to support Referrers API as well.

Tested with [COSE, JWS] + [OCI artifact manifest, OCI image manifest] on ACR.

This PR would also resolve issue.

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@patrickzheng200 patrickzheng200 self-assigned this Jan 16, 2023
@patrickzheng200 patrickzheng200 marked this pull request as draft January 16, 2023 03:43
@patrickzheng200 patrickzheng200 added the enhancement New feature or request label Jan 16, 2023
@patrickzheng200 patrickzheng200 added this to the RC-2 milestone Jan 16, 2023
@patrickzheng200 patrickzheng200 linked an issue Jan 16, 2023 that may be closed by this pull request
@patrickzheng200 patrickzheng200 requested review from shizhMSFT and JeyJeyGao and removed request for shizhMSFT January 16, 2023 03:47
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@codecov-commenter
Copy link

codecov-commenter commented Jan 16, 2023

Codecov Report

Merging #509 (65f2fcb) into main (8d52989) will increase coverage by 0.30%.
The diff coverage is 37.77%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #509      +/-   ##
==========================================
+ Coverage   36.50%   36.81%   +0.30%     
==========================================
  Files          29       30       +1     
  Lines        1504     1581      +77     
==========================================
+ Hits          549      582      +33     
- Misses        936      978      +42     
- Partials       19       21       +2     
Impacted Files Coverage Δ
cmd/notation/sign.go 42.00% <25.00%> (-1.68%) ⬇️
cmd/notation/registry.go 16.19% <34.84%> (+16.19%) ⬆️
cmd/notation/verify.go 27.27% <50.00%> (ø)
internal/slices/slices.go 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@patrickzheng200 patrickzheng200 marked this pull request as ready for review January 16, 2023 09:29
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
cmd/notation/registry.go Outdated Show resolved Hide resolved
cmd/notation/sign.go Outdated Show resolved Hide resolved
cmd/notation/sign.go Outdated Show resolved Hide resolved
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

cmd/notation/registry.go Outdated Show resolved Hide resolved
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Copy link
Contributor

@priteshbandi priteshbandi left a comment

Choose a reason for hiding this comment

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

Code LGTM with pending flag name discussion. For merge, I would suggest wait for that discussion to conclude and then based on it update the flag name.

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@patrickzheng200
Copy link
Contributor Author

patrickzheng200 commented Feb 7, 2023

The current build would fail due to dependency change in notation-go: notaryproject/notation-go#271

Please review notaryproject/notation-go#271 first.

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@patrickzheng200
Copy link
Contributor Author

Dependency updated. PR ready to be reviewed/merged.

cmd/notation/registry.go Outdated Show resolved Hide resolved
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
cmd/notation/sign.go Outdated Show resolved Hide resolved
cmd/notation/registry.go Outdated Show resolved Hide resolved
cmd/notation/registry.go Outdated Show resolved Hide resolved
cmd/notation/sign.go Outdated Show resolved Hide resolved
cmd/notation/sign.go Outdated Show resolved Hide resolved
cmd/notation/sign.go Outdated Show resolved Hide resolved
cmd/notation/registry.go Outdated Show resolved Hide resolved
Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@priteshbandi priteshbandi left a comment

Choose a reason for hiding this comment

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

LGTM, you will need to resolve conflict before change can be merged

Signed-off-by: Patrick Zheng <patrickzheng@microsoft.com>
@patrickzheng200 patrickzheng200 merged commit 4f573af into notaryproject:main Feb 9, 2023
@patrickzheng200 patrickzheng200 deleted the image branch February 9, 2023 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RC2] Fallback updates for Sign operation CLI switch to store signatures using OCI image manifest.