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

Add support for Cloudflare Workers #5258

Merged
merged 18 commits into from Sep 11, 2018
Merged

Conversation

pinkerton
Copy link
Contributor

@pinkerton pinkerton commented Aug 28, 2018

What did you implement:

Closes #4948

This PR adds templates and documentation for Cloudflare Workers as a part of the provider plugin that we're reviewing and will publish on NPM very soon.

How did you implement it:

Added cloudflare-workers and cloudflare-workers-enterprise templates and whitelisted them.

Regarding documentation, I am not 100% sure how this is generated and how images will be hosted (I see they are mostly in an S3 bucket).

How can we verify it:

  1. Set up a Cloudflare account and enable Workers
  2. export CLOUDFLARE_AUTH_KEY=YOUR_CLOUDFLARE_GLOBAL_API_KEY (get your Global API key from your Cloudflare profile page).
  3. export CLOUDFLARE_AUTH_EMAIL=you@example.com
  4. git clone https://github.com/pinkerton/serverless.git
  5. git clone https://github.com/cloudflare/serverless-cloudflare-workers.git
  6. cd serverless
  7. npm install
  8. npm link ../serverless-cloudflare-workers
  9. cd ..
  10. ./serverless/bin/serverless create --template cloudflare-workers --path my-project
  11. cd my-project
  12. npm install
  13. npm link ../serverless-cloudflare-workers
  14. Get your account ID and put it in serverless.yml (get it from the URL when using the Cloudflare dashboard).
  15. Get your zone ID and put it in serverless.yml (zoneId from the overview tab after selecting the desired zone from the Cloudflare dashboard).
  16. Update your zone and routes in serverless.yml.
  17. serverless deploy
  18. serverless invoke -f helloWorld

serverless create

serverless-create

serverless deploy & serverless invoke

serverless-deploy-invoke

Todos:

  • Write tests
  • Write documentation
  • Fix linting errors
  • Make sure code coverage hasn't dropped
  • Provide verification config / commands / resources
  • Enable "Allow edits from maintainers" for this PR
  • Update the messages below

Is this ready for review?: YES
Is it a breaking change?: NO

@horike37
Copy link
Member

@pinkerton
plugins list within README was updated as well. Is this intentional?

@pinkerton
Copy link
Contributor Author

@horike37 That was accidental. I must have committed it by mistake when rebasing / merging. Will revert.

@pinkerton
Copy link
Contributor Author

Un-committed the accidental README changes.

service:
name: hello-world
config:
accountId: CLOUDFLARE_ACCOUNT_ID
Copy link
Contributor

Choose a reason for hiding this comment

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

@pinkerton Is the accountId really required here. I've tried to deploy and remove without it and it seems to work fine without it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting, that may have been left in accidentally. I'm double-checking if we can remove that.

Copy link
Contributor

Choose a reason for hiding this comment

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

So, if the user has the multiscript feature enabled, the api call uses accountId as one of the parameters.
https://github.com/cloudflare/serverless-cloudflare-workers/blob/master/deploy/lib/multiscript.js#L27

Whereas, for singleScript feature, accountId is not required.
https://github.com/cloudflare/serverless-cloudflare-workers/blob/master/deploy/lib/singlescript.js#L27

All in all, accountId is required in MultiScript, but optional in SingleScript

Copy link
Contributor

@RaeesBhatti RaeesBhatti left a comment

Choose a reason for hiding this comment

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

Tested end to end. Missing documentation added. LGTM

@pinkerton
Copy link
Contributor Author

Thanks so much for taking a look @RaeesBhatti.

What version of the CLI tool will this ship in? It looks like maybe 1.30.4 or 1.31.0. Our docs have an old version in them as a placeholder for the minimum version that I'd like to update.

I also need to add our images to our documentation.

And how do we get our docs linked on this page https://serverless.com/framework/docs/ ?

@RaeesBhatti
Copy link
Contributor

@pinkerton The docs on the website will be updated automatically on new build. But I noticed that serverless-cloudflare-workers plugin is not published on npm, we're planning to publish a post on Tuesday at Serverless blog regarding Cloudflare support. It would be ideal if the plugin was published before that.
@eahefnawy Can you please address the question about versioning. Thanks

@pinkerton
Copy link
Contributor Author

Thanks @RaeesBhatti. I'm working on publishing our plugin to NPM now. Running into an issue where I can publish to our org as an org-scoped package, but that would make the name @cloudflare/serverless-cloudflare-workers, which I'm trying to avoid. I can publish it without the scope to my own profile, however.

Will have this resolved soon.

@eahefnawy
Copy link
Member

@pinkerton it'll likely be v1.31.0, since there are tons of new, awesome stuff in here! We really appreciate it!

@pinkerton
Copy link
Contributor Author

Made some copy and visual changes to the docs. They should be good to go!

@horike37 horike37 added this to the 1.31.0 milestone Sep 11, 2018
@RaeesBhatti RaeesBhatti merged commit f53f047 into serverless:master Sep 11, 2018
@pinkerton
Copy link
Contributor Author

Thanks for catching that @RaeesBhatti

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.

None yet

5 participants