-
Notifications
You must be signed in to change notification settings - Fork 636
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
Adds new resource cloudflare_ip_list #766
Conversation
Does IP Lists have public API support yet? If not, I'd probably wait for that to see what the schema needs to support. |
@jacobbednarz That's IP Lists aka Rules Lists we put to cloudflare-go some weeks ago: cloudflare/cloudflare-go#507 |
Ahhh, I remember that PR but didn't associate it with the issue I raised and thought was still open. Good to see! |
As for the updated dependency, once a new release is created an automatic PR will be opened here. If it's about time for a new release, we can ping @patryk to take a look. |
Sounds good. I honestly was expecting the checks to fail because |
} | ||
|
||
func resourceCloudflareIPListImport(d *schema.ResourceData, meta interface{}) ([]*schema.ResourceData, error) { | ||
listID := d.Id() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suspect we will need the ID to be a composite ID of accountID/listID
to allow people with access to multiple accounts to determine which one they would like to manage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh thanks for raising this. I was a victim to DefaultFunc: schema.EnvDefaultFunc("CLOUDFLARE_ACCOUNT_ID", nil),
in provider.go ... I'll get the whole PR ready by - hopefully - tomorrow!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No rush! I just finally had some time this morning to review some PRs and yours was at the top of the list 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty reasonable so far 👍 A question around multiple account usage but we'll also need some tests when you're ready.
Okay, I think this is in a decent shape. Reviews appreciated :-) |
I've pushed 5794234 as the test account has a limit of 1 list at a time and the parallel tests step on top of one another. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙇♂️ thanks heaps for this @cehrig!
add 'omitempty' to APIToken 'name' and 'policies' fields
This PR adds a new resource for managing account-level IP Lists (https://blog.cloudflare.com/introducing-ip-lists/)
WIP:
- Write acceptance tests- Update cloudflare-go @jacobbednarz, i might need a hand here- DocumentationSample resource block