Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
201 changes: 201 additions & 0 deletions cli/golem-cli/src/command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ use golem_common::model::agent::AgentTypeName;
use golem_common::model::application::ApplicationName;
use golem_common::model::component::{ComponentName, ComponentRevision};
use golem_common::model::deployment::DeploymentRevision;
use golem_common::model::worker::WorkerAgentConfigEntry;
use lenient_bool::LenientBool;
use std::collections::{BTreeSet, HashMap};
use std::ffi::OsString;
Expand Down Expand Up @@ -1072,6 +1073,7 @@ pub mod component {
pub mod worker {
use crate::command::parse_cursor;
use crate::command::parse_key_val;
use crate::command::parse_worker_agent_config;
use crate::command::shared_args::{
AgentIdArgs, PostDeployArgs, StreamArgs, WorkerFunctionArgument, WorkerFunctionName,
};
Expand All @@ -1080,6 +1082,7 @@ pub mod worker {
use clap::Subcommand;
use golem_client::model::ScanCursor;
use golem_common::model::component::{ComponentName, ComponentRevision};
use golem_common::model::worker::WorkerAgentConfigEntry;
use golem_common::model::IdempotencyKey;
use uuid::Uuid;

Expand All @@ -1095,6 +1098,9 @@ pub mod worker {
/// wasi:config entries visible for the agent
#[arg(short, long, value_parser = parse_key_val, value_name = "VAR=VAL")]
config_vars: Vec<(String, String)>,
/// agent config for entries
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not a very understandable help string

#[arg(short, long, value_parser = parse_worker_agent_config)]
agent_config: Vec<WorkerAgentConfigEntry>,
},
// TODO: json args
/// Invoke (or enqueue invocation for) agent
Expand Down Expand Up @@ -1654,6 +1660,91 @@ fn parse_key_val(key_and_val: &str) -> anyhow::Result<(String, String)> {
))
}

fn parse_worker_agent_config(s: &str) -> anyhow::Result<WorkerAgentConfigEntry> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe these parsing functions with their corresponding tests could live in a small submodule in a different file, this one is getting huge

let (path, value) = split_worker_agent_config_path_and_value(s)?;

let path = parse_worker_agent_config_path(path)?;

let value: serde_json::Value = serde_json::from_str(value)?;

Ok(WorkerAgentConfigEntry { path, value })
}

fn split_worker_agent_config_path_and_value(input: &str) -> anyhow::Result<(&str, &str)> {
let chars = input.char_indices();
let mut in_quotes = false;
let mut escape = false;

for (i, c) in chars {
if escape {
escape = false;
continue;
}

match c {
'\\' => escape = true,
'"' => in_quotes = !in_quotes,
'=' if !in_quotes => {
let key = &input[..i];
let value = &input[i + 1..];
return Ok((key, value));
}
_ => {}
}
}

Err(anyhow!("expected unescaped '=' separating key and value"))
}

fn parse_worker_agent_config_path(input: &str) -> anyhow::Result<Vec<String>> {
let mut keys = Vec::new();
let mut buf = String::new();

let mut chars = input.chars().peekable();
let mut in_quotes = false;

while let Some(c) = chars.next() {
match c {
'\\' => {
// escape next char
let next = chars.next().ok_or_else(|| anyhow!("dangling escape"))?;
buf.push(next);
}

'"' => {
in_quotes = !in_quotes;
}

'.' if !in_quotes => {
push_agent_config_path_segment(&mut keys, &mut buf)?;
}

_ => buf.push(c),
}
}

if in_quotes {
return Err(anyhow!("unterminated quote"));
}

push_agent_config_path_segment(&mut keys, &mut buf)?;

Ok(keys)
}

fn push_agent_config_path_segment(keys: &mut Vec<String>, buf: &mut String) -> anyhow::Result<()> {
let segment = buf.trim();

if segment.is_empty() {
return Err(anyhow!("empty path segment"));
}

keys.push(segment.to_string());
buf.clear();

Ok(())
}

// TODO: better error context and messages
fn parse_cursor(cursor: &str) -> anyhow::Result<ScanCursor> {
let parts = cursor.split('/').collect::<Vec<_>>();
Expand Down Expand Up @@ -1921,3 +2012,113 @@ mod test {
}
}
}

