Feat: Add Port Range by ENV vars#362
Conversation
|
This refers to #364 as the multiple PRs request 😄 |
scripts/package_microsandbox.sh
Outdated
| @@ -1,4 +1,4 @@ | |||
| #!/bin/sh | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
kindly revert all shebang-related changes.
There was a problem hiding this comment.
please revert this back to what it was.
microsandbox-utils/lib/env.rs
Outdated
| /// Environment variable for the minimum port in the sandbox port range | ||
| pub const MICROSANDBOX_PORT_MIN_ENV_VAR: &str = "MICROSANDBOX_PORT_MIN"; | ||
|
|
||
| /// Environment variable for the maximum port in the sandbox port range | ||
| pub const MICROSANDBOX_PORT_MAX_ENV_VAR: &str = "MICROSANDBOX_PORT_MAX"; | ||
|
|
There was a problem hiding this comment.
let's use just one env var, MSB_PORT_RANGE, and the format is: <lower:u16>..[=]<upper:u16>.
if the lower is absent, its safe to say the format is invalid. the = is optional
microsandbox-utils/lib/env.rs
Outdated
| /// Returns the port range for sandbox port allocation. | ||
| /// If both MICROSANDBOX_PORT_MIN and MICROSANDBOX_PORT_MAX are set, | ||
| /// returns Some((min, max)). Otherwise, returns None for dynamic allocation. | ||
| pub fn get_sandbox_port_range() -> Option<(u16, u16)> { |
microsandbox-cli/bin/msbserver.rs
Outdated
| config.get_port_range_min().as_ref().copied(), | ||
| config.get_port_range_max().as_ref().copied(), |
There was a problem hiding this comment.
per my other comment. if we used just one env, and returned a range, we can avoid this.
|
@toksdotdev All changes have been made based on your feedback; please let me know if any further changes are needed. |
scripts/package_microsandbox.sh
Outdated
| @@ -1,4 +1,4 @@ | |||
| #!/bin/sh | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
please revert this back to what it was.
microsandbox-utils/lib/env.rs
Outdated
| // Only upper exists (missing lower) should be invalid | ||
| assert_eq!(parse_sandbox_port_range("..2000"), None); | ||
| // Does not include '=' and lower < upper | ||
| assert_eq!(parse_sandbox_port_range("1000..2000"), Some(1000..=2000)); |
There was a problem hiding this comment.
this is wrong. 1000..2000 shouldn't include 2000.
…ock" This reverts commit 93cb923.
|
@toksdotdev Done! Please let me know if any other changes are needed 👌🏻 |
2 participants
This pull request introduces support for configuring a custom port range for sandbox port allocation in the microsandbox server, allowing administrators to control which ports are used for sandbox networking. It also standardizes the use of
bashin project scripts for improved compatibility and reliability. It is a complement to #349Sandbox Port Range Configuration:
MICROSANDBOX_PORT_MINandMICROSANDBOX_PORT_MAX, to allow specifying a minimum and maximum port for sandbox allocation. If these are set, the server will allocate ports only within this range; otherwise, it will fall back to dynamic OS allocation. (microsandbox-utils/lib/env.rs,microsandbox-server/lib/config.rs) [1] [2] [3] [4]PortManagerto support an optional port range, including logic to find available ports within the configured range and to fall back to OS allocation if none are available. (microsandbox-server/lib/port.rs) [1] [2] [3] [4]Script Standardization:
/bin/shto/usr/bin/env bashto ensure scripts are run withbash, which is required for some script features. (scripts/build_libkrun.sh,scripts/install_microsandbox.sh,scripts/package_microsandbox.sh,scripts/setup_env.sh) [1] [2] [3] [4]Makefileto explicitly invokebashwhen runningbuild_libkrun.shfor consistency with the new script requirements. (Makefile)