diff --git a/cf-check/dump.c b/cf-check/dump.c index f0a920629a..e1f8e2cd42 100644 --- a/cf-check/dump.c +++ b/cf-check/dump.c @@ -162,18 +162,20 @@ static void print_struct_lock_data( static void print_struct_averages( const MDB_val value, const bool strip_strings, const char *tskey_filename) { - assert(sizeof(Averages) == value.mv_size); - if (sizeof(Averages) != value.mv_size) + // Records hold only the in-use slots (AveragesUsedSize), so a short record + // is normal. Copy into a zeroed struct so unstored slots read as zero. + assert(value.mv_size <= sizeof(Averages)); + if (value.mv_size > sizeof(Averages)) { - // Fall back to simple printing in release builds: + // Larger than the struct: fall back to simple printing. print_json_string(value.mv_data, value.mv_size, strip_strings); } else { // TODO: clean up Averages char **obnames = NULL; - Averages averages; - memcpy(&averages, value.mv_data, sizeof(averages)); + Averages averages = {0}; + memcpy(&averages, value.mv_data, value.mv_size); const time_t last_seen = averages.last_seen; obnames = GetObservableNames(tskey_filename); @@ -288,18 +290,7 @@ static void print_struct_or_string( { if (structs) { - if ((StringContains(file, "cf_observations.lmdb") - || StringContains(file, "history.lmdb")) - && StringEqual(key.mv_data, "version")) - { - // After the CF_OBSERVABLES migration (dbm_migration_observations.c) - // these DBs hold a "version" bookkeeping key whose value is a short - // version string, not an Averages struct. Print it as a string; - // otherwise it would reach print_struct_averages() and trip its - // struct-size assertion. - print_json_string(value.mv_data, value.mv_size, strip_strings); - } - else if (StringContains(file, "cf_lastseen.lmdb") + if (StringContains(file, "cf_lastseen.lmdb") && StringStartsWith(key.mv_data, "q")) { print_struct_lastseen_quality(value, strip_strings); diff --git a/cf-check/observables.c b/cf-check/observables.c index 2619ad1dce..d58968bfe4 100644 --- a/cf-check/observables.c +++ b/cf-check/observables.c @@ -5,7 +5,7 @@ #include // FILE_SEPARATOR #include // xstrdup() #include // observable_strings -#include // StringEqual +#include // StringEqual(), StringCopy() #include /** @@ -100,16 +100,19 @@ char **GetObservableNames(const char *ts_key_path) if ((fields != 2) && (fields != 6)) { Log(LOG_LEVEL_ERR, "Wrong line format in ts_key: %s", line); + /* sscanf() may have left name untouched. */ + StringCopy("spare", name, sizeof(name)); } if (StringEqual(name, "spare")) { - temp[i] = xstrdup(name); + /* Numbered, not a shared "spare": these become JSON keys. */ + snprintf(buf, CF_MAXVARSIZE, "spare[%d]", i); + temp[i] = xstrdup(buf); } else { - snprintf(buf, CF_MAXVARSIZE, "spare[%d]", i); - temp[i] = xstrdup(buf); + temp[i] = xstrdup(name); } } fclose(f); diff --git a/cf-monitord/env_monitor.c b/cf-monitord/env_monitor.c index fa84e92115..7328bf5088 100644 --- a/cf-monitord/env_monitor.c +++ b/cf-monitord/env_monitor.c @@ -50,6 +50,7 @@ #include /* MonOtherInit,MonOtherGatherData */ #include /* HistoryUpdate */ #include /* GetObservable */ +#include /* AveragesUsedSize */ #include @@ -773,7 +774,7 @@ static void UpdateAverages(EvalContext *ctx, char *timekey, const Averages *cons Log(LOG_LEVEL_INFO, "Updated averages at '%s'", timekey); - WriteDB(dbp, timekey, newvals, sizeof(Averages)); + WriteDB(dbp, timekey, newvals, AveragesUsedSize()); WriteDB(dbp, "DATABASE_AGE", &AGE, sizeof(double)); CloseDB(dbp); diff --git a/cf-monitord/history.c b/cf-monitord/history.c index e0375b2a1b..d41f617300 100644 --- a/cf-monitord/history.c +++ b/cf-monitord/history.c @@ -26,6 +26,7 @@ #include #include /* MakeTimekey */ +#include /* AveragesUsedSize */ #include #include #include @@ -71,7 +72,7 @@ static void PutRecordForTime(CF_DB *db, time_t time, const Averages *values) MakeTimekey(time, timekey); - WriteDB(db, timekey, values, sizeof(Averages)); + WriteDB(db, timekey, values, AveragesUsedSize()); } static void Nova_SaveFilePosition(const char *handle, const char *name, long fileptr) diff --git a/cf-monitord/monitoring.c b/cf-monitord/monitoring.c index 52d3b4fc36..f58dccffe2 100644 --- a/cf-monitord/monitoring.c +++ b/cf-monitord/monitoring.c @@ -93,14 +93,14 @@ void NovaNamedEvent(const char *eventname, double value) static void Nova_DumpSlots(void) { -#define MAX_KEY_FILE_SIZE 16384 /* usually around 4000, cannot grow much */ - char filename[CF_BUFSIZE]; int i; snprintf(filename, CF_BUFSIZE - 1, "%s%cts_key", GetStateDir(), FILE_SEPARATOR); - char file_contents_new[MAX_KEY_FILE_SIZE] = {0}; + /* Line length depends on the name, description and units a measurement + * promise gives the slot, so there is no useful bound to size a buffer to. */ + Writer *contents = StringWriter(); for (i = 0; i < CF_OBSERVABLES; i++) { @@ -120,12 +120,15 @@ static void Nova_DumpSlots(void) snprintf(line, sizeof(line), "%d,spare,unused\n", i); } - strlcat(file_contents_new, line, sizeof(file_contents_new)); + WriterWrite(contents, line); } + char *file_contents_new = StringWriterClose(contents); + bool contents_changed = true; - Writer *w = FileRead(filename, MAX_KEY_FILE_SIZE, NULL); + /* One byte more than we generated: a longer file differs anyway. */ + Writer *w = FileRead(filename, strlen(file_contents_new) + 1, NULL); if (w) { if(strcmp(StringWriterData(w), file_contents_new) == 0) @@ -145,6 +148,8 @@ static void Nova_DumpSlots(void) GetErrorStr()); } } + + free(file_contents_new); } void GetObservable(int i, char *name, size_t name_size, char *desc, size_t desc_size) @@ -165,8 +170,9 @@ void GetObservable(int i, char *name, size_t name_size, char *desc, size_t desc_ } else { - strncpy(name, OBSERVABLES[i][0], name_size - 1); - strncpy(desc, OBSERVABLES[i][1], desc_size - 1); + /* OBSERVABLES has no rows at or above ob_spare. */ + strncpy(name, "spare", name_size - 1); + strncpy(desc, "unused", desc_size - 1); } } } diff --git a/libpromises/Makefile.am b/libpromises/Makefile.am index 4cef498d15..8f60e0f501 100644 --- a/libpromises/Makefile.am +++ b/libpromises/Makefile.am @@ -84,7 +84,6 @@ libpromises_la_SOURCES = \ dbm_api.c dbm_api.h dbm_api_types.h dbm_priv.h \ dbm_migration.c dbm_migration.h \ dbm_migration_lastseen.c \ - dbm_migration_observations.c \ dbm_lmdb.c \ dbm_quick.c \ dbm_tokyocab.c \ diff --git a/libpromises/constants.c b/libpromises/constants.c index 8ade1ced25..3844ede6b0 100644 --- a/libpromises/constants.c +++ b/libpromises/constants.c @@ -151,236 +151,5 @@ const char *const OBSERVABLES[CF_OBSERVABLES][2] = {"postgres_out", "PostgreSQL database client sessions (out)"}, {"ipp_in", "Internet Printer Protocol (in)"}, {"ipp_out", "Internet Printer Protocol (out)"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - /* ENT-6511: slots 100-299, added when CF_OBSERVABLES was raised - 100->300. Every slot must have a name entry: cf-monitord's - GetObservable() falls back to OBSERVABLES[i] for unregistered - slots, so a table shorter than CF_OBSERVABLES => NULL deref. */ - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, - {"spare", "unused"}, + /* Slots at/after ob_spare are reported "spare" by GetObservable(). */ }; diff --git a/libpromises/dbm_migration.c b/libpromises/dbm_migration.c index a833d1cf89..6066483533 100644 --- a/libpromises/dbm_migration.c +++ b/libpromises/dbm_migration.c @@ -29,7 +29,6 @@ #include extern const DBMigrationFunction dbm_migration_plan_lastseen[]; -extern const DBMigrationFunction dbm_migration_plan_observations[]; #ifndef LMDB @@ -52,11 +51,7 @@ static size_t DBVersion(DBHandle *db) } static const DBMigrationFunction *const dbm_migration_plans[dbid_max] = { - [dbid_lastseen] = dbm_migration_plan_lastseen, - /* Both DBs store the same fixed-size Averages records, so they share the - * plan that expands them when CF_OBSERVABLES grows. */ - [dbid_observations] = dbm_migration_plan_observations, - [dbid_history] = dbm_migration_plan_observations + [dbid_lastseen] = dbm_migration_plan_lastseen }; bool DBMigrate(DBHandle *db, dbid id) diff --git a/libpromises/dbm_migration_observations.c b/libpromises/dbm_migration_observations.c deleted file mode 100644 index d4f7fb0a30..0000000000 --- a/libpromises/dbm_migration_observations.c +++ /dev/null @@ -1,113 +0,0 @@ -/* - Copyright 2026 Northern.tech AS - - This file is part of CFEngine 3 - written and maintained by Northern.tech AS. - - This program is free software; you can redistribute it and/or modify it - under the terms of the GNU General Public License as published by the - Free Software Foundation; version 3. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with this program; if not, write to the Free Software - Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA - - To the extent this program is licensed as part of the Enterprise - versions of CFEngine, the applicable Commercial Open Source License - (COSL) may apply to this file if you as a licensee so wish it. See - included file COSL.txt. -*/ - -#include - -#include -#include - -/* Number of measurement slots (CF_OBSERVABLES) before ENT-6511 raised it from - * 100 to 300. A record written by an agent from before that change is this many - * slots long. */ -#define CF_OBSERVABLES_BEFORE_ENT_6511 100 - -typedef struct -{ - time_t last_seen; - QPoint Q[CF_OBSERVABLES_BEFORE_ENT_6511]; -} AveragesBeforeEnt6511; - -/* - * The observations (cf_observations.lmdb) and history (history.lmdb) databases - * store fixed-size Averages records keyed by time. Raising CF_OBSERVABLES grows - * that struct, so a record written by an older agent is shorter than the current - * struct. cf-monitord's own read path zero-extends short records, and it - * overwrites the observations records on its next cycle, but the history records - * are never rewritten. Migrate every old-size record to the current size, - * zero-filling the added slots, so all records on disk share one layout. - */ -static bool MeasurementsMigrationVersion0(DBHandle *db) -{ - DBCursor *cursor; - if (!NewDBCursor(db, &cursor)) - { - Log(LOG_LEVEL_ERR, - "Unable to create database cursor during measurement DB migration"); - return false; - } - - char *key; - void *value; - int key_size, value_size; - - while (NextDB(cursor, &key, &key_size, &value, &value_size)) - { - /* Only fixed-size Averages records written before CF_OBSERVABLES was - * raised need expanding. Scalar bookkeeping keys (e.g. "DATABASE_AGE" - * and "version") are a different size and are left untouched. */ - if (value_size != (int) sizeof(AveragesBeforeEnt6511)) - { - continue; - } - - /* Copy the old, shorter record into a full-size, zeroed struct so the - * slots added by the larger CF_OBSERVABLES read back as zero. */ - Averages expanded; - memset(&expanded, 0, sizeof(expanded)); - memcpy(&expanded, value, sizeof(AveragesBeforeEnt6511)); - - // This will overwrite the entry - if (!DBCursorWriteEntry(cursor, &expanded, sizeof(expanded))) - { - Log(LOG_LEVEL_ERR, - "Unable to expand measurement record for key '%s' during migration", - key); - DeleteDBCursor(cursor); - return false; - } - } - - if (!DeleteDBCursor(cursor)) - { - Log(LOG_LEVEL_ERR, - "Unable to close cursor during measurement DB migration"); - return false; - } - - if (!WriteDB(db, "version", "1", sizeof("1"))) - { - Log(LOG_LEVEL_ERR, - "Failed to update version number during measurement DB migration"); - return false; - } - - Log(LOG_LEVEL_INFO, "Migrated measurement database to version 1"); - return true; -} - -const DBMigrationFunction dbm_migration_plan_observations[] = -{ - MeasurementsMigrationVersion0, - NULL -}; diff --git a/libpromises/monitoring_read.c b/libpromises/monitoring_read.c index de0aae753b..363ce02784 100644 --- a/libpromises/monitoring_read.c +++ b/libpromises/monitoring_read.c @@ -25,6 +25,7 @@ #include +#include /* offsetof */ #include /* FILE_SEPARATOR */ #include @@ -225,6 +226,27 @@ bool NovaHasSlot(int idx) return idx < ob_spare || SLOTS[idx - ob_spare]; } +/** + * Bytes of an Averages record in use: the built-in observables plus every slot + * through the highest one a measurement has registered. cf-monitord writes only + * this much, so unused slots cost no disk. + */ +size_t AveragesUsedSize(void) +{ + Nova_LoadSlots(); + + size_t slots = ob_spare; + for (int i = CF_OBSERVABLES - 1; i >= ob_spare; i--) + { + if (SLOTS[i - ob_spare] != NULL) + { + slots = i + 1; + break; + } + } + return offsetof(Averages, Q) + slots * sizeof(QPoint); +} + const char *NovaGetSlotName(int idx) { Nova_LoadSlots(); @@ -324,6 +346,8 @@ bool GetRecordForTime(CF_DB *db, time_t time, Averages *result) MakeTimekey(time, timekey); + /* Records may be short (AveragesUsedSize); unstored slots must read zero. */ + memset(result, 0, sizeof(Averages)); return ReadDB(db, timekey, result, sizeof(Averages)); } diff --git a/libpromises/monitoring_read.h b/libpromises/monitoring_read.h index 0acad5630d..6ceb24b50c 100644 --- a/libpromises/monitoring_read.h +++ b/libpromises/monitoring_read.h @@ -40,6 +40,7 @@ MonitoringSlot *Nova_MakeSlot(const char *name, const char *description, bool consolidable); void Nova_LoadSlots(void); bool NovaHasSlot(int idx); +size_t AveragesUsedSize(void); const char *NovaGetSlotName(int idx); const char *NovaGetSlotDescription(int index); const char *NovaGetSlotUnits(int index); diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am index 3864b67876..024e25f78b 100644 --- a/tests/unit/Makefile.am +++ b/tests/unit/Makefile.am @@ -68,7 +68,6 @@ libdb_la_SOURCES = db_stubs.c \ ../../libpromises/dbm_lmdb.c \ ../../libpromises/dbm_migration.c \ ../../libpromises/dbm_migration_lastseen.c \ - ../../libpromises/dbm_migration_observations.c \ ../../libpromises/global_mutex.c \ ../../cf-check/backup.c \ ../../cf-check/diagnose.c \ @@ -104,7 +103,8 @@ check_PROGRAMS = \ lastseen_test \ lastseen_migration_test \ changes_migration_test \ - observations_migration_test \ + observations_storage_test \ + observables_names_test \ db_test \ db_concurrent_test \ item_lib_test \ @@ -263,8 +263,11 @@ db_concurrent_test_SOURCES = db_concurrent_test.c #db_concurrent_test_CPPFLAGS = $(libdb_la_CPPFLAGS) db_concurrent_test_LDADD = libtest.la libdb.la -observations_migration_test_SOURCES = observations_migration_test.c -observations_migration_test_LDADD = libtest.la libdb.la +observations_storage_test_SOURCES = observations_storage_test.c +observations_storage_test_LDADD = libtest.la ../../libpromises/libpromises.la + +observables_names_test_SOURCES = observables_names_test.c \ + ../../cf-check/observables.c lastseen_test_SOURCES = lastseen_test.c \ ../../libntech/libutils/statistics.c diff --git a/tests/unit/observables_names_test.c b/tests/unit/observables_names_test.c new file mode 100644 index 0000000000..43b2ebbc98 --- /dev/null +++ b/tests/unit/observables_names_test.c @@ -0,0 +1,230 @@ +/* + Copyright 2026 Northern.tech AS + + This file is part of CFEngine 3 - written and maintained by Northern.tech AS. + + This program is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by the + Free Software Foundation; version 3. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA + + To the extent this program is licensed as part of the Enterprise + versions of CFEngine, the applicable Commercial Open Source License + (COSL) may apply to this file if you as a licensee so wish it. See + included file COSL.txt. +*/ + +#include +#include + +#include /* CF_BUFSIZE, CF_MAXVARSIZE */ +#include /* observables_max, observable_strings */ +#include /* CF_OBSERVABLES, GetObservableNames */ +#include /* DeleteDirectoryTree, safe_fopen */ +#include /* StringEqual */ +#include /* xsnprintf */ + +/* cf-check's built-in list ends with a pseudo-entry named "spare", so + * observables_max is one more than the real named count. */ +#define LAST_NAMED_OBSERVABLE (observables_max - 2) +#define FIRST_SPARE_SLOT (observables_max - 1) + +/* Offsets from the first spare slot: the slot the test ts_key names, and the + * one whose line it corrupts. */ +#define CUSTOM_SLOT_OFFSET 8 +#define MALFORMED_SLOT_OFFSET 20 + +static char TESTDIR[CF_BUFSIZE]; + +/** + * dump.c indexes and free()s every entry, so a hole is an invalid free, not just + * a missing name. + */ +static void assert_all_names_present(char **names) +{ + assert_true(names != NULL); + for (int i = 0; i < CF_OBSERVABLES; i++) + { + assert_true(names[i] != NULL); + } +} + +static void free_names(char **names) +{ + for (int i = 0; i < CF_OBSERVABLES; i++) + { + free(names[i]); + } + free(names); +} + +/** + * Writes a ts_key as Nova_DumpSlots() does. Fewer than CF_OBSERVABLES lines + * gives the short file an older, smaller-CF_OBSERVABLES agent left behind. + */ +static void write_ts_key( + char *path, size_t path_size, const char *name, int lines, bool with_malformed) +{ + xsnprintf(path, path_size, "%s/%s", TESTDIR, name); + + FILE *f = safe_fopen(path, "w"); + assert_true(f != NULL); + + for (int i = 0; i < lines; i++) + { + if (i < observables_max) + { + fprintf(f, "%d,%s,Built-in observable %d\n", i, observable_strings[i], i); + } + else if (i == FIRST_SPARE_SLOT + CUSTOM_SLOT_OFFSET) + { + fprintf(f, "%d,test_measurement,A test measurement,units,0.000,100.000,1\n", i); + } + else if (with_malformed && i == FIRST_SPARE_SLOT + MALFORMED_SLOT_OFFSET) + { + fprintf(f, "%d no commas here at all\n", i); + } + else + { + fprintf(f, "%d,spare,unused\n", i); + } + } + + fclose(f); +} + +/* Before the fix, named observables came back as "spare[]" and every + * spare slot shared the name "spare", collapsing the tail into one JSON key. */ +static void test_named_slots_report_their_name(void) +{ + char path[CF_BUFSIZE]; + write_ts_key(path, sizeof(path), "ts_key_full", CF_OBSERVABLES, false); + char **names = GetObservableNames(path); + assert_all_names_present(names); + + /* Built-ins report the name ts_key gives them. */ + assert_true(StringEqual(names[0], observable_strings[0])); + assert_true(StringEqual(names[LAST_NAMED_OBSERVABLE], + observable_strings[LAST_NAMED_OBSERVABLE])); + + /* A custom measurement reports its name, not a placeholder. */ + const int custom = FIRST_SPARE_SLOT + CUSTOM_SLOT_OFFSET; + assert_true(StringEqual(names[custom], "test_measurement")); + + /* Nothing keeps the bare "spare" that used to collapse them. */ + for (int i = 0; i < CF_OBSERVABLES; i++) + { + assert_false(StringEqual(names[i], "spare")); + } + + free_names(names); +} + +/* The names become JSON keys, so two spares sharing one lose a slot. */ +static void test_spare_slots_are_numbered_and_distinct(void) +{ + char path[CF_BUFSIZE]; + write_ts_key(path, sizeof(path), "ts_key_spares", CF_OBSERVABLES, false); + char **names = GetObservableNames(path); + assert_all_names_present(names); + + char expected[CF_MAXVARSIZE]; + const int first_spare = FIRST_SPARE_SLOT; + const int last_spare = CF_OBSERVABLES - 1; + + xsnprintf(expected, sizeof(expected), "spare[%d]", first_spare); + assert_true(StringEqual(names[first_spare], expected)); + + xsnprintf(expected, sizeof(expected), "spare[%d]", last_spare); + assert_true(StringEqual(names[last_spare], expected)); + + assert_false(StringEqual(names[first_spare], names[last_spare])); + + free_names(names); +} + +/* sscanf() may leave name untouched, so a bad line must not be read as a name. */ +static void test_malformed_line_falls_back_to_numbered_spare(void) +{ + char path[CF_BUFSIZE]; + write_ts_key(path, sizeof(path), "ts_key_malformed", CF_OBSERVABLES, true); + char **names = GetObservableNames(path); + assert_all_names_present(names); + + const int bad = FIRST_SPARE_SLOT + MALFORMED_SLOT_OFFSET; + char expected[CF_MAXVARSIZE]; + xsnprintf(expected, sizeof(expected), "spare[%d]", bad); + assert_true(StringEqual(names[bad], expected)); + + free_names(names); +} + +/* A short ts_key must still fill every slot, or the caller free()s garbage. */ +static void test_short_ts_key_fills_the_remaining_slots(void) +{ + const int short_lines = 100; + char path[CF_BUFSIZE]; + write_ts_key(path, sizeof(path), "ts_key_short", short_lines, false); + char **names = GetObservableNames(path); + assert_all_names_present(names); + + char expected[CF_MAXVARSIZE]; + xsnprintf(expected, sizeof(expected), "spare[%d]", short_lines); + assert_true(StringEqual(names[short_lines], expected)); + + xsnprintf(expected, sizeof(expected), "spare[%d]", CF_OBSERVABLES - 1); + assert_true(StringEqual(names[CF_OBSERVABLES - 1], expected)); + + free_names(names); +} + +/* With no ts_key, names come from the built-in table. */ +static void test_missing_ts_key_uses_the_builtin_table(void) +{ + char path[CF_BUFSIZE]; + xsnprintf(path, CF_BUFSIZE, "%s/does_not_exist", TESTDIR); + + char **names = GetObservableNames(path); + assert_all_names_present(names); + + assert_true(StringEqual(names[0], observable_strings[0])); + + char expected[CF_MAXVARSIZE]; + xsnprintf(expected, sizeof(expected), "observable[%d]", CF_OBSERVABLES - 1); + assert_true(StringEqual(names[CF_OBSERVABLES - 1], expected)); + + free_names(names); +} + +int main(void) +{ + char template[] = "/tmp/observables_names_test.XXXXXX"; + assert_true(mkdtemp(template) != NULL); + strlcpy(TESTDIR, template, CF_BUFSIZE); + + const UnitTest tests[] = + { + unit_test(test_named_slots_report_their_name), + unit_test(test_spare_slots_are_numbered_and_distinct), + unit_test(test_malformed_line_falls_back_to_numbered_spare), + unit_test(test_short_ts_key_fills_the_remaining_slots), + unit_test(test_missing_ts_key_uses_the_builtin_table), + }; + + PRINT_TEST_BANNER(); + int ret = run_tests(tests); + + /* DeleteDirectoryTree() keeps the top directory, so rmdir() it too. */ + DeleteDirectoryTree(TESTDIR); + rmdir(TESTDIR); + + return ret; +} diff --git a/tests/unit/observations_migration_test.c b/tests/unit/observations_migration_test.c deleted file mode 100644 index 02f2565c79..0000000000 --- a/tests/unit/observations_migration_test.c +++ /dev/null @@ -1,242 +0,0 @@ -#include -#include -#include -#include - -#include -#include -#include /* StringEqual */ -#include /* DeleteDirectoryTree */ - -#ifndef LMDB - -/* The measurement DB migration only exists for the LMDB backend (see - * dbm_migration.c); with other backends DBMigrate() is a no-op. */ -int main(void) -{ - return 0; -} - -#else - -/* The Averages layout from before ENT-6511 raised CF_OBSERVABLES from 100 to - * 300. A record written by such an agent is this size on disk. */ -#define CF_OBSERVABLES_BEFORE_ENT_6511 100 - -typedef struct -{ - time_t last_seen; - QPoint Q[CF_OBSERVABLES_BEFORE_ENT_6511]; -} AveragesBeforeEnt6511; - -char CFWORKDIR[CF_BUFSIZE]; - -void tests_setup(void) -{ - static char env[] = /* Needs to be static for putenv() */ - "CFENGINE_TEST_OVERRIDE_WORKDIR=/tmp/observations_migration_test.XXXXXX"; - - char *workdir = strchr(env, '=') + 1; /* start of the path */ - assert(workdir - 1 && workdir[0] == '/'); - - mkdtemp(workdir); - strlcpy(CFWORKDIR, workdir, CF_BUFSIZE); - putenv(env); - mkdir(GetStateDir(), (S_IRWXU | S_IRWXG | S_IRWXO)); -} - -static void tests_teardown(void) -{ - /* DeleteDirectoryTree() empties the tree but keeps the top directory, so - * rmdir() the now-empty workdir afterwards. */ - DeleteDirectoryTree(CFWORKDIR); - rmdir(CFWORKDIR); -} - -/* - * Provides an observations DB in a pre-migration (version 0) state: OpenDB() - * runs the migration and writes a "version" marker, so wipe every entry to get - * back to how an old, never-migrated database looks on disk. - */ -static DBHandle *setup_unversioned(void) -{ - /* Empties the state dir (keeps the dir itself) so OpenDB() starts fresh. */ - assert_true(DeleteDirectoryTree(GetStateDir())); - - DBHandle *db; - assert_true(OpenDB(&db, dbid_observations)); - - DBCursor *cursor; - assert_true(NewDBCursor(db, &cursor)); - - char *key; - void *value; - int ksize, vsize; - while (NextDB(cursor, &key, &ksize, &value, &vsize)) - { - DBCursorDeleteEntry(cursor); - } - assert_true(DeleteDBCursor(cursor)); - - return db; -} - -/* Returns the on-disk size of a record, or -1 if the key is absent. */ -static int record_size(DBHandle *db, const char *want_key) -{ - DBCursor *cursor; - assert_true(NewDBCursor(db, &cursor)); - - char *key; - void *value; - int ksize, vsize; - int found = -1; - while (NextDB(cursor, &key, &ksize, &value, &vsize)) - { - if (StringEqual(key, want_key)) - { - found = vsize; - } - } - assert_true(DeleteDBCursor(cursor)); - return found; -} - -static void test_migrate_expands_old_record(void) -{ - DBHandle *db = setup_unversioned(); - - /* A measurement record in the old, shorter (100-slot) layout ... */ - AveragesBeforeEnt6511 old; - memset(&old, 0, sizeof(old)); - old.last_seen = 1234567; - old.Q[0].q = 1.0; - old.Q[0].expect = 2.0; - old.Q[0].var = 3.0; - old.Q[0].dq = 4.0; - old.Q[CF_OBSERVABLES_BEFORE_ENT_6511 - 1].q = 5.0; - old.Q[CF_OBSERVABLES_BEFORE_ENT_6511 - 1].expect = 6.0; - old.Q[CF_OBSERVABLES_BEFORE_ENT_6511 - 1].var = 7.0; - old.Q[CF_OBSERVABLES_BEFORE_ENT_6511 - 1].dq = 8.0; - assert_true(WriteDB(db, "Mon_Hr12_Q1", &old, sizeof(old))); - - /* ... and a scalar bookkeeping record that must NOT be touched. */ - double age = 42.0; - assert_true(WriteDB(db, "DATABASE_AGE", &age, sizeof(age))); - - CloseDB(db); - - /* Reopening runs the migration (no version marker present yet). */ - assert_true(OpenDB(&db, dbid_observations)); - - /* The measurement record is now the full, current size ... */ - assert_int_equal(record_size(db, "Mon_Hr12_Q1"), (int) sizeof(Averages)); - /* ... the scalar record is left at its original size ... */ - assert_int_equal(record_size(db, "DATABASE_AGE"), (int) sizeof(double)); - /* ... and the version marker was written. */ - assert_true(HasKeyDB(db, "version", strlen("version") + 1)); - - /* The original slots are preserved and the new slots read back as zero. */ - Averages migrated; - memset(&migrated, 0, sizeof(migrated)); - assert_true(ReadDB(db, "Mon_Hr12_Q1", &migrated, sizeof(migrated))); - assert_int_equal(migrated.last_seen, 1234567); - assert_double_close(migrated.Q[0].q, 1.0); - assert_double_close(migrated.Q[0].dq, 4.0); - assert_double_close(migrated.Q[CF_OBSERVABLES_BEFORE_ENT_6511 - 1].q, 5.0); - assert_double_close(migrated.Q[CF_OBSERVABLES_BEFORE_ENT_6511 - 1].dq, 8.0); - assert_double_close(migrated.Q[CF_OBSERVABLES_BEFORE_ENT_6511].q, 0.0); - assert_double_close(migrated.Q[CF_OBSERVABLES - 1].var, 0.0); - - double read_age = 0.0; - assert_true(ReadDB(db, "DATABASE_AGE", &read_age, sizeof(read_age))); - assert_double_close(read_age, 42.0); - - CloseDB(db); -} - -static void test_current_record_untouched(void) -{ - /* A record already at the current size must not be altered by the - * migration (the size-exact check must ignore it). */ - DBHandle *db = setup_unversioned(); - - Averages current; - memset(¤t, 0, sizeof(current)); - current.last_seen = 7654321; - current.Q[CF_OBSERVABLES - 1].q = 99.0; - assert_true(WriteDB(db, "Tue_Hr06_Q2", ¤t, sizeof(current))); - CloseDB(db); - - assert_true(OpenDB(&db, dbid_observations)); - - assert_int_equal(record_size(db, "Tue_Hr06_Q2"), (int) sizeof(Averages)); - - Averages read; - memset(&read, 0, sizeof(read)); - assert_true(ReadDB(db, "Tue_Hr06_Q2", &read, sizeof(read))); - assert_int_equal(read.last_seen, 7654321); - assert_double_close(read.Q[CF_OBSERVABLES - 1].q, 99.0); - - CloseDB(db); -} - -static void test_migration_is_idempotent(void) -{ - /* Once migrated, reopening must not run the migration again nor disturb - * the data. */ - DBHandle *db = setup_unversioned(); - - AveragesBeforeEnt6511 old; - memset(&old, 0, sizeof(old)); - old.last_seen = 111; - assert_true(WriteDB(db, "Wed_Hr18_Q3", &old, sizeof(old))); - CloseDB(db); - - /* First open migrates. */ - assert_true(OpenDB(&db, dbid_observations)); - assert_int_equal(record_size(db, "Wed_Hr18_Q3"), (int) sizeof(Averages)); - CloseDB(db); - - /* Second open is a no-op. */ - assert_true(OpenDB(&db, dbid_observations)); - assert_int_equal(record_size(db, "Wed_Hr18_Q3"), (int) sizeof(Averages)); - - char version[8]; - assert_true(ReadDB(db, "version", version, sizeof(version))); - assert_string_equal(version, "1"); - - CloseDB(db); -} - -int main(void) -{ - tests_setup(); - - const UnitTest tests[] = - { - unit_test(test_migrate_expands_old_record), - unit_test(test_current_record_untouched), - unit_test(test_migration_is_idempotent), - }; - - PRINT_TEST_BANNER(); - int ret = run_tests(tests); - - tests_teardown(); - return ret; -} - -/* STUBS */ - -/* OpenDB() pulls in libpromises DB code that references FatalError(), but the - * real implementation lives in the daemons, not the test binary. Provide a stub - * so the test links; the migration paths under test never call it, so reaching - * it is itself a test failure. */ -void FatalError(ARG_UNUSED char *s, ...) -{ - fail(); - exit(42); -} - -#endif // LMDB diff --git a/tests/unit/observations_storage_test.c b/tests/unit/observations_storage_test.c new file mode 100644 index 0000000000..3630b08a74 --- /dev/null +++ b/tests/unit/observations_storage_test.c @@ -0,0 +1,191 @@ +/* + Copyright 2026 Northern.tech AS + + This file is part of CFEngine 3 - written and maintained by Northern.tech AS. + + This program is free software; you can redistribute it and/or modify it + under the terms of the GNU General Public License as published by the + Free Software Foundation; version 3. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA + + To the extent this program is licensed as part of the Enterprise + versions of CFEngine, the applicable Commercial Open Source License + (COSL) may apply to this file if you as a licensee so wish it. See + included file COSL.txt. +*/ + +#include +#include /* offsetof */ +#include +#include +#include + +#include +#include +#include +#include /* DeleteDirectoryTree */ +#include /* xsnprintf */ + +/* The custom slot this test registers, offset from the first spare slot. */ +#define TEST_CUSTOM_SLOT_OFFSET 9 + +/* Stale read-buffer contents, distinct from zero and from any stored value. */ +#define STALE 12345.0 + +char CFWORKDIR[CF_BUFSIZE]; + +/* The size of a record holding slots 0..last_slot inclusive. */ +static size_t size_through_slot(int last_slot) +{ + return offsetof(Averages, Q) + (size_t) (last_slot + 1) * sizeof(QPoint); +} + +void tests_setup(void) +{ + static char env[] = /* Needs to be static for putenv() */ + "CFENGINE_TEST_OVERRIDE_WORKDIR=/tmp/observations_storage_test.XXXXXX"; + + char *workdir = strchr(env, '=') + 1; /* start of the path */ + assert(workdir - 1 && workdir[0] == '/'); + + mkdtemp(workdir); + strlcpy(CFWORKDIR, workdir, CF_BUFSIZE); + putenv(env); + mkdir(GetStateDir(), (S_IRWXU | S_IRWXG | S_IRWXO)); +} + +static void tests_teardown(void) +{ + /* DeleteDirectoryTree() keeps the top directory, so rmdir() it too. */ + DeleteDirectoryTree(CFWORKDIR); + rmdir(CFWORKDIR); +} + +/** + * Writes a ts_key as Nova_DumpSlots() does, registering one custom measurement + * so the in-use range reaches past the built-in observables. + */ +static void write_ts_key_with_one_custom_slot(void) +{ + char filename[CF_BUFSIZE]; + xsnprintf(filename, CF_BUFSIZE, "%s%cts_key", GetStateDir(), FILE_SEPARATOR); + + FILE *f = safe_fopen(filename, "w"); + assert_true(f != NULL); + + for (int i = 0; i < CF_OBSERVABLES; i++) + { + if (i == ob_spare + TEST_CUSTOM_SLOT_OFFSET) + { + fprintf(f, "%d,test_measurement,A test measurement,units,0.000,100.000,1\n", i); + } + else + { + fprintf(f, "%d,spare,unused\n", i); + } + } + + fclose(f); +} + +static void test_used_size_without_custom_slots(void) +{ + /* No ts_key yet, so only the built-in observables are in use. */ + assert_int_equal(AveragesUsedSize(), size_through_slot(ob_spare - 1)); + + /* The point of the change: less than a full-size record. */ + assert_true(AveragesUsedSize() < sizeof(Averages)); +} + +static void test_used_size_covers_highest_registered_slot(void) +{ + write_ts_key_with_one_custom_slot(); + + /* Through the registered slot and no further. */ + assert_int_equal( + AveragesUsedSize(), + size_through_slot(ob_spare + TEST_CUSTOM_SLOT_OFFSET)); + assert_true(AveragesUsedSize() < sizeof(Averages)); +} + +static void test_short_record_reads_back_zero_extended(void) +{ + /* Stored slots must read back intact and the unstored tail as zero, even + * when the caller's buffer held something else first. */ + const time_t when = 1234567890; + const int custom_slot = ob_spare + TEST_CUSTOM_SLOT_OFFSET; + const size_t used = AveragesUsedSize(); + + CF_DB *db; + assert_true(OpenDB(&db, dbid_observations)); + + Averages written; + memset(&written, 0, sizeof(written)); + written.last_seen = when; + written.Q[0].q = 1.0; + written.Q[0].expect = 2.0; + written.Q[0].var = 3.0; + written.Q[0].dq = 4.0; + written.Q[custom_slot].q = 5.0; + written.Q[custom_slot].dq = 6.0; + + char timekey[CF_MAXVARSIZE]; + MakeTimekey(when, timekey); + assert_true(WriteDB(db, timekey, &written, used)); + + /* Stale doubles, not a memset byte pattern: 0xAA-filled bytes read back as + * ~1e-103, which assert_double_close() cannot tell from zero. */ + Averages read_back; + read_back.last_seen = -1; + for (int i = 0; i < CF_OBSERVABLES; i++) + { + read_back.Q[i].q = STALE; + read_back.Q[i].expect = STALE; + read_back.Q[i].var = STALE; + read_back.Q[i].dq = STALE; + } + assert_true(GetRecordForTime(db, when, &read_back)); + + assert_int_equal(read_back.last_seen, when); + assert_double_close(read_back.Q[0].q, 1.0); + assert_double_close(read_back.Q[0].expect, 2.0); + assert_double_close(read_back.Q[0].var, 3.0); + assert_double_close(read_back.Q[0].dq, 4.0); + assert_double_close(read_back.Q[custom_slot].q, 5.0); + assert_double_close(read_back.Q[custom_slot].dq, 6.0); + + /* Never stored. */ + assert_double_close(read_back.Q[custom_slot + 1].q, 0.0); + assert_double_close(read_back.Q[CF_OBSERVABLES - 1].q, 0.0); + assert_double_close(read_back.Q[CF_OBSERVABLES - 1].dq, 0.0); + + CloseDB(db); +} + +int main(void) +{ + tests_setup(); + + /* Order matters: the first test needs no ts_key, the rest need the custom + * slot the second one registers. */ + const UnitTest tests[] = + { + unit_test(test_used_size_without_custom_slots), + unit_test(test_used_size_covers_highest_registered_slot), + unit_test(test_short_record_reads_back_zero_extended), + }; + + PRINT_TEST_BANNER(); + int ret = run_tests(tests); + + tests_teardown(); + return ret; +}