From 8123ad7542cc690c37d7902bb84cbfd9d7584b41 Mon Sep 17 00:00:00 2001 From: Amir Yalon Date: Tue, 13 Jun 2017 14:12:45 +0300 Subject: [PATCH 1/3] Fix Ruby warnings in Route53 provider --- lib/puppet/provider/dns_record/route53.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/puppet/provider/dns_record/route53.rb b/lib/puppet/provider/dns_record/route53.rb index 8818a58..5264214 100644 --- a/lib/puppet/provider/dns_record/route53.rb +++ b/lib/puppet/provider/dns_record/route53.rb @@ -27,8 +27,8 @@ def route53 @@zone ||= Fog::DNS.new({ :provider => "aws", :aws_access_key_id => @username, :aws_secret_access_key => @password } ) - end - end + end + end end Puppet::Type.type(:dns_record).provide(:route53) do @@ -49,7 +49,7 @@ def create begin Puppet.debug("Attempting to create record type #{resource[:type]} for #{resource[:name]} as #{resource[:content]}") - record = @zone.records.create( :name => resource[:name], + @zone.records.create( :name => resource[:name], :value => resource[:content], :type => resource[:type], :ttl => resource[:ttl] ) From 3879737c3657bbb97c8db61a2b4eddd2d4ef0f9c Mon Sep 17 00:00:00 2001 From: Amir Yalon Date: Tue, 13 Jun 2017 15:33:44 +0300 Subject: [PATCH 2/3] Fix bugs in comparing Route53 resources against AWS responses * Use .all! instead of .all, to really get beyond the first batch. * Use .chomp('.') and .to_s where necessary to compare values correctly. * Add a check for r.type == resource[:type], to avoid destroying innocent records. * Add mk_resource_methods, because Puppet was throwing exceptions otherwise. --- lib/puppet/provider/dns_record/route53.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/puppet/provider/dns_record/route53.rb b/lib/puppet/provider/dns_record/route53.rb index 5264214..dbea30e 100644 --- a/lib/puppet/provider/dns_record/route53.rb +++ b/lib/puppet/provider/dns_record/route53.rb @@ -38,10 +38,12 @@ def route53 desc "Manage AWS Route 53 records." + mk_resource_methods + def create @zone = route53.zones.get(resource[:zone_id]) - @zone.records.all.each do |r| - if r.name == resource[:name] + @zone.records.all!.each do |r| + if r.name.chomp('.') == resource[:name] && r.type == resource[:type] Puppet.debug("Route53: Cannot modify records, must remove #{resource[:name]}.") r.destroy end @@ -64,8 +66,8 @@ def exists? @username, @password = resource[:username], resource[:password] resource[:content] = resource[:content].is_a?(Array) ? resource[:content] : resource[:content].to_a @zone = route53.zones.get(resource[:zone_id]) - records = @zone.records.all - if records.detect { |r| r.name == resource[:name] and r.value == resource[:content] and r.ttl == resource[:ttl] } + records = @zone.records.all! + if records.detect { |r| r.name.chomp('.') == resource[:name] and r.value == resource[:content] and r.ttl == resource[:ttl].to_s } return true else return false @@ -74,8 +76,8 @@ def exists? def destroy @zone = route53.zones.get(resource[:zone_id]) - @zone.records.all.each do |r| - if ( r.name == resource[:name] ) and ( r.type == resource[:type] ) + @zone.records.all!.each do |r| + if ( r.name.chomp('.') == resource[:name] ) and ( r.type == resource[:type] ) r.destroy Puppet.info("Route53: destroyed #{resource[:type]} record for #{resource[:name]}.#{resource[:domain]}") end From 0f51d76c6101006e36ba463623535c803ac434c5 Mon Sep 17 00:00:00 2001 From: Amir Yalon Date: Tue, 13 Jun 2017 15:43:31 +0300 Subject: [PATCH 3/3] Improve debug/info messages in Route53 provider * Use info level message when performing a destructive action. * Emit messages before attempting the action, so that exceptions appear in context in the logs. * Remove references to unused resource property `domain`. * Add a few more debug messages. --- lib/puppet/provider/dns_record/route53.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/puppet/provider/dns_record/route53.rb b/lib/puppet/provider/dns_record/route53.rb index dbea30e..43f25b4 100644 --- a/lib/puppet/provider/dns_record/route53.rb +++ b/lib/puppet/provider/dns_record/route53.rb @@ -42,9 +42,11 @@ def route53 def create @zone = route53.zones.get(resource[:zone_id]) + Puppet.debug("Comparing all records to #{resource[:name]} #{resource[:type]}") @zone.records.all!.each do |r| + Puppet.debug("Existing record: #{r.name} #{r.type}") if r.name.chomp('.') == resource[:name] && r.type == resource[:type] - Puppet.debug("Route53: Cannot modify records, must remove #{resource[:name]}.") + Puppet.info("Route53: Cannot modify records, must remove #{resource[:name]}.") r.destroy end end @@ -55,7 +57,7 @@ def create :value => resource[:content], :type => resource[:type], :ttl => resource[:ttl] ) - Puppet.info("Route53: Created #{resource[:type]} record for #{resource[:name]}.#{resource[:domain]}") + Puppet.info("Route53: Created #{resource[:type]} record for #{resource[:name]}") rescue Excon::Errors::UnprocessableEntity output = Nokogiri::XML( e.response.body ).xpath( "//xmlns:Message" ).text Puppet.info("Route53: #{output}") @@ -68,8 +70,10 @@ def exists? @zone = route53.zones.get(resource[:zone_id]) records = @zone.records.all! if records.detect { |r| r.name.chomp('.') == resource[:name] and r.value == resource[:content] and r.ttl == resource[:ttl].to_s } + Puppet.debug("Record #{resource[:name]} #{resource[:type]} with content #{resource[:content]} #{resource[:ttl]} found.") return true else + Puppet.debug("Record #{resource[:name]} #{resource[:type]} with content #{resource[:content]} #{resource[:ttl]} not found.") return false end end @@ -78,8 +82,8 @@ def destroy @zone = route53.zones.get(resource[:zone_id]) @zone.records.all!.each do |r| if ( r.name.chomp('.') == resource[:name] ) and ( r.type == resource[:type] ) + Puppet.info("Route53: destroying #{resource[:type]} record for #{resource[:name]}") r.destroy - Puppet.info("Route53: destroyed #{resource[:type]} record for #{resource[:name]}.#{resource[:domain]}") end end end