allow virtual modules for load-balance and redundant-load-balance

so that they can have second names, and also a key.

Author: alandekok

doc/antora/modules/reference/pages/unlang/load-balance.adoc

@@ -3,30 +3,44 @@
 .Syntax
 [source,unlang]
 ----
-load-balance [ <key> ] {
+load-balance [ <name> ] [ <key> ] {
     [ statements ]
 }
 ----
 
 The `load-balance` section is similar to the `redundant` section
-except that only one module in the subsection is ever called.
+except that only one statement in the subsection is ever called.
 
 In general, the
 xref:unlang/redundant-load-balance.adoc[redundant-load-balance] statement is
 more useful than this one.
 
+<name>:: The instance name of the `load-balance` section.  This name
+is only used when the `load-balance` keyword is used inside of a
+xref:unlang/load-balance.adoc#virtual_modules[virtual module].  In
+other situations, it is not permitted.
++
+The name must be a fixed string.
+
 <key>:: An attribute reference or expansion which will be hashed in
-order to select the statement to execute.
+order to select the statement to execute.  Fixed strings or data are
+not permitted, as that would cause load-balancing to always select the
+same statement.
 +
 The hash will be used to pick a particular statement within the
 `load-balance` section.  This "keyed" load-balance can be used to
 deterministically shard requests across multiple modules.
 +
-When the `<key>` field is omitted, the module is chosen randomly, in a
-"load balanced" manner.
+When the `<key>` field is omitted, or the key is not found, the
+a random statement is chosen instead.
 
 [ statements ]:: One or more `unlang` commands.  Only one of the
 statements is executed.
++
+
+The statements should all be different.  Attribute assignments should
+be placed inside of a xref:unlang/group.adoc[group] section, otherwise
+an error will be generated.
 
 .Examples
 
@@ -38,7 +52,8 @@ load-balance User-Name {
 }
 ----
 
-== load-balance Sections as Modules
+[#virtual_modules]
+== Virtual Modules
 
 It can be useful to use the same `load-balance` section in multiple
 places.  Instead of copying the same text multiple times, a

src/lib/server/cf_file.c

@@ -2633,6 +2633,40 @@ static int parse_input(cf_stack_t *stack)
 
 		fr_skip_whitespace(ptr);
 
+		/*
+		 *	load-balance and redundant-load-balance MUST have a static module name, and MAY have
+		 *	an additional load-balance keyword.
+		 */
+		if ((parent->unlang == CF_UNLANG_MODULES) && (*ptr != '{') &&
+		    ((strcmp(buff[1], "load-balance") == 0) ||
+		     (strcmp(buff[1], "redundant-load-balance") == 0))) {
+			/*
+			 *	The third name could be an attribute name, xlat expansion, etc.
+			 */
+			if (cf_get_token(parent, &ptr, &value_token, buff[3], stack->bufsize,
+					 frame->filename, frame->lineno) < 0) {
+				return -1;
+			}
+
+			fr_skip_whitespace(ptr);
+
+			if (*ptr != '{') {
+				return parse_error(stack, ptr, "Expected '{'");
+			}
+
+			ptr++;
+
+			css = cf_section_alloc(parent, parent, buff[1], buff[2]);
+			if (!css) goto oom;
+
+			css->argc = 1;
+			css->argv = talloc_array(css, char const *, 1);
+			css->argv[0] = talloc_typed_strdup(css->argv, buff[3]);
+			css->argv_quote = talloc_array(css, fr_token_t, 1);
+			css->argv_quote[0] = value_token;
+			goto setup_section;
+		}
+
 		if (*ptr != '{') {
 			return parse_error(stack, ptr, "Missing '{' for configuration section");
 		}
@@ -2800,6 +2834,7 @@ static int parse_input(cf_stack_t *stack)
 		return -1;
 	}
 
+setup_section:
 	cf_filename_set(css, frame->filename);
 	cf_lineno_set(css, frame->lineno);
 	css->name2_quote = name2_token;

src/lib/unlang/load_balance.c

@@ -139,15 +139,14 @@ static unlang_action_t unlang_load_balance(unlang_result_t *p_result, request_t
 		char buffer[1024];
 
 		/*
-		 *	Integer data types let the admin
-		 *	select which frame is being used.
+		 *	Hash the attribute value to select the statement which will be used.
 		 */
 		if (tmpl_is_attr(gext->vpt)) {
 			fr_pair_t *vp;
 
 			slen = tmpl_find_vp(&vp, request, gext->vpt);
 			if (slen < 0) {
-				REDEBUG("Failed finding attribute %s", gext->vpt->name);
+				REDEBUG("Failed finding attribute %s - choosing random statement", gext->vpt->name);
 				goto randomly_choose;
 			}
 
@@ -159,11 +158,13 @@ static unlang_action_t unlang_load_balance(unlang_result_t *p_result, request_t
 			uint8_t *octets = NULL;
 
 			/*
-			 *	If the input is an IP address, prefix, etc., we don't need to convert it to a
-			 *	string.  We can just hash the raw data directly.
+			 *	Get the raw data, and then hash the data.
 			 */
 			slen = tmpl_expand(&octets, buffer, sizeof(buffer), request, gext->vpt);
-			if (slen <= 0) goto randomly_choose;
+			if (slen <= 0) {
+				REDEBUG("Failed expanding %s - choosing random statement", gext->vpt->name);
+				goto randomly_choose;
+			}
 
 			hash = fr_hash(octets, slen);
 
@@ -260,24 +261,27 @@ static unlang_t *compile_load_balance_subsection(unlang_t *parent, unlang_compil
 	 *	Allow for keyed load-balance / redundant-load-balance sections.
 	 */
 	name2 = cf_section_name2(cs);
-
-	/*
-	 *	Inside of the "modules" section, it's a virtual
-	 *	module.  The name is a module name, not a key.
-	 */
-	if (name2) {
-		if (strcmp(cf_section_name1(cf_item_to_section(cf_parent(cs))), "modules") == 0) name2 = NULL;
-	}
-
 	if (name2) {
 		fr_token_t quote;
 		ssize_t slen;
 
+		/*
+		 *	Inside of the "modules" section, it's a virtual module.  The key is the third argument, and
+		 *	the "name2" is the module name, which we ignore here.
+		 */
+		if (strcmp(cf_section_name1(cf_item_to_section(cf_parent(cs))), "modules") == 0) {
+			name2 = cf_section_argv(cs, 0);
+			fr_assert(name2);
+			quote = cf_section_argv_quote(cs, 0);
+
+		} else {
+			quote = cf_section_name2_quote(cs);
+		}
+
 		/*
 		 *	Create the template.  All attributes and xlats are
 		 *	defined by now.
 		 */
-		quote = cf_section_name2_quote(cs);
 		gext = unlang_group_to_load_balance(g);
 		slen = tmpl_afrom_substr(gext, &gext->vpt,
 					 &FR_SBUFF_IN_STR(name2),
@@ -300,6 +304,8 @@ static unlang_t *compile_load_balance_subsection(unlang_t *parent, unlang_compil
 			return NULL;
 		}
 
+		ERROR("SHIT %s %d", gext->vpt->name, gext->vpt->type);
+
 		switch (gext->vpt->type) {
 		default:
 			cf_log_err(cs, "Invalid type in '%s': data will not result in a load-balance key", name2);

src/tests/keywords/radius.conf

@@ -105,11 +105,24 @@ modules {
 	#
 	#  Just check that this can be referred to as "virtual_instantiate.post-auth"
 	#
+
 	load-balance virtual_instantiate {
 		ok
 		updated
 	}
 
+	#
+	#  Name is "keyed-virtual".  The key is Calling-Station-Id.
+	#
+	load-balance keyed-virtual Calling-Station-Id {
+		group {
+			control.Filter-Id := 1
+		}
+		group {
+			control.Filter-Id := 2
+		}
+	}
+
 	test test1 {
 	}
 

src/tests/keywords/virtual-keyed-load-balance

@@ -0,0 +1,10 @@
+#
+#  Just to see if this compiles and runs
+#
+keyed-virtual
+
+if (!control.Filter-Id) {
+	test_fail
+}
+
+success