Skip to content

Commit 5ba7128

Browse files
danielorihueladanielorihuela
committed
feat: nr-default in defaults
1 parent 7610d19 commit 5ba7128

8 files changed

Lines changed: 97 additions & 130 deletions

File tree

‎agent-control/agent-type-registry/newrelic/com.newrelic.infrastructure-0.1.0.yaml‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ variables:
3333
description: "Package registry url"
3434
type: string
3535
required: false
36-
default: oci.registry
36+
default: "${nr-default:oci.registry}"
3737
variants:
3838
ac_config_field: "oci_registry_urls"
3939
values: [ "docker.io" ]
@@ -51,17 +51,17 @@ variables:
5151
description: "Username for HTTP Basic authentication"
5252
type: string
5353
required: false
54-
default: oci.auth.basic.username
54+
default: "${nr-default:oci.auth.basic.username}"
5555
password:
5656
description: "Password for HTTP Basic authentication"
5757
type: string
5858
required: false
59-
default: oci.auth.basic.password
59+
default: "${nr-default:oci.auth.basic.password}"
6060
bearer:
6161
description: "Bearer token for authentication"
6262
type: string
6363
required: false
64-
default: oci.auth.bearer
64+
default: "${nr-default:oci.auth.bearer}"
6565
version:
6666
description: "Agent version"
6767
type: string
@@ -102,7 +102,7 @@ variables:
102102
description: "Package registry url"
103103
type: string
104104
required: false
105-
default: oci.registry
105+
default: "${nr-default:oci.registry}"
106106
variants:
107107
ac_config_field: "oci_registry_urls"
108108
values: [ "docker.io" ]
@@ -120,17 +120,17 @@ variables:
120120
description: "Username for HTTP Basic authentication"
121121
type: string
122122
required: false
123-
default: oci.auth.basic.username
123+
default: "${nr-default:oci.auth.basic.username}"
124124
password:
125125
description: "Password for HTTP Basic authentication"
126126
type: string
127127
required: false
128-
default: oci.auth.basic.password
128+
default: "${nr-default:oci.auth.basic.password}"
129129
bearer:
130130
description: "Bearer token for authentication"
131131
type: string
132132
required: false
133-
default: oci.auth.bearer
133+
default: "${nr-default:oci.auth.bearer}"
134134
version:
135135
description: "Agent version"
136136
type: string

‎agent-control/agent-type-registry/newrelic/com.newrelic.opentelemetry.collector-0.1.0.yaml‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ variables:
3737
description: "Package registry url"
3838
type: string
3939
required: false
40-
default: oci.registry
40+
default: "${nr-default:oci.registry}"
4141
variants:
4242
ac_config_field: "oci_nrdot_registry_urls"
4343
values: [ "docker.io" ]
@@ -55,17 +55,17 @@ variables:
5555
description: "Username for HTTP Basic authentication"
5656
type: string
5757
required: false
58-
default: oci.auth.basic.username
58+
default: "${nr-default:oci.auth.basic.username}"
5959
password:
6060
description: "Password for HTTP Basic authentication"
6161
type: string
6262
required: false
63-
default: oci.auth.basic.password
63+
default: "${nr-default:oci.auth.basic.password}"
6464
bearer:
6565
description: "Bearer token for authentication"
6666
type: string
6767
required: false
68-
default: oci.auth.bearer
68+
default: "${nr-default:oci.auth.bearer}"
6969
version:
7070
description: "Agent version"
7171
type: string
@@ -100,7 +100,7 @@ variables:
100100
description: "Package registry url"
101101
type: string
102102
required: false
103-
default: oci.registry
103+
default: "${nr-default:oci.registry}"
104104
variants:
105105
ac_config_field: "oci_nrdot_registry_repositories"
106106
values: [ "docker.io" ]
@@ -118,17 +118,17 @@ variables:
118118
description: "Username for HTTP Basic authentication"
119119
type: string
120120
required: false
121-
default: oci.auth.basic.username
121+
default: "${nr-default:oci.auth.basic.username}"
122122
password:
123123
description: "Password for HTTP Basic authentication"
124124
type: string
125125
required: false
126-
default: oci.auth.basic.password
126+
default: "${nr-default:oci.auth.basic.password}"
127127
bearer:
128128
description: "Bearer token for authentication"
129129
type: string
130130
required: false
131-
default: oci.auth.bearer
131+
default: "${nr-default:oci.auth.bearer}"
132132
version:
133133
description: "Agent version"
134134
type: string

‎agent-control/src/agent_type/definition.rs‎

Lines changed: 28 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -101,23 +101,40 @@ impl Tree<VariableDefinition> {
101101
}
102102

