-
Notifications
You must be signed in to change notification settings - Fork 11
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
add command to validate authoritative DNS for providers #162
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
6e2895b
tests and refactor of Provider lookup
sbfaulkner 18db8a9
test and implement provider_for accepting NS record as input
sbfaulkner aa1aecd
add validate_authority command
sbfaulkner e80e14b
remove duplicate test
sbfaulkner f2e5932
remove another duplicate test
sbfaulkner 5aec3bc
rubocop: Layout/SpaceAfterComma
sbfaulkner 51cbe72
test and added recognition of ns1 global nameservers
sbfaulkner File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
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_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_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) | ||
assert_equal('OracleCloudDNS', 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 |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why a case here instead of simple
if
?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.
to easily match the types... and I'd originally considered
when String
for the current else caseI can definitely switch to if
object.is_a?(Record::NS)
instead 🤷