tailhook/abstract-ns

Abstract name service traits for rust

abstract-ns's Issues

Reconsider Port Handling (or rather: mention them in the documentation)

I was recently very surprised to receive the following error message from this crate: error: name "foo.example.com" is invalid: SRV records are not supported, please specify default port: Nowhere in the crate documentation (at least not where I could find it) was a default port mentioned, and it came as a huge surprise that this crate would even concern itself with the concept of ports.

(My primary entrypoint was the ns-dns-tokio crate. While writing this issue I actually read the readme of abstract-ns and just came to the conclusion that the purpose of this crate is simply not what I wanted it to be (simple DNS resolution)).

Well anyway I propose that either of the following is done:

• Improve the documentation with regard to port handling
• Modify the crate to work with plain ip addresses although that probably goes contrary to the purpose of this crate

request: omit patch version in Cargo.toml

I have a lib which uses ns-dns-tokio, tokio-core and futures. I'm unable to update tokio to latest version, cause ns-dns-tokio requires exact version - 0.1.0.

cargo build -p p2p
Updating registry `https://github.com/rust-lang/crates.io-index`
error: failed to select a version for `tokio-core` (required by `ns-dns-tokio`):
all possible versions conflict with previously selected versions of `tokio-core`
  version 0.1.1 in use by tokio-core v0.1.1
  possible versions to select: 0.1.0

Can you omit patch version in Cargo.toml files? Use 0.1 instead of 0.1.0

An attempt to convert into std::sync::Arc<str> from std::convert::From<&str>

Hello!

I'm trying building a test project with dependencies (code, and so on) that were mentioned in PR#2 for lapin-futures-tls crate, but instead of getting a successfully compiled code, I'm receiving the following error:

   Compiling coco v0.1.1
   Compiling num-integer v0.1.35
   Compiling unreachable v1.0.0
   Compiling tokio-timer v0.1.2
   Compiling base64 v0.6.0
   Compiling idna v0.1.4
   Compiling bytes v0.4.6
   Compiling abstract-ns v0.4.2
error[E0277]: the trait bound `std::sync::Arc<str>: std::convert::From<&str>` is not satisfied
  --> /Users/savicvalera/.cargo/registry/src/github.com-1ecc6299db9ec823/abstract-ns-0.4.2/src/name.rs:67:23
   |
67 |         Ok(Name(value.into()))
   |                       ^^^^ the trait `std::convert::From<&str>` is not implemented for `std::sync::Arc<str>`
   |
   = help: the following implementations were found:
             <std::sync::Arc<T> as std::convert::From<T>>
   = note: required because of the requirements on the impl of `std::convert::Into<std::sync::Arc<str>>` for `&str`

error: aborting due to previous error

error: Could not compile `abstract-ns`.
warning: build failed, waiting for other jobs to finish...
error: build failed

Do you have any ideas how to fix this issue? I'd tried to clean up cargo caches and indices, but it wasn't helped.

crates.io / git version mismatch for ns-dns-tokio

The currently published version of ns-dns-tokio on crates.io is 0.3, however https://github.com/tailhook/abstract-ns/blob/master/ns-dns-tokio/Cargo.toml has a version of 0.2, which seems kinda inconsistent (and makes things annoying when using a git url and a version field in the dependency specification).

Another round of changes

Just quick overview of new traits and helpers (with many details omitted):

struct Name<'x>(&'x str, Option<u16>);
trait PollResovler {
  type Future: Future<Item=Address, Error>;
  fn resolve(&self, name: Name) -> Self::Future;
}
trait Resolver: PollResolver {
  type Stream: Stream<Item=Address, Void>;
  fn subscribe(&self, name: Name) -> Self::Stream;
}
struct PollAdapter<R: PollResolver>(R);
impl PollResolver for PollAdapter {...}
impl Resolver for PollAdapter {...}

struct Router {...}
impl PollResolver for Router {
  type Future: oneshot::Receiver<Address>;
}
impl Resolver for Router {
  type Stream: tip_channel::Receiver<Addresss>;
}

Details:

1. BoxStream and BoxFuture in Resolver has always been temporary hack, let's replace them with associated types
2. We should split out PollResolver from Resolver
3. What has been done in default method subscribe should be done in PollAdapter, so it's configurable (updates and poll interval)
4. Resolver::subscribe must return infallible stream, we should consider using void crate, or put our own void type or use one from futures when that is added. This caused pain previously. The idea here is to retry resolving name as long as stream is used.
5. Router::subscribe (the structure not trait) should spawn a stream from original resolver stream and connect it through something like a Tip channel, either from futures crate or vendored type. (or alternatively, use FuturesUnordered)
6. Name type is a structure

The Problems It Solves

1. Errors in subscriptions streams were pain. It's was unclear when it's valid to return error.
2. Streams become more and more complex, and connection pool should poll the DNS subscription stream on every wake up (this is how futures work). So let's make polled structure very cheap channel.
3. Middlewares can now be made without virtual call overhead
4. Updating Router configuration on the fly should work
5. Using bounded or unbounded stream between router and consumer has it's own downsides (bounded: old value may be used at any time in future, unbounded: memory leak can occur), hence the Tip channel type.
6. Parsing Name in the previous version was pain, as well as it wasn't clear that port might be specified. The original motivation was that if you create a cache, like HashMap<NameBuf, Address> it's not possible to Borrow<Name> from NameBuf (if NameBuf(String, u16) and Name(&str, u16)), and no clear design for this was foreseen. It turns out, that it's possible to use OwningRef for that, with not that big overhead.

More Enhancements To Do

1. The Router should be able to update the configuration and recheck each connected stream against new domain routing table. Presumably, this requires managing streams by router and connecting them through channel like described above.
2. We should add an abstraction that accepts a Stream<Item=Vec<Name>,_> rather than a Vec<Stream<Item=Address>> so that we can update the list of names connection pool does connect to, not just resolve already listed names (this allows better on-the-fly configuration reload in servers).

Questions

1. Associated errors? Probably it's too much pain.
2. Should name structure have public fields? Should NameBuf be implemented? Maybe Name/NameBuf should have Arc<String> because there is large chance that it will be moved between futures multiple times.

Tagging @srijs as you mentioned you have some thoughts. Also @seanmonstar for speaking about hyperium/hyper#1174

please publish ns-dns-tokio updates to crates.io

Hello,

It seems that the latest release of ns-dns-tokio published to crates.io is 0.4.0. When you have a moment, can you please publish 0.4.2?

Thank you!

Regression: IP addresses are accepted as DNS names

IIUC, this regression was caused by ce28aef.

A DNS name that contains only labels that only have digits is not a valid DNS name. For example, "1.2.3.4" is not a valid DNS name; it's an IP address. Previous versions rejected "1.2.3.4" correctly but also rejected "1.2.3.x" incorrectly. The commit mentioned above fixes the latter case, but broke the former case.

This is problematic for code that does something like this

1. Try to parse the host component of the URI as a Name. If that succeeds, return the Name.
2. Otherwise, try to parse the host component of the URI as an IP address. If that succeeds, return the IP address.

Inverting the logic so that parsing as an IP address is attempted first works around the issue. However, since DNS names are much more common in URLs than IP addresses, it would be preferable to get things working so that attempting parsing as a DNS name first would work the intended way.