Skip to content
This repository has been archived by the owner on May 15, 2024. It is now read-only.

Sample annotation #28

Closed
wants to merge 3 commits into from

Conversation

SteveLasker
Copy link
Contributor

Add an example of a set of annotations being added, with no additional content [blob]s

@dlorenc
Copy link

dlorenc commented Aug 25, 2021

Are these annotations meant to apply to the artifact referenced by the subjectManifest, or to the reference type object itself?

sajayantony
sajayantony previously approved these changes Aug 25, 2021
Copy link
Contributor

@sajayantony sajayantony left a comment

Choose a reason for hiding this comment

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

This clearly shows that the manifest can have annotations. We need to consider if these can be indexed by distribution in the future.

Comment on lines 10 to 13
"annotations": {
"io.acme-rockets.importDate": "2021-08-25T09:22:00Z",
"io.acme-rockets.expiration": "2021-11-21"
}
Copy link

@jonjohnsonjr jonjohnsonjr Aug 25, 2021

Choose a reason for hiding this comment

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

This seems like a particularly useless example, if the annotations are at the top level.

The description of this field appears to have been blindly copied and pasted from the image-spec, so its current definition is nonsensical, and I worry that this example will propagate misunderstanding.

Annotations are about the object they are embedded within. I interpret this example to mean this artifact manifest was imported on 2021-08-25 and expires on 2021-11-21. The artifact manifest has no content other than a pointer to the subject, so the import date and expiration seem completely useless. This is just a pointer that expires.

If you mean for these annotations to apply to the subject of this artifact manifest, I would recommend the annotations be placed on the subjectManifest descriptor to make that clear.

Copy link
Contributor

@sajayantony sajayantony Aug 26, 2021

Choose a reason for hiding this comment

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

Shouldn't we support annotations for both the subject and the manifest itself. Client tooling that creates the manifest should be able to define where the annotations are?

Copy link

@jonjohnsonjr jonjohnsonjr Aug 26, 2021

Choose a reason for hiding this comment

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

You can certainly put annotations wherever you want, and it makes sense to have annotations for both the artifact manifest and the subject, but especially if you want to use both, it's much better that your use of annotations aligns with the rest of the universe, IMO. This violates normal expectations, and people tend to just copy and paste examples without thinking about it.

I tried for months to explain how media types work because they have semantic meaning, but folks ended up just copying and pasting stuff from oras and artifacts without thinking about it. I've seen actual production artifacts in the wild with acme-rocket nonsense as their media types :(

I implore you to understand the semantics of the primitives you're using in this spec before misusing them in examples. It is hard to undo that.

If this example is literally meant to just be an expiring pointer, that's fine, but it should come with an explanation to that effect. As is, it's misleading.

Copy link
Contributor

Choose a reason for hiding this comment

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

The annotations here can also define the behavior that the reference manifest itself like details of the blobs or any other properties that client tooling may use to creating or purging as needed . I don't see the use case of hoisting annotations from the subject (image or chart).

Choose a reason for hiding this comment

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

The annotations here can also define the behavior that the reference manifest itself like details of the blobs or any other properties that client tooling may use to creating or purging as needed .

Yes, which is the point I'm making. Given that this example has no blobs or properties other than the subject descriptor, I can't imagine it being useful for anything.

I don't see the use case of hoisting annotations from the subject (image or chart).

I didn't suggest this, so I'm confused.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @toddysm,
We don't want to add any limits to the types that can be added as it would imply an over-constraint, and the registry shouldn't have to know about any specific types. The sample annotation prompts these types of questions about whether we should add some additional meta-data to the /referrers API results.
I'm also not convinced posting a document is the best way to support the meta-data services.
What were the scenarios you were thinking about?

Copy link

Choose a reason for hiding this comment

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

@dlorenc and @SteveLasker, it is not about restricting the types, it is about how many artifacts from a specific type one can have. To bring @sajayantony scenario above - if you have multiple annotations with the same name in different references, which one wins?

Also, question about the artifact names - is the filename of importance, i.e. does the name determine the meaning of the reference? For example, can one have net-monitor-annotations.json and net-monitor-annotations-v2.json both referencing the subject and defining the same annotations with different values? How do the tools reconcile the meaning of both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The overload of properties, associated with an artifact through the subject is a great discussion and something we do need to think about. It's not really unique to references be adding.
Let's say you have an automated build system that generates annotations based on information in the build.
There's nothing that says it wouldn't generate the same annotation multiple times in a single manifest.
You could argue the last, largest, or smaller value might win. That's likely a client configuration, specific to the artifactType. This is why I'm thinking we may need/want more metadata about what comes in/out of a registry: See the "data returned" section of the Show/Get-Info API Requirements #232 PR that was attempting at providing some stability to responses.

