From 59f9ef7e5cf19333c4eea45d0d2082a0a0b11c1d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wenkai=20Yin=28=E5=B0=B9=E6=96=87=E5=BC=80=29?= Date: Wed, 23 Sep 2020 10:42:47 +0800 Subject: [PATCH] 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 --- .../artifact/processor/base/manifest.go | 35 ++++++++++++------- .../artifact/processor/base/manifest_test.go | 10 ++++++ .../artifact/processor/chart/chart.go | 7 ---- .../artifact/processor/chart/chart_test.go | 10 ------ .../artifact/processor/image/index.go | 7 ---- .../artifact/processor/image/index_test.go | 7 ---- .../artifact/processor/image/manifest_v2.go | 34 +++++++++++------- .../processor/image/manifest_v2_test.go | 10 ++++-- 8 files changed, 62 insertions(+), 58 deletions(-) diff --git a/src/controller/artifact/processor/base/manifest.go b/src/controller/artifact/processor/base/manifest.go index a91299a0c..3c35feb3a 100644 --- a/src/controller/artifact/processor/base/manifest.go +++ b/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) +} diff --git a/src/controller/artifact/processor/base/manifest_test.go b/src/controller/artifact/processor/base/manifest_test.go index d29e23a3f..e16a17ee9 100644 --- a/src/controller/artifact/processor/base/manifest_test.go +++ b/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{}) } diff --git a/src/controller/artifact/processor/chart/chart.go b/src/controller/artifact/processor/chart/chart.go index 1cd8bd270..4a5206a62 100644 --- a/src/controller/artifact/processor/chart/chart.go +++ b/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). diff --git a/src/controller/artifact/processor/chart/chart_test.go b/src/controller/artifact/processor/chart/chart_test.go index 0d2dcd2fb..4543d5efc 100644 --- a/src/controller/artifact/processor/chart/chart_test.go +++ b/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") diff --git a/src/controller/artifact/processor/image/index.go b/src/controller/artifact/processor/image/index.go index c412bc74d..0d268a179 100644 --- a/src/controller/artifact/processor/image/index.go +++ b/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 } diff --git a/src/controller/artifact/processor/image/index_test.go b/src/controller/artifact/processor/image/index_test.go index ceb9c9a07..13f8937d5 100644 --- a/src/controller/artifact/processor/image/index_test.go +++ b/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)) } diff --git a/src/controller/artifact/processor/image/manifest_v2.go b/src/controller/artifact/processor/image/manifest_v2.go index 1576c8c76..b3a290192 100644 --- a/src/controller/artifact/processor/image/manifest_v2.go +++ b/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 { + config := &v1.Image{} + if err = m.ManifestProcessor.UnmarshalConfig(ctx, artifact.RepositoryName, content, config); 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 { - return nil, err - } - content, err = json.Marshal(image.History) + content, err = json.Marshal(config.History) if err != nil { return nil, err } diff --git a/src/controller/artifact/processor/image/manifest_v2_test.go b/src/controller/artifact/processor/image/manifest_v2_test.go index ffbf9b41f..e988cff13 100644 --- a/src/controller/artifact/processor/image/manifest_v2_test.go +++ b/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()) }