From ea51c17b647bc883aec167a0ef1c6b54ca9ac84e Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Wed, 17 Mar 2021 08:22:03 +0000 Subject: [PATCH 1/2] agent: Allow server address to be specified on kernel command-line To make debugging and testing easier, allow the ttRPC server address to be specified via `/proc/cmdline` as `agent.server_addr=`. Fixes: #1516. Signed-off-by: James O. D. Hunt --- src/agent/src/config.rs | 171 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 171 insertions(+) diff --git a/src/agent/src/config.rs b/src/agent/src/config.rs index 99822e459f..dd14e533e8 100644 --- a/src/agent/src/config.rs +++ b/src/agent/src/config.rs @@ -10,6 +10,7 @@ use std::time; const DEBUG_CONSOLE_FLAG: &str = "agent.debug_console"; const DEV_MODE_FLAG: &str = "agent.devmode"; const LOG_LEVEL_OPTION: &str = "agent.log"; +const SERVER_ADDR_OPTION: &str = "agent.server_addr"; const HOTPLUG_TIMOUT_OPTION: &str = "agent.hotplug_timeout"; const DEBUG_CONSOLE_VPORT_OPTION: &str = "agent.debug_console_vport"; const LOG_VPORT_OPTION: &str = "agent.log_vport"; @@ -94,6 +95,12 @@ impl AgentConfig { // parse cmdline options parse_cmdline_param!(param, LOG_LEVEL_OPTION, self.log_level, get_log_level); + parse_cmdline_param!( + param, + SERVER_ADDR_OPTION, + self.server_addr, + get_string_value + ); // ensure the timeout is a positive value parse_cmdline_param!( @@ -234,6 +241,34 @@ fn get_bool_value(param: &str) -> Result { }) } +// Return the value from a "name=value" string. +// +// Note: +// +// - A name *and* a value is required. +// - A value can contain any number of equal signs. +// - We could/should maybe check if the name is pure whitespace +// since this is considered to be invalid. +fn get_string_value(param: &str) -> Result { + let fields: Vec<&str> = param.split('=').collect(); + + if fields.len() < 2 { + return Err(anyhow!("expected name=value")); + } + + // We need name (but the value can be blank) + if fields[0] == "" { + return Err(anyhow!("name=value parameter missing name")); + } + + let value = fields[1..].join("="); + if value == "" { + return Err(anyhow!("name=value parameter missing value")); + } + + Ok(value) +} + fn get_container_pipe_size(param: &str) -> Result { let fields: Vec<&str> = param.split('=').collect(); @@ -270,6 +305,9 @@ mod tests { const ERR_INVALID_LOG_LEVEL: &str = "invalid log level"; const ERR_INVALID_LOG_LEVEL_PARAM: &str = "invalid log level parameter"; + const ERR_INVALID_GET_VALUE_PARAM: &str = "expected name=value"; + const ERR_INVALID_GET_VALUE_NO_NAME: &str = "name=value parameter missing name"; + const ERR_INVALID_GET_VALUE_NO_VALUE: &str = "name=value parameter missing value"; const ERR_INVALID_LOG_LEVEL_KEY: &str = "invalid log level key name"; const ERR_INVALID_HOTPLUG_TIMEOUT: &str = "invalid hotplug timeout parameter"; @@ -809,6 +847,61 @@ mod tests { server_addr: TEST_SERVER_ADDR, unified_cgroup_hierarchy: false, }, + TestData { + contents: "server_addr=unix:///tmp/foo.socket", + env_vars: Vec::new(), + debug_console: false, + dev_mode: false, + log_level: DEFAULT_LOG_LEVEL, + hotplug_timeout: DEFAULT_HOTPLUG_TIMEOUT, + container_pipe_size: DEFAULT_CONTAINER_PIPE_SIZE, + server_addr: TEST_SERVER_ADDR, + unified_cgroup_hierarchy: false, + }, + TestData { + contents: "agent.server_address=unix:///tmp/foo.socket", + env_vars: Vec::new(), + debug_console: false, + dev_mode: false, + log_level: DEFAULT_LOG_LEVEL, + hotplug_timeout: DEFAULT_HOTPLUG_TIMEOUT, + container_pipe_size: DEFAULT_CONTAINER_PIPE_SIZE, + server_addr: TEST_SERVER_ADDR, + unified_cgroup_hierarchy: false, + }, + TestData { + contents: "agent.server_addr=unix:///tmp/foo.socket", + env_vars: Vec::new(), + debug_console: false, + dev_mode: false, + log_level: DEFAULT_LOG_LEVEL, + hotplug_timeout: DEFAULT_HOTPLUG_TIMEOUT, + container_pipe_size: DEFAULT_CONTAINER_PIPE_SIZE, + server_addr: "unix:///tmp/foo.socket", + unified_cgroup_hierarchy: false, + }, + TestData { + contents: " agent.server_addr=unix:///tmp/foo.socket", + env_vars: Vec::new(), + debug_console: false, + dev_mode: false, + log_level: DEFAULT_LOG_LEVEL, + hotplug_timeout: DEFAULT_HOTPLUG_TIMEOUT, + container_pipe_size: DEFAULT_CONTAINER_PIPE_SIZE, + server_addr: "unix:///tmp/foo.socket", + unified_cgroup_hierarchy: false, + }, + TestData { + contents: " agent.server_addr=unix:///tmp/foo.socket a", + env_vars: Vec::new(), + debug_console: false, + dev_mode: false, + log_level: DEFAULT_LOG_LEVEL, + hotplug_timeout: DEFAULT_HOTPLUG_TIMEOUT, + container_pipe_size: DEFAULT_CONTAINER_PIPE_SIZE, + server_addr: "unix:///tmp/foo.socket", + unified_cgroup_hierarchy: false, + }, ]; let dir = tempdir().expect("failed to create tmpdir"); @@ -1195,4 +1288,82 @@ mod tests { assert_result!(d.result, result, msg); } } + + #[test] + fn test_get_string_value() { + #[derive(Debug)] + struct TestData<'a> { + param: &'a str, + result: Result, + } + + let tests = &[ + TestData { + param: "", + result: Err(make_err(ERR_INVALID_GET_VALUE_PARAM)), + }, + TestData { + param: "=", + result: Err(make_err(ERR_INVALID_GET_VALUE_NO_NAME)), + }, + TestData { + param: "==", + result: Err(make_err(ERR_INVALID_GET_VALUE_NO_NAME)), + }, + TestData { + param: "x=", + result: Err(make_err(ERR_INVALID_GET_VALUE_NO_VALUE)), + }, + TestData { + param: "x==", + result: Ok("=".into()), + }, + TestData { + param: "x===", + result: Ok("==".into()), + }, + TestData { + param: "x==x", + result: Ok("=x".into()), + }, + TestData { + param: "x=x", + result: Ok("x".into()), + }, + TestData { + param: "x=x=", + result: Ok("x=".into()), + }, + TestData { + param: "x=x=x", + result: Ok("x=x".into()), + }, + TestData { + param: "foo=bar", + result: Ok("bar".into()), + }, + TestData { + param: "x= =", + result: Ok(" =".into()), + }, + TestData { + param: "x= =", + result: Ok(" =".into()), + }, + TestData { + param: "x= = ", + result: Ok(" = ".into()), + }, + ]; + + for (i, d) in tests.iter().enumerate() { + let msg = format!("test[{}]: {:?}", i, d); + + let result = get_string_value(d.param); + + let msg = format!("{}: result: {:?}", msg, result); + + assert_result!(d.result, result, msg); + } + } } From 451b45f9d789c9bedfa205d17a8e7ae74eb19b48 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Fri, 19 Mar 2021 09:17:31 +0000 Subject: [PATCH 2/2] agent: Make use of test consts for error messages Make use of the `const` values for error messages that were previously only used for the unit tests. This guarantees consistency. Signed-off-by: James O. D. Hunt --- src/agent/src/config.rs | 58 ++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/agent/src/config.rs b/src/agent/src/config.rs index dd14e533e8..c3b311de31 100644 --- a/src/agent/src/config.rs +++ b/src/agent/src/config.rs @@ -27,6 +27,22 @@ const VSOCK_PORT: u16 = 1024; const SERVER_ADDR_ENV_VAR: &str = "KATA_AGENT_SERVER_ADDR"; const LOG_LEVEL_ENV_VAR: &str = "KATA_AGENT_LOG_LEVEL"; +const ERR_INVALID_LOG_LEVEL: &str = "invalid log level"; +const ERR_INVALID_LOG_LEVEL_PARAM: &str = "invalid log level parameter"; +const ERR_INVALID_GET_VALUE_PARAM: &str = "expected name=value"; +const ERR_INVALID_GET_VALUE_NO_NAME: &str = "name=value parameter missing name"; +const ERR_INVALID_GET_VALUE_NO_VALUE: &str = "name=value parameter missing value"; +const ERR_INVALID_LOG_LEVEL_KEY: &str = "invalid log level key name"; + +const ERR_INVALID_HOTPLUG_TIMEOUT: &str = "invalid hotplug timeout parameter"; +const ERR_INVALID_HOTPLUG_TIMEOUT_PARAM: &str = "unable to parse hotplug timeout"; +const ERR_INVALID_HOTPLUG_TIMEOUT_KEY: &str = "invalid hotplug timeout key name"; + +const ERR_INVALID_CONTAINER_PIPE_SIZE: &str = "invalid container pipe size parameter"; +const ERR_INVALID_CONTAINER_PIPE_SIZE_PARAM: &str = "unable to parse container pipe size"; +const ERR_INVALID_CONTAINER_PIPE_SIZE_KEY: &str = "invalid container pipe size key name"; +const ERR_INVALID_CONTAINER_PIPE_NEGATIVE: &str = "container pipe size should not be negative"; + #[derive(Debug)] pub struct AgentConfig { pub debug_console: bool, @@ -184,7 +200,7 @@ fn logrus_to_slog_level(logrus_level: &str) -> Result { "trace" => slog::Level::Trace, _ => { - return Err(anyhow!("invalid log level")); + return Err(anyhow!(ERR_INVALID_LOG_LEVEL)); } }; @@ -195,11 +211,11 @@ fn get_log_level(param: &str) -> Result { let fields: Vec<&str> = param.split('=').collect(); if fields.len() != 2 { - return Err(anyhow!("invalid log level parameter")); + return Err(anyhow!(ERR_INVALID_LOG_LEVEL_PARAM)); } if fields[0] != LOG_LEVEL_OPTION { - Err(anyhow!("invalid log level key name")) + Err(anyhow!(ERR_INVALID_LOG_LEVEL_KEY)) } else { Ok(logrus_to_slog_level(fields[1])?) } @@ -209,17 +225,17 @@ fn get_hotplug_timeout(param: &str) -> Result { let fields: Vec<&str> = param.split('=').collect(); if fields.len() != 2 { - return Err(anyhow!("invalid hotplug timeout parameter")); + return Err(anyhow!(ERR_INVALID_HOTPLUG_TIMEOUT)); } let key = fields[0]; if key != HOTPLUG_TIMOUT_OPTION { - return Err(anyhow!("invalid hotplug timeout key name")); + return Err(anyhow!(ERR_INVALID_HOTPLUG_TIMEOUT_KEY)); } let value = fields[1].parse::(); if value.is_err() { - return Err(anyhow!("unable to parse hotplug timeout")); + return Err(anyhow!(ERR_INVALID_HOTPLUG_TIMEOUT_PARAM)); } Ok(time::Duration::from_secs(value.unwrap())) @@ -253,17 +269,17 @@ fn get_string_value(param: &str) -> Result { let fields: Vec<&str> = param.split('=').collect(); if fields.len() < 2 { - return Err(anyhow!("expected name=value")); + return Err(anyhow!(ERR_INVALID_GET_VALUE_PARAM)); } // We need name (but the value can be blank) if fields[0] == "" { - return Err(anyhow!("name=value parameter missing name")); + return Err(anyhow!(ERR_INVALID_GET_VALUE_NO_NAME)); } let value = fields[1..].join("="); if value == "" { - return Err(anyhow!("name=value parameter missing value")); + return Err(anyhow!(ERR_INVALID_GET_VALUE_NO_VALUE)); } Ok(value) @@ -273,22 +289,22 @@ fn get_container_pipe_size(param: &str) -> Result { let fields: Vec<&str> = param.split('=').collect(); if fields.len() != 2 { - return Err(anyhow!("invalid container pipe size parameter")); + return Err(anyhow!(ERR_INVALID_CONTAINER_PIPE_SIZE)); } let key = fields[0]; if key != CONTAINER_PIPE_SIZE_OPTION { - return Err(anyhow!("invalid container pipe size key name")); + return Err(anyhow!(ERR_INVALID_CONTAINER_PIPE_SIZE_KEY)); } let res = fields[1].parse::(); if res.is_err() { - return Err(anyhow!("unable to parse container pipe size")); + return Err(anyhow!(ERR_INVALID_CONTAINER_PIPE_SIZE_PARAM)); } let value = res.unwrap(); if value < 0 { - return Err(anyhow!("container pipe size should not be negative")); + return Err(anyhow!(ERR_INVALID_CONTAINER_PIPE_NEGATIVE)); } Ok(value) @@ -303,22 +319,6 @@ mod tests { use std::time; use tempfile::tempdir; - const ERR_INVALID_LOG_LEVEL: &str = "invalid log level"; - const ERR_INVALID_LOG_LEVEL_PARAM: &str = "invalid log level parameter"; - const ERR_INVALID_GET_VALUE_PARAM: &str = "expected name=value"; - const ERR_INVALID_GET_VALUE_NO_NAME: &str = "name=value parameter missing name"; - const ERR_INVALID_GET_VALUE_NO_VALUE: &str = "name=value parameter missing value"; - const ERR_INVALID_LOG_LEVEL_KEY: &str = "invalid log level key name"; - - const ERR_INVALID_HOTPLUG_TIMEOUT: &str = "invalid hotplug timeout parameter"; - const ERR_INVALID_HOTPLUG_TIMEOUT_PARAM: &str = "unable to parse hotplug timeout"; - const ERR_INVALID_HOTPLUG_TIMEOUT_KEY: &str = "invalid hotplug timeout key name"; - - const ERR_INVALID_CONTAINER_PIPE_SIZE: &str = "invalid container pipe size parameter"; - const ERR_INVALID_CONTAINER_PIPE_SIZE_PARAM: &str = "unable to parse container pipe size"; - const ERR_INVALID_CONTAINER_PIPE_SIZE_KEY: &str = "invalid container pipe size key name"; - const ERR_INVALID_CONTAINER_PIPE_NEGATIVE: &str = "container pipe size should not be negative"; - // helper function to make errors less crazy-long fn make_err(desc: &str) -> Error { anyhow!(desc.to_string())