add support for manifest and config annotations at generation time

[deitch]

Also updates the advanced example to support annotations. This is a potential fix for #22 , and also supersedes #24

cc @luisdavim

[luisdavim]

Interesting, I think the content.File.StoreManifest() and content.GenerateManifest might be useful, but why not just allow passing the annotations to content.File.GenerateManifest directly?

[deitch]

FYI this still has lots of testing issues; I wanted to get the style across so people can have a look at it.

[deitch]

why not just allow passing the annotations to content.File.GenerateManifest directly?

It got messy. Now you need to pass 2 kinds of annotations - config and manifest - and possibly get back five values - config, configDesc, manifest, manifestDesc, error. But what if someone passed in a config, so we do not need those?

The UX ended up being messy. It was much cleaner to just do it in 2 steps: generate your config, generate your manifest, pass it.

[deitch]

Also, the arguments got really messy too. variadic desc, and multiple annotations maps, and the ref. Ugh.

[deitch]

OK, I think tests should pass finally now

[deitch]

Hmm, something is failing, not sure what. One of the other maintainers want to help?

[luisdavim]

I ran the failing test manually and this is what I get from the repo:

$ curl -sLi http://localhost:5000/v2/_catalog
HTTP/1.1 200 OK
Content-Type: application/json; charset=utf-8
Docker-Distribution-Api-Version: registry/2.0
X-Content-Type-Options: nosniff
Date: Fri, 10 Sep 2021 15:22:33 GMT
Content-Length: 26

{"repositories":["oras"]}

$ curl -sLi http://localhost:5000/v2/repositories/oras/tags/list
HTTP/1.1 404 Not Found
Content-Type: application/json; charset=utf-8
Docker-Distribution-Api-Version: registry/2.0
X-Content-Type-Options: nosniff
Date: Fri, 10 Sep 2021 15:23:26 GMT
Content-Length: 125

{"errors":[{"code":"NAME_UNKNOWN","message":"repository name not known to registry","detail":{"name":"repositories/oras"}}]}

So it seems there's some silent error when copying from the memory store to the registry.

[luisdavim]

@deitch I found the issue, the order of the returned vars for GenerateManifestAndConfig in the simple example is wrong.

[luisdavim]

BTW, running the simple example from your branch with my proposed change outputs:

$ go run .
Pushing hello.txt to localhost:5000/oras:test...
WARN[0000] reference for unknown type: application/vnd.unknown.config.v1+json
Pushed to localhost:5000/oras:test with digest sha256:bc4b23b58840b73e24d7be3ff6baa477a830999023af4c75c83ec4db6c86fc9a
Pulling from localhost:5000/oras:test and saving to hello.txt...
WARN[0000] unknown type: application/vnd.unknown.config.v1+json
Pulled from localhost:5000/oras:test with digest sha256:bc4b23b58840b73e24d7be3ff6baa477a830999023af4c75c83ec4db6c86fc9a
Try running 'cat hello.txt'

But the hello.txt file is not actually created:

$ ll
.rw-r--r-- 2.2k luisdavim 10 Sep 16:35 simple_push_pull.go

The same thing works fine from main, the hello.txt gets materialised in the FS.

[luisdavim]

I would also suggest removing the CopyOpt.WithConfigAnnotations and CopyOpt.WithManifestAnnotations since they don't seem to work and would confuse the users.

[deitch]

Yeah I will have those removed. But not in this PR. We are going to solve both of your issues - "give me the list of layers" and "give me the root manifest" - with some CopyOpt options; we will remove those extraneous ones at the same time.

[luisdavim]

Yeah I will have those removed. But not in this PR. We are going to solve both of your issues - "give me the list of layers" and "give me the root manifest" - with some CopyOpt options; we will remove those extraneous ones at the same time.

Awesome, thanks.

I got around getting the layers with something like:

var artifacts []ocispec.Descriptor
copyOpts := []oras.CopyOpt{
	oras.WithAllowedMediaTypes(opts.allowedMediaTypes),
	oras.WithPullStatusTrack(os.Stdout),
	oras.WithPullCallbackHandler(images.HandlerFunc(func(ctx context.Context, desc ocispec.Descriptor) (children []ocispec.Descriptor, err error) {
		artifacts = append(artifacts, desc)
		return
	})),
}

But I haven't yet figured out how to get the root manifest annotations without calling the registry API directly on the side, anyway, having this available in the lib would be great. Thanks.

[deitch]

But the hello.txt file is not actually created

Yup, that was as simple mistake (10 characters to fix it), but not quite as easy to track down. Isn't that always the case?

CI is clean. Maintainers, can we get a review and merge to fix this regression?

[luisdavim]

LGTM

[luisdavim]

@sajayantony sorry for the direct ping but, can we get this merged? It fixes some regressions introduced to the main branch when #8 was merged...

[sajayantony]

@deitch @jdolitsky @shizhMSFT If one of you can approve this PR or review that would be really helpful.