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

exporter: remove wrappers for oci data types #3600

Merged
merged 1 commit into from
Feb 11, 2023

Conversation

jedevc
Copy link
Member

@jedevc jedevc commented Feb 8, 2023

opencontainers/image-spec now contains the MediaType field for all of its structs (since opencontainers/image-spec@5b82148, part of the v1.1.0-rc2 release which we already vendor).

With this update, we can avoid manually wrapping the internal data types to present this field.

@jedevc jedevc requested a review from tonistiigi February 8, 2023 14:43
opencontainers/image-spec now contains the MediaType field for all of
its structs. With this update, we can avoid manually wrapping the
internal data types to present this field.

This requires updating the expected digest for source-date-epoch, since
the ordering of JSON fields is now slightly different :)

Signed-off-by: Justin Chadwell <me@jedevc.com>
@jedevc jedevc force-pushed the exporter-simplify-oci-media-types branch from 73f5b88 to a89f482 Compare February 8, 2023 15:31
@@ -6518,7 +6518,7 @@ FROM scratch
COPY --from=0 / /
`)

const expectedDigest = "sha256:9e36395384d073e711102b13bd0ba4b779ef6afbaf5cadeb77fe77dba8967d1f"
const expectedDigest = "sha256:23bfe9c494f4b4ae9368d989035c70b3a34aa0bfc991618b3e54dcce2eee4bf8"
Copy link
Member

Choose a reason for hiding this comment

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

Why did this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

The order of JSON field marshalling changes 😢

Essentially, the mediaType doesn't come first anymore (since it's not defined at the top-level).

Copy link
Member

Choose a reason for hiding this comment

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

Could you give an example. Is the problem that Version comes before Mediatype now? FYI @AkihiroSuda

Copy link
Member Author

@jedevc jedevc Feb 8, 2023

Choose a reason for hiding this comment

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

Previously:

{
  "mediaType": "application/vnd.oci.image.index.v1+json",
  "schemaVersion": 2,
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:27594184c4ad11392de489e7e9eba36e4685177b3df91a7b1e13e9dbec97d427",
      "size": 1056,
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:5a9a7a10ff48bf889374e0bb96dfd0f817fb96b0d86d9ff738543ecf2836c3bf",
      "size": 566,
      "annotations": {
        "vnd.docker.reference.digest": "sha256:27594184c4ad11392de489e7e9eba36e4685177b3df91a7b1e13e9dbec97d427",
        "vnd.docker.reference.type": "attestation-manifest"
      },
      "platform": {
        "architecture": "unknown",
        "os": "unknown"
      }
    }
  ]
}

Now:

{
  "schemaVersion": 2,
  "mediaType": "application/vnd.oci.image.index.v1+json",
  "manifests": [
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:eb0454aabfa3084e80359c6dd64b2e88f26ca542a2af8a36eae752168bee1317",
      "size": 1056,
      "platform": {
        "architecture": "amd64",
        "os": "linux"
      }
    },
    {
      "mediaType": "application/vnd.oci.image.manifest.v1+json",
      "digest": "sha256:113c2a57ab2a69ec43f1fab69464c613697ae288cb24126422fc309939805311",
      "size": 566,
      "annotations": {
        "vnd.docker.reference.digest": "sha256:eb0454aabfa3084e80359c6dd64b2e88f26ca542a2af8a36eae752168bee1317",
        "vnd.docker.reference.type": "attestation-manifest"
      },
      "platform": {
        "architecture": "unknown",
        "os": "unknown"
      }
    }
  ]
}

schemaVersion appears first now (I think because marshaling visits each struct recursively, starting with the top-most, then descending down into embedded structs afterwards).

(note the change in digests is caused by this property applied recursively).

Copy link
Member

Choose a reason for hiding this comment

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

Ah, yeah, noticed that as well some time ago; originally the MediaType part of specs.Versioned;
opencontainers/image-spec@4267a18 (opencontainers/image-spec#172)

Then got removed in opencontainers/image-spec@5fc84e5 (opencontainers/image-spec#411)

And when it was added back, it was added to the manifest itself (instead of Versioned) opencontainers/image-spec@5b82148 ()

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 this pull request may close these issues.

4 participants