diff --git a/.github/docker/docker-compose.yaml b/.github/docker/docker-compose.yaml index 18cfa86b14..4a96352b6a 100644 --- a/.github/docker/docker-compose.yaml +++ b/.github/docker/docker-compose.yaml @@ -45,6 +45,7 @@ services: - SCUBA_HEALTHCHECK_FREQUENCY - S3QUOTA - QUOTA_ENABLE_INFLIGHTS + - S3_VERSION_ID_ENCODING_TYPE env_file: - creds.env depends_on: diff --git a/.github/workflows/tests.yaml b/.github/workflows/tests.yaml index dbbaeaa450..bf4ed253fd 100644 --- a/.github/workflows/tests.yaml +++ b/.github/workflows/tests.yaml @@ -293,6 +293,7 @@ jobs: S3METADATA: mongodb S3KMS: file S3_LOCATION_FILE: /usr/src/app/tests/locationConfig/locationConfigTests.json + S3_VERSION_ID_ENCODING_TYPE: base62 DEFAULT_BUCKET_KEY_FORMAT: v1 METADATA_MAX_CACHED_BUCKETS: 1 MONGODB_IMAGE: ghcr.io/${{ github.repository }}/ci-mongodb:${{ github.sha }} diff --git a/lib/Config.js b/lib/Config.js index 025cf66e20..7f4ba2944a 100644 --- a/lib/Config.js +++ b/lib/Config.js @@ -968,6 +968,38 @@ class Config extends EventEmitter { this.replicationGroupId = 'RG001'; } + const instanceId = process.env.CLOUDSERVER_INSTANCE_ID || config.instanceId; + if (instanceId) { + assert(typeof instanceId === 'string', + 'bad config: instanceId must be a string'); + // versionID generation code will truncate instanceId to 6 characters + // so we enforce this limit here to make the behavior predictable + assert(instanceId.length <= 6, + 'bad config: instanceId must be at most 6 characters long'); + this.instanceId = instanceId; + } else if (process.env.HOSTNAME) { + // If running in Kubernetes, the pod name is set in the HOSTNAME env var + // and is of the form: cloudserver-7869f99955-nr984 + // where: + // - cloudserver is the name of the pod + // - 7869f99955 is a long hash that guarantees uniqueness within the namespace + // - nr984 is a short hash that is unique only within the current ReplicaSet/Deployment + // We use the last part of the pod name as the instanceId, prefixed with 'i' for internal + // or 'e' for external cloudserver. + // This allows having a unique identifier that will be used when generating versionIds + const instanceType = process.env.HOSTNAME.includes('internal') ? 'i' : 'e'; + const parts = process.env.HOSTNAME.split('-'); + if (parts.length >= 2) { + const shortId = parts[parts.length - 1]; + this.instanceId = instanceType + shortId; + } else { + // fallback: generate a random string if format is unexpected + this.instanceId = uuidv4().replace(/-/g, '').slice(0, 6); + } + } else { + this.instanceId = uuidv4().replace(/-/g, '').slice(0, 6); + } + this.replicationEndpoints = []; if (config.replicationEndpoints) { const { replicationEndpoints } = config; diff --git a/lib/metadata/wrapper.js b/lib/metadata/wrapper.js index 047c3c04da..6bd800e60f 100644 --- a/lib/metadata/wrapper.js +++ b/lib/metadata/wrapper.js @@ -30,6 +30,7 @@ if (clientName === 'mem') { params = { mongodb: config.mongodb, replicationGroupId: config.replicationGroupId, + instanceId: config.instanceId, config, }; } else if (clientName === 'cdmi') { diff --git a/package.json b/package.json index 7f05204e5a..2f24461885 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@zenko/cloudserver", - "version": "8.8.54", + "version": "8.8.55", "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol", "main": "index.js", "engines": { @@ -21,7 +21,7 @@ "dependencies": { "@azure/storage-blob": "^12.12.0", "@hapi/joi": "^17.1.0", - "arsenal": "git+https://github.com/scality/arsenal#8.1.162", + "arsenal": "git+https://github.com/scality/arsenal#8.1.163", "async": "~2.5.0", "aws-sdk": "2.905.0", "bucketclient": "scality/bucketclient#8.1.14", diff --git a/tests/unit/Config.js b/tests/unit/Config.js index 2894016e61..cd08b167c0 100644 --- a/tests/unit/Config.js +++ b/tests/unit/Config.js @@ -1,7 +1,10 @@ const assert = require('assert'); +const sinon = require('sinon'); +const fs = require('fs'); +const path = require('path'); const { azureArchiveLocationConstraintAssert, - ConfigObject: ConfigObjectForTest, + ConfigObject, } = require('../../lib/Config'); describe('Config', () => { @@ -25,7 +28,6 @@ describe('Config', () => { }); it('should emit an event when auth data is updated', done => { - const { ConfigObject } = require('../../lib/Config'); const config = new ConfigObject(); let emitted = false; config.on('authdata-update', () => { @@ -209,8 +211,6 @@ describe('Config', () => { }); describe('getAzureStorageAccountName', () => { - const { ConfigObject } = require('../../lib/Config'); - it('should return account name from config', () => { setEnv('azurebackend_AZURE_STORAGE_ACCOUNT_NAME', ''); const config = new ConfigObject(); @@ -259,7 +259,7 @@ describe('Config', () => { describe('time options', () => { it('should getTimeOptions', () => { - const config = new ConfigObjectForTest(); + const config = new ConfigObject(); const expectedOptions = { expireOneDayEarlier: false, transitionOneDayEarlier: false, @@ -273,7 +273,7 @@ describe('Config', () => { it('should getTimeOptions with TIME_PROGRESSION_FACTOR', () => { setEnv('TIME_PROGRESSION_FACTOR', 2); - const config = new ConfigObjectForTest(); + const config = new ConfigObject(); const expectedOptions = { expireOneDayEarlier: false, transitionOneDayEarlier: false, @@ -287,7 +287,7 @@ describe('Config', () => { it('should getTimeOptions with EXPIRE_ONE_DAY_EARLIER', () => { setEnv('EXPIRE_ONE_DAY_EARLIER', true); - const config = new ConfigObjectForTest(); + const config = new ConfigObject(); const expectedOptions = { expireOneDayEarlier: true, transitionOneDayEarlier: false, @@ -301,7 +301,7 @@ describe('Config', () => { it('should getTimeOptions with TRANSITION_ONE_DAY_EARLIER', () => { setEnv('TRANSITION_ONE_DAY_EARLIER', true); - const config = new ConfigObjectForTest(); + const config = new ConfigObject(); const expectedOptions = { expireOneDayEarlier: false, transitionOneDayEarlier: true, @@ -316,7 +316,7 @@ describe('Config', () => { setEnv('EXPIRE_ONE_DAY_EARLIER', true); setEnv('TRANSITION_ONE_DAY_EARLIER', true); - const config = new ConfigObjectForTest(); + const config = new ConfigObject(); const expectedOptions = { expireOneDayEarlier: true, transitionOneDayEarlier: true, @@ -331,14 +331,14 @@ describe('Config', () => { setEnv('EXPIRE_ONE_DAY_EARLIER', true); setEnv('TIME_PROGRESSION_FACTOR', 2); - assert.throws(() => new ConfigObjectForTest()); + assert.throws(() => new ConfigObject()); }); it('should throw error if TRANSITION_ONE_DAY_EARLIER and TIME_PROGRESSION_FACTOR', () => { setEnv('TRANSITION_ONE_DAY_EARLIER', true); setEnv('TIME_PROGRESSION_FACTOR', 2); - assert.throws(() => new ConfigObjectForTest()); + assert.throws(() => new ConfigObject()); }); it('should throw error if both EXPIRE/TRANSITION_ONE_DAY_EARLIER and TIME_PROGRESSION_FACTOR', () => { @@ -346,7 +346,7 @@ describe('Config', () => { setEnv('TRANSITION_ONE_DAY_EARLIER', true); setEnv('TIME_PROGRESSION_FACTOR', 2); - assert.throws(() => new ConfigObjectForTest()); + assert.throws(() => new ConfigObject()); }); }); @@ -364,7 +364,6 @@ describe('Config', () => { }); it('should set up scuba', () => { - const { ConfigObject } = require('../../lib/Config'); const config = new ConfigObject(); assert.deepStrictEqual( @@ -380,7 +379,6 @@ describe('Config', () => { setEnv('SCUBA_HOST', 'scubahost'); setEnv('SCUBA_PORT', 1234); - const { ConfigObject } = require('../../lib/Config'); const config = new ConfigObject(); assert.deepStrictEqual( @@ -407,7 +405,6 @@ describe('Config', () => { }); it('should set up quota', () => { - const { ConfigObject } = require('../../lib/Config'); const config = new ConfigObject(); assert.deepStrictEqual( @@ -423,7 +420,6 @@ describe('Config', () => { setEnv('QUOTA_MAX_STALENESS_MS', 1234); setEnv('QUOTA_ENABLE_INFLIGHTS', 'true'); - const { ConfigObject } = require('../../lib/Config'); const config = new ConfigObject(); assert.deepStrictEqual( @@ -439,7 +435,6 @@ describe('Config', () => { setEnv('QUOTA_MAX_STALENESS_MS', 'notanumber'); setEnv('QUOTA_ENABLE_INFLIGHTS', 'true'); - const { ConfigObject } = require('../../lib/Config'); const config = new ConfigObject(); assert.deepStrictEqual( @@ -466,7 +461,6 @@ describe('Config', () => { }); it('should set up utapi local cache', () => { - const { ConfigObject } = require('../../lib/Config'); const config = new ConfigObject(); assert.deepStrictEqual( @@ -480,7 +474,6 @@ describe('Config', () => { }); it('should set up utapi redis', () => { - const { ConfigObject } = require('../../lib/Config'); const config = new ConfigObject(); assert.deepStrictEqual( @@ -668,4 +661,82 @@ describe('Config', () => { assert.throws(() => azureArchiveLocationConstraintAssert(locationObj)); }); }); + + describe('instanceId', () => { + let defaultConfig; + + before(() => { + setEnv('S3_CONFIG_FILE', 'tests/unit/testConfigs/allOptsConfig/config.json'); + // Import config + const defaultConfigPath = path.join(__dirname, './testConfigs/allOptsConfig/config.json'); + defaultConfig = require(defaultConfigPath); + }); + + afterEach(() => { + sinon.restore(); + }); + + it('should use the instance id set in the config', () => { + // Stub fs.readFileSync to return a specific config.json content that includes instanceId + const originalReadFileSync = fs.readFileSync; + const readFileSyncStub = sinon.stub(fs, 'readFileSync'); + // Mock only config.json file + readFileSyncStub + .withArgs(sinon.match(/\/config\.json$/)) + .returns(JSON.stringify({ ...defaultConfig, instanceId: 'test' })); + // For all other files, use the original readFileSync + readFileSyncStub + .callsFake((filePath, ...args) => originalReadFileSync(filePath, ...args)); + // Create a new ConfigObject instance + const config = new ConfigObject(); + assert.strictEqual(config.instanceId, 'test'); + }); + + it('should assert if instanceId is not a string', () => { + // Stub fs.readFileSync to return a specific config.json content that includes instanceId + const originalReadFileSync = fs.readFileSync; + const readFileSyncStub = sinon.stub(fs, 'readFileSync'); + // Mock only config.json file + readFileSyncStub + .withArgs(sinon.match(/\/config\.json$/)) + .returns(JSON.stringify({ ...defaultConfig, instanceId: 1234 })); + // For all other files, use the original readFileSync + readFileSyncStub + .callsFake((filePath, ...args) => originalReadFileSync(filePath, ...args)); + // Create a new ConfigObject instance + assert.throws(() => new ConfigObject()); + }); + + it('should use the instance id set in the env var', () => { + setEnv('CLOUDSERVER_INSTANCE_ID', '123456'); + const config = new ConfigObject(); + assert.strictEqual(config.instanceId, '123456'); + }); + + it('should assert if instanceId is longer than 6 characters', () => { + setEnv('CLOUDSERVER_INSTANCE_ID', '1234567'); + assert.throws(() => new ConfigObject()); + }); + + it('should generate a new instanceId for external cloudserver', () => { + setEnv('HOSTNAME', 'connector-cloudserver-69958b4697-bs5pn'); + const config = new ConfigObject(); + assert.strictEqual(config.instanceId, 'ebs5pn'); + }); + + it('should generate a new instanceId for internal cloudserver', () => { + setEnv('HOSTNAME', 'internal-cloudserver-54dc76b796-48m2x'); + const config = new ConfigObject(); + assert.strictEqual(config.instanceId, 'i48m2x'); + }); + it('should generate a random instanceId if HOSTNAME has an unexpected format', () => { + setEnv('HOSTNAME', 'cldsrv1'); + const config = new ConfigObject(); + assert.strictEqual(config.instanceId.length, 6); + }); + it('should generate a random instanceId if HOSTNAME is not set', () => { + const config = new ConfigObject(); + assert.strictEqual(config.instanceId.length, 6); + }); + }); }); diff --git a/yarn.lock b/yarn.lock index 123ad14bfe..bcf6777ef2 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1222,9 +1222,9 @@ arraybuffer.slice@~0.0.7: optionalDependencies: ioctl "^2.0.2" -"arsenal@git+https://github.com/scality/arsenal#8.1.162": - version "8.1.162" - resolved "git+https://github.com/scality/arsenal#b0cd656d907b59ad958f86d50345279fbb5e1e02" +"arsenal@git+https://github.com/scality/arsenal#8.1.163": + version "8.1.163" + resolved "git+https://github.com/scality/arsenal#a8517be0f4dd485a47f6fae83940e61f3dee1661" dependencies: "@azure/identity" "^3.1.1" "@azure/storage-blob" "^12.12.0"