Skip to content

Commit

Permalink
update referrer manifest descriptor size
Browse files Browse the repository at this point in the history 
cache manifest when first time pull if cacheEnabled

Signed-off-by: yminer <[email protected]>
  • Loading branch information
@MinerYang
MinerYang committed Apr 9, 2024
1 parent ff1a505 commit ee09c60
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 ee09c60

Please sign in to comment.