-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Update cloudflare-go to 0.58.1 #3288
Update cloudflare-go to 0.58.1 #3288
Conversation
Welcome @arturhoo! |
5cb13b3
to
a4f75bf
Compare
36fec31
to
e2ff260
Compare
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: arturhoo, szuecs The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/assign @njuettner |
I am not yet allowed to press the test run button |
Signed-off-by: Artur Rodrigues <artur.rodrigues@lacework.net>
Signed-off-by: Artur Rodrigues <artur.rodrigues@lacework.net>
Signed-off-by: Artur Rodrigues <artur.rodrigues@lacework.net>
e2ff260
to
2d4ea13
Compare
@njuettner would you be able to help with the approval for running the test workflows? I suppose this PR also needs a lgtm. Once this gets merged, I'll create a PR for the changes in https://github.com/arturhoo/external-dns/compare/cloudflare-go-0.58.0...arturhoo:external-dns:cloudflare-paginated-list-requests?expand=1 - thanks |
@arturhoo can you fix the CI errors (linting). |
/lgtm |
Description
Update
cloudflare-go
to the latest version.There are breaking changes between 0.50.0 and 0.58.0 introduced in cloudflare/cloudflare-go#1151 that change the method signatures used in the provider.
I tried to minimize the changes here, by keeping the 'generic'
cloudFlareChange
struct that encapsulatescloudflare.DNSRecord
and is used to keep track of changes needed. Only by the time that we actually call the cloudflare library functions that we convert from the generic container to the newly specialized ones (cloudflare.UpdateDNSRecordParams
,cloudflare.CreateDNSRecordParams
) through a generic function.The alternative would've have been to split the slice of type
[]*cloudFlareChange{}
into perhaps a map that contains several change types holding the new parameter types.Why make this change?
Since version 0.58.0, the
cloudflare-go
library has aPerPage
attribute oncloudflare.ListDNSRecordsParams
which will help reduce the number of API calls made to Cloudflare and prevent rate limit events:https://api.cloudflare.com/#dns-records-for-a-zone-list-dns-records
Passing in the max allowed value (or parameterizing it) will be done in a follow-up PR: https://github.com/arturhoo/external-dns/compare/cloudflare-go-0.58.0...arturhoo:external-dns:cloudflare-paginated-list-requests?expand=1
Checklist