Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update referrer manifest descriptor size #20207

Merged
merged 2 commits into from
Apr 9, 2024
Merged

update referrer manifest descriptor size #20207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee09c60
update referrer manifest descriptor size
MinerYang Apr 2, 2024
4889c28
Merge branch 'main' into fix_refererr_descriptor_size
MinerYang Apr 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 53 additions & 6 deletions src/server/registry/referrers.go
View file
Open in desktop
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(),
}
}

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
MinerYang marked this conversation as resolved.
Show resolved Hide resolved
// 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
} 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
}
_, maniContent, err = mani.Payload()
if err != nil {
lib_http.SendError(w, err)
return
}
// 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)
}
}
}
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)
}
} 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
Open in desktop
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
Loading