Skip to content

exporter: remove wrappers for oci data types #3600

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.

Sign up for GitHub

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

Merged
tonistiigi merged 1 commit into from
Feb 11, 2023
Merged

exporter: remove wrappers for oci data types #3600

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 23 additions & 47 deletions exporter/containerimage/writer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,19 +178,11 @@ func (ic *ImageWriter) Commit(ctx context.Context, inp *exporter.Source, session
return nil, err
}

idx := struct {
// MediaType is reserved in the OCI spec but
// excluded from go types.
MediaType string `json:"mediaType,omitempty"`

ocispecs.Index
}{
MediaType: ocispecs.MediaTypeImageIndex,
Index: ocispecs.Index{
Annotations: opts.Annotations.Platform(nil).Index,
Versioned: specs.Versioned{
SchemaVersion: 2,
},
idx := ocispecs.Index{
MediaType: ocispecs.MediaTypeImageIndex,
Annotations: opts.Annotations.Platform(nil).Index,
Versioned: specs.Versioned{
SchemaVersion: 2,
},
}

Expand Down Expand Up @@ -386,24 +378,16 @@ func (ic *ImageWriter) commitDistributionManifest(ctx context.Context, opts *Ima
configType = images.MediaTypeDockerSchema2Config
}

mfst := struct {
// MediaType is reserved in the OCI spec but
// excluded from go types.
MediaType string `json:"mediaType,omitempty"`

ocispecs.Manifest
}{
MediaType: manifestType,
Manifest: ocispecs.Manifest{
Annotations: annotations.Manifest,
Versioned: specs.Versioned{
SchemaVersion: 2,
},
Config: ocispecs.Descriptor{
Digest: configDigest,
Size: int64(len(config)),
MediaType: configType,
},
mfst := ocispecs.Manifest{
MediaType: manifestType,
Annotations: annotations.Manifest,
Versioned: specs.Versioned{
SchemaVersion: 2,
},
Config: ocispecs.Descriptor{
Digest: configDigest,
Size: int64(len(config)),
MediaType: configType,
},
}

Expand Down Expand Up @@ -499,23 +483,15 @@ func (ic *ImageWriter) commitAttestationsManifest(ctx context.Context, opts *Ima
MediaType: configType,
}

mfst := struct {
// MediaType is reserved in the OCI spec but
// excluded from go types.
MediaType string `json:"mediaType,omitempty"`

ocispecs.Manifest
}{
mfst := ocispecs.Manifest{
MediaType: manifestType,
Manifest: ocispecs.Manifest{
Versioned: specs.Versioned{
SchemaVersion: 2,
},
Config: ocispecs.Descriptor{
Digest: configDigest,
Size: int64(len(config)),
MediaType: configType,
},
Versioned: specs.Versioned{
SchemaVersion: 2,
},
Config: ocispecs.Descriptor{
Digest: configDigest,
Size: int64(len(config)),
MediaType: configType,
},
}

Expand Down
2 changes: 1 addition & 1 deletion frontend/dockerfile/dockerfile_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6518,7 +6518,7 @@ FROM scratch
COPY --from=0 / /
`)

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

@tonistiigi tonistiigi Feb 8, 2023

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

@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.

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

@tonistiigi tonistiigi Feb 8, 2023

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 •
edited
Loading

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

@thaJeztah thaJeztah Feb 8, 2023

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 ()


dir, err := integration.Tmpdir(
t,
Expand Down