require http and metrics for respective flags (#4674)

## Issue Addressed

following discussion on https://github.com/sigp/lighthouse/pull/4639#discussion_r1305183750 this PR makes the `http` and `metrics` sub-flags to require those main flags enabled
This commit is contained in:
João Oliveira 2023-09-22 02:33:10 +00:00
parent 2441a247ab
commit c5588eb66e
4 changed files with 100 additions and 74 deletions

View File

@ -1,4 +1,4 @@
use clap::{App, Arg};
use clap::{App, Arg, ArgGroup};
use strum::VariantNames;
use types::ProgressiveBalancesMode;
@ -355,22 +355,25 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.arg(
Arg::with_name("http-address")
.long("http-address")
.requires("enable_http")
.value_name("ADDRESS")
.help("Set the listen address for the RESTful HTTP API server.")
.default_value("127.0.0.1")
.default_value_if("enable_http", None, "127.0.0.1")
.takes_value(true),
)
.arg(
Arg::with_name("http-port")
.long("http-port")
.requires("enable_http")
.value_name("PORT")
.help("Set the listen TCP port for the RESTful HTTP API server.")
.default_value("5052")
.default_value_if("enable_http", None, "5052")
.takes_value(true),
)
.arg(
Arg::with_name("http-allow-origin")
.long("http-allow-origin")
.requires("enable_http")
.value_name("ORIGIN")
.help("Set the value of the Access-Control-Allow-Origin response HTTP header. \
Use * to allow any origin (not recommended in production). \
@ -381,11 +384,13 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.arg(
Arg::with_name("http-disable-legacy-spec")
.long("http-disable-legacy-spec")
.requires("enable_http")
.hidden(true)
)
.arg(
Arg::with_name("http-spec-fork")
.long("http-spec-fork")
.requires("enable_http")
.value_name("FORK")
.help("Serve the spec for a specific hard fork on /eth/v1/config/spec. It should \
not be necessary to set this flag.")
@ -403,6 +408,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.arg(
Arg::with_name("http-tls-cert")
.long("http-tls-cert")
.requires("enable_http")
.help("The path of the certificate to be used when serving the HTTP API server \
over TLS.")
.takes_value(true)
@ -410,6 +416,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.arg(
Arg::with_name("http-tls-key")
.long("http-tls-key")
.requires("enable_http")
.help("The path of the private key to be used when serving the HTTP API server \
over TLS. Must not be password-protected.")
.takes_value(true)
@ -417,6 +424,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.arg(
Arg::with_name("http-allow-sync-stalled")
.long("http-allow-sync-stalled")
.requires("enable_http")
.help("Forces the HTTP to indicate that the node is synced when sync is actually \
stalled. This is useful for very small testnets. TESTING ONLY. DO NOT USE ON \
MAINNET.")
@ -424,8 +432,9 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.arg(
Arg::with_name("http-sse-capacity-multiplier")
.long("http-sse-capacity-multiplier")
.requires("enable_http")
.takes_value(true)
.default_value("1")
.default_value_if("enable_http", None, "1")
.value_name("N")
.help("Multiplier to apply to the length of HTTP server-sent-event (SSE) channels. \
Increasing this value can prevent messages from being dropped.")
@ -433,8 +442,9 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.arg(
Arg::with_name("http-duplicate-block-status")
.long("http-duplicate-block-status")
.requires("enable_http")
.takes_value(true)
.default_value("202")
.default_value_if("enable_http", None, "202")
.value_name("STATUS_CODE")
.help("Status code to send when a block that is already known is POSTed to the \
HTTP API.")
@ -442,13 +452,14 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.arg(
Arg::with_name("http-enable-beacon-processor")
.long("http-enable-beacon-processor")
.requires("enable_http")
.value_name("BOOLEAN")
.help("The beacon processor is a scheduler which provides quality-of-service and \
DoS protection. When set to \"true\", HTTP API requests will be queued and scheduled \
alongside other tasks. When set to \"false\", HTTP API responses will be executed \
immediately.")
.takes_value(true)
.default_value("true")
.default_value_if("enable_http", None, "true")
)
/* Prometheus metrics HTTP server related arguments */
.arg(
@ -461,22 +472,25 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
Arg::with_name("metrics-address")
.long("metrics-address")
.value_name("ADDRESS")
.requires("metrics")
.help("Set the listen address for the Prometheus metrics HTTP server.")
.default_value("127.0.0.1")
.default_value_if("metrics", None, "127.0.0.1")
.takes_value(true),
)
.arg(
Arg::with_name("metrics-port")
.long("metrics-port")
.requires("metrics")
.value_name("PORT")
.help("Set the listen TCP port for the Prometheus metrics HTTP server.")
.default_value("5054")
.default_value_if("metrics", None, "5054")
.takes_value(true),
)
.arg(
Arg::with_name("metrics-allow-origin")
.long("metrics-allow-origin")
.value_name("ORIGIN")
.requires("metrics")
.help("Set the value of the Access-Control-Allow-Origin response HTTP header. \
Use * to allow any origin (not recommended in production). \
If no value is supplied, the CORS allowed origin is set to the listen \
@ -1259,4 +1273,5 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.default_value("64")
.takes_value(true)
)
.group(ArgGroup::with_name("enable_http").args(&["http", "gui", "staking"]))
}

View File

@ -94,70 +94,70 @@ pub fn get_config<E: EthSpec>(
* Http API server
*/
if cli_args.is_present("http") {
if cli_args.is_present("enable_http") {
client_config.http_api.enabled = true;
if let Some(address) = cli_args.value_of("http-address") {
client_config.http_api.listen_addr = address
.parse::<IpAddr>()
.map_err(|_| "http-address is not a valid IP address.")?;
}
if let Some(port) = cli_args.value_of("http-port") {
client_config.http_api.listen_port = port
.parse::<u16>()
.map_err(|_| "http-port is not a valid u16.")?;
}
if let Some(allow_origin) = cli_args.value_of("http-allow-origin") {
// Pre-validate the config value to give feedback to the user on node startup, instead of
// as late as when the first API response is produced.
hyper::header::HeaderValue::from_str(allow_origin)
.map_err(|_| "Invalid allow-origin value")?;
client_config.http_api.allow_origin = Some(allow_origin.to_string());
}
if cli_args.is_present("http-disable-legacy-spec") {
warn!(
log,
"The flag --http-disable-legacy-spec is deprecated and will be removed"
);
}
if let Some(fork_name) = clap_utils::parse_optional(cli_args, "http-spec-fork")? {
client_config.http_api.spec_fork_name = Some(fork_name);
}
if cli_args.is_present("http-enable-tls") {
client_config.http_api.tls_config = Some(TlsConfig {
cert: cli_args
.value_of("http-tls-cert")
.ok_or("--http-tls-cert was not provided.")?
.parse::<PathBuf>()
.map_err(|_| "http-tls-cert is not a valid path name.")?,
key: cli_args
.value_of("http-tls-key")
.ok_or("--http-tls-key was not provided.")?
.parse::<PathBuf>()
.map_err(|_| "http-tls-key is not a valid path name.")?,
});
}
if cli_args.is_present("http-allow-sync-stalled") {
client_config.http_api.allow_sync_stalled = true;
}
client_config.http_api.sse_capacity_multiplier =
parse_required(cli_args, "http-sse-capacity-multiplier")?;
client_config.http_api.enable_beacon_processor =
parse_required(cli_args, "http-enable-beacon-processor")?;
client_config.http_api.duplicate_block_status_code =
parse_required(cli_args, "http-duplicate-block-status")?;
}
if let Some(address) = cli_args.value_of("http-address") {
client_config.http_api.listen_addr = address
.parse::<IpAddr>()
.map_err(|_| "http-address is not a valid IP address.")?;
}
if let Some(port) = cli_args.value_of("http-port") {
client_config.http_api.listen_port = port
.parse::<u16>()
.map_err(|_| "http-port is not a valid u16.")?;
}
if let Some(allow_origin) = cli_args.value_of("http-allow-origin") {
// Pre-validate the config value to give feedback to the user on node startup, instead of
// as late as when the first API response is produced.
hyper::header::HeaderValue::from_str(allow_origin)
.map_err(|_| "Invalid allow-origin value")?;
client_config.http_api.allow_origin = Some(allow_origin.to_string());
}
if cli_args.is_present("http-disable-legacy-spec") {
warn!(
log,
"The flag --http-disable-legacy-spec is deprecated and will be removed"
);
}
if let Some(fork_name) = clap_utils::parse_optional(cli_args, "http-spec-fork")? {
client_config.http_api.spec_fork_name = Some(fork_name);
}
if cli_args.is_present("http-enable-tls") {
client_config.http_api.tls_config = Some(TlsConfig {
cert: cli_args
.value_of("http-tls-cert")
.ok_or("--http-tls-cert was not provided.")?
.parse::<PathBuf>()
.map_err(|_| "http-tls-cert is not a valid path name.")?,
key: cli_args
.value_of("http-tls-key")
.ok_or("--http-tls-key was not provided.")?
.parse::<PathBuf>()
.map_err(|_| "http-tls-key is not a valid path name.")?,
});
}
if cli_args.is_present("http-allow-sync-stalled") {
client_config.http_api.allow_sync_stalled = true;
}
client_config.http_api.sse_capacity_multiplier =
parse_required(cli_args, "http-sse-capacity-multiplier")?;
client_config.http_api.enable_beacon_processor =
parse_required(cli_args, "http-enable-beacon-processor")?;
client_config.http_api.duplicate_block_status_code =
parse_required(cli_args, "http-duplicate-block-status")?;
if let Some(cache_size) = clap_utils::parse_optional(cli_args, "shuffling-cache-size")? {
client_config.chain.shuffling_cache_size = cache_size;
}

View File

@ -1466,6 +1466,7 @@ fn http_flag() {
fn http_address_flag() {
let addr = "127.0.0.99".parse::<IpAddr>().unwrap();
CommandLineTest::new()
.flag("http", None)
.flag("http-address", Some("127.0.0.99"))
.run_with_zero_port()
.with_config(|config| assert_eq!(config.http_api.listen_addr, addr));
@ -1474,6 +1475,7 @@ fn http_address_flag() {
fn http_address_ipv6_flag() {
let addr = "::1".parse::<IpAddr>().unwrap();
CommandLineTest::new()
.flag("http", None)
.flag("http-address", Some("::1"))
.run_with_zero_port()
.with_config(|config| assert_eq!(config.http_api.listen_addr, addr));
@ -1483,6 +1485,7 @@ fn http_port_flag() {
let port1 = unused_tcp4_port().expect("Unable to find unused port.");
let port2 = unused_tcp4_port().expect("Unable to find unused port.");
CommandLineTest::new()
.flag("http", None)
.flag("http-port", Some(port1.to_string().as_str()))
.flag("port", Some(port2.to_string().as_str()))
.run()
@ -2446,6 +2449,7 @@ fn http_sse_capacity_multiplier_default() {
#[test]
fn http_sse_capacity_multiplier_override() {
CommandLineTest::new()
.flag("http", None)
.flag("http-sse-capacity-multiplier", Some("10"))
.run_with_zero_port()
.with_config(|config| assert_eq!(config.http_api.sse_capacity_multiplier, 10));
@ -2463,6 +2467,7 @@ fn http_duplicate_block_status_default() {
#[test]
fn http_duplicate_block_status_override() {
CommandLineTest::new()
.flag("http", None)
.flag("http-duplicate-block-status", Some("301"))
.run_with_zero_port()
.with_config(|config| {

View File

@ -170,6 +170,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.arg(
Arg::with_name("http-address")
.long("http-address")
.requires("http")
.value_name("ADDRESS")
.help("Set the address for the HTTP address. The HTTP server is not encrypted \
and therefore it is unsafe to publish on a public network. When this \
@ -189,14 +190,16 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.arg(
Arg::with_name("http-port")
.long("http-port")
.requires("http")
.value_name("PORT")
.help("Set the listen TCP port for the RESTful HTTP API server.")
.default_value("5062")
.default_value_if("http", None, "5062")
.takes_value(true),
)
.arg(
Arg::with_name("http-allow-origin")
.long("http-allow-origin")
.requires("http")
.value_name("ORIGIN")
.help("Set the value of the Access-Control-Allow-Origin response HTTP header. \
Use * to allow any origin (not recommended in production). \
@ -207,21 +210,21 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.arg(
Arg::with_name("http-allow-keystore-export")
.long("http-allow-keystore-export")
.requires("http")
.help("If present, allow access to the DELETE /lighthouse/keystores HTTP \
API method, which allows exporting keystores and passwords to HTTP API \
consumers who have access to the API token. This method is useful for \
exporting validators, however it should be used with caution since it \
exposes private key data to authorized users.")
.required(false)
.takes_value(false),
)
.arg(
Arg::with_name("http-store-passwords-in-secrets-dir")
.long("http-store-passwords-in-secrets-dir")
.requires("http")
.help("If present, any validators created via the HTTP will have keystore \
passwords stored in the secrets-dir rather than the validator \
definitions file.")
.required(false)
.takes_value(false),
)
/* Prometheus metrics HTTP server related arguments */
@ -234,22 +237,25 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
.arg(
Arg::with_name("metrics-address")
.long("metrics-address")
.requires("metrics")
.value_name("ADDRESS")
.help("Set the listen address for the Prometheus metrics HTTP server.")
.default_value("127.0.0.1")
.default_value_if("metrics", None, "127.0.0.1")
.takes_value(true),
)
.arg(
Arg::with_name("metrics-port")
.long("metrics-port")
.requires("metrics")
.value_name("PORT")
.help("Set the listen TCP port for the Prometheus metrics HTTP server.")
.default_value("5064")
.default_value_if("metrics", None, "5064")
.takes_value(true),
)
.arg(
Arg::with_name("metrics-allow-origin")
.long("metrics-allow-origin")
.requires("metrics")
.value_name("ORIGIN")
.help("Set the value of the Access-Control-Allow-Origin response HTTP header. \
Use * to allow any origin (not recommended in production). \