Skip to content

Commit

Permalink
[file-asset-apis] Refactor Md5 middleware logic (#1180)
Browse files Browse the repository at this point in the history
This PR makes the following changes:

- removes `ChecksumValidation` arguments to the s3 configuration
- These don't seem necessary and didn't resolve the main issue here
#1157
- Refactored the s3 middleware md5 step to only remove checksum headers
instead of all headers
-
#1157 (comment)
  • Loading branch information
sotojn authored Feb 19, 2025
1 parent 9787db1 commit bd54462
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 27 deletions.
2 changes: 1 addition & 1 deletion asset/asset.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "file",
"version": "3.0.4",
"version": "3.0.5",
"description": "A set of processors for working with files",
"minimum_teraslice_version": "2.0.0"
}
4 changes: 2 additions & 2 deletions asset/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "file",
"displayName": "Asset",
"version": "3.0.4",
"version": "3.0.5",
"private": true,
"description": "A set of processors for working with files",
"repository": {
Expand All @@ -21,7 +21,7 @@
"test": "yarn --cwd ../ test"
},
"dependencies": {
"@terascope/file-asset-apis": "~1.0.3",
"@terascope/file-asset-apis": "~1.0.4",
"@terascope/job-components": "~1.9.5",
"csvtojson": "~2.0.10",
"fs-extra": "~11.3.0",
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "file-assets-bundle",
"displayName": "File Assets Bundle",
"version": "3.0.4",
"version": "3.0.5",
"private": true,
"description": "A set of processors for working with files",
"repository": "https://github.com/terascope/file-assets.git",
Expand Down Expand Up @@ -33,7 +33,7 @@
},
"devDependencies": {
"@terascope/eslint-config": "~1.1.6",
"@terascope/file-asset-apis": "~1.0.3",
"@terascope/file-asset-apis": "~1.0.4",
"@terascope/job-components": "~1.9.5",
"@terascope/scripts": "~1.10.3",
"@types/fs-extra": "~11.0.4",
Expand Down
2 changes: 1 addition & 1 deletion packages/file-asset-apis/package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@terascope/file-asset-apis",
"displayName": "File Asset Apis",
"version": "1.0.3",
"version": "1.0.4",
"description": "file reader and sender apis",
"homepage": "https://github.com/terascope/file-assets",
"repository": "[email protected]:terascope/file-assets.git",
Expand Down
3 changes: 0 additions & 3 deletions packages/file-asset-apis/src/s3/createS3Client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,6 @@ export async function genFinalS3ClientConfig(config: S3ClientConfig): Promise<Ba
throw new Error(`S3 endpoint ${config.endpoint} cannot be https if sslEnabled is false`);
}

config.responseChecksumValidation = 'WHEN_REQUIRED';
config.requestChecksumCalculation = 'WHEN_REQUIRED';

if (!config.credentials) {
config.credentials = createCredentialsObject(config);
}
Expand Down
18 changes: 9 additions & 9 deletions packages/file-asset-apis/src/s3/s3-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,22 +130,22 @@ export async function deleteS3Objects(
const body = request.body as string;
/// Check to see if the command is of the right type
if (context.commandName === 'DeleteObjectsCommand') {
// Remove any checksum headers
Object.keys(headers).forEach((header) => {
if (
header.toLowerCase().startsWith('x-amz-checksum-')
|| header.toLowerCase().startsWith('x-amz-sdk-checksum-')
) {
delete headers[header];
}
});
/// Ensure there is a body to make a hash from
if (typeof body === 'string' && body) {
const md5Hash = crypto.createHash('md5').update(body, 'utf8')
.digest('base64');
headers['Content-MD5'] = md5Hash;
}
request.headers = headers;

Object.entries(request.headers).forEach(
([key, value]: [string, string]): void => {
if (!request.headers) {
request.headers = {};
}
(request.headers as Record<string, string>)[key] = value;
}
);
}
return next(args);
},
Expand Down
6 changes: 0 additions & 6 deletions packages/file-asset-apis/test/s3/createS3Client-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,6 @@ describe('createS3Client', () => {
+ '-----END CERTIFICATE-----',
forcePathStyle: true,
bucketEndpoint: false,
responseChecksumValidation: 'WHEN_REQUIRED',
requestChecksumCalculation: 'WHEN_REQUIRED',
maxAttempts: 3,
requestHandler: {
metadata: { handlerProtocol: 'http/1.1' },
Expand Down Expand Up @@ -78,8 +76,6 @@ describe('createS3Client', () => {
sslEnabled: false,
forcePathStyle: true,
bucketEndpoint: false,
responseChecksumValidation: 'WHEN_REQUIRED',
requestChecksumCalculation: 'WHEN_REQUIRED',
maxAttempts: 3,
credentials: { accessKeyId: 'minioadmin', secretAccessKey: 'minioadmin' }
});
Expand Down Expand Up @@ -140,8 +136,6 @@ describe('createS3Client', () => {
sslEnabled: false,
forcePathStyle: true,
bucketEndpoint: false,
responseChecksumValidation: 'WHEN_REQUIRED',
requestChecksumCalculation: 'WHEN_REQUIRED',
maxAttempts: 3,
credentials: { accessKeyId: 'minioadmin', secretAccessKey: 'minioadmin' }
});
Expand Down
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2455,7 +2455,7 @@ __metadata:
languageName: node
linkType: hard

"@terascope/file-asset-apis@npm:~1.0.3, @terascope/file-asset-apis@workspace:packages/file-asset-apis":
"@terascope/file-asset-apis@npm:~1.0.4, @terascope/file-asset-apis@workspace:packages/file-asset-apis":
version: 0.0.0-use.local
resolution: "@terascope/file-asset-apis@workspace:packages/file-asset-apis"
dependencies:
Expand Down Expand Up @@ -5719,7 +5719,7 @@ __metadata:
resolution: "file-assets-bundle@workspace:."
dependencies:
"@terascope/eslint-config": "npm:~1.1.6"
"@terascope/file-asset-apis": "npm:~1.0.3"
"@terascope/file-asset-apis": "npm:~1.0.4"
"@terascope/job-components": "npm:~1.9.5"
"@terascope/scripts": "npm:~1.10.3"
"@types/fs-extra": "npm:~11.0.4"
Expand Down Expand Up @@ -5786,7 +5786,7 @@ __metadata:
version: 0.0.0-use.local
resolution: "file@workspace:asset"
dependencies:
"@terascope/file-asset-apis": "npm:~1.0.3"
"@terascope/file-asset-apis": "npm:~1.0.4"
"@terascope/job-components": "npm:~1.9.5"
csvtojson: "npm:~2.0.10"
fs-extra: "npm:~11.3.0"
Expand Down

0 comments on commit bd54462

Please sign in to comment.