Abstract more info into the extra attributes for images (#13014)

1. Abstract the "config" property(which contains labels) of config layer into the extra attributes for images
2. Try to get the author information from the "maintainer" label

fixes 12066
fixes 12734
Signed-off-by: Wenkai Yin <yinw@vmware.com>

src/controller/artifact/processor/base/manifest.go

@@ -41,20 +41,9 @@ type ManifestProcessor struct {
 
 // AbstractMetadata abstracts metadata of artifact
 func (m *ManifestProcessor) AbstractMetadata(ctx context.Context, artifact *artifact.Artifact, content []byte) error {
-	// get manifest
-	manifest := &v1.Manifest{}
-	if err := json.Unmarshal(content, manifest); err != nil {
-		return err
-	}
-	// get config layer
-	_, blob, err := m.RegCli.PullBlob(artifact.RepositoryName, manifest.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 {
+	if err := m.UnmarshalConfig(ctx, artifact.RepositoryName, content, &metadata); err != nil {
 		return err
 	}
 	// if no properties specified, populate all metadata into the ExtraAttrs
@@ -87,3 +76,25 @@ func (m *ManifestProcessor) GetArtifactType(ctx context.Context, artifact *artif
 func (m *ManifestProcessor) ListAdditionTypes(ctx context.Context, artifact *artifact.Artifact) []string {
 	return nil
 }
+
+// UnmarshalConfig unmarshal the config blob of the artifact into the specified object "v"
+func (m *ManifestProcessor) UnmarshalConfig(ctx context.Context, repository string, manifest []byte, v interface{}) error {
+	// unmarshal manifest
+	mani := &v1.Manifest{}
+	if err := json.Unmarshal(manifest, mani); err != nil {
+		return err
+	}
+	// if the size of the config blob is 0(empty config blob), return directly
+	if mani.Config.Size == 0 {
+		return nil
+	}
+	// get config layer
+	_, blob, err := m.RegCli.PullBlob(repository, mani.Config.Digest.String())
+	if err != nil {
+		return err
+	}
+	defer blob.Close()
+
+	// unmarshal config layer
+	return json.NewDecoder(blob).Decode(v)
+}

src/controller/artifact/processor/base/manifest_test.go

@@ -21,6 +21,7 @@ import (
 
 	"github.com/goharbor/harbor/src/pkg/artifact"
 	"github.com/goharbor/harbor/src/testing/pkg/registry"
+	"github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -153,6 +154,15 @@ func (m *manifestTestSuite) TestAbstractMetadata() {
 	m.Equal("linux", art.ExtraAttrs["os"])
 }
 
+func (m *manifestTestSuite) TestUnmarshalConfig() {
+	m.regCli.On("PullBlob").Return(0, ioutil.NopCloser(strings.NewReader(config)), nil)
+	config := &v1.Image{}
+	err := m.processor.UnmarshalConfig(nil, "library/hello-world", []byte(manifest), config)
+	m.Require().Nil(err)
+	m.Equal("amd64", config.Architecture)
+	m.regCli.AssertExpectations(m.T())
+}
+
 func TestManifestSuite(t *testing.T) {
 	suite.Run(t, &manifestTestSuite{})
 }

src/controller/artifact/processor/chart/chart.go

@@ -57,13 +57,6 @@ type processor struct {
 	chartOperator chart.Operator
 }
 
-func (p *processor) AbstractMetadata(ctx context.Context, artifact *artifact.Artifact, manifest []byte) error {
-	if err := p.ManifestProcessor.AbstractMetadata(ctx, artifact, manifest); err != nil {
-		return err
-	}
-	return nil
-}
-
 func (p *processor) AbstractAddition(ctx context.Context, artifact *artifact.Artifact, addition string) (*ps.Addition, error) {
 	if addition != AdditionTypeValues && addition != AdditionTypeReadme && addition != AdditionTypeDependencies {
 		return nil, errors.New(nil).WithCode(errors.BadRequestCode).

src/controller/artifact/processor/chart/chart_test.go

@@ -15,7 +15,6 @@
 package chart
 
 import (
-	"bytes"
 	"io/ioutil"
 	"strings"
 	"testing"
@@ -25,7 +24,6 @@ import (
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/pkg/artifact"
 	chartserver "github.com/goharbor/harbor/src/pkg/chart"
-	"github.com/goharbor/harbor/src/testing/mock"
 	"github.com/goharbor/harbor/src/testing/pkg/chart"
 	"github.com/goharbor/harbor/src/testing/pkg/registry"
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
@@ -76,14 +74,6 @@ func (p *processorTestSuite) SetupTest() {
 	p.processor.ManifestProcessor = &base.ManifestProcessor{RegCli: p.regCli}
 }
 
-func (p *processorTestSuite) TestAbstractMetadata() {
-	artifact := &artifact.Artifact{}
-	p.regCli.On("PullBlob", mock.Anything, mock.Anything).Return(0, ioutil.NopCloser(bytes.NewReader([]byte(chartYaml))), nil)
-	err := p.processor.AbstractMetadata(nil, artifact, []byte(chartManifest))
-	p.Require().Nil(err)
-	p.regCli.AssertExpectations(p.T())
-}
-
 func (p *processorTestSuite) TestAbstractAddition() {
 	// unknown addition
 	_, err := p.processor.AbstractAddition(nil, nil, "unknown_addition")

src/controller/artifact/processor/image/index.go

@@ -43,13 +43,6 @@ type indexProcessor struct {
 	*base.IndexProcessor
 }
 
-func (i *indexProcessor) AbstractMetadata(ctx context.Context, artifact *artifact.Artifact, manifest []byte) error {
-	if err := i.IndexProcessor.AbstractMetadata(ctx, artifact, manifest); err != nil {
-		return err
-	}
-	return nil
-}
-
 func (i *indexProcessor) GetArtifactType(ctx context.Context, artifact *artifact.Artifact) string {
 	return ArtifactTypeImage
 }

src/controller/artifact/processor/image/index_test.go

@@ -17,7 +17,6 @@ package image
 import (
 	"testing"
 
-	"github.com/goharbor/harbor/src/pkg/artifact"
 	"github.com/stretchr/testify/suite"
 )
 
@@ -30,12 +29,6 @@ func (i *indexProcessTestSuite) SetupTest() {
 	i.processor = &indexProcessor{}
 }
 
-func (i *indexProcessTestSuite) TestAbstractMetadata() {
-	artifact := &artifact.Artifact{}
-	err := i.processor.AbstractMetadata(nil, artifact, nil)
-	i.Require().Nil(err)
-}
-
 func (i *indexProcessTestSuite) TestGetArtifactType() {
 	i.Assert().Equal(ArtifactTypeImage, i.processor.GetArtifactType(nil, nil))
 }

src/controller/artifact/processor/image/manifest_v2.go

@@ -36,7 +36,7 @@ const (
 
 func init() {
 	pc := &manifestV2Processor{}
-	pc.ManifestProcessor = base.NewManifestProcessor("created", "author", "architecture", "os")
+	pc.ManifestProcessor = base.NewManifestProcessor()
 	mediaTypes := []string{
 		v1.MediaTypeImageConfig,
 		schema2.MediaTypeImageConfig,
@@ -53,9 +53,24 @@ type manifestV2Processor struct {
 }
 
 func (m *manifestV2Processor) AbstractMetadata(ctx context.Context, artifact *artifact.Artifact, manifest []byte) error {
-	if err := m.ManifestProcessor.AbstractMetadata(ctx, artifact, manifest); err != nil {
+	config := &v1.Image{}
+	if err := m.ManifestProcessor.UnmarshalConfig(ctx, artifact.RepositoryName, manifest, config); err != nil {
 		return err
 	}
+	if artifact.ExtraAttrs == nil {
+		artifact.ExtraAttrs = map[string]interface{}{}
+	}
+	artifact.ExtraAttrs["created"] = config.Created
+	artifact.ExtraAttrs["architecture"] = config.Architecture
+	artifact.ExtraAttrs["os"] = config.OS
+	artifact.ExtraAttrs["config"] = config.Config
+	// if the author is null, try to get it from labels:
+	// https://docs.docker.com/engine/reference/builder/#maintainer-deprecated
+	author := config.Author
+	if len(author) == 0 && len(config.Config.Labels) > 0 {
+		author = config.Config.Labels["maintainer"]
+	}
+	artifact.ExtraAttrs["author"] = author
 	return nil
 }
 
@@ -64,6 +79,7 @@ func (m *manifestV2Processor) AbstractAddition(ctx context.Context, artifact *ar
 		return nil, errors.New(nil).WithCode(errors.BadRequestCode).
 			WithMessage("addition %s isn't supported for %s(manifest version 2)", addition, ArtifactTypeImage)
 	}
+
 	mani, _, err := m.RegCli.PullManifest(artifact.RepositoryName, artifact.Digest)
 	if err != nil {
 		return nil, err
@@ -72,19 +88,11 @@ func (m *manifestV2Processor) AbstractAddition(ctx context.Context, artifact *ar
 	if err != nil {
 		return nil, err
 	}
-	manifest := &v1.Manifest{}
-	if err := json.Unmarshal(content, manifest); err != nil {
-		return nil, err
-	}
-	_, blob, err := m.RegCli.PullBlob(artifact.RepositoryName, manifest.Config.Digest.String())
-	if err != nil {
-		return nil, err
-	}
-	image := &v1.Image{}
-	if err := json.NewDecoder(blob).Decode(image); err != nil {
+	config := &v1.Image{}
+	if err = m.ManifestProcessor.UnmarshalConfig(ctx, artifact.RepositoryName, content, config); err != nil {
 		return nil, err
 	}
-	content, err = json.Marshal(image.History)
+	content, err = json.Marshal(config.History)
 	if err != nil {
 		return nil, err
 	}

src/controller/artifact/processor/image/manifest_v2_test.go

@@ -71,7 +71,9 @@ var (
     "WorkingDir": "",
     "Entrypoint": null,
     "OnBuild": null,
-    "Labels": null
+    "Labels": {
+		"maintainer": "tester@vmware.com"	
+	}
   },
   "container": "8e2caa5a514bb6d8b4f2a2553e9067498d261a0fd83a96aeaaf303943dff6ff9",
   "container_config": {
@@ -100,7 +102,6 @@ var (
     "Entrypoint": null,
     "OnBuild": null,
     "Labels": {
-
     }
   },
   "created": "2019-01-01T01:29:27.650294696Z",
@@ -143,6 +144,11 @@ func (m *manifestV2ProcessorTestSuite) TestAbstractMetadata() {
 	m.regCli.On("PullBlob", mock.Anything, mock.Anything).Return(0, ioutil.NopCloser(bytes.NewReader([]byte(config))), nil)
 	err := m.processor.AbstractMetadata(nil, artifact, []byte(manifest))
 	m.Require().Nil(err)
+	m.NotNil(artifact.ExtraAttrs["created"])
+	m.Equal("amd64", artifact.ExtraAttrs["architecture"])
+	m.Equal("linux", artifact.ExtraAttrs["os"])
+	m.NotNil(artifact.ExtraAttrs["config"])
+	m.Equal("tester@vmware.com", artifact.ExtraAttrs["author"])
 	m.regCli.AssertExpectations(m.T())
 }