search

Netflix / denominator

Portably control DNS clouds using java or bash
Apache License 2.0
580 stars 110 forks source link

WriteLiveTest- Case sensitive match for 'name' #361

Open sanjaympawar1 opened 9 years ago

sanjaympawar1 commented 9 years ago

WriteLiveTest passes for all existing providers, but doesn't for a draft implementation of Versign MDNS:

Resource record set comparision is failing for Verisign MDNS due to exact comparision in ResourceRecordSetAssert.hasName(). As per RFC-1035 (http://tools.ietf.org/html/rfc1035) section 2.3.1 page comparison should not be case sensitive.

AAAA record failing due to exact comparison of record data (for Verisign MDNS). As per RFC for ipv6 textual format http://tools.ietf.org/html/rfc5952#page-6. ipv6 addresses are case insensitive.

exact match of TXT rdata where Verisign MDNS is wrapping TXT rdata in double quotes. TXT record data comparison is performed as exact match. However some DNS system may wrap TXT data within double quotes. The TXT record data wrapping by quotes is prescribed in RFC-1035 (http://tools.ietf.org/html/rfc1035) section 3.3.14 and definition of "" at page #34

codefromthecrypt commented 9 years ago

thanks for the reports, Sanjay. I've consolidated the issues together so as to not spam watchers.

The meta-issue here is whether to address the RFC compliance in the rdata types or in the tests, and what to present to users who may not care about the "letter of the law". Changes might surprise users who are more interested in portability than growing their knowledge of DNS nuance.

Bear in mind that literally all current DNS providers pass tests as they exist today. Some of that was addressed by normalizing in the provider code. For example, we "unquote" txt in route53 https://github.com/Netflix/denominator/blob/master/route53/src/main/java/denominator/route53/ResourceRecordSetHandler.java#L69

I'm open to ideas for change, but I think that since the context is a draft provider, we should be on the same page with regards to the experience impact this might have. copying some other folks for opinion towards that: @jdamick @jonbodner @colmmacc @MisterK @everett-toews @jwbraucher

codefromthecrypt commented 9 years ago

@sanjaympawar1 I would change your parsers so that they pass the tests. Even if the spec says it is ok to change the case (ex. I write 'ABC', but the api stores 'abc'), it is odd that this happens. When you write your README, follow the example here: https://github.com/Netflix/denominator/blob/master/route53/README.md

This issue will linger until others feel strongly we should change our rdata types to follow RFC when doing equals etc. or a week has passed.