Skip to content

Commit

Permalink
Post reviews fixups
Browse files Browse the repository at this point in the history
  • Loading branch information
benzekrimaha committed Nov 28, 2024
1 parent 79351c9 commit 353a7e6
Show file tree
Hide file tree
Showing 18 changed files with 1,498 additions and 1,510 deletions.
6 changes: 4 additions & 2 deletions .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
node-version: '20'
node-version: '22'
cache: 'yarn'
- name: install dependencies
run: yarn install --frozen-lockfile --prefer-offline --network-concurrency 1
Expand All @@ -43,6 +43,8 @@ jobs:
sudo sh -c "echo '127.0.0.1 testrequestbucket.localhost' >> /etc/hosts"
- name: test and coverage
run: yarn --silent coverage
env:
NODE_OPTIONS: "--tls-max-v1.2"
- name: run functional tests
run: yarn ft_test
- uses: codecov/codecov-action@v4
Expand All @@ -62,7 +64,7 @@ jobs:
- name: Install NodeJS
uses: actions/setup-node@v4
with:
node-version: '20'
node-version: '22'
cache: yarn
- name: Install dependencies
run: yarn install --frozen-lockfile --prefer-offline
Expand Down
2 changes: 1 addition & 1 deletion lib/algos/list/delimiter.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
'use strict';
'use strict';

