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: minimal dep on containerd #312

Merged
merged 15 commits into from
Sep 20, 2021

Conversation

juliusl
Copy link

@juliusl juliusl commented Sep 14, 2021

Does not require a dependency on oras-project/containerd, moved everything it needs to build over.

It compiles and runs, but I haven't tested discover yet, I need some help with that cc: @sajayantony

Give it a shot with:

curl https://raw.githubusercontent.com/juliusl/oras/juliusl/prototype-3/scripts/get-oras.sh | REPO='juliusl/oras' TAG='v0.11.15-alpha' sh;. ./install-oras.sh

pkg/remotes/docker/discoverer.go Outdated Show resolved Hide resolved
pkg/oras/discover.go Show resolved Hide resolved
@juliusl
Copy link
Author

juliusl commented Sep 20, 2021

Sample output from oras from testing

# output from ./oras push 
Uploading 4eae2a2370b5 runc
Pushed localhost:5000/runc:v1

# output from ./oras push w/ --artifact-type --subject
Digest: sha256:1f853ca69214faeed10d055067b4439949a342a0a6f929d122526b53a59883bc
Uploading c71975642779 sbom.json
Pushed localhost:5000/runc

# output from ./oras push w/ --artifact-type --subject
Digest: sha256:2edf9ba4dff0203f1b26264bdf81948e86d4879a4de06f050052f471278b421d
Uploading 655032a47bb3 signature.json
Pushed localhost:5000/runc

# output from discover -o tree --artifact-type=sbom/example
Digest: sha256:81cde7efdb78ca7d6de1aabccf99e56b34bb33c84ce3b5249ce8e4938f37e05b
localhost:5000/runc:v1
└── sbom/example
    └── sha256:2edf9ba4dff0203f1b26264bdf81948e86d4879a4de06f050052f471278b421d

# output from oras pull @(oras discover -o json) 
Downloaded c71975642779 sbom.json
Pulled localhost:5000/runc@sha256:2edf9ba4dff0203f1b26264bdf81948e86d4879a4de06f050052f471278b421d
Digest: sha256:2edf9ba4dff0203f1b26264bdf81948e86d4879a4de06f050052f471278b421d

cmd/oras/push.go Outdated
// bake artifact
var pushOpts []oras.PushOpt
if opts.artifactType != "" {
refResolver := resolver
if iresolver.IsDummy(resolver) {
refResolver = newResolver(opts.username, opts.password, opts.insecure, opts.plainHTTP, opts.configs...)
refResolver, _ = newResolver(opts.username, opts.password, opts.insecure, opts.plainHTTP, opts.configs...)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we encapsulate this branching into the newResolver in resolve.go

Copy link
Author

Choose a reason for hiding this comment

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

For this particular branch I can't because it would break --dry-run, but I can remove that call to newResolver

"github.com/containerd/containerd/content"
"github.com/containerd/containerd/images"
"github.com/containerd/containerd/log"
"github.com/containerd/containerd/remotes"
orascontent "github.com/deislabs/oras/pkg/content"
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 we removed all deislabs referencs and should be under github.com/oras-project/oras/

Copy link
Author

Choose a reason for hiding this comment

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

https://github.com/oras-project/oras/blob/artifacts/go.mod -- it's still desilabs in the artifacts branch

@@ -2,6 +2,7 @@ package oras

Copy link
Contributor

Choose a reason for hiding this comment

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

Given that this is a prototype branch I don't have concerns on this change. For the oras-go would like to ensure that discover has some tests.

Copy link
Author

Choose a reason for hiding this comment

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

Actually I wrote tests already in oras-go so I can port them over here

pkg/oras/pull.go Show resolved Hide resolved
@@ -0,0 +1,266 @@
package docker
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the motivation of naming this docker? How about artifacts ?

Copy link
Author

Choose a reason for hiding this comment

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

This serves a couple of purposes:

  1. to denote that this pkg is an extension/modification of remotes/docker in containerd
  2. maintainers know which folder to look at in containerd
  3. so that people who want to take a reference on this pkg know that it also depends on containerd/docker pkgs

In summary, it's analagous to naming this windows.go or linux.go to denote os, but here I'm denoting flavor.

@sajayantony
Copy link
Contributor

Please fix DCO and let's get this merged -
git commit --amend --no-edit --signoff

sajayantony and others added 13 commits September 20, 2021 11:54
Signed-off-by: Sajay Antony <[email protected]>
Signed-off-by: Julius Liu <[email protected]>
Signed-off-by: Sajay Antony <[email protected]>
Signed-off-by: Julius Liu <[email protected]>
Signed-off-by: Julius Liu <[email protected]>
Bug: Fix host filtering
Bug: Fix references
Bug: Fix interface reference

Signed-off-by: Julius Liu <[email protected]>
Signed-off-by: Julius Liu <[email protected]>
Signed-off-by: Julius Liu <[email protected]>
Feat: Adds fetching artifact manifests
Bug: Fixes discover

Signed-off-by: Julius Liu <[email protected]>
(confirmed tested that --dry-run still works)

Signed-off-by: Julius Liu <[email protected]>
@juliusl
Copy link
Author

juliusl commented Sep 20, 2021

@sajayantony done

@juliusl
Copy link
Author

juliusl commented Sep 20, 2021

@sajayantony oops the license got deleted not sure why

@sajayantony sajayantony merged commit 09cb565 into oras-project:artifacts Sep 20, 2021
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.

3 participants