Add support for setting Proxy Protocol in exit server userdata

[welteki]

Description

Add a MakeExitServerUserdataWithProxyProto function that accepts a proxyProto parameter and sets the PROXY_PROTO environment variable in the exit server userdata script. The existing MakeExitServerUserdata function is preserved as a wrapper with an empty default.

Motivation and context

This is required to allow the inlets-operator to provision tunnel servers with Proxy Protocol enabled.

[reviewfn]

AI Pull Request Overview

Summary

• Adds support for Proxy Protocol in exit server userdata by introducing a new function MakeExitServerUserdataWithProxyProto.
• Preserves backward compatibility by keeping the existing MakeExitServerUserdata as a wrapper.
• Updates the generated bash script to export PROXY_PROTO environment variable.
• Modifies existing test to account for the new export line.
• Adds a new test case for the proxy protocol functionality.
• No changes to other parts of the codebase.

Approval rating (1-10)

8/10 - Solid implementation with good backward compatibility, but lacks input validation for proxyProto parameter.

Summary per file

Summary per file

File path	Summary
provision/userdata.go	Added new function for proxy proto support and updated wrapper (max 20 words)
provision/userdata_test.go	Updated existing test and added new test for proxy proto (max 20 words)

Overall Assessment

The PR successfully adds Proxy Protocol support to exit server userdata without introducing breaking changes. The implementation is backward compatible, well-tested, and maintains consistency with existing code patterns. However, the lack of input validation on the proxyProto parameter could pose a security risk if untrusted input is passed, potentially allowing script injection in the generated userdata.

Detailed Review

Detailed Review

provision/userdata.go

• The new MakeExitServerUserdataWithProxyProto function correctly accepts a proxyProto parameter and incorporates it into the generated script.
• The existing MakeExitServerUserdata function is properly preserved as a backward-compatible wrapper, calling the new function with an empty string for proxyProto.
• The script generation includes export PROXY_PROTO=" + proxyProto + ", which sets the environment variable as intended.
• No input validation is performed on proxyProto. If this parameter contains shell metacharacters (e.g., quotes, semicolons), it could lead to command injection in the generated bash script. Consider adding validation to restrict allowed values (e.g., only "v1", "v2", or empty string) or properly escaping the input.
• The function name is descriptive and follows Go conventions.
• No performance or efficiency concerns.

provision/userdata_test.go

• The existing test Test_makeUserdata_InletsPro has been correctly updated to include the new export PROXY_PROTO="" line in the expected output, reflecting the change in the wrapper function.
• A new test Test_makeUserdata_InletsPro_WithProxyProto has been added, covering the new functionality with proxyProto set to "v1".
• Test coverage is adequate for the changes, verifying both the backward-compatible behavior and the new feature.
• The tests use string comparison for the entire generated script, which is thorough but could be brittle if the script format changes. Consider extracting the PROXY_PROTO-specific assertions for better maintainability.

General Concerns

• Security Risk: The proxyProto parameter is directly interpolated into the bash script without sanitization. This could be exploited if the input comes from an untrusted source. Recommend implementing input validation or escaping.
• Impact on Migration: Existing callers of MakeExitServerUserdata will continue to work unchanged, as the function now sets PROXY_PROTO="". No migration steps are needed.
• Potential Regressions: The changes are isolated to the userdata generation and do not affect other parts of the codebase. Risk of regression is low.
• Consistency: The code follows existing patterns in the file, using string concatenation for script generation.
• Testing: Adequate test coverage is provided. Consider running the full test suite to ensure no indirect impacts.
• Efficiency/Performance: No issues; the changes are lightweight additions.
• No issues with vendor directory or external dependencies noted.

AI agent details.

Agent processing time: 31.26s
Environment preparation time: 5.397s
Total time from webhook: 39.314s

[welteki]

A version of MakeExitServerUserdata has been moved into the inelts-operator the remove the depencency on the cloud-provision package for this method.

[alexellis]

A version of MakeExitServerUserdata has been moved into the inelts-operator the remove the depencency on the cloud-provision package for this method.

It will still need the dependency. It just means it'll make its own userdata.

Does inletsctl still call into this method?

[welteki]

Does inletsctl still call into this method?

Yes this method is still used by inletsctl. Do wo also want to update that to make its own userdata so the method can be removed here entirely?

[alexellis]

The dependency cannot be removed entirely, but this userdata creation could be deleted and moved into both of the packages inline to simplify future changes.