Unverified Commit 15563450 authored by Moti Cohen's avatar Moti Cohen Committed by GitHub
Browse files

modules API: Support register unprefixed config parameters (#13656)

PR #10285 introduced support for modules to register four types of
configurations — Bool, Numeric, String, and Enum. Accessible through the
Redis config file and the CONFIG command.

With this PR, it will be possible to register configuration parameters
without automatically prefixing the parameter names. This provides
greater flexibility in configuration naming, enabling, for instance,
both `bf-initial-size` or `initial-size` to be defined in the module
without automatically prefixing with `<MODULE-NAME>.`. In addition it
will also be possible to create a single additional alias via the same
API. This brings us another step closer to integrate modules into redis
core.

**Example:** Register a configuration parameter `bf-initial-size` with
an alias `initial-size` without the automatic module name prefix, set
with new `REDISMODULE_CONFIG_UNPREFIXED` flag:
```
RedisModule_RegisterBoolConfig(ctx, "bf-initial-size|initial-size", default_val, optflags | REDISMODULE_CONFIG_UNPREFIXED, getfn, setfn, applyfn, privdata);
```
# API changes
Related functions that now support unprefixed configuration flag
(`REDISMODULE_CONFIG_UNPREFIXED`) along with optional alias:
```
RedisModule_RegisterBoolConfig
RedisModule_RegisterEnumConfig
RedisModule_RegisterNumericConfig
RedisModule_RegisterStringConfig
```

# Implementation Details:
`config.c`: On load server configuration, at function
`loadServerConfigFromString()`, it collects all unknown configurations
into `module_configs_queue` dictionary. These may include valid module
configurations or invalid ones. They will be validated later by
`loadModuleConfigs()` against the configurations declared by the loaded
module(s).
`Module.c:` The `ModuleConfig` structure has been modified to store now:
(1) Full configuration name (2) Alias (3) Unprefixed flag status -
ensuring that configurations retain their original registration format
when triggered in notifications.

Added error printout:
This change introduces an error printout for unresolved configurations,
detailing each unresolved parameter detected during startup. The last
line in the output existed prior to this change and has been retained to
systems relies on it:
```
595011:M 18 Nov 2024 08:26:23.616 # Unresolved Configuration(s) Detected:
595011:M 18 Nov 2024 08:26:23.616 #  >>> 'bf-initiel-size 8'
595011:M 18 Nov 2024 08:26:23.616 #  >>> 'search-sizex 32'
595011:M 18 Nov 2024 08:26:23.616 # Module Configuration detected without loadmodule directive or no ApplyConfig call: aborting
```

# Backward Compatibility:
Existing modules will function without modification, as the new
functionality only applies if REDISMODULE_CONFIG_UNPREFIXED is
explicitly set.

# Module vs. Core API Conflict Behavior
The new API allows to modules loading duplication of same configuration
name or same configuration alias, just like redis core configuration
allows (i.e. the users sets two configs with a different value, but
these two configs are actually the same one). Unlike redis core, given a
name and its alias, it doesn't allow have both configuration on load. To
implement it, it is required to modify DS `module_configs_queue` to
reflect the order of their loading and later on, during
`loadModuleConfigs()`, resolve pairs of names and aliases and which one
is the last one to apply. "Relaxing" this limitation can be deferred to
a future update if necessary, but for now, we error in this case.
parent 79fd2558
......@@ -268,7 +268,7 @@ dict *configs = NULL; /* Runtime config values */
/* Lookup a config by the provided sds string name, or return NULL
* if the config does not exist */
static standardConfig *lookupConfig(sds name) {
static standardConfig *lookupConfig(const sds name) {
dictEntry *de = dictFind(configs, name);
return de ? dictGetVal(de) : NULL;
}
......@@ -552,16 +552,6 @@ void loadServerConfigFromString(char *config) {
}
} else if (!strcasecmp(argv[0],"loadmodule") && argc >= 2) {
queueLoadModule(argv[1],&argv[2],argc-2);
} else if (strchr(argv[0], '.')) {
if (argc < 2) {
err = "Module config specified without value";
goto loaderr;
}
sds name = sdsdup(argv[0]);
sds val = sdsdup(argv[1]);
for (int i = 2; i < argc; i++)
val = sdscatfmt(val, " %S", argv[i]);
if (!dictReplace(server.module_configs_queue, name, val)) sdsfree(name);
} else if (!strcasecmp(argv[0],"sentinel")) {
/* argc == 1 is handled by main() as we need to enter the sentinel
* mode ASAP. */
......@@ -573,7 +563,20 @@ void loadServerConfigFromString(char *config) {
queueSentinelConfig(argv+1,argc-1,linenum,lines[i]);
}
} else {
err = "Bad directive or wrong number of arguments"; goto loaderr;
/* Collect all unknown configurations into `module_configs_queue`.
* These may include valid module configurations or invalid ones.
* They will be validated later by loadModuleConfigs() against the
* configurations declared by the loaded module(s). */
if (argc < 2) {
err = "Bad directive or wrong number of arguments";
goto loaderr;
}
sds name = sdsdup(argv[0]);
sds val = sdsdup(argv[1]);
for (int i = 2; i < argc; i++)
val = sdscatfmt(val, " %S", argv[i]);
if (!dictReplace(server.module_configs_queue, name, val)) sdsfree(name);
}
sdsfreesplitres(argv,argc);
argv = NULL;
......@@ -3312,16 +3315,34 @@ void removeConfig(sds name) {
standardConfig *config = lookupConfig(name);
if (!config) return;
if (config->flags & MODULE_CONFIG) {
sdsfree((sds) config->name);
if (config->type == ENUM_CONFIG) {
configEnum *enumNode = config->data.enumd.enum_value;
while(enumNode->name != NULL) {
zfree(enumNode->name);
enumNode++;
}
zfree(config->data.enumd.enum_value);
} else if (config->type == SDS_CONFIG) {
if (config->data.sds.default_value) sdsfree((sds)config->data.sds.default_value);
sdsfree((sds) config->alias);
switch (config->type) {
case BOOL_CONFIG:
break;
case NUMERIC_CONFIG:
break;
case SDS_CONFIG:
if (config->data.sds.default_value)
sdsfree((sds)config->data.sds.default_value);
break;
case ENUM_CONFIG:
{
configEnum *enumNode = config->data.enumd.enum_value;
while(enumNode->name != NULL) {
zfree(enumNode->name);
enumNode++;
}
zfree(config->data.enumd.enum_value);
}
break;
case SPECIAL_CONFIG: /* Not used by modules */
case STRING_CONFIG: /* Not used by modules */
default:
serverAssert(0);
break;
}
}
dictDelete(configs, name);
......@@ -3332,40 +3353,77 @@ void removeConfig(sds name) {
*----------------------------------------------------------------------------*/
/* Create a bool/string/enum/numeric standardConfig for a module config in the configs dictionary */
void addModuleBoolConfig(const char *module_name, const char *name, int flags, void *privdata, int default_val) {
sds config_name = sdscatfmt(sdsempty(), "%s.%s", module_name, name);
/* On removeConfig(), name and alias will be sdsfree() */
void addModuleBoolConfig(sds name, sds alias, int flags, void *privdata, int default_val) {
int config_dummy_address;
standardConfig module_config = createBoolConfig(config_name, NULL, flags | MODULE_CONFIG, config_dummy_address, default_val, NULL, NULL);
module_config.data.yesno.config = NULL;
module_config.privdata = privdata;
registerConfigValue(config_name, &module_config, 0);
standardConfig sc = createBoolConfig(name, alias, flags | MODULE_CONFIG, config_dummy_address, default_val, NULL, NULL);
sc.data.yesno.config = NULL;
sc.privdata = privdata;
registerConfigValue(name, &sc, 0);
/* If alias available, deep copy standardConfig and register again */
if (alias) {
sc.name = sdsdup(name);
sc.alias = sdsdup(alias);
registerConfigValue(sc.alias, &sc, 1);
}
}
void addModuleStringConfig(const char *module_name, const char *name, int flags, void *privdata, sds default_val) {
sds config_name = sdscatfmt(sdsempty(), "%s.%s", module_name, name);
/* On removeConfig(), name, default_val, and alias will be sdsfree() */
void addModuleStringConfig(sds name, sds alias, int flags, void *privdata, sds default_val) {
sds config_dummy_address;
standardConfig module_config = createSDSConfig(config_name, NULL, flags | MODULE_CONFIG, 0, config_dummy_address, default_val, NULL, NULL);
module_config.data.sds.config = NULL;
module_config.privdata = privdata;
registerConfigValue(config_name, &module_config, 0);
standardConfig sc = createSDSConfig(name, alias, flags | MODULE_CONFIG, 0, config_dummy_address, default_val, NULL, NULL);
sc.data.sds.config = NULL;
sc.privdata = privdata;
registerConfigValue(name, &sc, 0); /* memcpy sc */
/* If alias available, deep copy standardConfig and register again */
if (alias) {
sc.name = sdsdup(name);
sc.alias = sdsdup(alias);
if (default_val) sc.data.sds.default_value = sdsdup(default_val);
registerConfigValue(sc.alias, &sc, 1);
}
}
void addModuleEnumConfig(const char *module_name, const char *name, int flags, void *privdata, int default_val, configEnum *enum_vals) {
sds config_name = sdscatfmt(sdsempty(), "%s.%s", module_name, name);
/* On removeConfig(), name, default_val, alias and enum_vals will be freed */
void addModuleEnumConfig(sds name, sds alias, int flags, void *privdata, int default_val, configEnum *enum_vals, int num_enum_vals) {
int config_dummy_address;
standardConfig module_config = createEnumConfig(config_name, NULL, flags | MODULE_CONFIG, enum_vals, config_dummy_address, default_val, NULL, NULL);
module_config.data.enumd.config = NULL;
module_config.privdata = privdata;
registerConfigValue(config_name, &module_config, 0);
standardConfig sc = createEnumConfig(name, alias, flags | MODULE_CONFIG, enum_vals, config_dummy_address, default_val, NULL, NULL);
sc.data.enumd.config = NULL;
sc.privdata = privdata;
registerConfigValue(name, &sc, 0);
/* If alias available, deep copy standardConfig and register again */
if (alias) {
sc.name = sdsdup(name);
sc.alias = sdsdup(alias);
sc.data.enumd.enum_value = zmalloc((num_enum_vals + 1) * sizeof(configEnum));
for (int i = 0; i < num_enum_vals; i++) {
sc.data.enumd.enum_value[i].name = zstrdup(enum_vals[i].name);
sc.data.enumd.enum_value[i].val = enum_vals[i].val;
}
sc.data.enumd.enum_value[num_enum_vals].name = NULL;
sc.data.enumd.enum_value[num_enum_vals].val = 0;
registerConfigValue(sc.alias, &sc, 1);
}
}
void addModuleNumericConfig(const char *module_name, const char *name, int flags, void *privdata, long long default_val, int conf_flags, long long lower, long long upper) {
sds config_name = sdscatfmt(sdsempty(), "%s.%s", module_name, name);
/* On removeConfig(), it will free name, and alias if it is not NULL */
void addModuleNumericConfig(sds name, sds alias, int flags, void *privdata, long long default_val, int conf_flags, long long lower, long long upper) {
long long config_dummy_address;
standardConfig module_config = createLongLongConfig(config_name, NULL, flags | MODULE_CONFIG, lower, upper, config_dummy_address, default_val, conf_flags, NULL, NULL);
module_config.data.numeric.config.ll = NULL;
module_config.privdata = privdata;
registerConfigValue(config_name, &module_config, 0);
standardConfig sc = createLongLongConfig(name, alias, flags | MODULE_CONFIG, lower, upper, config_dummy_address, default_val, conf_flags, NULL, NULL);
sc.data.numeric.config.ll = NULL;
sc.privdata = privdata;
registerConfigValue(name, &sc, 0);
/* If alias available, deep copy standardConfig and register again */
if (alias) {
sc.name = sdsdup(name);
sc.alias = sdsdup(alias);
registerConfigValue(sc.alias, &sc, 1);
}
}
/*-----------------------------------------------------------------------------
......@@ -3418,3 +3476,7 @@ void configRewriteCommand(client *c) {
addReply(c,shared.ok);
}
}
int configExists(const sds name) {
return lookupConfig(name) != NULL;
}
......@@ -437,7 +437,13 @@ typedef int (*RedisModuleConfigApplyFunc)(RedisModuleCtx *ctx, void *privdata, R
 
/* Struct representing a module config. These are stored in a list in the module struct */
struct ModuleConfig {
sds name; /* Name of config without the module name appended to the front */
sds name; /* Fullname of the config (as it appears in the config file) */
sds alias; /* Optional alias for the configuration. NULL if none exists */
int unprefixedFlag; /* Indicates if the REDISMODULE_CONFIG_UNPREFIXED flag was set.
* If the configuration name was prefixed,during get_fn/set_fn
* callbacks, it should be reported without the prefix */
void *privdata; /* Optional data passed into the module config callbacks */
union get_fn { /* The get callback specified by the module */
RedisModuleConfigGetStringFunc get_string;
......@@ -2253,12 +2259,16 @@ int moduleIsModuleCommand(void *module_handle, struct redisCommand *cmd) {
* -------------------------------------------------------------------------- */
 
int moduleListConfigMatch(void *config, void *name) {
return strcasecmp(((ModuleConfig *) config)->name, (char *) name) == 0;
ModuleConfig *mc = (ModuleConfig *) config;
/* Compare the provided name with the config's name and alias if it exists */
return strcasecmp(mc->name, (char *) name) == 0 ||
((mc->alias) && strcasecmp(mc->alias, (char *) name) == 0);
}
 
void moduleListFree(void *config) {
ModuleConfig *module_config = (ModuleConfig *) config;
sdsfree(module_config->name);
sdsfree(module_config->alias);
zfree(config);
}
 
......@@ -12090,10 +12100,9 @@ void moduleRemoveConfigs(RedisModule *module) {
listRewind(module->module_configs, &li);
while ((ln = listNext(&li))) {
ModuleConfig *config = listNodeValue(ln);
sds module_name = sdsnew(module->name);
sds full_name = sdscat(sdscat(module_name, "."), config->name); /* ModuleName.ModuleConfig */
removeConfig(full_name);
sdsfree(full_name);
removeConfig(config->name);
if (config->alias)
removeConfig(config->alias);
}
}
 
......@@ -12132,6 +12141,12 @@ void moduleLoadFromQueue(void) {
listDelNode(server.loadmodule_queue, ln);
}
if (dictSize(server.module_configs_queue)) {
serverLog(LL_WARNING, "Unresolved Configuration(s) Detected:");
dictIterator *di = dictGetIterator(server.module_configs_queue);
dictEntry *de;
while ((de = dictNext(di)) != NULL) {
serverLog(LL_WARNING, ">>> '%s %s'", (char *)dictGetKey(de), (char *)dictGetVal(de));
}
serverLog(LL_WARNING, "Module Configuration detected without loadmodule directive or no ApplyConfig call: aborting");
exit(1);
}
......@@ -12582,7 +12597,8 @@ int moduleVerifyConfigFlags(unsigned int flags, configType type) {
| REDISMODULE_CONFIG_PROTECTED
| REDISMODULE_CONFIG_DENY_LOADING
| REDISMODULE_CONFIG_BITFLAGS
| REDISMODULE_CONFIG_MEMORY))) {
| REDISMODULE_CONFIG_MEMORY
| REDISMODULE_CONFIG_UNPREFIXED))) {
serverLogRaw(LL_WARNING, "Invalid flag(s) for configuration");
return REDISMODULE_ERR;
}
......@@ -12619,6 +12635,54 @@ int moduleVerifyResourceName(const char *name) {
return REDISMODULE_OK;
}
 
/* Verify unprefixed name config might be a single "<name>" or in the form
* "<name>|<alias>". Unlike moduleVerifyResourceName(), unprefixed name config
* allows a single dot in the name or alias.
*
* delim - Updates to point to "|" if it exists, NULL otherwise.
*/
int moduleVerifyUnprefixedName(const char *nameAlias, const char **delim) {
if (nameAlias[0] == '\0')
return REDISMODULE_ERR;
*delim = NULL;
int dot_count = 0, lname = 0;
for (size_t i = 0; nameAlias[i] != '\0'; i++) {
char ch = nameAlias[i];
if (((*delim) == NULL) && (ch == '|')) {
/* Handle single separator between name and alias */
if (!lname) {
serverLog(LL_WARNING, "Module configuration name is empty: %s", nameAlias);
return REDISMODULE_ERR;
}
*delim = &nameAlias[i];
dot_count = lname = 0;
} else if ( (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') ||
(ch >= '0' && ch <= '9') || (ch == '_') || (ch == '-') )
{
++lname;
} else if (ch == '.') {
/* Allow only one dot per section (name or alias) */
if (++dot_count > 1) {
serverLog(LL_WARNING, "Invalid character sequence in Module configuration name or alias: %s", nameAlias);
return REDISMODULE_ERR;
}
} else {
serverLog(LL_WARNING, "Invalid character %c in Module configuration name or alias %s.", ch, nameAlias);
return REDISMODULE_ERR;
}
}
if (!lname) {
serverLog(LL_WARNING, "Module configuration name or alias is empty : %s", nameAlias);
return REDISMODULE_ERR;
}
return REDISMODULE_OK;
}
/* This is a series of set functions for each type that act as dispatchers for
* config.c to call module set callbacks. */
#define CONFIG_ERR_SIZE 256
......@@ -12631,9 +12695,24 @@ static void propagateErrorString(RedisModuleString *err_in, const char **err) {
}
}
 
/* If configuration was originally registered with indication to prefix the name,
* return the name without the prefix by skipping prefix "<MODULE-NAME>.".
* Otherwise, return the stored name as is. */
static char *getRegisteredConfigName(ModuleConfig *config) {
if (config->unprefixedFlag)
return config->name;
/* For prefixed configuration, find the '.' indicating the end of the prefix */
char *endOfPrefix = strchr(config->name, '.');
serverAssert(endOfPrefix != NULL);
return endOfPrefix + 1;
}
int setModuleBoolConfig(ModuleConfig *config, int val, const char **err) {
RedisModuleString *error = NULL;
int return_code = config->set_fn.set_bool(config->name, val, config->privdata, &error);
char *rname = getRegisteredConfigName(config);
int return_code = config->set_fn.set_bool(rname, val, config->privdata, &error);
propagateErrorString(error, err);
return return_code == REDISMODULE_OK ? 1 : 0;
}
......@@ -12641,7 +12720,9 @@ int setModuleBoolConfig(ModuleConfig *config, int val, const char **err) {
int setModuleStringConfig(ModuleConfig *config, sds strval, const char **err) {
RedisModuleString *error = NULL;
RedisModuleString *new = createStringObject(strval, sdslen(strval));
int return_code = config->set_fn.set_string(config->name, new, config->privdata, &error);
char *rname = getRegisteredConfigName(config);
int return_code = config->set_fn.set_string(rname, new, config->privdata, &error);
propagateErrorString(error, err);
decrRefCount(new);
return return_code == REDISMODULE_OK ? 1 : 0;
......@@ -12656,7 +12737,8 @@ int setModuleEnumConfig(ModuleConfig *config, int val, const char **err) {
 
int setModuleNumericConfig(ModuleConfig *config, long long val, const char **err) {
RedisModuleString *error = NULL;
int return_code = config->set_fn.set_numeric(config->name, val, config->privdata, &error);
char *rname = getRegisteredConfigName(config);
int return_code = config->set_fn.set_numeric(rname, val, config->privdata, &error);
propagateErrorString(error, err);
return return_code == REDISMODULE_OK ? 1 : 0;
}
......@@ -12664,20 +12746,24 @@ int setModuleNumericConfig(ModuleConfig *config, long long val, const char **err
/* This is a series of get functions for each type that act as dispatchers for
* config.c to call module set callbacks. */
int getModuleBoolConfig(ModuleConfig *module_config) {
return module_config->get_fn.get_bool(module_config->name, module_config->privdata);
char *rname = getRegisteredConfigName(module_config);
return module_config->get_fn.get_bool(rname, module_config->privdata);
}
 
sds getModuleStringConfig(ModuleConfig *module_config) {
RedisModuleString *val = module_config->get_fn.get_string(module_config->name, module_config->privdata);
char *rname = getRegisteredConfigName(module_config);
RedisModuleString *val = module_config->get_fn.get_string(rname, module_config->privdata);
return val ? sdsdup(val->ptr) : NULL;
}
 
int getModuleEnumConfig(ModuleConfig *module_config) {
return module_config->get_fn.get_enum(module_config->name, module_config->privdata);
char *rname = getRegisteredConfigName(module_config);
return module_config->get_fn.get_enum(rname, module_config->privdata);
}
 
long long getModuleNumericConfig(ModuleConfig *module_config) {
return module_config->get_fn.get_numeric(module_config->name, module_config->privdata);
char *rname = getRegisteredConfigName(module_config);
return module_config->get_fn.get_numeric(rname, module_config->privdata);
}
 
/* This function takes a module and a list of configs stored as sds NAME VALUE pairs.
......@@ -12689,25 +12775,26 @@ int loadModuleConfigs(RedisModule *module) {
listRewind(module->module_configs, &li);
while ((ln = listNext(&li))) {
ModuleConfig *module_config = listNodeValue(ln);
sds config_name = sdscatfmt(sdsempty(), "%s.%s", module->name, module_config->name);
dictEntry *config_argument = dictFind(server.module_configs_queue, config_name);
if (config_argument) {
if (!performModuleConfigSetFromName(dictGetKey(config_argument), dictGetVal(config_argument), &err)) {
serverLog(LL_WARNING, "Issue during loading of configuration %s : %s", (sds) dictGetKey(config_argument), err);
sdsfree(config_name);
dictEntry *de = dictUnlink(server.module_configs_queue, module_config->name);
if ((!de) && (module_config->alias))
de = dictUnlink(server.module_configs_queue, module_config->alias);
/* If found in the queue, set the value. Otherwise, set the default value. */
if (de) {
if (!performModuleConfigSetFromName(dictGetKey(de), dictGetVal(de), &err)) {
serverLog(LL_WARNING, "Issue during loading of configuration %s : %s", (sds) dictGetKey(de), err);
dictFreeUnlinkedEntry(server.module_configs_queue, de);
dictEmpty(server.module_configs_queue, NULL);
return REDISMODULE_ERR;
}
dictFreeUnlinkedEntry(server.module_configs_queue, de);
} else {
if (!performModuleConfigSetDefaultFromName(config_name, &err)) {
if (!performModuleConfigSetDefaultFromName(module_config->name, &err)) {
serverLog(LL_WARNING, "Issue attempting to set default value of configuration %s : %s", module_config->name, err);
sdsfree(config_name);
dictEmpty(server.module_configs_queue, NULL);
return REDISMODULE_ERR;
}
}
dictDelete(server.module_configs_queue, config_name);
sdsfree(config_name);
}
module->configs_initialized = 1;
return REDISMODULE_OK;
......@@ -12757,26 +12844,93 @@ int moduleConfigApplyConfig(list *module_configs, const char **err, const char *
* ## Module Configurations API
* -------------------------------------------------------------------------- */
 
/* Create a module config object. */
ModuleConfig *createModuleConfig(const char *name, RedisModuleConfigApplyFunc apply_fn, void *privdata, RedisModule *module) {
/* Resolve config name and create a module config object */
ModuleConfig *createModuleConfig(const char *name, RedisModuleConfigApplyFunc apply_fn,
void *privdata, RedisModule *module, unsigned int flags)
{
sds cname, alias = NULL;
/* Determine the configuration name:
* - If the unprefixed flag is set, the "<MODULE-NAME>." prefix is omitted.
* - An optional alias can be specified using "<NAME>|<ALIAS>".
*
* Examples:
* - Unprefixed: "bf.initial_size" or "bf-initial-size|bf.initial_size".
* - Prefixed: "initial_size" becomes "<MODULE-NAME>.initial_size".
*/
if (flags & REDISMODULE_CONFIG_UNPREFIXED) {
const char *delim = strchr(name, '|');
cname = sdsnew(name);
if (delim) { /* Handle "<NAME>|<ALIAS>" format */
sdssubstr(cname, 0, delim - name);
alias = sdsnew(delim + 1);
}
} else {
/* Add the module name prefix */
cname = sdscatfmt(sdsempty(), "%s.%s", module->name, name);
}
ModuleConfig *new_config = zmalloc(sizeof(ModuleConfig));
new_config->name = sdsnew(name);
new_config->unprefixedFlag = flags & REDISMODULE_CONFIG_UNPREFIXED;
new_config->name = cname;
new_config->alias = alias;
new_config->apply_fn = apply_fn;
new_config->privdata = privdata;
new_config->module = module;
return new_config;
}
 
/* Verify the configuration name and check for duplicates.
*
* - If the configuration is flagged as unprefixed, it checks for duplicate
* names and optional aliases in the format <NAME>|<ALIAS>.
* - If the configuration is prefixed, it ensures the name is unique with
* the module name prepended (<MODULE_NAME>.<NAME>).
*/
int moduleConfigValidityCheck(RedisModule *module, const char *name, unsigned int flags, configType type) {
if (!module->onload) {
errno = EBUSY;
return REDISMODULE_ERR;
}
if (moduleVerifyConfigFlags(flags, type) || moduleVerifyResourceName(name)) {
if (moduleVerifyConfigFlags(flags, type)) {
errno = EINVAL;
return REDISMODULE_ERR;
}
if (isModuleConfigNameRegistered(module, name)) {
int isdup = 0;
if (flags & REDISMODULE_CONFIG_UNPREFIXED) {
const char *delim = NULL; /* Pointer to the '|' delimiter in <NAME>|<ALIAS> */
if (moduleVerifyUnprefixedName(name, &delim)){
errno = EINVAL;
return REDISMODULE_ERR;
}
if (delim) {
/* Temporary split the "<NAME>|<ALIAS>" for the check */
int count;
sds *ar = sdssplitlen(name, strlen(name), "|", 1, &count);
serverAssert(count == 2); /* Already validated */
isdup = configExists(ar[0]) ||
configExists(ar[1]) ||
(sdscmp(ar[0], ar[1]) == 0);
sdsfreesplitres(ar, count);
} else {
sds _name = sdsnew(name);
isdup = configExists(_name);
sdsfree(_name);
}
} else {
if (moduleVerifyResourceName(name)) {
errno = EINVAL;
return REDISMODULE_ERR;
}
sds fullname = sdscatfmt(sdsempty(), "%s.%s", module->name, name);
isdup = configExists(fullname);
sdsfree(fullname);
}
if (isdup) {
serverLog(LL_WARNING, "Configuration by the name: %s already registered", name);
errno = EALREADY;
return REDISMODULE_ERR;
......@@ -12886,12 +13040,14 @@ int RM_RegisterStringConfig(RedisModuleCtx *ctx, const char *name, const char *d
if (moduleConfigValidityCheck(module, name, flags, NUMERIC_CONFIG)) {
return REDISMODULE_ERR;
}
ModuleConfig *new_config = createModuleConfig(name, applyfn, privdata, module);
new_config->get_fn.get_string = getfn;
new_config->set_fn.set_string = setfn;
listAddNodeTail(module->module_configs, new_config);
flags = maskModuleConfigFlags(flags);
addModuleStringConfig(module->name, name, flags, new_config, default_val ? sdsnew(default_val) : NULL);
ModuleConfig *mc = createModuleConfig(name, applyfn, privdata, module, flags);
mc->get_fn.get_string = getfn;
mc->set_fn.set_string = setfn;
listAddNodeTail(module->module_configs, mc);
unsigned int cflags = maskModuleConfigFlags(flags);
addModuleStringConfig(sdsdup(mc->name), (mc->alias) ? sdsdup(mc->alias) : NULL,
cflags, mc, default_val ? sdsnew(default_val) : NULL);
return REDISMODULE_OK;
}
 
......@@ -12903,12 +13059,13 @@ int RM_RegisterBoolConfig(RedisModuleCtx *ctx, const char *name, int default_val
if (moduleConfigValidityCheck(module, name, flags, BOOL_CONFIG)) {
return REDISMODULE_ERR;
}
ModuleConfig *new_config = createModuleConfig(name, applyfn, privdata, module);
new_config->get_fn.get_bool = getfn;
new_config->set_fn.set_bool = setfn;
listAddNodeTail(module->module_configs, new_config);
flags = maskModuleConfigFlags(flags);
addModuleBoolConfig(module->name, name, flags, new_config, default_val);
ModuleConfig *mc = createModuleConfig(name, applyfn, privdata, module, flags);
mc->get_fn.get_bool = getfn;
mc->set_fn.set_bool = setfn;
listAddNodeTail(module->module_configs, mc);
unsigned int cflags = maskModuleConfigFlags(flags);
addModuleBoolConfig(sdsdup(mc->name), (mc->alias) ? sdsdup(mc->alias) : NULL,
cflags, mc, default_val);
return REDISMODULE_OK;
}
 
......@@ -12946,9 +13103,9 @@ int RM_RegisterEnumConfig(RedisModuleCtx *ctx, const char *name, int default_val
if (moduleConfigValidityCheck(module, name, flags, ENUM_CONFIG)) {
return REDISMODULE_ERR;
}
ModuleConfig *new_config = createModuleConfig(name, applyfn, privdata, module);
new_config->get_fn.get_enum = getfn;
new_config->set_fn.set_enum = setfn;
ModuleConfig *mc = createModuleConfig(name, applyfn, privdata, module, flags);
mc->get_fn.get_enum = getfn;
mc->set_fn.set_enum = setfn;
configEnum *enum_vals = zmalloc((num_enum_vals + 1) * sizeof(configEnum));
for (int i = 0; i < num_enum_vals; i++) {
enum_vals[i].name = zstrdup(enum_values[i]);
......@@ -12956,9 +13113,11 @@ int RM_RegisterEnumConfig(RedisModuleCtx *ctx, const char *name, int default_val
}
enum_vals[num_enum_vals].name = NULL;
enum_vals[num_enum_vals].val = 0;
listAddNodeTail(module->module_configs, new_config);
flags = maskModuleConfigFlags(flags) | maskModuleEnumConfigFlags(flags);
addModuleEnumConfig(module->name, name, flags, new_config, default_val, enum_vals);
listAddNodeTail(module->module_configs, mc);
unsigned int cflags = maskModuleConfigFlags(flags) | maskModuleEnumConfigFlags(flags);
addModuleEnumConfig(sdsdup(mc->name), (mc->alias) ? sdsdup(mc->alias) : NULL,
cflags, mc, default_val, enum_vals, num_enum_vals);
return REDISMODULE_OK;
}
 
......@@ -12971,13 +13130,15 @@ int RM_RegisterNumericConfig(RedisModuleCtx *ctx, const char *name, long long de
if (moduleConfigValidityCheck(module, name, flags, NUMERIC_CONFIG)) {
return REDISMODULE_ERR;
}
ModuleConfig *new_config = createModuleConfig(name, applyfn, privdata, module);
new_config->get_fn.get_numeric = getfn;
new_config->set_fn.set_numeric = setfn;
listAddNodeTail(module->module_configs, new_config);
ModuleConfig *mc = createModuleConfig(name, applyfn, privdata, module, flags);
mc->get_fn.get_numeric = getfn;
mc->set_fn.set_numeric = setfn;
listAddNodeTail(module->module_configs, mc);
unsigned int numeric_flags = maskModuleNumericConfigFlags(flags);
flags = maskModuleConfigFlags(flags);
addModuleNumericConfig(module->name, name, flags, new_config, default_val, numeric_flags, min, max);
unsigned int cflags = maskModuleConfigFlags(flags);
addModuleNumericConfig(sdsdup(mc->name), (mc->alias) ? sdsdup(mc->alias) : NULL,
cflags, mc, default_val, numeric_flags, min, max);
return REDISMODULE_OK;
}
 
......
......@@ -131,6 +131,7 @@ typedef long long ustime_t;
#define REDISMODULE_CONFIG_MEMORY (1ULL<<7) /* Indicates if this value can be set as a memory value */
#define REDISMODULE_CONFIG_BITFLAGS (1ULL<<8) /* Indicates if this value can be set as a multiple enum values */
#define REDISMODULE_CONFIG_UNPREFIXED (1ULL<<9) /* Provided configuration name won't be prefixed with the module name */
/* StreamID type. */
typedef struct RedisModuleStreamID {
......
......@@ -1590,7 +1590,7 @@ struct redisServer {
dict *moduleapi; /* Exported core APIs dictionary for modules. */
dict *sharedapi; /* Like moduleapi but containing the APIs that
modules share with each other. */
dict *module_configs_queue; /* Dict that stores module configurations from .conf file until after modules are loaded during startup or arguments to loadex. */
dict *module_configs_queue; /* Unmapped configs are queued here, assumed to be module config. Applied after modules are loaded during startup or arguments to loadex. */
list *loadmodule_queue; /* List of modules to load at startup. */
int module_pipe[2]; /* Pipe used to awake the event loop by module threads. */
pid_t child_pid; /* PID of current child */
......@@ -3346,10 +3346,10 @@ void freeServerClientMemUsageBuckets(void);
typedef struct ModuleConfig ModuleConfig;
int performModuleConfigSetFromName(sds name, sds value, const char **err);
int performModuleConfigSetDefaultFromName(sds name, const char **err);
void addModuleBoolConfig(const char *module_name, const char *name, int flags, void *privdata, int default_val);
void addModuleStringConfig(const char *module_name, const char *name, int flags, void *privdata, sds default_val);
void addModuleEnumConfig(const char *module_name, const char *name, int flags, void *privdata, int default_val, configEnum *enum_vals);
void addModuleNumericConfig(const char *module_name, const char *name, int flags, void *privdata, long long default_val, int conf_flags, long long lower, long long upper);
void addModuleBoolConfig(sds name, sds alias, int flags, void *privdata, int default_val);
void addModuleStringConfig(sds name, sds alias, int flags, void *privdata, sds default_val);
void addModuleEnumConfig(sds name, sds alias, int flags, void *privdata, int default_val, configEnum *enum_vals, int num_enum_vals);
void addModuleNumericConfig(sds name, sds alias, int flags, void *privdata, long long default_val, int conf_flags, long long lower, long long upper);
void addModuleConfigApply(list *module_configs, ModuleConfig *module_config);
int moduleConfigApplyConfig(list *module_configs, const char **err, const char **err_arg_name);
int getModuleBoolConfig(ModuleConfig *module_config);
......@@ -3775,6 +3775,7 @@ void configGetCommand(client *c);
void configResetStatCommand(client *c);
void configRewriteCommand(client *c);
void configHelpCommand(client *c);
int configExists(const sds name);
void hincrbyCommand(client *c);
void hincrbyfloatCommand(client *c);
void subscribeCommand(client *c);
......
#include "redismodule.h"
#include <strings.h>
int mutable_bool_val;
int mutable_bool_val, no_prefix_bool, no_prefix_bool2;
int immutable_bool_val;
long long longval;
long long memval;
long long longval, no_prefix_longval;
long long memval, no_prefix_memval;
RedisModuleString *strval = NULL;
int enumval;
RedisModuleString *strval2 = NULL;
int enumval, no_prefix_enumval;
int flagsval;
/* Series of get and set callbacks for each type of config, these rely on the privdata ptr
......@@ -103,6 +104,36 @@ int longlongApplyFunc(RedisModuleCtx *ctx, void *privdata, RedisModuleString **e
return REDISMODULE_OK;
}
RedisModuleString *getStringConfigUnprefix(const char *name, void *privdata) {
REDISMODULE_NOT_USED(name);
REDISMODULE_NOT_USED(privdata);
return strval2;
}
int setStringConfigUnprefix(const char *name, RedisModuleString *new, void *privdata, RedisModuleString **err) {
REDISMODULE_NOT_USED(name);
REDISMODULE_NOT_USED(err);
REDISMODULE_NOT_USED(privdata);
if (strval2) RedisModule_FreeString(NULL, strval2);
RedisModule_RetainString(NULL, new);
strval2 = new;
return REDISMODULE_OK;
}
int getEnumConfigUnprefix(const char *name, void *privdata) {
REDISMODULE_NOT_USED(name);
REDISMODULE_NOT_USED(privdata);
return no_prefix_enumval;
}
int setEnumConfigUnprefix(const char *name, int val, void *privdata, RedisModuleString **err) {
REDISMODULE_NOT_USED(name);
REDISMODULE_NOT_USED(err);
REDISMODULE_NOT_USED(privdata);
no_prefix_enumval = val;
return REDISMODULE_OK;
}
int registerBlockCheck(RedisModuleCtx *ctx, RedisModuleString **argv, int argc) {
REDISMODULE_NOT_USED(argv);
REDISMODULE_NOT_USED(argc);
......@@ -168,6 +199,30 @@ int RedisModule_OnLoad(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
if (RedisModule_RegisterNumericConfig(ctx, "numeric", -1, REDISMODULE_CONFIG_DEFAULT, -5, 2000, getNumericConfigCommand, setNumericConfigCommand, longlongApplyFunc, &longval) == REDISMODULE_ERR) {
return REDISMODULE_ERR;
}
/*** unprefixed and aliased configuration ***/
if (RedisModule_RegisterBoolConfig(ctx, "unprefix-bool|unprefix-bool-alias", 1, REDISMODULE_CONFIG_DEFAULT|REDISMODULE_CONFIG_UNPREFIXED,
getBoolConfigCommand, setBoolConfigCommand, NULL, &no_prefix_bool) == REDISMODULE_ERR) {
return REDISMODULE_ERR;
}
if (RedisModule_RegisterBoolConfig(ctx, "unprefix-noalias-bool", 1, REDISMODULE_CONFIG_DEFAULT|REDISMODULE_CONFIG_UNPREFIXED,
getBoolConfigCommand, setBoolConfigCommand, NULL, &no_prefix_bool2) == REDISMODULE_ERR) {
return REDISMODULE_ERR;
}
if (RedisModule_RegisterNumericConfig(ctx, "unprefix.numeric|unprefix.numeric-alias", -1, REDISMODULE_CONFIG_DEFAULT|REDISMODULE_CONFIG_UNPREFIXED,
-5, 2000, getNumericConfigCommand, setNumericConfigCommand, NULL, &no_prefix_longval) == REDISMODULE_ERR) {
return REDISMODULE_ERR;
}
if (RedisModule_RegisterStringConfig(ctx, "unprefix-string|unprefix.string-alias", "secret unprefix", REDISMODULE_CONFIG_DEFAULT|REDISMODULE_CONFIG_UNPREFIXED,
getStringConfigUnprefix, setStringConfigUnprefix, NULL, NULL) == REDISMODULE_ERR) {
return REDISMODULE_ERR;
}
if (RedisModule_RegisterEnumConfig(ctx, "unprefix-enum|unprefix-enum-alias", 1, REDISMODULE_CONFIG_DEFAULT|REDISMODULE_CONFIG_UNPREFIXED,
enum_vals, int_vals, 5, getEnumConfigUnprefix, setEnumConfigUnprefix, NULL, NULL) == REDISMODULE_ERR) {
return REDISMODULE_ERR;
}
size_t len;
if (argc && !strcasecmp(RedisModule_StringPtrLen(argv[0], &len), "noload")) {
return REDISMODULE_OK;
......@@ -191,5 +246,9 @@ int RedisModule_OnUnload(RedisModuleCtx *ctx) {
RedisModule_FreeString(ctx, strval);
strval = NULL;
}
if (strval2) {
RedisModule_FreeString(ctx, strval2);
strval2 = NULL;
}
return REDISMODULE_OK;
}
......@@ -13,6 +13,17 @@ start_server {tags {"modules"}} {
assert_equal [r config get moduleconfigs.enum] "moduleconfigs.enum one"
assert_equal [r config get moduleconfigs.flags] "moduleconfigs.flags {one two}"
assert_equal [r config get moduleconfigs.numeric] "moduleconfigs.numeric -1"
# Check un-prefixed and aliased configuration
assert_equal [r config get unprefix-bool] "unprefix-bool yes"
assert_equal [r config get unprefix-noalias-bool] "unprefix-noalias-bool yes"
assert_equal [r config get unprefix-bool-alias] "unprefix-bool-alias yes"
assert_equal [r config get unprefix.numeric] "unprefix.numeric -1"
assert_equal [r config get unprefix.numeric-alias] "unprefix.numeric-alias -1"
assert_equal [r config get unprefix-string] "unprefix-string {secret unprefix}"
assert_equal [r config get unprefix.string-alias] "unprefix.string-alias {secret unprefix}"
assert_equal [r config get unprefix-enum] "unprefix-enum one"
assert_equal [r config get unprefix-enum-alias] "unprefix-enum-alias one"
}
test {Config set commands work} {
......@@ -34,6 +45,25 @@ start_server {tags {"modules"}} {
assert_equal [r config get moduleconfigs.flags] "moduleconfigs.flags two"
r config set moduleconfigs.numeric -2
assert_equal [r config get moduleconfigs.numeric] "moduleconfigs.numeric -2"
# Check un-prefixed and aliased configuration
r config set unprefix-bool no
assert_equal [r config get unprefix-bool] "unprefix-bool no"
assert_equal [r config get unprefix-bool-alias] "unprefix-bool-alias no"
r config set unprefix-bool-alias yes
assert_equal [r config get unprefix-bool] "unprefix-bool yes"
assert_equal [r config get unprefix-bool-alias] "unprefix-bool-alias yes"
r config set unprefix.numeric 5
assert_equal [r config get unprefix.numeric] "unprefix.numeric 5"
assert_equal [r config get unprefix.numeric-alias] "unprefix.numeric-alias 5"
r config set unprefix.numeric-alias 6
assert_equal [r config get unprefix.numeric] "unprefix.numeric 6"
r config set unprefix.string-alias "blabla"
assert_equal [r config get unprefix-string] "unprefix-string blabla"
assert_equal [r config get unprefix.string-alias] "unprefix.string-alias blabla"
r config set unprefix-enum two
assert_equal [r config get unprefix-enum] "unprefix-enum two"
assert_equal [r config get unprefix-enum-alias] "unprefix-enum-alias two"
}
test {Config set commands enum flags} {
......@@ -93,11 +123,30 @@ start_server {tags {"modules"}} {
assert_equal [r config get moduleconfigs.enum] "moduleconfigs.enum one"
assert_equal [r config get moduleconfigs.flags] "moduleconfigs.flags {one two}"
assert_equal [r config get moduleconfigs.numeric] "moduleconfigs.numeric -1"
# Check un-prefixed and aliased configuration
assert_equal [r config get unprefix-bool] "unprefix-bool yes"
assert_equal [r config get unprefix-bool-alias] "unprefix-bool-alias yes"
assert_equal [r config get unprefix.numeric] "unprefix.numeric -1"
assert_equal [r config get unprefix.numeric-alias] "unprefix.numeric-alias -1"
assert_equal [r config get unprefix-string] "unprefix-string {secret unprefix}"
assert_equal [r config get unprefix.string-alias] "unprefix.string-alias {secret unprefix}"
assert_equal [r config get unprefix-enum] "unprefix-enum one"
assert_equal [r config get unprefix-enum-alias] "unprefix-enum-alias one"
r module unload moduleconfigs
}
test {test loadex functionality} {
r module loadex $testmodule CONFIG moduleconfigs.mutable_bool no CONFIG moduleconfigs.immutable_bool yes CONFIG moduleconfigs.memory_numeric 2mb CONFIG moduleconfigs.string tclortickle
r module loadex $testmodule CONFIG moduleconfigs.mutable_bool no \
CONFIG moduleconfigs.immutable_bool yes \
CONFIG moduleconfigs.memory_numeric 2mb \
CONFIG moduleconfigs.string tclortickle \
CONFIG unprefix-bool no \
CONFIG unprefix.numeric-alias 123 \
CONFIG unprefix-string abc_def \
assert_not_equal [lsearch [lmap x [r module list] {dict get $x name}] moduleconfigs] -1
assert_equal [r config get moduleconfigs.mutable_bool] "moduleconfigs.mutable_bool no"
assert_equal [r config get moduleconfigs.immutable_bool] "moduleconfigs.immutable_bool yes"
......@@ -107,6 +156,18 @@ start_server {tags {"modules"}} {
assert_equal [r config get moduleconfigs.enum] "moduleconfigs.enum one"
assert_equal [r config get moduleconfigs.flags] "moduleconfigs.flags {one two}"
assert_equal [r config get moduleconfigs.numeric] "moduleconfigs.numeric -1"
# Check un-prefixed and aliased configuration
assert_equal [r config get unprefix-bool] "unprefix-bool no"
assert_equal [r config get unprefix-bool-alias] "unprefix-bool-alias no"
assert_equal [r config get unprefix.numeric] "unprefix.numeric 123"
assert_equal [r config get unprefix.numeric-alias] "unprefix.numeric-alias 123"
assert_equal [r config get unprefix-string] "unprefix-string abc_def"
assert_equal [r config get unprefix.string-alias] "unprefix.string-alias abc_def"
assert_equal [r config get unprefix-enum] "unprefix-enum one"
assert_equal [r config get unprefix-enum-alias] "unprefix-enum-alias one"
}
test {apply function works} {
......@@ -121,9 +182,19 @@ start_server {tags {"modules"}} {
}
test {test double config argument to loadex} {
r module loadex $testmodule CONFIG moduleconfigs.mutable_bool yes CONFIG moduleconfigs.mutable_bool no
assert_equal [r config get moduleconfigs.mutable_bool] "moduleconfigs.mutable_bool no"
r module unload moduleconfigs
r module loadex $testmodule CONFIG moduleconfigs.mutable_bool yes \
CONFIG moduleconfigs.mutable_bool no \
CONFIG unprefix.numeric-alias 1 \
CONFIG unprefix.numeric-alias 2 \
CONFIG unprefix-string blabla
assert_equal [r config get moduleconfigs.mutable_bool] "moduleconfigs.mutable_bool no"
# Check un-prefixed and aliased configuration
assert_equal [r config get unprefix.numeric-alias] "unprefix.numeric-alias 2"
assert_equal [r config get unprefix.numeric] "unprefix.numeric 2"
assert_equal [r config get unprefix-string] "unprefix-string blabla"
assert_equal [r config get unprefix.string-alias] "unprefix.string-alias blabla"
r module unload moduleconfigs
}
test {missing loadconfigs call} {
......@@ -156,6 +227,9 @@ start_server {tags {"modules"}} {
assert_match {*ERR*} $e
assert_equal [r config get configs.test] "configs.test yes"
r module unload configs
# Verify config name and its alias being used together gets failed
catch {[r module loadex $testmodule CONFIG unprefix.numeric 1 CONFIG unprefix.numeric-alias 1]}
assert_match {*ERR*} $e
}
test {test config rewrite with dynamic load} {
......@@ -167,6 +241,10 @@ start_server {tags {"modules"}} {
r config set moduleconfigs.memory_numeric 750
r config set moduleconfigs.enum two
r config set moduleconfigs.flags "four two"
r config set unprefix-bool-alias no
r config set unprefix.numeric 456
r config set unprefix.string-alias "unprefix"
r config set unprefix-enum two
r config rewrite
restart_server 0 true false
# Ensure configs we rewrote are present and that the conf file is readable
......@@ -176,6 +254,17 @@ start_server {tags {"modules"}} {
assert_equal [r config get moduleconfigs.enum] "moduleconfigs.enum two"
assert_equal [r config get moduleconfigs.flags] "moduleconfigs.flags {two four}"
assert_equal [r config get moduleconfigs.numeric] "moduleconfigs.numeric -1"
# Check unprefixed configuration and alias
assert_equal [r config get unprefix-bool] "unprefix-bool no"
assert_equal [r config get unprefix-bool-alias] "unprefix-bool-alias no"
assert_equal [r config get unprefix.numeric] "unprefix.numeric 456"
assert_equal [r config get unprefix.numeric-alias] "unprefix.numeric-alias 456"
assert_equal [r config get unprefix-string] "unprefix-string unprefix"
assert_equal [r config get unprefix.string-alias] "unprefix.string-alias unprefix"
assert_equal [r config get unprefix-enum] "unprefix-enum two"
assert_equal [r config get unprefix-enum-alias] "unprefix-enum-alias two"
r module unload moduleconfigs
}
......@@ -241,6 +330,16 @@ start_server {tags {"modules"}} {
assert_equal [r config get moduleconfigs.flags] "moduleconfigs.flags {two four}"
assert_equal [r config get moduleconfigs.numeric] "moduleconfigs.numeric -1"
assert_equal [r config get moduleconfigs.memory_numeric] "moduleconfigs.memory_numeric 1024"
# Check un-prefixed and aliased configuration
assert_equal [r config get unprefix-bool] "unprefix-bool yes"
assert_equal [r config get unprefix-bool-alias] "unprefix-bool-alias yes"
assert_equal [r config get unprefix.numeric] "unprefix.numeric -1"
assert_equal [r config get unprefix.numeric-alias] "unprefix.numeric-alias -1"
assert_equal [r config get unprefix-string] "unprefix-string {secret unprefix}"
assert_equal [r config get unprefix.string-alias] "unprefix.string-alias {secret unprefix}"
assert_equal [r config get unprefix-enum] "unprefix-enum one"
assert_equal [r config get unprefix-enum-alias] "unprefix-enum-alias one"
}
}
}
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment