in_calyptia_fleet: fleet config fetch ignores older-timestamped files #10746
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
alecholmes
requested review from
edsiper,
fujimotos and
koleini
as code owners
WalkthroughAdds timestamp-aware gating for fleet config files: new helper parses numeric timestamps from config filenames, scans existing timestamped files, and requires an incoming Last-Modified timestamp to be strictly newer before creating a timestamped config file. No public API changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FleetPlugin as get_calyptia_file
participant Checker as check_timestamp_is_newer
participant FS as Filesystem
Client->>FleetPlugin: Request config URL + Last-Modified header
FleetPlugin->>Checker: Is Last-Modified strictly newer?
Checker->>FS: Glob files (*.conf / *.yaml)
FS-->>Checker: File list
Checker->>Checker: Extract numeric timestamps
Checker-->>FleetPlugin: FLB_TRUE / FLB_FALSE
alt Newer
FleetPlugin->>FS: Create timestamped config file (time-based name)
FS-->>FleetPlugin: Creation result
else Not newer
FleetPlugin-->>Client: Skip creation (debug log)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Contributor
Author
|
Valgrind output. The |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (1)
1088-1104: Fix HTTP date parsing and avoid parsing a non-terminated buffer; treat “not newer” as non-error
- HTTP Last-Modified is RFC 1123 (e.g., “Mon, 02 Jan 2006 …”), so month should be %b (abbrev), not %B (full). Parsing with %B can silently fail and yield a 0 timestamp.
- fbit_last_modified is not null-terminated; copy and terminate before flb_strptime to avoid reading past CRLF.
- Returning -1 for the “not newer” path overloads error semantics. Returning 0 makes it clear to callers that nothing was created but no error occurred.
Apply this diff:
- flb_strptime(fbit_last_modified, "%a, %d %B %Y %H:%M:%S GMT", &tm_last_modified); - last_modified = mktime(&tm_last_modified.tm); + /* Copy and null-terminate header before parsing */ + char *lm_buf = flb_malloc((size_t) fbit_last_modified_len + 1); + if (lm_buf == NULL) { + ret = -1; + goto client_error; + } + memcpy(lm_buf, fbit_last_modified, (size_t) fbit_last_modified_len); + lm_buf[fbit_last_modified_len] = '\0'; + + /* RFC 1123: abbreviated month name (%b) */ + if (flb_strptime(lm_buf, "%a, %d %b %Y %H:%M:%S GMT", &tm_last_modified) == NULL) { + flb_plg_warn(ctx->ins, "failed to parse Last-Modified header: %s", lm_buf); + flb_free(lm_buf); + ret = -1; + goto client_error; + } + flb_free(lm_buf); + last_modified = mktime(&tm_last_modified.tm);And consider making the “not newer” return neutral:
- ret = -1; + ret = 0; goto client_error;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
plugins/in_calyptia_fleet/in_calyptia_fleet.c(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-centos-7
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
🔇 Additional comments (1)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (1)
67-67: Forward declaration for read_glob is appropriateThis resolves earlier references and avoids implicit declaration warnings. Signature matches both platform-specific implementations below.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (3)
323-361: Avoid 32-bit truncation: return time_t from fleet_config_path_timestamp()The function parses a timestamp into a long, but returns int. This downcast risks truncation on platforms where time_t/long exceed 32 bits (2038+ and/or LLP64). Returning time_t aligns with the semantic intent and avoids narrowing.
Apply:
-/** - * Returns the timestamp of the fleet config file if it is a timestamped file, - * or 0 if it is not a timestamped file. - */ -static int fleet_config_path_timestamp(struct flb_in_calyptia_fleet_config *ctx, const char *path) +/** + * Returns the timestamp of the fleet config file if it is a timestamped file, + * or 0 if it is not a timestamped file. + */ +static time_t fleet_config_path_timestamp(struct flb_in_calyptia_fleet_config *ctx, const char *path) { char *fname; char *end; long val; @@ - if (ctx->fleet_config_legacy_format) { + if (ctx->fleet_config_legacy_format) { if (strcmp(end, ".conf") == 0) { - return val; + return (time_t) val; } } else if (strcmp(end, ".yaml") == 0) { - return val; + return (time_t) val; } return 0; }Follow-up: Update any local variables that store this return value to time_t (see suggestion below in check_timestamp_is_newer()).
973-1042: Tighten types, remove unused vars, and confirm glob-failure semantics
- file_timestamp should be time_t to match the value being compared (and to match the proposed return type).
- ext, fname, path_copy are unused; remove them to avoid warnings and improve clarity.
- Semantics: treating a glob failure as “no existing files” (return FLB_TRUE) is acceptable for resilience, but it means we may accept older configs if directory reads fail. If that’s intended, consider a short comment to document it.
Apply:
static int check_timestamp_is_newer(struct flb_in_calyptia_fleet_config *ctx, time_t new_timestamp) { flb_sds_t base_dir = NULL; flb_sds_t glob_pattern = NULL; struct cfl_array *files = NULL; size_t idx; int ret; - long file_timestamp; - char *ext; - char *fname; - char *path_copy = NULL; + time_t file_timestamp; const char *file_extension; @@ - files = read_glob(glob_pattern); + files = read_glob(glob_pattern); if (files == NULL) { - /* No existing files found - could be empty directory or glob failure */ + /* No existing files found or glob failure; treat as “newer” to stay resilient */ flb_plg_debug(ctx->ins, "no existing config files found in %s", base_dir); flb_sds_destroy(base_dir); flb_sds_destroy(glob_pattern); return FLB_TRUE; } @@ - file_timestamp = fleet_config_path_timestamp(ctx, files->entries[idx]->data.as_string); + file_timestamp = fleet_config_path_timestamp(ctx, files->entries[idx]->data.as_string); if (file_timestamp == 0) { continue; } /* Check if existing file timestamp is greater than or equal to new timestamp */ - if (file_timestamp >= (long)new_timestamp) { + if (file_timestamp >= new_timestamp) { flb_plg_debug(ctx->ins, "existing file with timestamp %ld >= new timestamp %ld", - file_timestamp, (long)new_timestamp); + (long) file_timestamp, (long) new_timestamp); ret = FLB_FALSE; break; } }Optional: for performance, you could compare only against the newest timestamped file (via calyptia_config_get_newest()) instead of scanning all entries.
1081-1092: Return a non-error when skipping older/equal configWhen the incoming Last-Modified is not strictly newer, this is a normal “no-op,” not an error. Prefer returning 0 over -1 to avoid conflating expected behavior with failure in upstream callers or metrics.
Apply:
- if (check_timestamp_is_newer(ctx, last_modified) == FLB_FALSE) { + if (check_timestamp_is_newer(ctx, last_modified) == FLB_FALSE) { flb_plg_debug(ctx->ins, "not creating file with timestamp %ld since it is not newer than existing files", (long)last_modified); - ret = -1; + ret = 0; goto client_error; } else { flb_plg_info(ctx->ins, "creating config file with timestamp %ld", (long)last_modified); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
plugins/in_calyptia_fleet/in_calyptia_fleet.c(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (2)
src/flb_sds.c (3)
flb_sds_create_size(92-95)flb_sds_destroy(389-399)flb_sds_printf(336-387)lib/cfl/src/cfl_array.c (1)
cfl_array_destroy(51-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (4)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (4)
67-67: Forward declaration for read_glob is correct and necessaryThis unblocks earlier call sites across POSIX/Windows variants. No issues.
373-374: Switch to timestamp check via fleet_config_path_timestamp() looks goodUsing a numeric timestamp check (> 0) is clearer than a boolean filename check.
1446-1448: Good switch to numeric timestamp check in delete_oldFiltering by fleet_config_path_timestamp(...) > 0 avoids accidental deletions of non-conforming files.
1512-1515: Newest-config selection now properly guards with timestamp parsingUsing fleet_config_path_timestamp(...) > 0 keeps the selection restricted to timestamped files. Looks good.
alecholmes
force-pushed
the
alec/2025-08-14-fleet-ignore-older-configs
branch
from
e2b4767 to
5412519
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (2)
323-361: Make timestamp parsing 64-bit-safe and reject non-positive valuesOn LLP64 platforms (e.g., Windows), long is 32-bit while time_t is 64-bit. Parsing filenames with strtol into long risks overflow around 2038 and loses range headroom. Also, negative or zero timestamps should be rejected explicitly.
- Parse with strtoll and check end==fname, ERANGE, and val <= 0.
- Keep returning 0 for “not timestamped”.
Apply this diff:
static time_t fleet_config_path_timestamp(struct flb_in_calyptia_fleet_config *ctx, const char *path) { char *fname; char *end; - long val; + long long val; if (path == NULL || ctx == NULL) { return 0; } fname = strrchr(path, PATH_SEPARATOR[0]); if (fname == NULL) { return 0; } fname++; errno = 0; - val = strtol(fname, &end, 10); - if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) || (errno != 0 && val == 0)) { + val = strtoll(fname, &end, 10); + if (errno == ERANGE) { + return 0; + } + /* No digits parsed or non-positive timestamp is not considered valid */ + if (end == fname || val <= 0) { return 0; } if (ctx->fleet_config_legacy_format) { if (strcmp(end, ".conf") == 0) { - return (time_t) val; + return (time_t) val; } } else if (strcmp(end, ".yaml") == 0) { - return (time_t) val; + return (time_t) val; } return 0; }
1078-1089: Return “no-op” (0) instead of -1 when skipping older/equal configs; fix timestamp logging formatSkipping creation for older/equal timestamps is not an error. Returning 0 keeps get_calyptia_fleet_config() semantics clearer (no new file) and avoids conflating “intentional skip” with errors. Also, print timestamps in a 64-bit-safe way.
Apply this diff:
- if (check_timestamp_is_newer(ctx, last_modified) == FLB_FALSE) { - flb_plg_debug(ctx->ins, "not creating file with timestamp %ld since it is not newer than existing files", - (long)last_modified); - ret = -1; - goto client_error; - } - else { - flb_plg_info(ctx->ins, "creating config file with timestamp %ld", - (long)last_modified); - } + if (check_timestamp_is_newer(ctx, last_modified) == FLB_FALSE) { + flb_plg_debug(ctx->ins, + "not creating file with timestamp %lld since it is not newer than existing files", + (long long) last_modified); + ret = 0; /* no new file created */ + goto client_error; + } + else { + flb_plg_info(ctx->ins, "creating config file with timestamp %lld", + (long long) last_modified); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
plugins/in_calyptia_fleet/in_calyptia_fleet.c(7 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (2)
src/flb_sds.c (3)
flb_sds_create_size(92-95)flb_sds_destroy(389-399)flb_sds_printf(336-387)lib/cfl/src/cfl_array.c (1)
cfl_array_destroy(51-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (4)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (4)
67-67: Forward declaration for read_glob is appropriatePrevents implicit declaration warnings and clarifies intent ahead of platform-specific implementations.
373-374: LGTM: reuse of fleet_config_path_timestamp for timestamped detectionClearer and avoids duplicate logic.
1443-1445: LGTM: robust filtering using numeric timestamp extractionSwitching to fleet_config_path_timestamp(ctx, ...) > 0 ensures non-conforming files (e.g., header) are excluded before deletion.
1509-1512: LGTM: newest config selection now ignores non-timestamped entriesUsing fleet_config_path_timestamp(ctx, curconf) > 0 avoids accidentally picking header or other non-timestamped files as “newest”.
alecholmes
force-pushed
the
alec/2025-08-14-fleet-ignore-older-configs
branch
from
5412519 to
0f5c4cc
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (2)
973-1040: Fix UB: mismatched printf format specifiers vs. casted argumentsThe debug log uses "%ld" but passes (long long) values. That’s undefined behavior on multiple platforms and can garble logs or crash.
Apply:
- flb_plg_debug(ctx->ins, - "existing file with timestamp %ld >= new timestamp %ld", - (long long)file_timestamp, (long long)new_timestamp); + flb_plg_debug(ctx->ins, + "existing file with timestamp %lld >= new timestamp %lld", + (long long) file_timestamp, (long long) new_timestamp);Optional (perf): instead of scanning all files, compare only against the newest timestamped config (via calyptia_config_get_newest + fleet_config_path_timestamp) to reduce I/O as the directory grows.
1078-1089: Use 64-bit-safe formatting for time_t in logsOn LLP64 platforms, time_t is 64-bit while long is 32-bit. Logging with %ld can truncate or misinterpret the argument. Cast to long long and use %lld consistently.
- if (check_timestamp_is_newer(ctx, last_modified) == FLB_FALSE) { - flb_plg_debug(ctx->ins, "not creating file with timestamp %ld since it is not newer than existing files", - (long)last_modified); + if (check_timestamp_is_newer(ctx, last_modified) == FLB_FALSE) { + flb_plg_debug(ctx->ins, + "not creating file with timestamp %lld since it is not newer than existing files", + (long long) last_modified); ret = -1; goto client_error; } else { - flb_plg_info(ctx->ins, "creating config file with timestamp %ld", - (long)last_modified); + flb_plg_info(ctx->ins, + "creating config file with timestamp %lld", + (long long) last_modified); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
plugins/in_calyptia_fleet/in_calyptia_fleet.c(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-centos-7
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: PR - fuzzing test
🔇 Additional comments (4)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (4)
67-67: Forward declaration unblocks cross-OS use of read_globGood addition; it makes later calls compile on both POSIX and Windows sections.
373-374: Simplified timestamped-config check looks goodSwitching to a numeric timestamp test via fleet_config_path_timestamp(...) > 0 is clearer and avoids duplicating logic.
1443-1446: Filtering to timestamped files before deletion is correctThis avoids accidental deletion of non-conforming files. Good guard.
1509-1513: Selecting the newest by numeric timestamp is robustUsing fleet_config_path_timestamp(...) to gate the selection ensures symlinks or non-timestamped files don’t interfere.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (1)
231-237: Avoid truncating time_t when naming files (Y2038/LLP64 safety)snprintf("%d", (int)t) can truncate 64-bit time_t on many platforms, risking wrong filenames and incorrect gating decisions. Emit the full width.
Proposed replacement outside this hunk:
static flb_sds_t time_fleet_config_filename(struct flb_in_calyptia_fleet_config *ctx, time_t t) { char s_last_modified[32]; /* Use long long to safely format 64-bit time_t */ snprintf(s_last_modified, sizeof(s_last_modified) - 1, "%lld", (long long) t); return fleet_config_filename(ctx, s_last_modified); }
♻️ Duplicate comments (2)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (2)
324-362: Harden timestamp parsing: fix ERANGE/overflow checks and reject negative/partial matchesCurrent checks are still tied to LONG_* while using strtoll and may admit invalid/partial parses. Also negative timestamps should be rejected.
Apply this diff:
static time_t fleet_config_path_timestamp(struct flb_in_calyptia_fleet_config *ctx, const char *path) { char *fname; char *end; - long long val; + long long val64; @@ - errno = 0; - val = strtoll(fname, &end, 10); - if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) || (errno != 0 && val == 0)) { - return 0; - } + errno = 0; + val64 = strtoll(fname, &end, 10); + /* No digits parsed, out of range, or negative values are invalid */ + if (end == fname || errno == ERANGE || val64 < 0) { + return 0; + } @@ - if (strcmp(end, ".conf") == 0) { - return (time_t)val; + if (strcmp(end, ".conf") == 0) { + return (time_t) val64; } @@ - else if (strcmp(end, ".yaml") == 0) { - return (time_t)val; + else if (strcmp(end, ".yaml") == 0) { + return (time_t) val64; }
974-1040: Printf format bug: %ld used with long long arguments → undefined behavior in logsAt Line 1028, the format string uses %ld while the arguments are cast to long long. This is UB and can garble logs. Use %lld (or switch to inttypes macros).
Apply this diff:
- flb_plg_debug(ctx->ins, - "existing file with timestamp %ld >= new timestamp %ld", - (long long)file_timestamp, (long long)new_timestamp); + flb_plg_debug(ctx->ins, + "existing file with timestamp %lld >= new timestamp %lld", + (long long) file_timestamp, (long long) new_timestamp);Optional follow-up (perf): Instead of scanning all files, compare new_timestamp only against the newest timestamped file (via calyptia_config_get_newest + fleet_config_path_timestamp). This scales better as files accumulate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
plugins/in_calyptia_fleet/in_calyptia_fleet.c(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (5)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (5)
25-26: Good: errno/limits includes add portability for parsingThese headers are required for robust errno- and range-based parsing. Looks good.
69-69: Good: forward declaration prevents implicit-declaration issuesDeclaring read_glob early avoids build order problems. LGTM.
374-374: LGTM: unified timestamped-check via numeric parserReplacing the boolean path-check with a numeric timestamp > 0 is clearer and re-usable.
1444-1444: LGTM: filter delete-old using numeric timestamp parserSkipping non-timestamped files before pruning is correct and safer.
1510-1510: LGTM: newest-config selection now ignores non-timestamped filesPicking the newest strictly among timestamped files is correct and aligns with the gating logic.
Comment on lines
+1079
to
+1090
| /* Check if there are existing files with timestamps >= new timestamp */ | ||
| if (check_timestamp_is_newer(ctx, last_modified) == FLB_FALSE) { | ||
| flb_plg_debug(ctx->ins, "not creating file with timestamp %lld since it is not newer than existing files", | ||
| (long long)last_modified); | ||
| ret = -1; | ||
| goto client_error; | ||
| } | ||
| else { | ||
| flb_plg_info(ctx->ins, "creating config file with timestamp %lld", | ||
| (long long)last_modified); | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Parse Last-Modified as RFC 1123 and convert in UTC; current code uses %B and mktime (local time)
- HTTP-date is RFC 1123 (e.g., "Sun, 06 Nov 1994 08:49:37 GMT") with abbreviated month (%b), not full month (%B).
- mktime interprets tm as local time. For a “GMT” timestamp, use timegm (POSIX) or _mkgmtime (Windows). Otherwise gating can be wrong in non-UTC timezones.
Apply this diff:
- flb_strptime(fbit_last_modified, "%a, %d %B %Y %H:%M:%S GMT", &tm_last_modified);
- last_modified = mktime(&tm_last_modified.tm);
+ /* RFC 1123 format (IMF-fixdate): abbreviated month and GMT */
+ flb_strptime(fbit_last_modified, "%a, %d %b %Y %H:%M:%S GMT", &tm_last_modified);
+#ifndef _WIN32
+ last_modified = timegm(&tm_last_modified.tm);
+#else
+ last_modified = _mkgmtime(&tm_last_modified.tm);
+#endif
+ if (last_modified <= 0) {
+ flb_plg_warn(ctx->ins, "invalid Last-Modified header: %.*s",
+ fbit_last_modified_len, fbit_last_modified);
+ ret = -1;
+ goto client_error;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /* Check if there are existing files with timestamps >= new timestamp */ | |
| if (check_timestamp_is_newer(ctx, last_modified) == FLB_FALSE) { | |
| flb_plg_debug(ctx->ins, "not creating file with timestamp %lld since it is not newer than existing files", | |
| (long long)last_modified); | |
| ret = -1; | |
| goto client_error; | |
| } | |
| else { | |
| flb_plg_info(ctx->ins, "creating config file with timestamp %lld", | |
| (long long)last_modified); | |
| } | |
| /* RFC 1123 format (IMF-fixdate): abbreviated month and GMT */ | |
| flb_strptime(fbit_last_modified, "%a, %d %b %Y %H:%M:%S GMT", &tm_last_modified); | |
| #ifndef _WIN32 | |
| last_modified = timegm(&tm_last_modified.tm); | |
| #else | |
| last_modified = _mkgmtime(&tm_last_modified.tm); | |
| #endif | |
| if (last_modified <= 0) { | |
| flb_plg_warn(ctx->ins, "invalid Last-Modified header: %.*s", | |
| fbit_last_modified_len, fbit_last_modified); | |
| ret = -1; | |
| goto client_error; | |
| } | |
| /* Check if there are existing files with timestamps >= new timestamp */ | |
| if (check_timestamp_is_newer(ctx, last_modified) == FLB_FALSE) { | |
| flb_plg_debug(ctx->ins, "not creating file with timestamp %lld since it is not newer than existing files", | |
| (long long)last_modified); | |
| ret = -1; | |
| goto client_error; | |
| } | |
| else { | |
| flb_plg_info(ctx->ins, "creating config file with timestamp %lld", | |
| (long long)last_modified); | |
| } |
alecholmes
force-pushed
the
alec/2025-08-14-fleet-ignore-older-configs
branch
from
f652952 to
2f08b28
Compare
edsiper
approved these changes
Aug 19, 2025
Signed-off-by: Alec Holmes <[email protected]>
alecholmes
force-pushed
the
alec/2025-08-14-fleet-ignore-older-configs
branch
from
2f08b28 to
c81ca20
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (4)
1001-1006: Good: guard sds formatting failureNice catch adding a return-value check for flb_sds_printf and cleaning up allocations before returning.
1074-1078: Parse Last-Modified as RFC 1123 and convert in UTC; mktime + %B is incorrect
- HTTP-date uses RFC 1123 (IMF-fixdate) with abbreviated month (%b), not %B.
- mktime interprets local time; the header is GMT. Use timegm (POSIX) or _mkgmtime (Windows). Otherwise gating can be wrong across timezones and lead to accepting/rejecting incorrect configs.
Apply:
- flb_strptime(fbit_last_modified, "%a, %d %B %Y %H:%M:%S GMT", &tm_last_modified); - last_modified = mktime(&tm_last_modified.tm); + flb_strptime(fbit_last_modified, "%a, %d %b %Y %H:%M:%S GMT", &tm_last_modified); +#ifndef _WIN32 + last_modified = timegm(&tm_last_modified.tm); +#else + last_modified = _mkgmtime(&tm_last_modified.tm); +#endif + if (last_modified <= 0) { + flb_plg_warn(ctx->ins, "invalid Last-Modified header: %.*s", + fbit_last_modified_len, fbit_last_modified); + ret = -1; + goto client_error; + }Note: Consider checking the return of flb_strptime as well to catch malformed dates earlier.
324-362: Fix strtoll overflow/errno handling; avoid 32-bit LONG_ pitfalls*Current ERANGE check compares strtoll’s return value against LONG_MAX/LONG_MIN, which won’t catch overflow (strtoll returns LLONG_MAX/LLONG_MIN on ERANGE). Also, negative timestamps should be rejected. This can misclassify filenames and corrupt gating on LLP64/Windows or large timestamps.
Apply:
static time_t fleet_config_path_timestamp(struct flb_in_calyptia_fleet_config *ctx, const char *path) { char *fname; char *end; - long long val; + long long val64; @@ errno = 0; - val = strtoll(fname, &end, 10); - if ((errno == ERANGE && (val == LONG_MAX || val == LONG_MIN)) || (errno != 0 && val == 0)) { + val64 = strtoll(fname, &end, 10); + if (errno == ERANGE || val64 < 0) { return 0; } @@ if (strcmp(end, ".conf") == 0) { - return (time_t)val; + return (time_t) val64; } @@ else if (strcmp(end, ".yaml") == 0) { - return (time_t)val; + return (time_t) val64; }
1026-1030: Fix printf format specifiers: %ld with long long is UBYou’re casting to (long long) but using %ld. That’s undefined behavior and can print garbage or crash on some platforms.
Apply:
- flb_plg_debug(ctx->ins, - "existing file with timestamp %ld >= new timestamp %ld", - (long long)file_timestamp, (long long)new_timestamp); + flb_plg_debug(ctx->ins, + "existing file with timestamp %lld >= new timestamp %lld", + (long long) file_timestamp, (long long) new_timestamp);
🧹 Nitpick comments (2)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (2)
1008-1015: Differentiate “no match” vs. “glob error” to avoid masking failuresread_glob() returns NULL both for GLOB_NOMATCH and real errors. Treating both as “no existing files” can accidentally greenlight stale configs if a permission or IO error occurs.
Option: have read_glob return a status code or emit/propagate a flag (e.g., via an out param) so you can distinguish and log/warn accordingly.
1017-1033: Optional: compare against only the newest timestamped fileScanning all files is O(n). Given you only need the maximum timestamp, consider:
- Fetch newest via calyptia_config_get_newest() and compare once, or
- Track max(file_timestamp) while iterating and early break when you see any >= new_timestamp.
This reduces IO and CPU with many historical configs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
plugins/in_calyptia_fleet/in_calyptia_fleet.c(8 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (2)
src/flb_sds.c (3)
flb_sds_create_size(92-95)flb_sds_destroy(389-399)flb_sds_printf(336-387)lib/cfl/src/cfl_array.c (1)
cfl_array_destroy(51-69)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (28)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit, x64, x64-windows-static, 3.31.6)
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 64bit (Arm64), amd64_arm64, -DCMAKE_SYSTEM_NAME=Windows -DCMA...
- GitHub Check: pr-windows-build / call-build-windows-package (Windows 32bit, x86, x86-windows-static, 3.31.6)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SIMD=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_THREAD=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_COVERAGE=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SANITIZE_MEMORY=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_UNDEFINED=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DSANITIZE_ADDRESS=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, clang, clang++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_SMALL=On, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=Off, 3.31.6, gcc, g++)
- GitHub Check: run-ubuntu-unit-tests (-DFLB_JEMALLOC=On, 3.31.6, gcc, g++)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-22.04, clang-12)
- GitHub Check: PR - fuzzing test
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, gcc, g++, ubuntu-24.04, clang-14)
- GitHub Check: pr-compile-system-libs (-DFLB_PREFER_SYSTEM_LIBS=On, 3.31.6, clang, clang++, ubuntu-22.04, clang-12)
- GitHub Check: pr-compile-centos-7
🔇 Additional comments (6)
plugins/in_calyptia_fleet/in_calyptia_fleet.c (6)
25-26: LGTM: required headers addedIncluding errno.h and limits.h is necessary for robust numeric parsing and error checks.
69-69: LGTM: forward declaration unblocks callersForward-declaring read_glob avoids implicit declarations and keeps usage sites type-checked.
374-375: LGTM: timestamp classification tightenedSwapping to fleet_config_path_timestamp(...) > 0 makes the intent explicit and de-duplicates logic.
1081-1089: LGTM: clear, actionable logs for gating outcomeThe info/debug messages around creating or skipping a config based on timestamp make the behavior observable and debuggable.
1444-1446: LGTM: consistent timestamped-file filteringUsing fleet_config_path_timestamp(...) > 0 centrally avoids false positives from non-conforming filenames.
1510-1513: LGTM: newest-config selection respects timestamped namingThis keeps “newest” aligned with your gating semantics.
7 tasks
7 tasks
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
The fleet plugin polls a backend for the latest config. Due to the backend consisting of multiple processes, each with its own in-memory fleet config cache, the fleet plugin may encounter out-of-order configs until caches cohere.
Example:
Change
This updates the fleet config fetch logic to ignore received files with timestamps at or before any locally saved configs. This is fairly straightforward since the server sends a last modified timestamp and our local config file naming scheme is timestamp-based.
cc @pwhelan who did an initial review here.
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.
Summary by CodeRabbit
Bug Fixes
Refactor
Documentation