Add a check for AAAA records #93
Conversation
| def message | ||
| <<-MSG | ||
| Your site's DNS settings are using a custom subdomain, #{domain.host}, | ||
| that's set up with an AAAA record. GitHub pages currently does not support |
| <<-MSG | ||
| Your site's DNS settings are using a custom subdomain, #{domain.host}, | ||
| that's set up with an AAAA record. GitHub pages currently does not support | ||
| ipv6 and this AAAA record will interfere with https certificate creation. |
There was a problem hiding this comment.
IPv6, and HTTPS
Not sure if we should say anything about HTTPS directly (though it applies!) because having IPv6 records could actually cause issues serving for clients that lookup IPv6 first before IPv4. So it could be a bigger issue.
| expect(subject).to be_an_a_record | ||
| expect(subject).to be_a_valid_domain | ||
| expect(subject.invalid_aaaa_record?).to be_truthy | ||
| end |
There was a problem hiding this comment.
Can we also add a test to ensure it raises the correct error?
it "raises InvalidAAAARecordError" do
expect(-> { subject.check! }).to raise_error(Errors::InvalidAAARecordError)
endThis ensures that the checking will surface this error in the right order in the check! method.
| @@ -286,6 +293,11 @@ def a_record? | |||
| dns.first.type == Dnsruby::Types::A | |||
| end | |||
|
|
|||
| def aaaa_record? | |||
| return unless dns? | |||
| dns.index { |r| r.type == Dnsruby::Types::AAAA } | |||
There was a problem hiding this comment.
Would dns.any? make better sense here? It returns a boolean value which is what I think you want here.
|
Thanks @parkr, I force-pushed an update that addresses your feedback (and renames |
GitHub pages only supports ipv4 at this time. Having AAAA records for a pages domain is wrong and will lead to certificate enrollment failures, as Let's Encrypt would connect to the wrong host.
|
Y'know what would be useful: running the tests after saving the file in your editor 🤦♂️ And that actually found a couple of bugs. |
|
Do you any further changes? If not, I'll push a 1.9.0 😄 |
|
Not sure, depends on what other issues come up while looking at support tickets. |
|
Cool, cut a release: https://github.com/github/pages-health-check/releases/tag/v1.9.0 |
GitHub pages only supports ipv4 at this time. Having AAAA records for a
pages domain is wrong and will lead to certificate enrollment failures,
as Let's Encrypt would connect to the wrong host.