Fix dns service SRV lookup when service address is a fqdn #6252
Conversation
|
|
There was a problem hiding this comment.
@tionebsalocin This looks like a good change overall to me. I definitely think that when the service address is a fqdn/hostname Consul should definitely return it as the SRV target.
I do have one request with regards to the Additional section and ensuring we keep those extra records in the response.
agent/dns.go
Outdated
| srvRec.Target = addr | ||
| } else { | ||
| srvRec.Target = fmt.Sprintf("%s.", addr) | ||
| } |
There was a problem hiding this comment.
I think we still need to append any other records besides the CNAME one. In some cases Consul will recursively resolve this CNAME and have the A/AAAA record in the additional section.
So adding something like the following would be good.
if len(records) > 1 {
resp.Extra = append(resp.Extra, records[1:]...)
}
|
@tionebsalocin good change, don't forget to sign the cla https://cla.hashicorp.com/hashicorp/consul?pullRequest=6252 |
Current implementation returns the node name instead of the service
address.
With this fix when querying for SRV record service address is return in
the SRV record.
And when performing a simple dns lookup it returns a CNAME to the
service address.
Note: Additionnal information has been removed because providing CNAME
does not make sens anymore. A perfect implementation should return A
record with the target IP of the service address.