Skip to content

Pass instanceId to mongodb metadata backend #5826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bert-e merged 4 commits into from
Jul 22, 2025
Merged

Pass instanceId to mongodb metadata backend #5826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cecde98
Pass instanceId to the mongodb metadata backend
Jul 1, 2025
98a23de
run mongo functional tests with base62 versionId encoding
Jul 1, 2025
eb2a9e1
update arsenal to include versionID changes
Jul 21, 2025
17cab88
bump version to 8.8.55
Jul 21, 2025
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
1 change: 1 addition & 0 deletions .github/docker/docker-compose.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ services:
- SCUBA_HEALTHCHECK_FREQUENCY
- S3QUOTA
- QUOTA_ENABLE_INFLIGHTS
- S3_VERSION_ID_ENCODING_TYPE
Copy link
Copy Markdown
Contributor

@francoisferrand francoisferrand Jul 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of setting this here, should we (can we?) set it automatically in config.js when backend is mongo ?

Copy link
Copy Markdown
Author

@welansari welansari Jul 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will need quite a rework as S3_VERSION_ID_ENCODING_TYPE is used by the versionID generation code inside of Arsenal, it's not a simple change in Cloudserver

env_file:
- creds.env
depends_on:
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
32 changes: 32 additions & 0 deletions lib/Config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions lib/metadata/wrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ if (clientName === 'mem') {
params = {
mongodb: config.mongodb,
replicationGroupId: config.replicationGroupId,
instanceId: config.instanceId,
config,
};
} else if (clientName === 'cdmi') {
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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": {
Expand All @@ -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",
Expand Down
109 changes: 90 additions & 19 deletions tests/unit/Config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -331,22 +331,22 @@ 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', () => {
setEnv('EXPIRE_ONE_DAY_EARLIER', true);
setEnv('TRANSITION_ONE_DAY_EARLIER', true);
setEnv('TIME_PROGRESSION_FACTOR', 2);

assert.throws(() => new ConfigObjectForTest());
assert.throws(() => new ConfigObject());
});
});

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

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

assert.deepStrictEqual(
Expand All @@ -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(
Expand All @@ -407,7 +405,6 @@ describe('Config', () => {
});

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

assert.deepStrictEqual(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -466,7 +461,6 @@ describe('Config', () => {
});

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

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

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

assert.deepStrictEqual(
Expand Down Expand Up @@ -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);
});
});
});
6 changes: 3 additions & 3 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Loading