Skip to content

Commit 766ecba

Browse files
bert-ebert-e
committed
Merge branch 'bugfix/CLDSRV-665' into q/8.8
2 parents 3588732 + 17cab88 commit 766ecba

File tree

7 files changed

+130
-24
lines changed

7 files changed

+130
-24
lines changed

‎.github/docker/docker-compose.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ services:
4545
- SCUBA_HEALTHCHECK_FREQUENCY
4646
- S3QUOTA
4747
- QUOTA_ENABLE_INFLIGHTS
48+
- S3_VERSION_ID_ENCODING_TYPE
4849
env_file:
4950
- creds.env
5051
depends_on:

‎.github/workflows/tests.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ jobs:
293293
S3METADATA: mongodb
294294
S3KMS: file
295295
S3_LOCATION_FILE: /usr/src/app/tests/locationConfig/locationConfigTests.json
296+
S3_VERSION_ID_ENCODING_TYPE: base62
296297
DEFAULT_BUCKET_KEY_FORMAT: v1
297298
METADATA_MAX_CACHED_BUCKETS: 1
298299
MONGODB_IMAGE: ghcr.io/${{ github.repository }}/ci-mongodb:${{ github.sha }}

‎lib/Config.js‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -968,6 +968,38 @@ class Config extends EventEmitter {
968968
this.replicationGroupId = 'RG001';
969969
}
970970

