Missing manifest annotations.

[luisdavim]

I don't know if I'm doing anything wrong, but I've used oras.WithManifestAnnotations() to push my artefact with some metadata, that worked well because if I curl the repo directly I see it there:

$ curl -sL -u"user:pass" https://my-repo/v2/oso-plugins/manifests/oso-legacy | jq .
{
  "schemaVersion": 2,
  "config": {
    "mediaType": "application/vnd.unknown.config.v1+json",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "size": 2
  },
  "layers": [
    {
      "mediaType": "application/vnd.oci.image.layer.v1.tar",
      "digest": "sha256:8e985dc2c81bd5e8bfafb2858b9a9cb420c02a29d5a96f32eb00f5850556410a",
      "size": 755,
      "annotations": {
        "hook": "chmod +x ~/.oso/bin/oso-legacy",
        "org.opencontainers.image.title": "oso-legacy"
      }
    }
  ],
  "annotations": {
    "caveats": "to get auto-completions do this...",
    "description": "a bridge to the past",
    "shortDescription": "a plugin"
  }
}

But then when I pull it using desc, artefacts, err := oras.Pull(...) I can loop through the artefacts and get the annotations from each of them, but desc.Annotations is empty...

The result of fmt.Println(desc.Annotations) is:

Annotations: map[]

[luisdavim]

And now, with the recent changes from #8 I don't know how to access the metadata from the individual layers, as mentioned in a comment in the PR, the Copy function doesn't return the list of descriptors for the pulled layers like Pull used to, I guess I can work around this issue by storing my metadata in the config layer, but I was hoping to be able to use the annotations...

[deitch]

storing my metadata in the config layer,

That isn't necessarily wrong, but storing annotations on individual descriptors of any kind is valid according to the spec, so you should be able to there as well, and retrieve it.

don't know how to access the metadata from the individual layers, as mentioned in a comment in the PR,

Understood. I commented there; let's see if the proposed direction works.

desc.Annotations is empty

I find that odd. I am going to dig a little deeper. It just does from.Resolve to get the original desc.

[deitch]

@luisdavim do you have a publicly available root manifest like the above, so I can just run Copy() against it?

[luisdavim]

Thanks for the reply, I don't have anything public, but I let me see what I can do.

[luisdavim]

BTW, meanwhile I've updated my code to use the new version and Copy() and I get the same result, even when I print the annotations from then descriptor I get from "pushing" they are empty, so the same issue in either direction. Could this be because I'm using the file store?

[deitch]

I am not sure, but we can try to replicate it. Share your sample code?

[luisdavim]

I've pushed a branch with some changes to the advanced example to support annotations #24
And I've noticed that with the new Copy() method my manifest annotations are not even getting pushed as they were before with Push() but I might be doing something wrong here...

[deitch]

I think I see what is going on here.

With the old Pull(), we were generating a root manifest on the fly, so it made sense to add the annotations at that point.

With Copy(), we take a more "trust the target" approach, and copy the Descriptor "as-is". In the example you linked to above #24, by the time we Copy(), we must have a manifest available in the target. That is why there is File.GenerateManifest() and Memory.GenerateManfifest(). (side note: OCI doesn't need it, because an OCI-compliant layout must have the manifest to be entered).

So when you do Copy(... , WithManifestAnnotations), it does nothing, because it has nothing to do. Unlike Pull(), it does not try to generate a manifest during a Pull() (which can lead to unpredictable results, not to mention mangling hashes), but just takes it as is. The right place to put it is when running GenerateManifest().

Now, if it still has those annotations and still loses them, that would be a different issue.

[luisdavim]

Ok, so to add the config annotations I'd do something like

if value, ok := annotations[annotationConfig]; ok {
	configDesc.Annotations = value
}

Instead of using WithConfigAnnotations but how would I get my annotations passed to the generated descriptor from GenerateManifest?

[deitch]

Hang tight, got a PR coming to test

[luisdavim]