#[cfg(test)]
mod parse_worker_agent_config_tests {
use crate::command::{parse_worker_agent_config, parse_worker_agent_config_path};
use golem_common::model::worker::WorkerAgentConfigEntry;
use serde_json::json;
use test_r::test;

fn parse(input: &str) -> WorkerAgentConfigEntry {
parse_worker_agent_config(input).unwrap()
}

#[test]
fn simple_path() {
let e = parse(r#"a.b.c=1"#);

assert_eq!(e.path, vec!["a", "b", "c"]);
assert_eq!(e.value, json!(1));
}

#[test]
fn string_value() {
let e = parse(r#"a.b="hello""#);

assert_eq!(e.path, vec!["a", "b"]);
assert_eq!(e.value, json!("hello"));
}

#[test]
fn json_object_value() {
let e = parse(r#"a.b={"x":1,"y":2}"#);

assert_eq!(e.path, vec!["a", "b"]);
assert_eq!(e.value, json!({"x":1,"y":2}));
}

#[test]
fn quoted_path_segment() {
let e = parse(r#""foo.bar".baz=1"#);

assert_eq!(e.path, vec!["foo.bar", "baz"]);
assert_eq!(e.value, json!(1));
}

#[test]
fn quoted_segment_with_spaces() {
let e = parse(r#""foo bar".baz=1"#);

assert_eq!(e.path, vec!["foo bar", "baz"]);
assert_eq!(e.value, json!(1));
}

#[test]
fn escaped_dot_in_segment() {
let e = parse(r#"foo\.bar.baz=1"#);

assert_eq!(e.path, vec!["foo.bar", "baz"]);
assert_eq!(e.value, json!(1));
}

#[test]
fn equals_inside_value() {
let e = parse(r#"a.b="foo=bar""#);

assert_eq!(e.path, vec!["a", "b"]);
assert_eq!(e.value, json!("foo=bar"));
}

#[test]
fn equals_inside_path() {
let e = parse(r#""foo=bar".baz=1"#);

assert_eq!(e.path, vec!["foo=bar", "baz"]);
assert_eq!(e.value, json!(1));
}

#[test]
fn escaped_equals_in_path() {
let e = parse(r#"foo\=bar.baz=1"#);

assert_eq!(e.path, vec!["foo=bar", "baz"]);
assert_eq!(e.value, json!(1));
}

#[test]
fn complex_case() {
let e = parse(r#""foo.bar=baz"."x.y"={"hello":"world"}"#);

assert_eq!(e.path, vec!["foo.bar=baz", "x.y"]);
assert_eq!(e.value, json!({"hello":"world"}));
}

#[test]
fn split_fails_without_equals() {
let err = parse_worker_agent_config("a.b.c").unwrap_err();
assert!(err.to_string().contains("expected unescaped '='"));
}

#[test]
fn unterminated_quote_in_path() {
let err = parse_worker_agent_config_path(r#""foo.bar.baz"#).unwrap_err();
assert!(err.to_string().contains("unterminated quote"));
}

#[test]
fn dangling_escape() {
let err = parse_worker_agent_config_path(r#"foo.bar\"#).unwrap_err();
assert!(err.to_string().contains("dangling escape"));
}
}
4 changes: 1 addition & 3 deletions cli/golem-cli/src/command_handler/app/deploy_diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -776,9 +776,7 @@ fn normalized_diff_deployment(
wasm_hash: component.wasm_hash,
files_by_path: component.files_by_path.clone(),
plugins_by_grant_id: component.plugins_by_grant_id.clone(),
local_agent_config_ordered_by_agent_and_key: component
.local_agent_config_ordered_by_agent_and_key
.clone(),
ordered_agent_config: component.ordered_agent_config.clone(),
}
.into(),
None => component.hash().into(),
Expand Down
11 changes: 9 additions & 2 deletions cli/golem-cli/src/command_handler/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1539,6 +1539,14 @@ impl AppCommandHandler {
&self,
deploy_diff: &DeployDiff,
) -> anyhow::Result<CurrentDeployment> {
let agent_secret_defaults = {
let app_ctx = self.ctx.app_context_lock().await;
app_ctx
.some_or_err()?
.application()
.deployment_agent_secret_defaults(&deploy_diff.environment.environment_name)
};

let clients = self.ctx.golem_clients().await?;

log_action("Deploying", "staged changes to the environment");
Expand All @@ -1551,8 +1559,7 @@ impl AppCommandHandler {
current_revision: deploy_diff.current_deployment_revision(),
expected_deployment_hash: deploy_diff.local_deployment_hash,
version: DeploymentVersion("".to_string()), // TODO: atomic
// FIXME: agent-config
agent_secret_defaults: Vec::new(),
agent_secret_defaults,
},
)
.await
Expand Down
22 changes: 15 additions & 7 deletions cli/golem-cli/src/command_handler/component/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ use golem_common::model::component::{
ComponentId, ComponentName, ComponentRevision, ComponentUpdate,
};
use golem_common::model::deployment::DeploymentPlanComponentEntry;
use golem_common::model::diff;
use golem_common::model::diff::{self, VecDiffable};
use golem_common::model::environment::EnvironmentName;
use itertools::Itertools;
use std::collections::{BTreeMap, HashMap, HashSet};
Expand Down Expand Up @@ -883,6 +883,7 @@ impl ComponentCommandHandler {
let plugins = component.plugins().clone();
let env = resolve_env_vars(component_name, component.env())?;
let config_vars = component.config_vars().clone();
let agent_config = component.agent_config().clone();

Ok(ComponentDeployProperties {
wasm_path,
Expand All @@ -891,6 +892,7 @@ impl ComponentCommandHandler {
plugins,
env,
config_vars,
agent_config,
})
}

Expand Down Expand Up @@ -997,8 +999,16 @@ impl ComponentCommandHandler {
wasm_hash: component_binary_hash.into(),
files_by_path,
plugins_by_grant_id,
// FIXME: agent-config
local_agent_config_ordered_by_agent_and_key: Vec::new(),
ordered_agent_config: properties
.agent_config
.iter()
.map(|lac| diff::AgentConfigEntry {
agent: lac.agent.0.clone(),
path: lac.path.clone(),
value: diff::into_normalized_json(lac.value.clone()),
})
.sorted_by(|v1, v2| v1.ordering_key().cmp(&v2.ordering_key()))
.collect(),
})
}

Expand Down Expand Up @@ -1045,8 +1055,7 @@ impl ComponentCommandHandler {
.unwrap_or_default(),
env: component_stager.env(),
config_vars: component_stager.config_vars(),
// FIXME: agent-config
local_agent_config: Vec::new(),
agent_config: component_stager.agent_config(),
agent_types,
plugins: component_stager.plugins(),
},
Expand Down Expand Up @@ -1141,8 +1150,7 @@ impl ComponentCommandHandler {
removed_files: changed_files.removed.clone(),
new_file_options: changed_files.merged_file_options(),
config_vars: component_stager.config_vars_if_changed(),
// FIXME: agent-config
local_agent_config: None,
agent_config: component_stager.agent_config_if_changed(),
env: component_stager.env_if_changed(),
agent_types,
plugin_updates: component_stager.plugins_if_changed(),
Expand Down
23 changes: 21 additions & 2 deletions cli/golem-cli/src/command_handler/component/staging.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ use anyhow::{anyhow, Context as AnyhowContext};
use golem_client::model::EnvironmentPluginGrantWithDetails;
use golem_common::model::agent::AgentType;
use golem_common::model::component::{
ComponentFileOptions, ComponentFilePath, PluginInstallation, PluginInstallationAction,
PluginInstallationUpdate, PluginPriority, PluginUninstallation,
AgentConfigEntry, ComponentFileOptions, ComponentFilePath, PluginInstallation,
PluginInstallationAction, PluginInstallationUpdate, PluginPriority, PluginUninstallation,
};

use golem_common::model::diff;
Expand Down Expand Up @@ -62,6 +62,13 @@ impl ComponentDiff {
ComponentDiff::Diff { diff } => diff.metadata_changed,
}
}

pub fn agent_config_changed(&self) -> bool {
match self {
ComponentDiff::All => true,
ComponentDiff::Diff { diff } => !diff.agent_config_changes.is_empty(),
}
}
}

pub struct ChangedComponentFiles {
Expand Down Expand Up @@ -269,6 +276,18 @@ impl<'a> ComponentStager<'a> {
}
}

pub fn agent_config(&self) -> Vec<AgentConfigEntry> {
self.component_deploy_properties.agent_config.clone()
}

pub fn agent_config_if_changed(&self) -> Option<Vec<AgentConfigEntry>> {
if self.diff.agent_config_changed() {
Some(self.agent_config())
} else {
None
}
}

pub fn plugins(&self) -> Vec<PluginInstallation> {
self.component_deploy_properties
.plugins
Expand Down
Loading
Loading