From: Nick Porter <nick@portercomputing.co.uk>
Date: Mon, 6 Feb 2023 20:56:53 +0000 (+0000)
Subject: v4: Fix up module boostrap sequence (#4883)
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=006bff02adf7bc43cab640957b6b987e0c34ce04;p=thirdparty%2Ffreeradius-server.git

v4: Fix up module boostrap sequence (#4883)

* Bootstrap all modules once module config parsing is complete

This allows submodules access to the fully parsed parent config data
when bootstrapping.

* Use bootstrapping for initialising SQL drivers

Moving these set up steps to bootstrap means the driver is fully set up
by the time the sql module is being instantiated and so it is safe to
start connections.
---

diff --git a/src/lib/server/module.c b/src/lib/server/module.c
index 0c16462bf70..489488efed8 100644
--- a/src/lib/server/module.c
+++ b/src/lib/server/module.c
@@ -342,16 +342,10 @@ int module_submodule_parse(UNUSED TALLOC_CTX *ctx, void *out, void *parent,
 
 	if (unlikely(module_conf_parse(mi, submodule_cs) < 0)) {
 		cf_log_err(submodule_cs, "Failed parsing submodule config");
-	error:
 		talloc_free(mi);
 		return -1;
 	}
 
-	if (unlikely(module_bootstrap(mi) < 0)) {
-		cf_log_err(submodule_cs, "Failed bootstrapping submodule");
-		goto error;
-
-	}
 	*((module_instance_t **)out) = mi;
 
 	return 0;
diff --git a/src/lib/server/module_rlm.c b/src/lib/server/module_rlm.c
index b78901c2ad2..97d70fd8c08 100644
--- a/src/lib/server/module_rlm.c
+++ b/src/lib/server/module_rlm.c
@@ -1051,11 +1051,6 @@ int modules_rlm_bootstrap(CONF_SECTION *root)
 			return -1;
 		}
 
-		if (module_bootstrap(mi) < 0) {
-			cf_log_perr(subcs, "Failed bootstrapping module");
-			goto error;
-		}
-
 		/*
 		 *	Compile the default "actions" subsection, which includes retries.
 		 */
@@ -1066,6 +1061,13 @@ int modules_rlm_bootstrap(CONF_SECTION *root)
 		}
 	}
 
+	/*
+	 *	Having parsed all the modules, bootstrap them.
+	 *	This needs to be after parsing so that submodules can access
+	 *	their parent's fully parsed data.
+	 */
+	modules_bootstrap(rlm_modules);
+
 	cf_log_debug(modules, " } # modules");
 
 	if (fr_command_register_hook(NULL, NULL, modules, module_cmd_list_table) < 0) {
diff --git a/src/modules/rlm_sql/drivers/rlm_sql_mysql/rlm_sql_mysql.c b/src/modules/rlm_sql/drivers/rlm_sql_mysql/rlm_sql_mysql.c
index e3b2766f9d8..f2c55738bd0 100644
--- a/src/modules/rlm_sql/drivers/rlm_sql_mysql/rlm_sql_mysql.c
+++ b/src/modules/rlm_sql/drivers/rlm_sql_mysql/rlm_sql_mysql.c
@@ -167,7 +167,7 @@ static int _sql_socket_destructor(rlm_sql_mysql_conn_t *conn)
 	return 0;
 }
 
