Skip to content

dns_resolver: harden Hickory config creation and resolve null-FFI paths#45106

Open
agrawroh wants to merge 1 commit into
envoyproxy:mainfrom
agrawroh:hick-3
Open

dns_resolver: harden Hickory config creation and resolve null-FFI paths#45106
agrawroh wants to merge 1 commit into
envoyproxy:mainfrom
agrawroh:hick-3

Conversation

@agrawroh
Copy link
Copy Markdown
Member

Description

This PR converts several fail-stop paths in the Hickory DNS resolver shell into observable, recoverable failures.


Commit Message: dns_resolver: harden Hickory config creation and resolve null-FFI paths
Additional Description: Converts several fail-stop paths in the Hickory DNS resolver shell into observable, recoverable failures.
Risk Level: Low
Testing: Added Tests
Docs Changes: Added
Release Notes: Added

Replaces process-aborting ``RELEASE_ASSERT`` calls in
``HickoryDnsResolverConfig::create()`` with ``absl::StatusOr`` returns so
operators see configuration errors as factory failures instead of crashes:

- Dynamic module load failure.
- ABI symbol resolution failure.
- ``HickoryDnsResolverConfig`` JSON serialization failure.
- Rust-side rejection of the configuration JSON.

The ``HickoryDnsResolverConfig`` destructor now null-guards both fields it
consults, so unwinding a partially-constructed instance from a failed
``create()`` no longer crashes by calling through a null destroy FFI or
passing null into the Rust SDK's destroy macro.

The per-query ``on_dns_resolve`` FFI returning null (Rust panic or
``DnsResolverInstance::resolve`` returning ``None``) now invokes the user
callback synchronously with ``Failure``, increments ``resolve_total`` and
``get_addr_failure`` to mirror the asynchronous completion accounting, and
returns a null query handle. Previously the path was guarded by a debug-only
``ASSERT`` that was stripped in production builds.

The ``HickoryDnsResolver`` constructor changes the same-shaped ``ASSERT``
on ``resolver_module_ptr_`` to ``RELEASE_ASSERT`` so production builds also
fail loudly on a Rust panic during resolver creation, rather than continuing
into undefined behavior.

Adds tests for the new synchronous-failure metric accounting and the
destructor null-guard.

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants