feat(publisher-s3): add Cloudflare R2 provider#4125
feat(publisher-s3): add Cloudflare R2 provider#4125matos-ed wants to merge 16 commits intoelectron:mainfrom
Conversation
|
|
||
| d(`executing: npx wrangler ${args.join(' ')}`); | ||
|
|
||
| return execa('npx', ['wrangler', ...args], { |
There was a problem hiding this comment.
It seems a bit heavy-handed to add execa as a dependency just to run an npx script. Since you're adding wrangler as a dependency anyways, does that library provide an equivalent JavaScript API that we could use?
There was a problem hiding this comment.
Hard agree. Would block this on the usage of npx, it's dangerous
There was a problem hiding this comment.
Yes! I didnt know but, as @MarshallOfSound said we could use the S3 endpoint, preventing the addition of wrangler and execa deps
I'll work on this
MarshallOfSound
left a comment
There was a problem hiding this comment.
Doesn't R2 have an S3 compatible endpoint? We should just point the S3 publisher at that and avoid duplicating code and apparently shelling out to wrangler?
…instead of apiToken
…d secretAccessKey
…Token with accessKeyId and secretAccessKey
|
BTW I've published a package to test the implementation in my app and it's working 🫶 |
malept
left a comment
There was a problem hiding this comment.
Does this have to be its own package? Can this functionality just be a set of configuration options for the existing publisher-s3 plugin, plus perhaps a guide on the docs site?
It should work well, since the API is like ~90% the same // Config would become:
{
name: '@electron-forge/publisher-s3',
config: {
provider: 'r2', // or 's3' (default)
// Common options
bucket: 'my-bucket',
accessKeyId: '...',
secretAccessKey: '...',
folder: 'releases',
region: "auto" // auto by default to r2
// R2-specific (only when provider: 'r2')
accountId: 'your-account-id',
// S3-specific (only when provider: 's3')
public: true,
sessionToken: '...',
s3ForcePathStyle: false,
releaseFileCacheControlMaxAge: 3600
}
} |
…s with detailed descriptions
…3 configuration details
erickzhao
left a comment
There was a problem hiding this comment.
Hey @matos-ed, thanks for refactoring your PR. I'm unfamiliar with the interop between R2 and the AWS S3 SDK, so I went reading in the docs. I noticed this from the Cloudflare docs:
JavaScript or TypeScript users may continue to use the @aws-sdk/client-s3 ↗ npm package as per normal. You must pass in the R2 configuration credentials when instantiating your S3 service client.
https://developers.cloudflare.com/r2/examples/aws/aws-sdk-js-v3/
A lot of config options were marked as "S3 only", but I can't tell where the source of truth is for that. Could you help explain a bit further?
Hey @erickzhao const S3 = new S3Client({
region: "auto", // Required by SDK but not used by R2
endpoint: `https://${ACCOUNT_ID}.r2.cloudflarestorage.com`,
credentials: {
accessKeyId: ACCESS_KEY_ID,
secretAccessKey: SECRET_ACCESS_KEY,
},
});So in our Publisher-S3 integration to S3Client, if the R2 provider is selected, we:
let endpoint = this.config.endpoint;
if (provider === 'r2' && !endpoint) {
endpoint = `https://${this.config.accountId}.r2.cloudflarestorage.com`;
}
const region =
this.config.region || (provider === 'r2' ? 'auto' : undefined);
const s3Client = new S3Client({
credentials:
provider === 'r2'
? {
accessKeyId: this.config.accessKeyId!,
secretAccessKey: this.config.secretAccessKey!,
}
: this.generateCredentials(),
region,
endpoint,
forcePathStyle: !!this.config.s3ForcePathStyle,
});All of this params are currently supported by // R2-specific: Set ContentType
if (provider === 'r2') {
params.ContentType =
mime.lookup(artifact.path) || 'application/octet-stream';
}I've published it in |
Closes #4124
Add Cloudflare R2 Publisher
This PR adds support for publishing Electron Forge artifacts to Cloudflare R2 storage buckets.
What's New
Use Case
Provides an alternative to AWS S3 for hosting Electron app artifacts, offering potentially lower costs and better performance through Cloudflare's global network.
Configuration Example