-static int mod_instantiate(module_inst_ctx_t const *mctx)
+static int mod_bootstrap(module_inst_ctx_t const *mctx)
 {
 	rlm_sql_mysql_t		*inst = talloc_get_type_abort(mctx->inst->data, rlm_sql_mysql_t);
 	int			warnings;
@@ -851,7 +851,7 @@ rlm_sql_driver_t rlm_sql_mysql = {
 		.onload				= mod_load,
 		.unload				= mod_unload,
 		.config				= driver_config,
-		.instantiate			= mod_instantiate
+		.bootstrap			= mod_bootstrap
 	},
 	.flags				= RLM_SQL_RCODE_FLAGS_ALT_QUERY,
 	.sql_socket_init		= sql_socket_init,
diff --git a/src/modules/rlm_sql/drivers/rlm_sql_oracle/rlm_sql_oracle.c b/src/modules/rlm_sql/drivers/rlm_sql_oracle/rlm_sql_oracle.c
index 843ca33c26a..7f53a9e20d4 100644
--- a/src/modules/rlm_sql/drivers/rlm_sql_oracle/rlm_sql_oracle.c
+++ b/src/modules/rlm_sql/drivers/rlm_sql_oracle/rlm_sql_oracle.c
@@ -153,7 +153,7 @@ static int mod_detach(module_detach_ctx_t const *mctx)
 	return 0;
 }
 
-static int mod_instantiate(module_inst_ctx_t const *mctx)
+static int mod_bootstrap(module_inst_ctx_t const *mctx)
 {
 	rlm_sql_t const		*parent = talloc_get_type_abort(mctx->inst->parent->data, rlm_sql_t);
 	rlm_sql_config_t const	*config = &parent->config;
@@ -617,7 +617,7 @@ rlm_sql_driver_t rlm_sql_oracle = {
 		.magic				= MODULE_MAGIC_INIT,
 		.inst_size			= sizeof(rlm_sql_oracle_t),
 		.config				= driver_config,
-		.instantiate			= mod_instantiate,
+		.bootstrap			= mod_bootstrap,
 		.detach				= mod_detach
 	},
 	.sql_socket_init		= sql_socket_init,
diff --git a/src/modules/rlm_sql/drivers/rlm_sql_postgresql/rlm_sql_postgresql.c b/src/modules/rlm_sql/drivers/rlm_sql_postgresql/rlm_sql_postgresql.c
index 7c70bac0bf5..b84c1e231e4 100644
--- a/src/modules/rlm_sql/drivers/rlm_sql_postgresql/rlm_sql_postgresql.c
+++ b/src/modules/rlm_sql/drivers/rlm_sql_postgresql/rlm_sql_postgresql.c
@@ -544,7 +544,7 @@ static size_t sql_escape_func(request_t *request, char *out, size_t outlen, char
 	return ret;
 }
 
-static int mod_instantiate(module_inst_ctx_t const *mctx)
+static int mod_bootstrap(module_inst_ctx_t const *mctx)
 {
 	rlm_sql_t const		*parent = talloc_get_type_abort(mctx->inst->parent->data, rlm_sql_t);
 	rlm_sql_config_t const	*config = &parent->config;
@@ -675,7 +675,7 @@ rlm_sql_driver_t rlm_sql_postgresql = {
 		.inst_size			= sizeof(rlm_sql_postgresql_t),
 		.onload				= mod_load,
 		.config				= driver_config,
-		.instantiate			= mod_instantiate
+		.bootstrap			= mod_bootstrap
 	},
 	.flags				= RLM_SQL_RCODE_FLAGS_ALT_QUERY,
 	.sql_socket_init		= sql_socket_init,
diff --git a/src/modules/rlm_sql/rlm_sql.c b/src/modules/rlm_sql/rlm_sql.c
index 7db6e2f599b..b6f62d91fa5 100644
--- a/src/modules/rlm_sql/rlm_sql.c
+++ b/src/modules/rlm_sql/rlm_sql.c
@@ -1149,14 +1149,6 @@ static int mod_instantiate(module_inst_ctx_t const *mctx)
 		return -1;
 	}
 
-	/*
-	 *	Ensure the driver is instantiated before attempting connections
-	 */
-	if (module_instantiate(inst->driver_submodule) < 0) {
-		cf_log_err(conf, "Failed instantiating SQL driver");
-		return -1;
-	}
-
 	/*
 	 *	Initialise the connection pool for this instance
 	 */