Label an IPv6 public address as AAAA, not A, in the DNS dialogs - #101
Closed
bluzername wants to merge 1 commit into
Closed
bluzername wants to merge 1 commit into
bluzername wants to merge 1 commit into
Conversation
Both BindDomainDialog and SandboxDomainCard hard-code the DNS record type to A, but the address they display come from detectPublicIp, which just take the first address in an ingress probe without checking if it is IPv4 or IPv6. When the only probed address is IPv6, the dialog show record type A next to a value that is not valid for an A record, so an IPv6-only operator get a copyable instruction that cannot work. Added dnsRecordType(ip) in publicIp.ts, it just check for a colon since every IPv6 textual form has one and no IPv4 dotted-quad does. Both dialogs now use it instead of the hard-coded 'A'. Test: publicIp.test.ts checks dnsRecordType on an IPv4 address, a compressed IPv6 address and a full-form IPv6 address. Without the function the test fail with "dnsRecordType is not a function", and after the fix all three pass.
Contributor
|
Thank you for taking the time to work on this. Dormice runs production fleets for several commercial services, and we have made the decision to keep code changes within the maintaining team so that every line is written, reviewed and verified on real hosts by the people who carry that responsibility. The policy is written up in CONTRIBUTING.md: This is about how the project is run, not about your patch. The issue it addresses (#55) stays open; when the fix lands we will credit you there and in the commit. If your PR contains reasoning that is not already in the issue, please add it to the issue so it is not lost. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes #55.
BindDomainDialog and SandboxDomainCard both hard-code the DNS record type row to
'A'. The value next to it come fromdetectPublicIp, which pick the first address out of an ingress probe'sdnsAddresseswithout checking if it is IPv4 or IPv6. If the only probed address is IPv6, the dialog show record type A with an IPv6 value, which Node itself reject as invalid for an A record. An IPv6-only operator would copy an instruction that cannot work.Change: added
dnsRecordType(ip)topackages/console/src/features/ingress/lib/publicIp.ts. It just check for a colon, every IPv6 textual form has one and no IPv4 dotted-quad does. Both dialogs (BindDomainDialog.tsx,SandboxDomainCard.tsx) now call it instead of the hard-coded string, falling back to'A'only when there is no public IP yet (placeholder state, unchanged from before).Test: added
publicIp.test.tswith three cases, an IPv4 address, a compressed IPv6 address, and a full-form IPv6 address. WithoutdnsRecordTypethe test fail with "dnsRecordType is not a function". With the fix all three pass.Ran the full chain locally:
pnpm build && pnpm typecheck && pnpm lint && pnpm test(server 681 passed, console 28 passed, sdk 31 passed, cli 64 passed, e2e 92 passed). No changeset since this only touch@dormice/console, not the published packages.