Conversation
djc
requested review from
bluejekyll,
japaric,
justahero,
listochkin,
marcus0x62 and
pvdrz
as code owners
djc
force-pushed
the
error-api
branch
6 times, most recently
from
8478cc1 to
b35201c
Compare
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
cratelyn
added a commit
to linkerd/linkerd2-proxy
that referenced
this pull request
Mar 20, 2025
see also #3782. this commit addresses breaking changes in the v0.25.0 release of `hickory-resolver`, used by our `linkerd-dns` crate to handle DNS resolution. see the release notes, here: <https://github.com/hickory-dns/hickory-dns/releases/tag/v0.25.0> > 0.25.0 represents a large release for the Hickory DNS project. Over 14 > months since 0.24.0, we've [..] addressed a number of findings from our > first security audit. changes that are relevant to us include: > * Support for TLS using native-tls or OpenSSL has been removed. We now > only provide first-party support for rustls (0.23, for DNS over TLS, > HTTP/2, QUIC and HTTP/3). We support ring or aws-lc-rs for > cryptographic operations both for DNSSEC and TLS. The > dns-over-rustls,dns-over-native-tls, dns-over-openssl, > dns-over-https-rustls, dns-over-https, dns-over-quic and dns-over-h3 > features have been removed in favor of a set of > {tls,https,quic,h3}-{aws-lc-rs,ring} features across our library > crates. > > * The synchronous API in the resolver and client crates, which > previously provided a thin partial wrapper over the asynchronous > API, has been removed. Downstream users will have to migrate to the > asynchronous API. > > * Error types are now exposed directly in the crate roots. this commit updates references to the `hickory_resolver::error::ResolveError` error with `hickory_resolver::ResolveError` now that the errors submodule is private. (hickory-dns/hickory-dns#2530) this commit replaces references to `hickory_resolver::TokioAsyncResolver` with its new name, `hickory_resolver::TokioResolver`. (hickory-dns/hickory-dns#2521) this commit inspects "no records found" errors according to the new api. this particular change isn't especially documented, explicitly, but occurred in hickory-dns/hickory-dns#2094. see in particular, in that respect, corresponding changes in the upstream repo's own code. for example: https://github.com/hickory-dns/hickory-dns/pull/2094/files#diff-330847b46040a30d449f85e8a804bea085f0974d3cba80d79d83acc56f33542dL176-R178 ```diff - match error.kind() { - ResolveErrorKind::NoRecordsFound { query, soa, .. } => { + match error.proto().map(ProtoError::kind) { + Some(ProtoErrorKind::NoRecordsFound { query, soa, .. }) => { ``` X-Ref: hickory-dns/hickory-dns#2521 X-Ref: hickory-dns/hickory-dns#2830 X-Ref: hickory-dns/hickory-dns#2094 X-Ref: hickory-dns/hickory-dns#2877 Signed-off-by: katelyn martin <kate@buoyant.io>
cratelyn
added a commit
to linkerd/linkerd2-proxy
that referenced
this pull request
Mar 20, 2025
see also #3782. this commit addresses breaking changes in the v0.25.0 release of `hickory-resolver`, used by our `linkerd-dns` crate to handle DNS resolution. see the release notes, here: <https://github.com/hickory-dns/hickory-dns/releases/tag/v0.25.0> > 0.25.0 represents a large release for the Hickory DNS project. Over 14 > months since 0.24.0, we've [..] addressed a number of findings from our > first security audit. changes that are relevant to us include: > * Support for TLS using native-tls or OpenSSL has been removed. We now > only provide first-party support for rustls (0.23, for DNS over TLS, > HTTP/2, QUIC and HTTP/3). We support ring or aws-lc-rs for > cryptographic operations both for DNSSEC and TLS. The > dns-over-rustls,dns-over-native-tls, dns-over-openssl, > dns-over-https-rustls, dns-over-https, dns-over-quic and dns-over-h3 > features have been removed in favor of a set of > {tls,https,quic,h3}-{aws-lc-rs,ring} features across our library > crates. > > * The synchronous API in the resolver and client crates, which > previously provided a thin partial wrapper over the asynchronous > API, has been removed. Downstream users will have to migrate to the > asynchronous API. > > * Error types are now exposed directly in the crate roots. this commit updates references to the `hickory_resolver::error::ResolveError` error with `hickory_resolver::ResolveError` now that the errors submodule is private. (hickory-dns/hickory-dns#2530) this commit replaces references to `hickory_resolver::TokioAsyncResolver` with its new name, `hickory_resolver::TokioResolver`. (hickory-dns/hickory-dns#2521) this commit inspects "no records found" errors according to the new api. this particular change isn't especially documented, explicitly, but occurred in hickory-dns/hickory-dns#2094. see in particular, in that respect, corresponding changes in the upstream repo's own code. for example: https://github.com/hickory-dns/hickory-dns/pull/2094/files#diff-330847b46040a30d449f85e8a804bea085f0974d3cba80d79d83acc56f33542dL176-R178 ```diff - match error.kind() { - ResolveErrorKind::NoRecordsFound { query, soa, .. } => { + match error.proto().map(ProtoError::kind) { + Some(ProtoErrorKind::NoRecordsFound { query, soa, .. }) => { ``` there is a small pull request being proposed upstream to introduce a `Builder::with_options()` method, which would make our construction of a dns resolver marginally more idiomatic. this however, is not a blocker, by any means. X-Ref: hickory-dns/hickory-dns#2521 X-Ref: hickory-dns/hickory-dns#2830 X-Ref: hickory-dns/hickory-dns#2094 X-Ref: hickory-dns/hickory-dns#2877 Signed-off-by: katelyn martin <kate@buoyant.io>
cratelyn
added a commit
to linkerd/linkerd2-proxy
that referenced
this pull request
Mar 20, 2025
* build(deps): bump the hickory group with 2 updates Bumps the hickory group with 2 updates: [hickory-resolver](https://github.com/hickory-dns/hickory-dns) and [hickory-proto](https://github.com/hickory-dns/hickory-dns). Updates `hickory-resolver` from 0.24.4 to 0.25.1 - [Release notes](https://github.com/hickory-dns/hickory-dns/releases) - [Changelog](https://github.com/hickory-dns/hickory-dns/blob/main/OLD-CHANGELOG.md) - [Commits](hickory-dns/hickory-dns@v0.24.4...v0.25.1) Updates `hickory-proto` from 0.24.4 to 0.25.1 - [Release notes](https://github.com/hickory-dns/hickory-dns/releases) - [Changelog](https://github.com/hickory-dns/hickory-dns/blob/main/OLD-CHANGELOG.md) - [Commits](hickory-dns/hickory-dns@v0.24.4...v0.25.1) --- updated-dependencies: - dependency-name: hickory-resolver dependency-type: direct:production update-type: version-update:semver-minor dependency-group: hickory - dependency-name: hickory-proto dependency-type: indirect update-type: version-update:semver-minor dependency-group: hickory ... Signed-off-by: dependabot[bot] <support@github.com> * chore(dns): address breaking changes in `hickory-resolver` see also #3782. this commit addresses breaking changes in the v0.25.0 release of `hickory-resolver`, used by our `linkerd-dns` crate to handle DNS resolution. see the release notes, here: <https://github.com/hickory-dns/hickory-dns/releases/tag/v0.25.0> > 0.25.0 represents a large release for the Hickory DNS project. Over 14 > months since 0.24.0, we've [..] addressed a number of findings from our > first security audit. changes that are relevant to us include: > * Support for TLS using native-tls or OpenSSL has been removed. We now > only provide first-party support for rustls (0.23, for DNS over TLS, > HTTP/2, QUIC and HTTP/3). We support ring or aws-lc-rs for > cryptographic operations both for DNSSEC and TLS. The > dns-over-rustls,dns-over-native-tls, dns-over-openssl, > dns-over-https-rustls, dns-over-https, dns-over-quic and dns-over-h3 > features have been removed in favor of a set of > {tls,https,quic,h3}-{aws-lc-rs,ring} features across our library > crates. > > * The synchronous API in the resolver and client crates, which > previously provided a thin partial wrapper over the asynchronous > API, has been removed. Downstream users will have to migrate to the > asynchronous API. > > * Error types are now exposed directly in the crate roots. this commit updates references to the `hickory_resolver::error::ResolveError` error with `hickory_resolver::ResolveError` now that the errors submodule is private. (hickory-dns/hickory-dns#2530) this commit replaces references to `hickory_resolver::TokioAsyncResolver` with its new name, `hickory_resolver::TokioResolver`. (hickory-dns/hickory-dns#2521) this commit inspects "no records found" errors according to the new api. this particular change isn't especially documented, explicitly, but occurred in hickory-dns/hickory-dns#2094. see in particular, in that respect, corresponding changes in the upstream repo's own code. for example: https://github.com/hickory-dns/hickory-dns/pull/2094/files#diff-330847b46040a30d449f85e8a804bea085f0974d3cba80d79d83acc56f33542dL176-R178 ```diff - match error.kind() { - ResolveErrorKind::NoRecordsFound { query, soa, .. } => { + match error.proto().map(ProtoError::kind) { + Some(ProtoErrorKind::NoRecordsFound { query, soa, .. }) => { ``` there is a small pull request being proposed upstream to introduce a `Builder::with_options()` method, which would make our construction of a dns resolver marginally more idiomatic. this however, is not a blocker, by any means. X-Ref: hickory-dns/hickory-dns#2521 X-Ref: hickory-dns/hickory-dns#2830 X-Ref: hickory-dns/hickory-dns#2094 X-Ref: hickory-dns/hickory-dns#2877 Signed-off-by: katelyn martin <kate@buoyant.io> --------- Signed-off-by: dependabot[bot] <support@github.com> Signed-off-by: katelyn martin <kate@buoyant.io> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The error types are a fairly important part of the public API, so IMO documentation is better if these types are not "hidden" in an
errormodule but instead visible at the top level.Similarly, the use of
Resultaliases IMO does not provide much value and does have some costs (an extra indirection level in the documentation being one obvious cost). Keep them private.