Skip to content

Fix DNS not supporting wildcard matching #13158

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.

Sign up for GitHub

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

Jump to bottom
Merged
giograno merged 5 commits into from
Sep 24, 2025
Merged

Fix DNS not supporting wildcard matching #13158

giograno merged 5 commits into from
Sep 24, 2025

Conversation

@skyrpex
Copy link
Contributor

@skyrpex skyrpex commented Sep 17, 2025

Motivation

The LocalStack DNS service doesn't support wildcards in aliases. In such cases, the DNS will not resolve the query to the record it points to.

For example, given the following DNS configuration:

domain          record    address
my-domain.io    A         1.1.1.1
*.my-domain.io  A         alias(my-domain.io)

See the following pseudo-output:

$ dig @127.0.0.1 my-domain.io
[...]
;; ANSWER SECTION:
my-domain.io. 300	IN	A	127.0.0.1
[...]

$ dig @127.0.0.1 subdomain.my-domain.io
[...]
;; ANSWER SECTION:
(not found)
[...]

After the fix, the output is as follows:

$ dig @127.0.0.1 my-domain.io
[...]
;; ANSWER SECTION:
my-domain.io. 300	IN	A	127.0.0.1
[...]

$ dig @127.0.0.1 subdomain.my-domain.io
[...]
;; ANSWER SECTION:
subdomain.my-domain.io. 300	IN	A	127.0.0.1
[...]

Changes

It refactors the alias resolution from raw string comparison in favor of a pattern matching comparison by reusing the DNSLabel.matchWildcard method.

@localstack-bot
Copy link
Contributor

localstack-bot commented Sep 17, 2025 •
edited
Loading

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

localstack-bot
localstack-bot reviewed Sep 17, 2025
View reviewed changes
Copy link
Contributor

@localstack-bot localstack-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome to LocalStack! Thanks for raising your first Pull Request and landing in your contributions. Our team will reach out with any reviews or feedbacks that we have shortly. We recommend joining our Slack Community and share your PR on the #community channel to share your contributions with us. Please make sure you are following our contributing guidelines and our Code of Conduct.

@github-actions
Copy link

github-actions bot commented Sep 17, 2025 •
edited
Loading

Test Results - Preflight, Unit

22 161 tests  +1   20 423 ✅ +1   6m 33s ⏱️ +2s
     1 suites ±0    1 738 💤 ±0 
     1 files   ±0        0 ❌ ±0 

Results for commit 17122ad. ± Comparison against base commit c33a935.

♻️ This comment has been updated with latest results.

@skyrpex skyrpex marked this pull request as ready for review September 17, 2025 15:15
@github-actions
Copy link

github-actions bot commented Sep 17, 2025

Test Results (amd64) - Acceptance

7 tests  ±0   5 ✅ ±0   3m 18s ⏱️ +2s
1 suites ±0   2 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 17122ad. ± Comparison against base commit c33a935.

@github-actions
Copy link

github-actions bot commented Sep 17, 2025

Test Results (amd64) - Integration, Bootstrap

    5 files      5 suites   2h 41m 45s ⏱️
5 046 tests 4 555 ✅ 491 💤 0 ❌
5 052 runs  4 555 ✅ 497 💤 0 ❌

Results for commit 17122ad.

@github-actions
Copy link

github-actions bot commented Sep 17, 2025

LocalStack Community integration with Pro

    2 files  ±0      2 suites  ±0   2h 0m 57s ⏱️ -21s
4 672 tests ±0  4 341 ✅ ±0  331 💤 ±0  0 ❌ ±0 
4 674 runs  ±0  4 341 ✅ ±0  333 💤 ±0  0 ❌ ±0 

Results for commit 17122ad. ± Comparison against base commit c33a935.

@skyrpex
Copy link
Contributor Author

skyrpex commented Sep 17, 2025

I have read the CLA Document and I hereby sign the CLA

localstack-bot added a commit that referenced this pull request Sep 17, 2025
@localstack-bot
@skyrpex has signed the CLA in #13158
bed5408
@skyrpex skyrpex added semver: patch Non-breaking changes which can be included in patch releases docs: skip Pull request does not require documentation changes labels Sep 17, 2025
dfangl
dfangl approved these changes Sep 22, 2025
View reviewed changes
Copy link
Member

@dfangl dfangl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Nice and clean implementation and an added test!

simonrw
simonrw approved these changes Sep 23, 2025
View reviewed changes
Copy link
Contributor

@simonrw simonrw left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Daniel has already approved, and the implementation looks good, thanks!

TIL that the matchWildcard method exists, which gives me more confidence that the implementation is correct 👍

@giograno giograno merged commit 6563601 into main Sep 24, 2025
44 of 48 checks passed
@giograno giograno deleted the fix-dns-not-supporting-wildcard-matching branch September 24, 2025 15:30
@alexrashed alexrashed added the notes: skip Pull request does not have to be mentioned in the release notes label Sep 24, 2025
@alexrashed alexrashed added this to the 4.9 milestone Sep 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@localstack-bot localstack-bot localstack-bot left review comments

@simonrw simonrw simonrw approved these changes

@dfangl dfangl dfangl approved these changes

Assignees

No one assigned

Labels

docs: skip Pull request does not require documentation changes notes: skip Pull request does not have to be mentioned in the release notes semver: patch Non-breaking changes which can be included in patch releases

Projects

None yet

Milestone

4.9

Development

Successfully merging this pull request may close these issues.

7 participants

@skyrpex @localstack-bot @simonrw @dfangl @alexrashed @giograno