Skip to content

Commit cc605e2

Browse files
francoisferrandfrancoisferrand
committed
Allow creating internalServer
This is used to handle internal traffic (esp. from backbeat) with specific handling of policies where needed. Issue: CLDSRV-650
1 parent 0d150e9 commit cc605e2

File tree

4 files changed

+324
-19
lines changed

4 files changed

+324
-19
lines changed

‎lib/Config.js‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -872,14 +872,26 @@ class Config extends EventEmitter {
872872
}
873873
}
874874

875-
this.port = 8000;
876875
if (config.port !== undefined) {
877876
assert(Number.isInteger(config.port) && config.port > 0,
878877
'bad config: port must be a positive integer');
879-
this.port = config.port;
880878
}
881879

880+
if (config.internalPort !== undefined) {
881+
assert(Number.isInteger(config.internalPort) && config.internalPort > 0,
882+
'bad config: internalPort must be a positive integer');
883+
}
884+
885+
this.port = config.port;
882886
this.listenOn = this._parseEndpoints(config.listenOn, 'listenOn');
887+
this.internalPort = config.internalPort;
888+
this.internalListenOn = this._parseEndpoints(config.internalListenOn, 'internalListenOn');
889+
890+
if (this.listenOn.length || (!this.internalPort && !this.internalListenOn.length)) {
891+
// default to 8000 unless running in "internal" API mode only (i.e. internalPort/ListenOn)
892+
// i.e. when either no port config is specified, or only listenOn is specified.
893+
this.port ||= 8000;
894+
}
883895