For the file name; should I assume you're asking about the files within the blobs being uploaded?
In that case, what we're trying to do is have the registry be as dumb a storage bucket as it can be. Smarter than blob storage, capable of differentiating artifact types and references, but not smart enough to process the contents of the artifact. So, I don't think the contents (files) of a blob are interesting to the registry service.

Copy link

Choose a reason for hiding this comment

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

OK, got it! Tools will be responsible to interpret the data inside the blobs. One last thing - you mention artifactType - is there a list of possible/official artifact types that we can look at?

Copy link
Contributor Author

@SteveLasker SteveLasker Aug 30, 2021

Choose a reason for hiding this comment

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

...is there a list of possible/official artifact types that we can look at?

The approach is sorta like asking for an official list of file extensions. There are Office extensions (Word, PowerPoint, Excel, ...) and of course many others from Adobe, AutoCad and many others.

Was there something specific you were looking for?

{
"mediaType": "application/vnd.oras.artifact.manifest.v1+json",
"artifactType": "example.annotations.v0",
"blobs": [],
Copy link
Contributor

Choose a reason for hiding this comment

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

If you want to show that blobs are null then why do you need this property?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wouldn't expect libraries to generate "blobs": [],, I included it in the example to make it obvious the artifact.manifest supports blobs as optional, with an example of annotations.

@sajayantony sajayantony dismissed their stale review August 26, 2021 00:51

Will reconsider given the input and discussions.

@michaelb990
Copy link
Contributor

What are we trying to show here? I'd prefer keeping the examples to the clearly identified use cases we've talked about -- signatures, SBoMs, and maybe scan results. Adding all the possible examples of things that could be done with this muddies the waters and makes it impossible to constrain the scope of the changes we want to make. Adding only a new JSON document with no context only adds to that confusion.

My understanding of our goal of this project is to deliver the specific set of features we need to prove out references in a registry and allow artifacts like signatures & SBoMs to be stored alongside the images they are related to. Let's focus on diving deep on those pieces and leave the forward-looking parts of this to larger OCI discussions and community meetings.

…xample

Signed-off-by: Steve Lasker <stevenlasker@hotmail.com>
@SteveLasker
Copy link
Contributor Author

I've made the examples more generic.
The purpose of this example is it's a valid use, and we should think through the implications. For instance, how do we differentiate the same annotation being added to a single manifest? Should we start to think about additional metadata for when an object was added to a registry, so the list of /referrers/ can make some additional statement about the different references?

As a comparison, there's nothing stopping someone from defining the same annotation in the original manifest.

"annotations": {
    "io.acme-rockets.foo": "values",
    "io.acme-rockets.bar": "more-values"
  },
"annotations": {
    "io.acme-rockets.foo": "blahh",
    "io.acme-rockets.bar": "42"
  }

A human likely wouldn't do it, but tooling could apply the same annotation multiple times.

I've also tagged this "work in progress" to think about, but not act upon right now.

@jonjohnsonjr
Copy link

I think most of the confusion stems from the example being presented without any context. Based on your comments, it seems like you expect this example to mean something, but I don't know what it is.

@SteveLasker
Copy link
Contributor Author

I think most of the confusion stems from the example being presented without any context

You seem to be arguing both points. I initially posted a specific example for tracking when an artifact was imported to the acme-rockets environment, and when it should be deleted the default expiration date. as specific examples.
I changed it to be generic, now you're asking for specific use cases.

I prefer the specific use-case as it prompts questions for how do we resolve duplicate entries as we're not returning when the artifact was created, or pushed to the registry.

@jonjohnsonjr
Copy link

You seem to be arguing both points.

You misunderstood my point, I think.

The top-level annotations map, as currently defined:

This OPTIONAL property contains arbitrary metadata for the image manifest

This is metadata for the manifest. In the original example, the expiration date was at the top level, which implies that it applies to the artifact manifest.

If you want metadata about the subject, I would expect the annotation to be present on the subject descriptor's annotations.

I initially posted a specific example for tracking when an artifact was imported to the acme-rockets environment, and when it should be deleted the default expiration date. as specific examples.

You didn't, though. You just posted a JSON blob with no description.

@michaelb990
Copy link
Contributor

What are we trying to show here?

I don't think my question has been answered yet. Sure you could do a lot of things, but unless we have a use case we're trying to show, this example isn't adding value. The spec itself should clarify the requirements for what should / should not be checked when an artifact manifest is parsed. These examples are supposed to show how it might be used. I'd encourage you to close the PR unless we can articulate the use case for this.

@SteveLasker SteveLasker marked this pull request as draft September 3, 2021 21:28
@SteveLasker
Copy link
Contributor Author

closing until we have more time to document the scenarios and expectations.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants