config: Change @group includes to be merged into parsed config immediately

This change expands the group settings into the settings tree using a new
low-priority CONFIG_PARSER_CHANGE_GROUP change_counter. Settings with this
counter are included in all checks, but they are not exported as part of the
configuration (to allow overriding the entire groups in cli/userdb).

This fixes using for example:

@metric_defaults = backend
metric auth_failures {
  fields = foo
}

Which otherwise would fail with "metric is required to have metric_filter
setting" error, because it didn't realize the auth_metrics already existed.

diff --git a/src/config/config-parser-private.h b/src/config/config-parser-private.h
index 7b3ab118ab7b0e50714eb854eeb9e06e225b9080..ed78e6f71fbe31ce133bd24ba811a09ae072cce8 100644 (file)
--- a/src/config/config-parser-private.h
+++ b/src/config/config-parser-private.h
@@ -74,6 +74,9 @@ struct input_stack {
        unsigned int linenum;
 };
 
+HASH_TABLE_DEFINE_TYPE(include_group, const char *,
+                      struct config_include_group_filters *);
+
 struct config_parser_context {
        pool_t pool;
        const char *path;
@@ -83,6 +86,7 @@ struct config_parser_context {
        ARRAY(struct config_filter_parser *) all_filter_parsers;
        HASH_TABLE(struct config_filter *,
                   struct config_filter_parser *) all_filter_parsers_hash;
+       HASH_TABLE_TYPE(include_group) all_include_groups;
        struct config_module_parser *root_module_parsers;
        struct config_section_stack *cur_section;
        struct input_stack *cur_input;

diff --git a/src/config/config-parser.c b/src/config/config-parser.c
index 5936ced86f61a9413a2d7ff363f003a596a28894..32fed9f7d875bfa29e060b2c4c99576158f8382d 100644 (file)
--- a/src/config/config-parser.c
+++ b/src/config/config-parser.c
@@ -60,7 +60,7 @@ struct config_parsed {
        ARRAY_TYPE(config_path) seen_paths;
        ARRAY_TYPE(const_string) errors;
        HASH_TABLE(const char *, const struct setting_define *) key_hash;
-       HASH_TABLE(const char *, struct config_include_group_filters *) include_groups;
+       HASH_TABLE_TYPE(include_group) include_groups;
 };
 
 ARRAY_DEFINE_TYPE(setting_parser_info_p, const struct setting_parser_info *);
@@ -1434,6 +1434,23 @@ config_filter_add_new_filter(struct config_parser_context *ctx,
                ctx->cur_section->filter_parser =
                        config_add_new_parser(ctx, &filter,
                                              ctx->cur_section->filter_parser);
+               if (key[0] == SETTINGS_INCLUDE_GROUP_PREFIX) {
+                       /* This is a group filter's root (which may have child
+                          filters) */
+                       const char *include_group = key + 1;
+                       struct config_include_group_filters *group =
+                               hash_table_lookup(ctx->all_include_groups,
+                                                 include_group);
+                       if (group == NULL) {
+                               group = p_new(ctx->pool,
+                                             struct config_include_group_filters, 1);
+                               group->label = p_strdup(ctx->pool, include_group);
+                               p_array_init(&group->filters, ctx->pool, 4);
+                               hash_table_insert(ctx->all_include_groups,
+                                                 group->label, group);
+                       }
+                       array_push_back(&group->filters, &ctx->cur_section->filter_parser);
+               }
        }
        ctx->cur_section->is_filter = TRUE;
 
@@ -2171,88 +2188,8 @@ static void config_filters_merge(struct config_parser_context *ctx,
 
        array_append_zero(&ctx->all_filter_parsers);
        array_pop_back(&ctx->all_filter_parsers);
-       config->filter_parsers = array_front(&ctx->all_filter_parsers);
-}
-
-static int
-config_parse_finish_includes(struct config_parser_context *ctx,
-                            struct config_parsed *config, bool expand,
-                            const char **error_r)
-{
-       hash_table_create(&config->include_groups, config->pool, 0,
-                         str_hash, strcmp);
-
-       for (unsigned int i = 0; config->filter_parsers[i] != NULL; i++) {
-               struct config_filter_parser *filter = config->filter_parsers[i];
-
-               if (!filter->filter.filter_name_array ||
-                   filter->filter.filter_name[0] != SETTINGS_INCLUDE_GROUP_PREFIX)
-                       continue;
-
-               /* This is a group filter's root (which may have child
-                  filters) */
-               T_BEGIN {
-                       const char *include_group =
-                               t_strcut(filter->filter.filter_name + 1, '/');
-                       struct config_include_group_filters *group =
-                               hash_table_lookup(config->include_groups,
-                                                 include_group);
-                       if (group == NULL) {
-                               group = p_new(config->pool,
-                                             struct config_include_group_filters, 1);
-                               group->label = p_strdup(config->pool,
-                                                       include_group);
-                               p_array_init(&group->filters, config->pool, 4);
-                               hash_table_insert(config->include_groups,
-                                                 group->label, group);
-                       }
-                       array_push_back(&group->filters, &filter);
-               } T_END;
-       }
-
-       for (unsigned int i = 0; config->filter_parsers[i] != NULL; i++) {
-               struct config_filter_parser *filter = config->filter_parsers[i];
-               struct config_include_group *include_group;
-
-               if (!array_is_created(&filter->include_groups))
-                       continue;
-
-               array_foreach_modifiable(&filter->include_groups, include_group) {
-                       struct config_include_group_filters *group =
-                               hash_table_lookup(config->include_groups,
-                                                 include_group->label);
-                       if (group == NULL) {
-                               *error_r = t_strdup_printf(
-                                       "Error in configuration file %s line %d: "
-                                       "Unknown group label @%s",
-                                       include_group->last_path,
-                                       include_group->last_linenum,
-                                       include_group->label);
-                               return -1;
-                       }
-
-                       struct config_filter_parser *include_filter =
-                               group_find_name(group, include_group->name);
-                       if (include_filter == NULL) {
-                               *error_r = t_strdup_printf(
-                                       "Error in configuration file %s line %d: "
-                                       "Unknown group @%s=%s",
-                                       include_group->last_path,
-                                       include_group->last_linenum,
-                                       include_group->label,
-                                       include_group->name);
-                               return -1;
-                       }
-                       if (expand) {
-                               config_filters_merge(ctx, config, filter,
-                                                    include_filter,
-                                                    FALSE, FALSE, 0);
-                       }
-               }
-               if (expand)
-                       array_free(&filter->include_groups);
-       }
-       return 0;
+       if (config != NULL)
+               config->filter_parsers = array_front(&ctx->all_filter_parsers);
 }
 
 static void
@@ -2405,16 +2342,13 @@ config_parse_finish(struct config_parser_context *ctx,
        new_config->filter_parsers = array_front(&ctx->all_filter_parsers);
        new_config->module_parsers = ctx->root_module_parsers;
 
+       new_config->include_groups = ctx->all_include_groups;
+       i_zero(&ctx->all_include_groups);
+
        /* Destroy it here, so config filter tree merging no longer attempts
           to update it. */
        hash_table_destroy(&ctx->all_filter_parsers_hash);
 
-       if (ret == 0) {
-               ret = config_parse_finish_includes(ctx, new_config,
-                       (flags & CONFIG_PARSE_FLAG_MERGE_GROUP_FILTERS) != 0,
-                       error_r);
-       }
-
        if (ret == 0 && dump_filter != NULL)
                config_parse_merge_filters(ctx, new_config, dump_filter);
 
@@ -2699,6 +2633,73 @@ static bool config_parser_get_version(struct config_parser_context *ctx,
        return TRUE;
 }
 
+static struct config_filter_parser *
+config_filter_parser_replace_parent(pool_t pool,
+                                   const struct config_filter_parser *src,
+                                   struct config_filter_parser *parent)
+{
+       struct config_filter_parser **p, *dest =
+               p_new(pool, struct config_filter_parser, 1);
+       *dest = *src;
+       dest->parent = parent;
+       dest->filter.parent = parent == NULL ? NULL : &parent->filter;
+
+       /* Fix the parent pointer in children also */
+       for (p = &dest->children_head; *p != NULL; p = &(*p)->next) {
+               *p = config_filter_parser_replace_parent(pool, *p, dest);
+               dest->children_tail = *p;
+       }
+       return dest;
+}
+
+static int
+config_parser_include_merge(struct config_parser_context *ctx,
+                           const struct config_include_group *include_group,
+                           const char **error_r)
+{
+       struct config_include_group_filters *group =
+               hash_table_lookup(ctx->all_include_groups,
+                                 include_group->label);
+       if (group == NULL) {
+               *error_r = t_strdup_printf("Unknown group label @%s",
+                                          include_group->label);
+               return -1;
+       }
+
+       struct config_filter_parser *include_filter =
+               group_find_name(group, include_group->name);
+       if (include_filter == NULL) {
+               *error_r = t_strdup_printf("Unknown group @%s=%s",
+                       include_group->label, include_group->name);
+               return -1;
+       }
+
+       config_module_parsers_merge(ctx->pool,
+               ctx->cur_section->filter_parser->module_parsers,
+               include_filter->module_parsers, FALSE,
+               CONFIG_PARSER_CHANGE_GROUP);
+
+       struct config_filter_parser *src_filter = include_filter->children_head;
+       for (; src_filter != NULL; src_filter = src_filter->next) {
+               /* replace @group parent with the current section */
+               struct config_filter_parser *dest, *new_src_filter =
+                       config_filter_parser_replace_parent(ctx->pool,
+                               src_filter, ctx->cur_section->filter_parser);
+
+               dest = config_filters_find_child(ctx->cur_section->filter_parser,
+                                                &new_src_filter->filter);
+               if (dest == NULL) {
+                       dest = config_add_new_parser(ctx, &new_src_filter->filter,
+                                                    ctx->cur_section->filter_parser);
+                       dest->filter_required_setting_seen =
+                               new_src_filter->filter_required_setting_seen;
+               }
+               config_filters_merge(ctx, NULL, dest, new_src_filter,
+                                    FALSE, FALSE, CONFIG_PARSER_CHANGE_GROUP);
+       }
+       return 0;
+}
+
 static void
 config_parser_include_add_or_update(struct config_parser_context *ctx,
                                    const char *group, const char *name)
@@ -2706,6 +2707,7 @@ config_parser_include_add_or_update(struct config_parser_context *ctx,
        struct config_filter_parser *filter_parser =
                ctx->cur_section->filter_parser;
        struct config_include_group *include_group = NULL;
+       const char *error;
        bool found = FALSE;
 
        if (!array_is_created(&filter_parser->include_groups))
@@ -2725,6 +2727,9 @@ config_parser_include_add_or_update(struct config_parser_context *ctx,
        include_group->last_path =
                p_strdup(ctx->pool, ctx->cur_input->path);
        include_group->last_linenum = ctx->cur_input->linenum;
+
+       if (config_parser_include_merge(ctx, include_group, &error) < 0)
+               ctx->error = p_strdup(ctx->pool, error);
 }
 
 void config_parser_apply_line(struct config_parser_context *ctx,
@@ -3011,6 +3016,8 @@ int config_parse_file(const char *path, enum config_parse_flags flags,
        p_array_init(&ctx.all_filter_parsers, ctx.pool, 128);
        hash_table_create(&ctx.all_filter_parsers_hash, ctx.pool, 0,
                          config_filter_hash, config_filters_cmp);
+       hash_table_create(&ctx.all_include_groups, ctx.pool, 0,
+                         str_hash, strcmp);
        ctx.cur_section = p_new(ctx.pool, struct config_section_stack, 1);
        /* Global settings filter must be the first. */
        struct config_filter root_filter = { };
@@ -3124,6 +3131,7 @@ prevfile:
        hash_table_destroy(&ctx.seen_settings);
        hash_table_destroy(&ctx.all_keys);
        hash_table_destroy(&ctx.all_filter_parsers_hash);
+       hash_table_destroy(&ctx.all_include_groups);
        str_free(&full_line);
        pool_unref(&ctx.pool);
        return ret < 0 ? ret : 1;

diff --git a/src/config/config-parser.h b/src/config/config-parser.h
index ddac6eb9374c13734e5cc890000a42e4a1887773..231edeeed567ed6b943113e57a8c54391e9ae52d 100644 (file)
--- a/src/config/config-parser.h
+++ b/src/config/config-parser.h
@@ -7,10 +7,11 @@
 
 #define CONFIG_MODULE_DIR MODULEDIR"/settings"
 
+#define CONFIG_PARSER_CHANGE_GROUP 1
 /* change_counter used for default settings created internally */
-#define CONFIG_PARSER_CHANGE_DEFAULTS 1
+#define CONFIG_PARSER_CHANGE_DEFAULTS 2
 /* change_counter used for settings changed by configuration file */
-#define CONFIG_PARSER_CHANGE_EXPLICIT 2
+#define CONFIG_PARSER_CHANGE_EXPLICIT 3
 
 struct config_parsed;
 struct setting_parser_context;
@@ -31,9 +32,6 @@ enum config_parse_flags {
           is stored under filter_name { filter_name_key }. This makes the
           output nicer for the human-readable doveconf. */
        CONFIG_PARSE_FLAG_PREFIXES_IN_FILTERS = BIT(7),
-       /* Expand include @groups after parsing settings. This can be useful
-          for doveconf output. */
-       CONFIG_PARSE_FLAG_MERGE_GROUP_FILTERS = BIT(8),
        /* Merge default filters with non-default filters. This can be useful
           for doveconf output. */
        CONFIG_PARSE_FLAG_MERGE_DEFAULT_FILTERS = BIT(9),

diff --git a/src/config/config-request.c b/src/config/config-request.c
index 1c79e3703997a137a85bbe6929e7c4507a14c803..2357f21f764490eeffe88095a65c1383219a5253 100644 (file)
--- a/src/config/config-request.c
+++ b/src/config/config-request.c
@@ -193,8 +193,9 @@ settings_export(struct config_export_context *ctx,
                        dump_default = TRUE;
                        break;
                case CONFIG_DUMP_SCOPE_SET_AND_DEFAULT_OVERRIDES:
-                       if (change_value == 0) {
-                               /* setting is completely unchanged */
+                       if (change_value < CONFIG_PARSER_CHANGE_DEFAULTS) {
+                               /* setting is completely unchanged, or it's
+                                  from an included group. */
                                continue;
                        }
                        dump_default = TRUE;
@@ -281,6 +282,10 @@ settings_export(struct config_export_context *ctx,
                                /* default not changed by old version checks */
                                str_append(ctx->value,
                                        module_parser->settings[define_idx].str);
+                       } else if (module_parser->change_counters[define_idx] ==
+                                  CONFIG_PARSER_CHANGE_GROUP) {
+                               str_append(ctx->value,
+                                       module_parser->settings[define_idx].str);
                        } else {
                                str_append_str(ctx->value, default_str);
                        }

diff --git a/src/config/doveconf.c b/src/config/doveconf.c
index 0131a09f490da0621b121d542e5793ed61f28771..2f66d05b6b9541de12372f30269023d168d1c691 100644 (file)
--- a/src/config/doveconf.c
+++ b/src/config/doveconf.c
@@ -1198,8 +1198,7 @@ int main(int argc, char *argv[])
                setting_name_filters = argv+optind;
                if (scope == CONFIG_DUMP_SCOPE_ALL_WITHOUT_HIDDEN)
                        scope = CONFIG_DUMP_SCOPE_ALL_WITH_HIDDEN;
-               flags |= CONFIG_PARSE_FLAG_MERGE_GROUP_FILTERS |
-                       CONFIG_PARSE_FLAG_MERGE_DEFAULT_FILTERS;
+               flags |= CONFIG_PARSE_FLAG_MERGE_DEFAULT_FILTERS;
                /* Named filters are a bit troublesome here. For example we
                   can have imapc { ... } named filter, and imapc_master_user
                   setting, which is normally written by doveconf as