From d3370429475f7522c8d3a73eb1d5489e3f1a3f51 Mon Sep 17 00:00:00 2001 From: Kalev Lember Date: Tue, 11 Apr 2023 16:37:31 +0200 Subject: [PATCH 1/7] Use the term URL throughout the code instead of URI We are using the server URL to locate the server, not just identify. --- daemon/emer-daemon.c | 80 +++++++++++++++++++------------------- daemon/emer-daemon.h | 2 +- tests/daemon/test-daemon.c | 14 +++---- 3 files changed, 48 insertions(+), 48 deletions(-) diff --git a/daemon/emer-daemon.c b/daemon/emer-daemon.c index a7eab46..888e965 100644 --- a/daemon/emer-daemon.c +++ b/daemon/emer-daemon.c @@ -150,8 +150,8 @@ struct _EmerDaemon GRand *rand; - gboolean use_default_server_uri; - gchar *server_uri; + gboolean use_default_server_url; + gchar *server_url; guint upload_events_timeout_source_id; guint report_invalid_cache_data_source_id; @@ -176,7 +176,7 @@ enum { PROP_0, PROP_RANDOM_NUMBER_GENERATOR, - PROP_SERVER_URI, + PROP_SERVER_URL, PROP_NETWORK_SEND_INTERVAL, PROP_PERMISSIONS_PROVIDER, PROP_PERSISTENT_CACHE_DIRECTORY, @@ -358,24 +358,24 @@ get_random_backoff_interval (GRand *rand, * done. */ static SoupURI * -get_http_request_uri (EmerDaemon *self, +get_http_request_url (EmerDaemon *self, const guchar *data, gsize length) { gchar *checksum = g_compute_checksum_for_data (G_CHECKSUM_SHA512, data, length); - gchar *http_request_uri_string = - g_build_filename (self->server_uri, checksum, NULL); + gchar *http_request_url_string = + g_build_filename (self->server_url, checksum, NULL); g_free (checksum); - SoupURI *http_request_uri = soup_uri_new (http_request_uri_string); + SoupURI *http_request_url = soup_uri_new (http_request_url_string); - if (http_request_uri == NULL) - g_error ("Invalid URI: %s.", http_request_uri_string); + if (http_request_url == NULL) + g_error ("Invalid URL: %s.", http_request_url_string); - g_free (http_request_uri_string); + g_free (http_request_url_string); - return http_request_uri; + return http_request_url; } /* @@ -472,12 +472,12 @@ queue_http_request (GTask *upload_task) return; } - SoupURI *http_request_uri = - get_http_request_uri (self, serialized_request_body, + SoupURI *http_request_url = + get_http_request_url (self, serialized_request_body, serialized_request_body_length); SoupMessage *http_message = - soup_message_new_from_uri ("PUT", http_request_uri); - soup_uri_free (http_request_uri); + soup_message_new_from_uri ("PUT", http_request_url); + soup_uri_free (http_request_url); soup_message_headers_append (http_message->request_headers, "X-Endless-Content-Encoding", "gzip"); @@ -552,7 +552,7 @@ handle_http_response (SoupSession *http_session, "%" G_GSIZE_FORMAT " events from buffer to %s.", callback_data->num_stored_events, callback_data->num_buffer_events, - self->server_uri); + self->server_url); g_task_return_boolean (upload_task, TRUE); finish_network_callback (upload_task); return; @@ -829,11 +829,11 @@ get_ping_socket (EmerDaemon *self) { GError *error = NULL; GSocketConnectable *ping_socket = - g_network_address_parse_uri (self->server_uri, 443 /* SSL default port */, + g_network_address_parse_uri (self->server_url, 443 /* SSL default port */, &error); if (ping_socket == NULL) - g_error ("Invalid server URI '%s' could not be parsed because: %s.", - self->server_uri, error->message); + g_error ("Invalid server URL '%s' could not be parsed because: %s.", + self->server_url, error->message); return ping_socket; } @@ -894,10 +894,10 @@ dequeue_and_do_upload (EmerDaemon *self, return; } - if (self->use_default_server_uri) + if (self->use_default_server_url) { - g_free (self->server_uri); - self->server_uri = + g_free (self->server_url); + self->server_url = g_strconcat ("https://", environment, "." DEFAULT_METRICS_SERVER "/" CLIENT_VERSION_NUMBER "/", NULL); } @@ -1020,15 +1020,15 @@ set_random_number_generator (EmerDaemon *self, } static void -set_server_uri (EmerDaemon *self, - const gchar *server_uri) +set_server_url (EmerDaemon *self, + const gchar *server_url) { - self->use_default_server_uri = (server_uri == NULL); - if (!self->use_default_server_uri) + self->use_default_server_url = (server_url == NULL); + if (!self->use_default_server_url) { - g_free (self->server_uri); - self->server_uri = - g_build_filename (server_uri, CLIENT_VERSION_NUMBER "/", NULL); + g_free (self->server_url); + self->server_url = + g_build_filename (server_url, CLIENT_VERSION_NUMBER "/", NULL); } } @@ -1431,8 +1431,8 @@ emer_daemon_set_property (GObject *object, set_random_number_generator (self, g_value_get_pointer (value)); break; - case PROP_SERVER_URI: - set_server_uri (self, g_value_get_string (value)); + case PROP_SERVER_URL: + set_server_url (self, g_value_get_string (value)); break; case PROP_NETWORK_SEND_INTERVAL: @@ -1496,7 +1496,7 @@ emer_daemon_finalize (GObject *object) g_clear_pointer (&self->variant_array, g_ptr_array_unref); g_rand_free (self->rand); - g_clear_pointer (&self->server_uri, g_free); + g_clear_pointer (&self->server_url, g_free); g_clear_object (&self->permissions_provider); g_clear_object (&self->aggregate_tally); g_clear_pointer (&self->persistent_cache_directory, g_free); @@ -1531,15 +1531,15 @@ emer_daemon_class_init (EmerDaemonClass *klass) G_PARAM_STATIC_STRINGS); /* - * EmerDaemon:server-uri: + * EmerDaemon:server-url: * - * The URI to which events are uploaded. The URI must contain the protocol and + * The URL to which events are uploaded. The URL must contain the protocol and * may contain the port number. If unspecified, the port number defaults to * 443, which is the standard port number for SSL. */ - emer_daemon_props[PROP_SERVER_URI] = - g_param_spec_string ("server-uri", "Server URI", - "URI to which events are uploaded", + emer_daemon_props[PROP_SERVER_URL] = + g_param_spec_string ("server-url", "Server URL", + "URL to which events are uploaded", NULL, G_PARAM_CONSTRUCT | G_PARAM_WRITABLE | G_PARAM_STATIC_STRINGS); @@ -1687,7 +1687,7 @@ emer_daemon_new (const gchar *persistent_cache_directory, * emer_daemon_new_full: * @rand: (allow-none): random number generator to use for randomized * exponential backoff, or %NULL to use the default. - * @server_uri: (allow-none): the URI (including protocol and, optionally, port + * @server_url: (allow-none): the URL (including protocol and, optionally, port * number) to which to upload events, or %NULL to use the default. Must * include trailing forward slash. If the port number is unspecified, it * defaults to 443 (the standard port used by SSL). @@ -1710,7 +1710,7 @@ emer_daemon_new (const gchar *persistent_cache_directory, */ EmerDaemon * emer_daemon_new_full (GRand *rand, - const gchar *server_uri, + const gchar *server_url, guint network_send_interval, EmerPermissionsProvider *permissions_provider, EmerPersistentCache *persistent_cache, @@ -1719,7 +1719,7 @@ emer_daemon_new_full (GRand *rand, { return g_object_new (EMER_TYPE_DAEMON, "random-number-generator", rand, - "server-uri", server_uri, + "server-url", server_url, "network-send-interval", network_send_interval, "permissions-provider", permissions_provider, "persistent-cache", persistent_cache, diff --git a/daemon/emer-daemon.h b/daemon/emer-daemon.h index 4acee88..92c4d87 100644 --- a/daemon/emer-daemon.h +++ b/daemon/emer-daemon.h @@ -40,7 +40,7 @@ EmerDaemon * emer_daemon_new (const gchar EmerPermissionsProvider *permissions_provider); EmerDaemon * emer_daemon_new_full (GRand *rand, - const gchar *server_uri, + const gchar *server_url, guint network_send_interval, EmerPermissionsProvider *permissions_provider, EmerPersistentCache *persistent_cache, diff --git a/tests/daemon/test-daemon.c b/tests/daemon/test-daemon.c index 3579b2c..fe5faab 100644 --- a/tests/daemon/test-daemon.c +++ b/tests/daemon/test-daemon.c @@ -71,7 +71,7 @@ typedef struct _Fixture EmerAggregateTally *mock_aggregate_tally; GSubprocess *mock_server; - gchar *server_uri; + gchar *server_url; gint64 relative_time; gint64 absolute_time; @@ -802,15 +802,15 @@ wait_for_upload_to_finish (Fixture *fixture) } static gchar * -get_server_uri (GSubprocess *mock_server) +get_server_url (GSubprocess *mock_server) { gchar *port_number; read_lines_from_stdout (mock_server, (ProcessLineSourceFunc) remove_last_character, &port_number); - gchar *server_uri = g_strconcat ("http://localhost:", port_number, "/", NULL); + gchar *server_url = g_strconcat ("http://localhost:", port_number, "/", NULL); g_free (port_number); - return server_uri; + return server_url; } static void @@ -826,7 +826,7 @@ create_test_object (Fixture *fixture) { fixture->test_object = emer_daemon_new_full (g_rand_new_with_seed (18), - fixture->server_uri, + fixture->server_url, 2 /* network send interval */, fixture->mock_permissions_provider, fixture->mock_persistent_cache, @@ -852,7 +852,7 @@ setup_most (Fixture *fixture, g_assert_nonnull (fixture->mock_server); g_object_unref (subprocess_launcher); - fixture->server_uri = get_server_uri (fixture->mock_server); + fixture->server_url = get_server_url (fixture->mock_server); fixture->mock_permissions_provider = emer_permissions_provider_new (); fixture->mock_persistent_cache = NULL; @@ -892,7 +892,7 @@ teardown (Fixture *fixture, g_clear_object (&fixture->mock_persistent_cache); g_clear_object (&fixture->mock_aggregate_tally); g_clear_pointer (&fixture->mock_server, terminate_subprocess_and_wait); - g_clear_pointer (&fixture->server_uri, g_free); + g_clear_pointer (&fixture->server_url, g_free); } // Unit Tests next: From d0fa13eb23fd7b7811753b56bb505442b9704c3c Mon Sep 17 00:00:00 2001 From: Kalev Lember Date: Tue, 11 Apr 2023 17:13:52 +0200 Subject: [PATCH 2/7] Bump minimum GLib version to 2.68 and port away from deprecated APIs Replace deprecated g_binding_get_source() with g_binding_dup_source(), and g_memdup() to g_memdup2(). This is required by the libsoup 3 port as SOUP_HTTP_URI_FLAGS definition needs GLib 2.68. --- daemon/emer-aggregate-tally.c | 4 ++-- daemon/emer-gzip.c | 2 +- daemon/emer-main.c | 8 ++++---- daemon/emer-persistent-cache.c | 2 +- debian/control | 2 +- meson.build | 6 +++--- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/daemon/emer-aggregate-tally.c b/daemon/emer-aggregate-tally.c index 2a9c88e..bff79bc 100644 --- a/daemon/emer-aggregate-tally.c +++ b/daemon/emer-aggregate-tally.c @@ -123,7 +123,7 @@ column_to_variant (sqlite3_stmt *stmt, { g_autoptr(GVariant) variant = NULL; gconstpointer sqlite_data; - gint size; + gsize size; /* Unlike sqlite3_column_text, "The return value from sqlite3_column_blob() * for a zero-length BLOB is a NULL pointer." even if the value is not NULL @@ -135,7 +135,7 @@ column_to_variant (sqlite3_stmt *stmt, return NULL; size = sqlite3_column_bytes (stmt, i); - gpointer data = g_memdup (sqlite_data, size); + gpointer data = g_memdup2 (sqlite_data, size); variant = g_variant_new_from_data (G_VARIANT_TYPE_VARIANT, data, size, FALSE, diff --git a/daemon/emer-gzip.c b/daemon/emer-gzip.c index 40d2f97..0938260 100644 --- a/daemon/emer-gzip.c +++ b/daemon/emer-gzip.c @@ -126,7 +126,7 @@ emer_gzip_compress (gconstpointer input_data, } g_object_unref (zlib_compressor); - gpointer compressed_data = g_memdup (byte_array->data, total_bytes_written); + gpointer compressed_data = g_memdup2 (byte_array->data, total_bytes_written); g_byte_array_free (byte_array, TRUE); *compressed_length = total_bytes_written; return compressed_data; diff --git a/daemon/emer-main.c b/daemon/emer-main.c index fc564fe..5e6f843 100644 --- a/daemon/emer-main.c +++ b/daemon/emer-main.c @@ -296,8 +296,8 @@ sync_recorder_server_enabled_for_upload (GBinding *binding, GValue *to_value, gpointer user_data) { - EmerPermissionsProvider *permissions = - EMER_PERMISSIONS_PROVIDER (g_binding_get_source (binding)); + g_autoptr(EmerPermissionsProvider) permissions = + EMER_PERMISSIONS_PROVIDER (g_binding_dup_source (binding)); gboolean daemon_enabled = emer_permissions_provider_get_daemon_enabled (permissions); @@ -314,8 +314,8 @@ sync_recorder_server_enabled_for_daemon (GBinding *binding, GValue *to_value, gpointer user_data) { - EmerPermissionsProvider *permissions = - EMER_PERMISSIONS_PROVIDER (g_binding_get_source (binding)); + g_autoptr(EmerPermissionsProvider) permissions = + EMER_PERMISSIONS_PROVIDER (g_binding_dup_source (binding)); gboolean daemon_enabled = g_value_get_boolean (from_value); gboolean uploading_enabled = diff --git a/daemon/emer-persistent-cache.c b/daemon/emer-persistent-cache.c index b21efbc..a84dd08 100644 --- a/daemon/emer-persistent-cache.c +++ b/daemon/emer-persistent-cache.c @@ -1083,7 +1083,7 @@ emer_persistent_cache_read (EmerPersistentCache *self, const GVariantType *variant_type = G_VARIANT_TYPE (elem_data); const gchar *start_of_variant = end_of_type + 1; gsize variant_size = (elem_data + elem_size) - start_of_variant; - gpointer variant_data = g_memdup (start_of_variant, variant_size); + gpointer variant_data = g_memdup2 (start_of_variant, variant_size); GVariant *curr_variant = g_variant_new_from_data (variant_type, variant_data, variant_size, FALSE /* trusted */, g_free, variant_data); diff --git a/debian/control b/debian/control index 5d3da39..8c605e1 100644 --- a/debian/control +++ b/debian/control @@ -4,7 +4,7 @@ Maintainer: Endless OS Team Build-Depends: ca-certificates, debhelper-compat (= 13), eos-metrics-0-dev (>= 0.2), - libglib2.0-dev (>= 2.38), + libglib2.0-dev (>= 2.68), libostree-dev (>= 2013.7), libpolkit-gobject-1-dev, libsoup2.4-dev (>= 2.42), diff --git a/meson.build b/meson.build index 477a7e7..83655a5 100644 --- a/meson.build +++ b/meson.build @@ -8,7 +8,7 @@ project('eos-event-recorder-daemon', cc = meson.get_compiler('c') -glib_dep = dependency('glib-2.0', version: '>= 2.66') +glib_dep = dependency('glib-2.0', version: '>= 2.68') eosmetrics_dep = dependency('eosmetrics-0', version: '>= 0.2') polkit_gobject_dep = dependency('polkit-gobject-1') @@ -53,8 +53,8 @@ add_project_arguments( '-DPERMISSIONS_FILE="@0@"'.format(permissions_file), '-DPERSISTENT_CACHE_DIR="@0@"'.format(persistent_cache_dir), '-DSYSCONFDIR="@0@"'.format(get_option('sysconfdir')), - '-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_66', - '-DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_66', + '-DGLIB_VERSION_MIN_REQUIRED=GLIB_VERSION_2_68', + '-DGLIB_VERSION_MAX_ALLOWED=GLIB_VERSION_2_68', '-Wno-unused-parameter', ], language: 'c', From 5fd72961012a5388ba9ea41ec10f675370ac7412 Mon Sep 17 00:00:00 2001 From: Kalev Lember Date: Fri, 31 Mar 2023 11:54:02 +0200 Subject: [PATCH 3/7] Make the server URL configurable in the config file This makes it easier if we want to test a different server without needing to recompile the daemon for this. The default server URL that is passed to meson (as -Ddefault_metrics_server_url) is written out to the config file, but at the same time also hardcoded as a default fallback in case clients don't get the updated configs (this can easily happen if ostree or rpm are managing the config files). The default_metrics_server meson option is now renamed to default_metrics_server_url to signify that it now accepts a slightly different syntax: instead of azafea.example, it now takes https://${environment}.azafea.example:443 where ${environment} variable gets replaced by the environment variable value from the same config file. This avoids the previous behaviour where the environment value was magically prepended to the host address and makes it more obvious what the actual host address that is used is going to be. --- daemon/emer-daemon.c | 31 +++++--- daemon/emer-permissions-provider.c | 75 +++++++++++++++++++ daemon/emer-permissions-provider.h | 5 ++ ...s.conf => eos-metrics-permissions.conf.in} | 1 + data/meson.build | 9 ++- debian/rules | 2 +- meson.build | 4 +- meson_options.txt | 6 +- tests/daemon/mock-permissions-provider.c | 6 ++ 9 files changed, 119 insertions(+), 20 deletions(-) rename data/{eos-metrics-permissions.conf => eos-metrics-permissions.conf.in} (63%) diff --git a/daemon/emer-daemon.c b/daemon/emer-daemon.c index 888e965..56f27a8 100644 --- a/daemon/emer-daemon.c +++ b/daemon/emer-daemon.c @@ -150,7 +150,6 @@ struct _EmerDaemon GRand *rand; - gboolean use_default_server_url; gchar *server_url; guint upload_events_timeout_source_id; @@ -894,12 +893,13 @@ dequeue_and_do_upload (EmerDaemon *self, return; } - if (self->use_default_server_url) + if (strstr (self->server_url, "${environment}") != NULL) { + GString *s = g_string_new (self->server_url); + g_string_replace (s, "${environment}", environment, 0); + g_free (self->server_url); - self->server_url = - g_strconcat ("https://", environment, "." DEFAULT_METRICS_SERVER "/" - CLIENT_VERSION_NUMBER "/", NULL); + self->server_url = g_string_free (s, FALSE); } GNetworkMonitor *network_monitor = g_network_monitor_get_default (); @@ -1023,13 +1023,11 @@ static void set_server_url (EmerDaemon *self, const gchar *server_url) { - self->use_default_server_url = (server_url == NULL); - if (!self->use_default_server_url) - { - g_free (self->server_url); - self->server_url = - g_build_filename (server_url, CLIENT_VERSION_NUMBER "/", NULL); - } + g_free (self->server_url); + self->server_url = NULL; + + if (server_url != NULL) + self->server_url = g_build_filename (server_url, CLIENT_VERSION_NUMBER "/", NULL); } static void @@ -1386,6 +1384,15 @@ emer_daemon_constructed (GObject *object) } buffer_past_aggregate_events (self); + if (self->server_url == NULL) + { + g_autofree gchar *server_url = emer_permissions_provider_get_server_url (self->permissions_provider); + if (server_url != NULL) + set_server_url (self, server_url); + else + set_server_url (self, DEFAULT_METRICS_SERVER_URL); + } + gchar *environment = emer_permissions_provider_get_environment (self->permissions_provider); schedule_upload (self, environment); diff --git a/daemon/emer-permissions-provider.c b/daemon/emer-permissions-provider.c index 7fc8e71..9b6019a 100644 --- a/daemon/emer-permissions-provider.c +++ b/daemon/emer-permissions-provider.c @@ -52,6 +52,7 @@ G_DEFINE_TYPE_WITH_PRIVATE (EmerPermissionsProvider, emer_permissions_provider, #define DAEMON_ENABLED_KEY_NAME "enabled" #define DAEMON_UPLOADING_ENABLED_KEY_NAME "uploading_enabled" #define DAEMON_ENVIRONMENT_KEY_NAME "environment" +#define DAEMON_SERVER_URL_KEY_NAME "server_url" #define FALLBACK_CONFIG_FILE_DATA \ "[" DAEMON_GLOBAL_GROUP_NAME "]\n" \ @@ -66,6 +67,7 @@ enum PROP_OSTREE_CONFIG_FILE_PATH, PROP_DAEMON_ENABLED, PROP_UPLOADING_ENABLED, + PROP_SERVER_URL, NPROPS }; @@ -177,6 +179,9 @@ read_config_file_sync (EmerPermissionsProvider *self) emer_permissions_provider_props[PROP_UPLOADING_ENABLED]; g_object_notify_by_pspec (G_OBJECT (self), uploading_enabled_pspec); + GParamSpec *server_url_pspec = + emer_permissions_provider_props[PROP_SERVER_URL]; + g_object_notify_by_pspec (G_OBJECT (self), server_url_pspec); } static gchar * @@ -385,6 +390,11 @@ emer_permissions_provider_get_property (GObject *object, emer_permissions_provider_get_uploading_enabled (self)); break; + case PROP_SERVER_URL: + g_value_set_string (value, + emer_permissions_provider_get_server_url (self)); + break; + default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); } @@ -418,6 +428,11 @@ emer_permissions_provider_set_property (GObject *object, g_value_get_boolean (value)); break; + case PROP_SERVER_URL: + emer_permissions_provider_set_server_url (self, + g_value_get_string (value)); + break; + default: G_OBJECT_WARN_INVALID_PROPERTY_ID (object, property_id, pspec); } @@ -467,6 +482,11 @@ emer_permissions_provider_class_init (EmerPermissionsProviderClass *klass) "Whether to upload events via the network", FALSE, G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); + emer_permissions_provider_props[PROP_SERVER_URL] = + g_param_spec_string ("server-url", "Server URL", + "Metrics server URL to use", + NULL, + G_PARAM_READWRITE | G_PARAM_STATIC_STRINGS); g_object_class_install_properties (object_class, NPROPS, emer_permissions_provider_props); @@ -674,3 +694,58 @@ emer_permissions_provider_get_environment (EmerPermissionsProvider *self) g_clear_pointer (&ostree_url, g_free); return environment; } + +/* + * emer_permissions_provider_get_server_url: + * @self: the permissions provider + * + * Gets the metrics server URL from the configuration file. + * + * Returns: The metrics server URL. + */ +gchar * +emer_permissions_provider_get_server_url (EmerPermissionsProvider *self) +{ + EmerPermissionsProviderPrivate *priv = + emer_permissions_provider_get_instance_private (self); + + GError *error = NULL; + gchar *server_url = + g_key_file_get_value (priv->permissions, DAEMON_GLOBAL_GROUP_NAME, + DAEMON_SERVER_URL_KEY_NAME, &error); + if (error != NULL) + { + g_debug ("Couldn't find key '%s:%s' in permissions config file. " + "Returning default value. Error: %s.", + DAEMON_GLOBAL_GROUP_NAME, DAEMON_SERVER_URL_KEY_NAME, + error->message); + g_error_free (error); + return NULL; + } + + return server_url; +} + +/* + * emer_permissions_provider_set_server_url: + * @self: the permissions provider + * @server_url: the metrics server URL to use + * + * Sets the metrics server URL in the configuration file. + */ +void +emer_permissions_provider_set_server_url (EmerPermissionsProvider *self, + const gchar *server_url) +{ + EmerPermissionsProviderPrivate *priv = + emer_permissions_provider_get_instance_private (self); + + g_key_file_set_value (priv->permissions, DAEMON_GLOBAL_GROUP_NAME, + DAEMON_SERVER_URL_KEY_NAME, server_url); + + schedule_config_file_update (self); + + GParamSpec *server_url_pspec = + emer_permissions_provider_props[PROP_SERVER_URL]; + g_object_notify_by_pspec (G_OBJECT (self), server_url_pspec); +} diff --git a/daemon/emer-permissions-provider.h b/daemon/emer-permissions-provider.h index 57018e8..e2bb52f 100644 --- a/daemon/emer-permissions-provider.h +++ b/daemon/emer-permissions-provider.h @@ -83,6 +83,11 @@ void emer_permissions_provider_set_uploading_enabled (EmerPe gchar *emer_permissions_provider_get_environment (EmerPermissionsProvider *self); +gchar *emer_permissions_provider_get_server_url (EmerPermissionsProvider *self); + +void emer_permissions_provider_set_server_url (EmerPermissionsProvider *self, + const gchar *server_url); + G_END_DECLS #endif /* EMER_PERMISSIONS_PROVIDER_H */ diff --git a/data/eos-metrics-permissions.conf b/data/eos-metrics-permissions.conf.in similarity index 63% rename from data/eos-metrics-permissions.conf rename to data/eos-metrics-permissions.conf.in index 7f25595..68b7aee 100644 --- a/data/eos-metrics-permissions.conf +++ b/data/eos-metrics-permissions.conf.in @@ -2,3 +2,4 @@ enabled=true uploading_enabled=false environment=production +server_url=@default_metrics_server_url@ diff --git a/data/meson.build b/data/meson.build index 417f654..2bdba16 100644 --- a/data/meson.build +++ b/data/meson.build @@ -39,8 +39,13 @@ configure_file( ) # Daemon configuration -install_data( - 'eos-metrics-permissions.conf', +configure_file( + input: 'eos-metrics-permissions.conf.in', + output: 'eos-metrics-permissions.conf', + configuration: configuration_data({ + 'default_metrics_server_url': default_metrics_server_url, + }), + install: true, install_dir: config_dir, install_mode: ['rwxrwsr-x', 'metrics', 'metrics'], ) diff --git a/debian/rules b/debian/rules index 599f8bd..1eb7881 100755 --- a/debian/rules +++ b/debian/rules @@ -14,7 +14,7 @@ # Build our documentation as well override_dh_auto_configure: - dh_auto_configure -- -Ddefault_metrics_server=metrics.endlessm.com + dh_auto_configure -- -Ddefault_metrics_server_url='https://${environment}.metrics.endlessm.com' # Avoid changing the owner of configuration files and the persistent cache # directory to root:root. diff --git a/meson.build b/meson.build index 83655a5..02ca955 100644 --- a/meson.build +++ b/meson.build @@ -39,7 +39,7 @@ py = import('python').find_installation('python3', systemd_dep = dependency('systemd') event_recorder_server_xml = eosmetrics_dep.get_variable(pkgconfig: 'datadir') / 'eos-metrics' / 'com.endlessm.Metrics.xml' -default_metrics_server = get_option('default_metrics_server') +default_metrics_server_url = get_option('default_metrics_server_url') prefix = get_option('prefix') libexec_dir = join_paths(prefix, get_option('libexecdir')) @@ -60,7 +60,7 @@ add_project_arguments( language: 'c', ) conf_data = configuration_data() -conf_data.set_quoted('DEFAULT_METRICS_SERVER', default_metrics_server) +conf_data.set_quoted('DEFAULT_METRICS_SERVER_URL', default_metrics_server_url) configure_file( output: 'config.h', configuration: conf_data, diff --git a/meson_options.txt b/meson_options.txt index 03b67a5..64949ee 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -1,5 +1,5 @@ -option('default_metrics_server', +option('default_metrics_server_url', type: 'string', - description: 'Hostname of default metrics server', - value: 'azafea.example', + description: 'URL of default metrics server', + value: 'https://${environment}.azafea.example', ) diff --git a/tests/daemon/mock-permissions-provider.c b/tests/daemon/mock-permissions-provider.c index 4858d07..5d1c110 100644 --- a/tests/daemon/mock-permissions-provider.c +++ b/tests/daemon/mock-permissions-provider.c @@ -113,6 +113,12 @@ emer_permissions_provider_get_environment (EmerPermissionsProvider *self) return g_strdup ("test"); } +gchar * +emer_permissions_provider_get_server_url (EmerPermissionsProvider *self) +{ + return NULL; +} + /* API OF MOCK OBJECT */ /* Sets the value to return from From 177bb9cd580e92c662822d882b42ab847fa0401c Mon Sep 17 00:00:00 2001 From: Kalev Lember Date: Tue, 11 Apr 2023 18:41:53 +0200 Subject: [PATCH 4/7] Port from libsoup 2 to 3 Fixes https://github.com/endlessm/eos-event-recorder-daemon/issues/294 --- daemon/emer-daemon.c | 59 +++++++++++++++++++++++--------------------- debian/control | 2 +- meson.build | 2 +- 3 files changed, 33 insertions(+), 30 deletions(-) diff --git a/daemon/emer-daemon.c b/daemon/emer-daemon.c index 56f27a8..b026e03 100644 --- a/daemon/emer-daemon.c +++ b/daemon/emer-daemon.c @@ -198,9 +198,9 @@ static guint emer_daemon_signals[NSIGNALS] = { 0u, }; static gboolean handle_upload_timer (EmerDaemon *self); -static void handle_http_response (SoupSession *http_session, - SoupMessage *http_message, - GTask *upload_task); +static void handle_http_response (GObject *source_object, + GAsyncResult *result, + GTask *upload_task); static void aggregate_timer_sender_data_free (AggregateTimerSenderData *sender_data) @@ -353,10 +353,10 @@ get_random_backoff_interval (GRand *rand, } /* - * Returned object is owned by calling code. Free with soup_uri_free() when + * Returned object is owned by calling code. Free with g_uri_unref() when * done. */ -static SoupURI * +static GUri * get_http_request_url (EmerDaemon *self, const guchar *data, gsize length) @@ -367,10 +367,12 @@ get_http_request_url (EmerDaemon *self, g_build_filename (self->server_url, checksum, NULL); g_free (checksum); - SoupURI *http_request_url = soup_uri_new (http_request_url_string); + g_autoptr(GError) error = NULL; + GUri *http_request_url = g_uri_parse (http_request_url_string, SOUP_HTTP_URI_FLAGS, &error); if (http_request_url == NULL) - g_error ("Invalid URL: %s.", http_request_url_string); + g_error ("Invalid http request URL '%s' could not be parsed because: %s.", + http_request_url_string, error->message); g_free (http_request_url_string); @@ -471,21 +473,21 @@ queue_http_request (GTask *upload_task) return; } - SoupURI *http_request_url = + g_autoptr(GUri) http_request_url = get_http_request_url (self, serialized_request_body, serialized_request_body_length); - SoupMessage *http_message = + g_autoptr(SoupMessage) http_message = soup_message_new_from_uri ("PUT", http_request_url); - soup_uri_free (http_request_url); - soup_message_headers_append (http_message->request_headers, + soup_message_headers_append (soup_message_get_request_headers (http_message), "X-Endless-Content-Encoding", "gzip"); - soup_message_set_request (http_message, "application/octet-stream", - SOUP_MEMORY_TAKE, compressed_request_body, - compressed_request_body_length); - soup_session_queue_message (self->http_session, http_message, - (SoupSessionCallback) handle_http_response, - upload_task); + + g_autoptr(GBytes) request_body = g_bytes_new (compressed_request_body, compressed_request_body_length); + soup_message_set_request_body_from_bytes (http_message, "application/octet-stream", request_body); + + soup_session_send_async (self->http_session, http_message, G_PRIORITY_DEFAULT, NULL, + (GAsyncReadyCallback) handle_http_response, + upload_task); } static gboolean @@ -516,15 +518,18 @@ handle_backoff_timer (GTask *upload_task) // Handles HTTP or HTTPS responses. static void -handle_http_response (SoupSession *http_session, - SoupMessage *http_message, - GTask *upload_task) +handle_http_response (GObject *source_object, + GAsyncResult *result, + GTask *upload_task) { EmerDaemon *self = g_task_get_source_object (upload_task); NetworkCallbackData *callback_data = g_task_get_task_data (upload_task); - guint status_code; - g_object_get (http_message, "status-code", &status_code, NULL); + g_autoptr(GInputStream) stream = soup_session_send_finish (SOUP_SESSION (source_object), result, NULL); + + SoupMessage *http_message = soup_session_get_async_result_message (SOUP_SESSION (source_object), result); + guint status_code = soup_message_get_status (http_message); + if (SOUP_STATUS_IS_SUCCESSFUL (status_code)) { GCancellable *cancellable = @@ -578,8 +583,7 @@ handle_http_response (SoupSession *http_session, return; } - if (SOUP_STATUS_IS_TRANSPORT_ERROR (status_code) || - SOUP_STATUS_IS_CLIENT_ERROR (status_code) || + if (SOUP_STATUS_IS_CLIENT_ERROR (status_code) || SOUP_STATUS_IS_SERVER_ERROR (status_code)) { guint random_backoff_interval = @@ -1655,11 +1659,10 @@ emer_daemon_init (EmerDaemon *self) self->upload_queue = g_queue_new (); self->http_session = - soup_session_new_with_options (SOUP_SESSION_MAX_CONNS, 1, - SOUP_SESSION_MAX_CONNS_PER_HOST, 1, - SOUP_SESSION_ADD_FEATURE_BY_TYPE, - SOUP_TYPE_CACHE, + soup_session_new_with_options ("max-conns", 1, + "max-conns-per-host", 1, NULL); + soup_session_add_feature_by_type (self->http_session, SOUP_TYPE_CACHE); self->aggregate_timers = g_hash_table_new_full (NULL, NULL, g_object_unref, NULL); diff --git a/debian/control b/debian/control index 8c605e1..c7f6e71 100644 --- a/debian/control +++ b/debian/control @@ -7,7 +7,7 @@ Build-Depends: ca-certificates, libglib2.0-dev (>= 2.68), libostree-dev (>= 2013.7), libpolkit-gobject-1-dev, - libsoup2.4-dev (>= 2.42), + libsoup-3.0-dev, meson (>= 0.52), pkg-config, python3-dbus, diff --git a/meson.build b/meson.build index 02ca955..52929d9 100644 --- a/meson.build +++ b/meson.build @@ -24,7 +24,7 @@ emer_required_modules = emer_shared_required_modules + [ cc.find_library('m', required: false), dependency('ostree-1', version: '>= 2013.7'), polkit_gobject_dep, - dependency('libsoup-2.4', version: '>= 2.42'), + dependency('libsoup-3.0'), ] gnome = import('gnome') From d2cdc51b64bd3306efed3c502bdc0305e4116d38 Mon Sep 17 00:00:00 2001 From: Kalev Lember Date: Mon, 8 May 2023 15:39:09 +0200 Subject: [PATCH 5/7] Add GitHub Actions CI Unlike Endless's Jenkins-based CI, this is visible to those outside Endless OS Foundation. The job runs on Fedora and makes use of the kalev/metrics copr for installing the eos-metrics library. --- .github/workflows/ci.yml | 43 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..313972d --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,43 @@ +name: CI + +on: [push, pull_request] + +jobs: + fedora: + runs-on: ubuntu-latest + container: fedora:latest + steps: + - uses: actions/checkout@v3 + - name: Check we are on Fedora + run: cat /etc/os-release + - name: Update + run: | + sudo dnf -y update + - name: Enable kalev/metrics copr + run: | + sudo dnf -y install 'dnf-command(copr)' + sudo dnf -y copr enable kalev/metrics + - name: Install dependencies + run: | + sudo dnf -y install \ + gcc \ + meson \ + 'pkgconfig(eosmetrics-0)' \ + 'pkgconfig(gio-unix-2.0)' \ + 'pkgconfig(glib-2.0)' \ + 'pkgconfig(gobject-2.0)' \ + 'pkgconfig(libsoup-3.0)' \ + 'pkgconfig(ostree-1)' \ + 'pkgconfig(polkit-gobject-1)' \ + 'pkgconfig(sqlite3)' \ + 'pkgconfig(systemd)' \ + 'pkgconfig(uuid)' \ + python3-dbus \ + python3-dbusmock \ + ${NULL+} + - name: Meson setup + run: meson setup _build + - name: Build + run: ninja -C _build + - name: Run tests + run: meson test -C _build From 355925805150c2d5dedfb2a37d807ccf2292dab8 Mon Sep 17 00:00:00 2001 From: Kalev Lember Date: Tue, 23 May 2023 12:21:16 +0200 Subject: [PATCH 6/7] daemon: Assert that url is set --- daemon/emer-daemon.c | 1 + 1 file changed, 1 insertion(+) diff --git a/daemon/emer-daemon.c b/daemon/emer-daemon.c index b026e03..0281a5a 100644 --- a/daemon/emer-daemon.c +++ b/daemon/emer-daemon.c @@ -897,6 +897,7 @@ dequeue_and_do_upload (EmerDaemon *self, return; } + g_assert (self->server_url != NULL); if (strstr (self->server_url, "${environment}") != NULL) { GString *s = g_string_new (self->server_url); From 42c2f14978368434b40bb571ee5fd7c74466ddde Mon Sep 17 00:00:00 2001 From: Will Thompson Date: Wed, 10 Jan 2024 18:07:59 +0000 Subject: [PATCH 7/7] daemon: Handle error from soup_session_send_finish() Some errors that were reported via SoupStatus in the Soup 2.4 API are now reported through the normal GError mechanism. Handle and log those, and add a test that a malformed HTTP response from the server is handled somewhat gracefully. https://libsoup.org/libsoup-3.0/migrating-from-libsoup-2.html#status-codes-no-longer-used-for-internal-errors --- daemon/emer-daemon.c | 19 ++++++++++++------- tests/daemon/mock-server.py | 8 +++++++- tests/daemon/test-daemon.c | 21 +++++++++++++++++++++ 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/daemon/emer-daemon.c b/daemon/emer-daemon.c index 0281a5a..037499c 100644 --- a/daemon/emer-daemon.c +++ b/daemon/emer-daemon.c @@ -524,13 +524,14 @@ handle_http_response (GObject *source_object, { EmerDaemon *self = g_task_get_source_object (upload_task); NetworkCallbackData *callback_data = g_task_get_task_data (upload_task); + g_autoptr(GError) error = NULL; - g_autoptr(GInputStream) stream = soup_session_send_finish (SOUP_SESSION (source_object), result, NULL); + g_autoptr(GInputStream) stream = soup_session_send_finish (SOUP_SESSION (source_object), result, &error); SoupMessage *http_message = soup_session_get_async_result_message (SOUP_SESSION (source_object), result); guint status_code = soup_message_get_status (http_message); - if (SOUP_STATUS_IS_SUCCESSFUL (status_code)) + if (stream != NULL && SOUP_STATUS_IS_SUCCESSFUL (status_code)) { GCancellable *cancellable = g_task_get_cancellable (upload_task); @@ -562,10 +563,12 @@ handle_http_response (GObject *source_object, return; } - gchar *reason_phrase; - g_object_get (http_message, "reason-phrase", &reason_phrase, NULL); - g_warning ("Attempt to upload metrics failed: %s.", reason_phrase); - g_free (reason_phrase); + const gchar *reason; + if (error != NULL) + reason = error->message; + else + reason = soup_message_get_reason_phrase (http_message); + g_warning ("Attempt to upload metrics failed: %s.", reason); if (g_task_return_error_if_cancelled (upload_task)) { @@ -584,7 +587,9 @@ handle_http_response (GObject *source_object, } if (SOUP_STATUS_IS_CLIENT_ERROR (status_code) || - SOUP_STATUS_IS_SERVER_ERROR (status_code)) + SOUP_STATUS_IS_SERVER_ERROR (status_code) || + g_error_matches (error, SOUP_SESSION_ERROR, SOUP_SESSION_ERROR_PARSING) || + g_error_matches (error, SOUP_SESSION_ERROR, SOUP_SESSION_ERROR_ENCODING)) { guint random_backoff_interval = get_random_backoff_interval (self->rand, callback_data->attempt_num); diff --git a/tests/daemon/mock-server.py b/tests/daemon/mock-server.py index e6a2ba1..aa05a4e 100755 --- a/tests/daemon/mock-server.py +++ b/tests/daemon/mock-server.py @@ -39,7 +39,13 @@ def do_PUT(self): status_code_str = sys.stdin.readline() status_code = int(status_code_str) - self.send_response(status_code) + if status_code < 0: + # Send garbage back + self.send_header("I am a teapot", "\nhello\n\n\n\r\n\r\n") + self.send_header("X-Y", "Z") + self.send_response(200) + else: + self.send_response(status_code) self.end_headers() diff --git a/tests/daemon/test-daemon.c b/tests/daemon/test-daemon.c index fe5faab..819863c 100644 --- a/tests/daemon/test-daemon.c +++ b/tests/daemon/test-daemon.c @@ -984,6 +984,25 @@ test_daemon_retries_aggregate_uploads (Fixture *fixture, g_test_assert_expected_messages (); } +static void +test_daemon_retries_after_malformed_response (Fixture *fixture, + gconstpointer unused) +{ + record_singulars (fixture->test_object); + + read_network_request (fixture, + (ProcessBytesSourceFunc) assert_singulars_received); + send_http_response (fixture->mock_server, -1); + + g_test_expect_message (G_LOG_DOMAIN, G_LOG_LEVEL_WARNING, + "Attempt to upload metrics failed: " + "Could not parse HTTP response."); + read_network_request (fixture, + (ProcessBytesSourceFunc) assert_singulars_received); + wait_for_upload_to_finish (fixture); + g_test_assert_expected_messages (); +} + static void test_daemon_only_reports_singulars_when_uploading_enabled (Fixture *fixture, gconstpointer unused) @@ -1343,6 +1362,8 @@ main (gint argc, test_daemon_retries_singular_uploads); ADD_DAEMON_TEST ("/daemon/retries-aggregate-uploads", test_daemon_retries_aggregate_uploads); + ADD_DAEMON_TEST ("/daemon/retries-after-malformed-response", + test_daemon_retries_after_malformed_response); ADD_DAEMON_TEST ("/daemon/only-reports-singulars-when-uploading-enabled", test_daemon_only_reports_singulars_when_uploading_enabled); ADD_DAEMON_TEST ("/daemon/only-reports-aggregates-when-uploading-enabled",