Merge branch '2139-rrset-order-fixes-and-cleanups' into 'main'

"rrset-order" fixes and cleanups

Closes #2139

See merge request isc-projects/bind9!4195

CHANGES

+5513.	[doc]		The ARM section describing the "rrset-order" statement
+			was rewritten to make it unambiguous and up-to-date with
+			the source code. [GL #2139]
+
+5512.	[bug]		"rrset-order" rules using "order none" were causing
+			named to crash despite named-checkconf treating them as
+			valid. [GL #2139]
+
 5511.	[bug]		'dig -u +yaml' failed to display timestamps to the
 			microsecond. [GL #2190]
 

bin/tests/system/checkconf/good-rrset-order-none.conf

+options {
+	rrset-order {
+		order none;
+	};
+};

bin/tests/system/rrsetorder/clean.sh

@@ -10,7 +10,7 @@
 # information regarding copyright ownership.
 
 rm -f dig.out.test*
-rm -f dig.out.cyclic dig.out.fixed dig.out.random dig.out.nomatch
+rm -f dig.out.cyclic dig.out.fixed dig.out.random dig.out.nomatch dig.out.none
 rm -f dig.out.0 dig.out.1 dig.out.2 dig.out.3
 rm -f dig.out.cyclic2
 rm -f ns2/root.bk

bin/tests/system/rrsetorder/ns1/named.conf.in

@@ -24,6 +24,7 @@ options {
 		name "fixed.example" order fixed;
 		name "random.example" order random;
 		name "cyclic.example" order cyclic;
+		name "none.example" order none;
 		type NS order random;
 		order cyclic;
 	};

bin/tests/system/rrsetorder/ns1/root.db

@@ -42,3 +42,8 @@ nomatch.example.	A	1.2.3.1
 nomatch.example.	A	1.2.3.2
 nomatch.example.	A	1.2.3.3
 nomatch.example.	A	1.2.3.4
+;
+none.example.		A	1.2.3.1
+none.example.		A	1.2.3.2
+none.example.		A	1.2.3.3
+none.example.		A	1.2.3.4

bin/tests/system/rrsetorder/ns2/named.conf.in

@@ -24,6 +24,7 @@ options {
 		name "fixed.example" order fixed;
 		name "random.example" order random;
 		name "cyclic.example" order cyclic;
+		name "none.example" order none;
 		type NS order random;
 		order cyclic;
 	};

bin/tests/system/rrsetorder/ns3/named.conf.in

@@ -24,6 +24,7 @@ options {
 		name "fixed.example" order fixed;
 		name "random.example" order random;
 		name "cyclic.example" order cyclic;
+		name "none.example" order none;
 		type NS order random;
 		order cyclic;
 	};

bin/tests/system/rrsetorder/tests.sh

@@ -142,6 +142,21 @@ if [ $match -lt `expr ${GOOD_RANDOM_NO} / 3` ]; then ret=1; fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
+echo_i "Checking order none (primary)"
+ret=0
+# Fetch the "reference" response and ensure it contains the expected records.
+$DIGCMD @10.53.0.1 none.example > dig.out.none || ret=1
+for i in 1 2 3 4; do
+	grep -F -q 1.2.3.$i dig.out.none || ret=1
+done
+# Ensure 20 further queries result in the same response as the "reference" one.
+for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do
+	$DIGCMD @10.53.0.1 none.example > dig.out.test$i || ret=1
+	$DIFF dig.out.none dig.out.test$i >/dev/null || ret=1
+done
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
 #
 #
 #
@@ -236,6 +251,21 @@ if [ $match -lt `expr ${GOOD_RANDOM_NO} / 3` ]; then ret=1; fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
+echo_i "Checking order none (secondary)"
+ret=0
+# Fetch the "reference" response and ensure it contains the expected records.
+$DIGCMD @10.53.0.2 none.example > dig.out.none || ret=1
+for i in 1 2 3 4; do
+	grep -F -q 1.2.3.$i dig.out.none || ret=1
+done
+# Ensure 20 further queries result in the same response as the "reference" one.
+for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do
+	$DIGCMD @10.53.0.2 none.example > dig.out.test$i || ret=1
+	$DIFF dig.out.none dig.out.test$i >/dev/null || ret=1
+done
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
 echo_i "Shutting down secondary"
 
 (cd ..; $SHELL stop.sh rrsetorder ns2 )
@@ -346,6 +376,21 @@ if [ $match -lt `expr ${GOOD_RANDOM_NO} / 3` ]; then ret=1; fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=`expr $status + $ret`
 
+echo_i "Checking order none (secondary loaded from disk)"
+ret=0
+# Fetch the "reference" response and ensure it contains the expected records.
+$DIGCMD @10.53.0.2 none.example > dig.out.none || ret=1
+for i in 1 2 3 4; do
+	grep -F -q 1.2.3.$i dig.out.none || ret=1
+done
+# Ensure 20 further queries result in the same response as the "reference" one.
+for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do
+	$DIGCMD @10.53.0.2 none.example > dig.out.test$i || ret=1
+	$DIFF dig.out.none dig.out.test$i >/dev/null || ret=1
+done
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
 #
 #
 #
@@ -443,6 +488,21 @@ echo_i "Random selection return $match of ${GOOD_RANDOM_NO} possible orders in 3
 if [ $match -lt `expr ${GOOD_RANDOM_NO} / 3` ]; then ret=1; fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
 
+echo_i "Checking order none (cache)"
+ret=0
+# Fetch the "reference" response and ensure it contains the expected records.
+$DIGCMD @10.53.0.3 none.example > dig.out.none || ret=1
+for i in 1 2 3 4; do
+	grep -F -q 1.2.3.$i dig.out.none || ret=1
+done
+# Ensure 20 further queries result in the same response as the "reference" one.
+for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do
+	$DIGCMD @10.53.0.3 none.example > dig.out.test$i || ret=1
+	$DIFF dig.out.none dig.out.test$i >/dev/null || ret=1
+done
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
+
 echo_i "Checking default order (cache)"
 ret=0
 for i in $GOOD_RANDOM
@@ -468,33 +528,22 @@ done
 echo_i "Default selection return $match of ${GOOD_RANDOM_NO} possible orders in 36 samples"
 if [ $match -lt `expr ${GOOD_RANDOM_NO} / 3` ]; then ret=1; fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
+status=`expr $status + $ret`
 
-echo_i "Checking default order no match in rrset-order (no shuffling)"
+echo_i "Checking default order no match in rrset-order (cache)"
 ret=0
-for i in $GOOD_RANDOM
-do
-	eval match$i=0
+# Fetch the "reference" response and ensure it contains the expected records.
+$DIGCMD @10.53.0.4 nomatch.example > dig.out.nomatch || ret=1
+for i in 1 2 3 4; do
+	grep -F -q 1.2.3.$i dig.out.nomatch || ret=1
 done
-for i in a b c d e f g h i j k l m n o p q r s t u v w x y z 0 1 2 3 4 5 6 7 9
-do
-$DIGCMD @10.53.0.4 nomatch.example > dig.out.nomatch|| ret=1
-	match=0
-	for j in $GOOD_RANDOM
-	do
-		eval "$DIFF dig.out.nomatch dig.out.random.good$j >/dev/null && match$j=1 match=1"
-		if [ $match -eq 1 ]; then break; fi
-	done
-	if [ $match -eq 0 ]; then ret=1; fi
-done
-match=0
-for i in $GOOD_RANDOM
-do
-eval "match=\`expr \$match + \$match$i\`"
+# Ensure 20 further queries result in the same response as the "reference" one.
+for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do
+	$DIGCMD @10.53.0.4 nomatch.example > dig.out.test$i || ret=1
+	$DIFF dig.out.nomatch dig.out.test$i >/dev/null || ret=1
 done
-echo_i "Consistent selection return $match of ${GOOD_RANDOM_NO} possible orders in 36 samples"
-if [ $match -ne 1 ]; then ret=1; fi
 if [ $ret != 0 ]; then echo_i "failed"; fi
-
 status=`expr $status + $ret`
+
 echo_i "exit status: $status"
 [ $status -eq 0 ] || exit 1

doc/arm/reference.rst

@@ -3139,58 +3139,104 @@ are not sorted.
 RRset Ordering
 ^^^^^^^^^^^^^^
 
-When multiple records are returned in an answer, it may be useful to
-configure the order of the records placed into the response. The
-``rrset-order`` statement permits configuration of the ordering of the
-records in a multiple-record response. See also the ``sortlist``
-statement, :ref:`the_sortlist_statement`.
+.. note::
+
+    While alternating the order of records in a DNS response between
+    subsequent queries is a known load distribution technique, certain
+    caveats apply (mostly stemming from caching) which usually make it a
+    suboptimal choice for load balancing purposes when used on its own.
+
+The ``rrset-order`` statement permits configuration of the ordering of
+the records in a multiple-record response. See also:
+:ref:`the_sortlist_statement`.
+
+Each rule in an ``rrset-order`` statement is defined as follows:
+
+::
+
+    [class <class_name>] [type <type_name>] [name "<domain_name>"] order <ordering>
 
-An ``order_spec`` is defined as follows:
+The default qualifiers for each rule are:
 
-[class *class_name*] [type *type_name*] [name "*domain_name*"] order *ordering*
+  - If no ``class`` is specified, the default is ``ANY``.
+  - If no ``type`` is specified, the default is ``ANY``.
+  - If no ``name`` is specified, the default is ``*`` (asterisk).
 
-If no class is specified, the default is ``ANY``. If no type is
-specified, the default is ``ANY``. If no name is specified, the default
-is ``*`` (asterisk).
+``<domain_name>`` only matches the name itself, not any of its
+subdomains.  To make a rule match all subdomains of a given name, a
+wildcard name (``*.<domain_name>``) must be used.  Note that
+``*.<domain_name>`` does *not* match ``<domain_name>`` itself; to
+specify RRset ordering for a name and all of its subdomains, two
+separate rules must be defined: one for ``<domain_name>`` and one for
+``*.<domain_name>``.
 
 The legal values for ``ordering`` are:
 
 ``fixed``
-    Records are returned in the order they are defined in the zone file. This option is only available if BIND is configured with ``--enable-fixed-rrset`` at compile time.
+    Records are returned in the order they are defined in the zone file.
+
+.. note::
+
+    The ``fixed`` option is only available if BIND is configured with
+    ``--enable-fixed-rrset`` at compile time.
 
 ``random``
     Records are returned in a random order.
 
 ``cyclic``
-    Records are returned in a cyclic round-robin order, rotating by one record per query. If BIND is configured with ``--enable-fixed-rrset`` at compile time, the initial ordering of the RRset matches the one specified in the zone file; otherwise the initial ordering is indeterminate.
+    Records are returned in a cyclic round-robin order, rotating by one
+    record per query.
 
 ``none``
-    Records are returned in whatever order they were retrieved from the database. This order is indeterminate, but remains consistent as long as the database is not modified. When no ordering is specified, this is the default.
+    Records are returned in the order they were retrieved from the
+    database. This order is indeterminate, but remains consistent as
+    long as the database is not modified.
 
-For example:
+The default RRset order used depends on whether any ``rrset-order``
+statements are present in the configuration file used by ``named``:
 
-::
+  - If no ``rrset-order`` statement is present in the configuration
+    file, the implicit default is to return all records in ``random``
+    order.
 
-   rrset-order {
-      class IN type A name "host.example.com" order random;
-      order cyclic;
-   };
+  - If any ``rrset-order`` statements are present in the configuration
+    file, but no ordering rule specified in these statements matches a
+    given RRset, the default order for that RRset is ``none``.
 
-causes any responses for type A records in class IN, that have
-``host.example.com`` as a suffix, to always be returned in random
-order. All other records are returned in cyclic order.
+Note that if multiple ``rrset-order`` statements are present in the
+configuration file (at both the ``options`` and ``view`` levels), they
+are *not* combined; instead, the more-specific one (``view``) replaces
+the less-specific one (``options``).
 
-If multiple ``rrset-order`` statements appear, they are not combined;
-the last one applies.
+If multiple rules within a single ``rrset-order`` statement match a
+given RRset, the first matching rule is applied.
 
-By default, records are returned in ``random`` order.
+Example:
 
-.. note::
+::
 
-   "Fixed" ordering of the ``rrset-order`` statement by default is not
-   currently supported in BIND 9. Fixed ordering can be enabled at
-   compile time by specifying "--enable-fixed-rrset" on the "configure"
-   command line.
+    rrset-order {
+        type A name "foo.isc.org" order random;
+        type AAAA name "foo.isc.org" order cyclic;
+        name "bar.isc.org" order fixed;
+        name "*.bar.isc.org" order random;
+        name "*.baz.isc.org" order cyclic;
+    };
+
+With the above configuration, the following RRset ordering is used:
+
+===================    ========    ===========
+QNAME                  QTYPE       RRset Order
+===================    ========    ===========
+``foo.isc.org``        ``A``       ``random``
+``foo.isc.org``        ``AAAA``    ``cyclic``
+``foo.isc.org``        ``TXT``     ``none``
+``sub.foo.isc.org``    all         ``none``
+``bar.isc.org``        all         ``fixed``
+``sub.bar.isc.org``    all         ``random``
+``baz.isc.org``        all         ``none``
+``sub.baz.isc.org``    all         ``cyclic``
+===================    ========    ===========
 
 .. _tuning:
 

lib/dns/order.c

@@ -73,7 +73,8 @@ dns_order_add(dns_order_t *order, const dns_name_t *name,
 	REQUIRE(DNS_ORDER_VALID(order));
 	REQUIRE(mode == DNS_RDATASETATTR_RANDOMIZE ||
 		mode == DNS_RDATASETATTR_FIXEDORDER ||
-		mode == DNS_RDATASETATTR_CYCLIC);
+		mode == DNS_RDATASETATTR_CYCLIC ||
+		mode == DNS_RDATASETATTR_NONE);
 
 	ent = isc_mem_get(order->mctx, sizeof(*ent));