Thanks for working on this, #25 solves the issue of uploading the annotations, so it solves the regression, but the original issue is still there.

Upload a file:

$ ./advanced copy my-repo/oso-plugins:cake --from files --to registry cake.txt --manifest-annotations foo=bar,bar=foo                                                                                                                                                                                                                                                                         
WARN[0000] reference for unknown type: application/vnd.unknown.config.v1+json                                                                                                                                                                                                                                                                                                                                                         
v1.Descriptor{MediaType:"application/vnd.oci.image.manifest.v1+json", Digest:"sha256:415617200d07ba9e41f49d92ef842410f1467ed6717f60fc634926252c93018f", Size:433, URLs:[]string(nil), Annotations:map[string]string(nil), Platform:(*v1.Platform)(nil)} 

The annotations are there 🎉 :

$ curl -sL -u"user:pass" https://my-repo/v2/oso-plugins/manifests/cake | jq .                                                                      
Enter MFA code for arn:aws:iam::814074717971:mfa/luis.davim:        
{                                                                   
  "schemaVersion": 2,                                                                                    
  "config": {        
    "mediaType": "application/vnd.unknown.config.v1+json",                                                                                                                                                         
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",                 
    "size": 2                                                       
  },                                                                
  "layers": [                           
    {                                                                                                    
      "mediaType": "application/vnd.oci.image.layer.v1.tar",                                             
      "digest": "sha256:f5244e13720ccfb68afc0112821207fee6f4c14a65f634e1ab7be136f27ee433",                                                                                                                                                                                                                                                                                                                                            
      "size": 9,                                                                                                                                                                                                                                                                                                                                                                                                                      
      "annotations": {                                                                                   
        "org.opencontainers.image.title": "cake.txt"
      }                                                                                                  
    }                                                                                                                                                                                                                                                                                                                                                                                                                                 
  ],            
  "annotations": {                                                               
    "bar": "foo",                                                    
    "foo": "bar"   
  }                                                                                                      
}  

But if I download it, the annotations are not part of the descriptor:

./advanced copy my-repo/oso-plugins:cake --to files:cakes.txt --from registry                                                                                                                                                                                                                                                                                                               
v1.Descriptor{MediaType:"application/vnd.oci.image.manifest.v1+json", Digest:"sha256:415617200d07ba9e41f49d92ef842410f1467ed6717f60fc634926252c93018f", Size:433, URLs:[]string(nil), Annotations:map[string]string(nil), Platform:(*v1.Platform)(nil)}

[luisdavim]

Meanwhile, as a workaround, I was trying to do the following after copying into store:

_, rootManifestBytes, err := store.Ref(opts.targetRef)
if err != nil {
return err
}
var rootManifest ocispec.Manifest
if err := json.Unmarshal(rootManifestBytes, &rootManifest); err != nil {
return err
}
fmt.Println(rootManifest.Annotations)

Where store is a File store, but I get a 404...

[deitch]

OK, so half the issue is solved with that. Let's get that merged in and then solve the other.

[deitch]

@luisdavim are you sure it isn't working? I ran a simple test, from local files to oci, in order to generate and save the manifest, and then from one local oci to another local oci. The manifest itself includes the annotations both in the first and second oci layout.

The descriptor pointing to the manifest does not, but it should not either.

go run . copy my-repo/oso-plugins:cake --from files --to oci:/tmp/reg  cake.txt --manifest-annotations my.annotation=foo.bar,other=bar.val
``

When that is done, I end up with the following in `/tmp/reg`:

