feat(dns): function to force no sync toip()

[locao]

Summary

This change adds new individual_toip() public function to the DNS client.

This function bypasses the noSynchronisation = false option, forcing individual resolve of the hostnames. It avoids race-conditions when multiple calls to the access() phase handler happen in the context of the same request.

Checklist

• The Pull Request has tests
• A changelog file has been created under changelog/unreleased/kong or skip-changelog label added on PR if changelog is unnecessary. README.md
• There is a user-facing docs PR against https://github.com/Kong/docs.konghq.com - PUT DOCS PR HERE

Issue reference

KAG-3795

[chobits]

hi @locao as I’m trying to implement a new DNS client library based on mlcache (see PR #12305), and in that implmentation, the DNS query operation was implemented in the mlcache L3 callback. It deprecates the dns_no_sync option and multiple DNS queries for the same name will always be synchronized (even across workers) because it ultilizes mlcache concurrent control mechanism.

So I have one concern that if we update kong to the new DNS library, we might not support individual_toip().

If the wasm module must rely on a singleton DNS library, I need to reconsider how to handle it in the new DNS library. (I currently haven't come up with a good method to implement individual queries on top of mlcache, as this would completely disrupt mlcache's optimal usage pattern and make the code more complex. 😿 )

[locao]

Hi @chobits. If the new DNS client library behaves exactly the same way for the wasm module as for the lua land, there's nothing to worry about.

As the current DNS client is not being removed at the moment, I think we should move forward with this change. Maybe we can change the individual_toip() name to something that makes sense to exist in the new DNS client library, or add a compatibility function in the new library.

What do you think?

[chobits]

Hi @chobits. If the new DNS client library behaves exactly the same way for the wasm module as for the lua land, there's nothing to worry about.

Yes, it behaves the same way for all the upper callers. I have take a thorough look at the KAG issue, and know that your concern is the race-condition of current DNS client library. But in new DNS client library we still have that race-condition caused by the internal use of resty-lock by mlcache. Could that work for wasm?

BTW, I know in that KAG issue, there is a semaphore lock race condition in the scenario of wasm using the DNS library. Could you explain why wasm could not tolerant the race condition if there is no dead-lock problem and semaphore lock seems not to cause CPU blocking. And I think it could help kong alleviate the pressure of queries on the DNS server.

As the current DNS client is not being removed at the moment, I think we should move forward with this change. Maybe we can change the individual_toip() name to something that makes sense to exist in the new DNS client library, or add a compatibility function in the new library.

It does not matter for the function name. I can provide a same function for wasm to be compatible with current DNS library.

What do you think?

[thibaultcha]

The old DNS library is problematic for the ngx_wasm_module Lua bridge because of its use of the semaphore Lua library, which the no_sync option disables. Using the lock library that mlcache relies on in the new DNS resolver should not cause any problem to the WasmX Lua bridge, so we should be good once we do the switch in the future...

[chobits]

As I discussed with @locao offline, the WASM module could be responsible for compatibility with both new and old libraries, like

-- In the wasm module:
local toip = dns_client.toip

-- For original dns client library, it will use its `individual_toip` function.
if dns_client.individual_toip then
  topip =  dns_client.individual_toip
end

[team-gateway-bot]

Successfully created cherry-pick PR for master:

• https://github.com/kong/kong-ee/pull/8617