Skip to content
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.

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

feat: update s3 to v3 of aws-sdk #22

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ Persist benchmark history in Github Actions cache. Requires Github authenticatio

### `--historyS3`, `--s3`

Persist benchmark history in an Amazon S3 bucket. Requires Github authentication
Persist benchmark history in an Amazon S3 bucket. Requires Github authentication. Must pass AWS credentials via environment variables (S3_ACCESS_KEY, S3_SECRET_KEY) or can set AWS_PROFILE to pull from the shared INI credentials file.

- type: string
- default:
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@
"dependencies": {
"@actions/cache": "^1.0.7",
"@actions/github": "^5.0.0",
"@aws-sdk/client-s3": "^3.484.0",
"@aws-sdk/credential-providers": "^3.495.0",
"ajv": "^8.6.0",
"aws-sdk": "^2.932.0",
"csv-parse": "^4.16.0",
"csv-stringify": "^5.6.2",
"yargs": "^17.1.1"
Expand Down
67 changes: 42 additions & 25 deletions src/history/s3.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import path from "path";
import S3 from "aws-sdk/clients/s3";
import {S3, S3ClientConfig} from "@aws-sdk/client-s3";
import {fromIni} from "@aws-sdk/credential-providers";
import {Benchmark, BenchmarkResults} from "../types";
import {fromCsv, toCsv, extendError, AwsError} from "../utils";
import {HistoryProviderType, IHistoryProvider} from "./provider";

export type S3Config = Pick<S3.Types.ClientConfiguration, "accessKeyId" | "secretAccessKey" | "region" | "endpoint"> & {
Bucket: string;
export type S3Config = Pick<S3ClientConfig, "region" | "endpoint"> & {
bucket: string;
keyPrefix: string;
accessKeyId?: string;
secretAccessKey?: string;
profile?: string; // from INI file
};

const historyDir = "history";
Expand All @@ -23,7 +27,19 @@ export class S3HistoryProvider implements IHistoryProvider {
private s3: S3;

constructor(private readonly config: S3Config) {
this.s3 = new S3(config);
if (!(config.profile || (config.accessKeyId && config.secretAccessKey))) {
throw new Error("No S3credentials provided");
}
const credentials = config.profile
? fromIni({profile: config.profile})
: {
accessKeyId: config.accessKeyId as string,
secretAccessKey: config.secretAccessKey as string,
};
this.s3 = new S3({
region: config.region,
credentials,
});
}

/**
Expand All @@ -45,7 +61,17 @@ export class S3HistoryProvider implements IHistoryProvider {
* https://docs.aws.amazon.com/sdk-for-javascript/v2/developer-guide/loading-node-credentials-environment.html
*/
static fromEnv(): S3HistoryProvider {
const {S3_ACCESS_KEY, S3_SECRET_KEY, S3_REGION, S3_BUCKET, S3_ENDPOINT, S3_KEY_PREFIX} = process.env;
const {
AWS_PROFILE,
S3_ACCESS_KEY,
AWS_ACCESS_KEY_ID,
S3_SECRET_KEY,
AWS_SECRET_ACCESS_KEY,
S3_REGION,
S3_BUCKET,
S3_ENDPOINT,
S3_KEY_PREFIX,
} = process.env;

if (!S3_BUCKET) throw Error("No ENV S3_BUCKET");
if (!S3_KEY_PREFIX) throw Error("No ENV S3_KEY_PREFIX");
Expand All @@ -55,17 +81,18 @@ export class S3HistoryProvider implements IHistoryProvider {
// S3_ENDPOINT is optional

return new S3HistoryProvider({
accessKeyId: ifSet(S3_ACCESS_KEY),
secretAccessKey: ifSet(S3_SECRET_KEY),
bucket: S3_BUCKET,
keyPrefix: S3_KEY_PREFIX,
accessKeyId: ifSet(S3_ACCESS_KEY) || ifSet(AWS_ACCESS_KEY_ID),
secretAccessKey: ifSet(S3_SECRET_KEY) || ifSet(AWS_SECRET_ACCESS_KEY),
profile: ifSet(AWS_PROFILE),
region: ifSet(S3_REGION),
Bucket: S3_BUCKET,
endpoint: ifSet(S3_ENDPOINT),
keyPrefix: S3_KEY_PREFIX,
});
}

providerInfo(): string {
return `S3HistoryProvider, Bucket ${this.config.Bucket}`;
return `S3HistoryProvider, Bucket ${this.config.bucket}`;
}

async readLatestInBranch(branch: string): Promise<Benchmark | null> {
Expand All @@ -82,10 +109,9 @@ export class S3HistoryProvider implements IHistoryProvider {
const objects = await this.s3
.listObjects({
Prefix: this.getHistoryDir(),
Bucket: this.config.Bucket,
Bucket: this.config.bucket,
MaxKeys: maxItems,
})
.promise()
.catch((e) => {
throw extendError(e, "Error on listObjects");
});
Expand Down Expand Up @@ -131,10 +157,9 @@ export class S3HistoryProvider implements IHistoryProvider {
private async readBenchFile(key: string): Promise<Benchmark> {
const res = await this.s3
.getObject({
Bucket: this.config.Bucket,
Bucket: this.config.bucket,
Key: key,
})
.promise()
.catch((e) => {
throw extendError(e as Error, `Error on getObject ${key}`);
});
Expand All @@ -143,14 +168,7 @@ export class S3HistoryProvider implements IHistoryProvider {
throw Error("s3 response.Body is falsy");
}

let str: string;
if (typeof res.Body === "string") {
str = res.Body;
} else {
const buff = Buffer.from(res.Body as ArrayBuffer);
str = buff.toString("utf8");
}

const str = await res.Body.transformToString("utf8");
const {data, metadata} = fromCsv<BenchmarkResults>(str);
const csvMeta = metadata as unknown as CsvMeta;
return {commitSha: csvMeta.commit, results: data};
Expand All @@ -162,12 +180,11 @@ export class S3HistoryProvider implements IHistoryProvider {
const str = toCsv(benchmark.results, csvMeta as unknown as Record<string, string>);

await this.s3
.upload({
Bucket: this.config.Bucket,
.putObject({
Bucket: this.config.bucket,
Body: str,
Key: key,
})
.promise()
.catch((e) => {
throw extendError(e as Error, `Error on upload ${key}`);
});
Expand Down
3 changes: 2 additions & 1 deletion src/options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ export const options: ICliCommandOptions<CliOpts> = {
},
historyS3: {
alias: ["s3"],
description: "Persist benchmark history in an Amazon S3 bucket. Requires Github authentication",
description:
"Persist benchmark history in an Amazon S3 bucket. Requires Github authentication. Must pass AWS credentials via environment variables (S3_ACCESS_KEY, S3_SECRET_KEY) or can set AWS_PROFILE to pull from the shared INI credentials file.",
type: "string",
},

Expand Down
8 changes: 4 additions & 4 deletions test/unit/history/s3.test.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
import {expect} from "chai";
import S3 from "aws-sdk/clients/s3";
import {S3} from "@aws-sdk/client-s3";
import {Benchmark} from "../../../src/types";
import {S3HistoryProvider} from "../../../src/history/s3";
import dotenv from "dotenv";
dotenv.config();

describe("benchmark history S3 paths", () => {
const Bucket = "myproject-benchmark-data";
const bucket = "myproject-benchmark-data";
const keyPrefix = "myorg/myproject/Linux";

let historyProvider: S3HistoryProvider;
before(() => {
historyProvider = new S3HistoryProvider({Bucket, keyPrefix});
historyProvider = new S3HistoryProvider({bucket, keyPrefix});
});

it("getLatestInBranchKey", () => {
Expand Down Expand Up @@ -72,7 +72,7 @@ describe.skip("benchmark history S3", function () {
];
for (const key of keys) {
try {
await s3.deleteObject({Bucket: config.Bucket, Key: key}).promise();
await s3.deleteObject({Bucket: config.bucket, Key: key});
} catch (e) {
console.error(`Error deleting key ${key}`, e);
}
Expand Down
Loading
Loading