971+
const instanceId = process.env.CLOUDSERVER_INSTANCE_ID || config.instanceId;
972+
if (instanceId) {
973+
assert(typeof instanceId === 'string',
974+
'bad config: instanceId must be a string');
975+
// versionID generation code will truncate instanceId to 6 characters
976+
// so we enforce this limit here to make the behavior predictable
977+
assert(instanceId.length <= 6,
978+
'bad config: instanceId must be at most 6 characters long');
979+
this.instanceId = instanceId;
980+
} else if (process.env.HOSTNAME) {
981+
// If running in Kubernetes, the pod name is set in the HOSTNAME env var
982+
// and is of the form: cloudserver-7869f99955-nr984
983+
// where:
984+
// - cloudserver is the name of the pod
985+
// - 7869f99955 is a long hash that guarantees uniqueness within the namespace
986+
// - nr984 is a short hash that is unique only within the current ReplicaSet/Deployment
987+
// We use the last part of the pod name as the instanceId, prefixed with 'i' for internal
988+
// or 'e' for external cloudserver.
989+
// This allows having a unique identifier that will be used when generating versionIds
990+
const instanceType = process.env.HOSTNAME.includes('internal') ? 'i' : 'e';
991+
const parts = process.env.HOSTNAME.split('-');
992+
if (parts.length >= 2) {
993+
const shortId = parts[parts.length - 1];
994+
this.instanceId = instanceType + shortId;
995+
} else {
996+
// fallback: generate a random string if format is unexpected
997+
this.instanceId = uuidv4().replace(/-/g, '').slice(0, 6);
998+
}
999+
} else {
1000+
this.instanceId = uuidv4().replace(/-/g, '').slice(0, 6);
1001+
}
1002+
9711003
this.replicationEndpoints = [];
9721004
if (config.replicationEndpoints) {
9731005
const { replicationEndpoints } = config;

‎lib/metadata/wrapper.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ if (clientName === 'mem') {
3030
params = {
3131
mongodb: config.mongodb,
3232
replicationGroupId: config.replicationGroupId,
33+
instanceId: config.instanceId,
3334
config,
3435
};
3536
} else if (clientName === 'cdmi') {

‎package.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@zenko/cloudserver",
3-
"version": "8.8.54",
3+
"version": "8.8.55",
44
"description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",
55
"main": "index.js",
66
"engines": {
@@ -21,7 +21,7 @@
2121
"dependencies": {
2222
"@azure/storage-blob": "^12.12.0",
2323
"@hapi/joi": "^17.1.0",
24-
"arsenal": "git+https://github.com/scality/arsenal#8.1.162",
24+
"arsenal": "git+https://github.com/scality/arsenal#8.1.163",
2525
"async": "~2.5.0",
2626
"aws-sdk": "2.905.0",
2727
"bucketclient": "scality/bucketclient#8.1.14",

‎tests/unit/Config.js‎

Lines changed: 90 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
const assert = require('assert');
2+
const sinon = require('sinon');
3+
const fs = require('fs');
4+
const path = require('path');
25
const {
36
azureArchiveLocationConstraintAssert,
4-
ConfigObject: ConfigObjectForTest,
7+
ConfigObject,
58
} = require('../../lib/Config');
69

710
describe('Config', () => {
@@ -25,7 +28,6 @@ describe('Config', () => {
2528
});
2629

2730
it('should emit an event when auth data is updated', done => {
28-
const { ConfigObject } = require('../../lib/Config');
2931
const config = new ConfigObject();
3032
let emitted = false;
3133
config.on('authdata-update', () => {
@@ -209,8 +211,6 @@ describe('Config', () => {
209211
});
210212

211213
describe('getAzureStorageAccountName', () => {
212-
const { ConfigObject } = require('../../lib/Config');
213-
214214
it('should return account name from config', () => {
215215
setEnv('azurebackend_AZURE_STORAGE_ACCOUNT_NAME', '');
216216
const config = new ConfigObject();
@@ -259,7 +259,7 @@ describe('Config', () => {
259259

260260
describe('time options', () => {
261261
it('should getTimeOptions', () => {
262-
const config = new ConfigObjectForTest();
262+
const config = new ConfigObject();
263263
const expectedOptions = {
264264
expireOneDayEarlier: false,
265265
transitionOneDayEarlier: false,
@@ -273,7 +273,7 @@ describe('Config', () => {
273273
it('should getTimeOptions with TIME_PROGRESSION_FACTOR', () => {
274274
setEnv('TIME_PROGRESSION_FACTOR', 2);
275275

276-
const config = new ConfigObjectForTest();
276+
const config = new ConfigObject();
277277
const expectedOptions = {
278278
expireOneDayEarlier: false,
279279
transitionOneDayEarlier: false,
@@ -287,7 +287,7 @@ describe('Config', () => {
287287
it('should getTimeOptions with EXPIRE_ONE_DAY_EARLIER', () => {
288288
setEnv('EXPIRE_ONE_DAY_EARLIER', true);
289289

290-
const config = new ConfigObjectForTest();
290+
const config = new ConfigObject();
291291
const expectedOptions = {
292292
expireOneDayEarlier: true,
293293
transitionOneDayEarlier: false,
@@ -301,7 +301,7 @@ describe('Config', () => {
301301
it('should getTimeOptions with TRANSITION_ONE_DAY_EARLIER', () => {
302302
setEnv('TRANSITION_ONE_DAY_EARLIER', true);
303303

304-
const config = new ConfigObjectForTest();
304+
const config = new ConfigObject();
305305
const expectedOptions = {
306306
expireOneDayEarlier: false,
307307
transitionOneDayEarlier: true,
@@ -316,7 +316,7 @@ describe('Config', () => {
316316
setEnv('EXPIRE_ONE_DAY_EARLIER', true);
317317
setEnv('TRANSITION_ONE_DAY_EARLIER', true);
318318

319-
const config = new ConfigObjectForTest();
319+
const config = new ConfigObject();
320320
const expectedOptions = {
321321
expireOneDayEarlier: true,
322322
transitionOneDayEarlier: true,
@@ -331,22 +331,22 @@ describe('Config', () => {
331331
setEnv('EXPIRE_ONE_DAY_EARLIER', true);
332332
setEnv('TIME_PROGRESSION_FACTOR', 2);
333333

334-
assert.throws(() => new ConfigObjectForTest());
334+
assert.throws(() => new ConfigObject());
335335
});
336336

337337
it('should throw error if TRANSITION_ONE_DAY_EARLIER and TIME_PROGRESSION_FACTOR', () => {
338338
setEnv('TRANSITION_ONE_DAY_EARLIER', true);
339339
setEnv('TIME_PROGRESSION_FACTOR', 2);
340340

341-
assert.throws(() => new ConfigObjectForTest());
341+
assert.throws(() => new ConfigObject());
342342
});
343343

344344
it('should throw error if both EXPIRE/TRANSITION_ONE_DAY_EARLIER and TIME_PROGRESSION_FACTOR', () => {
345345
setEnv('EXPIRE_ONE_DAY_EARLIER', true);
346346
setEnv('TRANSITION_ONE_DAY_EARLIER', true);
347347
setEnv('TIME_PROGRESSION_FACTOR', 2);
348348

349-
assert.throws(() => new ConfigObjectForTest());
349+
assert.throws(() => new ConfigObject());
350350
});
351351
});
352352

@@ -364,7 +364,6 @@ describe('Config', () => {
364364
});
365365

366366
it('should set up scuba', () => {
367-
const { ConfigObject } = require('../../lib/Config');
368367
const config = new ConfigObject();
369368

370369
assert.deepStrictEqual(
@@ -380,7 +379,6 @@ describe('Config', () => {
380379
setEnv('SCUBA_HOST', 'scubahost');
381380
setEnv('SCUBA_PORT', 1234);
382381

383-
const { ConfigObject } = require('../../lib/Config');
384382
const config = new ConfigObject();
385383

386384
assert.deepStrictEqual(
@@ -407,7 +405,6 @@ describe('Config', () => {
407405
});
408406

409407
it('should set up quota', () => {
410-
const { ConfigObject } = require('../../lib/Config');
411408
const config = new ConfigObject();
412409

413410
assert.deepStrictEqual(
@@ -423,7 +420,6 @@ describe('Config', () => {
423420
setEnv('QUOTA_MAX_STALENESS_MS', 1234);
424421
setEnv('QUOTA_ENABLE_INFLIGHTS', 'true');
425422

426-
const { ConfigObject } = require('../../lib/Config');
427423
const config = new ConfigObject();
428424

429425
assert.deepStrictEqual(
@@ -439,7 +435,6 @@ describe('Config', () => {
439435
setEnv('QUOTA_MAX_STALENESS_MS', 'notanumber');
440436
setEnv('QUOTA_ENABLE_INFLIGHTS', 'true');
441437

442-
const { ConfigObject } = require('../../lib/Config');
443438
const config = new ConfigObject();
444439

445440
assert.deepStrictEqual(
@@ -466,7 +461,6 @@ describe('Config', () => {
466461
});
467462

468463
it('should set up utapi local cache', () => {
469-
const { ConfigObject } = require('../../lib/Config');
470464
const config = new ConfigObject();
471465

472466
assert.deepStrictEqual(
@@ -480,7 +474,6 @@ describe('Config', () => {
480474
});
481475

482476
it('should set up utapi redis', () => {
483-
const { ConfigObject } = require('../../lib/Config');
484477
const config = new ConfigObject();
485478

486479
assert.deepStrictEqual(
@@ -668,4 +661,82 @@ describe('Config', () => {
668661
assert.throws(() => azureArchiveLocationConstraintAssert(locationObj));
669662
});
670663
});
664+
665+
describe('instanceId', () => {
666+
let defaultConfig;
667+
668+
before(() => {
669+
setEnv('S3_CONFIG_FILE', 'tests/unit/testConfigs/allOptsConfig/config.json');
670+
// Import config
671+
const defaultConfigPath = path.join(__dirname, './testConfigs/allOptsConfig/config.json');
672+
defaultConfig = require(defaultConfigPath);
673+
});
674+
675+
afterEach(() => {
676+
sinon.restore();
677+
});
678+
679+
it('should use the instance id set in the config', () => {
680+
// Stub fs.readFileSync to return a specific config.json content that includes instanceId
681+
const originalReadFileSync = fs.readFileSync;
682+
const readFileSyncStub = sinon.stub(fs, 'readFileSync');
683+
// Mock only config.json file
684+
readFileSyncStub
685+
.withArgs(sinon.match(/\/config\.json$/))
686+
.returns(JSON.stringify({ ...defaultConfig, instanceId: 'test' }));
687+
// For all other files, use the original readFileSync
688+
readFileSyncStub
689+
.callsFake((filePath, ...args) => originalReadFileSync(filePath, ...args));
690+
// Create a new ConfigObject instance
691+
const config = new ConfigObject();
692+
assert.strictEqual(config.instanceId, 'test');
693+
});
694+
695+
it('should assert if instanceId is not a string', () => {
696+
// Stub fs.readFileSync to return a specific config.json content that includes instanceId
697+
const originalReadFileSync = fs.readFileSync;
698+
const readFileSyncStub = sinon.stub(fs, 'readFileSync');
699+
// Mock only config.json file
700+
readFileSyncStub
701+
.withArgs(sinon.match(/\/config\.json$/))
702+
.returns(JSON.stringify({ ...defaultConfig, instanceId: 1234 }));
703+
// For all other files, use the original readFileSync
704+
readFileSyncStub
705+
.callsFake((filePath, ...args) => originalReadFileSync(filePath, ...args));
706+
// Create a new ConfigObject instance
707+
assert.throws(() => new ConfigObject());
708+
});
709+
710+
it('should use the instance id set in the env var', () => {
711+
setEnv('CLOUDSERVER_INSTANCE_ID', '123456');
712+
const config = new ConfigObject();
713+
assert.strictEqual(config.instanceId, '123456');
714+
});
715+
716+
it('should assert if instanceId is longer than 6 characters', () => {
717+
setEnv('CLOUDSERVER_INSTANCE_ID', '1234567');
718+
assert.throws(() => new ConfigObject());
719+
});
720+
721+
it('should generate a new instanceId for external cloudserver', () => {
722+
setEnv('HOSTNAME', 'connector-cloudserver-69958b4697-bs5pn');
723+
const config = new ConfigObject();
724+
assert.strictEqual(config.instanceId, 'ebs5pn');
725+
});
726+
727+
it('should generate a new instanceId for internal cloudserver', () => {
728+
setEnv('HOSTNAME', 'internal-cloudserver-54dc76b796-48m2x');
729+
const config = new ConfigObject();
730+
assert.strictEqual(config.instanceId, 'i48m2x');
731+
});
732+
it('should generate a random instanceId if HOSTNAME has an unexpected format', () => {
733+
setEnv('HOSTNAME', 'cldsrv1');
734+
const config = new ConfigObject();
735+
assert.strictEqual(config.instanceId.length, 6);
736+
});
737+
it('should generate a random instanceId if HOSTNAME is not set', () => {
738+
const config = new ConfigObject();
739+
assert.strictEqual(config.instanceId.length, 6);
740+
});
741+
});
671742
});

‎yarn.lock‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1222,9 +1222,9 @@ arraybuffer.slice@~0.0.7:
12221222
optionalDependencies:
12231223
ioctl "^2.0.2"
12241224

1225-
"arsenal@git+https://github.com/scality/arsenal#8.1.162":
1226-
version "8.1.162"
1227-
resolved "git+https://github.com/scality/arsenal#b0cd656d907b59ad958f86d50345279fbb5e1e02"
1225+
"arsenal@git+https://github.com/scality/arsenal#8.1.163":
1226+
version "8.1.163"
1227+
resolved "git+https://github.com/scality/arsenal#a8517be0f4dd485a47f6fae83940e61f3dee1661"
12281228
dependencies:
12291229
"@azure/identity" "^3.1.1"
12301230
"@azure/storage-blob" "^12.12.0"

0 commit comments

Comments
 (0)