Merge pull request #12897 from reasonerjt/icon-refactor

Fill in the icon of known artifacts in artifact controller

make/migrations/postgresql/0040_2.1.0_schema.up.sql

@@ -142,16 +142,3 @@ ALTER TABLE schema_migrations DROP COLUMN IF EXISTS data_version;
 
 ALTER TABLE artifact ADD COLUMN IF NOT EXISTS icon varchar(255);
 
-UPDATE artifact
-SET icon=(
-CASE
-    WHEN type='IMAGE' THEN
-        'sha256:0048162a053eef4d4ce3fe7518615bef084403614f8bca43b40ae2e762e11e06'
-    WHEN type='CHART' THEN
-        'sha256:61cf3a178ff0f75bf08a25d96b75cf7355dc197749a9f128ed3ef34b0df05518'
-    WHEN type='CNAB' THEN
-        'sha256:089bdda265c14d8686111402c8ad629e8177a1ceb7dcd0f7f39b6480f623b3bd'
-    ELSE
-        'sha256:da834479c923584f4cbcdecc0dac61f32bef1d51e8aae598cf16bd154efab49f'
-END);
-

src/controller/artifact/controller.go

@@ -22,12 +22,10 @@ import (
 	"strings"
 	"time"
 
-	// registry image resolvers
+	"github.com/goharbor/harbor/src/controller/artifact/processor/chart"
-	_ "github.com/goharbor/harbor/src/controller/artifact/processor/image"
+	"github.com/goharbor/harbor/src/controller/artifact/processor/cnab"
-	// register chart resolver
+	"github.com/goharbor/harbor/src/controller/artifact/processor/image"
-	_ "github.com/goharbor/harbor/src/controller/artifact/processor/chart"
+	"github.com/goharbor/harbor/src/lib/icon"
-	// register CNAB resolver
-	_ "github.com/goharbor/harbor/src/controller/artifact/processor/cnab"
 
 	"github.com/goharbor/harbor/src/controller/artifact/processor"
 	"github.com/goharbor/harbor/src/controller/event/metadata"
@@ -64,6 +62,13 @@ var (
 
 	// ErrSkip error to skip walk the children of the artifact
 	ErrSkip = stderrors.New("skip")
+
+	// icon digests for each known type
+	defaultIcons = map[string]string{
+		image.ArtifactTypeImage: icon.DigestOfIconImage,
+		chart.ArtifactTypeChart: icon.DigestOfIconChart,
+		cnab.ArtifactTypeCNAB:   icon.DigestOfIconCNAB,
+	}
 )
 
 // Controller defines the operations related with artifacts and tags
@@ -565,6 +570,9 @@ func (c *controller) assembleArtifact(ctx context.Context, art *artifact.Artifac
 	// populate addition links
 	c.populateAdditionLinks(ctx, artifact)
 
+	// populate icon for the known artifact types
+	c.populateIcon(artifact)
+
 	if option == nil {
 		return artifact
 	}
@@ -577,6 +585,16 @@ func (c *controller) assembleArtifact(ctx context.Context, art *artifact.Artifac
 	return artifact
 }
 
+func (c *controller) populateIcon(art *Artifact) {
+	if len(art.Icon) == 0 {
+		if i, ok := defaultIcons[art.Type]; ok {
+			art.Icon = i
+		} else {
+			art.Icon = icon.DigestOfIconDefault
+		}
+	}
+}
+
 func (c *controller) populateTags(ctx context.Context, art *Artifact, option *tag.Option) {
 	tags, err := c.tagCtl.List(ctx, &q.Query{
 		Keywords: map[string]interface{}{

src/controller/artifact/controller_test.go

@@ -20,9 +20,13 @@ import (
 	"time"
 
 	"github.com/goharbor/harbor/src/common/models"
+	"github.com/goharbor/harbor/src/controller/artifact/processor/chart"
+	"github.com/goharbor/harbor/src/controller/artifact/processor/cnab"
+	"github.com/goharbor/harbor/src/controller/artifact/processor/image"
 	"github.com/goharbor/harbor/src/controller/tag"
 	"github.com/goharbor/harbor/src/lib"
 	"github.com/goharbor/harbor/src/lib/errors"
+	"github.com/goharbor/harbor/src/lib/icon"
 	"github.com/goharbor/harbor/src/lib/orm"
 	"github.com/goharbor/harbor/src/lib/q"
 	"github.com/goharbor/harbor/src/pkg/artifact"
@@ -124,11 +128,68 @@ func (c *controllerTestSuite) TestAssembleArtifact() {
 	artifact := c.ctl.assembleArtifact(ctx, art, option)
 	c.Require().NotNil(artifact)
 	c.Equal(art.ID, artifact.ID)
+	c.Equal(icon.DigestOfIconDefault, artifact.Icon)
 	c.Contains(artifact.Tags, tg)
 	c.Contains(artifact.Labels, lb)
 	// TODO check other fields of option
 }
 
+func (c *controllerTestSuite) TestPopulateIcon() {
+	cases := []struct {
+		art *artifact.Artifact
+		ico string
+	}{
+		{
+			art: &artifact.Artifact{
+				ID:     1,
+				Digest: "sha256:123",
+				Type:   image.ArtifactTypeImage,
+			},
+			ico: icon.DigestOfIconImage,
+		},
+		{
+			art: &artifact.Artifact{
+				ID:     2,
+				Digest: "sha256:456",
+				Type:   cnab.ArtifactTypeCNAB,
+			},
+			ico: icon.DigestOfIconCNAB,
+		},
+		{
+			art: &artifact.Artifact{
+				ID:     3,
+				Digest: "sha256:1234",
+				Type:   chart.ArtifactTypeChart,
+			},
+			ico: icon.DigestOfIconChart,
+		},
+		{
+			art: &artifact.Artifact{
+				ID:     4,
+				Digest: "sha256:1234",
+				Type:   "other",
+			},
+			ico: icon.DigestOfIconDefault,
+		},
+		{
+			art: &artifact.Artifact{
+				ID:     5,
+				Digest: "sha256:2345",
+				Type:   image.ArtifactTypeImage,
+				Icon:   "sha256:abcd",
+			},
+			ico: "sha256:abcd",
+		},
+	}
+	for _, cs := range cases {
+		a := &Artifact{
+			Artifact: *cs.art,
+		}
+		c.ctl.populateIcon(a)
+		c.Equal(cs.ico, a.Icon)
+	}
+}
+
 func (c *controllerTestSuite) TestEnsureArtifact() {
 	digest := "sha256:418fb88ec412e340cdbef913b8ca1bbe8f9e8dc705f9617414c1f2c8db980180"
 

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

@@ -21,7 +21,6 @@ import (
 
 	ps "github.com/goharbor/harbor/src/controller/artifact/processor"
 	"github.com/goharbor/harbor/src/controller/artifact/processor/base"
-	"github.com/goharbor/harbor/src/controller/icon"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/pkg/artifact"
@@ -62,7 +61,6 @@ func (p *processor) AbstractMetadata(ctx context.Context, artifact *artifact.Art
 	if err := p.ManifestProcessor.AbstractMetadata(ctx, artifact, manifest); err != nil {
 		return err
 	}
-	artifact.Icon = icon.DigestOfIconChart
 	return nil
 }
 

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

@@ -16,9 +16,12 @@ package chart
 
 import (
 	"bytes"
+	"io/ioutil"
+	"strings"
+	"testing"
+
 	"github.com/docker/distribution"
 	"github.com/goharbor/harbor/src/controller/artifact/processor/base"
-	"github.com/goharbor/harbor/src/controller/icon"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/pkg/artifact"
 	chartserver "github.com/goharbor/harbor/src/pkg/chart"
@@ -28,9 +31,6 @@ import (
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/suite"
 	helm_chart "helm.sh/helm/v3/pkg/chart"
-	"io/ioutil"
-	"strings"
-	"testing"
 )
 
 var (
@@ -81,7 +81,6 @@ func (p *processorTestSuite) TestAbstractMetadata() {
 	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.Equal(icon.DigestOfIconChart, artifact.Icon)
 	p.regCli.AssertExpectations(p.T())
 }
 

src/controller/artifact/processor/cnab/cnab.go

@@ -19,7 +19,6 @@ import (
 
 	ps "github.com/goharbor/harbor/src/controller/artifact/processor"
 	"github.com/goharbor/harbor/src/controller/artifact/processor/base"
-	"github.com/goharbor/harbor/src/controller/icon"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/pkg/artifact"
 )
@@ -47,7 +46,6 @@ type processor struct {
 }
 
 func (p *processor) AbstractMetadata(ctx context.Context, art *artifact.Artifact, manifest []byte) error {
-	art.Icon = icon.DigestOfIconCNAB
 	cfgManiDgt := ""
 	// try to get the digest of the manifest that the config layer is referenced by
 	for _, reference := range art.References {

src/controller/artifact/processor/cnab/cnab_test.go

@@ -15,16 +15,16 @@
 package cnab
 
 import (
+	"io/ioutil"
+	"strings"
+	"testing"
+
 	"github.com/docker/distribution"
 	"github.com/goharbor/harbor/src/controller/artifact/processor/base"
-	"github.com/goharbor/harbor/src/controller/icon"
 	"github.com/goharbor/harbor/src/pkg/artifact"
 	"github.com/goharbor/harbor/src/testing/pkg/registry"
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/stretchr/testify/suite"
-	"io/ioutil"
-	"strings"
-	"testing"
 )
 
 type processorTestSuite struct {
@@ -99,7 +99,6 @@ func (p *processorTestSuite) TestAbstractMetadata() {
 	p.Len(art.ExtraAttrs, 7)
 	p.Equal("0.1.1", art.ExtraAttrs["version"].(string))
 	p.Equal("helloworld", art.ExtraAttrs["name"].(string))
-	p.Equal(icon.DigestOfIconCNAB, art.Icon)
 }
 
 func (p *processorTestSuite) TestGetArtifactType() {

src/controller/artifact/processor/default.go

@@ -21,7 +21,6 @@ import (
 	"strings"
 
 	"github.com/docker/distribution/manifest/schema2"
-	"github.com/goharbor/harbor/src/controller/icon"
 	// annotation parsers will be registered
 	"github.com/goharbor/harbor/src/controller/artifact/annotation"
 	"github.com/goharbor/harbor/src/lib/errors"
@@ -122,9 +121,6 @@ func (d *defaultProcessor) AbstractMetadata(ctx context.Context, artifact *artif
 		log.Errorf("the annotation parser parse annotation for artifact error: %v", err)
 	}
 
-	if artifact.Icon == "" {
-		artifact.Icon = icon.DigestOfIconDefault
-	}
 	return nil
 }
 

src/controller/artifact/processor/default_test.go

@@ -16,14 +16,14 @@ package processor
 
 import (
 	"context"
-	"github.com/goharbor/harbor/src/controller/icon"
-	"github.com/goharbor/harbor/src/pkg/distribution"
-	"github.com/goharbor/harbor/src/testing/mock"
-	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"io/ioutil"
 	"strings"
 	"testing"
 
+	"github.com/goharbor/harbor/src/pkg/distribution"
+	"github.com/goharbor/harbor/src/testing/mock"
+	v1 "github.com/opencontainers/image-spec/specs-go/v1"
+
 	"github.com/goharbor/harbor/src/pkg/artifact"
 	"github.com/goharbor/harbor/src/testing/pkg/parser"
 	"github.com/goharbor/harbor/src/testing/pkg/registry"
@@ -184,7 +184,6 @@ func (d *defaultProcessorTestSuite) TestAbstractMetadata() {
 	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.Equal(icon.DigestOfIconDefault, art.Icon)
 }
 
 func TestDefaultProcessorTestSuite(t *testing.T) {

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

@@ -20,7 +20,6 @@ import (
 	"github.com/docker/distribution/manifest/manifestlist"
 	"github.com/goharbor/harbor/src/controller/artifact/processor"
 	"github.com/goharbor/harbor/src/controller/artifact/processor/base"
-	"github.com/goharbor/harbor/src/controller/icon"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/pkg/artifact"
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
@@ -48,7 +47,6 @@ func (i *indexProcessor) AbstractMetadata(ctx context.Context, artifact *artifac
 	if err := i.IndexProcessor.AbstractMetadata(ctx, artifact, manifest); err != nil {
 		return err
 	}
-	artifact.Icon = icon.DigestOfIconImage
 	return nil
 }
 

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

@@ -17,7 +17,6 @@ package image
 import (
 	"testing"
 
-	"github.com/goharbor/harbor/src/controller/icon"
 	"github.com/goharbor/harbor/src/pkg/artifact"
 	"github.com/stretchr/testify/suite"
 )
@@ -35,7 +34,6 @@ func (i *indexProcessTestSuite) TestAbstractMetadata() {
 	artifact := &artifact.Artifact{}
 	err := i.processor.AbstractMetadata(nil, artifact, nil)
 	i.Require().Nil(err)
-	i.Equal(icon.DigestOfIconImage, artifact.Icon)
 }
 
 func (i *indexProcessTestSuite) TestGetArtifactType() {

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

@@ -17,9 +17,9 @@ package image
 import (
 	"context"
 	"encoding/json"
+
 	"github.com/docker/distribution/manifest/schema1"
 	"github.com/goharbor/harbor/src/controller/artifact/processor"
-	"github.com/goharbor/harbor/src/controller/icon"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/pkg/artifact"
@@ -46,7 +46,6 @@ func (m *manifestV1Processor) AbstractMetadata(ctx context.Context, artifact *ar
 		artifact.ExtraAttrs = map[string]interface{}{}
 	}
 	artifact.ExtraAttrs["architecture"] = mani.Architecture
-	artifact.Icon = icon.DigestOfIconImage
 	return nil
 }
 

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

@@ -17,7 +17,6 @@ package image
 import (
 	"testing"
 
-	"github.com/goharbor/harbor/src/controller/icon"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/pkg/artifact"
 	"github.com/stretchr/testify/suite"
@@ -83,7 +82,6 @@ func (m *manifestV1ProcessorTestSuite) TestAbstractMetadata() {
 	err := m.processor.AbstractMetadata(nil, artifact, []byte(manifest))
 	m.Require().Nil(err)
 	m.Assert().Equal("amd64", artifact.ExtraAttrs["architecture"].(string))
-	m.Equal(icon.DigestOfIconImage, artifact.Icon)
 }
 
 func (m *manifestV1ProcessorTestSuite) TestAbstractAddition() {

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

@@ -21,7 +21,6 @@ import (
 	"github.com/docker/distribution/manifest/schema2"
 	"github.com/goharbor/harbor/src/controller/artifact/processor"
 	"github.com/goharbor/harbor/src/controller/artifact/processor/base"
-	"github.com/goharbor/harbor/src/controller/icon"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/lib/log"
 	"github.com/goharbor/harbor/src/pkg/artifact"
@@ -57,7 +56,6 @@ func (m *manifestV2Processor) AbstractMetadata(ctx context.Context, artifact *ar
 	if err := m.ManifestProcessor.AbstractMetadata(ctx, artifact, manifest); err != nil {
 		return err
 	}
-	artifact.Icon = icon.DigestOfIconImage
 	return nil
 }
 

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

@@ -23,7 +23,6 @@ import (
 	"github.com/docker/distribution"
 	"github.com/docker/distribution/manifest/schema2"
 	"github.com/goharbor/harbor/src/controller/artifact/processor/base"
-	"github.com/goharbor/harbor/src/controller/icon"
 	"github.com/goharbor/harbor/src/lib/errors"
 	"github.com/goharbor/harbor/src/pkg/artifact"
 	"github.com/goharbor/harbor/src/testing/mock"
@@ -144,7 +143,6 @@ 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.Equal(icon.DigestOfIconImage, artifact.Icon)
 	m.regCli.AssertExpectations(m.T())
 }
 

src/controller/icon/controller.go

@@ -29,26 +29,19 @@ import (
 	"sync"
 
 	"github.com/goharbor/harbor/src/lib/errors"
+	"github.com/goharbor/harbor/src/lib/icon"
 	"github.com/goharbor/harbor/src/lib/q"
 	"github.com/goharbor/harbor/src/pkg/artifact"
 	"github.com/goharbor/harbor/src/pkg/registry"
 	"github.com/nfnt/resize"
 )
 
-// const definitions
-const (
-	DigestOfIconImage   = "sha256:0048162a053eef4d4ce3fe7518615bef084403614f8bca43b40ae2e762e11e06"
-	DigestOfIconChart   = "sha256:61cf3a178ff0f75bf08a25d96b75cf7355dc197749a9f128ed3ef34b0df05518"
-	DigestOfIconCNAB    = "sha256:089bdda265c14d8686111402c8ad629e8177a1ceb7dcd0f7f39b6480f623b3bd"
-	DigestOfIconDefault = "sha256:da834479c923584f4cbcdecc0dac61f32bef1d51e8aae598cf16bd154efab49f"
-)
-
 var (
 	builtInIcons = map[string]string{
-		DigestOfIconImage:   "./icons/image.png",
+		icon.DigestOfIconImage:   "./icons/image.png",
-		DigestOfIconChart:   "./icons/chart.png",
+		icon.DigestOfIconChart:   "./icons/chart.png",
-		DigestOfIconCNAB:    "./icons/cnab.png",
+		icon.DigestOfIconCNAB:    "./icons/cnab.png",
-		DigestOfIconDefault: "./icons/default.png",
+		icon.DigestOfIconDefault: "./icons/default.png",
 	}
 	// Ctl is a global icon controller instance
 	Ctl = NewController()

src/controller/icon/controller_test.go

@@ -30,7 +30,7 @@ import (
 
 var (
 	// base64 encoded png icon for testing
-	icon = "iVBORw0KGgoAAAANSUhEUgAAAEAAAABACAMAAACdt4HsAAAAb1BMVEUAAAAkJG0kJFuAgIeDg4MmJmIjI2MnIV4mIWKCgoKAgIQmImCCgoYnI2EkIWKAgIOBgYWBgYOAgoSAgoQnI2EmImAmI2IlImEmIWEmImEmImEmImGAgYQmImGAgYWAgYQmImGAgYQmImGAgYT////3jpbIAAAAInRSTlMABw4gISIkLi8zNDU7QkZGSWdodoSHmp2g1djg4OPj8fT1aPEwQAAAAAFiS0dEJLQG+ZkAAACMSURBVFjD7dfLDoIwEEbho6ioeENQvCP6/u/opiSQNMYBF6b+Zz35Nm0mLY+eIUBAC9jnhgoPMMVQEi4wv3nbApyurvgNsHh6ywDu9WAi4G+Aat0sAliuXMNPgLLjVf5tYDxxDToCtlMQICAQoNo0G2krCzABs4u3FOB4dsVhP/N6AcXO0EE/FgHfBV5wuoevcrdCfQAAAABJRU5ErkJggg=="
+	iconStr = "iVBORw0KGgoAAAANSUhEUgAAAEAAAABACAMAAACdt4HsAAAAb1BMVEUAAAAkJG0kJFuAgIeDg4MmJmIjI2MnIV4mIWKCgoKAgIQmImCCgoYnI2EkIWKAgIOBgYWBgYOAgoSAgoQnI2EmImAmI2IlImEmIWEmImEmImEmImGAgYQmImGAgYWAgYQmImGAgYQmImGAgYT////3jpbIAAAAInRSTlMABw4gISIkLi8zNDU7QkZGSWdodoSHmp2g1djg4OPj8fT1aPEwQAAAAAFiS0dEJLQG+ZkAAACMSURBVFjD7dfLDoIwEEbho6ioeENQvCP6/u/opiSQNMYBF6b+Zz35Nm0mLY+eIUBAC9jnhgoPMMVQEi4wv3nbApyurvgNsHh6ywDu9WAi4G+Aat0sAliuXMNPgLLjVf5tYDxxDToCtlMQICAQoNo0G2krCzABs4u3FOB4dsVhP/N6AcXO0EE/FgHfBV5wuoevcrdCfQAAAABJRU5ErkJggg=="
 )
 
 type controllerTestSuite struct {
@@ -66,7 +66,7 @@ func (c *controllerTestSuite) TestGet() {
 			RepositoryName: "library/hello-world",
 		},
 	}, nil)
-	blob := ioutil.NopCloser(base64.NewDecoder(base64.StdEncoding, strings.NewReader(icon)))
+	blob := ioutil.NopCloser(base64.NewDecoder(base64.StdEncoding, strings.NewReader(iconStr)))
 	c.regCli.On("PullBlob", mock.Anything, mock.Anything).Return(0, blob, nil)
 	icon, err := c.controller.Get(nil, "sha256:364feec11702f7ee079ba81da723438373afb0921f3646e9e5015406ee150986")
 	c.Require().Nil(err)

src/lib/icon/const.go

@@ -0,0 +1,9 @@
+package icon
+
+// const definitions
+const (
+	DigestOfIconImage   = "sha256:0048162a053eef4d4ce3fe7518615bef084403614f8bca43b40ae2e762e11e06"
+	DigestOfIconChart   = "sha256:61cf3a178ff0f75bf08a25d96b75cf7355dc197749a9f128ed3ef34b0df05518"
+	DigestOfIconCNAB    = "sha256:089bdda265c14d8686111402c8ad629e8177a1ceb7dcd0f7f39b6480f623b3bd"
+	DigestOfIconDefault = "sha256:da834479c923584f4cbcdecc0dac61f32bef1d51e8aae598cf16bd154efab49f"
+)