-
Notifications
You must be signed in to change notification settings - Fork 2k
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
New plugin "reverse lookup cache" #6459
base: master
Are you sure you want to change the base?
Conversation
As a start, maybe we should consider making this plugin an external plugin? |
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.
I'm wondering why this isn't part of the regular cache
plugin?
FYI, |
The PTR is generated synthetically. So for the original cache plugin there is actually nothing to cache. |
We chose |
b87e9fb
to
67aacd6
Compare
This sounds useful to me, and IMO the use case is compelling enough for consideration as an internal plugin. One possible issue is that the answers this plugin produces will be slightly different than "real" answers, and which one we respond with will depend on order and timing of prior lookups. This will need to be addressed in the README. A potential problematic permutation being that if the upstream reverse lookup is a negative answer with a long TTL, it could effectively block the response that this plugin produces until the TTL expires. Other less problematic differences could include the number of records with the PTR name (e.g. as a result of n:n PTR:A relationships). |
Another issue: This doesn't work with multiple instance of CoreDNS, since they each cache responses independently. From a client perspective this would result in flip-flopping of answers between real answers and the synthesized ones, depending on which instance handles the PTR request. |
This is only true, if the |
It's was indirect before, but with this proposed change, a direct dependency. |
The possible inconsistency is a very good point. I´ve updated the readme. The 1:n problem is partly tackled by sorting the results by last "active" query. But this is only usable for clients beeing actually aware of this sorting. The basic assumption is, that most clients will simply select the first returned answer an thus the most probable one. |
Original cache plugin caches any non-error responses. That a record is synthesized would not matter. Order of plugins matters here though - in directives you've put it before cache plugin, in which case it would not get cached. In plugin.cfg though, you've put it after cache, in which case the responses it synthesizes would be cached. (this needs to be made consistent BTW, so that the order does not change when someone compiles in new external plugins) |
Signed-off-by: Carsten Zeumer <carsten.zeumer@autonubil.de>
b9128d9
to
45d2700
Compare
45d2700
to
a15bc16
Compare
Signed-off-by: Carsten Zeumer <carsten.zeumer@autonubil.de>
a15bc16
to
5655e6c
Compare
Looking back on this, there are several of caveats (most are unavoidable).
These make the behavior somewhat inconsistent. For that reason I'm leaning toward making this an external plugin. |
74cd767
to
e028810
Compare
1. Why is this pull request needed and what does it do?
It adds a new plugin that allows IP addresses returned by name lookups (A/AAA) to be cached and used as results for later reverse DNS requests (PTR).
It is useful in scenarios where monitoring or security solutions need to correctly resolve the target DNS name of an IP address. Since many services hosted on content delivery networks or in the cloud do not have matching PTR records, this plugin can greatly improve the information value of IP addresses used in a certain context. For example, the
console.aws.amazon.com
's IP resolves toa751973eac2731385.awsglobalaccelerator.com
in a reverse lookup, and the IP addresses oflogin.microsoft.com
does not resolve at all.The plugin uses
groupcache
in a clustered environment. On a Kubernetes cluster, the cache uses service discovery.2. Which issues (if any) are related?
3. Which documentation changes (if any) need to be made?
The plugin could be added to the list of plugins available.
4. Does this introduce a backward incompatible change or deprecation?
no