Skip to content

Commit e9586e5

Browse files
committed
Manage CRR endpoints without explicit port
Issue: BB-746
1 parent 02be9e5 commit e9586e5

File tree

4 files changed

+108
-8
lines changed

4 files changed

+108
-8
lines changed

bin/replication.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ function _createSetupReplication(command, options, log) {
4040
const destinationEndpoint =
4141
config.getBootstrapList()
4242
.find(dest => Array.isArray(dest.servers));
43+
const destSiteConfig = destinationEndpoint ? config.getReplicationSiteDestConfig(destinationEndpoint.site) : null;
4344
return new SetupReplication({
4445
source: {
4546
bucket: sourceBucket,
@@ -51,9 +52,8 @@ function _createSetupReplication(command, options, log) {
5152
target: {
5253
bucket: targetBucket,
5354
credentials: targetCredentials,
54-
hosts: targetIsExternal ?
55-
undefined : new RoundRobin(destinationEndpoint.servers),
56-
transport: destination.transport,
55+
hosts: targetIsExternal ? undefined : new RoundRobin(destSiteConfig.replicationEndpoint.servers),
56+
transport: destSiteConfig?.transport || destination.transport,
5757
isExternal: targetIsExternal,
5858
siteName,
5959
},

extensions/replication/queueProcessor/QueueProcessor.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ class QueueProcessor extends EventEmitter {
259259

260260
if (Array.isArray(this.destConfig.replicationEndpoint.servers)) {
261261
this.destHosts =
262-
new RoundRobin(this.destConfig.replicationEndpoint.servers, { defaultPort: 80 });
262+
new RoundRobin(this.destConfig.replicationEndpoint.servers);
263263
if (this.destConfig.replicationEndpoint.echo) {
264264
this._setupEcho();
265265
}

lib/Config.js

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,13 +309,42 @@ class Config extends EventEmitter {
309309
* @returns {object} site specific destination configuration
310310
*/
311311
getReplicationSiteDestConfig(site) {
312-
const destConfig = this.extensions.replication.destination;
312+
const { destination } = this.extensions.replication;
313+
const siteConfig = destination.sites?.[site] || {};
314+
315+
const transport = siteConfig.transport || destination.transport || 'http';
316+
const auth = siteConfig.auth || destination.auth;
317+
const endpoint = this.bootstrapList.find(e => e.site === site);
318+
313319
return {
314-
transport: destConfig.sites?.[site]?.transport || destConfig.transport,
315-
auth: destConfig.sites?.[site]?.auth || destConfig.auth,
316-
replicationEndpoint: this.bootstrapList.find(endpoint => endpoint.site === site),
320+
transport,
321+
auth,
322+
replicationEndpoint: this._normalizeEndpointServers(endpoint, transport),
317323
};
318324
}
325+
326+
/**
327+
* Normalize replication endpoint servers by adding the default port
328+
* based on the transport protocol when no explicit port is specified.
329+
* @param {object|undefined} endpoint - replication endpoint object
330+
* @param {string} transport - transport protocol ('http' or 'https')
331+
* @returns {object|undefined} endpoint with normalized servers
332+
*/
333+
_normalizeEndpointServers(endpoint, transport) {
334+
if (!endpoint || !Array.isArray(endpoint.servers)) {
335+
return endpoint;
336+
}
337+
338+
const defaultPort = transport === 'https' ? 443 : 80;
339+
const normalizedServers = endpoint.servers.map(server => {
340+
if (server.includes(':')) {
341+
return server;
342+
}
343+
return `${server}:${defaultPort}`;
344+
});
345+
346+
return { ...endpoint, servers: normalizedServers };
347+
}
319348
}
320349

321350
module.exports = new Config();

tests/unit/conf/Config.js

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,77 @@ describe('Config', () => {
122122
});
123123
});
124124

125+
it('should normalize server entries with default port 443 for https transport', () => {
126+
const conf = new Config();
127+
conf.bootstrapList = [{ site: 'site1', servers: ['s3.example.com']}];
128+
conf.extensions.replication.destination = {
129+
transport: 'https',
130+
sites: {
131+
site1: { transport: 'https' },
132+
},
133+
bootstrapList: [
134+
{ site: 'site1', servers: ['s3.example.com'] },
135+
],
136+
};
137+
138+
const destConfig = conf.getReplicationSiteDestConfig('site1');
139+
assert.deepStrictEqual(destConfig.replicationEndpoint.servers, ['s3.example.com:443']);
140+
});
141+
142+
it('should normalize server entries with default port 80 for http transport', () => {
143+
const conf = new Config();
144+
conf.bootstrapList = [{ site: 'site1', servers: ['s3.example.com']}];
145+
conf.extensions.replication.destination = {
146+
transport: 'http',
147+
sites: {
148+
site1: { transport: 'http' },
149+
},
150+
bootstrapList: [
151+
{ site: 'site1', servers: ['s3.example.com'] },
152+
],
153+
};
154+
155+
const destConfig = conf.getReplicationSiteDestConfig('site1');
156+
assert.deepStrictEqual(destConfig.replicationEndpoint.servers, ['s3.example.com:80']);
157+
});
158+
159+
it('should preserve explicit port in server entries', () => {
160+
const conf = new Config();
161+
conf.bootstrapList = [{ site: 'site1', servers: ['s3.example.com:8443']}];
162+
conf.extensions.replication.destination = {
163+
transport: 'https',
164+
sites: {
165+
site1: { transport: 'https' },
166+
},
167+
bootstrapList: [
168+
{ site: 'site1', servers: ['s3.example.com:8443'] },
169+
],
170+
};
171+
172+
const destConfig = conf.getReplicationSiteDestConfig('site1');
173+
assert.deepStrictEqual(destConfig.replicationEndpoint.servers, ['s3.example.com:8443']);
174+
});
175+
176+
it('should not modify endpoint without servers array', () => {
177+
const conf = new Config();
178+
conf.bootstrapList = [{ site: 'aws1', type: 'aws_s3' }];
179+
conf.extensions.replication.destination = {
180+
transport: 'http',
181+
sites: {
182+
aws1: { transport: 'http' },
183+
},
184+
bootstrapList: [
185+
{ site: 'aws1', type: 'aws_s3' },
186+
],
187+
};
188+
189+
const destConfig = conf.getReplicationSiteDestConfig('aws1');
190+
assert.deepStrictEqual(destConfig.replicationEndpoint, {
191+
site: 'aws1',
192+
type: 'aws_s3',
193+
});
194+
});
195+
125196
it('should return default replication destination config when site one is not available', () => {
126197
process.env.BACKBEAT_CONFIG_FILE = `${__dirname}/configs/replicationMultiDestConfig.json`;
127198
const conf = new Config();

0 commit comments

Comments
 (0)