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 our provider to match dnsimple-ruby 4.5.0 #60
Changes from 7 commits
69c0174
3fa9f7c
3f6f299
abfd6f2
3d6ad35
8539798
989272e
a4c643e
1fa36b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -31,7 +31,7 @@ def load_current_resource | |||||
@current_resource.domain(@new_resource.domain) | ||||||
@current_resource.type(@new_resource.type) | ||||||
|
||||||
records = dnsimple_client.zones.all_records(dnsimple_client_account_id, | ||||||
records = dnsimple_client.zones.list_zone_records(dnsimple_client_account_id, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
The method was renamed from |
||||||
@new_resource.domain) | ||||||
@existing_record = records.data.detect do |record| | ||||||
(record.name == @new_resource.record_name) && (record.type == @new_resource.type) | ||||||
|
@@ -68,9 +68,10 @@ def load_current_resource | |||||
|
||||||
def create_record | ||||||
converge_by("create record #{new_resource.record_name} for domain #{new_resource.domain}") do | ||||||
dnsimple_client.zones.create_record( | ||||||
dnsimple_client_account_id, new_resource.domain, **record_options | ||||||
) | ||||||
dnsimple_client.zones.create_zone_record(dnsimple_client_account_id, | ||||||
new_resource.domain, | ||||||
**record_options | ||||||
) | ||||||
Chef::Log.info "DNSimple: created #{new_resource.type} record for #{new_resource.name}.#{new_resource.domain}" | ||||||
end | ||||||
rescue Dnsimple::RequestError => e | ||||||
|
@@ -79,9 +80,10 @@ def create_record | |||||
|
||||||
def delete_record | ||||||
converge_by("delete record #{@new_resource.record_name} from domain #{@new_resource.domain}") do | ||||||
dnsimple_client.zones.delete_record(dnsimple_client_account_id, | ||||||
@current_resource.domain, | ||||||
existing_record_id) | ||||||
dnsimple_client.zones.delete_zone_record(dnsimple_client_account_id, | ||||||
@current_resource.domain, | ||||||
existing_record_id | ||||||
) | ||||||
Chef::Log.info "DNSimple: destroyed #{@new_resource.type} record " \ | ||||||
"for #{@new_resource.name}.#{@new_resource.domain}" | ||||||
end | ||||||
|
@@ -92,10 +94,11 @@ def delete_record | |||||
def update_record | ||||||
return unless changed_record? | ||||||
converge_by("update record #{new_resource.record_name} for domain #{new_resource.domain}") do | ||||||
dnsimple_client.zones.update_record(dnsimple_client_account_id, | ||||||
new_resource.domain, | ||||||
existing_record_id, | ||||||
**record_options) | ||||||
dnsimple_client.zones.update_zone_record(dnsimple_client_account_id, | ||||||
new_resource.domain, | ||||||
existing_record_id, | ||||||
**record_options | ||||||
) | ||||||
Chef::Log.info "DNSimple: updated #{new_resource.type} record for #{new_resource.name}.#{new_resource.domain}" | ||||||
end | ||||||
end | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -29,7 +29,7 @@ | |||||
let(:response) { instance_double(Dnsimple::Response, data: data) } | ||||||
let(:data) { instance_double(Dnsimple::Struct::Whoami, account: account) } | ||||||
let(:account) { instance_double(Dnsimple::Struct::Account, id: 1) } | ||||||
let(:zones) { instance_double(Dnsimple::Client::ZonesService, all_records: zone_records, create_record: zone_record) } | ||||||
let(:zones) { instance_double(Dnsimple::Client::ZonesService, list_zone_records: zone_records, create_zone_record: zone_record) } | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
let(:zone_records) { instance_double(Dnsimple::CollectionResponse, data: [zone_record]) } | ||||||
let(:zone_record) { instance_double(Dnsimple::Struct::ZoneRecord, name: 'example_record') } | ||||||
let(:dns_record) do | ||||||
|
@@ -60,7 +60,7 @@ | |||||
|
||||||
context 'when it fails request validation' do | ||||||
before do | ||||||
allow(zones).to receive(:create_record) | ||||||
allow(zones).to receive(:create_zone_record) | ||||||
.and_raise(Dnsimple::RequestError, request_error) | ||||||
end | ||||||
|
||||||
|
@@ -90,15 +90,15 @@ | |||||
@new_resource.ttl = dns_record[:ttl] | ||||||
@new_resource.domain = dns_record_domain | ||||||
@provider.current_resource = @new_resource | ||||||
allow(zones).to receive(:delete_record) | ||||||
allow(zones).to receive(:delete_zone_record) | ||||||
end | ||||||
|
||||||
let(:client) { instance_double(Dnsimple::Client, identity: identity, zones: zones) } | ||||||
let(:identity) { instance_double(Dnsimple::Client::Identity, whoami: response) } | ||||||
let(:response) { instance_double(Dnsimple::Response, data: data) } | ||||||
let(:data) { instance_double(Dnsimple::Struct::Whoami, account: account) } | ||||||
let(:account) { instance_double(Dnsimple::Struct::Account, id: 1) } | ||||||
let(:zones) { instance_double(Dnsimple::Client::ZonesService, all_records: zone_records) } | ||||||
let(:zones) { instance_double(Dnsimple::Client::ZonesService, list_zone_records: zone_records) } | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
let(:zone_records) { instance_double(Dnsimple::CollectionResponse, data: [zone_record]) } | ||||||
let(:zone_record) { instance_double(Dnsimple::Struct::ZoneRecord, **dns_record) } | ||||||
let(:dns_record_domain) { 'example.com' } | ||||||
|
@@ -132,7 +132,7 @@ | |||||
|
||||||
context 'when it fails validation' do | ||||||
before do | ||||||
allow(zones).to receive(:delete_record) | ||||||
allow(zones).to receive(:delete_zone_record) | ||||||
.and_raise(Dnsimple::RequestError, request_error) | ||||||
allow(@provider).to receive(:existing_record_id).and_return(0) | ||||||
end | ||||||
|
@@ -171,7 +171,7 @@ | |||||
let(:response) { instance_double(Dnsimple::Response, data: data) } | ||||||
let(:data) { instance_double(Dnsimple::Struct::Whoami, account: account) } | ||||||
let(:account) { instance_double(Dnsimple::Struct::Account, id: 1) } | ||||||
let(:zones) { instance_double(Dnsimple::Client::ZonesService, all_records: zone_records, update_record: zone_record) } | ||||||
let(:zones) { instance_double(Dnsimple::Client::ZonesService, list_zone_records: zone_records, update_zone_record: zone_record) } | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
let(:zone_records) { instance_double(Dnsimple::CollectionResponse, data: [zone_record]) } | ||||||
let(:zone_record) { instance_double(Dnsimple::Struct::ZoneRecord, **dns_record) } | ||||||
let(:dns_record_domain) { 'example.com' } | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -40,7 +40,7 @@ def response | |||||
|
||||||
filter = {} | ||||||
filter = { type: @type } if @type | ||||||
client.zones.records(account_id, @zone, filter: filter).data.detect do |record| | ||||||
client.zones.list_zone_records(account_id, @zone, filter: filter).data.detect do |record| | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @martinisoft there is this one missing |
||||||
record.name == @name | ||||||
end | ||||||
end | ||||||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -40,7 +40,7 @@ def response | |||||
|
||||||
filter = {} | ||||||
filter = { type: @type } if @type | ||||||
client.zones.records(account_id, @zone, filter: filter).data.detect do |record| | ||||||
client.zones.list_zone_records(account_id, @zone, filter: filter).data.detect do |record| | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also this one |
||||||
record.name == @name | ||||||
end | ||||||
end | ||||||
|
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 for taking a look at this. I tried the patch and it does fix that error, but
list_zone_records
is paginated with 30 records per page so the logic below to detect if this in existing record often fails once someone has more than 30 records.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.
It should be
all_zone_records
instead