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.

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

Jump to bottom

update default processor for unknwon type config #19372

Merged
merged 1 commit into from
Sep 21, 2023
Merged

update default processor for unknwon type config #19372

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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
43 changes: 43 additions & 0 deletions src/controller/artifact/annotation/v1alpha1_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,31 @@ var (
}
]
}`

unknownManifestwithIcon = `{
"schemaVersion":2,
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"config": {
"mediaType": "application/vnd.nhl.peanut.butter.bagel",
"digest": "sha256:ee29d2e91da0e5dbf6536f5b369148a83ef59b0ce96e49da65dd6c25eb1fa44f",
"size": 33
},
"layers":[
{
"mediaType": "image/png",
"digest": "sha256:d923b93eadde0af5c639a972710a4d919066aba5d0dfbf4b9385099f70272da0",
"size": 166015,
"annotations": {
"io.goharbor.artifact.v1alpha1.icon": ""
}
},
{
"mediaType":"application/tar+gzip",
"digest":"sha256:eb6063fecbb50a9d98268cb61746a0fd62a27a4af9e850ffa543a1a62d3948b2",
"size":166022
}
]
}`
ormbManifestWithoutSkipList = `{
"schemaVersion":2,
"mediaType": "application/vnd.oci.image.manifest.v1+json",
Expand Down Expand Up @@ -183,8 +208,24 @@ func (p *v1alpha1TestSuite) SetupTest() {
regCli: p.regCli,
}
}
func (p *v1alpha1TestSuite) TestParseUnknownConfig() {
manifest, _, err := distribution.UnmarshalManifest(v1.MediaTypeImageManifest, []byte(unknownManifestwithIcon))
p.Require().Nil(err)
manifestMediaType, content, err := manifest.Payload()
p.Require().Nil(err)

art := &artifact.Artifact{ManifestMediaType: manifestMediaType}

blob := io.NopCloser(base64.NewDecoder(base64.StdEncoding, strings.NewReader(ormbIcon)))
p.regCli.On("PullBlob", mock.Anything, mock.Anything).Return(int64(0), blob, nil)
err = p.v1alpha1Parser.Parse(nil, art, content)
p.Require().Nil(err)
p.Len(art.ExtraAttrs, 0)
p.Equal("sha256:d923b93eadde0af5c639a972710a4d919066aba5d0dfbf4b9385099f70272da0", art.Icon)

}
func (p *v1alpha1TestSuite) TestParse() {
// ormbManifest
manifest, _, err := distribution.UnmarshalManifest(v1.MediaTypeImageManifest, []byte(ormbManifest))
p.Require().Nil(err)
manifestMediaType, content, err := manifest.Payload()
Expand All @@ -206,6 +247,7 @@ func (p *v1alpha1TestSuite) TestParse() {
p.Equal([]interface{}{map[string]interface{}{"name": "batch_size", "value": "32"}}, art.ExtraAttrs["hyperparameters"])
p.Equal("sha256:d923b93eadde0af5c639a972710a4d919066aba5d0dfbf4b9385099f70272da0", art.Icon)

// ormbManifestWithoutSkipList
// reset the mock
p.SetupTest()
manifest, _, err = distribution.UnmarshalManifest(v1.MediaTypeImageManifest, []byte(ormbManifestWithoutSkipList))
Expand All @@ -229,6 +271,7 @@ func (p *v1alpha1TestSuite) TestParse() {
p.Equal([]interface{}{map[string]interface{}{"name": "batch_size", "value": "32"}}, art.ExtraAttrs["hyperparameters"])
p.Equal("sha256:d923b93eadde0af5c639a972710a4d919066aba5d0dfbf4b9385099f70272da0", art.Icon)

// ormbManifestWithoutIcon
// reset the mock
p.SetupTest()
manifest, _, err = distribution.UnmarshalManifest(v1.MediaTypeImageManifest, []byte(ormbManifestWithoutIcon))
Expand Down
29 changes: 15 additions & 14 deletions src/controller/artifact/processor/default.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,24 +101,25 @@
if err := json.Unmarshal(manifest, mani); err != nil {
return err
}
// get config layer
_, blob, err := d.regCli.PullBlob(artifact.RepositoryName, mani.Config.Digest.String())
if err != nil {
return err
}
defer blob.Close()
// parse metadata from config layer
metadata := map[string]interface{}{}
// Some artifact may not have empty config layer.
if mani.Config.Size != 0 {
if err := json.NewDecoder(blob).Decode(&metadata); err != nil {
// if artifact.MediaType match regex, will set artifact.ExtraAttrs
if d.GetArtifactType(ctx, artifact) != ArtifactTypeUnknown {
// get config layer
_, blob, err := d.regCli.PullBlob(artifact.RepositoryName, mani.Config.Digest.String())
if err != nil {

Check warning on line 108 in src/controller/artifact/processor/default.go

View check run for this annotation

Codecov / codecov/patch

src/controller/artifact/processor/default.go#L106-L108 

Added lines #L106 - L108 were not covered by tests
return err
}
defer blob.Close()
// parse metadata from config layer
metadata := map[string]interface{}{}
if err = json.NewDecoder(blob).Decode(&metadata); err != nil {
return err
}

Check warning on line 116 in src/controller/artifact/processor/default.go

View check run for this annotation

Codecov / codecov/patch

src/controller/artifact/processor/default.go#L111-L116 

Added lines #L111 - L116 were not covered by tests
// Populate all metadata into the ExtraAttrs first.
artifact.ExtraAttrs = metadata

Check warning on line 118 in src/controller/artifact/processor/default.go

View check run for this annotation

Codecov / codecov/patch

src/controller/artifact/processor/default.go#L118 

Added line #L118 was not covered by tests
}
// Populate all metadata into the ExtraAttrs first.
artifact.ExtraAttrs = metadata

annotationParser := annotation.NewParser()
err = annotationParser.Parse(ctx, artifact, manifest)
err := annotationParser.Parse(ctx, artifact, manifest)
if err != nil {
log.Errorf("the annotation parser parse annotation for artifact error: %v", err)
}
Expand Down
61 changes: 60 additions & 1 deletion src/controller/artifact/processor/default_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package processor

import (
"context"
"encoding/json"
"io"
"strings"
"testing"
Expand Down Expand Up @@ -116,6 +117,31 @@ var (
}
]
}`
v2ManifestWithUnknownConfig = `{
"schemaVersion": 2,
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"config": {
"mediaType": "application/vnd.nhl.peanut.butter.bagel",
"digest": "sha256:ee29d2e91da0e5dbf6536f5b369148a83ef59b0ce96e49da65dd6c25eb1fa44f",
"size": 33,
"newUnspecifiedField": null
},
"layers": [
{
"mediaType": "application/vnd.oci.empty.v1+json",
"digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
"size": 2,
"newUnspecifiedField": "null"
}
],
"subject": {
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"digest": "sha256:5a01bbc4ce6f52541cbc7e6af4b22bb107991a4bdd433103ff65aeb00756e906",
"size": 714,
"newUnspecifiedField": null
}
}`
unknownConfig = `{NHL Peanut Butter on my NHL bagel}`
MinerYang marked this conversation as resolved.
Show resolved Hide resolved
)

type defaultProcessorTestSuite struct {
Expand Down Expand Up @@ -146,6 +172,18 @@ func (d *defaultProcessorTestSuite) TestGetArtifactType() {
typee = processor.GetArtifactType(nil, art)
d.Equal(ArtifactTypeUnknown, typee)

mediaType = "application/vnd.oci.empty.v1+json"
art = &artifact.Artifact{MediaType: mediaType}
processor = &defaultProcessor{}
typee = processor.GetArtifactType(nil, art)
d.Equal(ArtifactTypeUnknown, typee)

mediaType = "application/vnd.nhl.peanut.butter.bagel"
art = &artifact.Artifact{MediaType: mediaType}
processor = &defaultProcessor{}
typee = processor.GetArtifactType(nil, art)
d.Equal(ArtifactTypeUnknown, typee)

mediaType = "application/vnd.oci.image.config.v1+json"
art = &artifact.Artifact{MediaType: mediaType}
processor = &defaultProcessor{}
Expand Down Expand Up @@ -177,12 +215,33 @@ func (d *defaultProcessorTestSuite) TestAbstractMetadata() {
manifestMediaType, content, err := manifest.Payload()
d.Require().Nil(err)

metadata := map[string]interface{}{}
configBlob := io.NopCloser(strings.NewReader(ormbConfig))
art := &artifact.Artifact{ManifestMediaType: manifestMediaType}
err = json.NewDecoder(configBlob).Decode(&metadata)
d.Require().Nil(err)
art := &artifact.Artifact{ManifestMediaType: manifestMediaType, ExtraAttrs: metadata}
d.Len(art.ExtraAttrs, 13)

d.regCli.On("PullBlob", mock.Anything, mock.Anything).Return(int64(0), configBlob, nil)
d.parser.On("Parse", context.TODO(), mock.AnythingOfType("*artifact.Artifact"), mock.AnythingOfType("[]byte")).Return(nil)
err = d.processor.AbstractMetadata(nil, art, content)
d.Require().Nil(err)
d.Len(art.ExtraAttrs, 12)
}

func (d *defaultProcessorTestSuite) TestAbstractMetadataWithUnknownConfig() {
manifest, _, err := distribution.UnmarshalManifest(v1.MediaTypeImageManifest, []byte(v2ManifestWithUnknownConfig))
d.Require().Nil(err)
manifestMediaType, content, err := manifest.Payload()
d.Require().Nil(err)

configBlob := io.NopCloser(strings.NewReader(unknownConfig))
d.regCli.On("PullBlob", mock.Anything, mock.Anything).Return(int64(0), configBlob, nil)
art := &artifact.Artifact{ManifestMediaType: manifestMediaType}
err = d.processor.AbstractMetadata(nil, art, content)
d.Require().Nil(err)
d.Len(art.ExtraAttrs, 0)
d.Len(unknownConfig, 35)
}

func TestDefaultProcessorTestSuite(t *testing.T) {
Expand Down
4 changes: 3 additions & 1 deletion src/server/middleware/subject/subject.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,9 @@ func Middleware() func(http.Handler) http.Handler {
return err
}
}
w.Header().Set("OCI-Subject", subjectArt.Digest)
// when subject artifact is pushed after accessory artifact, current subject artifact do not exist.
// so we use reference manifest subject digest instead of subjectArt.Digest
w.Header().Set("OCI-Subject", mf.Subject.Digest.String())
}

return nil
Expand Down