Skip to content

Adds Backblaze B2 proxy function for Cloudflare workers#11

Open
lukestanley wants to merge 4 commits into
masterfrom
b2_s3_like_proxy
Open

Adds Backblaze B2 proxy function for Cloudflare workers#11
lukestanley wants to merge 4 commits into
masterfrom
b2_s3_like_proxy

Conversation

@lukestanley

@lukestanley lukestanley commented Dec 10, 2024

Copy link
Copy Markdown
Member

It made sense to have a service specific proxy for the S3 like API, Backblaze B2 to share all (audio files) from a file bucket in a way that is transparent to the end user.
The most simple way to deploy seemed to be a Cloudflare worker.

@lukestanley lukestanley marked this pull request as ready for review December 10, 2024 10:39
@lukestanley

lukestanley commented Dec 10, 2024

Copy link
Copy Markdown
Member Author

I have tested this on a Cloudflare worker in my name, and need to test it on a more central Kendraio organisation worker.
Then I need to use it for Fireflies meetings.

@CodeKrakken CodeKrakken left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just one small point - otherwise very nice work!

Comment thread functions/cloudflare_b2_proxy.js Outdated
console.log('Download authorisation successful');

const signedUrlData = await downloadAuthorisation.json();
const downloadUrl = `${authData.downloadUrl}/file/${B2_BUCKET_NAME}/${fileName}?Authorization=${signedUrlData.authorizationToken}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks a bit untidy - can you split it onto multiple lines more gracefully?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mh, I don't fully agree @CodeKrakken
I think that is good to have it in one line as it is 1 URL.
To me, the naming is clear enough to make it readable, and I would probably feel a bit more lost if they started to be in separate lines or even more split.
Anyway, just because is clear to me doesn't mean it is for everybody.
Can you make a example of how it would be written if it were on multiple lines @CodeKrakken ?

I am thinking maybe also something like that can be helpful for you:

const b2AuthUrl = authData.downloadUrl;
const authorizationParam =  `Authorization=${signedUrlData.authorizationToken}`;
const downloadUrl = `${b2AuthUrl}/file/${B2_BUCKET_NAME}/${fileName}? authorizationParam`;

What do you all think?

@gsambrotta gsambrotta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great job @lukestanley !
Just some minor styling comments and a bit more clarity in regard of Cloudflare.
Thank you 🙏

Comment thread functions/cloudflare_b2_proxy.js Outdated

const url = new URL(request.url);

// Return hello world if no path specified

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doesn't seems very helpful to me.
Can we not return random text which can result in confusion?
I would suggest to either return an error or a message like "please specify path" (if we want people to have hints)

Comment thread functions/cloudflare_b2_proxy.js Outdated
console.log(`Using bucket: ${B2_BUCKET_NAME} (${B2_BUCKET_ID})`);

// Step 1: Authorise with Backblaze B2
const authResponse = await fetch('https://api.backblazeb2.com/b2api/v2/b2_authorize_account', {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you please wrap the fetch() in a try/catch block?

Comment thread functions/cloudflare_b2_proxy.js Outdated
const apiUrl = authData.apiUrl;

// Step 2: Get Download Authorisation
const downloadAuthorisation = await fetch(`${apiUrl}/b2api/v2/b2_get_download_authorization`, {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same here. A try/catch block pls

Comment thread functions/cloudflare_b2_proxy.js Outdated
console.log('Download authorisation successful');

const signedUrlData = await downloadAuthorisation.json();
const downloadUrl = `${authData.downloadUrl}/file/${B2_BUCKET_NAME}/${fileName}?Authorization=${signedUrlData.authorizationToken}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mh, I don't fully agree @CodeKrakken
I think that is good to have it in one line as it is 1 URL.
To me, the naming is clear enough to make it readable, and I would probably feel a bit more lost if they started to be in separate lines or even more split.
Anyway, just because is clear to me doesn't mean it is for everybody.
Can you make a example of how it would be written if it were on multiple lines @CodeKrakken ?

I am thinking maybe also something like that can be helpful for you:

const b2AuthUrl = authData.downloadUrl;
const authorizationParam =  `Authorization=${signedUrlData.authorizationToken}`;
const downloadUrl = `${b2AuthUrl}/file/${B2_BUCKET_NAME}/${fileName}? authorizationParam`;

What do you all think?

Comment thread functions/cloudflare_b2_proxy.js Outdated

// Step 3: Fetch the content from signed URL
console.log(`Fetching content from signed URL for: ${fileName}`);
const fileResponse = await fetch(downloadUrl);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

and try/catch again :)

Comment thread functions/cloudflare_b2_proxy.js Outdated
@@ -0,0 +1,113 @@
/**
* Cloudflare Workers script to proxy requests to Backblaze B2 with

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I am a bit confuse, why is this a Cloudflare workers script? I don't see anywhere in the code that this script talks to Cloudflare. Or am I missing something?

Or maybe you means that this script is written and meant to be hosted in Cloudflare?
If so, I would encourage to change the comment as one day we could decided to host it somewhere else and i believe it will still work.

Comment thread functions/b2_proxy.js

// Return response if no path is specified
if (url.pathname === "/" || !url.pathname) {
return new Response("The proxy server is active. This is the root path. Note: To access a file resource, append the filename to the path.", {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤩

@gsambrotta gsambrotta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me! Thank you for your changes @lukestanley 🙏

@CodeKrakken CodeKrakken left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Lovely stuff!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants