Skip to content

Commit

Permalink
update referrer manifest descriptor size
Browse files Browse the repository at this point in the history 
Signed-off-by: yminer <[email protected]>

use manifest payload calculate size

get manifest from cache first

update UT with cache disabled

cache manifest when first pull

update comment

error handle for manifest payload

update log msg

update error.As
  • Loading branch information
@MinerYang
MinerYang committed Apr 9, 2024
1 parent 96ba34a commit c1f1096
Show file tree
Hide file tree
Showing 2 changed files with 177 additions and 11 deletions.
59 changes: 53 additions & 6 deletions src/server/registry/referrers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,16 @@ import (
"github.com/opencontainers/go-digest"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"

"github.com/goharbor/harbor/src/lib/cache"
"github.com/goharbor/harbor/src/lib/config"
"github.com/goharbor/harbor/src/lib/errors"
lib_http "github.com/goharbor/harbor/src/lib/http"
"github.com/goharbor/harbor/src/lib/log"
"github.com/goharbor/harbor/src/lib/q"
"github.com/goharbor/harbor/src/pkg/accessory"
"github.com/goharbor/harbor/src/pkg/artifact"
"github.com/goharbor/harbor/src/pkg/cached/manifest/redis"
"github.com/goharbor/harbor/src/pkg/registry"
"github.com/goharbor/harbor/src/server/router"
"github.com/goharbor/harbor/src/server/v2.0/handler"
)
Expand All @@ -38,12 +43,16 @@ func newReferrersHandler() http.Handler {
return &referrersHandler{
artifactManager: artifact.NewManager(),
accessoryManager: accessory.NewManager(),
registryClient: registry.Cli,
maniCacheManager: redis.NewManager(),

Check warning on line 47 in src/server/registry/referrers.go

View check run for this annotation

Codecov / codecov/patch

src/server/registry/referrers.go#L46-L47 

Added lines #L46 - L47 were not covered by tests
}
}

type referrersHandler struct {
artifactManager artifact.Manager
accessoryManager accessory.Manager
registryClient registry.Client
maniCacheManager redis.CachedManager
}