```console
.
├── blobs
│   └── sha256
│       ├── 288114bd0a979404a875def75db19e79adda6378971247317291a057bfb4e262
│       ├── 3c554aaf97d785cbb144167ee2007a7a14695bb73f946adc63a018e9becffe41
│       └── 44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a
├── index.json
├── ingest
└── oci-layout

Looking at index.json:

{
  "schemaVersion": 2,
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:3c554aaf97d785cbb144167ee2007a7a14695bb73f946adc63a018e9becffe41",
      "size": 453,
      "annotations": {
        "org.opencontainers.image.ref.name": "my-repo/oso-plugins:cake"
      }
    }
  ]
}

That makes sense. That is at the registry level. It is the Descriptor pointing to sha256:3c554aaf97d785cbb144167ee2007a7a14695bb73f946adc63a018e9becffe41, and has the annotation showing the name:

      "annotations": {
        "org.opencontainers.image.ref.name": "my-repo/oso-plugins:cake"
      }

If I look at that one via cat blobs/sha256/3c554aaf97d785cbb144167ee2007a7a14695bb73f946adc63a018e9becffe41 | jq:

{
  "schemaVersion": 2,
  "config": {
    "mediaType": "application/vnd.unknown.config.v1+json",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "size": 2
  },
  "layers": [
    {
      "mediaType": "application/vnd.oci.image.layer.v1.tar",
      "digest": "sha256:288114bd0a979404a875def75db19e79adda6378971247317291a057bfb4e262",
      "size": 5,
      "annotations": {
        "org.opencontainers.image.title": "cake.txt"
      }
    }
  ],
  "annotations": {
    "my.annotation": "foo.bar",
    "other": "bar.val"
  }
}

That all works. That is my actual manifest, with the annotations I added.

Now I run a copy from there to another:

go run . copy my-repo/oso-plugins:cake --from oci:/tmp/reg  --to oci:/tmp/other

And look in /tmp/other:

.
├── blobs
│   └── sha256
│       ├── 288114bd0a979404a875def75db19e79adda6378971247317291a057bfb4e262
│       ├── 3c554aaf97d785cbb144167ee2007a7a14695bb73f946adc63a018e9becffe41
│       └── 44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a
├── index.json
├── ingest
└── oci-layout

That all looks right. index.json is built correctly:

{
  "schemaVersion": 2,
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:3c554aaf97d785cbb144167ee2007a7a14695bb73f946adc63a018e9becffe41",
      "size": 453,
      "annotations": {
        "org.opencontainers.image.ref.name": "my-repo/oso-plugins:cake"
      }
    }
  ]
}

And the root manifest to which it points:

{
  "schemaVersion": 2,
  "config": {
    "mediaType": "application/vnd.unknown.config.v1+json",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "size": 2
  },
  "layers": [
    {
      "mediaType": "application/vnd.oci.image.layer.v1.tar",
      "digest": "sha256:288114bd0a979404a875def75db19e79adda6378971247317291a057bfb4e262",
      "size": 5,
      "annotations": {
        "org.opencontainers.image.title": "cake.txt"
      }
    }
  ],
  "annotations": {
    "my.annotation": "foo.bar",
    "other": "bar.val"
  }
}

Annotations all copied over fine.

So it looks like all of the copying is working fine.

Where is it not copying annotations?

[luisdavim]

Oh, yeah, that works the issue is that the descriptor I get back from the copy doesn't have the annotations, this was about that from the beginning.

[luisdavim]

So, I have ways to work around my issue, like I could copy to a temp local OCI like you did and then parse the index to move stuff into the right place, but I was trying to keep it simple and just use the file store to copy directly to the final destination and then read the metadata to do the rest of the stuff I want to do....

[luisdavim]

Here's a snippet to give you an idea of what I was going for:

func install(opts pullOptions) error {
	ctx := context.Background()
	if opts.debug {
		logrus.SetLevel(logrus.DebugLevel)
	} else if !opts.verbose {
		ctx = ctxo.WithLoggerDiscarded(ctx)
	}

	// by default we only pull files and folders, skipping the config manifest
	if opts.allowAllMediaTypes {
		opts.allowedMediaTypes = nil
	} else if len(opts.allowedMediaTypes) == 0 {
		opts.allowedMediaTypes = []string{content.DefaultBlobMediaType, content.DefaultBlobDirMediaType}
	}

	registry, err := content.NewRegistry(opts.RegistryOptions)
	if err != nil {
		return err
	}
	// the file store saves the pulled layers as regular files
	// there are other store types, like a in memory store that might be useful for listing plugins from the repository
	store := content.NewFile(opts.output)
	defer store.Close()
	store.DisableOverwrite = opts.keepOldFiles
	store.AllowPathTraversalOnWrite = opts.pathTraversal

	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
		})),
	}

	// if a name is provided for the config manifest pull it as a file with that name
	// if opts.manifestConfigRef is empty, it should take the name from the annotation
	// if the annotation is also missing or empty the config will not be pulled
	// so to pull the config, we need to either specify the name here when pulling
	// or set the ocispec.AnnotationTitle annotation when pushing
	if opts.manifestConfigRef != "" {
		copyOpts = appendCopyManifestConfigHandlers(copyOpts, opts.manifestConfigRef)
	}

	// pull the layers referenced by opts.targetRef into the store
	desc, err := oras.Copy(ctx, registry, opts.targetRef, store, "", copyOpts...)
	if err != nil {
		if errors.Is(err, reference.ErrObjectRequired) {
			return fmt.Errorf("image reference format is invalid. Please specify <name:tag|name@digest>")
		}
		return err
	}

	// update the index of installed plugins
	if err := updateIndex(opts.targetRef, desc); err != nil {
		return err
	}

	// run manifest hook --- desc.Annotations is always empty
	if desc.Annotations["hook"] != "" {
		hook := command.Parse(desc.Annotations["hook"])
		logrus.Debugf("running plugin hook: %s", hook)
		if res := hook.Exec(); res != 0 {
			return fmt.Errorf("failed to run hook")
		}
	}
	// run layer hooks --- Tese work just fine
	for _, a := range artifacts {
		name := a.Annotations[ocispec.AnnotationTitle]
		if a.Annotations["hook"] != "" {
			hook := command.Parse(a.Annotations["hook"])
			logrus.Debugf("running plugin hook: %s", hook)
			if res := hook.Exec(); res != 0 {
				return fmt.Errorf("failed to run hook for %s", name)
			}
		}
	}

	// TODO: why doesn't this alternative work?
	// using the manifest to run hooks
	// _, rootManifestBytes, err := store.Ref(opts.targetRef)
	// if err != nil {
	// 	return err
	// }
	// var rootManifest ocispec.Manifest
	// if err := json.Unmarshal(rootManifestBytes, &rootManifest); err != nil {
	// 	return err
	// }
	// fmt.Println(rootManifest.Annotations)
	// // run manifest hook
	// if rootManifest.Annotations["hook"] != "" {
	// 	hook := command.Parse(desc.Annotations["hook"])
	// 	logrus.Debugf("running plugin hook: %s", hook)
	// 	if res := hook.Exec(); res != 0 {
	// 		return fmt.Errorf("failed to run hook")
	// 	}
	// }
	// // run layer hooks
	// for _, l := range rootManifest.Layers {
	// 	name := l.Annotations[ocispec.AnnotationTitle]
	// 	if l.Annotations["hook"] != "" {
	// 		hook := command.Parse(l.Annotations["hook"])
	// 		logrus.Debugf("running plugin hook: %s", hook)
	// 		if res := hook.Exec(); res != 0 {
	// 			return fmt.Errorf("failed to run hook for %s", name)
	// 		}
	// 	}
	// }

	// plugins need to be executable
	files, err := ioutil.ReadDir(opts.output)
	if err != nil {
		return err
	}

	for _, file := range files {
		if file.IsDir() {
			continue
		}
		if err := os.Chmod(filepath.Join(opts.output, file.Name()), 0o755); err != nil {
			return fmt.Errorf("failed make %s executable: %w", file.Name(), err)
		}
	}

	fmt.Println("Pulled", opts.targetRef)
	fmt.Println("Digest:", desc.Digest)
	fmt.Println("Annotations:", desc.Annotations)

	return nil
}

But desc.Annotations is always empty.

[luisdavim]

I think the problem is the registry resolver never sets the annotations: https://github.com/containerd/containerd/blob/main/remotes/docker/resolver.go#L375-L379

[deitch]

I think the problem is the registry resolver never sets the annotations:
https://github.com/containerd/containerd/blob/main/remotes/docker/resolver.go#L375-L379

That isn't it. That mixes together the resolver and the manifest:

1. Request tag, get Descriptor that points to manifest - this is what Resolve() does
2. Request manifest by hash (based on above Descriptor), get manifest - this is a blob that has the annotations

[luisdavim]

I might be reading something wrong and might have taken some wrong turn somewhere but, If I follow the code, that's where it gets me, that's the Resolve() implementation for from.Resolve() in the Copy() method when the from is a registry and the result of it is what Copy() returns as the descriptor.

[luisdavim]

So, the problem might be that Copy() should not be returning that and should instead return the descriptor of the target, what was created by the copy.

[deitch]

It was pretty close. We are passing back the descriptor of the target, rather than the content pointed to by that descriptor. It isn't too hard. Almost got it here.

[luisdavim]

Question, if I copy from registry to a File store named store shouldn't I be able to do the following to get the manifest?

desc, err := oras.Copy(ctx, registry, opts.targetRef, store, "", copyOpts...)
// ...
_, rootManifestBytes, err := store.Ref(opts.targetRef + "@" + desc.Digest.String())

But with either store.Resolve(ctx, opts.targetRef+"@"+desc.Digest.String()) or store.Ref(opts.targetRef + "@" + desc.Digest.String()) I get a 404.

[deitch]

OK, actually, I don't see any way to do this. We have two artifacts we could return:

1. The root descriptor (that which the tag resolves to)
2. The manifest (that which the descriptor points to)

The root descriptor is going to look like this:

type Descriptor struct {
	// MediaType is the media type of the object this schema refers to.
	MediaType string `json:"mediaType,omitempty"`

	// Digest is the digest of the targeted content.
	Digest digest.Digest `json:"digest"`

	// Size specifies the size in bytes of the blob.
	Size int64 `json:"size"`

	// URLs specifies a list of URLs from which this object MAY be downloaded
	URLs []string `json:"urls,omitempty"`

	// Annotations contains arbitrary metadata relating to the targeted content.
	Annotations map[string]string `json:"annotations,omitempty"`

	// Platform describes the platform which the image in the manifest runs on.
	//
	// This should only be used when referring to a manifest.
	Platform *Platform `json:"platform,omitempty"`
}

It will not have your annotations, because the annotations don't actually get appended to that. E.g. if you pull an image from a registry, it generates (or sends you) that descriptor as is.

If you look at the root manifest, it will be either an index:

type Index struct {
	specs.Versioned

	// Manifests references platform specific manifests.
	Manifests []Descriptor `json:"manifests"`

	// Annotations contains arbitrary metadata for the image index.
	Annotations map[string]string `json:"annotations,omitempty"`
}

or a manifest:

type Manifest struct {
	specs.Versioned

	// Config references a configuration object for a container, by digest.
	// The referenced configuration object is a JSON blob that the runtime uses to set up the container.
	Config Descriptor `json:"config"`

	// Layers is an indexed list of layers referenced by the manifest.
	Layers []Descriptor `json:"layers"`

	// Annotations contains arbitrary metadata for the image manifest.
	Annotations map[string]string `json:"annotations,omitempty"`
}

Or something else entirely, as the list of potential root artifacts expands.

Those will have whatever annotations you set on them, as we saw with the "foo=bar" example, but that is not a Descriptor, but a different artifact (that which is pointed to by the Descriptor).

Here is the plan:

1. For root Descriptor, you already get it from Copy.
2. I will modify the PR add options to save layers and root manifest; remove unused opts #27 to return the json of the root manifest; you can parse it to your heart's content (it is beyond oras's scope to worry about what format it is and then unmarshal it).

Beyond that, I don't know what you can do. The distribution spec does not provide for a way to control the root Descriptor AFAIK. You send a reference (tag or hash) it gives you a Descriptor. That is not content you uploaded, but content generated by the distribution implementation (aka registry)./

[luisdavim]

Yeah, for now I'm making an extra http call to the registry to get the manifest and unmartial it so if I could get it from the copy operation that would be great :)
But why can't I resolve the manifest ref from the store as in my question above?

[deitch]

Question, if I copy from registry to a File store named store shouldn't I be able to do the following to get the manifest?

Not quite. I think you are misconstruing how File works.

File is set up to read files (which will be layers, and potentially config as well) from disk. It then builds the additional artifacts needed (optionally config, manifest) to complete a valid spec to send to a target (oci, registry, etc.) in memory. It then sends those.

So when you use a File as a to, it isn't really keeping those in memory. The File as a target for pushing just discards anything without a proper name annotation.

In theory, it would be possible to have it keep those artifacts that are just manifests/indexes and do not have a name in memory, so as long as the struct is around, you would be ok. But that is a life and a separate issue.

[luisdavim]

That might be a cleaner solution than just returning the raw JSON don't you think?

[luisdavim]

This is not really solved, can it be reopened?
I mean, I can get the annotations my unmarsheling the raw manifest I get from oras.WithRootManifest but this feels more like a workaround.

[deitch]

@luisdavim let's try to bring this back to the beginning.

Where are you adding annotations, and where are they not showing up? If we look at a sample image (I am using the current docker.io/library/alpine:3.14 as a sample tree):

docker.io/library/alpine:3.14 -> v1.Descriptor {
        MediaType: "application/vnd.docker.distribution.manifest.list.v2+json",
	Size: 1638,
	Digest: "sha256:e1c082e3d3c45cccac829840a25941e679c25d438cc8412c2fa221cf1a824e6a",
	Annotations: map[string]string nil,
   }

sha256:e1c082e3d3c45cccac829840a25941e679c25d438cc8412c2fa221cf1a824e6a -> 
{
	"manifests": [
		{
			"digest": "sha256:69704ef328d05a9f806b6b8502915e6a0a4faa4d72018dc42343f511490daf8a",
			"mediaType": "application\/vnd.docker.distribution.manifest.v2+json",
			"platform": {
				"architecture": "amd64",
				"os": "linux"
			},
			"size": 528
		},
		{
			"digest": "sha256:b06a5cf61b2956088722c4f1b9a6f71dfe95f0b1fe285d44195452b8a1627de7",
			"mediaType": "application\/vnd.docker.distribution.manifest.v2+json",
			"platform": {
				"architecture": "arm64",
				"os": "linux",
				"variant": "v8"
			},
			"size": 528
		},
                ....
                ....
	],
	"mediaType": "application\/vnd.docker.distribution.manifest.list.v2+json",
	"schemaVersion": 2
}

The first descriptor (resolved from the tag) is completely out of your control, as provided by the registry (although in theory a registry can open it up, I have not seen it).

The second one is completely in control of whoever pushes it.

So which case are you talking about? If it is the first, we can pull it as is, but cannot control what goes in it. If it is the second, we can (and do) add annotations, and when we copy, we get the annotations.

I am not sure which case is the problem.

[shizhMSFT]

@luisdavim I'm closing this issue as this issue is in an inactive state for a long time. Meanwhile, oras-go has evolved into v2 and maybe you can try again with v2 APIs.

Additionally, I think there is a concept misunderstanding. The annotations field of the manifest descriptor is not the same as the annotations field of the manifest itself. The annotations field of the manifest will not and should not be populated to the annotations field of the manifest descriptor by default. Therefore, fetching the raw JSON of the manifest and extracting the annotations field is not a workaround but the right way to do so.