diff --git a/include/maxscale/config.h b/include/maxscale/config.h index 8e616aae4..6f02ec613 100644 --- a/include/maxscale/config.h +++ b/include/maxscale/config.h @@ -32,6 +32,7 @@ #include #include #include +#include MXS_BEGIN_DECLS @@ -129,13 +130,91 @@ typedef struct } GATEWAY_CONF; +/** + * @brief Creates an empty configuration context + * + * @param section Context name + * @return New context or NULL on memory allocation failure + */ +CONFIG_CONTEXT* config_context_create(const char *section); + +/** + * @brief Free a configuration context + * + * @param context The context to free + */ +void config_context_free(CONFIG_CONTEXT *context); + +/** + * @brief Get a configuration parameter + * + * @param params List of parameters + * @param name Name of parameter to get + * @return The parameter or NULL if the parameter was not found + */ +CONFIG_PARAMETER* config_get_param(CONFIG_PARAMETER* params, const char* name); + +/** + * @brief Add a parameter to a configuration context + * + * @param obj Context where the parameter should be added + * @param key Key to add + * @param value Value for the key + * @return True on success, false on memory allocation error + */ +bool config_add_param(CONFIG_CONTEXT* obj, const char* key, const char* value); + +/** + * @brief Append to an existing parameter + * + * @param obj Configuration context + * @param key Parameter name + * @param value Value to append to the parameter + * @return True on success, false on memory allocation error + */ +bool config_append_param(CONFIG_CONTEXT* obj, const char* key, const char* value); + +/** + * @brief Check if all SSL parameters are defined + * + * Helper function to check whether all of the required SSL parameters are defined + * in the configuration context. The checked parameters are 'ssl', 'ssl_key', + * 'ssl_cert' and 'ssl_ca_cert'. The 'ssl' parameter must also have a value of + * 'required'. + * + * @param obj Configuration context + * @return True if all required parameters are present + */ +bool config_have_required_ssl_params(CONFIG_CONTEXT *obj); + +/** + * @brief Helper function for checking SSL parameters + * + * @param key Parameter name + * @return True if the parameter is an SSL parameter + */ +bool config_is_ssl_parameter(const char *key); + +/** + * @brief Construct an SSL structure + * + * The SSL structure is used by both listeners and servers. + * + * TODO: Rename to something like @c config_construct_ssl + * + * @param obj Configuration context + * @param require_cert Whether certificates are required + * @param error_count Pointer to an int which is incremented for each error + * @return New SSL_LISTENER structure or NULL on error + */ +SSL_LISTENER *make_ssl_structure(CONFIG_CONTEXT *obj, bool require_cert, int *error_count); + char* config_clean_string_list(const char* str); CONFIG_PARAMETER* config_clone_param(const CONFIG_PARAMETER* param); void config_enable_feedback_task(void); void config_disable_feedback_task(void); unsigned long config_get_gateway_id(void); GATEWAY_CONF* config_get_global_options(); -CONFIG_PARAMETER* config_get_param(CONFIG_PARAMETER* params, const char* name); config_param_type_t config_get_paramtype(const CONFIG_PARAMETER* param); bool config_get_valint(int* val, const CONFIG_PARAMETER* param, diff --git a/include/maxscale/monitor.h b/include/maxscale/monitor.h index 55e795922..1db55f8c3 100644 --- a/include/maxscale/monitor.h +++ b/include/maxscale/monitor.h @@ -233,6 +233,22 @@ connect_result_t mon_connect_to_db(MONITOR* mon, MONITOR_SERVERS *database); void mon_log_connect_error(MONITOR_SERVERS* database, connect_result_t rval); void mon_log_state_change(MONITOR_SERVERS *ptr); +/** + * @brief Serialize a monitor to a file + * + * This partially converts @c monitor into an INI format file. Only the servers + * of the monitor are serialized. This allows the monitor to keep monitoring + * the servers that were added at runtime even after a restart. + * + * NOTE: This does not persist the complete monitor configuration and requires + * that an existing monitor configuration is in the main configuration file. + * Changes to monitor parameters are not persisted. + * + * @param monitor Monitor to serialize + * @return False if the serialization of the monitor fails, true if it was successful + */ +bool monitor_serialize_servers(const MONITOR *monitor); + /** * Check if a monitor uses @c servers * @param server Server that is queried diff --git a/include/maxscale/server.h b/include/maxscale/server.h index eb28e1271..51fe48bea 100644 --- a/include/maxscale/server.h +++ b/include/maxscale/server.h @@ -245,6 +245,18 @@ extern bool server_create(const char *name, const char *address, const char *por const char *protocol, const char *authenticator, const char *options); +/** + * @brief Serialize a server to a file + * + * This converts @c server into an INI format file. This allows created servers + * to be persisted to disk. This will replace any existing files with the same + * name. + * + * @param server Server to serialize + * @return False if the serialization of the server fails, true if it was successful + */ +bool server_serialize(const SERVER *server); + /** * @brief Destroy a server * @@ -280,7 +292,5 @@ extern void server_update_port(SERVER *, unsigned short); extern RESULTSET *serverGetList(); extern unsigned int server_map_status(char *str); extern bool server_set_version_string(SERVER* server, const char* string); -extern bool server_is_ssl_parameter(const char *key); -extern void server_update_ssl(SERVER *server, const char *key, const char *value); MXS_END_DECLS diff --git a/include/maxscale/service.h b/include/maxscale/service.h index e8fd7850e..95ac954c6 100644 --- a/include/maxscale/service.h +++ b/include/maxscale/service.h @@ -200,7 +200,7 @@ extern int serviceHasProtocol(SERVICE *service, const char *protocol, const char* address, unsigned short port); extern void serviceAddBackend(SERVICE *, SERVER *); extern void serviceRemoveBackend(SERVICE *, const SERVER *); -extern int serviceHasBackend(SERVICE *, SERVER *); +extern bool serviceHasBackend(SERVICE *, SERVER *); extern void serviceAddRouterOption(SERVICE *, char *); extern void serviceClearRouterOptions(SERVICE *); extern int serviceStart(SERVICE *); @@ -264,4 +264,20 @@ static inline uint64_t service_get_capabilities(const SERVICE *service) */ bool service_server_in_use(const SERVER *server); +/** + * @brief Serialize a service to a file + * + * This partially converts @c service into an INI format file. Only the servers + * of the service are serialized. This allows the service to keep using the servers + * added at runtime even after a restart. + * + * NOTE: This does not persist the complete service configuration and requires + * that an existing service configuration is in the main configuration file. + * Changes to service parameters are not persisted. + * + * @param service Service to serialize + * @return False if the serialization of the service fails, true if it was successful + */ +bool service_serialize_servers(const SERVICE *service); + MXS_END_DECLS diff --git a/server/core/config.c b/server/core/config.c index 43a738397..ea812487f 100644 --- a/server/core/config.c +++ b/server/core/config.c @@ -80,7 +80,6 @@ static void duplicate_context_finish(DUPLICATE_CONTEXT* context); extern int setipaddress(struct in_addr *, char *); static bool process_config_context(CONFIG_CONTEXT *); static bool process_config_update(CONFIG_CONTEXT *); -static void free_config_context(CONFIG_CONTEXT *); static char *config_get_value(CONFIG_PARAMETER *, const char *); static char *config_get_password(CONFIG_PARAMETER *); static const char *config_get_value_string(CONFIG_PARAMETER *, const char *); @@ -90,13 +89,11 @@ static void global_defaults(); static void feedback_defaults(); static bool check_config_objects(CONFIG_CONTEXT *context); static int maxscale_getline(char** dest, int* size, FILE* file); -static SSL_LISTENER *make_ssl_structure(CONFIG_CONTEXT *obj, bool require_cert, int *error_count); int config_truth_value(char *str); int config_get_ifaddr(unsigned char *output); static int config_get_release_string(char* release); FEEDBACK_CONF *config_get_feedback_data(); -void config_add_param(CONFIG_CONTEXT*, char*, char*); bool config_has_duplicate_sections(const char* config, DUPLICATE_CONTEXT* context); int create_new_service(CONFIG_CONTEXT *obj); int create_new_server(CONFIG_CONTEXT *obj); @@ -326,6 +323,20 @@ char* config_clean_string_list(const char* str) return dest; } +CONFIG_CONTEXT* config_context_create(const char *section) +{ + CONFIG_CONTEXT* ctx = (CONFIG_CONTEXT *)MXS_MALLOC(sizeof(CONFIG_CONTEXT)); + if (ctx) + { + ctx->object = MXS_STRDUP_A(section); + ctx->parameters = NULL; + ctx->next = NULL; + ctx->element = NULL; + } + + return ctx; +} + /** * Config item handler for the ini file reader * @@ -340,7 +351,6 @@ ini_handler(void *userdata, const char *section, const char *name, const char *v { CONFIG_CONTEXT *cntxt = (CONFIG_CONTEXT *)userdata; CONFIG_CONTEXT *ptr = cntxt; - CONFIG_PARAMETER *param, *p1; if (strcmp(section, "gateway") == 0 || strcasecmp(section, "MaxScale") == 0) { @@ -368,55 +378,27 @@ ini_handler(void *userdata, const char *section, const char *name, const char *v if (!ptr) { - if ((ptr = (CONFIG_CONTEXT *)MXS_MALLOC(sizeof(CONFIG_CONTEXT))) == NULL) + if ((ptr = config_context_create(section)) == NULL) { return 0; } - ptr->object = MXS_STRDUP_A(section); - ptr->parameters = NULL; ptr->next = cntxt->next; - ptr->element = NULL; cntxt->next = ptr; } - /* Check to see if the parameter already exists for the section */ - p1 = ptr->parameters; - while (p1) + + if (config_get_param(ptr->parameters, name)) { - if (!strcmp(p1->name, name)) + if (!config_append_param(ptr, name, value)) { - char *tmp; - int paramlen = strlen(p1->value) + strlen(value) + 2; - - if ((tmp = MXS_REALLOC(p1->value, sizeof(char) * (paramlen))) == NULL) - { - return 0; - } - strcat(tmp, ","); - strcat(tmp, value); - if ((p1->value = config_clean_string_list(tmp)) == NULL) - { - p1->value = tmp; - MXS_ERROR("[%s] Cleaning configuration parameter failed.", __FUNCTION__); - return 0; - } - MXS_FREE(tmp); - return 1; + return 0; } - p1 = p1->next; } - - if ((param = (CONFIG_PARAMETER *)MXS_MALLOC(sizeof(CONFIG_PARAMETER))) == NULL) + else if (!config_add_param(ptr, name, value)) { return 0; } - param->name = MXS_STRDUP_A(name); - param->value = MXS_STRDUP_A(value); - param->next = ptr->parameters; - param->qfd_param_type = UNDEFINED_TYPE; - ptr->parameters = param; - return 1; } @@ -581,6 +563,47 @@ static bool is_directory(const char *dir) return rval; } +/** + * @brief Check if a directory contains .cnf files + * + * @param path Path to a directory + * @return True if the directory contained one or more .cnf files + */ +static bool contains_cnf_files(const char *path) +{ + bool rval = false; + glob_t matches; + const char suffix[] = "/*.cnf"; + char pattern[strlen(path) + sizeof(suffix)]; + + strcpy(pattern, path); + strcat(pattern, suffix); + int rc = glob(pattern, GLOB_NOSORT, NULL, &matches); + + switch (rc) + { + case 0: + rval = true; + break; + + case GLOB_NOSPACE: + MXS_OOM(); + break; + + case GLOB_ABORTED: + MXS_ERROR("Failed to read directory '%s'", path); + break; + + default: + ss_dassert(rc == GLOB_NOMATCH); + break; + } + + globfree(&matches); + + return rval; +} + /** * @brief Load the specified configuration file for MaxScale * @@ -625,7 +648,23 @@ config_load_and_process(const char* filename, bool (*process_config)(CONFIG_CONT if (is_directory(persist_cnf)) { - rval = config_load_dir(persist_cnf, &dcontext, &ccontext); + DUPLICATE_CONTEXT p_dcontext; + /** + * We need to initialize a second duplicate context for the + * generated configuration files as the monitors and services will + * have duplicate sections. The duplicate sections are used to + * store changes to the list of servers the services and monitors + * use, and thus should not be treated as errors. + */ + if (duplicate_context_init(&p_dcontext)) + { + rval = config_load_dir(persist_cnf, &p_dcontext, &ccontext); + duplicate_context_finish(&p_dcontext); + } + else + { + rval = false; + } } if (rval) @@ -633,11 +672,18 @@ config_load_and_process(const char* filename, bool (*process_config)(CONFIG_CONT if (!check_config_objects(ccontext.next) || !process_config(ccontext.next)) { rval = false; + if (contains_cnf_files(persist_cnf)) + { + MXS_WARNING("One or more generated configurations were found at '%s'. " + "If the error relates to any of the files located there, " + "remove the offending configurations from this directory.", + persist_cnf); + } } } } - free_config_context(ccontext.next); + config_context_free(ccontext.next); duplicate_context_finish(&dcontext); } @@ -876,10 +922,7 @@ config_get_value_string(CONFIG_PARAMETER *params, const char *name) return ""; } - -CONFIG_PARAMETER* config_get_param( - CONFIG_PARAMETER* params, - const char* name) +CONFIG_PARAMETER* config_get_param(CONFIG_PARAMETER* params, const char* name) { while (params) { @@ -1042,22 +1085,15 @@ void free_config_parameter(CONFIG_PARAMETER* p1) } } -/** - * Free a config tree - * - * @param context The configuration data - */ -static void -free_config_context(CONFIG_CONTEXT *context) +void config_context_free(CONFIG_CONTEXT *context) { CONFIG_CONTEXT *obj; - CONFIG_PARAMETER *p1, *p2; while (context) { - MXS_FREE(context->object); - free_config_parameter(context->parameters); obj = context->next; + free_config_parameter(context->parameters); + MXS_FREE(context->object); MXS_FREE(context); context = obj; } @@ -1362,8 +1398,7 @@ free_ssl_structure(SSL_LISTENER *ssl) * @param *error_count An error count which may be incremented * @return SSL_LISTENER structure or NULL */ -static SSL_LISTENER * -make_ssl_structure (CONFIG_CONTEXT *obj, bool require_cert, int *error_count) +SSL_LISTENER* make_ssl_structure (CONFIG_CONTEXT *obj, bool require_cert, int *error_count) { char *ssl, *ssl_version, *ssl_cert, *ssl_key, *ssl_ca_cert, *ssl_cert_verify_depth; int local_errors = 0; @@ -2288,26 +2323,57 @@ unsigned long config_get_gateway_id() return gateway.id; } -void config_add_param(CONFIG_CONTEXT* obj, char* key, char* value) +bool config_add_param(CONFIG_CONTEXT* obj, const char* key, const char* value) { - key = MXS_STRDUP(key); - value = MXS_STRDUP(value); + ss_dassert(config_get_param(obj->parameters, key) == NULL); + bool rval = false; + char *my_key = MXS_STRDUP(key); + char *my_value = MXS_STRDUP(value); + CONFIG_PARAMETER* param = (CONFIG_PARAMETER *)MXS_MALLOC(sizeof(*param)); - CONFIG_PARAMETER* param = (CONFIG_PARAMETER *)MXS_MALLOC(sizeof(CONFIG_PARAMETER)); - - if (!key || !value || !param) + if (my_key && my_value && param) { - MXS_FREE(key); - MXS_FREE(value); + param->name = my_key; + param->value = my_value; + param->qfd_param_type = UNDEFINED_TYPE; + param->next = obj->parameters; + obj->parameters = param; + rval = true; + } + else + { + MXS_FREE(my_key); + MXS_FREE(my_value); MXS_FREE(param); - return; } - param->name = key; - param->value = value; - param->next = obj->parameters; - obj->parameters = param; + return rval; } + +bool config_append_param(CONFIG_CONTEXT* obj, const char* key, const char* value) +{ + CONFIG_PARAMETER *param = config_get_param(obj->parameters, key); + ss_dassert(param); + int paramlen = strlen(param->value) + strlen(value) + 2; + char tmp[paramlen]; + bool rval = false; + + strcpy(tmp, param->value); + strcat(tmp, ","); + strcat(tmp, value); + + char *new_value = config_clean_string_list(tmp); + + if (new_value) + { + MXS_FREE(param->value); + param->value = new_value; + rval = true; + } + + return rval; +} + /** * Return the pointer to the global options for MaxScale. * @return Pointer to the GATEWAY_CONF structure. This is a static structure and @@ -3158,3 +3224,38 @@ int create_new_filter(CONFIG_CONTEXT *obj) return error_count; } + +bool config_have_required_ssl_params(CONFIG_CONTEXT *obj) +{ + CONFIG_PARAMETER *param = obj->parameters; + + return config_get_param(param, "ssl") && + config_get_param(param, "ssl_key") && + config_get_param(param, "ssl_cert") && + config_get_param(param, "ssl_ca_cert") && + strcmp(config_get_value_string(param, "ssl"), "required") == 0; +} + +bool config_is_ssl_parameter(const char *key) +{ + const char *ssl_params[] = + { + "ssl_cert", + "ssl_ca_cert", + "ssl", + "ssl_key", + "ssl_version", + "ssl_cert_verify_depth", + NULL + }; + + for (int i = 0; ssl_params[i]; i++) + { + if (strcmp(key, ssl_params[i]) == 0) + { + return true; + } + } + + return false; +} diff --git a/server/core/monitor.c b/server/core/monitor.c index 94f930a8a..39c2c14f1 100644 --- a/server/core/monitor.c +++ b/server/core/monitor.c @@ -40,6 +40,7 @@ #include #include #include +#include /* * Create declarations of the enum for monitor events and also the array of @@ -244,46 +245,62 @@ monitorStopAll() void monitorAddServer(MONITOR *mon, SERVER *server) { - MONITOR_SERVERS *db = (MONITOR_SERVERS *)MXS_MALLOC(sizeof(MONITOR_SERVERS)); - MXS_ABORT_IF_NULL(db); - - db->server = server; - db->con = NULL; - db->next = NULL; - db->mon_err_count = 0; - db->log_version_err = true; - /** Server status is uninitialized */ - db->mon_prev_status = -1; - /* pending status is updated by get_replication_tree */ - db->pending_status = 0; - - monitor_state_t old_state = mon->state; - - if (old_state == MONITOR_STATE_RUNNING) - { - monitorStop(mon); - } - + bool new_server = true; spinlock_acquire(&mon->lock); - if (mon->databases == NULL) + for (MONITOR_SERVERS *db = mon->databases; db; db = db->next) { - mon->databases = db; - } - else - { - MONITOR_SERVERS *ptr = mon->databases; - while (ptr->next != NULL) + if (db->server == server) { - ptr = ptr->next; + new_server = false; } - ptr->next = db; } + spinlock_release(&mon->lock); - if (old_state == MONITOR_STATE_RUNNING) + if (new_server) { - monitorStart(mon, mon->parameters); + MONITOR_SERVERS *db = (MONITOR_SERVERS *)MXS_MALLOC(sizeof(MONITOR_SERVERS)); + MXS_ABORT_IF_NULL(db); + + db->server = server; + db->con = NULL; + db->next = NULL; + db->mon_err_count = 0; + db->log_version_err = true; + /** Server status is uninitialized */ + db->mon_prev_status = -1; + /* pending status is updated by get_replication_tree */ + db->pending_status = 0; + + monitor_state_t old_state = mon->state; + + if (old_state == MONITOR_STATE_RUNNING) + { + monitorStop(mon); + } + + spinlock_acquire(&mon->lock); + + if (mon->databases == NULL) + { + mon->databases = db; + } + else + { + MONITOR_SERVERS *ptr = mon->databases; + while (ptr->next != NULL) + { + ptr = ptr->next; + } + ptr->next = db; + } + spinlock_release(&mon->lock); + + if (old_state == MONITOR_STATE_RUNNING) + { + monitorStart(mon, mon->parameters); + } } } @@ -1152,3 +1169,86 @@ bool monitor_server_in_use(const SERVER *server) return rval; } + +/** + * Creates a monitor configuration at the location pointed by @c filename + * + * @param monitor Monitor to serialize into a configuration + * @param filename Filename where configuration is written + * @return True on success, false on error + */ +static bool create_monitor_config(const MONITOR *monitor, const char *filename) +{ + int file = open(filename, O_EXCL | O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); + + if (file == -1) + { + char errbuf[MXS_STRERROR_BUFLEN]; + MXS_ERROR("Failed to open file '%s' when serializing monitor '%s': %d, %s", + filename, monitor->name, errno, strerror_r(errno, errbuf, sizeof(errbuf))); + return false; + } + + /** + * Only additional parameters are added to the configuration. This prevents + * duplication or addition of parameters that don't support it. + * + * TODO: Check for return values on all of the dprintf calls + */ + dprintf(file, "[%s]\n", monitor->name); + + if (monitor->databases) + { + dprintf(file, "servers="); + for (MONITOR_SERVERS *db = monitor->databases; db; db = db->next) + { + if (db != monitor->databases) + { + dprintf(file, ","); + } + dprintf(file, "%s", db->server->unique_name); + } + dprintf(file, "\n"); + } + + close(file); + + return true; +} + +bool monitor_serialize_servers(const MONITOR *monitor) +{ + bool rval = false; + char filename[PATH_MAX]; + snprintf(filename, sizeof(filename), "%s/%s.cnf.tmp", get_config_persistdir(), + monitor->name); + + if (unlink(filename) == -1 && errno != ENOENT) + { + char err[MXS_STRERROR_BUFLEN]; + MXS_ERROR("Failed to remove temporary monitor configuration at '%s': %d, %s", + filename, errno, strerror_r(errno, err, sizeof(err))); + } + else if (create_monitor_config(monitor, filename)) + { + char final_filename[PATH_MAX]; + strcpy(final_filename, filename); + + char *dot = strrchr(final_filename, '.'); + ss_dassert(dot); + *dot = '\0'; + + if (rename(filename, final_filename) == 0) + { + rval = true; + } + else + { + char err[MXS_STRERROR_BUFLEN]; + MXS_ERROR("Failed to rename temporary monitor configuration at '%s': %d, %s", + filename, errno, strerror_r(errno, err, sizeof(err))); + } + } + + return rval; +} diff --git a/server/core/server.c b/server/core/server.c index 1faee1076..729f8916a 100644 --- a/server/core/server.c +++ b/server/core/server.c @@ -987,6 +987,8 @@ server_update_address(SERVER *server, char *address) spinlock_acquire(&server_spin); if (server && address) { + MXS_NOTICE("Updated the address of server '%s' from '%s' to '%s'.", + server->unique_name, server->name, address); strcpy(server->name, address); } spinlock_release(&server_spin); @@ -1005,6 +1007,8 @@ server_update_port(SERVER *server, unsigned short port) spinlock_acquire(&server_spin); if (server && port > 0) { + MXS_NOTICE("Updated the port of server '%s' from %d to %d.", + server->unique_name, server->port, port); server->port = port; } spinlock_release(&server_spin); @@ -1082,7 +1086,7 @@ bool server_set_version_string(SERVER* server, const char* string) * @param filename Filename where configuration is written * @return True on success, false on error */ -static bool create_server_config(SERVER *server, const char *filename) +static bool create_server_config(const SERVER *server, const char *filename) { int file = open(filename, O_EXCL | O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); @@ -1182,17 +1186,7 @@ static bool create_server_config(SERVER *server, const char *filename) return true; } -/** - * @brief Serialize a server to a file - * - * This converts @c server into an INI format file. This allows created servers - * to be persisted to disk. This will replace any existing files with the same - * name. - * - * @param server Server to serialize - * @return False if the serialization of the server fails, true if it was successful - */ -static bool server_serialize(SERVER *server) +bool server_serialize(const SERVER *server) { bool rval = false; char filename[PATH_MAX]; @@ -1293,15 +1287,19 @@ bool server_create(const char *name, const char *address, const char *port, server->is_active = true; rval = true; } - else if ((server = server_alloc(name, address, atoi(port), protocol, authenticator, - authenticator_options))) + else { - if (server_serialize(server)) - { - /** server_alloc will add the server to the global list of - * servers so we don't need to manually add it. */ - rval = true; - } + /** + * server_alloc will add the server to the global list of + * servers so we don't need to manually add it. + */ + server = server_alloc(name, address, atoi(port), protocol, + authenticator, authenticator_options); + } + + if (server && server_serialize(server)) + { + rval = true; } } @@ -1354,14 +1352,3 @@ bool server_destroy(SERVER *server) return rval; } - -bool server_is_ssl_parameter(const char *key) -{ - // TODO: Implement this - return false; -} - -void server_update_ssl(SERVER *server, const char *key, const char *value) -{ - // TODO: Implement this -} diff --git a/server/core/service.c b/server/core/service.c index 7fe22d93e..f205338ed 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -43,6 +43,10 @@ #include #include #include +#include +#include +#include +#include #include #include #include @@ -56,12 +60,9 @@ #include #include #include -#include -#include #include #include #include -#include #include #include #include @@ -769,43 +770,46 @@ static SERVER_REF* server_ref_create(SERVER *server) void serviceAddBackend(SERVICE *service, SERVER *server) { - SERVER_REF *new_ref = server_ref_create(server); - - if (new_ref) + if (!serviceHasBackend(service, server)) { - spinlock_acquire(&service->spin); + SERVER_REF *new_ref = server_ref_create(server); - service->n_dbref++; - - if (service->dbref) + if (new_ref) { - SERVER_REF *ref = service->dbref; - SERVER_REF *prev = ref; + spinlock_acquire(&service->spin); - while (ref) + service->n_dbref++; + + if (service->dbref) { - if (ref->server == server) + SERVER_REF *ref = service->dbref; + SERVER_REF *prev = ref; + + while (ref) { - ref->active = true; - break; + if (ref->server == server) + { + ref->active = true; + break; + } + prev = ref; + ref = ref->next; } - prev = ref; - ref = ref->next; - } - if (ref == NULL) - { - /** A new server that hasn't been used by this service */ - atomic_synchronize(); - prev->next = new_ref; + if (ref == NULL) + { + /** A new server that hasn't been used by this service */ + atomic_synchronize(); + prev->next = new_ref; + } } + else + { + atomic_synchronize(); + service->dbref = new_ref; + } + spinlock_release(&service->spin); } - else - { - atomic_synchronize(); - service->dbref = new_ref; - } - spinlock_release(&service->spin); } } @@ -841,7 +845,7 @@ void serviceRemoveBackend(SERVICE *service, const SERVER *server) * @param server The server to add * @return Non-zero if the server is already part of the service */ -int +bool serviceHasBackend(SERVICE *service, SERVER *server) { SERVER_REF *ptr; @@ -2217,3 +2221,85 @@ bool service_server_in_use(const SERVER *server) return rval; } + +/** + * Creates a service configuration at the location pointed by @c filename + * + * @param service Service to serialize into a configuration + * @param filename Filename where configuration is written + * @return True on success, false on error + */ +static bool create_service_config(const SERVICE *service, const char *filename) +{ + int file = open(filename, O_EXCL | O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH); + + if (file == -1) + { + char errbuf[MXS_STRERROR_BUFLEN]; + MXS_ERROR("Failed to open file '%s' when serializing service '%s': %d, %s", + filename, service->name, errno, strerror_r(errno, errbuf, sizeof(errbuf))); + return false; + } + + /** + * Only additional parameters are added to the configuration. This prevents + * duplication or addition of parameters that don't support it. + * + * TODO: Check for return values on all of the dprintf calls + */ + dprintf(file, "[%s]\n", service->name); + if (service->dbref) + { + dprintf(file, "servers="); + for (SERVER_REF *db = service->dbref; db; db = db->next) + { + if (db != service->dbref) + { + dprintf(file, ","); + } + dprintf(file, "%s", db->server->unique_name); + } + dprintf(file, "\n"); + } + + close(file); + + return true; +} + +bool service_serialize_servers(const SERVICE *service) +{ + bool rval = false; + char filename[PATH_MAX]; + snprintf(filename, sizeof(filename), "%s/%s.cnf.tmp", get_config_persistdir(), + service->name); + + if (unlink(filename) == -1 && errno != ENOENT) + { + char err[MXS_STRERROR_BUFLEN]; + MXS_ERROR("Failed to remove temporary service configuration at '%s': %d, %s", + filename, errno, strerror_r(errno, err, sizeof(err))); + } + else if (create_service_config(service, filename)) + { + char final_filename[PATH_MAX]; + strcpy(final_filename, filename); + + char *dot = strrchr(final_filename, '.'); + ss_dassert(dot); + *dot = '\0'; + + if (rename(filename, final_filename) == 0) + { + rval = true; + } + else + { + char err[MXS_STRERROR_BUFLEN]; + MXS_ERROR("Failed to rename temporary service configuration at '%s': %d, %s", + filename, errno, strerror_r(errno, err, sizeof(err))); + } + } + + return rval; +} diff --git a/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c b/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c index 89de78cc5..df89c4b01 100644 --- a/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c +++ b/server/modules/protocol/MySQL/MySQLBackend/mysql_backend.c @@ -617,6 +617,11 @@ gw_read_backend_event(DCB *dcb) gw_reply_on_error(dcb, proto->protocol_auth_state); } } + else if (proto->protocol_auth_state == MXS_AUTH_STATE_CONNECTED && + dcb->ssl_state == SSL_ESTABLISHED) + { + proto->protocol_auth_state = gw_send_backend_auth(dcb); + } } return rc; diff --git a/server/modules/protocol/MySQL/mysql_common.c b/server/modules/protocol/MySQL/mysql_common.c index 5104720be..9873b30ae 100644 --- a/server/modules/protocol/MySQL/mysql_common.c +++ b/server/modules/protocol/MySQL/mysql_common.c @@ -1339,7 +1339,7 @@ mxs_auth_state_t gw_send_backend_auth(DCB *dcb) (dcb->session->state != SESSION_STATE_READY && dcb->session->state != SESSION_STATE_ROUTER_READY) || (dcb->server->server_ssl && - dcb->ssl_state != SSL_HANDSHAKE_FAILED)) + dcb->ssl_state == SSL_HANDSHAKE_FAILED)) { return MXS_AUTH_STATE_FAILED; } diff --git a/server/modules/routing/debugcli/debugcmd.c b/server/modules/routing/debugcli/debugcmd.c index eff2e3bef..0b19e3bb3 100644 --- a/server/modules/routing/debugcli/debugcmd.c +++ b/server/modules/routing/debugcli/debugcmd.c @@ -749,10 +749,12 @@ static void cmd_AddServer(DCB *dcb, void *a, void *b) if (service) { serviceAddBackend(service, server); + service_serialize_servers(service); } else if (monitor) { monitorAddServer(monitor, server); + monitor_serialize_servers(monitor); } const char *target = service ? "service" : "monitor"; @@ -805,10 +807,12 @@ static void cmd_RemoveServer(DCB *dcb, void *a, void *b) if (service) { serviceRemoveBackend(service, server); + service_serialize_servers(service); } else if (monitor) { monitorRemoveServer(monitor, server); + monitor_serialize_servers(monitor); } const char *target = service ? "service" : "monitor"; @@ -980,9 +984,9 @@ static void createServer(DCB *dcb, char *name, char *address, char *port, struct subcommand createoptions[] = { { - "server", 3, 6, createServer, + "server", 2, 6, createServer, "Create a new server", - "Usage: create server NAME HOST PORT [PROTOCOL] [AUTHENTICATOR] [OPTIONS]\n" + "Usage: create server NAME HOST [PORT] [PROTOCOL] [AUTHENTICATOR] [OPTIONS]\n" "Create a new server from the following parameters.\n" "NAME Server name\n" "HOST Server host address\n" @@ -1032,9 +1036,9 @@ struct subcommand destroyoptions[] = } }; -static void alterServer(DCB *dcb, SERVER *server, char *key, char *value) +static bool handle_alter_server(SERVER *server, char *key, char *value) { - bool unknown = false; + bool valid = true; if (strcmp(key, "address") == 0) { @@ -1052,18 +1056,108 @@ static void alterServer(DCB *dcb, SERVER *server, char *key, char *value) { server_update_credentials(server, server->monuser, value); } - else if (server_is_ssl_parameter(key)) + else { - server_update_ssl(server, key, value); + valid = false; + } + + return valid; +} + +void handle_server_ssl(DCB *dcb, SERVER *server, CONFIG_CONTEXT *obj) +{ + if (config_have_required_ssl_params(obj)) + { + int err = 0; + SSL_LISTENER *ssl = make_ssl_structure(obj, true, &err); + + if (err == 0 && ssl && listener_init_SSL(ssl) == 0) + { + /** Sync to prevent reads on partially initialized server_ssl */ + atomic_synchronize(); + + server->server_ssl = ssl; + if (server_serialize(server)) + { + dcb_printf(dcb, "SSL enabled for server '%s'\n", server->unique_name); + } + else + { + dcb_printf(dcb, "SSL enabled for server '%s' but persisting " + "it to disk failed, see log for more details.\n", + server->unique_name); + } + } + else + { + dcb_printf(dcb, "Enabling SSL for server '%s' failed, see log " + "for more details.\n", server->unique_name); + } } else { - unknown = true; + dcb_printf(dcb, "Error: SSL configuration requires the following parameters:\n" + "ssl=required ssl_key=PATH ssl_cert=PATH ssl_ca_cert=PATH\n"); + } +} + +/** + * @brief Process multiple alter operations at once + * + * This is a somewhat ugly way to handle multiple key-value changes in one operation + * with one function. This could be handled with a variadic function but the + * required complexity would probably negate any benefits. + */ +static void alterServer(DCB *dcb, SERVER *server, char *v1, char *v2, char *v3, + char *v4, char *v5, char *v6, char *v7, char *v8, char *v9, + char *v10, char *v11) +{ + char *values[11] = {v1, v2, v3, v4, v5, v6, v7, v8, v9, v10, v11}; + const int items = sizeof(values) / sizeof(values[0]); + CONFIG_CONTEXT *obj = NULL; + + for (int i = 0; i < items; i++) + { + if (values[i]) + { + char *key = values[i]; + char *value = strchr(key, '='); + + if (value) + { + *value++ = '\0'; + + if (config_is_ssl_parameter(key)) + { + /** + * All the required SSL parameters must be defined at once to + * enable SSL for created servers. This removes the problem + * of partial configuration and allows a somewhat atomic + * operation. + */ + if ((obj == NULL && (obj = config_context_create(server->unique_name)) == NULL) || + (!config_add_param(obj, key, value))) + { + dcb_printf(dcb, "Internal error, see log for more details\n"); + } + } + else if (!handle_alter_server(server, key, value)) + { + dcb_printf(dcb, "Error: Bad key-value parameter: %s=%s\n", key, value); + } + } + else + { + dcb_printf(dcb, "Error: not a key-value parameter: %s\n", values[i]); + } + } } - if (unknown) + if (obj) { - dcb_printf(dcb, "Unknown parameter '%s'", key); + /** We have SSL parameters, try to process them */ + handle_server_ssl(dcb, server, obj); + config_context_free(obj); } } @@ -1072,11 +1166,10 @@ static void alterServer(DCB *dcb, SERVER *server, char *key, char *value) * * If the value is not a positive integer, an error is printed to @c dcb. * - * @param dcb Client DCB * @param value String value * @return 0 on error, otherwise a positive integer */ -static long get_positive_int(DCB *dcb, const char *value) +static long get_positive_int(const char *value) { char *endptr; long ival = strtol(value, &endptr, 10); @@ -1086,82 +1179,119 @@ static long get_positive_int(DCB *dcb, const char *value) return ival; } - dcb_printf(dcb, "Invalid value: %s", value); return 0; } -static void alterMonitor(DCB *dcb, MONITOR *monitor, char *key, char *value) +static bool handle_alter_monitor(MONITOR *monitor, char *key, char *value) { - bool unknown = false; + bool valid = false; + if (strcmp(key, "user") == 0) { + valid = true; monitorAddUser(monitor, value, monitor->password); } else if (strcmp(key, "password") == 0) { + valid = true; monitorAddUser(monitor, monitor->user, value); } else if (strcmp(key, "monitor_interval") == 0) { - long ival = get_positive_int(dcb, value); + long ival = get_positive_int(value); if (ival) { + valid = true; monitorSetInterval(monitor, ival); } } else if (strcmp(key, "backend_connect_timeout") == 0) { - long ival = get_positive_int(dcb, value); + long ival = get_positive_int(value); if (ival) { + valid = true; monitorSetNetworkTimeout(monitor, MONITOR_CONNECT_TIMEOUT, ival); } } else if (strcmp(key, "backend_write_timeout") == 0) { - long ival = get_positive_int(dcb, value); + long ival = get_positive_int(value); if (ival) { - monitorSetNetworkTimeout(monitor, MONITOR_READ_TIMEOUT, ival); + valid = true; + monitorSetNetworkTimeout(monitor, MONITOR_WRITE_TIMEOUT, ival); } } else if (strcmp(key, "backend_read_timeout") == 0) { - long ival = get_positive_int(dcb, value); + long ival = get_positive_int(value); if (ival) { - monitorSetNetworkTimeout(monitor, MONITOR_WRITE_TIMEOUT, ival); + valid = true; + monitorSetNetworkTimeout(monitor, MONITOR_READ_TIMEOUT, ival); } } - else + + return valid; +} + +static void alterMonitor(DCB *dcb, MONITOR *monitor, char *v1, char *v2, char *v3, + char *v4, char *v5, char *v6, char *v7, char *v8, char *v9, + char *v10, char *v11) +{ + char *values[11] = {v1, v2, v3, v4, v5, v6, v7, v8, v9, v10, v11}; + const int items = sizeof(values) / sizeof(values[0]); + + for (int i = 0; i < items; i++) { - unknown = true; + if (values[i]) + { + char *key = values[i]; + char *value = strchr(key, '='); + + if (value) + { + *value++ = '\0'; + + if (!handle_alter_monitor(monitor, key, value)) + { + dcb_printf(dcb, "Error: Bad key-value parameter: %s=%s\n", key, value); + } + } + else + { + dcb_printf(dcb, "Error: not a key-value parameter: %s\n", values[i]); + } + } } - if (unknown) - { - dcb_printf(dcb, "Unknown parameter '%s'", key); - } } struct subcommand alteroptions[] = { { - "server", 3, 3, alterServer, + "server", 2, 12, alterServer, "Alter server parameters", - "Usage: alter server NAME KEY VALUE\n" + "Usage: alter server NAME KEY=VALUE ...\n" "This will alter an existing parameter of a server. The accepted values\n" - "for KEY are: 'address', 'port', 'monuser', 'monpw'", - {ARG_TYPE_SERVER, ARG_TYPE_STRING, ARG_TYPE_STRING} + "for KEY are: 'address', 'port', 'monuser', 'monpw'\n" + "A maximum of 11 parameters can be changed at one time", + { ARG_TYPE_SERVER, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, + ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, + ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING} }, { - "monitor", 3, 3, alterMonitor, + "monitor", 2, 12, alterMonitor, "Alter monitor parameters", - "Usage: alter monitor NAME KEY VALUE\n" + "Usage: alter monitor NAME KEY=VALUE ...\n" "This will alter an existing parameter of a monitor. The accepted values\n" "for KEY are: 'user', 'password', 'monitor_interval',\n" - "'backend_connect_timeout', 'backend_write_timeout', 'backend_read_timeout'", - {ARG_TYPE_MONITOR, ARG_TYPE_STRING, ARG_TYPE_STRING} + "'backend_connect_timeout', 'backend_write_timeout', 'backend_read_timeout'\n" + "A maximum of 11 parameters can be changed at one time", + {ARG_TYPE_MONITOR, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, + ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, + ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING, ARG_TYPE_STRING} }, { EMPTY_OPTION @@ -1456,12 +1586,29 @@ execute_cmd(CLI_SESSION *cli) if (strcasecmp(args[1], cmds[i].options[j].arg1) == 0) { found = 1; /**< command and sub-command match */ - if (argc < cmds[i].options[j].argc_min) + + if (cmds[i].options[j].argc_min == cmds[i].options[j].argc_max && + argc != cmds[i].options[j].argc_min) { + /** Wrong number of arguments */ + dcb_printf(dcb, "Incorrect number of arguments: %s %s expects %d arguments\n", + cmds[i].cmd, cmds[i].options[j].arg1, + cmds[i].options[j].argc_min); + } + else if (argc < cmds[i].options[j].argc_min) + { + /** Not enough arguments */ dcb_printf(dcb, "Incorrect number of arguments: %s %s expects at least %d arguments\n", cmds[i].cmd, cmds[i].options[j].arg1, cmds[i].options[j].argc_min); } + else if (argc > cmds[i].options[j].argc_max) + { + /** Too many arguments */ + dcb_printf(dcb, "Incorrect number of arguments: %s %s expects at most %d arguments\n", + cmds[i].cmd, cmds[i].options[j].arg1, + cmds[i].options[j].argc_max); + } else { unsigned long arg_list[MAXARGS] = {};