BUG/MINOR: filters: Reject id-less implicit instances of multi-instance classes

An id is mandatory when a filter class allows several instances per proxy,
and this was verified when a filter-config directive was parsed. But it was
not when an implicit instance was created by flt_add_implicit_instance()
called from another directive: for such a class, an instance without id
could not be distinguished from a second one.

Like for an explicit declaration, the implicit declaration is now reported
and ignored if no id is provided for such a class. All the in-tree
directives creating implicit instances of a multi-instance class (the
cache-store rules) already pass the cache name as id, so nothing changes for
existing configurations.

The filters documentation was updated accordingly.

No backport needed.

diff --git a/doc/internals/api/filters.txt b/doc/internals/api/filters.txt
index 4a90205ddbc53586df4321d71c15fc8e5b0f7fc8..473caeeae073a366607e78e4e34b06aa4bde5e7b 100644 (file)
--- a/doc/internals/api/filters.txt
+++ b/doc/internals/api/filters.txt
@@ -273,7 +273,8 @@ A filter may also be implicitly declared by another directive, for instance by
 the options of the feature it implements. In the new mode, use
 flt_add_implicit_instance() from the directive parser (see section 3.2.1):
 it creates an enabled instance, with an optional id, that an explicit
-"filter-config" declaration may replace. Unlike the legacy mode, an implicit
+"filter-config" declaration may replace. The id is mandatory when the class
+supports several instances per proxy. Unlike the legacy mode, an implicit
 declaration may coexist with other filters in the same proxy.
 
 
@@ -754,7 +755,10 @@ parsing, from the directive that enables the filter :
 
 where 'cls' is the filter class, 'args' the arguments of the filter instance
 (or NULL), and 'file'/'line' the location of the directive. The instance is
-created enabled and marked FLT_INST_F_IMPLICIT.
+created enabled and marked FLT_INST_F_IMPLICIT. The id is mandatory when the
+class supports several instances per proxy (FLT_CLS_FL_MULTI), like for an
+explicit "filter-config" declaration: without an id, the implicit declaration
+is reported and ignored.
 
 
 3.2.2. THE LEGACY KEYWORD PARSER

diff --git a/src/filters.c b/src/filters.c
index 46fa7e72a064e6cf9cbad9146bb4740b3796bba1..a48a822c3b103e3b534a81a0981efc390b67b298 100644 (file)
--- a/src/filters.c
+++ b/src/filters.c
@@ -2359,11 +2359,15 @@ int flt_copy_instances(struct proxy *px, const struct proxy *defpx)
 }
 
 /* Adds an implicit filter instance for the class <cls> to the proxy <px>,
- * with the id <id> (may be NULL) and the arguments <args> (terminated by an
- * empty string). It is used when a filter is implicitly configured by
- * another directive (e.g. with the cache-store action). The instance is
- * marked with FLT_INST_F_IMPLICIT and is enabled, since the directive
- * implies the filter usage.
+ * with the id <id> and the arguments <args> (terminated by an empty
+ * string). It is used when a filter is implicitly configured by another
+ * directive (e.g. with the cache-store action). The instance is marked
+ * with FLT_INST_F_IMPLICIT and is enabled, since the directive implies
+ * the filter usage.
+ *
+ * An id is mandatory when the class supports several instances per proxy:
+ * an id-less instance could not be distinguished from a second one. For
+ * the other classes the id is optional and may be NULL.
  *
  * If a instance with the same class and id already exists, nothing is
  * done: an explicit declaration always wins over an implicit one, and the
@@ -2377,6 +2381,13 @@ int flt_add_implicit_instance(struct proxy *px, struct filter_class *cls, const
        int argc = 0;
        int i;
 
+       if ((cls->flags & FLT_CLS_FL_MULTI) && !id) {
+               ha_warning("config: %s '%s' : implicit filter instance of class '%s' without id ignored, an id is required for this filter class (see %s:%d).\n",
+                          proxy_type_str(px), px->id, cls->name,
+                          file, line);
+               return 0;
+       }
+
        /* a instance already exists for this class and id: explicit or
         * implicit, it wins over a new implicit one.
         */