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 r2_bucket_binding to cloudflare_worker_script #1825

Conversation

yonran
Copy link
Contributor

@yonran yonran commented Aug 8, 2022

Add support for adding r2 bucket bindings to a cloudflare_worker_script. This is part of #1664

The new block r2_bucket_binding within a cloudflare_worker_script allows you to bind an r2 bucket to a worker script. Each binding becomes a global variable within the worker script (cloudflare_worker_script does not support creating a worker with format: module yet).

No tests yet since cloudflare-go does not yet support creating r2 buckets.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 8, 2022

changelog detected ✅

@jacobbednarz
Copy link
Member

thanks @yonran; can you please add acceptance test coverage for this new feature?

@jacobbednarz
Copy link
Member

No tests yet since cloudflare-go does not yet support creating r2 buckets.

you were about 3 hours too quick 😉 cloudflare/cloudflare-go#1028 added support for R2 bucket creation and deletion.

@yonran
Copy link
Contributor Author

yonran commented Aug 10, 2022

thanks @yonran; can you please add acceptance test coverage for this new feature?

@jacobbednarz, I tried to update the acceptance test. As cloudflare-go 0.47.0, has not been released yet, compiling requires you to upgrade to master (go get github.com/cloudflare/cloudflare-go@master) or to override to master (go mod edit -replace github.com/cloudflare/cloudflare-go=github.com/cloudflare/cloudflare-go@master)

However, I am running into a problem that the create bucket API does not seem to be working for me; when I try to POST https://api.cloudflare.com/client/v4/accounts/<ACCOUNT_ID>/r2/buckets/mytest, it returns HTTP status 404 with message “No route matches this url.” The same thing happens when I try to run npx wrangler r2 bucket create mytest, which calls PUT instead of POST.

@Cyb3r-Jak3
Copy link
Contributor

@yonran Looks like R2 had a change cloudflare/cloudflare-go#1033. Waiting on R2 team response's before patching.

@yonran
Copy link
Contributor Author

yonran commented Aug 16, 2022

Now that CloudFlare has fixed the create bucket api (cloudflare/workers-sdk#1654 (comment)), my acceptance test passes against cloudflare-go master. Still waiting on cloudflare-go to release 0.47.0 though.

TF_ACC=1 CLOUDFLARE_EMAIL=yonathan@gmail.com CLOUDFLARE_API_KEY=… CLOUDFLARE_DOMAIN=… CLOUDFLARE_ZONE_ID=… CLOUDFLARE_ACCOUNT_ID=… go test github.com/cloudflare/terraform-provider-cloudflare/internal/provider -run TestAccCloudflareWorkerScript_MultiScriptEnt

@jacobbednarz jacobbednarz added the workflow/pending-upstream-library Indicates an issue or PR requires changes from an upstream library. label Aug 17, 2022
@yonran
Copy link
Contributor Author

yonran commented Aug 18, 2022

cloudflare-go@0.47.0 has been released but it contained a number of minor breaking changes and an accidental regression to CreateZoneLockdown (lost the Priority field). So now we have to wait for cloudflare-go@0.48.0

@jacobbednarz
Copy link
Member

v0.47.1 was released to address that specific issue.

@jacobbednarz
Copy link
Member

I'll bump the library version in another PR and we can pull it in here once it is ready.

@yonran
Copy link
Contributor Author

yonran commented Aug 21, 2022

Thanks @jacobbednarz for merging the cloudflare-go upgrade! Do you want any other fixes or rebases on this PR?

@jacobbednarz
Copy link
Member

nothing more from you; i just ran out of time to run the acceptance test suite last week. the API for the R2 bucket create has (hopefully, 🤞 ) stabilised with cloudflare/cloudflare-go#1035 and i'm cutting a release now for that and another API issue. aiming to get this sorted today.

@jacobbednarz
Copy link
Member

acceptance tests are green

TF_ACC=1 go test $(go list ./...) -v -run "^TestAccCloudflareWorkerScript_" -count 1 -parallel 1 -timeout 120m -parallel 1
?       github.com/cloudflare/terraform-provider-cloudflare     [no test files]
=== RUN   TestAccCloudflareWorkerScript_Import
--- PASS: TestAccCloudflareWorkerScript_Import (14.94s)
=== RUN   TestAccCloudflareWorkerScript_MultiScriptEnt
=== PAUSE TestAccCloudflareWorkerScript_MultiScriptEnt
=== CONT  TestAccCloudflareWorkerScript_MultiScriptEnt
--- PASS: TestAccCloudflareWorkerScript_MultiScriptEnt (36.51s)
PASS
ok      github.com/cloudflare/terraform-provider-cloudflare/internal/provider   52.039s
?       github.com/cloudflare/terraform-provider-cloudflare/tools/cmd/changelog-check   [no test files]
?       github.com/cloudflare/terraform-provider-cloudflare/tools/cmd/maintainer-only-file-check        [no test files]
?       github.com/cloudflare/terraform-provider-cloudflare/tools/cmd/tf-log-check      [no test files]
?       github.com/cloudflare/terraform-provider-cloudflare/version     [no test files]

@jacobbednarz jacobbednarz merged commit e30b99a into cloudflare:master Aug 22, 2022
@jacobbednarz
Copy link
Member

thanks for the persistence here @yonran, this is great 🥇

@github-actions github-actions bot added this to the v3.22.0 milestone Aug 22, 2022
github-actions bot pushed a commit that referenced this pull request Aug 22, 2022
@github-actions
Copy link
Contributor

This functionality has been released in v3.22.0 of the Terraform Cloudflare Provider.

Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
workflow/pending-upstream-library Indicates an issue or PR requires changes from an upstream library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants