Skip to content
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.

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

fix(tencent-cloud-cls): DNS parsing failure #9843

Merged
merged 6 commits into from
Aug 17, 2023

Conversation

jiangfucheng
Copy link
Member

Description

Fixes #9842

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@@ -62,8 +62,15 @@ local params_cache = {
local function get_ip(hostname)
local _, resolved = socket.dns.toip(hostname)
local ip_list = {}
for _, v in ipairs(resolved.ip) do
insert_tab(ip_list, v)
if not resolved.ip then
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

The original code used socket.dns.topip, I didn't modify it. What is the difference between these two methods?

Copy link
Contributor

@monkeyDluffy6017 monkeyDluffy6017 Jul 18, 2023

Choose a reason for hiding this comment

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

You could check the logic between the two functions, we always use parse_domain, maybe it will resolve the problem of the socket.dns.toip occasionally returns nil

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems can't resolve this problem.

#   Failed test 't/plugin/tencent-cloud-cls.t TEST 10: log use log_format - pattern "[error]" should not match any line in error.log but matches line "2023/07/18 21:16:48 [error] 28051\#33045887: *50 [lua] resolver.lua:80: parse_domain(): failed to parse domain: jiangfuchengdeMacBook-Pro.local, error: failed to query the DNS server: dns server error: 3 name error, context: ngx.timer, client: 127.0.0.1, server: 0.0.0.0:1984" (req 0)

If we can't merge this temporary fix, I will investigate the reasons for the failure.

Copy link
Contributor

Choose a reason for hiding this comment

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

This pr is ok, but we need to know the reason

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

@Sn0rt Ok, give me some time to test it.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jiangfucheng do you have any update for this ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, there is currently no conclusion yet.

@Revolyssup
Copy link
Contributor

@jiangfucheng Any updates?

@monkeyDluffy6017
Copy link
Contributor

monkeyDluffy6017 commented Aug 13, 2023

Could you add a test case to cover this? You could hack the function socket.dns.toip

@jiangfucheng
Copy link
Member Author

Could you add a test case to cover this? You could hack the function socket.dns.toip

Added, thanks.

@moonming moonming merged commit 87cb2f8 into apache:master Aug 17, 2023
31 checks passed
hongbinhsu pushed a commit to fitphp/apix that referenced this pull request Aug 22, 2023
* upstream/master: (77 commits)
  docs: Update admin-api.md (apache#10056)
  ci: fix a bug that can not open nginx.pid (apache#10061)
  feat: remove rust dependency by rollback lua-resty-ldap on master (apache#9936)
  docs: fix grpc-transcode.md error (apache#10059)
  feat: upgrade lua dependencies (apache#10051)
  fix: rollback lua-resty-session to 3.10 (apache#10046)
  feat: upgrade resty-redis-cluster from  1.02-4->1.05-1 (apache#10041)
  feat: update lua library (apache#10037)
  fix: worker not exited when executing quit or reload command (apache#9909)
  fix: traffic split plugin not validating upstream_id (apache#10008)
  ci: update the timeout value in cli.yml (apache#10026)
  fix(tencent-cloud-cls): DNS parsing failure (apache#9843)
  chore(deps): bump actions/setup-node from 3.7.0 to 3.8.0 (apache#10025)
  feat(openid-connect): add proxy_opts attribute (apache#9948)
  perf(log-rotate): replace string.sub with string.byte (apache#9984)
  fix(ci): replace github action in update-labels.yml (apache#9987)
  fix: can't sync etcd data if key has special character (apache#9967)
  perf(aws-lambda): cache the index of the array (apache#9944)
  fix: add support for dependency installation on endeavouros (apache#9985)
  chore(ci): automate management of unresponded issues (apache#9927)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: dns parsing failure bug in tencent-cloud-cls plugin
5 participants