func (r *referrersHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
Expand Down Expand Up @@ -75,29 +84,67 @@ func (r *referrersHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
lib_http.SendError(w, err)
return
}

// Build index manifest from accessories
mfs := make([]ocispec.Descriptor, 0)
for _, acc := range accs {
accArt, err := r.artifactManager.GetByDigest(ctx, repository, acc.GetData().Digest)
accArtDigest := acc.GetData().Digest
accArt, err := r.artifactManager.GetByDigest(ctx, repository, accArtDigest)
if err != nil {
lib_http.SendError(w, err)
return
}
mf := ocispec.Descriptor{
// whether get manifest from cache
fromCache := false
// whether need write manifest to cache
writeCache := false
var maniContent []byte

// pull manifest, will try to pull from cache first
// and write to cache when pull manifest from registry at first time
if config.CacheEnabled() {
maniContent, err = r.maniCacheManager.Get(req.Context(), accArtDigest)
if err == nil {
fromCache = true

Check warning on line 107 in src/server/registry/referrers.go

View check run for this annotation

Codecov / codecov/patch

src/server/registry/referrers.go#L107 

Added line #L107 was not covered by tests
} else {
log.Debugf("failed to get manifest %s from cache, will fallback to registry, error: %v", accArtDigest, err)
if errors.As(err, &cache.ErrNotFound) {
writeCache = true
}
}
}
if !fromCache {
mani, _, err := r.registryClient.PullManifest(accArt.RepositoryName, accArtDigest)
if err != nil {
lib_http.SendError(w, err)
return
}

Check warning on line 120 in src/server/registry/referrers.go

View check run for this annotation

Codecov / codecov/patch

src/server/registry/referrers.go#L118-L120 

Added lines #L118 - L120 were not covered by tests
_, maniContent, err = mani.Payload()
if err != nil {
lib_http.SendError(w, err)
return
}

Check warning on line 125 in src/server/registry/referrers.go

View check run for this annotation

Codecov / codecov/patch

src/server/registry/referrers.go#L123-L125 

Added lines #L123 - L125 were not covered by tests
// write manifest to cache when first time pulling
if writeCache {
err = r.maniCacheManager.Save(req.Context(), accArtDigest, maniContent)
if err != nil {
log.Warningf("failed to save accArt manifest %s to cache, error: %v", accArtDigest, err)
}

Check warning on line 131 in src/server/registry/referrers.go

View check run for this annotation

Codecov / codecov/patch

src/server/registry/referrers.go#L130-L131 

Added lines #L130 - L131 were not covered by tests
}
}
desc := ocispec.Descriptor{
MediaType: accArt.ManifestMediaType,
Size: accArt.Size,
Size: int64(len(maniContent)),
Digest: digest.Digest(accArt.Digest),
Annotations: accArt.Annotations,
ArtifactType: accArt.ArtifactType,
}
// filter use accArt.ArtifactType as artifactType
if at != "" {
if accArt.ArtifactType == at {
mfs = append(mfs, mf)
mfs = append(mfs, desc)

Check warning on line 144 in src/server/registry/referrers.go

View check run for this annotation

Codecov / codecov/patch

src/server/registry/referrers.go#L144 

Added line #L144 was not covered by tests
}
} else {
mfs = append(mfs, mf)
mfs = append(mfs, desc)
}
}

Expand Down
129 changes: 124 additions & 5 deletions src/server/registry/referrers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,50 @@ package registry
import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"testing"

beegocontext "github.com/beego/beego/v2/server/web/context"
ocispec "github.com/opencontainers/image-spec/specs-go/v1"
v1 "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/stretchr/testify/assert"

"github.com/goharbor/harbor/src/lib/cache"
"github.com/goharbor/harbor/src/lib/config"
accessorymodel "github.com/goharbor/harbor/src/pkg/accessory/model"
basemodel "github.com/goharbor/harbor/src/pkg/accessory/model/base"
"github.com/goharbor/harbor/src/pkg/artifact"
"github.com/goharbor/harbor/src/pkg/distribution"
"github.com/goharbor/harbor/src/server/router"
"github.com/goharbor/harbor/src/testing/mock"
accessorytesting "github.com/goharbor/harbor/src/testing/pkg/accessory"
arttesting "github.com/goharbor/harbor/src/testing/pkg/artifact"
testmanifest "github.com/goharbor/harbor/src/testing/pkg/cached/manifest/redis"
regtesting "github.com/goharbor/harbor/src/testing/pkg/registry"
)

var (
OCIManifest = `{
"schemaVersion": 2,
"mediaType": "application/vnd.oci.image.manifest.v1+json",
"config": {
"mediaType": "application/vnd.example.sbom",
"digest": "sha256:5891b5b522d5df086d0ff0b110fbd9d21bb4fc7163af34d08286a2e846f6be03",
"size": 123
},
"layers": [
{
"mediaType": "application/vnd.example.data.v1.tar+gzip",
"digest": "sha256:e258d248fda94c63753607f7c4494ee0fcbe92f1a76bfdac795c9d84101eb317",
"size": 1234
}
],
"annotations": {
"name": "test-image"
}
}`
)

func TestReferrersHandlerOK(t *testing.T) {
Expand All @@ -35,10 +65,10 @@ func TestReferrersHandlerOK(t *testing.T) {

artifactMock.On("GetByDigest", mock.Anything, mock.Anything, mock.Anything).
Return(&artifact.Artifact{
Digest: digestVal,
Digest: "sha256:4911bb745e19a6b5513755f3d033f10ef10c34b40edc631809e28be8a7c005f6",
ManifestMediaType: "application/vnd.oci.image.manifest.v1+json",
MediaType: "application/vnd.example.sbom",
ArtifactType: "application/vnd.example+type",
ArtifactType: "application/vnd.example.sbom",
Size: 1000,
Annotations: map[string]string{
"name": "test-image",
Expand All @@ -56,13 +86,23 @@ func TestReferrersHandlerOK(t *testing.T) {
SubArtifactDigest: digestVal,
SubArtifactRepo: "goharbor",
Type: accessorymodel.TypeCosignSignature,
Digest: "sha256:4911bb745e19a6b5513755f3d033f10ef10c34b40edc631809e28be8a7c005f6",
},
},
}, nil)

manifest, _, err := distribution.UnmarshalManifest(v1.MediaTypeImageManifest, []byte(OCIManifest))
if err != nil {
t.Fatal(err)
}
regCliMock := &regtesting.Client{}
config.DefaultMgr().Set(context.TODO(), "cache_enabled", false)
mock.OnAnything(regCliMock, "PullManifest").Return(manifest, "", nil)

handler := &referrersHandler{
artifactManager: artifactMock,
accessoryManager: accessoryMock,
registryClient: regCliMock,
}

handler.ServeHTTP(rec, req)
Expand All @@ -72,10 +112,89 @@ func TestReferrersHandlerOK(t *testing.T) {
t.Errorf("Expected status code %d, but got %d", http.StatusOK, rec.Code)
}
index := &ocispec.Index{}
json.Unmarshal([]byte(rec.Body.String()), index)
if index.Manifests[0].ArtifactType != "application/vnd.example+type" {
t.Errorf("Expected response body %s, but got %s", "application/vnd.example+type", rec.Body.String())
json.Unmarshal(rec.Body.Bytes(), index)
if index.Manifests[0].ArtifactType != "application/vnd.example.sbom" {
t.Errorf("Expected response body %s, but got %s", "application/vnd.example.sbom", rec.Body.String())
}
_, content, _ := manifest.Payload()
assert.Equal(t, int64(len(content)), index.Manifests[0].Size)
}

func TestReferrersHandlerSavetoCache(t *testing.T) {
rec := httptest.NewRecorder()
digestVal := "sha256:6c3c624b58dbbcd3c0dd82b4c53f04194d1247c6eebdaab7c610cf7d66709b3b"
req, err := http.NewRequest("GET", "/v2/test/repository/referrers/sha256:6c3c624b58dbbcd3c0dd82b4c53f04194d1247c6eebdaab7c610cf7d66709b3b", nil)
if err != nil {
t.Fatal(err)
}
input := &beegocontext.BeegoInput{}
input.SetParam(":reference", digestVal)
*req = *(req.WithContext(context.WithValue(req.Context(), router.ContextKeyInput{}, input)))

artifactMock := &arttesting.Manager{}
accessoryMock := &accessorytesting.Manager{}

artifactMock.On("GetByDigest", mock.Anything, mock.Anything, mock.Anything).
Return(&artifact.Artifact{
Digest: "sha256:4911bb745e19a6b5513755f3d033f10ef10c34b40edc631809e28be8a7c005f6",
ManifestMediaType: "application/vnd.oci.image.manifest.v1+json",
MediaType: "application/vnd.example.sbom",
ArtifactType: "application/vnd.example.sbom",
Size: 1000,
Annotations: map[string]string{
"name": "test-image",
},
}, nil)

accessoryMock.On("Count", mock.Anything, mock.Anything).
Return(int64(1), nil)
accessoryMock.On("List", mock.Anything, mock.Anything).
Return([]accessorymodel.Accessory{
&basemodel.Default{
Data: accessorymodel.AccessoryData{
ID: 1,
ArtifactID: 2,
SubArtifactDigest: digestVal,
SubArtifactRepo: "goharbor",
Type: accessorymodel.TypeCosignSignature,
Digest: "sha256:4911bb745e19a6b5513755f3d033f10ef10c34b40edc631809e28be8a7c005f6",
},
},
}, nil)

manifest, _, err := distribution.UnmarshalManifest(v1.MediaTypeImageManifest, []byte(OCIManifest))
if err != nil {
t.Fatal(err)
}

// cache_enabled pull from cahce
config.DefaultMgr().Set(context.TODO(), "cache_enabled", true)
cacheManagerMock := &testmanifest.CachedManager{}
mock.OnAnything(cacheManagerMock, "Get").Return(nil, fmt.Errorf("unable to do stuff: %w", cache.ErrNotFound))
regCliMock := &regtesting.Client{}
mock.OnAnything(regCliMock, "PullManifest").Return(manifest, "", nil)
mock.OnAnything(cacheManagerMock, "Save").Return(nil)

handler := &referrersHandler{
artifactManager: artifactMock,
accessoryManager: accessoryMock,
registryClient: regCliMock,
maniCacheManager: cacheManagerMock,
}

handler.ServeHTTP(rec, req)

// check that the response has the expected status code (200 OK)
if rec.Code != http.StatusOK {
t.Errorf("Expected status code %d, but got %d", http.StatusOK, rec.Code)
}
index := &ocispec.Index{}
json.Unmarshal(rec.Body.Bytes(), index)
if index.Manifests[0].ArtifactType != "application/vnd.example.sbom" {
t.Errorf("Expected response body %s, but got %s", "application/vnd.example.sbom", rec.Body.String())
}
_, content, _ := manifest.Payload()
assert.Equal(t, int64(len(content)), index.Manifests[0].Size)
}

func TestReferrersHandlerEmpty(t *testing.T) {
Expand Down

0 comments on commit c1f1096

Please sign in to comment.