Skip to content

Commit 6c29371

Browse files
shimish2rchincha
shimish2
authored and
rchincha
committed
storage: different subpaths can point to same root directory
currently different subpaths can only point to same root directory only when one or both of the storage config does not enable dedupe different subpath should be able to point to same root directory and in that case their storage config should be same i.e GC,Dedupe, GC delay and GC interval Signed-off-by: Shivam Mishra <[email protected]>
1 parent 3bccea7 commit 6c29371

File tree

8 files changed

+441
-50
lines changed

8 files changed

+441
-50
lines changed

‎pkg/api/config/config.go

+22
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package config
22

33
import (
44
"fmt"
5+
"os"
56
"time"
67

78
"github.com/getlantern/deepcopy"
@@ -144,6 +145,27 @@ func New() *Config {
144145
}
145146
}
146147

148+
func (expConfig StorageConfig) ParamsEqual(actConfig StorageConfig) bool {
149+
return expConfig.GC == actConfig.GC && expConfig.Dedupe == actConfig.Dedupe &&
150+
expConfig.GCDelay == actConfig.GCDelay && expConfig.GCInterval == actConfig.GCInterval
151+
}
152+
153+
// SameFile compare two files.
154+
// This method will first do the stat of two file and compare using os.SameFile method.
155+
func SameFile(str1, str2 string) (bool, error) {
156+
sFile, err := os.Stat(str1)
157+
if err != nil {
158+
return false, err
159+
}
160+
161+
tFile, err := os.Stat(str2)
162+
if err != nil {
163+
return false, err
164+
}
165+
166+
return os.SameFile(sFile, tFile), nil
167+
}
168+
147169
// Sanitize makes a sanitized copy of the config removing any secrets.
148170
func (c *Config) Sanitize() *Config {
149171
sanitizedConfig := &Config{}

‎pkg/api/config/config_elevated_test.go

+32
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
//go:build needprivileges
2+
// +build needprivileges
3+
4+
package config_test
5+
6+
import (
7+
"syscall"
8+
"testing"
9+
10+
. "github.com/smartystreets/goconvey/convey"
11+
12+
"zotregistry.io/zot/pkg/api/config"
13+
)
14+
15+
func TestMountConfig(t *testing.T) {
16+
Convey("Test config utils mounting same directory", t, func() {
17+
// If two dirs are mounting to same location SameFile should be same
18+
dir1 := t.TempDir()
19+
dir2 := t.TempDir()
20+
dir3 := t.TempDir()
21+
22+
err := syscall.Mount(dir3, dir1, "", syscall.MS_BIND, "")
23+
So(err, ShouldBeNil)
24+
25+
err = syscall.Mount(dir3, dir2, "", syscall.MS_BIND, "")
26+
So(err, ShouldBeNil)
27+
28+
isSame, err := config.SameFile(dir1, dir2)
29+
So(err, ShouldBeNil)
30+
So(isSame, ShouldBeTrue)
31+
})
32+
}

‎pkg/api/config/config_test.go

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
package config_test
2+
3+
import (
4+
"testing"
5+
"time"
6+
7+
. "github.com/smartystreets/goconvey/convey"
8+
"zotregistry.io/zot/pkg/api/config"
9+
)
10+
11+
func TestConfig(t *testing.T) {
12+
Convey("Test config utils", t, func() {
13+
firstStorageConfig := config.StorageConfig{
14+
GC: true, Dedupe: true,
15+
GCDelay: 1 * time.Minute, GCInterval: 1 * time.Hour,
16+
}
17+
secondStorageConfig := config.StorageConfig{
18+
GC: true, Dedupe: true,
19+
GCDelay: 1 * time.Minute, GCInterval: 1 * time.Hour,
20+
}
21+
22+
So(firstStorageConfig.ParamsEqual(secondStorageConfig), ShouldBeTrue)
23+
24+
firstStorageConfig.GC = false
25+
26+
So(firstStorageConfig.ParamsEqual(secondStorageConfig), ShouldBeFalse)
27+
28+
firstStorageConfig.GC = true
29+
firstStorageConfig.Dedupe = false
30+
31+
So(firstStorageConfig.ParamsEqual(secondStorageConfig), ShouldBeFalse)
32+
33+
firstStorageConfig.Dedupe = true
34+
firstStorageConfig.GCDelay = 2 * time.Minute
35+
36+
So(firstStorageConfig.ParamsEqual(secondStorageConfig), ShouldBeFalse)
37+
38+
firstStorageConfig.GCDelay = 1 * time.Minute
39+
firstStorageConfig.GCInterval = 2 * time.Hour
40+
41+
So(firstStorageConfig.ParamsEqual(secondStorageConfig), ShouldBeFalse)
42+
43+
firstStorageConfig.GCInterval = 1 * time.Hour
44+
45+
So(firstStorageConfig.ParamsEqual(secondStorageConfig), ShouldBeTrue)
46+
47+
isSame, err := config.SameFile("test-config", "test")
48+
So(err, ShouldNotBeNil)
49+
So(isSame, ShouldBeFalse)
50+
51+
dir1 := t.TempDir()
52+
53+
isSame, err = config.SameFile(dir1, "test")
54+
So(err, ShouldNotBeNil)
55+
So(isSame, ShouldBeFalse)
56+
57+
dir2 := t.TempDir()
58+
59+
isSame, err = config.SameFile(dir1, dir2)
60+
So(err, ShouldBeNil)
61+
So(isSame, ShouldBeFalse)
62+
63+
isSame, err = config.SameFile(dir1, dir1)
64+
So(err, ShouldBeNil)
65+
So(isSame, ShouldBeTrue)
66+
})
67+
}

‎pkg/api/controller.go

+99-47
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"zotregistry.io/zot/pkg/api/config"
2222
ext "zotregistry.io/zot/pkg/extensions"
2323
extconf "zotregistry.io/zot/pkg/extensions/config"
24+
"zotregistry.io/zot/pkg/extensions/lint"
2425
"zotregistry.io/zot/pkg/extensions/monitoring"
2526
"zotregistry.io/zot/pkg/log"
2627
"zotregistry.io/zot/pkg/storage"
@@ -281,54 +282,11 @@ func (c *Controller) InitImageStore(reloadCtx context.Context) error {
281282
if len(c.Config.Storage.SubPaths) > 0 {
282283
subPaths := c.Config.Storage.SubPaths
283284

284-
subImageStore := make(map[string]storage.ImageStore)
285-
286-
// creating image store per subpaths
287-
for route, storageConfig := range subPaths {
288-
// no need to validate hard links work on s3
289-
if storageConfig.Dedupe && storageConfig.StorageDriver == nil {
290-
err := storage.ValidateHardLink(storageConfig.RootDirectory)
291-
if err != nil {
292-
c.Log.Warn().Msg("input storage root directory filesystem does not supports hardlinking, " +
293-
"disabling dedupe functionality")
294-
295-
storageConfig.Dedupe = false
296-
}
297-
}
285+
subImageStore, err := c.getSubStore(subPaths, linter)
286+
if err != nil {
287+
c.Log.Error().Err(err).Msg("controller: error getting sub image store")
298288

299-
if storageConfig.StorageDriver == nil {
300-
// false positive lint - linter does not implement Lint method
301-
// nolint: typecheck
302-
subImageStore[route] = storage.NewImageStore(storageConfig.RootDirectory,
303-
storageConfig.GC, storageConfig.GCDelay, storageConfig.Dedupe, storageConfig.Commit, c.Log, c.Metrics, linter)
304-
} else {
305-
storeName := fmt.Sprintf("%v", storageConfig.StorageDriver["name"])
306-
if storeName != storage.S3StorageDriverName {
307-
c.Log.Fatal().Err(errors.ErrBadConfig).Msgf("unsupported storage driver: %s", storageConfig.StorageDriver["name"])
308-
}
309-
310-
// Init a Storager from connection string.
311-
store, err := factory.Create(storeName, storageConfig.StorageDriver)
312-
if err != nil {
313-
c.Log.Error().Err(err).Str("rootDir", storageConfig.RootDirectory).Msg("Unable to create s3 service")
314-
315-
return err
316-
}
317-
318-
/* in the case of s3 c.Config.Storage.RootDirectory is used for caching blobs locally and
319-
c.Config.Storage.StorageDriver["rootdirectory"] is the actual rootDir in s3 */
320-
rootDir := "/"
321-
if c.Config.Storage.StorageDriver["rootdirectory"] != nil {
322-
rootDir = fmt.Sprintf("%v", c.Config.Storage.StorageDriver["rootdirectory"])
323-
}
324-
325-
// false positive lint - linter does not implement Lint method
326-
// nolint: typecheck
327-
subImageStore[route] = s3.NewImageStore(rootDir, storageConfig.RootDirectory,
328-
storageConfig.GC, storageConfig.GCDelay,
329-
storageConfig.Dedupe, storageConfig.Commit, c.Log, c.Metrics, linter, store,
330-
)
331-
}
289+
return err
332290
}
333291

334292
c.StoreController.SubStore = subImageStore
@@ -340,6 +298,100 @@ func (c *Controller) InitImageStore(reloadCtx context.Context) error {
340298
return nil
341299
}
342300

301+
func (c *Controller) getSubStore(subPaths map[string]config.StorageConfig,
302+
linter *lint.Linter,
303+
) (map[string]storage.ImageStore, error) {
304+
imgStoreMap := make(map[string]storage.ImageStore, 0)
305+
306+
subImageStore := make(map[string]storage.ImageStore)
307+
308+
// creating image store per subpaths
309+
for route, storageConfig := range subPaths {
310+
// no need to validate hard links work on s3
311+
if storageConfig.Dedupe && storageConfig.StorageDriver == nil {
312+
err := storage.ValidateHardLink(storageConfig.RootDirectory)
313+
if err != nil {
314+
c.Log.Warn().Msg("input storage root directory filesystem does not supports hardlinking, " +
315+
"disabling dedupe functionality")
316+
317+
storageConfig.Dedupe = false
318+
}
319+
}
320+
321+
if storageConfig.StorageDriver == nil {
322+
// Compare if subpath root dir is same as default root dir
323+
isSame, _ := config.SameFile(c.Config.Storage.RootDirectory, storageConfig.RootDirectory)
324+
325+
if isSame {
326+
c.Log.Error().Err(errors.ErrBadConfig).Msg("sub path storage directory is same as root directory")
327+
328+
return nil, errors.ErrBadConfig
329+
}
330+
331+
isUnique := true
332+
333+
// Compare subpath unique files
334+
for file := range imgStoreMap {
335+
// We already have image storage for this file
336+
if compareImageStore(file, storageConfig.RootDirectory) {
337+
subImageStore[route] = imgStoreMap[file]
338+
339+
isUnique = true
340+
}
341+
}
342+
343+
// subpath root directory is unique
344+
// add it to uniqueSubFiles
345+
// Create a new image store and assign it to imgStoreMap
346+
if isUnique {
347+
imgStoreMap[storageConfig.RootDirectory] = storage.NewImageStore(storageConfig.RootDirectory,
348+
storageConfig.GC, storageConfig.GCDelay, storageConfig.Dedupe, storageConfig.Commit, c.Log, c.Metrics, linter)
349+
350+
subImageStore[route] = imgStoreMap[storageConfig.RootDirectory]
351+
}
352+
} else {
353+
storeName := fmt.Sprintf("%v", storageConfig.StorageDriver["name"])
354+
if storeName != storage.S3StorageDriverName {
355+
c.Log.Fatal().Err(errors.ErrBadConfig).Msgf("unsupported storage driver: %s", storageConfig.StorageDriver["name"])
356+
}
357+
358+
// Init a Storager from connection string.
359+
store, err := factory.Create(storeName, storageConfig.StorageDriver)
360+
if err != nil {
361+
c.Log.Error().Err(err).Str("rootDir", storageConfig.RootDirectory).Msg("Unable to create s3 service")
362+
363+
return nil, err
364+
}
365+
366+
/* in the case of s3 c.Config.Storage.RootDirectory is used for caching blobs locally and
367+
c.Config.Storage.StorageDriver["rootdirectory"] is the actual rootDir in s3 */
368+
rootDir := "/"
369+
if c.Config.Storage.StorageDriver["rootdirectory"] != nil {
370+
rootDir = fmt.Sprintf("%v", c.Config.Storage.StorageDriver["rootdirectory"])
371+
}
372+
373+
// false positive lint - linter does not implement Lint method
374+
// nolint: typecheck
375+
subImageStore[route] = s3.NewImageStore(rootDir, storageConfig.RootDirectory,
376+
storageConfig.GC, storageConfig.GCDelay,
377+
storageConfig.Dedupe, storageConfig.Commit, c.Log, c.Metrics, linter, store,
378+
)
379+
}
380+
}
381+
382+
return subImageStore, nil
383+
}
384+
385+
func compareImageStore(root1, root2 string) bool {
386+
isSameFile, err := config.SameFile(root1, root2)
387+
// This error is path error that means either of root directory doesn't exist, in that case do string match
388+
if err != nil {
389+
return strings.EqualFold(root1, root2)
390+
}
391+
392+
return isSameFile
393+
}
394+
343395
func (c *Controller) LoadNewConfig(reloadCtx context.Context, config *config.Config) {
344396
// reload access control config
345397
c.Config.AccessControl = config.AccessControl

‎pkg/api/controller_test.go

+64
Original file line numberDiff line numberDiff line change
@@ -839,6 +839,70 @@ func TestMultipleInstance(t *testing.T) {
839839
So(resp, ShouldNotBeNil)
840840
So(resp.StatusCode(), ShouldEqual, http.StatusOK)
841841
})
842+
843+
Convey("Test zot multiple subpath with same root directory", t, func() {
844+
port := test.GetFreePort()
845+
baseURL := test.GetBaseURL(port)
846+
conf := config.New()
847+
conf.HTTP.Port = port
848+
htpasswdPath := test.MakeHtpasswdFile()
849+
defer os.Remove(htpasswdPath)
850+
851+
conf.HTTP.Auth = &config.AuthConfig{
852+
HTPasswd: config.AuthHTPasswd{
853+
Path: htpasswdPath,
854+
},
855+
}
856+
ctlr := api.NewController(conf)
857+
globalDir := t.TempDir()
858+
subDir := t.TempDir()
859+
860+
ctlr.Config.Storage.RootDirectory = globalDir
861+
subPathMap := make(map[string]config.StorageConfig)
862+
subPathMap["/a"] = config.StorageConfig{RootDirectory: globalDir, Dedupe: true, GC: true}
863+
subPathMap["/b"] = config.StorageConfig{RootDirectory: subDir, Dedupe: true, GC: true}
864+
865+
ctlr.Config.Storage.SubPaths = subPathMap
866+
867+
err := ctlr.Run(context.Background())
868+
So(err, ShouldNotBeNil)
869+
870+
// subpath root directory does not exist.
871+
subPathMap["/a"] = config.StorageConfig{RootDirectory: globalDir, Dedupe: true, GC: true}
872+
subPathMap["/b"] = config.StorageConfig{RootDirectory: subDir, Dedupe: false, GC: true}
873+
874+
ctlr.Config.Storage.SubPaths = subPathMap
875+
876+
err = ctlr.Run(context.Background())
877+
So(err, ShouldNotBeNil)
878+
879+
subPathMap["/a"] = config.StorageConfig{RootDirectory: subDir, Dedupe: true, GC: true}
880+
subPathMap["/b"] = config.StorageConfig{RootDirectory: subDir, Dedupe: true, GC: true}
881+
882+
ctlr.Config.Storage.SubPaths = subPathMap
883+
884+
go startServer(ctlr)
885+
defer stopServer(ctlr)
886+
test.WaitTillServerReady(baseURL)
887+
888+
// without creds, should get access error
889+
resp, err := resty.R().Get(baseURL + "/v2/")
890+
So(err, ShouldBeNil)
891+
So(resp, ShouldNotBeNil)
892+
So(resp.StatusCode(), ShouldEqual, http.StatusUnauthorized)
893+
var e api.Error
894+
err = json.Unmarshal(resp.Body(), &e)
895+
So(err, ShouldBeNil)
896+
897+
// with creds, should get expected status code
898+
resp, _ = resty.R().SetBasicAuth(username, passphrase).Get(baseURL)
899+
So(resp, ShouldNotBeNil)
900+
So(resp.StatusCode(), ShouldEqual, http.StatusNotFound)
901+
902+
resp, _ = resty.R().SetBasicAuth(username, passphrase).Get(baseURL + "/v2/")
903+
So(resp, ShouldNotBeNil)
904+
So(resp.StatusCode(), ShouldEqual, http.StatusOK)
905+
})
842906
}
843907

844908
func TestTLSWithBasicAuth(t *testing.T) {

0 commit comments

Comments
 (0)