Create and own userdata within inletsctl

[alexellis]

Description

Create and own userdata within inletsctl

How Has This Been Tested?

HTTPS userdata was already owned inside inletsctl, but not for TCP tunnels. This has now been pulled up from cloudprovision for easier maintenance.

Unit tests updated and pass. Converted from string concat to string interpolation with fmt.Sprintf.

How are existing users impacted? What migration steps/scripts do we need?

No expected impact

[reviewfn]

AI Pull Request Overview

Summary

• Refactored userdata generation functions from cloudprovision library to local inletsctl code
• Converted string concatenation to fmt.Sprintf for safer interpolation
• Added VERSION environment variable export in userdata scripts
• Corrected systemd service file for TCP tunnels (inlets-pro.service instead of inlets-pro-http.service)
• Updated unit tests to match new function signatures and expected outputs

Approval rating (1-10)

8 - Solid refactoring that improves maintainability and code safety, with no breaking changes.

Summary per file

Summary per file

File path	Summary
cmd/create.go	Refactored makeHTTPSUserdata and added makeExitServerUserdata functions using fmt.Sprintf instead of string concatenation, added VERSION export in userdata scripts.
cmd/create_test.go	Added new TCP userdata test, updated existing HTTPS tests to use new function names and expected outputs with VERSION variable.

Overall Assessment

The PR successfully pulls userdata generation functionality from the external cloudprovision library into inletsctl, making the codebase more self-contained and easier to maintain. The refactoring to use fmt.Sprintf improves code safety by reducing potential injection risks. Tests have been appropriately updated and should pass.

Detailed Review

Detailed Review

cmd/create.go

• Function renaming and visibility: Changing MakeHTTPSUserdata to makeHTTPSUserdata follows Go naming conventions for unexported functions. This is appropriate since the function is now internal to the package.
• String interpolation refactoring: Replacing string concatenation with fmt.Sprintf is a security and maintainability improvement. The old approach risked injection issues if inputs contained special characters, though in this context the risk was minimal.
• VERSION environment variable: Adding export VERSION="%s" and using $VERSION in curl URLs allows for runtime version specification in the userdata scripts. This is more flexible than hardcoding versions.
• TCP userdata function: Moving provision.MakeExitServerUserdata to a local makeExitServerUserdata function maintains consistency and reduces external dependencies.
• Service file correction: For TCP tunnels, using inlets-pro.service instead of inlets-pro-http.service is correct, as TCP mode doesn't require HTTP-specific configuration.
• Domain flags handling: The strings.TrimSpace(domainFlags) ensures no trailing spaces in the DOMAINS variable, which is good.

cmd/create_test.go

• New TCP test: Adding Test_MakeTCPUserdata_OneTunnel provides coverage for the new TCP userdata function. The test name should be updated to match the function name (Test_makeExitServerUserdata_OneTunnel) for consistency.
• Test updates: Updating existing tests to use makeHTTPSUserdata and expect the new output format with VERSION export is appropriate.
• Test file writes: Writing test outputs to /tmp/tcp.txt and /tmp/t.txt is unusual for unit tests. Consider removing these debug writes or using t.TempDir() for proper temporary files.
• Test assertions: The tests correctly verify exact string matches, which is necessary for userdata script validation.

Potential Concerns

• Dependency reduction: While reducing dependency on cloudprovision is positive, ensure that any shared logic between inletsctl and cloudprovision remains synchronized if cloudprovision is still used elsewhere.
• Version handling: The VERSION export assumes the version string is safe for shell interpolation. If versions could contain special characters, additional sanitization might be needed.
• Error handling: The curl commands use -SLsf which will fail silently on errors. Consider if more robust error handling is needed in the userdata scripts.
• Backwards compatibility: Although the PR states no migration needed, confirm that the generated userdata produces identical behavior to the previous version.

Recommendations

• Update test function name to Test_makeExitServerUserdata_OneTunnel for naming consistency.
• Remove debug file writes from tests or use proper temporary directories.
• Consider adding integration tests to verify the generated userdata scripts execute correctly in a real environment.
• Review if the cloudprovision library still needs the old MakeExitServerUserdata function or if it can be deprecated.

AI agent details.

Agent processing time: 29.921s
Environment preparation time: 6.484s
Total time from webhook: 44.068s