[Asterisk-code-review] res sorcery config: Allow configuration section to be used b... (asterisk[13])
Joshua Colp
asteriskteam at digium.com
Wed Jul 18 12:40:35 CDT 2018
Joshua Colp has uploaded this change for review. ( https://gerrit.asterisk.org/9514
Change subject: res_sorcery_config: Allow configuration section to be used based on name.
......................................................................
res_sorcery_config: Allow configuration section to be used based on name.
A problem I've seen countless times is a global or system section
for PJSIP not getting applied. This is inevitably the result of
the "type=" line missing. This change alleviates that problem.
The ability to specify an explicit section name has been
added to res_sorcery_config. If the configured section
name matches this and there are no unknown things configured
the section is taken as being for the given type.
ASTERISK-27972
Change-Id: Ie22723663c1ddd24f869af8c9b4c1b59e2476893
---
M res/res_pjsip.c
M res/res_pjsip/config_global.c
M res/res_pjsip/config_system.c
M res/res_sorcery_config.c
4 files changed, 100 insertions(+), 11 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/14/9514/1
diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index c440454..e087564 100644
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -1661,7 +1661,7 @@
</description>
</configOption>
<configOption name="type">
- <synopsis>Must be of type 'system'.</synopsis>
+ <synopsis>Must be of type 'system' UNLESS the object name is 'system'.</synopsis>
</configOption>
</configObject>
<configObject name="global">
@@ -1708,7 +1708,7 @@
twice the unidentified_request_period are pruned.</synopsis>
</configOption>
<configOption name="type">
- <synopsis>Must be of type 'global'.</synopsis>
+ <synopsis>Must be of type 'global' UNLESS the object name is 'global'.</synopsis>
</configOption>
<configOption name="user_agent" default="Asterisk <Asterisk Version>">
<synopsis>Value used in User-Agent header for SIP requests and Server header for SIP responses.</synopsis>
diff --git a/res/res_pjsip/config_global.c b/res/res_pjsip/config_global.c
index fc1227d..2a80312 100644
--- a/res/res_pjsip/config_global.c
+++ b/res/res_pjsip/config_global.c
@@ -491,7 +491,7 @@
snprintf(default_useragent, sizeof(default_useragent), "%s %s",
DEFAULT_USERAGENT_PREFIX, ast_get_version());
- ast_sorcery_apply_default(sorcery, "global", "config", "pjsip.conf,criteria=type=global");
+ ast_sorcery_apply_default(sorcery, "global", "config", "pjsip.conf,criteria=type=global,single_object=yes,explicit_name=global");
if (ast_sorcery_object_register(sorcery, "global", global_alloc, NULL, global_apply)) {
return -1;
diff --git a/res/res_pjsip/config_system.c b/res/res_pjsip/config_system.c
index 65e4e2c..2ff0a65 100644
--- a/res/res_pjsip/config_system.c
+++ b/res/res_pjsip/config_system.c
@@ -175,7 +175,7 @@
return -1;
}
- ast_sorcery_apply_default(system_sorcery, "system", "config", "pjsip.conf,criteria=type=system");
+ ast_sorcery_apply_default(system_sorcery, "system", "config", "pjsip.conf,criteria=type=system,single_object=yes,explicit_name=system");
if (ast_sorcery_object_register_no_reload(system_sorcery, "system", system_alloc, NULL, system_apply)) {
ast_log(LOG_ERROR, "Failed to register with sorcery (is res_sorcery_config loaded?)\n");
diff --git a/res/res_sorcery_config.c b/res/res_sorcery_config.c
index b02000e..a458d91 100644
--- a/res/res_sorcery_config.c
+++ b/res/res_sorcery_config.c
@@ -52,12 +52,18 @@
/*! \brief Any specific variable criteria for considering a defined category for this object */
struct ast_variable *criteria;
+ /*! \brief An explicit name for the configuration section, with it there can be only one */
+ char *explicit_name;
+
/*! \brief Number of buckets to use for objects */
unsigned int buckets;
/*! \brief Enable file level integrity instead of object level */
unsigned int file_integrity:1;
+ /*! \brief Enable enforcement of a single configuration object of this type */
+ unsigned int single_object:1;
+
/*! \brief Filename of the configuration file */
char filename[];
};
@@ -115,6 +121,7 @@
ao2_global_obj_release(config->objects);
ast_rwlock_destroy(&config->objects.lock);
ast_variables_destroy(config->criteria);
+ ast_free(config->explicit_name);
}
static int sorcery_config_fields_cmp(void *obj, void *arg, int flags)
@@ -239,12 +246,66 @@
ao2_callback(config_objects, OBJ_NODATA | OBJ_MULTIPLE, sorcery_config_fields_cmp, ¶ms);
}
-/*! \brief Internal function which determines if criteria has been met for considering an object set applicable */
-static int sorcery_is_criteria_met(struct ast_variable *objset, struct ast_variable *criteria)
+/*! \brief Internal function which determines if a category matches based on explicit name */
+static int sorcery_is_explicit_name_met(const struct ast_sorcery *sorcery, const char *type,
+ struct ast_category *category, struct sorcery_config *config)
+{
+ struct ast_sorcery_object_type *object_type;
+ struct ast_variable *field;
+ int met = 1;
+
+ if (ast_strlen_zero(config->explicit_name) || strcmp(ast_category_get_name(category), config->explicit_name)) {
+ return 0;
+ }
+
+ object_type = ast_sorcery_get_object_type(sorcery, type);
+ if (!object_type) {
+ return 0;
+ }
+
+ /* We iterate the configured fields to see if we don't know any, if we don't then
+ * this is likely not for the given type and we skip it. If it actually is then criteria
+ * may pick it up in which case it would just get rejected as an invalid configuration later.
+ */
+ for (field = ast_category_first(category); field; field = field->next) {
+ if (!ast_sorcery_is_object_field_registered(object_type, field->name)) {
+ met = 0;
+ break;
+ }
+ }
+
+ ao2_ref(object_type, -1);
+
+ return met;
+}
+
+/*! \brief Internal function which determines if a category matches based on criteria */
+static int sorcery_is_criteria_met(struct ast_category *category, struct sorcery_config *config)
{
RAII_VAR(struct ast_variable *, diff, NULL, ast_variables_destroy);
- return (!criteria || (!ast_sorcery_changeset_create(objset, criteria, &diff) && !diff)) ? 1 : 0;
+ if (!config->criteria) {
+ return 0;
+ }
+
+ return (!ast_sorcery_changeset_create(ast_category_first(category), config->criteria, &diff) && !diff) ? 1 : 0;
+}
+
+/*! \brief Internal function which determines if criteria has been met for considering an object set applicable */
+static int sorcery_is_configuration_met(const struct ast_sorcery *sorcery, const char *type,
+ struct ast_category *category, struct sorcery_config *config)
+{
+ if (!config->criteria && ast_strlen_zero(config->explicit_name)) {
+ /* Nothing is configured to allow specific matching, so accept it! */
+ return 1;
+ } else if (sorcery_is_explicit_name_met(sorcery, type, category, config)) {
+ return 1;
+ } else if (sorcery_is_criteria_met(category, config)) {
+ return 1;
+ } else {
+ /* Nothing explicitly matched so reject */
+ return 0;
+ }
}
static void sorcery_config_internal_load(void *data, const struct ast_sorcery *sorcery, const char *type, unsigned int reload)
@@ -271,8 +332,8 @@
if (!config->buckets) {
while ((category = ast_category_browse_filtered(cfg, NULL, category, NULL))) {
- /* If given criteria has not been met skip the category, it is not applicable */
- if (!sorcery_is_criteria_met(ast_category_first(category), config->criteria)) {
+ /* If given configuration has not been met skip the category, it is not applicable */
+ if (!sorcery_is_configuration_met(sorcery, type, category, config)) {
continue;
}
@@ -294,6 +355,16 @@
buckets = config->buckets;
}
+ /* For single object configurations there can only ever be one bucket, if there's more than the single
+ * object requirement has been violated.
+ */
+ if (config->single_object && buckets > 1) {
+ ast_log(LOG_ERROR, "Config file '%s' could not be loaded; configuration contains more than one object of type '%s'\n",
+ config->filename, type);
+ ast_config_destroy(cfg);
+ return;
+ }
+
ast_debug(2, "Using bucket size of '%d' for objects of type '%s' from '%s'\n",
buckets, type, config->filename);
@@ -310,8 +381,8 @@
RAII_VAR(void *, obj, NULL, ao2_cleanup);
id = ast_category_get_name(category);
- /* If given criteria has not been met skip the category, it is not applicable */
- if (!sorcery_is_criteria_met(ast_category_first(category), config->criteria)) {
+ /* If given configurationhas not been met skip the category, it is not applicable */
+ if (!sorcery_is_configuration_met(sorcery, type, category, config)) {
continue;
}
@@ -420,6 +491,24 @@
ao2_ref(config, -1);
return NULL;
}
+ } else if (!strcasecmp(name, "explicit_name")) {
+ ast_free(config->explicit_name);
+ config->explicit_name = ast_strdup(value);
+ if (ast_strlen_zero(config->explicit_name)) {
+ /* This is fatal since it could stop a configuration section from getting applied */
+ ast_log(LOG_ERROR, "Could not create explicit name entry of '%s' for configuration file '%s'\n",
+ value, filename);
+ ao2_ref(config, -1);
+ return NULL;
+ }
+ } else if (!strcasecmp(name, "single_object")) {
+ if (ast_strlen_zero(value)) {
+ ast_log(LOG_ERROR, "Could not set single object value for configuration file '%s' as the value is empty\n",
+ filename);
+ ao2_ref(config, -1);
+ return NULL;
+ }
+ config->single_object = ast_true(value);
} else {
ast_log(LOG_ERROR, "Unsupported option '%s' used for configuration file '%s'\n", name, filename);
}
--
To view, visit https://gerrit.asterisk.org/9514
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie22723663c1ddd24f869af8c9b4c1b59e2476893
Gerrit-Change-Number: 9514
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20180718/b5b26c3c/attachment-0001.html>
More information about the asterisk-code-review
mailing list