From 210faa2b7ade161ce292fcdd0f60735aca7a0aec Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Tue, 11 Aug 2026 04:57:31 +0000 Subject: [PATCH] feat(component-failover-adoption): add candidate-list-to-client-config helper xce_cluster_config_to_client_config() converts a validated candidate list into a fully-populated xce_client_config_t, copying candidates in order with no special-casing for a single candidate. The truncate-with-warning safety net stays in place as a defensive guard now that N_MAX_MASTERS and MAX_HA_CANDIDATES match. Also removes a WARN check in xce_cluster_config_validate() that became unreachable dead code once task 1.1 reconciled the two candidate-count constants (both checks tested the same field against the same constant with no mutation between them). Task 1.2 of component-failover-adoption (Requirement 1 AC1-AC7). --- .../component-failover-adoption/tasks.md | 4 +- src/include/xce/cluster.h | 20 ++- src/lib/cluster.c | 36 ++++- src/lib/testsuite/test_cluster.c | 135 ++++++++++++++++++ 4 files changed, 190 insertions(+), 5 deletions(-) diff --git a/.kiro/specs/component-failover-adoption/tasks.md b/.kiro/specs/component-failover-adoption/tasks.md index 918dda99..4a1ea14b 100644 --- a/.kiro/specs/component-failover-adoption/tasks.md +++ b/.kiro/specs/component-failover-adoption/tasks.md @@ -8,7 +8,7 @@ - Observable completion: the project builds clean with the candidate-master array now sized to 4; the two updated startup checks and the updated test fixture all reference the same symbolic constant, not a literal - _Requirements: 1.7_ -- [ ] 1.2 Implement the candidate-list-to-client-config conversion helper in libxce +- [x] 1.2 Implement the candidate-list-to-client-config conversion helper in libxce - Add a pure, socket-free helper that converts a validated candidate list into a fully-populated client config, copying candidates in order and leaving identity/auth fields for the caller to set - A one-candidate input is handled through the identical code path as any other count — no special-cased branch for "single controller" - The truncate-with-warning safety net for an over-count input stays in place as a defensive guard, even though it can no longer be reached via a properly validated input now that the two limits match @@ -161,3 +161,5 @@ ## Implementation Notes - Task 1.1: `src/lib/cluster.c:99-102`'s `xce_cluster_config_validate()` still has a third hardcoded `>3` WARN check (message says "<=3 only") of the same magic-number class Req 1.7 targets, and `src/include/xce/cluster.h:31`'s doc comment repeats the stale "3" figure. Task 1.1's text named only "the two" worker/scheduler checks, so this was left untouched there — task 1.2 already touches `cluster.c` to add the new conversion helper, so fold this fix in there rather than letting it silently persist as the one remaining un-reconciled magic number. +- Task 1.1 follow-up resolved in 1.2: the `>3`→`>MAX_HA_CANDIDATES` rename initially proposed for `xce_cluster_config_validate()`'s WARN check turned out to be dead code — after task 1.1's constant reconciliation, that check tests the identical field against the identical constant as an earlier `-EINVAL` return in the same function, so it can never fire. Deleted rather than renamed (with the matching stale doc-comment line also removed), caught by task 1.2's review round 1. +- Task 1.2: its 8 new tests in `test_cluster.c` substantively cover task 1.3's stated checklist already (candidate order, one-candidate-no-branch, truncation-with-warning, NULL/empty rejection) — task 1.3 should scope down to just the constant-equality assertion (`N_MAX_MASTERS == MAX_HA_CANDIDATES`) rather than duplicating this coverage. diff --git a/src/include/xce/cluster.h b/src/include/xce/cluster.h index f6e5f12c..a0e6565c 100644 --- a/src/include/xce/cluster.h +++ b/src/include/xce/cluster.h @@ -7,6 +7,7 @@ #define XCE_CLUSTER_H #include +#include "client.h" /* xce_client_config_t, N_MAX_MASTERS */ #include "config.h" /* MAX_HA_CANDIDATES, XCE_MAXHOSTNAME_LEN */ /** @@ -28,7 +29,6 @@ typedef struct { * (encryption without client-side peer certificate verification). * * candidates_count must be in [1, MAX_HA_CANDIDATES]. - * A WARN is logged at startup if candidates_count > 3. */ typedef struct { xce_candidate_t candidates[MAX_HA_CANDIDATES]; @@ -79,6 +79,24 @@ typedef struct { */ int xce_cluster_config_validate(const xce_cluster_config_t* cfg); +/** + * xce_cluster_config_to_client_config() — convert a validated candidate list + * into an xce_client_config_t's master fields (nmasters/masters/master_ports). + * + * Copies up to N_MAX_MASTERS candidates from cfg->candidates[] in order. + * If cfg->candidates_count > N_MAX_MASTERS, copies the first N_MAX_MASTERS and + * logs a WARN identifying the truncated count (mirrors xce_test's existing + * clamp behavior). + * + * Does NOT set out->from / out->key — caller sets those separately. + * Does NOT validate cfg — caller must have already called + * xce_cluster_config_validate() successfully. + * + * Returns 0 on success, -EINVAL if cfg or out is NULL or cfg->candidates_count < 1. + */ +int xce_cluster_config_to_client_config(const xce_cluster_config_t* cfg, + xce_client_config_t* out); + /** * xce_connect_cluster() — connect to the cluster via candidate list. * diff --git a/src/lib/cluster.c b/src/lib/cluster.c index bd06dceb..9568eee7 100644 --- a/src/lib/cluster.c +++ b/src/lib/cluster.c @@ -96,10 +96,40 @@ int xce_cluster_config_validate(const xce_cluster_config_t* cfg) } if (cfg->identity[0] == '\0') return -EINVAL; - if (cfg->candidates_count > 3) + return 0; +} + +/* ── Config conversion ─────────────────────────────────────────── */ + +/* Mirrors xce_test's inline masters-array population at + * src/controller/testsuite/test.c:360-372, generalized into a reusable + * function. out->masters[i] are pointers into cfg->candidates[i].hostname, + * not copies -- same non-owning-storage contract xce_test already relies on + * (see that file's comment on master_cfg's static lifetime), so the caller + * must keep cfg alive at least as long as out is in use. */ +int xce_cluster_config_to_client_config(const xce_cluster_config_t* cfg, + xce_client_config_t* out) +{ + if (!cfg || !out) + return -EINVAL; + if (cfg->candidates_count < 1) + return -EINVAL; + + int count = cfg->candidates_count; + if (count > N_MAX_MASTERS) { fprintf(stderr, - "WARN: HA cluster has %d candidates; validated for <=3 only\n", - cfg->candidates_count); + "WARN: HA cluster has %d candidates; using first %d " + "(N_MAX_MASTERS)\n", + count, N_MAX_MASTERS); + count = N_MAX_MASTERS; + } + + out->nmasters = count; + for (int i = 0; i < count; i++) { + out->masters[i] = (char*)cfg->candidates[i].hostname; + out->master_ports[i] = cfg->candidates[i].port; + } + return 0; } diff --git a/src/lib/testsuite/test_cluster.c b/src/lib/testsuite/test_cluster.c index c5626097..15985685 100644 --- a/src/lib/testsuite/test_cluster.c +++ b/src/lib/testsuite/test_cluster.c @@ -3,6 +3,7 @@ #include #include +#include "client.h" #include "xce/cluster.h" /* ── Test seam: mock connect function ───────────────────────────── */ @@ -163,6 +164,132 @@ static void test_reconnect_step_backs_off_on_failure(void) printf("test_reconnect_step_backs_off_on_failure: PASS\n"); } +/* ── xce_cluster_config_to_client_config() tests ────────────────── + * Builds a cfg with `n` candidates at distinct host/port values, so the + * copy-in-order behavior can be verified precisely. */ +static xce_cluster_config_t make_config_with_n_candidates(int n) +{ + xce_cluster_config_t cfg = {0}; + for (int i = 0; i < n && i < MAX_HA_CANDIDATES; i++) { + char host[XCE_MAXHOSTNAME_LEN]; + snprintf(host, sizeof(host), "10.0.2.%d", i + 1); + strncpy(cfg.candidates[i].hostname, host, XCE_MAXHOSTNAME_LEN - 1); + cfg.candidates[i].port = 9000 + i; + } + cfg.candidates_count = n; + strncpy(cfg.identity, "conv-test", sizeof(cfg.identity) - 1); + return cfg; +} + +static void assert_converted_masters_match(const xce_cluster_config_t* cfg, + const xce_client_config_t* out, + int expected_count) +{ + TEST_ASSERT_EQUAL_INT(expected_count, out->nmasters); + for (int i = 0; i < expected_count; i++) { + TEST_ASSERT_EQUAL_STRING(cfg->candidates[i].hostname, + out->masters[i]); + TEST_ASSERT_EQUAL_INT(cfg->candidates[i].port, out->master_ports[i]); + } +} + +/* Test 10: one candidate goes through the identical code path as any other + * count -- no special-cased "single controller" branch (Req 1.5). */ +static void test_convert_one_candidate(void) +{ + xce_cluster_config_t cfg = make_config_with_n_candidates(1); + xce_client_config_t out = {0}; + int rc = xce_cluster_config_to_client_config(&cfg, &out); + TEST_ASSERT_EQUAL_INT(0, rc); + assert_converted_masters_match(&cfg, &out, 1); + printf("test_convert_one_candidate: PASS\n"); +} + +/* Test 11: three candidates -- ordinary multi-candidate case. */ +static void test_convert_three_candidates(void) +{ + xce_cluster_config_t cfg = make_config_with_n_candidates(3); + xce_client_config_t out = {0}; + int rc = xce_cluster_config_to_client_config(&cfg, &out); + TEST_ASSERT_EQUAL_INT(0, rc); + assert_converted_masters_match(&cfg, &out, 3); + printf("test_convert_three_candidates: PASS\n"); +} + +/* Test 12: four candidates -- the N_MAX_MASTERS boundary, still the + * ordinary (non-clamped) path since N_MAX_MASTERS == MAX_HA_CANDIDATES. */ +static void test_convert_four_candidates(void) +{ + xce_cluster_config_t cfg = make_config_with_n_candidates(4); + xce_client_config_t out = {0}; + int rc = xce_cluster_config_to_client_config(&cfg, &out); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_INT(N_MAX_MASTERS, 4); + assert_converted_masters_match(&cfg, &out, 4); + printf("test_convert_four_candidates: PASS\n"); +} + +/* Test 13: NULL cfg is rejected. */ +static void test_convert_rejects_null_cfg(void) +{ + xce_client_config_t out = {0}; + int rc = xce_cluster_config_to_client_config(NULL, &out); + TEST_ASSERT_EQUAL_INT(-EINVAL, rc); + printf("test_convert_rejects_null_cfg: PASS\n"); +} + +/* Test 14: NULL out is rejected. */ +static void test_convert_rejects_null_out(void) +{ + xce_cluster_config_t cfg = make_config_with_n_candidates(1); + int rc = xce_cluster_config_to_client_config(&cfg, NULL); + TEST_ASSERT_EQUAL_INT(-EINVAL, rc); + printf("test_convert_rejects_null_out: PASS\n"); +} + +/* Test 15: candidates_count < 1 is rejected. */ +static void test_convert_rejects_zero_candidates(void) +{ + xce_cluster_config_t cfg = make_config_with_n_candidates(0); + xce_client_config_t out = {0}; + int rc = xce_cluster_config_to_client_config(&cfg, &out); + TEST_ASSERT_EQUAL_INT(-EINVAL, rc); + printf("test_convert_rejects_zero_candidates: PASS\n"); +} + +/* Test 16: an artificially invalid over-count input (candidates_count > + * N_MAX_MASTERS, bypassing xce_cluster_config_validate()) exercises the + * defensive truncation path -- clamps to N_MAX_MASTERS and still populates + * the first N_MAX_MASTERS candidates correctly. A properly validated input + * can never reach this path today since N_MAX_MASTERS == MAX_HA_CANDIDATES. */ +static void test_convert_truncates_over_max_candidates(void) +{ + xce_cluster_config_t cfg = make_config_with_n_candidates(MAX_HA_CANDIDATES); + cfg.candidates_count = MAX_HA_CANDIDATES + 1; /* artificially invalid */ + xce_client_config_t out = {0}; + int rc = xce_cluster_config_to_client_config(&cfg, &out); + TEST_ASSERT_EQUAL_INT(0, rc); + assert_converted_masters_match(&cfg, &out, N_MAX_MASTERS); + printf("test_convert_truncates_over_max_candidates: PASS\n"); +} + +/* Test 17: the helper does not set identity/auth fields -- caller sets + * out->from / out->key separately. */ +static void test_convert_does_not_set_identity_fields(void) +{ + xce_cluster_config_t cfg = make_config_with_n_candidates(2); + xce_client_config_t out = {0}; + char sentinel_from[] = "sentinel-from"; + char sentinel_key[] = "sentinel-key"; + out.from = sentinel_from; + out.key = sentinel_key; + int rc = xce_cluster_config_to_client_config(&cfg, &out); + TEST_ASSERT_EQUAL_INT(0, rc); + TEST_ASSERT_EQUAL_PTR(sentinel_from, out.from); + TEST_ASSERT_EQUAL_PTR(sentinel_key, out.key); + printf("test_convert_does_not_set_identity_fields: PASS\n"); +} + /* ── Test group entry point ─────────────────────────────────────── */ void test_cluster(void) { @@ -175,4 +302,12 @@ void test_cluster(void) RUN_TEST(test_connect_returns_null_all_fail); RUN_TEST(test_reconnect_step_reaches_connected); RUN_TEST(test_reconnect_step_backs_off_on_failure); + RUN_TEST(test_convert_one_candidate); + RUN_TEST(test_convert_three_candidates); + RUN_TEST(test_convert_four_candidates); + RUN_TEST(test_convert_rejects_null_cfg); + RUN_TEST(test_convert_rejects_null_out); + RUN_TEST(test_convert_rejects_zero_candidates); + RUN_TEST(test_convert_truncates_over_max_candidates); + RUN_TEST(test_convert_does_not_set_identity_fields); }