From caabdff6d406936810ec97dc7ee738fa7107821b Mon Sep 17 00:00:00 2001 From: Shawn Catanzarite Date: Sat, 21 Sep 2013 12:23:05 -0700 Subject: [PATCH 1/6] fix missing record data in dynect dns currently, when fog requests all records: * make a NodeList request returns a list of nodes (domains) * for each domain, make a secondary AnyRecord request for each domain returns a list of records (without data) The resulting Record objects do not contain the data of where the record points (address or cname). 'rdata' is nil I've changed the process to be: * make an AllRecord request returns a list of records (without data) * make a secondary request to *Record (using the type returned from the AllRecord request) this returns a *complete* record object (including data) The number of requests to Dynect API is the same, but the resulting objects contain all of the information. TODO: * Please help me integrate with fog testing * I had to remove (comment) the API Version header, not sure what the correct value for this is ** Dynect documentation is a bit difficult to work through --- lib/fog/dynect/dns.rb | 4 +-- lib/fog/dynect/models/dns/records.rb | 32 ++++++++++++------- .../{get_node_list.rb => get_all_records.rb} | 6 ++-- 3 files changed, 25 insertions(+), 17 deletions(-) rename lib/fog/dynect/requests/dns/{get_node_list.rb => get_all_records.rb} (89%) diff --git a/lib/fog/dynect/dns.rb b/lib/fog/dynect/dns.rb index 185af5f90..7bdd298ee 100644 --- a/lib/fog/dynect/dns.rb +++ b/lib/fog/dynect/dns.rb @@ -18,7 +18,7 @@ module Fog request_path 'fog/dynect/requests/dns' request :delete_record request :delete_zone - request :get_node_list + request :get_all_records request :get_record request :get_zone request :post_record @@ -85,7 +85,7 @@ module Fog params[:headers] ||= {} params[:headers]['Content-Type'] = 'application/json' - params[:headers]['API-Version'] = @version + #params[:headers]['API-Version'] = @version params[:headers]['Auth-Token'] = auth_token unless params[:path] == 'Session' params[:path] = "#{@path}/#{params[:path]}" unless params[:path] =~ %r{^#{Regexp.escape(@path)}/} diff --git a/lib/fog/dynect/models/dns/records.rb b/lib/fog/dynect/models/dns/records.rb index 12122e61f..496f1f3df 100644 --- a/lib/fog/dynect/models/dns/records.rb +++ b/lib/fog/dynect/models/dns/records.rb @@ -14,20 +14,28 @@ module Fog def all(options = {}) requires :zone data = [] - service.get_node_list(zone.domain, options).body['data'].each do |fqdn| - records = service.get_record('ANY', zone.domain, fqdn).body['data'] + service.get_all_records(zone.domain, options).body['data'].each do |url| + (_, r, t, z, fqdn, id) = url.split('/') + type = t.gsub(/Record$/, '') + record = service.get_record(type, zone.domain, fqdn, 'record_id' => id).body['data'] # data in format ['/REST/xRecord/domain/fqdn/identity] - records.map! do |record| - tokens = record.split('/') - { - :identity => tokens.last, - :fqdn => fqdn, - :type => tokens[2][0...-6] # everything before 'Record' - } - end - - data.concat(records) + #records.map! do |record| + # tokens = record.split('/') + # { + # :identity => tokens.last, + # :fqdn => fqdn, + # :type => tokens[2][0...-6] # everything before 'Record' + # } + #end + # + #data.concat(records) + data << { + :identity => record['record_id'], + :fqdn => record['fqdn'], + :type => record['record_type'], + :rdata => record['rdata'] + } end # leave out the default, read only records diff --git a/lib/fog/dynect/requests/dns/get_node_list.rb b/lib/fog/dynect/requests/dns/get_all_records.rb similarity index 89% rename from lib/fog/dynect/requests/dns/get_node_list.rb rename to lib/fog/dynect/requests/dns/get_all_records.rb index fc7bb7153..38dd45c93 100644 --- a/lib/fog/dynect/requests/dns/get_node_list.rb +++ b/lib/fog/dynect/requests/dns/get_all_records.rb @@ -10,19 +10,19 @@ module Fog # * options<~Hash> # * fqdn<~String> - fully qualified domain name of node to lookup - def get_node_list(zone, options = {}) + def get_all_records(zone, options = {}) requested_fqdn = options['fqdn'] || options[:fqdn] request( :expects => 200, :idempotent => true, :method => :get, - :path => ['NodeList', zone, requested_fqdn].compact.join('/') + :path => ['AllRecord', zone, requested_fqdn].compact.join('/') ) end end class Mock - def get_node_list(zone, options = {}) + def get_all_records(zone, options = {}) raise Fog::DNS::Dynect::NotFound unless zone = self.data[:zones][zone] response = Excon::Response.new From 25b69f828901cf9358c7b9411df1abde706c8bbf Mon Sep 17 00:00:00 2001 From: Shawn Catanzarite Date: Sat, 21 Sep 2013 12:38:15 -0700 Subject: [PATCH 2/6] remove commented code and update API version --- lib/fog/dynect/dns.rb | 4 ++-- lib/fog/dynect/models/dns/records.rb | 13 +------------ 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/lib/fog/dynect/dns.rb b/lib/fog/dynect/dns.rb index 7bdd298ee..34f0155c7 100644 --- a/lib/fog/dynect/dns.rb +++ b/lib/fog/dynect/dns.rb @@ -70,7 +70,7 @@ module Fog @path = options[:path] || '/REST' @persistent = options[:persistent] || false @scheme = options[:scheme] || 'https' - @version = options[:version] || '2.3.1' + @version = options[:version] || '3.5.2' @connection = Fog::Connection.new("#{@scheme}://#{@host}:#{@port}", @persistent, @connection_options) end @@ -85,7 +85,7 @@ module Fog params[:headers] ||= {} params[:headers]['Content-Type'] = 'application/json' - #params[:headers]['API-Version'] = @version + params[:headers]['API-Version'] = @version params[:headers]['Auth-Token'] = auth_token unless params[:path] == 'Session' params[:path] = "#{@path}/#{params[:path]}" unless params[:path] =~ %r{^#{Regexp.escape(@path)}/} diff --git a/lib/fog/dynect/models/dns/records.rb b/lib/fog/dynect/models/dns/records.rb index 496f1f3df..87c81e7d0 100644 --- a/lib/fog/dynect/models/dns/records.rb +++ b/lib/fog/dynect/models/dns/records.rb @@ -15,21 +15,10 @@ module Fog requires :zone data = [] service.get_all_records(zone.domain, options).body['data'].each do |url| - (_, r, t, z, fqdn, id) = url.split('/') + (_, _, t, _, fqdn, id) = url.split('/') type = t.gsub(/Record$/, '') record = service.get_record(type, zone.domain, fqdn, 'record_id' => id).body['data'] - # data in format ['/REST/xRecord/domain/fqdn/identity] - #records.map! do |record| - # tokens = record.split('/') - # { - # :identity => tokens.last, - # :fqdn => fqdn, - # :type => tokens[2][0...-6] # everything before 'Record' - # } - #end - # - #data.concat(records) data << { :identity => record['record_id'], :fqdn => record['fqdn'], From 20fbbf9ee00d9e64c3ecf0ca0cacade1a16bee5e Mon Sep 17 00:00:00 2001 From: Shawn Catanzarite Date: Mon, 23 Sep 2013 11:38:59 -0700 Subject: [PATCH 3/6] add get_node_list request again, add test for get_all_records request --- lib/fog/dynect/dns.rb | 1 + lib/fog/dynect/requests/dns/get_node_list.rb | 56 ++++++++++++++++++++ tests/dynect/requests/dns/dns_tests.rb | 8 +++ 3 files changed, 65 insertions(+) create mode 100644 lib/fog/dynect/requests/dns/get_node_list.rb diff --git a/lib/fog/dynect/dns.rb b/lib/fog/dynect/dns.rb index 34f0155c7..24f6c4321 100644 --- a/lib/fog/dynect/dns.rb +++ b/lib/fog/dynect/dns.rb @@ -18,6 +18,7 @@ module Fog request_path 'fog/dynect/requests/dns' request :delete_record request :delete_zone + request :get_node_list request :get_all_records request :get_record request :get_zone diff --git a/lib/fog/dynect/requests/dns/get_node_list.rb b/lib/fog/dynect/requests/dns/get_node_list.rb new file mode 100644 index 000000000..3418c2153 --- /dev/null +++ b/lib/fog/dynect/requests/dns/get_node_list.rb @@ -0,0 +1,56 @@ +module Fog + module DNS + class Dynect + class Real + + # Get one or more node lists + # + # ==== Parameters + # * zone<~String> - zone to lookup node lists for + # * options<~Hash> + # * fqdn<~String> - fully qualified domain name of node to lookup + + def get_node_list(zone, options = {}) + requested_fqdn = options['fqdn'] || options[:fqdn] + request( + :expects => 200, + :idempotent => true, + :method => :get, + :path => ['AllRecord', zone, requested_fqdn].compact.join('/') + ) + end + end + + class Mock + def get_node_list(zone, options = {}) + raise Fog::DNS::Dynect::NotFound unless zone = self.data[:zones][zone] + + response = Excon::Response.new + response.status = 200 + + data = [zone[:zone]] + + if fqdn = options[:fqdn] + data = data | zone[:records].collect { |type, records| records.select { |record| record[:fqdn] == fqdn } }.flatten.compact + else + data = data | zone[:records].collect { |type, records| records.collect { |record| record[:fqdn] } }.flatten + end + + response.body = { + "status" => "success", + "data" => data, + "job_id" => Fog::Dynect::Mock.job_id, + "msgs" => [{ + "INFO" => "get_tree: Here is your zone tree", + "SOURCE" => "BLL", + "ERR_CD" => nil, + "LVL" => "INFO" + }] + } + + response + end + end + end + end +end diff --git a/tests/dynect/requests/dns/dns_tests.rb b/tests/dynect/requests/dns/dns_tests.rb index a65deb868..bac3d60c5 100644 --- a/tests/dynect/requests/dns/dns_tests.rb +++ b/tests/dynect/requests/dns/dns_tests.rb @@ -116,6 +116,14 @@ Shindo.tests('Dynect::dns | DNS requests', ['dynect', 'dns']) do @dns.get_node_list(@domain).body end + get_all_records_format = shared_format.merge({ + 'data' => [String] + }) + + tests("get_all_records('#{@domain}')").formats(get_all_records_format) do + @dns.get_all_records(@domain).body + end + get_records_format = shared_format.merge({ 'data' => [String] }) From 7be677403f30d17bfe7beb77dd35c518275c50d5 Mon Sep 17 00:00:00 2001 From: Shawn Catanzarite Date: Mon, 23 Sep 2013 12:43:20 -0700 Subject: [PATCH 4/6] speed things up for get calls. add find_by_name filter. still needs some refactoring --- lib/fog/dynect/models/dns/records.rb | 55 +++++++++++++++++++++++----- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/lib/fog/dynect/models/dns/records.rb b/lib/fog/dynect/models/dns/records.rb index 87c81e7d0..9b7527d09 100644 --- a/lib/fog/dynect/models/dns/records.rb +++ b/lib/fog/dynect/models/dns/records.rb @@ -17,6 +17,51 @@ module Fog service.get_all_records(zone.domain, options).body['data'].each do |url| (_, _, t, _, fqdn, id) = url.split('/') type = t.gsub(/Record$/, '') + + # leave out the default, read only records + # by putting this here we don't make the secondary request for these records + next if ['NS', 'SOA'].include?(type) + + record = service.get_record(type, zone.domain, fqdn, 'record_id' => id).body['data'] + + data << { + :identity => record['record_id'], + :fqdn => record['fqdn'], + :type => record['record_type'], + :rdata => record['rdata'] + } + end + + load(data) + end + + def get(record_id) + requires :zone + + list = service.get_all_records(zone.domain, options).body['data'] + url = list.detect { |e| e =~ /\/#{record_id}$/ } + (_, _, t, _, fqdn, id) = url.split('/') + type = t.gsub(/Record$/, '') + record = service.get_record(type, zone.domain, fqdn, 'record_id' => id).body['data'] + + new({ + :identity => record['record_id'], + :fqdn => record['fqdn'], + :type => record['record_type'], + :rdata => record['rdata'] + }) + end + + def find_by_name(name) + requires :zone + data = [] + service.get_all_records(zone.domain, options).body['data'].select{|url| url =~ /\/#{name}\//}.each do |url| + (_, _, t, _, fqdn, id) = url.split('/') + type = t.gsub(/Record$/, '') + + # leave out the default, read only records + next if ['NS', 'SOA'].include?(type) + record = service.get_record(type, zone.domain, fqdn, 'record_id' => id).body['data'] data << { @@ -27,20 +72,12 @@ module Fog } end - # leave out the default, read only records - data = data.reject {|record| ['NS', 'SOA'].include?(record[:type])} - load(data) end - def get(record_id) - # FIXME: can this be done more efficiently? - all.detect {|record| record.identity == record_id} - end - def new(attributes = {}) requires :zone - super({ :zone => zone }.merge!(attributes)) + super({:zone => zone}.merge!(attributes)) end end From 113dadd48d23aceba84f5330a4629411f20832fa Mon Sep 17 00:00:00 2001 From: Shawn Catanzarite Date: Wed, 25 Sep 2013 10:29:22 -0700 Subject: [PATCH 5/6] remove find_by_name for now --- lib/fog/dynect/models/dns/records.rb | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/lib/fog/dynect/models/dns/records.rb b/lib/fog/dynect/models/dns/records.rb index 9b7527d09..ab09ca4fe 100644 --- a/lib/fog/dynect/models/dns/records.rb +++ b/lib/fog/dynect/models/dns/records.rb @@ -52,29 +52,6 @@ module Fog }) end - def find_by_name(name) - requires :zone - data = [] - service.get_all_records(zone.domain, options).body['data'].select{|url| url =~ /\/#{name}\//}.each do |url| - (_, _, t, _, fqdn, id) = url.split('/') - type = t.gsub(/Record$/, '') - - # leave out the default, read only records - next if ['NS', 'SOA'].include?(type) - - record = service.get_record(type, zone.domain, fqdn, 'record_id' => id).body['data'] - - data << { - :identity => record['record_id'], - :fqdn => record['fqdn'], - :type => record['record_type'], - :rdata => record['rdata'] - } - end - - load(data) - end - def new(attributes = {}) requires :zone super({:zone => zone}.merge!(attributes)) From cccb3775bb3f056749b329f9f6b5cfb4e3cd6902 Mon Sep 17 00:00:00 2001 From: Shawn Catanzarite Date: Wed, 25 Sep 2013 12:21:20 -0700 Subject: [PATCH 6/6] options not used. return nil unless we find a matching url in response --- lib/fog/dynect/models/dns/records.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/fog/dynect/models/dns/records.rb b/lib/fog/dynect/models/dns/records.rb index ab09ca4fe..2a202ea3a 100644 --- a/lib/fog/dynect/models/dns/records.rb +++ b/lib/fog/dynect/models/dns/records.rb @@ -38,8 +38,9 @@ module Fog def get(record_id) requires :zone - list = service.get_all_records(zone.domain, options).body['data'] + list = service.get_all_records(zone.domain, {}).body['data'] url = list.detect { |e| e =~ /\/#{record_id}$/ } + return unless url (_, _, t, _, fqdn, id) = url.split('/') type = t.gsub(/Record$/, '') record = service.get_record(type, zone.domain, fqdn, 'record_id' => id).body['data']