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 outstanding problems in claiming and add SOCKS5 support. #8406

Merged
merged 14 commits into from
Mar 17, 2020
Merged

Fix outstanding problems in claiming and add SOCKS5 support. #8406

merged 14 commits into from
Mar 17, 2020

Conversation

amoss
Copy link
Contributor

@amoss amoss commented Mar 16, 2020

Summary

Closes #8090
Closes #8263

Fix the problems in running the claiming script.

Merging from other PR (will integrete shortly).

some things in claiming script have to be fixed as well for this to be 100% (see #8263) - therefore this PR works and brings SOCKS5 proxy but there are error reporting issues that will be fixes in #8263.

If agent configured with SOCKS5 proxy [agent_cloud_link] key proxy it should pass that setting to curl when claiming script is called from within netdata agent with netdata -D -Wclaim.

Component Name

ACLK

Test Plan
  • Claiming against the local cloud env with a valid url.
  • Claiming against the local cloud env with an invalid url.
  • Claiming against the local cloud env with incorrect port.
  • Claiming against the local cloud env with nonexistent hostname.
  • The -verbose flag shows the JSON and raw curl command.
  • The '-insecure` flag works against a self-signed cert.
  • Claiming against the cloud test platform connects and rejects an invalid token.
  • Claiming against the cloud test platform connects and claims with a valid token.
  • Claiming against the cloud test platform with an incorrect port.
  • Claiming against the cloud test platform with an nonexistent hosthame.
  • Claiming against the cloud test platform with the wrong proxy port -> confusing error message.
  • Claiming against the cloud test platform with the wrong proxy hostname -> confusing error message.
  • Claiming against the cloud test platform with a proxy ip on an unreachable network -> confusing error message.
  • Claiming with https_proxy set in the environment.
  • Claiming using the -proxy= option on the script.
  • Claiming using -W claim "-url=....

Merging from other PR (will integrate shortly):

Make sure claiming script can be found by $PATH (e.g. if you test with NETDATA installed in custom dir)!

Test with local cloud test environment hidden behind socks5 proxy. (example: run agent on VM that doesn't have direct access to local cloud environment)

In config file of Netdata set options:

[agent_cloud_link]
proxy = socks5://X.X.X.X:YYYY
and for other test
proxy = none

When curl not present in system and wget present. Show error when trying to use SOCKS with wget (as wget doesn't have support).

Additional Information

@squash-labs
Copy link

squash-labs bot commented Mar 16, 2020

Manage this branch in Squash

Test this branch here: https://amossclaiming-0wxk7.squash.io

@amoss amoss requested review from mfundul and stelfrag and removed request for cosmix and ktsaou March 16, 2020 09:56
@underhood underhood changed the title [WIP] Claiming Adds SOCKS5 support to claiming from agent Mar 16, 2020
@underhood underhood changed the title Adds SOCKS5 support to claiming from agent [WIP] claiming Mar 16, 2020
mfundul
mfundul previously approved these changes Mar 16, 2020
@amoss amoss linked an issue Mar 16, 2020 that may be closed by this pull request
@amoss
Copy link
Contributor Author

amoss commented Mar 16, 2020

@mfundul I didn't see your comments before I merged in @underhood 's commits and Github won't show them to me now. Could you dig them out and stick them in a fresh comment or send them to me on Slack?

@amoss amoss changed the title [WIP] claiming [WIP] Fix outstanding problems in claiming and add SOCKS5 support. Mar 16, 2020
@mfundul
Copy link
Contributor

mfundul commented Mar 16, 2020

@mfundul I didn't see your comments before I merged in @underhood 's commits and Github won't show them to me now. Could you dig them out and stick them in a fresh comment or send them to me on Slack?

I had only approved the commits. No comments.

@amoss amoss linked an issue Mar 16, 2020 that may be closed by this pull request
2 tasks
@amoss amoss changed the title [WIP] Fix outstanding problems in claiming and add SOCKS5 support. Fix outstanding problems in claiming and add SOCKS5 support. Mar 16, 2020
@amoss amoss requested a review from mfundul March 16, 2020 18:19
claim/claim.c Outdated Show resolved Hide resolved
mfundul
mfundul previously approved these changes Mar 17, 2020
@amoss amoss requested a review from joelhans as a code owner March 17, 2020 08:30
@amoss amoss requested review from a user and mfundul and removed request for joelhans and a user March 17, 2020 08:30
@amoss amoss merged commit 9e32f03 into netdata:master Mar 17, 2020
@amoss amoss deleted the claiming branch March 17, 2020 12:59
Saruspete pushed a commit to Saruspete/netdata that referenced this pull request May 21, 2020
…#8406)

This commit fixes the known problems in claiming: incorrect reports of success, better treatment of error code and improved visibility of what the script is doing. There has been extensive testing against both environments to check that it works. The socks5 proxy support has been integrated and works for both methods of calling the claiming script.

Co-authored-by: Timotej Šiškovič <timotej@netdata.cloud>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix known issues with claiming ACLK Claiming should support Socks5 proxy
4 participants