Skip to content

Commit

Permalink
refactor(storage): refactor storage into a single ImageStore
Browse files Browse the repository at this point in the history 
refactor(gc): drop umoci dependency, implemented internal gc

Signed-off-by: Petu Eusebiu <[email protected]>
  • Loading branch information
@eusebiu-constantin-petu-dbk
eusebiu-constantin-petu-dbk committed Jul 28, 2023
1 parent 635d718 commit 69ddb95
Show file tree
Hide file tree
Showing 26 changed files with 3,601 additions and 4,109 deletions.
3 changes: 3 additions & 0 deletions errors/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,4 +109,7 @@ var (
ErrInvalidCertificateContent = errors.New("signatures: invalid certificate content")
ErrInvalidStateCookie = errors.New("auth: state cookie not present or differs from original state")
ErrSyncNoURLsLeft = errors.New("sync: no valid registry urls left after filtering local ones")
ErrFileAlreadyCancelled = errors.New("already cancelled")
ErrFileAlreadyClosed = errors.New("already closed")
ErrFileAlreadyCommitted = errors.New("already committed")
)
6 changes: 2 additions & 4 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.20
require (
github.com/99designs/gqlgen v0.17.35
github.com/Masterminds/semver v1.5.0
github.com/apex/log v1.9.0
github.com/apex/log v1.9.0 // indirect
github.com/aquasecurity/trivy-db v0.0.0-20230703082116-dc52e83376ce
github.com/bmatcuk/doublestar/v4 v4.6.0
github.com/briandowns/spinner v1.23.0
Expand All @@ -23,7 +23,6 @@ require (
github.com/gorilla/mux v1.8.0
github.com/hashicorp/golang-lru/v2 v2.0.3
github.com/json-iterator/go v1.1.12
github.com/minio/sha256-simd v1.0.1
github.com/mitchellh/mapstructure v1.5.0
github.com/nmcclain/ldap v0.0.0-20210720162743-7f8d1e44eeba
github.com/olekukonko/tablewriter v0.0.5
Expand Down Expand Up @@ -367,7 +366,6 @@ require (
github.com/jtolds/gls v4.20.0+incompatible // indirect
github.com/kevinburke/ssh_config v1.2.0 // indirect
github.com/klauspost/compress v1.16.5 // indirect
github.com/klauspost/cpuid/v2 v2.2.3 // indirect
github.com/klauspost/pgzip v1.2.6-0.20220930104621-17e8dac29df8 // indirect
github.com/knqyf263/go-apk-version v0.0.0-20200609155635-041fdbb8563f // indirect
github.com/knqyf263/go-deb-version v0.0.0-20230223133812-3ed183d23422 // indirect
Expand Down Expand Up @@ -422,7 +420,7 @@ require (
github.com/sigstore/fulcio v1.3.1 // indirect
github.com/sigstore/rekor v1.2.2-0.20230530122220-67cc9e58bd23 // indirect
github.com/sigstore/sigstore v1.7.1
github.com/sirupsen/logrus v1.9.3 // indirect
github.com/sirupsen/logrus v1.9.3
github.com/skratchdot/open-golang v0.0.0-20200116055534-eef842397966 // indirect
github.com/spf13/afero v1.9.5 // indirect
github.com/spf13/cast v1.5.1 // indirect
Expand Down
5 changes: 0 additions & 5 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1128,8 +1128,6 @@ github.com/klauspost/compress v1.13.6/go.mod h1:/3/Vjq9QcHkK5uEr5lBEmyoZ1iFhe47e
github.com/klauspost/compress v1.15.11/go.mod h1:QPwzmACJjUTFsnSHH934V6woptycfrDDJnH7hvFVbGM=
github.com/klauspost/compress v1.16.5 h1:IFV2oUNUzZaz+XyusxpLzpzS8Pt5rh0Z16For/djlyI=
github.com/klauspost/compress v1.16.5/go.mod h1:ntbaceVETuRiXiv4DpjP66DpAtAGkEQskQzEyD//IeE=
github.com/klauspost/cpuid/v2 v2.2.3 h1:sxCkb+qR91z4vsqw4vGGZlDgPz3G7gjaLyK3V8y70BU=
github.com/klauspost/cpuid/v2 v2.2.3/go.mod h1:RVVoqg1df56z8g3pUjL/3lE5UfnlrJX8tyFgg4nqhuY=
github.com/klauspost/pgzip v1.2.5/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQKQE9RUs=
github.com/klauspost/pgzip v1.2.6-0.20220930104621-17e8dac29df8 h1:BcxbplxjtczA1a6d3wYoa7a0WL3rq9DKBMGHeKyjEF0=
github.com/klauspost/pgzip v1.2.6-0.20220930104621-17e8dac29df8/go.mod h1:Ch1tH69qFZu15pkjo5kYi6mth2Zzwzt50oCQKQE9RUs=
Expand Down Expand Up @@ -1256,8 +1254,6 @@ github.com/miekg/pkcs11 v1.1.1 h1:Ugu9pdy6vAYku5DEpVWVFPYnzV+bxB+iRdbuFSu7TvU=
github.com/miekg/pkcs11 v1.1.1/go.mod h1:XsNlhZGX73bx86s2hdc/FuaLm2CPZJemRLMA+WTFxgs=
github.com/migueleliasweb/go-github-mock v0.0.19 h1:z/88f6wPqZVFnE7s9DbwXMhCtmV/0FofNxc4M7FuSdU=
github.com/migueleliasweb/go-github-mock v0.0.19/go.mod h1:dBoCB3W9NjzyABhoGkfI0iSlFpzulAXhI7M+9A4ONYI=
github.com/minio/sha256-simd v1.0.1 h1:6kaan5IFmwTNynnKKpDHe6FWHohJOHhCPchzK49dzMM=
github.com/minio/sha256-simd v1.0.1/go.mod h1:Pz6AKMiUdngCLpeTL/RJY1M9rUuPMYujV5xJjtbRSN8=
github.com/mitchellh/cli v1.0.0/go.mod h1:hNIlj7HEI86fIcpObd7a0FcrxTWetlwJDGcceTlRvqc=
github.com/mitchellh/cli v1.1.5/go.mod h1:v8+iFts2sPIKUV1ltktPXMCC8fumSKFItNcD2cLtRR4=
github.com/mitchellh/copystructure v1.0.0/go.mod h1:SNtv71yrdKgLRyLFxmLdkAbkKEFWgYaq1OVrnRcwhnw=
Expand Down Expand Up @@ -2062,7 +2058,6 @@ golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20220610221304-9f5ed59c137d/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220615213510-4f61da869c0c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220624220833-87e55d714810/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220704084225-05e143d24a9e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220728004956-3c1f35247d10/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
Expand Down
241 changes: 3 additions & 238 deletions pkg/api/controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -984,7 +984,7 @@ func TestInterruptedBlobUpload(t *testing.T) {
defer cm.StopServer()

client := resty.New()
blob := make([]byte, 50*1024*1024)
blob := make([]byte, 200*1024*1024)
digest := godigest.FromBytes(blob).String()

//nolint: dupl
Expand Down Expand Up @@ -1037,6 +1037,7 @@ func TestInterruptedBlobUpload(t *testing.T) {
So(resp.StatusCode(), ShouldEqual, http.StatusNotFound)
})

//nolint: dupl
Convey("Test negative interrupt PATCH blob upload", func() {
resp, err := client.R().Post(baseURL + "/v2/" + AuthorizedNamespace + "/blobs/uploads/")
So(err, ShouldBeNil)
Expand Down Expand Up @@ -1139,6 +1140,7 @@ func TestInterruptedBlobUpload(t *testing.T) {
So(resp.StatusCode(), ShouldEqual, http.StatusNotFound)
})

//nolint: dupl
Convey("Test negative interrupt PUT blob upload", func() {
resp, err := client.R().Post(baseURL + "/v2/" + AuthorizedNamespace + "/blobs/uploads/")
So(err, ShouldBeNil)
Expand Down Expand Up @@ -7043,243 +7045,6 @@ func TestInjectInterruptedImageManifest(t *testing.T) {
})
}

func TestInjectTooManyOpenFiles(t *testing.T) {
Convey("Make a new controller", t, func() {
port := test.GetFreePort()
baseURL := test.GetBaseURL(port)
conf := config.New()
conf.HTTP.Port = port

dir := t.TempDir()
ctlr := makeController(conf, dir, "")
conf.Storage.RemoteCache = false

cm := test.NewControllerManager(ctlr)
cm.StartAndWait(port)
defer cm.StopServer()

rthdlr := api.NewRouteHandler(ctlr)

// create a blob/layer
resp, err := resty.R().Post(baseURL + "/v2/repotest/blobs/uploads/")
So(err, ShouldBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusAccepted)
loc := test.Location(baseURL, resp)
So(loc, ShouldNotBeEmpty)

// since we are not specifying any prefix i.e provided in config while starting server,
// so it should store repotest to global root dir
_, err = os.Stat(path.Join(dir, "repotest"))
So(err, ShouldBeNil)

resp, err = resty.R().Get(loc)
So(err, ShouldBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusNoContent)
content := []byte("this is a dummy blob")
digest := godigest.FromBytes(content)
So(digest, ShouldNotBeNil)

// monolithic blob upload
injected := inject.InjectFailure(0)
if injected {
request, _ := http.NewRequestWithContext(context.TODO(), http.MethodPut, loc, bytes.NewReader(content))
tokens := strings.Split(loc, "/")
request = mux.SetURLVars(request, map[string]string{"name": "repotest", "session_id": tokens[len(tokens)-1]})
q := request.URL.Query()
q.Add("digest", digest.String())
request.URL.RawQuery = q.Encode()
request.Header.Set("Content-Type", "application/octet-stream")
request.Header.Set("Content-Length", fmt.Sprintf("%d", len(content)))
response := httptest.NewRecorder()

rthdlr.UpdateBlobUpload(response, request)

resp := response.Result()
defer resp.Body.Close()

So(resp, ShouldNotBeNil)
So(resp.StatusCode, ShouldEqual, http.StatusInternalServerError)
} else {
resp, err = resty.R().SetQueryParam("digest", digest.String()).
SetHeader("Content-Type", "application/octet-stream").
SetBody(content).Put(loc)
So(err, ShouldBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusCreated)

blobLoc := resp.Header().Get("Location")
So(blobLoc, ShouldNotBeEmpty)
So(resp.Header().Get("Content-Length"), ShouldEqual, "0")
So(resp.Header().Get(constants.DistContentDigestKey), ShouldNotBeEmpty)
}

// upload image config blob
resp, err = resty.R().Post(baseURL + "/v2/repotest/blobs/uploads/")
So(err, ShouldBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusAccepted)
loc = test.Location(baseURL, resp)
cblob, cdigest := test.GetRandomImageConfig()

resp, err = resty.R().
SetContentLength(true).
SetHeader("Content-Length", fmt.Sprintf("%d", len(cblob))).
SetHeader("Content-Type", "application/octet-stream").
SetQueryParam("digest", cdigest.String()).
SetBody(cblob).
Put(loc)
So(err, ShouldBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusCreated)

// create a manifest
manifest := ispec.Manifest{
Config: ispec.Descriptor{
MediaType: "application/vnd.oci.image.config.v1+json",
Digest: cdigest,
Size: int64(len(cblob)),
},
Layers: []ispec.Descriptor{
{
MediaType: "application/vnd.oci.image.layer.v1.tar",
Digest: digest,
Size: int64(len(content)),
},
},
}
manifest.SchemaVersion = 2
content, err = json.Marshal(manifest)
So(err, ShouldBeNil)
digest = godigest.FromBytes(content)
So(digest, ShouldNotBeNil)

// Testing router path: @Router /v2/{name}/manifests/{reference} [put]
//nolint:lll // gofumpt conflicts with lll
Convey("Uploading an image manifest blob (when injected simulates that PutImageManifest failed due to 'too many open files' error)", func() {
injected := inject.InjectFailure(1)

request, _ := http.NewRequestWithContext(context.TODO(), http.MethodPut, baseURL, bytes.NewReader(content))
request = mux.SetURLVars(request, map[string]string{"name": "repotest", "reference": "1.0"})
request.Header.Set("Content-Type", "application/vnd.oci.image.manifest.v1+json")
response := httptest.NewRecorder()

rthdlr.UpdateManifest(response, request)

resp := response.Result()
So(resp, ShouldNotBeNil)
defer resp.Body.Close()

if injected {
So(resp.StatusCode, ShouldEqual, http.StatusInternalServerError)
} else {
So(resp.StatusCode, ShouldEqual, http.StatusCreated)
}
})
Convey("when injected simulates a `too many open files` error inside PutImageManifest method of img store", func() {
injected := inject.InjectFailure(2)

request, _ := http.NewRequestWithContext(context.TODO(), http.MethodPut, baseURL, bytes.NewReader(content))
request = mux.SetURLVars(request, map[string]string{"name": "repotest", "reference": "1.0"})
request.Header.Set("Content-Type", "application/vnd.oci.image.manifest.v1+json")
response := httptest.NewRecorder()

rthdlr.UpdateManifest(response, request)

resp := response.Result()
defer resp.Body.Close()

So(resp, ShouldNotBeNil)

if injected {
So(resp.StatusCode, ShouldEqual, http.StatusInternalServerError)
} else {
So(resp.StatusCode, ShouldEqual, http.StatusCreated)
}
})
Convey("code coverage: error inside PutImageManifest method of img store (unable to marshal JSON)", func() {
injected := inject.InjectFailure(1)

request, _ := http.NewRequestWithContext(context.TODO(), http.MethodPut, baseURL, bytes.NewReader(content))
request = mux.SetURLVars(request, map[string]string{"name": "repotest", "reference": "1.0"})
request.Header.Set("Content-Type", "application/vnd.oci.image.manifest.v1+json")
response := httptest.NewRecorder()

rthdlr.UpdateManifest(response, request)

resp := response.Result()
defer resp.Body.Close()

So(resp, ShouldNotBeNil)

if injected {
So(resp.StatusCode, ShouldEqual, http.StatusInternalServerError)
} else {
So(resp.StatusCode, ShouldEqual, http.StatusCreated)
}
})
Convey("code coverage: error inside PutImageManifest method of img store (umoci.OpenLayout error)", func() {
injected := inject.InjectFailure(3)

request, _ := http.NewRequestWithContext(context.TODO(), http.MethodPut, baseURL, bytes.NewReader(content))
request = mux.SetURLVars(request, map[string]string{"name": "repotest", "reference": "1.0"})
request.Header.Set("Content-Type", "application/vnd.oci.image.manifest.v1+json")
response := httptest.NewRecorder()

rthdlr.UpdateManifest(response, request)

resp := response.Result()
defer resp.Body.Close()

So(resp, ShouldNotBeNil)

if injected {
So(resp.StatusCode, ShouldEqual, http.StatusInternalServerError)
} else {
So(resp.StatusCode, ShouldEqual, http.StatusCreated)
}
})
Convey("code coverage: error inside PutImageManifest method of img store (oci.GC)", func() {
injected := inject.InjectFailure(4)

request, _ := http.NewRequestWithContext(context.TODO(), http.MethodPut, baseURL, bytes.NewReader(content))
request = mux.SetURLVars(request, map[string]string{"name": "repotest", "reference": "1.0"})
request.Header.Set("Content-Type", "application/vnd.oci.image.manifest.v1+json")
response := httptest.NewRecorder()

rthdlr.UpdateManifest(response, request)

resp := response.Result()
defer resp.Body.Close()

So(resp, ShouldNotBeNil)

if injected {
So(resp.StatusCode, ShouldEqual, http.StatusInternalServerError)
} else {
So(resp.StatusCode, ShouldEqual, http.StatusCreated)
}
})
Convey("when index.json is not in json format", func() {
resp, err = resty.R().SetHeader("Content-Type", "application/vnd.oci.image.manifest.v1+json").
SetBody(content).Put(baseURL + "/v2/repotest/manifests/v1.0")
So(err, ShouldBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusCreated)
digestHdr := resp.Header().Get(constants.DistContentDigestKey)
So(digestHdr, ShouldNotBeEmpty)
So(digestHdr, ShouldEqual, digest.String())

indexFile := path.Join(dir, "repotest", "index.json")
_, err = os.Stat(indexFile)
So(err, ShouldBeNil)
indexContent := []byte(`not a JSON content`)
err = os.WriteFile(indexFile, indexContent, 0o600)
So(err, ShouldBeNil)

resp, err = resty.R().SetHeader("Content-Type", "application/vnd.oci.image.manifest.v1+json").
SetBody(content).Put(baseURL + "/v2/repotest/manifests/v1.1")
So(err, ShouldBeNil)
So(resp.StatusCode(), ShouldEqual, http.StatusInternalServerError)
})
})
}

func TestGCSignaturesAndUntaggedManifests(t *testing.T) {
Convey("Make controller", t, func() {
repoName := "testrepo" //nolint:goconst
Expand Down
5 changes: 2 additions & 3 deletions pkg/cli/root.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ import (
extconf "zotregistry.io/zot/pkg/extensions/config"
"zotregistry.io/zot/pkg/extensions/monitoring"
storageConstants "zotregistry.io/zot/pkg/storage/constants"
"zotregistry.io/zot/pkg/storage/s3"
)

// metadataConfig reports metadata after parsing, which we use to track
Expand Down Expand Up @@ -626,7 +625,7 @@ func applyDefaultValues(config *config.Config, viperInstance *viper.Viper) {
// s3 dedup=false, check for previous dedup usage and set to true if cachedb found
if !config.Storage.Dedupe && config.Storage.StorageDriver != nil {
cacheDir, _ := config.Storage.StorageDriver["rootdirectory"].(string)
cachePath := path.Join(cacheDir, s3.CacheDBName+storageConstants.DBExtensionName)
cachePath := path.Join(cacheDir, storageConstants.BoltdbName+storageConstants.DBExtensionName)

if _, err := os.Stat(cachePath); err == nil {
log.Info().Msg("Config: dedupe set to false for s3 driver but used to be true.")
Expand All @@ -647,7 +646,7 @@ func applyDefaultValues(config *config.Config, viperInstance *viper.Viper) {
// s3 dedup=false, check for previous dedup usage and set to true if cachedb found
if !storageConfig.Dedupe && storageConfig.StorageDriver != nil {
subpathCacheDir, _ := storageConfig.StorageDriver["rootdirectory"].(string)
subpathCachePath := path.Join(subpathCacheDir, s3.CacheDBName+storageConstants.DBExtensionName)
subpathCachePath := path.Join(subpathCacheDir, storageConstants.BoltdbName+storageConstants.DBExtensionName)

if _, err := os.Stat(subpathCachePath); err == nil {
log.Info().Msg("Config: dedupe set to false for s3 driver but used to be true. ")
Expand Down
5 changes: 2 additions & 3 deletions pkg/cli/root_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
"zotregistry.io/zot/pkg/api/config"
"zotregistry.io/zot/pkg/cli"
storageConstants "zotregistry.io/zot/pkg/storage/constants"
"zotregistry.io/zot/pkg/storage/s3"
. "zotregistry.io/zot/pkg/test"
)

Expand Down Expand Up @@ -521,7 +520,7 @@ func TestVerify(t *testing.T) {

// s3 dedup=false, check for previous dedup usage and set to true if cachedb found
cacheDir := t.TempDir()
existingDBPath := path.Join(cacheDir, s3.CacheDBName+storageConstants.DBExtensionName)
existingDBPath := path.Join(cacheDir, storageConstants.BoltdbName+storageConstants.DBExtensionName)
_, err = os.Create(existingDBPath)
So(err, ShouldBeNil)

Expand All @@ -537,7 +536,7 @@ func TestVerify(t *testing.T) {

// subpath s3 dedup=false, check for previous dedup usage and set to true if cachedb found
cacheDir = t.TempDir()
existingDBPath = path.Join(cacheDir, s3.CacheDBName+storageConstants.DBExtensionName)
existingDBPath = path.Join(cacheDir, storageConstants.BoltdbName+storageConstants.DBExtensionName)
_, err = os.Create(existingDBPath)
So(err, ShouldBeNil)

Expand Down
Loading

0 comments on commit 69ddb95

Please sign in to comment.