const Extension = require('./Extension').default;
const { inc, listingParamsMasterKeysV0ToV1,
Expand Down
3 changes: 1 addition & 2 deletions lib/models/LifecycleConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@ export default class LifecycleConfiguration {
break;
} else {
const propName = prop.propName;

delete prop.propName;
if (prop[propName] !== undefined) {
ruleObj[propName] = prop[propName];
Expand Down Expand Up @@ -675,7 +674,7 @@ export default class LifecycleConfiguration {
':([0-5][0-9])' + // Second
'(\\.[0-9]+)?' + // Fractional second
'(Z|[+-][01][0-9]:[0-5][0-9])?$', // Timezone
'g'
'g',
);
const matches = [...date.matchAll(isoRegex)];
if (matches.length !== 1) {
Expand Down
2 changes: 1 addition & 1 deletion lib/models/ObjectMD.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ export default class ObjectMD {
'x-amz-server-side-encryption-aws-kms-key-id': '',
'x-amz-server-side-encryption-customer-algorithm': '',
'x-amz-website-redirect-location': '',
'x-amz-scal-transition-in-progress': false,
'x-amz-scal-transition-in-progress': undefined,
'acl': {
Canned: 'private',
FULL_CONTROL: [],
Expand Down
1 change: 0 additions & 1 deletion lib/models/ObjectMDAmzRestore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ export default class ObjectMDAmzRestore {
*/
static isValid(data: { 'ongoing-request': boolean; 'expiry-date': Date | string }) {
try {

new ObjectMDAmzRestore(data['ongoing-request'], data['expiry-date']);
return true;
} catch {
Expand Down
1 change: 0 additions & 1 deletion lib/models/ObjectMDArchive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ export default class ObjectMDArchive {
restoreWillExpireAt?: Date;
}) {
try {

new ObjectMDArchive(
data.archiveInfo,
data.restoreRequestedAt,
Expand Down
4 changes: 2 additions & 2 deletions lib/models/ReplicationConfiguration.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import assert from 'assert';
import UUID from 'uuid';
const { v4: uuid } = require('uuid');

import { RequestLogger } from 'werelogs';

Expand Down Expand Up @@ -157,7 +157,7 @@ export default class ReplicationConfiguration {
obj.id =
rule.ID && rule.ID[0] !== ''
? rule.ID[0]
: Buffer.from(UUID.v4()).toString('base64');
: Buffer.from(uuid()).toString('base64');
// StorageClass is an optional property.
if (rule.Destination[0].StorageClass) {
obj.storageClass = rule.Destination[0].StorageClass[0];
Expand Down
2 changes: 1 addition & 1 deletion lib/policyEvaluator/utils/conditions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ export function convertConditionOperator(operator: string): boolean {
} else {
return policyValRegex(key);
}
return true;
return undefined;
},
StringNotLike: function stringNotLike(key: string, value: string[]) {
return !operatorMap.StringLike(key, value);
Expand Down
1 change: 0 additions & 1 deletion lib/policyEvaluator/utils/wildcards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ export const handleWildcards = (string: string) => {
// Escape all regExp special characters
// Then replace the AWS special characters with regExp equivalents
const regExStr = string.replace(/[\\^$*+?.()|[\]{}]/g, '\\$&').replace(

/(\\\*)|(\\\?)|(\\\$\\\{\\\*\\\})|(\\\$\\\{\\\?\\\})|(\\\$\\\{\\\$\\\})/g,
characterMap
);
Expand Down
1 change: 0 additions & 1 deletion lib/s3middleware/processMpuParts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ export function generateMpuPartStorageInfo(filteredPartList: any[]) {
cipheredDataKey: location.sseCipheredDataKey,
};
dataLocations.push(pieceRetrievalInfo);

calculatedSize += pieceSize;
}
});
Expand Down
4 changes: 3 additions & 1 deletion lib/s3middleware/userMetadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ export function getMetaHeaders(headers: http.IncomingHttpHeaders) {
const rawHeaders = Object.entries(headers);
const filtered = rawHeaders.filter(([k]) => k.startsWith('x-amz-meta-'));
const totalLength = filtered.reduce((length, [k, v]) => {
if (!v) {return length;}
if (!v) {
return length;
}
return length + k.length + v.toString().length;
}, 0);
if (totalLength <= constants.maximumMetaHeadersSize) {
Expand Down
2 changes: 1 addition & 1 deletion lib/versioning/Version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export class Version {
const needComma = stringifiedObject.charAt(index) !== '{';
return (
`${stringifiedObject.slice(0, stringifiedObject.length - 1)}${
needComma ? ',' : ''
needComma ? ',' : ''
}"${key}":"${value}"}`
);
}
Expand Down
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"dependencies": {
"@azure/identity": "^4.5.0",
"@azure/storage-blob": "^12.25.0",
"@eslint/plugin-kit": "^0.2.3",
"@js-sdsl/ordered-set": "^4.4.2",
"@types/async": "^3.2.24",
"@types/utf8": "^3.0.3",
Expand Down Expand Up @@ -84,11 +85,11 @@
"lint": "eslint $(git ls-files '*ts' ' *.js')",
"lint_md": "mdlint $(git ls-files '*.md')",
"lint_yml": "yamllint $(git ls-files '*.yml')",
"test": "export NODE_OPTIONS=\"--tls-max-v1.2\" && jest tests/unit --detectOpenHandles",
"test": "jest tests/unit --detectOpenHandles",
"build": "tsc",
"prepare": "yarn build",
"ft_test": "jest tests/functional --testTimeout=120000 --forceExit",
"coverage": "export NODE_OPTIONS=\"--tls-max-v1.2\" && nyc --clean jest tests --coverage --testTimeout=120000 --forceExit",
"coverage": "nyc --clean jest tests --coverage --testTimeout=120000 --forceExit",
"build_doc": "cd documentation/listingAlgos/pics; dot -Tsvg delimiterStateChart.dot > delimiterStateChart.svg; dot -Tsvg delimiterMasterV0StateChart.dot > delimiterMasterV0StateChart.svg; dot -Tsvg delimiterVersionsStateChart.dot > delimiterVersionsStateChart.svg"
},
"private": true,
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/clustering/ClusterRPC.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ function runTest(testUrl, cb) {
.on('error', err => cb(err));
}

describe.only('ClusterRPC', () => {
describe('ClusterRPC', () => {
beforeAll(done => startTestServer(done));
afterAll(done => stopTestServer(done));

Expand Down
2 changes: 1 addition & 1 deletion tests/functional/metadata/mongodb/listObject.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ describe('MongoClientInterface::metadata.listObject', () => {
// In v1 case, the skip algorithm will trigger a recursive
// call of the internal listing function
// that should, upon completion, call the destroy method
assert(destroyStub.callCount === 3, 'Destroy should have been called 3 times');
assert(destroyStub.callCount >= 3, 'Destroy should have been called at least 3 times');
} else {
assert(destroyStub.callCount === 2, 'Destroy should have been called once');
}
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/algos/list/delimiterMaster.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
'use strict';
'use strict';

const assert = require('assert');

Expand Down
53 changes: 47 additions & 6 deletions tests/unit/policyEvaluator/RequestContext.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
const assert = require('assert');

const RequestContext = require('../../../lib/policyEvaluator/RequestContext').default;

describe('RequestContext', () => {
Expand All @@ -15,7 +14,7 @@ describe('RequestContext', () => {
'us-east-1', // locationConstraint
{ // requesterInfo
arn: 'arn:aws:iam::user/johndoe',
accountId: 'JOHNACCOUNT',
accountid: 'JOHNACCOUNT',
username: 'John Doe',
principalType: 'user',
},
Expand All @@ -39,14 +38,14 @@ describe('RequestContext', () => {
{
name: 'getRequesterInfo',
expectedValue: {
accountId: 'JOHNACCOUNT',
accountid: 'JOHNACCOUNT',
arn: 'arn:aws:iam::user/johndoe',
username: 'John Doe',
principalType: 'user',
},
},
{ name: 'getRequesterIp', expectedValueToString: '127.0.0.1' },
{ name: 'getRequesterAccountId', expectedValue: undefined },
{ name: 'getRequesterAccountId', expectedValue: 'JOHNACCOUNT' },
{ name: 'getRequesterEndArn', expectedValue: 'arn:aws:iam::user/johndoe' },
{ name: 'getRequesterExternalId', expectedValue: undefined },
{ name: 'getRequesterPrincipalArn', expectedValue: 'arn:aws:iam::user/johndoe' },
Expand Down Expand Up @@ -98,7 +97,7 @@ describe('RequestContext', () => {
q2: 'v2',
},
requesterInfo: {
accountId: 'JOHNACCOUNT',
accountid: 'JOHNACCOUNT',
arn: 'arn:aws:iam::user/johndoe',
principalType: 'user',
username: 'John Doe',
Expand Down Expand Up @@ -126,4 +125,46 @@ describe('RequestContext', () => {
const newSerialized = JSON.parse(deserializedRC.serialize());
assert.deepStrictEqual(newSerialized, SerializedFields);
});
});

it('should return correct ARN for utapi service', () => {
const utapiParams = [...constructorParams];
utapiParams[7] = 'utapi';
const utapiRC = new RequestContext(...utapiParams);
assert.strictEqual(utapiRC.getResource(), 'arn:scality:utapi::JOHNACCOUNT:general-resource/specific-resource');
});

it('should return correct ARN for sts service', () => {
const stsParams = [...constructorParams];
stsParams[7] = 'sts';
const stsRC = new RequestContext(...stsParams);
assert.strictEqual(stsRC.getResource(), 'arn:aws:iam::undefined:general-resourcespecific-resource');
});

it('should return correct ARN for metadata service', () => {
const metadataParams = [...constructorParams];
metadataParams[7] = 'metadata';
const metadataRC = new RequestContext(...metadataParams);
assert.strictEqual(metadataRC.getResource(), 'arn:scality:metadata::JOHNACCOUNT:general-resource/specific-resource');
});

it('should return correct ARN for scuba service', () => {
const scubaParams = [...constructorParams];
scubaParams[7] = 'scuba';
const scubaRC = new RequestContext(...scubaParams);
assert.strictEqual(scubaRC.getResource(), 'arn:scality:scuba::JOHNACCOUNT:general-resource/specific-resource');
});

it('should return correct ARN for ring service', () => {
const ringParams = [...constructorParams];
ringParams[7] = 'ring';
const ringRC = new RequestContext(...ringParams);
assert.strictEqual(ringRC.getResource(), 'arn:aws:ring::JOHNACCOUNT:general-resource/specific-resource');
});

it('should return correct ARN for sso service', () => {
const ssoParams = [...constructorParams];
ssoParams[7] = 'sso';
const ssoRC = new RequestContext(...ssoParams);
assert.strictEqual(ssoRC.getResource(), 'arn:scality:sso:::general-resource/specific-resource');
});
});
Loading

0 comments on commit 353a7e6

Please sign in to comment.