Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -220,3 +220,19 @@ set; and the cert subscription shares the same debounced `scheduleRebuild` (same
its coalescing must stay a superset-safe no-op for the single-swap #586 case. Regression coverage:
`integrationTests/security/cert-key-reload.test.ts` deterministically pins the cert-before-key ordering
(it fails by design without the rebuild trigger); `cert-reload.test.ts` guards the cert-only #586 path.

## `set_configuration` replication is opt-in; `replicateOperation` is default-on (`config/configUtils.ts`)

`server.replication.replicateOperation` (installed by harper-pro's replicator) fans out whenever
`req.replicated \!== false` — absence of the flag means "replicate". That default-on contract is what
DDL ops rely on (`dropSchema`/`dropTable` call it unconditionally), so a handler that mirrors the
drop_schema pattern without a guard silently becomes replicate-by-default. `setConfiguration` must
stay **opt-in** (`if (replicated)` truthy guard) because config bodies routinely carry node-local
params (ports, paths, node identity) that would clobber peers. Two invariants to preserve:
`replicated` must remain in the handler's destructure strip-list on both origin and peers (peers
receive `replicated: false` in the forwarded body; anything not stripped is treated as a config
param), and there is deliberately **no** per-param node-local/cluster-wide guard here — per-field
replicability metadata is deferred to the cluster-level-config work (CORE-3018), which will own that
schema. Per-peer failures never reject: they come back as `{status: 'failed', reason, node}` entries
in `response.replicated[]`, and `message` still reads as success (same contract as drop_schema), so
operators must inspect the array for per-node outcomes.
26 changes: 25 additions & 1 deletion config/configUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -830,9 +830,33 @@ export function getConfiguration() {
*/
export async function setConfiguration(setConfigJson) {
// eslint-disable-next-line no-unused-vars
const { operation, hdb_user, hdbAuthHeader, ...configFields } = setConfigJson;
const { operation, hdb_user, hdbAuthHeader, replicated, ...configFields } = setConfigJson;
// Operation-control field, not a config param: enforce boolean (matching other
// `replicated` surfaces, e.g. analyticsValidator) before any local write so a
// malformed value like the string "false" — which is truthy — can't apply config
// locally or trigger an unintended fan-out.
if (replicated !== undefined && typeof replicated !== 'boolean') {
throw handleHDBError(
new Error(),
`'replicated' must be a boolean`,
HTTP_STATUS_CODES.BAD_REQUEST,
undefined,
undefined,
true
);
}
try {
updateConfigValue(undefined, undefined, configFields, true);
if (replicated) {
// Opt-in fan-out to all cluster nodes (#660). replicateOperation forwards the
// body with `replicated: false`, so peers apply locally without re-replicating;
// per-node outcomes are returned on `response.replicated`. `replicated` must
// stay out of configFields on both origin and peers, or it would be written to
// the config file as a config param.
const response = await server.replication.replicateOperation(setConfigJson);
response.message = CONFIGURE_SUCCESS_RESPONSE;
return response;
Comment thread
heskew marked this conversation as resolved.
}
return CONFIGURE_SUCCESS_RESPONSE;
} catch (err) {
if (typeof err === 'string' || err instanceof String) {
Expand Down
41 changes: 41 additions & 0 deletions integrationTests/apiTests/configuration.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,47 @@ suite('Configuration', (ctx) => {
.expect(400);
});

// ── set_configuration + replicated (#660) ───────────────────────────────
// Real cluster fan-out lives in harper-pro; without it, the base server's
// replication stub rejects a truthy `replicated`. These tests pin the
// single-node contract: the flag routes through the operation as an option
// (never a config key), the local write still lands first (same origin-first
// ordering as drop_schema), and the rejection is explicit.

test('set_configuration with replicated: true on non-clustered instance rejects explicitly', async () => {
const r = await client
.req()
.send({ operation: 'set_configuration', logging_rotation_maxSize: '14M', replicated: true });
assert.ok(r.status >= 400, `expected error status, got ${r.status}\n${r.text}`);
assert.match(r.body.error ?? '', /Replication not implemented/, r.text);
});

test('set_configuration with string "false" replicated is rejected before applying', async () => {
await client
.req()
.send({ operation: 'set_configuration', logging_rotation_maxSize: '16M', replicated: 'false' })
.expect((r) => assert.match(r.body.error ?? '', /replicated/, r.text))
.expect(400);
// The malformed request must not have applied its config change locally.
await client
.req()
.send({ operation: 'get_configuration' })
.expect((r) => assert.notEqual(r.body.logging.rotation.maxSize, '16M', r.text))
.expect(200);
});

test('replicated flag is never persisted to configuration', async () => {
await client
.req()
.send({ operation: 'get_configuration' })
.expect((r) => {
// Local write from the rejected replicated call above still applied (origin-first).
assert.equal(r.body.logging.rotation.maxSize, '14M', r.text);
assert.equal(r.body.replicated, undefined, r.text);
})
.expect(200);
});

// ── non-superuser role restrictions ─────────────────────────────────────

test('add non-SU role and user', async () => {
Expand Down
124 changes: 124 additions & 0 deletions unitTests/config/configUtils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1073,6 +1073,130 @@ describe('Test configUtils module', () => {

expect(error.name).to.equal(STRING_ERROR);
});

describe('replicated: true', () => {
// configUtils reads `server` from the shared module object, so stubbing the
// property (the same way harper-pro installs the real implementation) reaches it.
const { server } = require('#src/server/Server');
let replicate_operation_stub;
let original_replicate_operation;

beforeEach(() => {
original_replicate_operation = server.replication.replicateOperation;
replicate_operation_stub = sandbox.stub().resolves({
message: '',
replicated: [{ message: 'ok', node: 'peer-1' }],
});
server.replication.replicateOperation = replicate_operation_stub;
});

afterEach(() => {
server.replication.replicateOperation = original_replicate_operation;
});

it('Test replicated: true fans out and returns per-node results', async () => {
const test_set_config_json = {
operation: 'set_configuration',
http_corsAccessList: ['harper.fast'],
replicated: true,
hdb_user: {},
hdb_auth_header: 'test_header',
};

const result = await config_utils_rw.setConfiguration(test_set_config_json);

expect(replicate_operation_stub.calledOnceWith(test_set_config_json)).to.equal(true);
expect(result.message).to.equal(CONFIGURE_SUCCESS_RESPONSE);
expect(result.replicated).to.eql([{ message: 'ok', node: 'peer-1' }]);
});

it('Test replicated is stripped from the config fields written to file', async () => {
const test_set_config_json = {
operation: 'set_configuration',
http_corsAccessList: ['harper.fast'],
replicated: true,
hdb_user: {},
hdb_auth_header: 'test_header',
};

await config_utils_rw.setConfiguration(test_set_config_json);

const config_fields = update_config_value_stub.firstCall.args[2];
expect(config_fields).to.eql({
http_corsAccessList: ['harper.fast'],
hdb_auth_header: 'test_header',
});
});

it('Test no replicated flag does not fan out', async () => {
const test_set_config_json = {
operation: 'set_configuration',
operationsApi_processes: 18,
hdb_user: {},
hdb_auth_header: 'test_header',
};

const result = await config_utils_rw.setConfiguration(test_set_config_json);

expect(replicate_operation_stub.called).to.equal(false);
expect(result).to.equal(CONFIGURE_SUCCESS_RESPONSE);
});

it('Test replicated: false (peer receiving a fanned-out call) does not re-replicate', async () => {
const test_set_config_json = {
operation: 'set_configuration',
http_corsAccessList: ['harper.fast'],
replicated: false,
hdb_user: {},
hdb_auth_header: 'test_header',
};

const result = await config_utils_rw.setConfiguration(test_set_config_json);

expect(replicate_operation_stub.called).to.equal(false);
expect(result).to.equal(CONFIGURE_SUCCESS_RESPONSE);
const config_fields = update_config_value_stub.firstCall.args[2];
expect(config_fields).to.not.have.property('replicated');
});

it('Test non-boolean replicated is rejected before any local write', async () => {
for (const bad of ['false', 'true', 1, 0]) {
let error;
try {
await config_utils_rw.setConfiguration({
operation: 'set_configuration',
http_corsAccessList: ['harper.fast'],
replicated: bad,
});
} catch (err) {
error = err;
}

expect(error, `expected rejection for replicated: ${JSON.stringify(bad)}`).to.not.equal(undefined);
expect(error.http_resp_msg ?? error.message).to.include('replicated');
}
expect(update_config_value_stub.called).to.equal(false);
expect(replicate_operation_stub.called).to.equal(false);
});

it('Test local config write failure propagates without fanning out', async () => {
update_config_value_stub.throws(STRING_ERROR);

let error;
try {
await config_utils_rw.setConfiguration({
operation: 'set_configuration',
http_corsAccessList: ['harper.fast'],
replicated: true,
});
} catch (err) {
error = err;
}

expect(error.name).to.equal(STRING_ERROR);
expect(replicate_operation_stub.called).to.equal(false);
});
});
});
describe('Test readConfigFile function', () => {
let properties_reader_rw;
Expand Down
Loading