-
Notifications
You must be signed in to change notification settings - Fork 339
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
fix(dns) allow resolution of service names with "." #1448
Conversation
This patch makes DNS resolution more restrictive so it's potentially not backward compatible depending on your treatment of undefined behavior as breaking changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the tests as suggested. Looks great otherwise.
// given | ||
dnsResolver.SetVIPs(vips.List{ | ||
"my.service": "240.0.0.1", | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do this in the It
let's remove it from BeforeEach
and put it in each It
.
When specifying a `kuma.io/service` with a "." DNS resolution would not work because it would only look at the first part of the DNS name. This would also cause `foo.bar.mesh` and `foo.mesh` to resolve to the same thing which may give a false sense of isolation of meshes in the DNS We now only strips the last part of the DNS name if it matches the `.mesh` domain suffix. Signed-off-by: Charly Molter <charly@koyeb.com>
b45000b
to
11be298
Compare
@nickolaev updated thanks! |
@nickolaev seems like the build failure is unrelated can you retrigger the build? |
When specifying a `kuma.io/service` with a "." DNS resolution would not work because it would only look at the first part of the DNS name. This would also cause `foo.bar.mesh` and `foo.mesh` to resolve to the same thing which may give a false sense of isolation of meshes in the DNS We now only strips the last part of the DNS name if it matches the `.mesh` domain suffix. Signed-off-by: Charly Molter <charly@koyeb.com> (cherry picked from commit de170f1)
When specifying a `kuma.io/service` with a "." DNS resolution would not work because it would only look at the first part of the DNS name. This would also cause `foo.bar.mesh` and `foo.mesh` to resolve to the same thing which may give a false sense of isolation of meshes in the DNS We now only strips the last part of the DNS name if it matches the `.mesh` domain suffix. Signed-off-by: Charly Molter <charly@koyeb.com> (cherry picked from commit de170f1) Co-authored-by: Charly Molter <charly@koyeb.com>
When specifying a
kuma.io/service
with a "." DNS resolution would not workbecause it would only look at the first part of the DNS name.
This would also cause
foo.bar.mesh
andfoo.mesh
to resolve to the same thingwhich may give a false sense of isolation of meshes in the DNS
We now only strips the last part of the DNS name if it matches the
.mesh
domain suffix.