feat(metadb): add keyPrefix parameter for redis and remove unneeded m…

…ethod meta.Crate()

Signed-off-by: Andrei Aaron <[email protected]>

examples/config-redis.json

@@ -6,7 +6,8 @@
         "rootDirectory": "/tmp/zot",
         "cacheDriver": {
             "name": "redis",
-            "url": "redis://localhost:6379"
+            "url": "redis://localhost:6379",
+            "keyprefix": "zot"
         },
         "storageDriver": {
             "name": "s3",

pkg/meta/meta.go

@@ -3,10 +3,6 @@ package meta
 import (
 	"fmt"
 
-	"github.com/aws/aws-sdk-go-v2/service/dynamodb"
-	"github.com/redis/go-redis/v9"
-	"go.etcd.io/bbolt"
-
 	"zotregistry.dev/zot/errors"
 	"zotregistry.dev/zot/pkg/api/config"
 	"zotregistry.dev/zot/pkg/log"
@@ -27,19 +23,18 @@ func New(storageConfig config.StorageConfig, log log.Logger) (mTypes.MetaDB, err
 				return nil, err
 			}
 
-			return Create(sconstants.DynamoDBDriverName, client, dynamoParams, log) //nolint:contextcheck
+			return mdynamodb.New(client, dynamoParams, log) //nolint:contextcheck
 		}
 
 		if storageConfig.CacheDriver["name"] == sconstants.RedisDriverName {
-			// go-redis supports connecting via the redis uri specification (more convenient than parameter parsing)
-			redisURL := getRedisURL(storageConfig.CacheDriver, log)
+			redisParams := getRedisParams(storageConfig.CacheDriver, log)
 
-			client, err := redisdb.GetRedisClient(redisURL)
+			client, err := redisdb.GetRedisClient(redisParams)
 			if err != nil { //nolint:wsl
 				return nil, err
 			}
 
-			return Create(sconstants.RedisDriverName, client, nil, log) //nolint:contextcheck
+			return redisdb.New(client, redisParams, log) //nolint:contextcheck
 		}
 
 		// this behavior is also mentioned in the configuration validation logic inside the cli package
@@ -61,90 +56,34 @@ func New(storageConfig config.StorageConfig, log log.Logger) (mTypes.MetaDB, err
 		return nil, err
 	}
 
-	return Create("boltdb", driver, params, log) //nolint:contextcheck
-}
-
-func Create(dbtype string, dbDriver, parameters interface{}, log log.Logger, //nolint:contextcheck
-) (mTypes.MetaDB, error,
-) {
-	switch dbtype {
-	case "boltdb":
-		{
-			properDriver, ok := dbDriver.(*bbolt.DB)
-			if !ok {
-				log.Error().Err(errors.ErrTypeAssertionFailed).
-					Msgf("failed to cast type, expected type '%T' but got '%T'", &bbolt.DB{}, dbDriver)
-
-				return nil, errors.ErrTypeAssertionFailed
-			}
-
-			return boltdb.New(properDriver, log)
-		}
-	case "redis":
-		{
-			properDriver, ok := dbDriver.(*redis.Client)
-			if !ok {
-				log.Error().Err(errors.ErrTypeAssertionFailed).
-					Msgf("failed to cast type, expected type '%T' but got '%T'", &redis.Client{}, dbDriver)
-
-				return nil, errors.ErrTypeAssertionFailed
-			}
-
-			return redisdb.New(properDriver, log)
-		}
-	case "dynamodb":
-		{
-			properDriver, ok := dbDriver.(*dynamodb.Client)
-			if !ok {
-				log.Error().Err(errors.ErrTypeAssertionFailed).
-					Msgf("failed to cast type, expected type '%T' but got '%T'", &dynamodb.Client{}, dbDriver)
-
-				return nil, errors.ErrTypeAssertionFailed
-			}
-
-			properParameters, ok := parameters.(mdynamodb.DBDriverParameters)
-			if !ok {
-				log.Error().Err(errors.ErrTypeAssertionFailed).
-					Msgf("failed to cast type, expected type '%T' but got '%T'", mdynamodb.DBDriverParameters{},
-						parameters)
-
-				return nil, errors.ErrTypeAssertionFailed
-			}
-
-			return mdynamodb.New(properDriver, properParameters, log)
-		}
-	default:
-		{
-			return nil, errors.ErrBadConfig
-		}
-	}
+	return boltdb.New(driver, log) //nolint:contextcheck
 }
 
 func getDynamoParams(cacheDriverConfig map[string]interface{}, log log.Logger) mdynamodb.DBDriverParameters {
 	allParametersOk := true
 
-	endpoint, ok := toStringIfOk(cacheDriverConfig, "endpoint", log)
+	endpoint, ok := toStringIfOk(cacheDriverConfig, "endpoint", "", log)
 	allParametersOk = allParametersOk && ok
 
-	region, ok := toStringIfOk(cacheDriverConfig, "region", log)
+	region, ok := toStringIfOk(cacheDriverConfig, "region", "", log)
 	allParametersOk = allParametersOk && ok
 
-	repoMetaTablename, ok := toStringIfOk(cacheDriverConfig, "repometatablename", log)
+	repoMetaTablename, ok := toStringIfOk(cacheDriverConfig, "repometatablename", "", log)
 	allParametersOk = allParametersOk && ok
 
-	repoBlobsInfoTablename, ok := toStringIfOk(cacheDriverConfig, "repoblobsinfotablename", log)
+	repoBlobsInfoTablename, ok := toStringIfOk(cacheDriverConfig, "repoblobsinfotablename", "", log)
 	allParametersOk = allParametersOk && ok
 
-	imageMetaTablename, ok := toStringIfOk(cacheDriverConfig, "imagemetatablename", log)
+	imageMetaTablename, ok := toStringIfOk(cacheDriverConfig, "imagemetatablename", "", log)
 	allParametersOk = allParametersOk && ok
 
-	apiKeyTablename, ok := toStringIfOk(cacheDriverConfig, "apikeytablename", log)
+	apiKeyTablename, ok := toStringIfOk(cacheDriverConfig, "apikeytablename", "", log)
 	allParametersOk = allParametersOk && ok
 
-	versionTablename, ok := toStringIfOk(cacheDriverConfig, "versiontablename", log)
+	versionTablename, ok := toStringIfOk(cacheDriverConfig, "versiontablename", "", log)
 	allParametersOk = allParametersOk && ok
 
-	userDataTablename, ok := toStringIfOk(cacheDriverConfig, "userdatatablename", log)
+	userDataTablename, ok := toStringIfOk(cacheDriverConfig, "userdatatablename", "", log)
 	allParametersOk = allParametersOk && ok
 
 	if !allParametersOk {
@@ -163,20 +102,38 @@ func getDynamoParams(cacheDriverConfig map[string]interface{}, log log.Logger) m
 	}
 }
 
