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

Clarify and correct OCI Artifact Type for SIF #4437

Closed
vsoch opened this issue Sep 13, 2019 · 12 comments · Fixed by #5658
Closed

Clarify and correct OCI Artifact Type for SIF #4437

vsoch opened this issue Sep 13, 2019 · 12 comments · Fixed by #5658
Assignees
Milestone

Comments

@vsoch
Copy link
Collaborator

vsoch commented Sep 13, 2019

Hey folks! I want to pass forward some important discussion that needs your attention - on the OCI board there is a somewhat incorrect representation of the content type string for Singularity (SIF), specifically it has a spelling mistake and the incorrect format (tar). The original discussion is here: opencontainers/image-spec#791 (comment) and I think it would be really important for someone that official represents Singularity (at Sylabs) to step in and offer opinion / wisdom. Thanks!

@jscook2345
Copy link
Contributor

@ikaneshiro Are you looking into this?

@vsoch
Copy link
Collaborator Author

vsoch commented Sep 13, 2019

I think so - he was pinged on one of the issues, and @jmstover mentioned in Slack he would chat with him about him. Thanks to all for being so responsive! OCI is pretty important.

@dtrudg
Copy link
Contributor

dtrudg commented Sep 13, 2019

I'm probably not super useful here as I lack the context somewhat, but let me know if there's any way I can assist.

@ikaneshiro
Copy link
Contributor

@jscook2345 yes, sorry about the latency needed to read through a couple things related to OCI naming conventions.

So as mentioned above, there are clear problems with the current sif layer mediaType found here: https://github.com/sylabs/singularity/blob/master/internal/pkg/oras/oras.go#L40

// SifLayerMediaType is the mediaType for the "layer" which contains the actual SIF file
	SifLayerMediaType = "appliciation/vnd.sylabs.sif.layer.tar"
  1. appliciation is a spelling mistake.
  2. This does not contain a tar but a sif image
  3. There should be a version attached to media type.

Since this is part of the artifacts component of OCI I'm planning on following this guidence as it is part of an open pr within the opencontainers/artifacts repo.

It seems best to change the mediaType name to:

	SifLayerMediaType = "application/vnd.sylabs.sif.layer.v1"

I do not see a need to have a +suffix , but if it would be best to use one, then +sif is the only one that seems reasonable to me given they are used to describe the format of data within the layer blob. The problem with applying a suffix here is it seems they are supposed to reflect general MIME types like compression algorithms(gzip) or standard notation forms(json) which sif is not(yet?).

Open to any suggestions!

@SteveLasker Will a change to this mediaType affect anything for azure?
It also looks like you are proposing some new ideas for defining artifact types. How mature is that work? Should we make efforts on that front as well?

@jmstover
Copy link
Contributor

SifLayerMediaType = "application/vnd.sylabs.sif.layer.v1"

What is the .v1 reference? Is that the version of the MediaType, not necessarily the SIF file format version right?

@SteveLasker
Copy link

.sif was the original plan, but that is raising some questions. This is why we want/need more feedback on the artifact Prs.
That said, the artifacts proposal is each artifact owner decides what they want for their type. The only requirement would be to uniquely identify and version it.
Yes ACR is open at this point to enable people to innovate.

@ikaneshiro
Copy link
Contributor

@jmstover Not referring to SIF version strictly, but there may be a point in the future where we want to change exactly what we are storing in this blob, so I think it's prudent to include a version piece.

@SteveLasker
Copy link

I like the example of versioning the persistence, even though the sif version may not change.

@dtrudg dtrudg added this to the 3.7.0 milestone Oct 16, 2020
@dtrudg
Copy link
Contributor

dtrudg commented Oct 26, 2020

Reading through all of this @ikaneshiro and looking at https://github.com/SteveLasker/artifacts/blob/registry-operators-how-to/authoring-artifacts.md#defining-the-artifact-type maybe we want...

SifLayerMediaTypeV1 = "application/vnd.sylabs.sif.layer.v1+sif"

Would love to get this set properly for 3.7.0 - I'll open a PR for comment ASAP so we don't lose track of it.

@SteveLasker
Copy link

Thanks David,
This is your value to set. Happy to accept a PR, including a PR that perhaps shows a history for others to understand the versioning aspects as well.

@dtrudg
Copy link
Contributor

dtrudg commented Oct 27, 2020

I think I might be off now...from looking at the opencontainers artifacts repo, rather than the SteveLasker one my use of +sif is incorrect.

https://github.com/opencontainers/artifacts/blob/master/artifact-authors.md#defining-layermediatypes

^ according to that one the +xxx bit is +[optional-compressionFormat]

So we'd be instead application/vnd.sylabs.sif.layer.v1.sif - which is the exact thing given as an example at that link.

@ikaneshiro - I'm going to update the PR #5658 for this - sorry!

@ikaneshiro
Copy link
Contributor

@dctrud Ah, good catch!

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 a pull request may close this issue.

7 participants