Add support for Access Groups #510
Conversation
ghost
added
size/XL
kind/documentation
|
Let's table work (and review) on this until cloudflare/cloudflare-go#361 lands. Otherwise we need to keep two PRs in sync. |
|
The dependency has landed here now. Would you mind pulling in the latest changes on master and we'll kick off a review? |
This reverts commit 407b378.
|
@jacobbednarz master merged |
|
Can we please also get some tests added to this new resource? |
|
I've kicked off the integration tests for this branch and will post the results here. |
| var before, after cloudflare.AccessGroup | ||
| name := "cloudflare_access_group.test" | ||
| resourceName := strings.Split(name, ".")[1] | ||
| newAccountID := "01a7362d577a6c3019a474fd6f485634" |
There was a problem hiding this comment.
We don't currently have another test zone but perhaps if this is needed for a real resource, we can create it in CI.
|
This PR broke CI due to using hardcoded account IDs which it doesn't have access to. I've made some recommendations in the PR which should allow us to run these automatically. |
We already have this value inside of `rnd` so we don't need to re-split a string to use it.
We don't need to hardcode the resource names (or `fmt.Sprintf` them) as we can simplify it and reuse `name` which we already have available.
These tests all use the same basic setup so instead of doing another elaborate one, just duplicate the basic one with the `-updated` appended to the name string.
|
@matthieudolci i've pushed a few changes to your branch to get CI working. i'm still trying to see if we can get a second account setup to test the feel free to ping me on any commits you'd like more details on. |
|
@jacobbednarz everything looks good, thanks for the changes :-) |
We don't currently have a second account setup for this purpose. We'll just need to manually test it or keep it in mind for now.
Depends on cloudflare/cloudflare-go#361