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(oci-layout): support in oras push and oras attach #750

Merged
merged 46 commits into from
Jan 18, 2023

Conversation

qweeah
Copy link
Contributor

@qweeah qweeah commented Jan 16, 2023

Related to #378

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@codecov-commenter
Copy link

codecov-commenter commented Jan 16, 2023

Codecov Report

Merging #750 (270e86f) into main (2e9b0a6) will increase coverage by 0.18%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main     #750      +/-   ##
==========================================
+ Coverage   63.96%   64.14%   +0.18%     
==========================================
  Files          19       19              
  Lines         702      700       -2     
==========================================
  Hits          449      449              
+ Misses        222      220       -2     
  Partials       31       31              
Impacted Files Coverage Δ
cmd/oras/internal/option/target.go 20.23% <0.00%> (+0.47%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah qweeah changed the title feat: support OCI image layout in oras push and oras attach feat(oci-layout): support in oras push and oras attach Jan 17, 2023
@qweeah qweeah marked this pull request as ready for review January 17, 2023 04:35
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Copy link
Member

@FeynmanZhou FeynmanZhou left a comment

Choose a reason for hiding this comment

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

The examples in CLI help look good to me!

cmd/oras/attach.go Outdated Show resolved Hide resolved
cmd/oras/attach.go Outdated Show resolved Hide resolved
cmd/oras/push.go Show resolved Hide resolved
cmd/oras/attach.go Outdated Show resolved Hide resolved
cmd/oras/attach.go Outdated Show resolved Hide resolved
Comment on lines +171 to +178
// Reassemble a reference with subject digest
if repo, ok := dst.(*remote.Repository); ok {
ref := repo.Reference
ref.Reference = subject.Digest.String()
opts.RawReference = ref.String()
} else if opts.Type == option.TargetTypeOCILayout {
opts.RawReference = fmt.Sprintf("%s@%s", opts.Path, subject.Digest)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We can just do switch opts.Type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But repo.Reference (including Host name and repository name) need *remote.Repository type assertion anyway.

cmd/oras/push.go Show resolved Hide resolved
oerrors "oras.land/oras/cmd/oras/internal/errors"
"oras.land/oras/cmd/oras/internal/option"
)

type attachOptions struct {
option.Common
option.Remote
option.Packer
option.ImageSpec
option.DistributionSpec
Copy link
Contributor

Choose a reason for hiding this comment

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

I just noticed that the DistributionSpec should be stick to the option.Remote. This should also be applied to oras cp and oras manifest push.

Copy link
Contributor Author

@qweeah qweeah Jan 18, 2023

Choose a reason for hiding this comment

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

No it should not. If we do that, DistributionSpec will be applied to all commands related to option.Remote, which is wrong considering the following cases:

  1. Since oras push is not related to referrers maintanance, the execution is always the same no matter what value is provided to --distribution-spec. So we should NEVER apply this flag to oras push. Same for other commands.
  2. If user specify --distribution-spec v1.1-referrers-tag to oras discover, should the command looking for referrers via tag scheme? We should design the expected behaviour and then add this flag to those commands. Before that, --distribution-spec should not be applied to oras discover.

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@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

@shizhMSFT shizhMSFT merged commit 7e861eb into oras-project:main Jan 18, 2023
TerryHowe pushed a commit to TerryHowe/oras that referenced this pull request Feb 2, 2023
…ect#750)

Related to oras-project#378

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
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.

5 participants