-func getRedisURL(cacheDriverConfig map[string]interface{}, log log.Logger) string {
-	url, ok := toStringIfOk(cacheDriverConfig, "url", log)
+func getRedisParams(cacheDriverConfig map[string]interface{}, log log.Logger) redisdb.DBDriverParameters {
+	url, ok := toStringIfOk(cacheDriverConfig, "url", "", log)
+
+	if !ok {
+		log.Panic().Msg("redis parameters are not specified correctly, can't proceed")
+	}
+
+	keyPrefix, ok := toStringIfOk(cacheDriverConfig, "keyprefix", "zot", log)
 
 	if !ok {
 		log.Panic().Msg("redis parameters are not specified correctly, can't proceed")
 	}
 
-	return url
+	return redisdb.DBDriverParameters{
+		URL:       url,
+		KeyPrefix: keyPrefix,
+	}
 }
 
-func toStringIfOk(cacheDriverConfig map[string]interface{}, param string, log log.Logger) (string, bool) {
+func toStringIfOk(cacheDriverConfig map[string]interface{},
+	param string,
+	defaultVal string,
+	log log.Logger,
+) (string, bool) {
 	val, ok := cacheDriverConfig[param]
 
-	if !ok {
+	if !ok && defaultVal != "" {
+		log.Info().Str("field", param).Str("default", defaultVal).
+			Msg("field is not present in CacheDriver config, using default value")
+
+		return defaultVal, true
+	} else if !ok {
 		log.Error().Str("field", param).Msg("failed to parse CacheDriver config, field is not present")
 
 		return "", false

pkg/meta/meta_test.go

@@ -12,17 +12,16 @@ import (
 	"time"
 
 	"github.com/alicebob/miniredis/v2"
-	"github.com/aws/aws-sdk-go-v2/service/dynamodb"
 	guuid "github.com/gofrs/uuid"
 	"github.com/notaryproject/notation-core-go/signature/jws"
 	"github.com/notaryproject/notation-go"
 	"github.com/notaryproject/notation-go/signer"
 	godigest "github.com/opencontainers/go-digest"
 	ispec "github.com/opencontainers/image-spec/specs-go/v1"
-	"github.com/redis/go-redis/v9"
 	. "github.com/smartystreets/goconvey/convey"
 
 	zerr "zotregistry.dev/zot/errors"
+	"zotregistry.dev/zot/pkg/api/config"
 	zcommon "zotregistry.dev/zot/pkg/common"
 	"zotregistry.dev/zot/pkg/extensions/imagetrust"
 	"zotregistry.dev/zot/pkg/extensions/search/convert"
@@ -175,10 +174,15 @@ func TestRedisDB(t *testing.T) {
 		rootDir := t.TempDir()
 		log := log.NewLogger("debug", "")
 
-		redisDriver, err := redisdb.GetRedisClient("redis://" + miniRedis.Addr())
+		params := redisdb.DBDriverParameters{
+			KeyPrefix: "zot",
+			URL:       "redis://" + miniRedis.Addr(),
+		}
+
+		redisDriver, err := redisdb.GetRedisClient(params)
 		So(err, ShouldBeNil)
 
-		metaDB, err := redisdb.New(redisDriver, log)
+		metaDB, err := redisdb.New(redisDriver, params, log)
 		So(metaDB, ShouldNotBeNil)
 		So(err, ShouldBeNil)
 
@@ -2629,97 +2633,98 @@ func TestRelevanceSorting(t *testing.T) {
 	})
 }
 
-func TestCreateDynamo(t *testing.T) {
-	tskip.SkipDynamo(t)
-
-	Convey("Create", t, func() {
-		dynamoDBDriverParams := mdynamodb.DBDriverParameters{
-			Endpoint:               os.Getenv("DYNAMODBMOCK_ENDPOINT"),
-			RepoMetaTablename:      "RepoMetadataTable",
-			RepoBlobsInfoTablename: "RepoBlobs",
-			ImageMetaTablename:     "ImageMeta",
-			UserDataTablename:      "UserDataTable",
-			APIKeyTablename:        "ApiKeyTable",
-			VersionTablename:       "Version",
-			Region:                 "us-east-2",
-		}
+func TestCreateBoltDB(t *testing.T) {
+	Convey("New() succeeds", t, func() {
+		rootDir := t.TempDir()
 
-		client, err := mdynamodb.GetDynamoClient(dynamoDBDriverParams)
-		So(err, ShouldBeNil)
+		conf := config.New()
+		conf.Storage.RootDirectory = rootDir
 
 		log := log.NewLogger("debug", "")
+		So(log, ShouldNotBeNil)
 
-		metaDB, err := meta.Create("dynamodb", client, dynamoDBDriverParams, log)
-		So(metaDB, ShouldNotBeNil)
-		So(err, ShouldBeNil)
-	})
-
-	Convey("Fails", t, func() {
-		log := log.NewLogger("debug", "")
+		Convey("Test New() with unspecified driver", func() {
+			conf.Storage.CacheDriver = map[string]interface{}{}
+		})
 
-		_, err := meta.Create("dynamodb", nil, boltdb.DBParameters{RootDir: "root"}, log)
-		So(err, ShouldNotBeNil)
+		Convey("Test New() with bad driver", func() {
+			// we default to bolt in case of misconfiguration
+			conf.Storage.CacheDriver = map[string]interface{}{"name": "somedriver"}
+		})
 
-		_, err = meta.Create("dynamodb", &dynamodb.Client{}, "bad", log)
-		So(err, ShouldNotBeNil)
+		Convey("Test New() with specified driver", func() {
+			conf.Storage.CacheDriver = map[string]interface{}{"name": "cache"}
+		})
 
-		metaDB, err := meta.Create("random", nil, boltdb.DBParameters{RootDir: "root"}, log)
-		So(metaDB, ShouldBeNil)
-		So(err, ShouldNotBeNil)
-	})
-}
+		repoDBPath := path.Join(rootDir, "meta.db")
+		defer os.Remove(repoDBPath)
 
-func TestCreateBoltDB(t *testing.T) {
-	Convey("Create", t, func() {
-		rootDir := t.TempDir()
-		params := boltdb.DBParameters{
-			RootDir: rootDir,
-		}
-		boltDriver, err := boltdb.GetBoltDriver(params)
+		metaDB, err := meta.New(conf.Storage.StorageConfig, log)
 		So(err, ShouldBeNil)
-
-		log := log.NewLogger("debug", "")
-
-		metaDB, err := meta.Create("boltdb", boltDriver, params, log)
 		So(metaDB, ShouldNotBeNil)
-		So(err, ShouldBeNil)
-	})
 
-	Convey("fails", t, func() {
-		log := log.NewLogger("debug", "")
+		err = os.Chmod(repoDBPath, 0o200)
+		So(err, ShouldBeNil)
 
-		_, err := meta.Create("boltdb", nil, mdynamodb.DBDriverParameters{}, log)
+		metaDB, err = meta.New(conf.Storage.StorageConfig, log)
 		So(err, ShouldNotBeNil)
+		So(metaDB, ShouldBeNil)
+
+		err = os.Chmod(repoDBPath, 0o600)
+		So(err, ShouldBeNil)
 	})
 }
 
 func TestCreateRedisDB(t *testing.T) {
-	Convey("Create", t, func() {
-		miniRedis := miniredis.RunT(t)
+	Convey("Test New()", t, func() {
+		conf := config.New()
+		conf.Storage.RemoteCache = true
 
 		log := log.NewLogger("debug", "")
 		So(log, ShouldNotBeNil)
 
-		redisDriver, err := redisdb.GetRedisClient("redis://" + miniRedis.Addr())
-		So(err, ShouldBeNil)
+		Convey("Succeeds with default key prefix", func() {
+			miniRedis := miniredis.RunT(t)
 
-		metaDB, err := meta.Create("redis", redisDriver, nil, log)
-		So(metaDB, ShouldNotBeNil)
-		So(err, ShouldBeNil)
-	})
+			cacheDriverParams := map[string]interface{}{
+				"name": "redis",
+				"url":  "redis://" + miniRedis.Addr(),
+			}
 
-	Convey("fails", t, func() {
-		log := log.NewLogger("debug", "")
+			conf.Storage.CacheDriver = cacheDriverParams
 
-		_, err := meta.Create("redis", nil, mdynamodb.DBDriverParameters{}, log)
-		So(err, ShouldNotBeNil)
+			metaDB, err := meta.New(conf.Storage.StorageConfig, log)
+			So(err, ShouldBeNil)
+			So(metaDB, ShouldNotBeNil)
+		})
 
-		// Redis client will not be responding
-		redisURL := "redis://127.0.0.1:" + tCommon.GetFreePort() // must not match miniRedis.Addr()
-		connOpts, _ := redis.ParseURL(redisURL)
-		cacheDB := redis.NewClient(connOpts)
+		Convey("Succeeds with specific key prefix", func() {
+			miniRedis := miniredis.RunT(t)
 
-		_, err = meta.Create("redis", cacheDB, nil, log)
-		So(err, ShouldNotBeNil)
+			cacheDriverParams := map[string]interface{}{
+				"name": "redis",
+				"url":  "redis://" + miniRedis.Addr(),
+				"key":  "keyPrefix",
+			}
+
+			conf.Storage.CacheDriver = cacheDriverParams
+
+			metaDB, err := meta.New(conf.Storage.StorageConfig, log)
+			So(err, ShouldBeNil)
+			So(metaDB, ShouldNotBeNil)
+		})
+
+		Convey("Fails on Ping()", func() {
+			// Redis client will not be responding
+			cacheDriverParams := map[string]interface{}{
+				"name": "redis",
+				"url":  "redis://127.0.0.1:" + tCommon.GetFreePort(),
+			}
+
+			conf.Storage.CacheDriver = cacheDriverParams
+
+			_, err := meta.New(conf.Storage.StorageConfig, log)
+			So(err, ShouldNotBeNil)
+		})
 	})
 }