Skip to content
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

Failing test - DNS multiaddr resolution #2221

Closed
acerone85 opened this issue Sep 19, 2024 · 2 comments · Fixed by #2222
Closed

Failing test - DNS multiaddr resolution #2221

acerone85 opened this issue Sep 19, 2024 · 2 comments · Fixed by #2222

Comments

@acerone85
Copy link
Contributor

When running source ci_checks.sh

---- dnsaddr_resolution::tests::dns_resolver__parses_all_multiaddresses_from_mainnet_dnsaddr_entry stdout ----
thread 'dnsaddr_resolution::tests::dns_resolver__parses_all_multiaddresses_from_mainnet_dnsaddr_entry' panicked at crates/services/p2p/src/dnsaddr_resolution.rs:101:14:
called `Result::unwrap()` on an `Err` value: no record found for Query { name: Name("_dnsaddr.core-test.fuellabs.net.local."), query_type: TXT, query_class: IN, mdns_unicast_response: false }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
@rymnc
Copy link
Member

rymnc commented Sep 19, 2024

that's interesting, why is the .local. getting appended for you when we make the call to core-test.fuellabs.net ?
perhaps something in your /etc/hosts/ file?

@acerone85
Copy link
Contributor Author

ok, I run some scenarios:
VPN disabled, no . at the end: test passes
VPN enabled, no . at the end: test errs with called Result::unwrap() on an Err value: no record found for Query { name: Name("_dnsaddr.core-test.fuellabs.net.local."), query_type: TXT, query_class: IN, mdns_unicast_response: false }
VPN disabled, with . at the end: test passes
VPN enabled, with . at the end: test errs with called Result::unwrap() on an Err value: request timed out
I guess nordVPN is tunnelling dns requests to their servers, and the app does not provide many functionalities to set split-tunneling 😞

rymnc added a commit that referenced this issue Sep 19, 2024
…ns resolvers (#2222)

## Linked Issues/PRs
<!-- List of related issues/PRs -->
- fixes #2221

## Description
<!-- List of detailed changes -->
Some dns resolvers `nslookup` etc, suffix dns lookups with the local
domain if they are not fully-qualified. This PR makes the dns lookup
FQDN, with a dot at the end.

## Checklist
- [x] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] New behavior is reflected in tests
- [ ] [The specification](https://github.com/FuelLabs/fuel-specs/)
matches the implemented behavior (link update PR if changes are needed)

### Before requesting review
- [x] I have reviewed the code myself
- [ ] I have created follow-up issues caused by this PR and linked them
here

### After merging, notify other teams

[Add or remove entries as needed]

- [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/)
- [ ] [Sway compiler](https://github.com/FuelLabs/sway/)
- [ ] [Platform
documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+)
(for out-of-organization contributors, the person merging the PR will do
this)
- [ ] Someone else?
MitchTurner pushed a commit that referenced this issue Sep 24, 2024
…ns resolvers (#2222)

## Linked Issues/PRs
<!-- List of related issues/PRs -->
- fixes #2221

## Description
<!-- List of detailed changes -->
Some dns resolvers `nslookup` etc, suffix dns lookups with the local
domain if they are not fully-qualified. This PR makes the dns lookup
FQDN, with a dot at the end.

## Checklist
- [x] Breaking changes are clearly marked as such in the PR description
and changelog
- [x] New behavior is reflected in tests
- [ ] [The specification](https://github.com/FuelLabs/fuel-specs/)
matches the implemented behavior (link update PR if changes are needed)

### Before requesting review
- [x] I have reviewed the code myself
- [ ] I have created follow-up issues caused by this PR and linked them
here

### After merging, notify other teams

[Add or remove entries as needed]

- [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/)
- [ ] [Sway compiler](https://github.com/FuelLabs/sway/)
- [ ] [Platform
documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+)
(for out-of-organization contributors, the person merging the PR will do
this)
- [ ] Someone else?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants