Skip to content

Commit

Permalink
fix: pack should not set artifactType when config is specified
Browse files Browse the repository at this point in the history
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
  • Loading branch information
Wwwsylvia committed Aug 7, 2023
1 parent d5cefe9 commit 385acbc
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 13 deletions.
12 changes: 5 additions & 7 deletions pack.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,11 @@ func packImageRC2(ctx context.Context, pusher content.Pusher, configMediaType st
// If succeeded, returns a descriptor of the manifest.
// Reference: https://github.com/opencontainers/image-spec/blob/v1.1.0-rc4/manifest.md#guidelines-for-artifact-usage
func packImageRC4(ctx context.Context, pusher content.Pusher, artifactType string, layers []ocispec.Descriptor, opts PackOptions) (ocispec.Descriptor, error) {
if artifactType == "" && (opts.ConfigDescriptor == nil || opts.ConfigDescriptor.MediaType == ocispec.MediaTypeEmptyJSON) {
// artifactType MUST be set when config.mediaType is set to the empty value
return ocispec.Descriptor{}, ErrMissingArtifactType
}

var emptyBlobExists bool
var configDesc ocispec.Descriptor
if opts.ConfigDescriptor != nil {
Expand All @@ -232,13 +237,6 @@ func packImageRC4(ctx context.Context, pusher content.Pusher, artifactType strin
}
emptyBlobExists = true
}
if artifactType == "" {
if configDesc.MediaType == ocispec.MediaTypeEmptyJSON {
// artifactType MUST be set when config.mediaType is set to the empty value
return ocispec.Descriptor{}, ErrMissingArtifactType
}
artifactType = configDesc.MediaType
}

annotations, err := ensureAnnotationCreated(opts.ManifestAnnotations, ocispec.AnnotationCreated)
if err != nil {
Expand Down
33 changes: 27 additions & 6 deletions pack_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -678,6 +678,14 @@ func Test_Pack_ImageRC4_WithOptions(t *testing.T) {
}

// test Pack with ConfigDescriptor, but without artifactType
opts = PackOptions{
PackImageManifest: true,
PackManifestType: PackManifestTypeImageV1_1_0_RC4,
Subject: &subjectDesc,
ConfigDescriptor: &configDesc,
ConfigAnnotations: configAnnotations,
ManifestAnnotations: annotations,
}
manifestDesc, err = Pack(ctx, s, "", layers, opts)
if err != nil {
t.Fatal("Oras.Pack() error =", err)
Expand All @@ -687,12 +695,11 @@ func Test_Pack_ImageRC4_WithOptions(t *testing.T) {
Versioned: specs.Versioned{
SchemaVersion: 2, // historical value. does not pertain to OCI or docker version
},
MediaType: ocispec.MediaTypeImageManifest,
ArtifactType: configDesc.MediaType,
Subject: &subjectDesc,
Config: configDesc,
Layers: layers,
Annotations: annotations,
MediaType: ocispec.MediaTypeImageManifest,
Subject: &subjectDesc,
Config: configDesc,
Layers: layers,
Annotations: annotations,
}
expectedManifestBytes, err = json.Marshal(expectedManifest)
if err != nil {
Expand Down Expand Up @@ -783,6 +790,7 @@ func Test_Pack_ImageRC4_NoArtifactType(t *testing.T) {
s := memory.New()

ctx := context.Background()
// test no artifact type and no config
opts := PackOptions{
PackImageManifest: true,
PackManifestType: PackManifestTypeImageV1_1_0_RC4,
Expand All @@ -791,6 +799,19 @@ func Test_Pack_ImageRC4_NoArtifactType(t *testing.T) {
if wantErr := ErrMissingArtifactType; !errors.Is(err, wantErr) {
t.Errorf("Oras.Pack() error = %v, wantErr = %v", err, wantErr)
}

// test no artifact type and config with media type empty
opts = PackOptions{
PackImageManifest: true,
PackManifestType: PackManifestTypeImageV1_1_0_RC4,
ConfigDescriptor: &ocispec.Descriptor{
MediaType: ocispec.DescriptorEmptyJSON.MediaType,
},
}
_, err = Pack(ctx, s, "", nil, opts)
if wantErr := ErrMissingArtifactType; !errors.Is(err, wantErr) {
t.Errorf("Oras.Pack() error = %v, wantErr = %v", err, wantErr)
}
}

func Test_Pack_ImageRC4_NoLayer(t *testing.T) {
Expand Down

0 comments on commit 385acbc

Please sign in to comment.