chg: dev: refactor view creation/configuration loops in dedicated functions

Refactor a bit of `apply_configuration` by extracting (into respective dedicated function) the logic to build the keystores list, the KASP list as well as creating the view/zones and configuring those. This is the next step of MR !10895 and !10901

While the code is extracted, some global variables has been changed into a function parameters which enable to have a clear view of the dependency of the function, typically, to know if it depends on local configuration object or runtime "production" object. The end goal (not in this MR, but later on) is to move as much as possible initialization logic outside of the exclusive mode. 

As a first step, latest commits move the keystores list, KASP list and view/zones creation outside of the exclusive mode. (The view/zone configuration remain in exclusive mode for now, because of a dependency to the runtime "cachelist". This is the target of a next MR.

For the record; while moving the keystores list, KASP list and view/zone creation doesn't have a significant impact on the time the exclusive mode is taken (from my experiment on a 1M small zones instance); moving `configure_views` did have a _massive_ impact (basically, the time spend in the exclusive mode is then non calculable). Configuring views outside the exclusive mode needs more work, which will be done in future MRs.

See #4673

Merge branch 'colin/refactor-applyconfig' into 'main'

See merge request isc-projects/bind9!10910

bin/named/include/named/globals.h

@@ -93,8 +93,7 @@ EXTERN const char *named_g_conffile	   INIT(NAMED_SYSCONFDIR "/named.conf");
 EXTERN const char *named_g_defaultbindkeys INIT(NULL);
 EXTERN const char *named_g_keyfile	   INIT(NAMED_SYSCONFDIR "/rndc.key");
 
-EXTERN bool named_g_conffileset		    INIT(false);
-EXTERN cfg_aclconfctx_t *named_g_aclconfctx INIT(NULL);
+EXTERN bool named_g_conffileset INIT(false);
 
 /*
  * Misc.

bin/named/include/named/server.h

@@ -40,6 +40,9 @@
 
 #include <named/types.h>
 
+struct cfg_aclconfctx;
+typedef struct cfg_aclconfctx cfg_aclconfctx_t;
+
 /*%
  * Name server state.  Better here than in lots of separate global variables.
  */
@@ -106,6 +109,8 @@ struct named_server {
 
 	isc_signal_t *sighup;
 	isc_signal_t *sigusr1;
+
+	cfg_aclconfctx_t *aclconfctx;
 };
 
 #define NAMED_SERVER_MAGIC    ISC_MAGIC('S', 'V', 'E', 'R')
@@ -416,5 +421,5 @@ named_server_getmemprof(void);
  */
 isc_result_t
 named_register_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj,
-			  const char *plugin_path, const char *parameters,
-			  void *callback_data);
+			  cfg_aclconfctx_t *actx, const char *plugin_path,
+			  const char *parameters, void *callback_data);

bin/named/include/named/zoneconf.h

@@ -84,7 +84,8 @@ named_zone_templateopts(const cfg_obj_t *config, const cfg_obj_t *zoptions);
 
 isc_result_t
 named_zone_loadplugins(dns_zone_t *zone, const cfg_obj_t *config,
-		       const cfg_obj_t *toptions, const cfg_obj_t *zoptions);
+		       const cfg_obj_t *toptions, const cfg_obj_t *zoptions,
+		       cfg_aclconfctx_t *actx);
 /*%<
  * Load plugins that should run for this specific zone. Take care of cleaning
  * up any pre-existing plugins first, if the zone is re-used.
@@ -95,4 +96,5 @@ named_zone_loadplugins(dns_zone_t *zone, const cfg_obj_t *config,
  * \li	'zoptions' to be a valid zone configuration tree
  * \li	'toptions' to be NULL or valid template configuration tree
  * \li	'zoptions' to be NULL or a valid zone configuration tree
+ * \li  'actx' to be NULL (confcheck case only) or a valid acl conf ctx
  */

bin/named/zoneconf.c