884896
this.metricsPort = 8002;
885897
if (config.metricsPort !== undefined) {

‎lib/server.js‎

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,11 @@ class S3Server {
6262
/**
6363
* This represents our S3 connector.
6464
* @constructor
65+
* @param {Object} config - Configuration object
6566
* @param {Worker} [worker=null] - Track the worker when using cluster
6667
*/
67-
constructor(worker) {
68+
constructor(config, worker) {
69+
this.config = config;
6870
this.worker = worker;
6971
this.cluster = true;
7072
this.servers = [];
@@ -144,14 +146,14 @@ class S3Server {
144146
dataRetrievalParams: {
145147
client,
146148
implName,
147-
config: _config,
149+
config: this.config,
148150
kms,
149151
metadata,
150152
locStorageCheckFn: locationStorageCheck,
151153
vault,
152154
},
153155
};
154-
routes(req, res, params, logger, _config);
156+
routes(req, res, params, logger, this.config);
155157
}
156158

157159
/**
@@ -210,11 +212,11 @@ class S3Server {
210212
_startServer(listener, port, ipAddress) {
211213
// Todo: http.globalAgent.maxSockets, http.globalAgent.maxFreeSockets
212214
let server;
213-
if (_config.https) {
215+
if (this.config.https) {
214216
server = https.createServer({
215-
cert: _config.https.cert,
216-
key: _config.https.key,
217-
ca: _config.https.ca,
217+
cert: this.config.https.cert,
218+
key: this.config.https.key,
219+
ca: this.config.https.ca,
218220
ciphers: arsenal.https.ciphers.ciphers,
219221
dhparam: arsenal.https.dhparam.dhparam,
220222
rejectUnauthorized: true,
@@ -305,21 +307,30 @@ class S3Server {
305307
}
306308

307309
// Start API server(s)
308-
if (_config.listenOn.length > 0) {
309-
_config.listenOn.forEach(item => {
310+
if (this.config.listenOn.length > 0) {
311+
this.config.listenOn.forEach(item => {
310312
this._startServer(this.routeRequest, item.port, item.ip);
311313
});
312-
} else {
313-
this._startServer(this.routeRequest, _config.port);
314+
} else if (this.config.port) {
315+
this._startServer(this.routeRequest, this.config.port);
316+
}
317+
318+
// Start internal API server(s)
319+
if (this.config.internalListenOn.length > 0) {
320+
this.config.internalListenOn.forEach(item => {
321+
this._startServer(this.routeRequest, item.port, item.ip);
322+
});
323+
} else if (this.config.internalPort) {
324+
this._startServer(this.routeRequest, this.config.internalPort);
314325
}
315326

316327
// Start metrics server(s)
317-
if (_config.metricsListenOn.length > 0) {
318-
_config.metricsListenOn.forEach(item => {
328+
if (this.config.metricsListenOn.length > 0) {
329+
this.config.metricsListenOn.forEach(item => {
319330
this._startServer(this.routeAdminRequest, item.port, item.ip);
320331
});
321332
} else {
322-
this._startServer(this.routeAdminRequest, _config.metricsPort);
333+
this._startServer(this.routeAdminRequest, this.config.metricsPort);
323334
}
324335

325336
// Start quota service health checks
@@ -353,7 +364,7 @@ function main() {
353364
this.cluster = workers > 1;
354365
if (!this.cluster) {
355366
process.env.REPORT_TOKEN = _config.reportToken;
356-
const server = new S3Server();
367+
const server = new S3Server(_config);
357368
server.initiateStartup(logger.newRequestLogger());
358369
}
359370
if (this.cluster && cluster.isMaster) {
@@ -399,10 +410,11 @@ function main() {
399410
});
400411
}
401412
if (this.cluster && cluster.isWorker) {
402-
const server = new S3Server(cluster.worker);
413+
const server = new S3Server(_config, cluster.worker);
403414
server.initiateStartup(logger.newRequestLogger());
404415
}
405416
monitoringClient.collectDefaultMetrics({ timeout: 5000 });
406417
}
407418

408419
module.exports = main;
420+
module.exports.S3Server = S3Server;

‎tests/unit/Config.js‎

Lines changed: 132 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ const {
1212
const { ValidLifecycleRules: supportedLifecycleRules } = require('arsenal').models;
1313

1414
describe('Config', () => {
15+
const defaultConfig = JSON.parse(fs.readFileSync('config.json'), { encoding: 'utf-8' });
16+
1517
const envToRestore = [];
1618
const setEnv = (key, value) => {
1719
if (key in process.env) {
@@ -446,7 +448,6 @@ describe('Config', () => {
446448
});
447449

448450
describe('multiObjectDeleteEnableOptimizations', () => {
449-
const defaultConfig = JSON.parse(fs.readFileSync('config.json'), { encoding: 'utf-8' });
450451
let sandbox;
451452
let readFileStub;
452453

@@ -679,4 +680,134 @@ describe('Config', () => {
679680
assert.deepStrictEqual(parsedRules, rules);
680681
});
681682
});
683+
684+
describe('port and internalPort configuration', () => {
685+
// same as `defaultConfig` (i.e. config.json) but without the port settings
686+
const baseConfig = (() => {
687+
const config = { ...defaultConfig };
688+
delete config.port;
689+
delete config.listenOn;
690+
delete config.internalPort;
691+
delete config.internalListenOn;
692+
delete config.metricsPort;
693+
delete config.metricsListenOn;
694+
return config;
695+
})();
696+
697+
let sandbox;
698+
let readFileStub;
699+
700+
beforeEach(() => {
701+
sandbox = sinon.createSandbox();
702+
readFileStub = sandbox.stub(fs, 'readFileSync');
703+
readFileStub.callThrough();
704+
});
705+
706+
afterEach(() => {
707+
sandbox.restore();
708+
});
709+
710+
it('should throw an error for negative port number', () => {
711+
const testConfig = { ...baseConfig, port: -1 };
712+
readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
713+
714+
assert.throws(() => new ConfigObject(), /bad config: port must be a positive integer/);
715+
});
716+
717+
it('should throw an error for non-integer port number', () => {
718+
const testConfig = { ...baseConfig, port: 8000.5 };
719+
readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
720+
721+
assert.throws(() => new ConfigObject(), /bad config: port must be a positive integer/);
722+
});
723+
724+
it('should throw an error for negative internalPort number', () => {
725+
const testConfig = { ...baseConfig, internalPort: -1 };
726+
readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
727+
728+
assert.throws(() => new ConfigObject(), /bad config: internalPort must be a positive integer/);
729+
});
730+
731+
it('should throw an error for non-integer internalPort number', () => {
732+
const testConfig = { ...baseConfig, internalPort: 9000.5 };
733+
readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
734+
735+
assert.throws(() => new ConfigObject(), /bad config: internalPort must be a positive integer/);
736+
});
737+
738+
it('should accept a valid port number', () => {
739+
const testConfig = { ...baseConfig, port: 8765 };
740+
readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
741+
742+
const config = new ConfigObject();
743+
assert.strictEqual(config.port, 8765);
744+
assert.strictEqual(config.internalPort, undefined);
745+
});
746+
747+
it('should accept a valid internalPort number', () => {
748+
const testConfig = { ...baseConfig, internalPort: 9000 };
749+
readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
750+
751+
const config = new ConfigObject();
752+
assert.strictEqual(config.port, undefined);
753+
assert.strictEqual(config.internalPort, 9000);
754+
});
755+
756+
it('should accept both port and internalPort when provided', () => {
757+
const testConfig = { ...baseConfig, port: 8000, internalPort: 9000 };
758+
readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
759+
760+
const config = new ConfigObject();
761+
assert.strictEqual(config.port, 8000);
762+
assert.strictEqual(config.internalPort, 9000);
763+
});
764+
765+
it('should use default port 8000 if no port or internalPort specified', () => {
766+
readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(baseConfig));
767+
768+
const config = new ConfigObject();
769+
assert.strictEqual(config.port, 8000);
770+
assert.strictEqual(config.internalPort, undefined);
771+
});
772+
773+
it('should default port if listenOn is provided', () => {
774+
const testConfig = {
775+
...baseConfig,
776+
listenOn: ['127.0.0.1:9000'],
777+
};
778+
readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
779+
780+
const config = new ConfigObject();
781+
assert.strictEqual(config.port, 8000);
782+
assert.strictEqual(config.internalPort, undefined);
783+
assert.deepEqual(config.listenOn, [{ ip: '127.0.0.1', port: 9000 }]);
784+
});
785+
786+
it('should not default port if internalListenOn is provided', () => {
787+
const testConfig = {
788+
...baseConfig,
789+
internalListenOn: ['127.0.0.1:9000'],
790+
};
791+
readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
792+
793+
const config = new ConfigObject();
794+
assert.strictEqual(config.port, undefined);
795+
assert.strictEqual(config.internalPort, undefined);
796+
assert.deepEqual(config.internalListenOn, [{ ip: '127.0.0.1', port: 9000 }]);
797+
});
798+
799+
it('should properly handle listenOn and internalListenOn configuration', () => {
800+
const testConfig = {
801+
...baseConfig,
802+
listenOn: ['0.0.0.0:8000'],
803+
internalListenOn: ['127.0.0.1:9000'],
804+
};
805+
readFileStub.withArgs(sinon.match(/config.json$/)).returns(JSON.stringify(testConfig));
806+
807+
const config = new ConfigObject();
808+
assert.strictEqual(config.port, 8000);
809+
assert.deepEqual(config.listenOn, [{ ip: '0.0.0.0', port: 8000 }]);
810+
assert.deepEqual(config.internalListenOn, [{ ip: '127.0.0.1', port: 9000 }]);
811+
});
812+
});
682813
});

0 commit comments

Comments
 (0)