update default processor for unknwon type config (#19372)

update OCI-Subject header Signed-off-by: yminer <[email protected]> update default processor & add ut for artifact icon update ut coverage
goharbor · Sep 21, 2023 · 6fd4a2b · 6fd4a2b
1 parent 50e466b
commit 6fd4a2b
Show file tree

Hide file tree

Showing 4 changed files with 121 additions and 16 deletions.
diff --git a/src/controller/artifact/annotation/v1alpha1_test.go b/src/controller/artifact/annotation/v1alpha1_test.go
@@ -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",
@@ -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()
@@ -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))
@@ -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))

diff --git a/src/controller/artifact/processor/default.go b/src/controller/artifact/processor/default.go
@@ -101,24 +101,25 @@ func (d *defaultProcessor) AbstractMetadata(ctx context.Context, artifact *artif
 	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 {
 			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
+		}
+		// Populate all metadata into the ExtraAttrs first.
+		artifact.ExtraAttrs = metadata
 	}
-	// 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)
 	}

diff --git a/src/controller/artifact/processor/default_test.go b/src/controller/artifact/processor/default_test.go
@@ -16,6 +16,7 @@ package processor
 
 import (
 	"context"
+	"encoding/json"
 	"io"
 	"strings"
 	"testing"
@@ -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}`
 )
 
 type defaultProcessorTestSuite struct {
@@ -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{}
@@ -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) {

diff --git a/src/server/middleware/subject/subject.go b/src/server/middleware/subject/subject.go
@@ -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