103103
impl VariableTree {
104-
/// Returns a new [VariableTree] with the provided values assigned.
105-
/// After user values are applied, global defaults are used to override agent-type defaults
106-
/// for any variables that the user didn't provide.
107-
pub fn fill_with_values(
104+
pub fn template_defaults(
108105
self,
109-
values: YAMLConfig,
110-
global_defaults: HashMap<String, serde_yaml::Value>,
106+
global_defaults_vars: &Variables,
111107
) -> Result<Self, AgentTypeError> {
112108
let mut vars = self.0;
113-
// First apply user-provided values
109+
template_defaults_recursive(&mut vars, global_defaults_vars)?;
110+
Ok(Self(vars))
111+
}
112+
113+
/// Returns a new [VariableTree] with the provided values assigned.
114+
pub fn fill_with_values(self, values: YAMLConfig) -> Result<Self, AgentTypeError> {
115+
let mut vars = self.0;
114116
update_specs(values.into(), &mut vars)?;
115-
// Then apply global defaults to variables not provided by user
116-
apply_global_defaults(&mut vars, &global_defaults)?;
117117
Ok(Self(vars))
118118
}
119119
}
120120

121+
fn template_defaults_recursive(
122+
agent_vars: &mut HashMap<String, Tree<Variable>>,
123+
global_defaults_vars: &Variables,
124+
) -> Result<(), AgentTypeError> {
125+
for (_key, var_tree) in agent_vars.iter_mut() {
126+
match var_tree {
127+
Tree::End(variable) => {
128+
variable.template_default(global_defaults_vars)?;
129+
}
130+
Tree::Mapping(nested) => {
131+
template_defaults_recursive(nested, global_defaults_vars)?;
132+
}
133+
}
134+
}
135+
Ok(())
136+
}
137+
121138
fn update_specs(
122139
values: HashMap<String, serde_yaml::Value>,
123140
agent_vars: &mut HashMap<String, Tree<Variable>>,
@@ -139,35 +156,6 @@ fn update_specs(
139156
Ok(())
140157
}
141158

142-
/// Applies global defaults to variables that don't have user-provided values.
143-
/// For each variable in the tree, if the variable's default value exists as a key
144-
/// in global_defaults and the variable doesn't have a final_value (user didn't provide it),
145-
/// the global default value replaces the agent-type's default.
146-
///
147-
/// Example: If a variable has default "registry_url" and global_defaults contains
148-
/// {"registry_url": "docker.io"}, the variable's default will be replaced with "docker.io".
149-
fn apply_global_defaults(
150-
agent_vars: &mut HashMap<String, Tree<Variable>>,
151-
global_defaults: &HashMap<String, serde_yaml::Value>,
152-
) -> Result<(), AgentTypeError> {
153-
for (_key, var_tree) in agent_vars.iter_mut() {
154-
match var_tree {
155-
Tree::End(variable) => {
156-
// Look up by the variable's default value, not by the variable name
157-
if let Some(default_key) = variable.get_default_as_key()
158-
&& let Some(global_value) = global_defaults.get(&default_key)
159-
{
160-
variable.set_default_from_global(global_value.clone())?;
161-
}
162-
}
163-
Tree::Mapping(nested) => {
164-
apply_global_defaults(nested, global_defaults)?;
165-
}
166-
}
167-
}
168-
Ok(())
169-
}
170-
171159
/// Represents a normalized version of [VariableTree].
172160
///
173161
/// Example of the end node in the tree:
@@ -322,7 +310,7 @@ pub mod tests {
322310
let values = serde_yaml::from_str::<YAMLConfig>(yaml_values).unwrap();
323311
self.variables
324312
.clone()
325-
.fill_with_values(values, HashMap::new())
313+
.fill_with_values(values)
326314
.unwrap()
327315
.flatten()
328316
}
@@ -588,7 +576,7 @@ restart_policy:
588576
let filled_variables_result = agent_type
589577
.variables
590578
.clone()
591-
.fill_with_values(invalid_values, HashMap::new());
579+
.fill_with_values(invalid_values);
592580
assert!(filled_variables_result.is_err());
593581
assert_eq!(
594582
filled_variables_result.unwrap_err().to_string(),

‎agent-control/src/agent_type/render.rs‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::agent_type::{
22
agent_attributes::AgentAttributes,
3-
definition::AgentType,
3+
definition::{AgentType, Variables},
44
error::AgentTypeError,
55
runtime_config::rendered::Runtime,
66
templates::Templateable,
@@ -38,9 +38,13 @@ impl TemplateRenderer {
3838
// Expand templates in global defaults (so they can reference secrets/env vars)
3939
let global_defaults_expanded = global_defaults.template_with(&secrets)?;
4040

41-
// Fill agent variables with user values and global defaults, then flatten
41+
// Convert global defaults to Variables with nr-default namespace
42+
let global_defaults_vars = global_defaults_to_variables(global_defaults_expanded);
43+
44+
// Template defaults in variables, then fill with user values, then flatten
4245
let filled_variables = variables
43-
.fill_with_values(values_expanded, global_defaults_expanded)?
46+
.template_defaults(&global_defaults_vars)?
47+
.fill_with_values(values_expanded)?
4448
.flatten();
4549

4650
Self::check_all_vars_are_populated(&filled_variables)?;
@@ -106,6 +110,24 @@ impl TemplateRenderer {
106110
}
107111
}
108112

113+
fn global_defaults_to_variables(global_defaults: HashMap<String, serde_yaml::Value>) -> Variables {
114+
global_defaults
115+
.into_iter()
116+
.map(|(key, value)| {
117+
// Convert the YAML value to a string representation
118+
let string_value = match value {
119+
serde_yaml::Value::String(s) => s,
120+
other => serde_yaml::to_string(&other)
121+
.unwrap_or_else(|_| String::new())
122+
.trim()
123+
.to_string(),
124+
};
125+
let var = Variable::new_final_string_variable(string_value);
126+
(Namespace::Default.namespaced_name(&key), var)
127+
})
128+
.collect()
129+
}
130+
109131
#[cfg(test)]
110132
pub(crate) mod tests {
111133
use std::path::PathBuf;
@@ -330,7 +352,7 @@ pub(crate) mod tests {
330352
agent_type
331353
.variables
332354
.clone()
333-
.fill_with_values(values, HashMap::new())
355+
.fill_with_values(values)
334356
.is_err()
335357
)
336358
}
@@ -651,19 +673,19 @@ variables:
651673
description: "registry url"
652674
type: string
653675
required: false
654-
default: registry_url
676+
default: "${nr-default:registry_url}"
655677
deployment:
656678
linux:
657679
executables:
658680
- id: first
659681
path: /opt/first
660-
args:
682+
args:
661683
- "${nr-var:registry}"
662684
windows:
663685
executables:
664686
- id: first
665687
path: /opt/first
666-
args:
688+
args:
667689
- "${nr-var:registry}"
668690
"#,
669691
&AGENT_CONTROL_MODE_ON_HOST,
@@ -714,19 +736,19 @@ variables:
714736
description: "registry url"
715737
type: string
716738
required: false
717-
default: registry_url
739+
default: "${nr-default:registry_url}"
718740
deployment:
719741
linux:
720742
executables:
721743
- id: first
722744
path: /opt/first
723-
args:
745+
args:
724746
- "${nr-var:registry}"
725747
windows:
726748
executables:
727749
- id: first
728750
path: /opt/first
729-
args:
751+
args:
730752
- "${nr-var:registry}"
731753
"#,
732754
&AGENT_CONTROL_MODE_ON_HOST,

‎agent-control/src/agent_type/variable.rs‎

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ pub mod tree;
1414
pub mod variable_type;
1515
pub mod variants;
1616

17+
use std::collections::HashMap;
18+
1719
use serde::{Deserialize, Serialize};
1820

1921
use crate::agent_type::{
@@ -80,25 +82,11 @@ impl Variable {
8082
self.variable_type.merge_with_yaml_value(yaml)
8183
}
8284

83-
/// Sets the default value from a global default configuration.
84-
/// Only applies if the user hasn't already provided a final value.
85-
pub(crate) fn set_default_from_global(
85+
pub fn template_default(
8686
&mut self,
87-
value: serde_yaml::Value,
87+
global_defaults_vars: &HashMap<String, Variable>,
8888
) -> Result<(), AgentTypeError> {
89-
// Only set default if we don't already have a final_value (user didn't provide it)
90-
// Check final_value directly, not get_final_value() which includes defaults
91-
if self.variable_type.has_final_value() {
92-
return Ok(());
93-
}
94-
95-
self.variable_type.set_default(value)
96-
}
97-
98-
/// Gets the default value as a string key for looking up in global defaults.
99-
/// Returns None if there's no default or if the default is not a string.
100-
pub(crate) fn get_default_as_key(&self) -> Option<String> {
101-
self.variable_type.get_default_as_key()
89+
self.variable_type.template_default(global_defaults_vars)
10290
}
10391

10492
pub fn kind(&self) -> &VariableType {

‎agent-control/src/agent_type/variable/fields.rs‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,6 @@ where
119119
self.final_value = Some(value);
120120
Ok(())
121121
}
122-
123-
pub(crate) fn set_default(&mut self, value: T) -> Result<(), AgentTypeError> {
124-
self.default = Some(value);
125-
Ok(())
126-
}
127122
}
128123

129124
impl StringFields {
@@ -134,14 +129,6 @@ impl StringFields {
134129
self.inner.set_final_value(value)?;
135130
Ok(())
136131
}
137-
138-
pub(crate) fn set_default(&mut self, value: String) -> Result<(), AgentTypeError> {
139-
if !self.variants.is_valid(&value) {
140-
return Err(AgentTypeError::InvalidVariant(self.variants.to_string()));
141-
}
142-
self.inner.set_default(value)?;
143-
Ok(())
144-
}
145132
}
146133

147134
impl<'de, T> Deserialize<'de> for FieldsDefinition<T>

0 commit comments

Comments
 (0)