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

Request for Feedback: OCI image-spec 1.1.0 #1011

Closed
sajayantony opened this issue Jul 6, 2023 · 9 comments
Closed

Request for Feedback: OCI image-spec 1.1.0 #1011

sajayantony opened this issue Jul 6, 2023 · 9 comments
Labels
enhancement New feature or request stale Inactive issues or pull requests

Comments

@sajayantony
Copy link
Contributor

sajayantony commented Jul 6, 2023

OCI is approaching a 1.1.0 release on the image-spec and distribution-spec, so we are reaching out to implementations to see if you have any concerns implementing support for those changes that we should address before a GA release.

A full diff of the changes can be viewed at:

I'm happy to take feedback here, you can raise an issue in OCI, or respond in our tracking issue at opencontainers/image-spec#1093.

@sajayantony sajayantony added the enhancement New feature or request label Jul 6, 2023
@qweeah
Copy link
Contributor

qweeah commented Aug 3, 2023

Several questions regarding what kind of artifacts should be produced by oras commands with OCI image-spec 1.1.0.

Taking oras push as an example, currently it supports user-specified artifact type via --artifact-type. If the flag is used when packing an OCI image manifest, oras will put the specified artifact type into config.mediatype.

The guidelines for artifact usage in image-spec rc4, the artifact can be put into either 1) artifactType or 2) config.mediaType.

Question 1: Between artifactType and config.mediaType, which is the preferred choice to store artifact type when packing an artifact? We need a default value for packing, the latter provides better compatibility, and the former is newly introduced in the spec and provides better performance for pushing (dont need to upload config blob).

Question 2: If the artifact type is packed into the artifactType field of the image manifest, if the registry doesn't support pushing a manifest with artifactType, should oras provide auto fallback to pack the artifact type into config.mediaType?

@shizhMSFT
Copy link
Contributor

As I've mentioned in #1043 (comment), oras push is a stable feature of v1.x versions and thus we should not change the behavior of putting artifact type in config.mediaType by default. Meanwhile, we should provide an option for users like --image-spec v1.1 to put artifact type in artifactType and an empty value to config.

For question 2, I would suggest no fallback based on the above default behavior.

@qweeah
Copy link
Contributor

qweeah commented Aug 3, 2023

If a user provides an artifact type without specifying image spec version, and oras v1.1 pack it into artifactType field, should it be regarded as a breaking change? @sajayantony

Here is a detailed example based on oras push: suppose below command is run:

oras push localhost:5000/test:artifact --artifact-type test.artifact.type

oras v1.0.0 will generate below manifest:

{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.manifest.v1+json",
  "config": {
    "mediaType": "test.artifact.type",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "size": 2
  },
  "layers": [],
  "annotations": {
    ...
  }
}

My question is: if oras v1.1.x generates below manifest for the same command, should it be considered as a breaking change?

{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.manifest.v1+json",
  "artifactType": "test.artifact.type",
  "config": {
    "mediaType": "application/vnd.oci.empty.v1+json",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "size": 2
  },
  "layers": [],
  "annotations": {
    ...
  }
}

To facilitate understanding, here is the diff of manifests generated by v1.0.0(before) and v1.1.x(after)

{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.manifest.v1+json",
+  "artifactType": "test.artifact.type",
  "config": {
-    "mediaType": "test.artifact.type",
+    "mediaType": "application/vnd.oci.empty.v1+json",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "size": 2
  },
  "layers": [],
  "annotations": {
    ...
  }
}

@sudo-bmitch
Copy link

We had to add the artifactType to the spec because in the example:

{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.manifest.v1+json",
  "config": {
    "mediaType": "test.artifact.type",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "size": 2
  },
  "layers": [],
  "annotations": {
    // ...
  }
}

The blob sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a does not match "mediaType": "test.artifact.type". That means we were in violation of the descriptor spec which specified for the mediaType of the config descriptor (along with every other descriptor):

This REQUIRED property contains the media type of the referenced content.

There was a conflict between that mediaType being used for the artifact and that mediaType being used to reference the blob content. Hence the empty descriptor definition was added along with the artifactType field to get back into compliance with the spec while supporting artifact use cases that did not have their own dedicated config blob.

@qweeah
Copy link
Contributor

qweeah commented Aug 3, 2023

@sudo-bmitch Thanks for the feedback. Just to confirm, are you suggesting that below is the preferred artifact manifest that should be produced by oras CLI to avoid any ambiguity:

{
...
  "artifactType": "test.artifact.type",  // specified via `--artifact-type`
  "config": {
    "mediaType": "application/vnd.oci.empty.v1+json",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "size": 2
  },
...
}

@sudo-bmitch
Copy link

@qweeah yes. I'd still read the old format, but write the new one for artifacts going forward. Let me know if you see registries rejecting that today. At least check, I think ECR started allowing it, which was the only hold out that I was aware of.

@sudo-bmitch
Copy link

To clarify my above comment (since it's getting linked outside of this issue), generating with the artifactType field defined and config set to the empty descriptor is preferred when the artifact does not have a dedicated config blob.

When there is a config blob for the artifact (e.g. helm charts), there's often no need to add an additional artifactType field to duplicate the config.mediaType.

If an artifact needs to override the config.mediaType value for some reason (e.g. multiple artifacts of the same type but with different configs, maybe the configs are versioned or available in different formats and the artifactType is common base), then defining both may make sense.

@github-actions
Copy link

github-actions bot commented Oct 3, 2023

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the stale Inactive issues or pull requests label Oct 3, 2023
Copy link

github-actions bot commented Nov 3, 2023

This issue was closed because it has been stalled for 30 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request stale Inactive issues or pull requests
Projects
None yet
Development

No branches or pull requests

4 participants