Skip to content

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

Merged
moonming merged 6 commits into
apache:masterfrom
jiangfucheng:tencent-cls
Aug 17, 2023
Merged

fix(tencent-cloud-cls): DNS parsing failure#9843
moonming merged 6 commits into
apache:masterfrom
jiangfucheng:tencent-cls

Conversation

@jiangfucheng

@jiangfucheng jiangfucheng commented Jul 15, 2023

Copy link
Copy Markdown
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)

juststillthinking
juststillthinking reviewed Jul 17, 2023
View reviewed changes
Comment thread apisix/plugins/tencent-cloud-cls/cls-sdk.lua
local ip_list = {}
for _, v in ipairs(resolved.ip) do
insert_tab(ip_list, v)
if not resolved.ip then

@juststillthinking juststillthinking Jul 17, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about use https://github.com/apache/apisix/blob/master/apisix/core/resolver.lua#L61 to resolve domain?

@jiangfucheng jiangfucheng Jul 17, 2023

Copy link
Copy Markdown
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?

@juststillthinking juststillthinking Jul 18, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@jiangfucheng jiangfucheng Jul 18, 2023

Copy link
Copy Markdown
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.

@juststillthinking juststillthinking Jul 18, 2023

Copy link
Copy Markdown
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

@Sn0rt Sn0rt Jul 21, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jiangfucheng PTAL

@jiangfucheng jiangfucheng Jul 21, 2023

Copy link
Copy Markdown
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.

@Sn0rt Sn0rt Jul 31, 2023

Copy link
Copy Markdown
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 ?

@jiangfucheng jiangfucheng Jul 31, 2023

Copy link
Copy Markdown
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

Revolyssup commented Aug 7, 2023

Copy link
Copy Markdown
Contributor

@jiangfucheng Any updates?

juststillthinking
juststillthinking reviewed Aug 7, 2023
View reviewed changes
Comment thread apisix/plugins/tencent-cloud-cls/cls-sdk.lua Outdated
juststillthinking
juststillthinking reviewed Aug 7, 2023
View reviewed changes
Comment thread apisix/plugins/tencent-cloud-cls/cls-sdk.lua Outdated
@juststillthinking

juststillthinking commented Aug 13, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

@jiangfucheng

jiangfucheng commented Aug 14, 2023

Copy link
Copy Markdown
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
hongbinhsu pushed a commit to fitphp/apix that referenced this pull request Aug 22, 2023
Merge branch 'upstream/master' into github/master
aa6b2b5 
* 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

Reviewers

@moonming moonming moonming approved these changes
+2 more reviewers
@Sn0rt Sn0rt Sn0rt left review comments
@juststillthinking juststillthinking juststillthinking approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

5 participants

@jiangfucheng @Revolyssup @juststillthinking @Sn0rt @moonming