MINOR: filters: Support global instance sequencing

Accept global alongside request and response in filter-sequence. Global
constraints use the same class/instance references and loop checks, but
may order instances whose classes do not share a channel side. Moving a
instance in this order leaves its request and response positions alone.

Add native VTC coverage for global moves, inherited sequences and local
overrides. Reject self-references and cycles in all three orders, including
further lookups after a rejected move, with -conf-BAD instances for the
configuration diagnostics. CLI assertions are added with the inspection
commands in the next patch.

diff --git a/reg-tests/filters/global-order.vtc b/reg-tests/filters/global-order.vtc
new file mode 100644 (file)
index 0000000..d4a606d
--- /dev/null
+++ b/reg-tests/filters/global-order.vtc
@@ -0,0 +1,202 @@
+varnishtest "Global filter instance order is independent of request/response orders"
+
+feature ignore_unknown_macro
+
+haproxy h1 -conf {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+
+    cache store
+        total-max-size 1
+
+    defaults filters
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+        # Deliberately not in global class order. Include request-only,
+        # response-only and bidirectional classes, and multiple instances.
+        filter-config bwlim-out id download default-limit 1m default-period 1s
+        filter-config cache-store id store
+        filter-config trace id b
+        filter-config bwlim-in id upload default-limit 1m default-period 1s
+        filter-config trace id a
+
+    frontend baseline from filters
+        bind "fd@${baseline}"
+        http-request return status 200
+
+    frontend reordered from filters
+        bind "fd@${reordered}"
+        http-request return status 200
+        filter-enable trace
+        filter-disable trace/b
+        # Global moves can cross classes which have no common channel side.
+        # Subsequent moves must still find already-reordered instances.
+        filter-sequence global bwlim-out/download:before(trace/a) trace/b:after(cache-store)
+        filter-sequence global bwlim-in/upload:after(cache-store/store) cache-store/store:before(trace/a)
+        filter-sequence request trace/b:before(trace/a)
+        filter-sequence response trace/a:after(cache-store/store)
+
+    defaults plain
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    frontend empty from plain
+        bind "fd@${empty}"
+        http-request return status 200
+
+    frontend single from plain
+        bind "fd@${single}"
+        filter-config trace id only
+        http-request return status 200
+} -start
+
+# Inherited global sequences are applied to the child's instances, including
+# local overrides, without changing either channel's parsing/class order.
+haproxy h2 -conf {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+
+    defaults ordered
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+        filter-config trace id a
+        filter-config trace id b
+        filter-sequence global trace/b:before(trace/a)
+
+    frontend fe from ordered
+        bind "fd@${fe}"
+        filter-config trace id b enabled
+        filter-config trace id c
+        http-request return status 200
+
+    frontend single from ordered
+        bind "fd@${single}"
+        http-request return status 200
+} -start
+
+# Reject self-references in each order.
+haproxy h4 -conf-BAD {a loop is detected} {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    frontend fe
+        bind "fd@${fe}"
+        filter-config trace id a
+        filter-config trace id b
+        filter-sequence global trace/a:before(trace/a)
+}
+
+haproxy h5 -conf-BAD {a loop is detected} {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    frontend fe
+        bind "fd@${fe}"
+        filter-config trace id a
+        filter-config trace id b
+        filter-sequence request trace/a:before(trace/a)
+}
+
+haproxy h6 -conf-BAD {a loop is detected} {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    frontend fe
+        bind "fd@${fe}"
+        filter-config trace id a
+        filter-config trace id b
+        filter-sequence response trace/a:before(trace/a)
+}
+
+# Keep the trees valid after a rejected cycle. The final sequence in each case
+# exercises a subsequent lookup before error cleanup.
+haproxy h7 -conf-BAD {a loop is detected} {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    frontend fe
+        bind "fd@${fe}"
+        filter-config trace id a
+        filter-config trace id b
+        filter-sequence global trace/a:before(trace/b) trace/b:before(trace/a) trace/a:before(trace)
+}
+
+haproxy h8 -conf-BAD {a loop is detected} {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    frontend fe
+        bind "fd@${fe}"
+        filter-config trace id a
+        filter-config trace id b
+        filter-sequence request trace/a:before(trace/b) trace/b:before(trace/a) trace/a:before(trace)
+}
+
+haproxy h9 -conf-BAD {a loop is detected} {
+    global
+    .if feature(THREAD)
+        thread-groups 1
+    .endif
+
+    defaults
+        mode http
+        timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout client  "${HAPROXY_TEST_TIMEOUT-5s}"
+        timeout server  "${HAPROXY_TEST_TIMEOUT-5s}"
+
+    frontend fe
+        bind "fd@${fe}"
+        filter-config trace id a
+        filter-config trace id b
+        filter-sequence response trace/a:before(trace/b) trace/b:before(trace/a) trace/a:before(trace)
+}

diff --git a/src/filters.c b/src/filters.c
index f55152b614e9f4174dc02d38a5a519dfca11842a..3acdfbab1b14302a56e05611f166183003a1a62e 100644 (file)
--- a/src/filters.c
+++ b/src/filters.c
@@ -77,6 +77,18 @@ static const char *flt_side_name(unsigned int side)
        return "global";
 }
 
+/* Return 0 for an invalid order name. */
+static unsigned int flt_parse_side(const char *name)
+{
+       if (strcmp(name, "global") == 0)
+               return FLT_SIDE_GLOBAL;
+       if (strcmp(name, "request") == 0)
+               return FLT_SIDE_REQ;
+       if (strcmp(name, "response") == 0)
+               return FLT_SIDE_RES;
+       return 0;
+}
+
 static struct list *flt_class_refs(struct proxy *px, unsigned int side)
 {
        if (side == FLT_SIDE_REQ)
@@ -2248,7 +2260,7 @@ static int flt_parse_entity(const char *str, const char **cls_name, const char *
 /*
  * Parses the "filter-sequence" keyword. The syntax is:
  *
- *   filter-sequence {request|response} <inst>:<pos>(<entity>) [<inst>:<pos>(<entity>) ...]
+ *   filter-sequence {global|request|response} <inst>:<pos>(<entity>) [<inst>:<pos>(<entity>) ...]
  *
  * where <pos> is 'before' or 'after': "A:before(B)" means the instance A
  * is evaluated before the entity B (a class or a instance), "A:after(B)"
@@ -2283,16 +2295,13 @@ static int parse_filter_sequence(char **args, int section_type, struct proxy *cu
                memprintf(err, "missing argument for '%s' in %s '%s'.", args[0], proxy_type_str(curpx), curpx->id);
                goto error;
        }
-       if (strcmp(args[1], "request") == 0)
-               side = FLT_SIDE_REQ;
-       else if (strcmp(args[1], "response") == 0)
-               side = FLT_SIDE_RES;
-       else {
-               memprintf(err, "'request' or 'response' expected.");
+       side = flt_parse_side(args[1]);
+       if (!side) {
+               memprintf(err, "'global', 'request' or 'response' expected.");
                goto error;
        }
        if (!*args[2]) {
-               memprintf(err, "missing sequence. The syntax is: filter-sequence {request|response} <inst>:<pos>(<entity>) [<inst>:<pos>(<entity>) ...], with <pos> being 'before' or 'after'.");
+               memprintf(err, "missing sequence. The syntax is: filter-sequence {global|request|response} <inst>:<pos>(<entity>) [<inst>:<pos>(<entity>) ...], with <pos> being 'before' or 'after'.");
                goto error;
        }