pdns
pdns copied to clipboard
Auth: clean up cache purging
XXX: as noted in comments below, #11066 was merged. This PR also tried cleaning up the internal functions, and IMO should still be done. But a lot of code changed in the meantime and this needs redoing.
Short description
Some people want to add domains to the zonecache as soon as possible. This is currently automatic when the zone is created using the API or autoprimary. For database-based provisioning systems this allows the provisioning system to add the zone by calling the /cache/flush endpoint.
Checklist
I have:
- [x] read the CONTRIBUTING.md document
- [x] compiled this code
- [x] tested this code
- [ ] included documentation (including possible behaviour changes)
- [ ] documented the code
- [ ] added or modified regression test(s)
- [ ] added or modified unit test(s)
- [ ] checked that this code was merged to master
In our use case the flush would be a benefit:
- Customer adds zone
- Customer requests lets encrypt certificated
- Lets encrypt performs a DNS lookup
- The zone-list cache does not have the new zone in the cache -> REFUSED is returned and put in packet cache.
- Lets encrypt fails. Customer has to wait until packet cache expires.
With "flush newzone" the zone would be added to the zone list cache and old cache entries could be purged.
How is the flush working? will it flush from query-cache and packet-cache?
code lgtm. Small question: but would it make sense to have a separate endpoint for the zonecache specifically so the records are not purged from the caches?
I think that would just lead to two requests in sequence - I don't see a valid usecase for -not- clearing the packet and query caches really.
Any chance to have this in 4.5.2? It can be seen as a missing feature of the new zone-list cache. I promise to test it. Thanks
I do not see why you need this. The api and autoprimary both add the zone to the zone cache. Adding a zone a second time will not help you when the packet cache is the actual problem. Flushing the packetcache is already possible and this should be enough.
Autoprimary was fixed in #10432
The packet cache is not the actual problem. The problem is that zones added directly to the database (DB replication to public facing name servers) will not be seen immediately. Hence, I need a way to make the zone-list cache aware of the new zone (without completely reloading the list). I do not mind if this is done via the "flush" endpoint or a new endpoint. In my case I like the above solution as I would need to call "flush" anyways - but if necessary - i could also live with a second API call to add and flush.
I do not see why you need this. The api and autoprimary both add the zone to the zone cache.
As said in the original MR text: direct database inserts.
I'm no fan of the "put it in flush" approach. Flush and add-zone have very different use cases. A separate add-zone endpoint to support cases like this might be an option. But we must make it very clear in the docs you do not need this for zones which are created via the api.
I can split it into a new endpoint, but:
- naming it is hard. Certainly needs the “most people should not call this” explanation. Recommendations on the name?
- people who want to call this will also end up calling flush, to clear any old entries in packetcache
Therefore I am not sure it’s worth splitting really.
I have a question about the cache-flush: does it flush single domain names or all domain names belonging to a zone? For example, can I flush www.example.com? Or only example.com which will flush also www.example.com? In the first case, if someone flushes www.example.com I will not that www.example.com will be added to the zone-list cache.
Maybe an additional URL parameter to the flush endpoint can be used to add the zone to the zone-list cache.
I believe that "removing the virtual negative cache entry from zonecache" shall be thought of as "flushing the (zone)cache". I am inclined to put the code into purgeAuthCaches(Exact) instead of the API code though.
Would appreciate someone to step into this discussion to un-stuck it.
Postponed to 4.7, I asked @zeha to separately PR the original, smaller version of this, so that we release 4.6-beta1 without a lot of new changes.
#11066 targets 4.6.0.