From d2bf7f2977b8154b949392c72345b910a6519960 Mon Sep 17 00:00:00 2001 From: kirill reshke Date: Wed, 28 Jul 2021 13:42:00 +0500 Subject: [PATCH] Fix reload of ldap endpoints (#343) * Better indentation for client_max, print ldap endpoint name for ldap-auth route * Better logging in ldap connection initialization * Fix compiler usused arg warning * Fix reload of ldap endpoints: make them a part of od rules struct; Co-authored-by: reshke --- sources/config_reader.c | 32 +++++++++++++++++++------------- sources/extention.h | 13 ------------- sources/ldap.c | 13 +++++++------ sources/ldap_endpoint.h | 2 +- sources/rules.c | 26 +++++++++++++++++++------- sources/rules.h | 11 ++++++++++- sources/server.h | 2 +- 7 files changed, 57 insertions(+), 42 deletions(-) diff --git a/sources/config_reader.c b/sources/config_reader.c index 43ac0459..85671124 100644 --- a/sources/config_reader.c +++ b/sources/config_reader.c @@ -569,12 +569,21 @@ static int od_config_reader_listen(od_config_reader_t *reader) static int od_config_reader_storage(od_config_reader_t *reader) { od_rule_storage_t *storage; - storage = od_rules_storage_add(reader->rules); + storage = od_rules_storage_allocate(); if (storage == NULL) - return -1; + return NULL; + /* name */ if (!od_config_reader_string(reader, &storage->name)) return -1; + + if (od_rules_storage_match(reader->rules, storage->name) != NULL) { + od_config_reader_error(reader, NULL, + "duplicate storage definition: %s", + storage->name); + return -1; + } + od_rules_storage_add(reader->rules, storage); /* { */ if (!od_config_reader_symbol(reader, '{')) return -1; @@ -1010,7 +1019,8 @@ static int od_config_reader_route(od_config_reader_t *reader, char *db_name, &rule->ldap_endpoint_name)) return -1; od_ldap_endpoint_t *le = od_ldap_endpoint_find( - extentions->ldaps, rule->ldap_endpoint_name); + &reader->rules->ldap_endpoints, + rule->ldap_endpoint_name); if (le == NULL) { od_config_reader_error( reader, NULL, @@ -1038,8 +1048,7 @@ static int od_config_reader_route(od_config_reader_t *reader, char *db_name, #ifdef LDAP_FOUND static inline od_retcode_t -od_config_reader_ldap_endpoint(od_config_reader_t *reader, - od_ldap_endpoint_t *ldaps) +od_config_reader_ldap_endpoint(od_config_reader_t *reader) { od_ldap_endpoint_t *ldap_current; ldap_current = od_ldap_endpoint_alloc(); @@ -1052,13 +1061,16 @@ od_config_reader_ldap_endpoint(od_config_reader_t *reader, goto error; } - if (od_ldap_endpoint_find(ldaps, ldap_current->name) != NULL) { + if (od_ldap_endpoint_find(&reader->rules->ldap_endpoints, + ldap_current->name) != NULL) { od_config_reader_error(reader, NULL, "duplicate ldap endpoint definition: %s", ldap_current->name); goto error; } + od_rules_ldap_endpoint_add(reader->rules, ldap_current); + /* { */ if (!od_config_reader_symbol(reader, '{')) { goto error; @@ -1160,11 +1172,6 @@ init: "failed to initialize ldap endpoint"); goto error; } - if (od_ldap_endpoint_add(ldaps, ldap_current) != OK_RESPONSE) { - od_config_reader_error(reader, NULL, - "failed to initialize ldap endpoint"); - goto error; - } /* unreach */ return OK_RESPONSE; @@ -1675,8 +1682,7 @@ static int od_config_reader_parse(od_config_reader_t *reader, /* ldap service */ case OD_LLDAP_ENDPOINT: { #ifdef LDAP_FOUND - rc = od_config_reader_ldap_endpoint(reader, - extentions->ldaps); + rc = od_config_reader_ldap_endpoint(reader); if (rc != OK_RESPONSE) { goto error; } diff --git a/sources/extention.h b/sources/extention.h index 50f62639..16f22ef3 100644 --- a/sources/extention.h +++ b/sources/extention.h @@ -5,9 +5,6 @@ typedef struct od_extention od_extention_t; struct od_extention { od_module_t *modules; -#ifdef LDAP_FOUND - od_ldap_endpoint_t *ldaps; -#endif od_global_t *glob; }; @@ -16,10 +13,6 @@ static inline od_retcode_t od_extentions_init(od_extention_t *extentions) { extentions->modules = malloc(sizeof(od_module_t)); od_modules_init(extentions->modules); -#ifdef LDAP_FOUND - extentions->ldaps = od_ldap_endpoint_alloc(); - //od_ldap_endpoint_init(extentions->ldaps); -#endif return OK_RESPONSE; } @@ -31,12 +24,6 @@ static inline od_retcode_t od_extention_free(od_logger_t *l, od_modules_unload(l, extentions->modules); } -#ifdef LDAP_FOUND - if (extentions->ldaps) { - // TODO : unload alll & free - } -#endif - return OK_RESPONSE; } diff --git a/sources/ldap.c b/sources/ldap.c index 9fb960d9..aeb0192b 100644 --- a/sources/ldap.c +++ b/sources/ldap.c @@ -292,12 +292,13 @@ static inline od_ldap_server_t *od_ldap_server_attach(od_route_t *route, od_server_pool_total(&route->ldap_pool); int pool_size = route->rule->ldap_pool_size; - od_debug(logger, "auth_ldap", NULL, NULL, - "openning new connection to ldap"); - if (pool_size == 0 || connections_in_pool < pool_size) { - // TODO: so limit logic here + // TODO: better limit logic here // We are allowed to spun new server connection + od_debug( + logger, "auth_ldap", NULL, NULL, + "spun new connection to ldap server %s", + route->rule->ldap_endpoint_name); break; } } @@ -517,11 +518,11 @@ od_retcode_t od_ldap_endpoint_add(od_ldap_endpoint_t *ldaps, return OK_RESPONSE; } -od_ldap_endpoint_t *od_ldap_endpoint_find(od_ldap_endpoint_t *ldaps, char *name) +od_ldap_endpoint_t *od_ldap_endpoint_find(od_list_t *ldaps, char *name) { od_list_t *i; - od_list_foreach(&ldaps->link, i) + od_list_foreach(ldaps, i) { od_ldap_endpoint_t *serv = od_container_of(i, od_ldap_endpoint_t, link); diff --git a/sources/ldap_endpoint.h b/sources/ldap_endpoint.h index b8b98b6f..ca0a1025 100644 --- a/sources/ldap_endpoint.h +++ b/sources/ldap_endpoint.h @@ -33,7 +33,7 @@ extern od_retcode_t od_ldap_endpoint_prepare(od_ldap_endpoint_t *); extern od_retcode_t od_ldap_endpoint_add(od_ldap_endpoint_t *ldaps, od_ldap_endpoint_t *target); -extern od_ldap_endpoint_t *od_ldap_endpoint_find(od_ldap_endpoint_t *ldaps, +extern od_ldap_endpoint_t *od_ldap_endpoint_find(od_list_t *ldaps, char *target); extern od_retcode_t od_ldap_endpoint_remove(od_ldap_endpoint_t *ldaps, diff --git a/sources/rules.c b/sources/rules.c index f23542ad..c0afd506 100644 --- a/sources/rules.c +++ b/sources/rules.c @@ -15,6 +15,7 @@ void od_rules_init(od_rules_t *rules) { od_list_init(&rules->storages); + od_list_init(&rules->ldap_endpoints); od_list_init(&rules->rules); } @@ -31,7 +32,7 @@ void od_rules_free(od_rules_t *rules) } } -static inline od_rule_storage_t *od_rules_storage_allocate(void) +od_rule_storage_t *od_rules_storage_allocate(void) { od_rule_storage_t *storage; storage = (od_rule_storage_t *)malloc(sizeof(*storage)); @@ -64,12 +65,16 @@ void od_rules_storage_free(od_rule_storage_t *storage) free(storage); } -od_rule_storage_t *od_rules_storage_add(od_rules_t *rules) +od_ldap_endpoint_t *od_rules_ldap_endpoint_add(od_rules_t *rules, + od_ldap_endpoint_t *ldap) +{ + od_list_append(&rules->ldap_endpoints, &ldap->link); + return ldap; +} + +od_rule_storage_t *od_rules_storage_add(od_rules_t *rules, + od_rule_storage_t *storage) { - od_rule_storage_t *storage; - storage = od_rules_storage_allocate(); - if (storage == NULL) - return NULL; od_list_append(&rules->storages, &storage->link); return storage; } @@ -852,6 +857,7 @@ int od_rules_validate(od_rules_t *rules, od_config_t *config, od_rules_storage_free(storage); } od_list_init(&rules->storages); + od_list_init(&rules->ldap_endpoints); return 0; } @@ -927,7 +933,8 @@ void od_rules_print(od_rules_t *rules, od_logger_t *logger) if (rule->client_max_set) od_log(logger, "rules", NULL, NULL, - " client_max %d", rule->client_max); + " client_max %d", + rule->client_max); od_log(logger, "rules", NULL, NULL, " client_fwd_error %s", od_rules_yes_no(rule->client_fwd_error)); @@ -935,6 +942,11 @@ void od_rules_print(od_rules_t *rules, od_logger_t *logger) " reserve_session_server_connection %s", od_rules_yes_no( rule->reserve_session_server_connection)); + if (rule->ldap_endpoint_name) { + od_log(logger, "rules", NULL, NULL, + " ldap_endpoint_name %s", + rule->ldap_endpoint_name); + } od_log(logger, "rules", NULL, NULL, " storage %s", rule->storage_name); diff --git a/sources/rules.h b/sources/rules.h index 0d75539f..dce8a4ff 100644 --- a/sources/rules.h +++ b/sources/rules.h @@ -139,6 +139,9 @@ struct od_rule { struct od_rules { od_list_t storages; +#ifdef LDAP_FOUND + od_list_t ldap_endpoints; +#endif od_list_t rules; }; @@ -160,9 +163,11 @@ od_rule_t *od_rules_forward(od_rules_t *, char *, char *); od_rule_t *od_rules_match(od_rules_t *, char *, char *, int, int); void od_rules_rule_free(od_rule_t *rule); +od_rule_storage_t *od_rules_storage_allocate(void); /* storage */ -od_rule_storage_t *od_rules_storage_add(od_rules_t *); +od_rule_storage_t *od_rules_storage_add(od_rules_t *rules, + od_rule_storage_t *storage); od_rule_storage_t *od_rules_storage_match(od_rules_t *, char *); @@ -170,6 +175,10 @@ od_rule_storage_t *od_rules_storage_copy(od_rule_storage_t *); void od_rules_storage_free(od_rule_storage_t *); +/* ldap endpoint */ +od_ldap_endpoint_t *od_rules_ldap_endpoint_add(od_rules_t *rules, + od_ldap_endpoint_t *ldap); + /* auth */ od_rule_auth_t *od_rules_auth_add(od_rule_t *); diff --git a/sources/server.h b/sources/server.h index ca4e8282..91a170e4 100644 --- a/sources/server.h +++ b/sources/server.h @@ -123,7 +123,7 @@ static inline int od_server_grac_shutdown(od_server_t *server) return 0; } -static inline int od_server_reload(od_server_t *server) +static inline int od_server_reload(od_attribute_unused() od_server_t *server) { // TODO: set offline to 1 if storage/auth rules chaged return 0;