hubble: Trim FQDN trailing dots in GetNames#12366
Conversation
|
test-focus K8sPolicy* |
Hubble v0.5 used to display FQDN names without a trailing dot for
absolute paths, i.e. `cilium.io` instead of `cilium.io.`.
We accidentally changed this in Hubble v0.6 when we started accessing
the Cilium DNS cache directly. This in in turn broke filtering on FQDNs
(--{from-,to-,}fqdn), as the filtering logic does not assume trailing
dots, same as Hubble UI, the CLI and metrics.
This commit restores the old behavior of stripping trailing dots from
the source and destination name.
Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
8a5d382 to
7900024
Compare
|
test-focus K8sPolicy* |
This performs an additional `hubble observe` invocation when testing DNS-based policies. We expect Hubble to annotate the `destination_names` field of the test flows with the observed DNS name exactly as stated in the test. Signed-off-by: Sebastian Wicki <sebastian@isovalent.com>
7900024 to
0346ee7
Compare
|
test-me-please |
| // GetNamesOf fetches FQDNs of a given IP from the perspective of | ||
| // the endpoint with ID sourceEpID | ||
| // the endpoint with ID sourceEpID. The returned names must not have | ||
| // trailing dots. |
There was a problem hiding this comment.
even though the reason is included in the commit, commits are volatile. I think the same reason should be included here since devs are more likely to find this comment than the commit message.
There was a problem hiding this comment.
on a side note: if this is a requirement, shouldn't a consumer of DNSGetter strip the dot so there is no new getters added without this requirement?
There was a problem hiding this comment.
I can extend the comment in a follow-up PR. I would like to avoid to restart CI just for this.
on a side note: if this is a requirement, shouldn't a consumer of DNSGetter strip the dot so there is no new getters added without this requirement?
It's part of the interface implementation contract to not return trailing dots. Having to modify each and every call site to clean up the result because it was violating the implementation contract seems weird to me.
Hubble v0.5 used to display FQDN names without a trailing dot for
absolute paths, i.e.
cilium.ioinstead ofcilium.io.We accidentally changed this in Hubble v0.6 when we started accessing
the Cilium DNS cache directly. This in in turn broke filtering on FQDNs
(
--{from-,to-,}fqdn), as the filtering logic does not work with trailingdots. Output in Hubble UI, Hubble CLI and metrics also are incompatible
with v0.5.
This PR restores the old behavior of stripping trailing dots from
the source and destination name via
DNSGetter. A test is added to CIto ensure we don't regress again.