@@ -2101,7 +2101,8 @@ named_zone_templateopts(const cfg_obj_t *config, const cfg_obj_t *zoptions) {
 
 isc_result_t
 named_zone_loadplugins(dns_zone_t *zone, const cfg_obj_t *config,
-		       const cfg_obj_t *toptions, const cfg_obj_t *zoptions) {
+		       const cfg_obj_t *toptions, const cfg_obj_t *zoptions,
+		       cfg_aclconfctx_t *actx) {
 	isc_result_t result = ISC_R_SUCCESS;
 	const cfg_obj_t *zpluginlist = NULL;
 	const cfg_obj_t *tpluginlist = NULL;
@@ -2135,14 +2136,14 @@ named_zone_loadplugins(dns_zone_t *zone, const cfg_obj_t *config,
 		ns_plugins_create(zmctx, &hookdata.plugins);
 		dns_zone_setplugins(zone, hookdata.plugins, ns_plugins_free);
 
-		result = cfg_pluginlist_foreach(config, tpluginlist,
+		result = cfg_pluginlist_foreach(config, tpluginlist, actx,
 						named_register_one_plugin,
 						&hookdata);
 		if (result != ISC_R_SUCCESS) {
 			return result;
 		}
 
-		result = cfg_pluginlist_foreach(config, zpluginlist,
+		result = cfg_pluginlist_foreach(config, zpluginlist, actx,
 						named_register_one_plugin,
 						&hookdata);
 	}

bin/tests/system/configloading/tests_configloading.py

@@ -13,8 +13,9 @@
 def test_configloading_log(ns1):
     """
     This test is a "guard/warning" to make sure the named.conf loading
-    (parsing) is done outside of the exclusive mode (so, named is still able to
-    answer queries and operating normally in case of configuration reload). It
+    (parsing), keystore building, kasplist building and view creation is done
+    outside of the exclusive mode (so, named is still able to answer queries
+    and operating normally in case of configuration reload). It
     is currently based on logging, so it's quite brittle.
     """
 
@@ -22,6 +23,9 @@ def test_configloading_log(ns1):
         "load_configuration",
         "parsing user configuration from ",
         "apply_configuration",
+        "apply_configuration: configure_keystores",
+        "apply_configuration: configure_kasplist",
+        "apply_configuration: create_views",
         "loop exclusive mode: starting",
     ]
 

lib/isccfg/check.c

@@ -2940,12 +2940,14 @@ struct check_one_plugin_data {
  */
 static isc_result_t
 check_one_plugin(const cfg_obj_t *config, const cfg_obj_t *obj,
-		 const char *plugin_path, const char *parameters,
-		 void *callback_data) {
+		 cfg_aclconfctx_t *actx, const char *plugin_path,
+		 const char *parameters, void *callback_data) {
 	struct check_one_plugin_data *data = callback_data;
 	char full_path[PATH_MAX];
 	isc_result_t result = ISC_R_SUCCESS;
 
+	UNUSED(actx);
+
 	result = ns_plugin_expandpath(plugin_path, full_path,
 				      sizeof(full_path));
 	if (result != ISC_R_SUCCESS) {
@@ -2978,7 +2980,7 @@ check_plugins(const cfg_obj_t *plugins, const cfg_obj_t *config,
 		.check_result = &result,
 	};
 
-	(void)cfg_pluginlist_foreach(config, plugins, check_one_plugin,
+	(void)cfg_pluginlist_foreach(config, plugins, actx, check_one_plugin,
 				     &check_one_plugin_data);
 
 	return result;

lib/isccfg/include/isccfg/cfg.h

@@ -39,6 +39,8 @@
  *** Types
  ***/
 
+typedef struct cfg_aclconfctx cfg_aclconfctx_t;
+
 /*%
  * A configuration parser.
  */
@@ -586,11 +588,9 @@ const char *
 cfg_map_nextclause(const cfg_type_t *map, const void **clauses,
 		   unsigned int *idx);
 
-typedef isc_result_t(pluginlist_cb_t)(const cfg_obj_t *config,
-				      const cfg_obj_t *obj,
-				      const char      *plugin_path,
-				      const char      *parameters,
-				      void	      *callback_data);
+typedef isc_result_t(pluginlist_cb_t)(
+	const cfg_obj_t *config, const cfg_obj_t *obj, cfg_aclconfctx_t *actx,
+	const char *plugin_path, const char *parameters, void *callback_data);
 /*%<
  * Function prototype for the callback used with cfg_pluginlist_foreach().
  * Called once for each element of the list passed to cfg_pluginlist_foreach().
@@ -606,7 +606,8 @@ typedef isc_result_t(pluginlist_cb_t)(const cfg_obj_t *config,
 
 isc_result_t
 cfg_pluginlist_foreach(const cfg_obj_t *config, const cfg_obj_t *list,
-		       pluginlist_cb_t *callback, void *callback_data);
+		       cfg_aclconfctx_t *actx, pluginlist_cb_t *callback,
+		       void *callback_data);
 /*%<
  * For every "plugin" stanza present in 'list' (which in turn is a part of
  * 'config'), invoke the given 'callback', passing 'callback_data' to it along

lib/isccfg/parser.c

@@ -3945,7 +3945,8 @@ cleanup:
 
 isc_result_t
 cfg_pluginlist_foreach(const cfg_obj_t *config, const cfg_obj_t *list,
-		       pluginlist_cb_t *callback, void *callback_data) {
+		       cfg_aclconfctx_t *actx, pluginlist_cb_t *callback,
+		       void *callback_data) {
 	isc_result_t result = ISC_R_SUCCESS;
 
 	REQUIRE(config != NULL);
@@ -3975,7 +3976,7 @@ cfg_pluginlist_foreach(const cfg_obj_t *config, const cfg_obj_t *list,
 			parameters = cfg_obj_asstring(obj);
 		}
 
-		result = callback(config, obj, library, parameters,
+		result = callback(config, obj, actx, library, parameters,
 				  callback_data);
 		if (result != ISC_R_SUCCESS) {
 			break;