From 6e2895b90b4fe9d52a98119c54f6034693fdfaa2 Mon Sep 17 00:00:00 2001 From: "S. Brent Faulkner" Date: Wed, 15 Apr 2020 10:42:31 -0400 Subject: [PATCH 1/7] tests and refactor of Provider lookup --- lib/record_store/provider.rb | 13 ++++++--- test/provider_test.rb | 51 ++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 test/provider_test.rb diff --git a/lib/record_store/provider.rb b/lib/record_store/provider.rb index 96ac1174..9c1f748f 100644 --- a/lib/record_store/provider.rb +++ b/lib/record_store/provider.rb @@ -4,12 +4,11 @@ module RecordStore class Provider class << self def provider_for(zone_name) - dns = Resolv::DNS.new(nameserver: ['8.8.8.8', '8.8.4.4']) - begin - ns_server = dns.getresource(zone_name, Resolv::DNS::Resource::IN::SOA).mname.to_s + ns_server = master_nameserver_for(zone_name) rescue Resolv::ResolvError - abort("Domain doesn't exist") + $stderr.puts "Domain doesn't exist (#{zone_name})" + return end case ns_server @@ -115,6 +114,12 @@ def remove(record) def update(id, record) raise NotImplementedError end + + def master_nameserver_for(zone_name) + dns = Resolv::DNS.new(nameserver: ['8.8.8.8', '8.8.4.4']) + + dns.getresource(zone_name, Resolv::DNS::Resource::IN::SOA).mname.to_s + end end end end diff --git a/test/provider_test.rb b/test/provider_test.rb new file mode 100644 index 00000000..70966b9a --- /dev/null +++ b/test/provider_test.rb @@ -0,0 +1,51 @@ +require 'test_helper' + +class ProviderTest < Minitest::Test + def test_provider_for_recognizes_dnsimple_soa_for_zone_name + Provider.expects(:master_nameserver_for).with('dnsimple.com').returns('ns1.dnsimple.com') + provider = Provider.provider_for('dnsimple.com') + assert_equal('DNSimple', provider) + end + + def test_provider_for_recognizes_dynect_soa_for_zone_name + Provider.expects(:master_nameserver_for).with('dynect.net').returns('ns1.p19.dynect.net') + provider = Provider.provider_for('dynect.net') + assert_equal('DynECT', provider) + end + + def test_provider_for_recognizes_google_cloud_dns_soa_for_zone_name + Provider.expects(:master_nameserver_for).with('googledomains.com').returns('ns5.googledomains.com') + provider = Provider.provider_for('googledomains.com') + assert_equal('GoogleCloudDNS', provider) + end + + def test_provider_for_recognizes_nsone_soa_for_zone_name + Provider.expects(:master_nameserver_for).with('nsone.net').returns('dns1.p01.nsone.net') + provider = Provider.provider_for('nsone.net') + assert_equal('NS1', provider) + end + + def test_provider_for_recognizes_oraclecloud_soa_for_zone_name + Provider.expects(:master_nameserver_for).with('oraclecloud.net').returns('ns1.p68.dns.oraclecloud.net') + provider = Provider.provider_for('oraclecloud.net') + assert_equal('OracleCloudDNS', provider) + end + + def test_provider_for_recognizes_dnsimple_soa_for_zone_name + Provider.expects(:master_nameserver_for).with('dnsimple.com').returns('ns1.dnsimple.com') + provider = Provider.provider_for('dnsimple.com') + assert_equal('DNSimple', provider) + end + + def test_provider_for_handles_unknown_provider + Provider.expects(:master_nameserver_for).with('example.com').returns('ns.icann.org') + provider = Provider.provider_for('example.com') + assert_nil(provider) + end + + def test_provider_for_handles_unknown_domain + Provider.expects(:master_nameserver_for).with('unknown-domain.com').raises(Resolv::ResolvError) + provider = Provider.provider_for('unknown-domain.com') + assert_nil(provider) + end +end From 18db8a9bd3e46cc3a02a6b327352f894275cb3d8 Mon Sep 17 00:00:00 2001 From: "S. Brent Faulkner" Date: Wed, 15 Apr 2020 10:58:26 -0400 Subject: [PATCH 2/7] test and implement provider_for accepting NS record as input --- lib/record_store/provider.rb | 20 +++++++++++++------- test/provider_test.rb | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/lib/record_store/provider.rb b/lib/record_store/provider.rb index 9c1f748f..b88f6714 100644 --- a/lib/record_store/provider.rb +++ b/lib/record_store/provider.rb @@ -3,13 +3,19 @@ module RecordStore class Provider class << self - def provider_for(zone_name) - begin - ns_server = master_nameserver_for(zone_name) - rescue Resolv::ResolvError - $stderr.puts "Domain doesn't exist (#{zone_name})" - return - end + def provider_for(object) + ns_server = + case object + when Record::NS + object.nsdname.chomp('.') + else + begin + master_nameserver_for(object) + rescue Resolv::ResolvError + $stderr.puts "Domain doesn't exist (#{object})" + return + end + end case ns_server when /\.dnsimple\.com\z/ diff --git a/test/provider_test.rb b/test/provider_test.rb index 70966b9a..a64478bd 100644 --- a/test/provider_test.rb +++ b/test/provider_test.rb @@ -37,6 +37,42 @@ def test_provider_for_recognizes_dnsimple_soa_for_zone_name assert_equal('DNSimple', provider) end + def test_provider_for_recognizes_dnsimple_ns_record + ns = Record::NS.new(fqdn: 'dnsimple.com', ttl: 172_800, nsdname: 'ns1.dnsimple.com.') + provider = Provider.provider_for(ns) + assert_equal('DNSimple', provider) + end + + def test_provider_for_recognizes_dynect_ns_record + ns = Record::NS.new(fqdn: 'dynect.net', ttl: 172_800, nsdname: 'ns1.p19.dynect.net.') + provider = Provider.provider_for(ns) + assert_equal('DynECT', provider) + end + + def test_provider_for_recognizes_google_cloud_dns_ns_record + ns = Record::NS.new(fqdn: 'googledomains.com', ttl: 172_800, nsdname: 'ns5.googledomains.com.') + provider = Provider.provider_for(ns) + assert_equal('GoogleCloudDNS', provider) + end + + def test_provider_for_recognizes_nsone_ns_record + ns = Record::NS.new(fqdn: 'nsone.net', ttl: 172_800, nsdname: 'dns1.p01.nsone.net.') + provider = Provider.provider_for(ns) + assert_equal('NS1', provider) + end + + def test_provider_for_recognizes_oraclecloud_ns_record + ns = Record::NS.new(fqdn: 'oraclecloud.net', ttl: 172_800, nsdname: 'ns1.p68.dns.oraclecloud.net.') + provider = Provider.provider_for(ns) + assert_equal('OracleCloudDNS', provider) + end + + def test_provider_for_recognizes_dnsimple_ns_record + ns = Record::NS.new(fqdn: 'dnsimple.com', ttl: 172_800, nsdname: 'ns1.dnsimple.com.') + provider = Provider.provider_for(ns) + assert_equal('DNSimple', provider) + end + def test_provider_for_handles_unknown_provider Provider.expects(:master_nameserver_for).with('example.com').returns('ns.icann.org') provider = Provider.provider_for('example.com') From aa1aecdcdf31c19ba43403291901f163c3530d9c Mon Sep 17 00:00:00 2001 From: "S. Brent Faulkner" Date: Wed, 15 Apr 2020 15:32:00 -0400 Subject: [PATCH 3/7] add validate_authority command --- lib/record_store/cli.rb | 54 +++++++++- test/cli/validate_authority_test.rb | 151 ++++++++++++++++++++++++++++ 2 files changed, 204 insertions(+), 1 deletion(-) create mode 100644 test/cli/validate_authority_test.rb diff --git a/lib/record_store/cli.rb b/lib/record_store/cli.rb index 2c2068d0..8aa4ad3d 100644 --- a/lib/record_store/cli.rb +++ b/lib/record_store/cli.rb @@ -59,8 +59,8 @@ def list end end - option :verbose, desc: 'Print records that haven\'t diverged', aliases: '-v', type: :boolean, default: false desc 'diff', 'Displays the DNS differences between the zone files in this repo and production' + option :verbose, desc: 'Print records that haven\'t diverged', aliases: '-v', type: :boolean, default: false def diff puts "Diffing #{Zone.defined.count} zones" @@ -215,6 +215,58 @@ def assert_empty_diff end end + desc 'validate_authority', 'Validates that authoritative nameservers match the providers' + option :verbose, desc: 'Include valid zones in output', aliases: '-v', type: :boolean, default: false + def validate_authority + verbose = options.fetch('verbose') + + Zone.each do |name, zone| + authority = zone.fetch_authority + + delegation = Hash.new { |h,k| h[k] = [] } + authority.each do |ns| + delegation[Provider.provider_for(ns)] << ns + end + + delegated = delegation.keys.sort + configured = zone.config.providers.sort + + ok = configured & delegated + missing = configured - delegated + unconfigured = delegated - configured + + next if !verbose && missing.empty? && unconfigured.empty? + + puts "\n" + puts "Zone: #{name}" + + if verbose + ok.each do |provider| + puts "- #{provider}:" + delegation[provider].each do |ns| + puts " - #{ns.nsdname}" + end + end + end + + missing.each do |provider| + puts "- #{provider}: authoritative nameservers not found for configured provider" + end + + unconfigured.each do |provider| + if provider + puts "- #{provider}: unexpected authoritative nameservers found" + else + puts "- Unknown: unknown authoritative nameservers found" + end + + delegation[provider].each do |ns| + puts " - #{ns.nsdname}" + end + end + end + end + desc 'validate_records', 'Validates that all DNS records have valid definitions' def validate_records invalid_zones = [] diff --git a/test/cli/validate_authority_test.rb b/test/cli/validate_authority_test.rb new file mode 100644 index 00000000..33570e8f --- /dev/null +++ b/test/cli/validate_authority_test.rb @@ -0,0 +1,151 @@ +require 'test_helper' + +module CLI + class ValidateAuthorityTest < Minitest::Test + def teardown + super + RecordStore.config_path = DUMMY_CONFIG_PATH + end + + def test_prints_zones + mock_zone('business.new', authority: ns_unknown('new')) + + RecordStore::CLI.start(%w(validate_authority)) + + assert_includes($stdout.string, "Zone: business.new") + end + + def test_excludes_valid_zone + mock_zone('shopify.com') + + RecordStore::CLI.start(%w(validate_authority)) + + refute_includes($stdout.string, "Zone: shopify.com") + end + + def test_verbose_option_includes_valid_zones + mock_zone('shopify.com') + + RecordStore::CLI.start(%w(validate_authority --verbose)) + + assert_includes($stdout.string, "Zone: shopify.com") + end + + def test_reports_missing_authority + mock_zone('myshopify.cloud', authority: ns_dnsimple('myshopify.cloud')) + RecordStore::CLI.start(%w(validate_authority)) + + assert_includes($stdout.string, "- NS1: authoritative nameservers not found for configured provider") + end + + def test_excludes_valid_authority + mock_zone('myshopify.cloud', authority: ns_dnsimple('myshopify.cloud')) + RecordStore::CLI.start(%w(validate_authority)) + + refute_includes($stdout.string, "- DNSimple:") + end + + def test_verbose_option_includes_valid_authority + mock_zone('myshopify.cloud', authority: ns_dnsimple('myshopify.cloud')) + RecordStore::CLI.start(%w(validate_authority --verbose)) + + authority = <<~AUTHORITY + - DNSimple: + - ns1.dnsimple.com. + - ns2.dnsimple.com. + - ns3.dnsimple.com. + - ns4.dnsimple.com. + AUTHORITY + + assert_includes($stdout.string, authority) + end + + def test_reports_extra_authority + mock_zone('myshopify.cloud', providers: %w(DNSimple)) + RecordStore::CLI.start(%w(validate_authority)) + + authority = <<~AUTHORITY + - NS1: unexpected authoritative nameservers found + - ns1.p06.nsone.net. + - ns2.p06.nsone.net. + - ns3.p06.nsone.net. + - ns4.p06.nsone.net. + AUTHORITY + + assert_includes($stdout.string, authority) + end + + def test_reports_unknown_authority + mock_zone('business.new', authority: ns_unknown('new')) + + RecordStore::CLI.start(%w(validate_authority)) + + assert_includes($stdout.string, "Zone: business.new") + + authority = <<~AUTHORITY + - Unknown: unknown authoritative nameservers found + - ns1.charlestonroadregistry.com. + - ns2.charlestonroadregistry.com. + - ns3.charlestonroadregistry.com. + - ns4.charlestonroadregistry.com. + - ns5.charlestonroadregistry.com. + AUTHORITY + + assert_includes($stdout.string, authority) + end + + private + + def mock_zone(zone_name, providers: %w(DNSimple NS1), authority: ns_dnsimple(zone_name) + ns_ns1(zone_name)) + zone = Zone.new(name: zone_name, config: { providers: providers }) + zone.expects(:fetch_authority).returns(authority) + Zone.expects(:defined).returns(zone_name => zone) + end + + def ns_records(fqdn:, nsdomain:, count:, ttl: 172800) + count.times.map do |i| + Record::NS.new(fqdn: fqdn, ttl: ttl, nsdname: "ns#{i + 1}.#{nsdomain}.") + end + end + + def ns_dnsimple(fqdn) + ns_records(fqdn: fqdn, nsdomain: 'dnsimple.com', count: 4) + end + + def ns_googlecloud(fqdn) + ns_records(fqnd: fqdn, nsdomain: 'googledomains.com', count: 4) + end + + def ns_ns1(fqdn) + ns_records(fqdn: fqdn, nsdomain: 'p06.nsone.net', count: 4) + end + + def ns_unknown(fqdn) + ns_records(fqdn: fqdn, nsdomain: 'charlestonroadregistry.com', count: 5) + end + + # def test_lists_providers + # RecordStore::CLI.start(%w(info)) + + # providers = <<~PROVIDERS + # Providers: + # - DNSimple + # - NS1 + # PROVIDERS + + # assert_includes($stdout.string, providers) + # end + + # def test_lists_authoritative_nameservers + # RecordStore::CLI.start(%w(info)) + + # authority = <<~AUTHORITY + # Authoritative nameservers: + # - [NSRecord] example.com. 172800 IN NS a.iana-servers.net. + # - [NSRecord] example.com. 172800 IN NS b.iana-servers.net. + # AUTHORITY + + # assert_includes($stdout.string, authority) + # end + end +end From e80e14b1b3e7343fff503b410dffcd62cd5c5e14 Mon Sep 17 00:00:00 2001 From: "S. Brent Faulkner" Date: Wed, 15 Apr 2020 15:42:26 -0400 Subject: [PATCH 4/7] remove duplicate test --- test/provider_test.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/provider_test.rb b/test/provider_test.rb index a64478bd..d853384c 100644 --- a/test/provider_test.rb +++ b/test/provider_test.rb @@ -31,12 +31,6 @@ def test_provider_for_recognizes_oraclecloud_soa_for_zone_name assert_equal('OracleCloudDNS', provider) end - def test_provider_for_recognizes_dnsimple_soa_for_zone_name - Provider.expects(:master_nameserver_for).with('dnsimple.com').returns('ns1.dnsimple.com') - provider = Provider.provider_for('dnsimple.com') - assert_equal('DNSimple', provider) - end - def test_provider_for_recognizes_dnsimple_ns_record ns = Record::NS.new(fqdn: 'dnsimple.com', ttl: 172_800, nsdname: 'ns1.dnsimple.com.') provider = Provider.provider_for(ns) From f2e593264892c756d690e4845c63f22afdcd9d9e Mon Sep 17 00:00:00 2001 From: "S. Brent Faulkner" Date: Wed, 15 Apr 2020 15:46:07 -0400 Subject: [PATCH 5/7] remove another duplicate test --- test/provider_test.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/provider_test.rb b/test/provider_test.rb index d853384c..79e277c8 100644 --- a/test/provider_test.rb +++ b/test/provider_test.rb @@ -61,12 +61,6 @@ def test_provider_for_recognizes_oraclecloud_ns_record assert_equal('OracleCloudDNS', provider) end - def test_provider_for_recognizes_dnsimple_ns_record - ns = Record::NS.new(fqdn: 'dnsimple.com', ttl: 172_800, nsdname: 'ns1.dnsimple.com.') - provider = Provider.provider_for(ns) - assert_equal('DNSimple', provider) - end - def test_provider_for_handles_unknown_provider Provider.expects(:master_nameserver_for).with('example.com').returns('ns.icann.org') provider = Provider.provider_for('example.com') From 5aec3bc28217343f9cad4764be0d68753ea4e7f9 Mon Sep 17 00:00:00 2001 From: "S. Brent Faulkner" Date: Wed, 15 Apr 2020 15:46:51 -0400 Subject: [PATCH 6/7] rubocop: Layout/SpaceAfterComma --- lib/record_store/cli.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/record_store/cli.rb b/lib/record_store/cli.rb index 8aa4ad3d..face7b60 100644 --- a/lib/record_store/cli.rb +++ b/lib/record_store/cli.rb @@ -223,7 +223,7 @@ def validate_authority Zone.each do |name, zone| authority = zone.fetch_authority - delegation = Hash.new { |h,k| h[k] = [] } + delegation = Hash.new { |h, k| h[k] = [] } authority.each do |ns| delegation[Provider.provider_for(ns)] << ns end From 51cbe72399e153ef866512efd8dc9f3b7cd8a663 Mon Sep 17 00:00:00 2001 From: "S. Brent Faulkner" Date: Wed, 15 Apr 2020 16:09:43 -0400 Subject: [PATCH 7/7] test and added recognition of ns1 global nameservers --- lib/record_store/provider.rb | 4 +++- test/provider_test.rb | 12 ++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/lib/record_store/provider.rb b/lib/record_store/provider.rb index b88f6714..f71687c3 100644 --- a/lib/record_store/provider.rb +++ b/lib/record_store/provider.rb @@ -24,7 +24,9 @@ def provider_for(object) 'DynECT' when /\.googledomains\.com\z/ 'GoogleCloudDNS' - when /\.nsone\.net\z/ + when /\.nsone\.net\z/, + /\.ns1global\.net\z/, + /\.ns1global\.org\z/ 'NS1' when /\.oraclecloud\.net\z/ 'OracleCloudDNS' diff --git a/test/provider_test.rb b/test/provider_test.rb index 79e277c8..c2255803 100644 --- a/test/provider_test.rb +++ b/test/provider_test.rb @@ -55,6 +55,18 @@ def test_provider_for_recognizes_nsone_ns_record assert_equal('NS1', provider) end + def test_provider_for_recognizes_nsone_global_org_ns_record + ns = Record::NS.new(fqdn: 'nsone.net', ttl: 172_800, nsdname: 'dns1.g01.ns1global.org.') + provider = Provider.provider_for(ns) + assert_equal('NS1', provider) + end + + def test_provider_for_recognizes_nsone_global_net_ns_record + ns = Record::NS.new(fqdn: 'nsone.net', ttl: 172_800, nsdname: 'dns1.g01.ns1global.net.') + provider = Provider.provider_for(ns) + assert_equal('NS1', provider) + end + def test_provider_for_recognizes_oraclecloud_ns_record ns = Record::NS.new(fqdn: 'oraclecloud.net', ttl: 172_800, nsdname: 'ns1.p68.dns.oraclecloud.net.') provider = Provider.provider_for(ns)