Skip to content

Commit 5142735

Browse files
Wang YanAloui-Ikram
Wang Yan
authored and
Aloui-Ikram
committed
disable land tag in the backend storage (goharbor#22507)
According to the gc performance proposal, harbor will skip to write the tag file into the data storage since we already use the harbor database to manage the tag CRUD. Proposal: goharbor/community#265 fixed goharbor#22405 Signed-off-by: wang yan <yan-yw.wang@broadcom.com> Co-authored-by: wang yan <yan-yw.wang@broadcom.com> Signed-off-by: Aloui-Ikram <ikram@container-registry.com>
1 parent de841fc commit 5142735

2 files changed

Lines changed: 164 additions & 27 deletions

File tree

‎src/server/registry/manifest.go‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,9 @@
1515
package registry
1616

1717
import (
18+
"bytes"
1819
"fmt"
20+
"io"
1921
"net/http"
2022
"strings"
2123

@@ -182,7 +184,26 @@ func putManifest(w http.ResponseWriter, req *http.Request) {
182184
}
183185

184186
buffer := lib.NewResponseBuffer(w)
187+
185188
// proxy the req to the backend docker registry
189+
// If reference is a tag (not a digest), replace it with the computed digest
190+
// before proxying to the backend. This prevents tags from being stored in the
191+
// backend registry storage, while Harbor maintains the tag-to-digest mapping in the database.
192+
if _, err := digest.Parse(reference); err != nil {
193+
// reference is a tag, not a digest
194+
data, err := io.ReadAll(req.Body)
195+
if err != nil {
196+
lib_http.SendError(w, err)
197+
return
198+
}
199+
200+
dgst := digest.FromBytes(data)
201+
req = req.Clone(req.Context())
202+
req.URL.Path = strings.TrimSuffix(req.URL.Path, reference) + dgst.String()
203+
req.URL.RawPath = req.URL.EscapedPath()
204+
req.Body = io.NopCloser(bytes.NewReader(data))
205+
req.ContentLength = int64(len(data))
206+
}
186207
proxy.ServeHTTP(buffer, req)
187208
if !buffer.Success() {
188209
if _, err := buffer.Flush(); err != nil {
@@ -198,15 +219,14 @@ func putManifest(w http.ResponseWriter, req *http.Request) {
198219
var tags []string
199220
dgt := reference
200221
// the reference is tag, get the digest from the response header
201-
if _, err = digest.Parse(reference); err != nil {
222+
if _, err := digest.Parse(reference); err != nil {
202223
dgt = buffer.Header().Get("Docker-Content-Digest")
203224
tags = append(tags, reference)
204225
}
205226

206-
_, _, err = artifact.Ctl.Ensure(req.Context(), repo, dgt, &artifact.ArtOption{
227+
if _, _, err := artifact.Ctl.Ensure(req.Context(), repo, dgt, &artifact.ArtOption{
207228
Tags: tags,
208-
})
209-
if err != nil {
229+
}); err != nil {
210230
lib_http.SendError(w, err)
211231
return
212232
}

‎src/server/registry/manifest_test.go‎

Lines changed: 140 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,16 @@
1515
package registry
1616

1717
import (
18+
"bytes"
1819
"context"
20+
"io"
1921
"net/http"
2022
"net/http/httptest"
23+
"strings"
2124
"testing"
2225

2326
beegocontext "github.com/beego/beego/v2/server/web/context"
27+
"github.com/opencontainers/go-digest"
2428
"github.com/stretchr/testify/suite"
2529

2630
"github.com/goharbor/harbor/src/controller/artifact"
@@ -71,18 +75,14 @@ func (m *manifestTestSuite) TearDownSuite() {
7175
}
7276

7377
func (m *manifestTestSuite) TestGetManifest() {
74-
// doesn't exist
7578
req := httptest.NewRequest(http.MethodGet, "/v2/library/hello-world/manifests/latest", nil)
7679
w := &httptest.ResponseRecorder{}
77-
7880
mock.OnAnything(m.artCtl, "GetByReference").Return(nil, errors.New(nil).WithCode(errors.NotFoundCode))
7981
getManifest(w, req)
8082
m.Equal(http.StatusNotFound, w.Code)
8183

82-
// reset the mock
8384
m.SetupTest()
8485

85-
// exist
8686
proxy = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
8787
if req.Method == http.MethodGet && req.URL.Path == "/v2/library/hello-world/manifests/sha256:418fb88ec412e340cdbef913b8ca1bbe8f9e8dc705f9617414c1f2c8db980180" {
8888
w.WriteHeader(http.StatusOK)
@@ -91,7 +91,6 @@ func (m *manifestTestSuite) TestGetManifest() {
9191
w.WriteHeader(http.StatusNotFound)
9292
})
9393

94-
// as we cannot set the beego input in the context, here the request doesn't carry reference part
9594
req = httptest.NewRequest(http.MethodGet, "/v2/library/hello-world/manifests/", nil)
9695
w = &httptest.ResponseRecorder{}
9796

@@ -101,14 +100,12 @@ func (m *manifestTestSuite) TestGetManifest() {
101100
getManifest(w, req)
102101
m.Equal(http.StatusOK, w.Code)
103102

104-
// if etag match, return 304
105103
req = httptest.NewRequest(http.MethodGet, "/v2/library/hello-world/manifests/", nil)
106104
w = &httptest.ResponseRecorder{}
107105
req.Header.Set("If-None-Match", "sha256:418fb88ec412e340cdbef913b8ca1bbe8f9e8dc705f9617414c1f2c8db980180")
108106
getManifest(w, req)
109107
m.Equal(http.StatusNotModified, w.Code)
110108

111-
// should get from cache if enable cache.
112109
config.DefaultMgr().Set(req.Context(), "cache_enabled", true)
113110
defer config.DefaultMgr().Set(req.Context(), "cache_enabled", false)
114111
req = httptest.NewRequest(http.MethodGet, "/v2/library/hello-world/manifests/", nil)
@@ -121,21 +118,14 @@ func (m *manifestTestSuite) TestGetManifest() {
121118
}
122119

123120
func (m *manifestTestSuite) TestDeleteManifest() {
124-
// doesn't exist
125121
req := httptest.NewRequest(http.MethodDelete, "/v2/library/hello-world/manifests/latest", nil)
126122
w := &httptest.ResponseRecorder{}
127-
128123
mock.OnAnything(m.artCtl, "GetByReference").Return(nil, errors.New(nil).WithCode(errors.NotFoundCode))
129124
deleteManifest(w, req)
130125
m.Equal(http.StatusBadRequest, w.Code)
131126

132-
// reset the mock
133-
m.SetupTest()
134-
135-
// reset the mock
136127
m.SetupTest()
137128

138-
// exist
139129
proxy = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
140130
if req.Method == http.MethodPut && req.URL.Path == "/v2/library/hello-world/manifests/latest" {
141131
w.WriteHeader(http.StatusInternalServerError)
@@ -154,10 +144,8 @@ func (m *manifestTestSuite) TestDeleteManifest() {
154144
deleteManifest(w, req)
155145
m.Equal(http.StatusAccepted, w.Code)
156146

157-
// should get from cache if enable cache.
158147
config.DefaultMgr().Set(req.Context(), "cache_enabled", true)
159148
defer config.DefaultMgr().Set(req.Context(), "cache_enabled", false)
160-
// should delete cache when manifest be deleted.
161149
req = httptest.NewRequest(http.MethodDelete, "/v2/library/hello-world/manifests/sha256:418fb88ec412e340cdbef913b8ca1bbe8f9e8dc705f9617414c1f2c8db980180", nil)
162150
input = &beegocontext.BeegoInput{}
163151
input.SetParam(":reference", "sha256:418fb88ec412e340cdbef913b8ca1bbe8f9e8dc705f9617414c1f2c8db980180")
@@ -170,40 +158,169 @@ func (m *manifestTestSuite) TestDeleteManifest() {
170158
}
171159

172160
func (m *manifestTestSuite) TestPutManifest() {
161+
manifestContent := []byte(`{"schemaVersion":2}`)
162+
expectedDigest := digest.FromBytes(manifestContent).String()
163+
173164
// the backend registry response with 500
174165
proxy = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
175-
if req.Method == http.MethodPut && req.URL.Path == "/v2/library/hello-world/manifests/latest" {
166+
if req.Method == http.MethodPut && strings.Contains(req.URL.Path, "/v2/library/hello-world/manifests/") {
176167
w.WriteHeader(http.StatusInternalServerError)
177168
return
178169
}
179170
w.WriteHeader(http.StatusNotFound)
180171
})
181-
req := httptest.NewRequest(http.MethodPut, "/v2/library/hello-world/manifests/latest", nil)
172+
req := httptest.NewRequest(http.MethodPut, "/v2/library/hello-world/manifests/latest", bytes.NewReader(manifestContent))
173+
input := &beegocontext.BeegoInput{}
174+
input.SetParam(":splat", "library/hello-world")
175+
input.SetParam(":reference", "latest")
176+
*req = *(req.WithContext(context.WithValue(req.Context(), router.ContextKeyInput{}, input)))
182177
w := &httptest.ResponseRecorder{}
183178
mock.OnAnything(m.repoCtl, "Ensure").Return(false, int64(1), nil)
184179
putManifest(w, req)
185180
m.Equal(http.StatusInternalServerError, w.Code)
186181

187-
// reset the mock
188182
m.SetupTest()
189183

190-
// // the backend registry serves the request successfully
184+
// the backend registry serves the request successfully
191185
proxy = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
192-
if req.Method == http.MethodPut && req.URL.Path == "/v2/library/hello-world/manifests/latest" {
193-
w.Header().Set("Docker-Content-Digest", "sha256:418fb88ec412e340cdbef913b8ca1bbe8f9e8dc705f9617414c1f2c8db980180")
186+
// After our changes, the URL path will contain the digest, not the tag
187+
if req.Method == http.MethodPut && strings.Contains(req.URL.Path, "/v2/library/hello-world/manifests/") {
188+
w.Header().Set("Docker-Content-Digest", expectedDigest)
194189
w.WriteHeader(http.StatusCreated)
195190
return
196191
}
197192
w.WriteHeader(http.StatusNotFound)
198193
})
199-
req = httptest.NewRequest(http.MethodPut, "/v2/library/hello-world/manifests/latest", nil)
194+
req = httptest.NewRequest(http.MethodPut, "/v2/library/hello-world/manifests/latest", bytes.NewReader(manifestContent))
195+
input = &beegocontext.BeegoInput{}
196+
input.SetParam(":splat", "library/hello-world")
197+
input.SetParam(":reference", "latest")
198+
*req = *(req.WithContext(context.WithValue(req.Context(), router.ContextKeyInput{}, input)))
200199
w = &httptest.ResponseRecorder{}
201200
mock.OnAnything(m.repoCtl, "Ensure").Return(false, int64(1), nil)
202201
mock.OnAnything(m.artCtl, "Ensure").Return(true, int64(1), nil)
203202
putManifest(w, req)
204203
m.Equal(http.StatusCreated, w.Code)
205204
}
206205

206+
func (m *manifestTestSuite) TestPutManifestWithTagToDigestReplacement() {
207+
manifestContent := []byte(`{
208+
"schemaVersion": 2,
209+
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
210+
"config": {
211+
"mediaType": "application/vnd.docker.container.image.v1+json",
212+
"size": 1234,
213+
"digest": "sha256:abcd1234"
214+
}
215+
}`)
216+
expectedDigest := digest.FromBytes(manifestContent).String()
217+
218+
var proxyRequest *http.Request
219+
proxy = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
220+
proxyRequest = req
221+
if !strings.Contains(req.URL.Path, expectedDigest) {
222+
m.T().Errorf("Expected URL path to contain digest %s, got %s", expectedDigest, req.URL.Path)
223+
}
224+
body, err := io.ReadAll(req.Body)
225+
if err != nil {
226+
m.T().Errorf("Failed to read body: %v", err)
227+
}
228+
if len(body) == 0 {
229+
m.T().Error("Request body is empty - body restoration failed")
230+
}
231+
if !bytes.Equal(body, manifestContent) {
232+
m.T().Errorf("Body content mismatch. Expected %s, got %s", string(manifestContent), string(body))
233+
}
234+
if req.ContentLength != int64(len(manifestContent)) {
235+
m.T().Errorf("ContentLength mismatch. Expected %d, got %d", len(manifestContent), req.ContentLength)
236+
}
237+
w.Header().Set("Docker-Content-Digest", expectedDigest)
238+
w.WriteHeader(http.StatusCreated)
239+
})
240+
241+
req := httptest.NewRequest(http.MethodPut, "/v2/library/hello-world/manifests/latest", bytes.NewReader(manifestContent))
242+
req.Header.Set("Content-Type", "application/vnd.docker.distribution.manifest.v2+json")
243+
input := &beegocontext.BeegoInput{}
244+
input.SetParam(":splat", "library/hello-world")
245+
input.SetParam(":reference", "latest")
246+
*req = *(req.WithContext(context.WithValue(req.Context(), router.ContextKeyInput{}, input)))
247+
248+
w := &httptest.ResponseRecorder{}
249+
mock.OnAnything(m.repoCtl, "Ensure").Return(false, int64(1), nil)
250+
mock.OnAnything(m.artCtl, "Ensure").Return(true, int64(1), nil)
251+
putManifest(w, req)
252+
m.Equal(http.StatusCreated, w.Code)
253+
m.NotNil(proxyRequest, "Request was not captured by proxy")
254+
m.Contains(proxyRequest.URL.Path, expectedDigest, "URL should contain digest")
255+
}
256+
257+
func (m *manifestTestSuite) TestPutManifestWithDigest() {
258+
manifestContent := []byte(`{
259+
"schemaVersion": 2,
260+
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
261+
"config": {
262+
"mediaType": "application/vnd.docker.container.image.v1+json",
263+
"size": 1234,
264+
"digest": "sha256:abcd1234"
265+
}
266+
}`)
267+
providedDigest := "sha256:418fb88ec412e340cdbef913b8ca1bbe8f9e8dc705f9617414c1f2c8db980180"
268+
269+
var proxyRequest *http.Request
270+
proxy = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
271+
proxyRequest = req
272+
if !strings.Contains(req.URL.Path, providedDigest) {
273+
m.T().Errorf("Expected URL path to contain original digest %s, got %s", providedDigest, req.URL.Path)
274+
}
275+
body, err := io.ReadAll(req.Body)
276+
if err != nil {
277+
m.T().Errorf("Failed to read body: %v", err)
278+
}
279+
if len(body) == 0 {
280+
m.T().Error("Request body is empty")
281+
}
282+
w.Header().Set("Docker-Content-Digest", providedDigest)
283+
w.WriteHeader(http.StatusCreated)
284+
})
285+
286+
req := httptest.NewRequest(http.MethodPut, "/v2/library/hello-world/manifests/"+providedDigest, bytes.NewReader(manifestContent))
287+
req.Header.Set("Content-Type", "application/vnd.docker.distribution.manifest.v2+json")
288+
input := &beegocontext.BeegoInput{}
289+
input.SetParam(":splat", "library/hello-world")
290+
input.SetParam(":reference", providedDigest)
291+
*req = *(req.WithContext(context.WithValue(req.Context(), router.ContextKeyInput{}, input)))
292+
293+
w := &httptest.ResponseRecorder{}
294+
mock.OnAnything(m.repoCtl, "Ensure").Return(false, int64(1), nil)
295+
mock.OnAnything(m.artCtl, "Ensure").Return(true, int64(1), nil)
296+
putManifest(w, req)
297+
m.Equal(http.StatusCreated, w.Code)
298+
m.NotNil(proxyRequest, "Request was not captured by proxy")
299+
m.Contains(proxyRequest.URL.Path, providedDigest, "URL should contain original digest")
300+
}
301+
302+
func (m *manifestTestSuite) TestPutManifestEmptyBody() {
303+
proxy = http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
304+
body, _ := io.ReadAll(req.Body)
305+
dgst := digest.FromBytes(body).String()
306+
w.Header().Set("Docker-Content-Digest", dgst)
307+
w.WriteHeader(http.StatusCreated)
308+
})
309+
310+
req := httptest.NewRequest(http.MethodPut, "/v2/library/empty/manifests/latest", bytes.NewReader([]byte{}))
311+
input := &beegocontext.BeegoInput{}
312+
input.SetParam(":splat", "library/empty")
313+
input.SetParam(":reference", "latest")
314+
*req = *(req.WithContext(context.WithValue(req.Context(), router.ContextKeyInput{}, input)))
315+
316+
w := &httptest.ResponseRecorder{}
317+
mock.OnAnything(m.repoCtl, "Ensure").Return(false, int64(1), nil)
318+
mock.OnAnything(m.artCtl, "Ensure").Return(true, int64(1), nil)
319+
320+
putManifest(w, req)
321+
m.Equal(http.StatusCreated, w.Code)
322+
}
323+
207324
func TestManifestTestSuite(t *testing.T) {
208325
suite.Run(t, &manifestTestSuite{})
209326
}

0 commit comments

Comments
 (0)