Skip to content

Commit 4994113

Browse files
paologallinaharburpaologallinaharbur
authored
fix(agent): hidden double expansion (#940)
* fix(agent): hidden double expansion
1 parent a3751f8 commit 4994113

File tree

6 files changed

+87
-66
lines changed

6 files changed

+87
-66
lines changed

‎Cargo.lock‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎super-agent/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
[package]
22
name = "newrelic_super_agent"
33
description = "New Relic Super Agent Limited Preview"
4-
version = "0.26.0"
4+
version = "0.26.1"
55
authors.workspace = true
66
edition.workspace = true
77
rust-version.workspace = true

‎super-agent/src/agent_type/renderer.rs‎

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ impl<C: ConfigurationPersister> Renderer for TemplateRenderer<C> {
5555
// `filled_variables` needs to be mutable, in case there are `File` or `MapStringFile` variables, whose path
5656
// needs to be expanded, checkout out the TODO below for details.
5757
let mut filled_variables = variables.fill_with_values(values_expanded)?.flatten();
58+
5859
Self::check_all_vars_are_populated(&filled_variables)?;
5960

6061
// TODO: the persister performs specific actions for file and `File` and `MapStringFile` variables kind only.
@@ -513,16 +514,16 @@ pub(crate) mod tests {
513514

514515
let expected_spec_yaml = r#"
515516
values:
516-
key: value
517517
another_key:
518-
nested: nested_value
518+
nested: nested_value ${UNTOUCHED}
519519
nested_list:
520520
- item1
521521
- item2
522522
- item3_nested: value
523523
empty_key:
524524
from_sub_agent: some-agent-id
525-
collision_avoided: ${config.values}-${env:agent_id}
525+
text_values: "key: value\nkey2: ${UNTOUCHED}\n\n"
526+
collision_avoided: ${config.values}-${env:agent_id}-${UNTOUCHED}
526527
"#;
527528
let expected_spec_value: serde_yaml::Value =
528529
serde_yaml::from_str(expected_spec_yaml).unwrap();
@@ -558,17 +559,16 @@ collision_avoided: ${config.values}-${env:agent_id}
558559

559560
let expected_spec_yaml = r#"
560561
values:
561-
key: value
562562
another_key:
563-
nested: nested_value
563+
nested: nested_value ${UNTOUCHED}
564564
nested_list:
565565
- item1
566566
- item2
567567
- item3_nested: value
568568
empty_key:
569569
from_sub_agent: some-agent-id
570570
substituted: my-value
571-
collision_avoided: ${config.values}-${env:agent_id}
571+
collision_avoided: ${config.values}-${env:agent_id}-${UNTOUCHED}
572572
substituted_2: my-value-2
573573
"#;
574574

@@ -600,6 +600,9 @@ substituted_2: my-value-2
600600
let values = testing_values(
601601
r#"
602602
config:
603+
text_values:
604+
key: value
605+
key2: ${UNTOUCHED}
603606
values:
604607
key: ${nr-env:DOUBLE_EXPANSION}
605608
key-2: ${nr-env:DOUBLE_EXPANSION_2}
@@ -615,7 +618,8 @@ values:
615618
key: test
616619
key-2: test-2
617620
from_sub_agent: some-agent-id
618-
collision_avoided: ${config.values}-${env:agent_id}
621+
text_values: "key: value\nkey2: ${UNTOUCHED}\n\n"
622+
collision_avoided: ${config.values}-${env:agent_id}-${UNTOUCHED}
619623
"#;
620624

621625
let expected_spec_value: serde_yaml::Value =
@@ -668,6 +672,10 @@ variables:
668672
description: "yaml values"
669673
type: yaml
670674
required: true
675+
text_values:
676+
description: "yaml values"
677+
type: yaml
678+
required: true
671679
deployment:
672680
k8s:
673681
objects:
@@ -676,7 +684,6 @@ deployment:
676684
kind: ObjectKind
677685
metadata:
678686
name: test
679-
680687
substituted: ${nr-env:MY_VARIABLE}
681688
"#,
682689
&Environment::K8s,
@@ -902,6 +909,10 @@ variables:
902909
description: "yaml values"
903910
type: yaml
904911
required: true
912+
text_values:
913+
description: "text values"
914+
type: yaml
915+
required: true
905916
deployment:
906917
k8s:
907918
objects:
@@ -913,7 +924,9 @@ deployment:
913924
spec:
914925
values: ${nr-var:config.values}
915926
from_sub_agent: ${nr-sub:agent_id}
916-
collision_avoided: ${config.values}-${env:agent_id}
927+
text_values: |
928+
${nr-var:config.text_values}
929+
collision_avoided: ${config.values}-${env:agent_id}-${UNTOUCHED}
917930
"#;
918931

919932
const K8S_AGENT_TYPE_YAML_ENVIRONMENT_VARIABLES: &str = r#"
@@ -927,6 +940,10 @@ variables:
927940
description: "yaml values"
928941
type: yaml
929942
required: true
943+
text_values:
944+
description: "text values"
945+
type: yaml
946+
required: true
930947
deployment:
931948
k8s:
932949
objects:
@@ -939,20 +956,21 @@ deployment:
939956
values: ${nr-var:config.values}
940957
from_sub_agent: ${nr-sub:agent_id}
941958
substituted: ${nr-env:MY_VARIABLE}
942-
collision_avoided: ${config.values}-${env:agent_id}
959+
collision_avoided: ${config.values}-${env:agent_id}-${UNTOUCHED}
943960
substituted_2: ${nr-env:MY_VARIABLE_2}
944961
"#;
945962

946963
const K8S_CONFIG_YAML_VALUES: &str = r#"
947964
config:
948-
values:
965+
text_values:
949966
key: value
967+
key2: ${UNTOUCHED}
968+
values:
950969
another_key:
951-
nested: nested_value
970+
nested: nested_value ${UNTOUCHED}
952971
nested_list:
953972
- item1
954973
- item2
955974
- item3_nested: value
956-
empty_key:
957-
"#;
975+
empty_key:"#;
958976
}

‎super-agent/src/agent_type/runtime_config_templates.rs‎

Lines changed: 46 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,3 @@
1-
use std::collections::{BTreeMap, HashMap};
2-
use std::sync::OnceLock;
3-
4-
use regex::Regex;
5-
use tracing::warn;
6-
71
use super::definition::Variables;
82
use super::runtime_config::Env;
93
use super::variable::definition::VariableDefinition;
@@ -13,6 +7,10 @@ use super::{
137
restart_policy::{BackoffStrategyConfig, RestartPolicyConfig},
148
runtime_config::{Deployment, Executable, K8s, K8sObject, K8sObjectMeta, OnHost, Runtime},
159
};
10+
use regex::Regex;
11+
use std::collections::{BTreeMap, HashMap};
12+
use std::sync::OnceLock;
13+
use tracing::warn;
1614

1715
/// Regex that extracts the template values from a string.
1816
///
@@ -59,20 +57,18 @@ fn normalized_var<'a>(
5957
.ok_or(AgentTypeError::MissingTemplateKey(name.to_string()))
6058
}
6159

62-
/// Returns a string with the first match of a variable replaced with the corresponding value
63-
/// (according to the provided normalized variable).
60+
/// Returns a string with the variable replaced with the corresponding value .
6461
fn replace(
65-
re: &Regex,
62+
variable: &str,
6663
s: &str,
67-
var_name: &str,
6864
normalized_var: &VariableDefinition,
6965
) -> Result<String, AgentTypeError> {
7066
let value = normalized_var
7167
.get_template_value()
72-
.ok_or(AgentTypeError::MissingTemplateKey(var_name.to_string()))?
68+
.ok_or(AgentTypeError::MissingTemplateKey(variable.to_string()))?
7369
.to_string();
7470

75-
Ok(re.replace(s, value).to_string())
71+
Ok(s.replace(variable, value.as_str()))
7672
}
7773

7874
pub trait Templateable {
@@ -102,10 +98,10 @@ impl Templateable for String {
10298
fn template_string(s: String, variables: &Variables) -> Result<String, AgentTypeError> {
10399
let re = template_re();
104100
re.find_iter(&s)
105-
.map(|i| i.as_str())
106-
.try_fold(s.clone(), |r, i| {
107-
let var_name = template_trim(i);
108-
replace(re, &r, var_name, normalized_var(var_name, variables)?)
101+
.try_fold(s.clone(), |r, variable_to_substitute| {
102+
let var_name = template_trim(variable_to_substitute.as_str());
103+
let normalized_var = normalized_var(var_name, variables)?;
104+
replace(variable_to_substitute.as_str(), &r, normalized_var)
109105
})
110106
}
111107

@@ -345,16 +341,23 @@ mod tests {
345341
let variables = Variables::from([
346342
(
347343
"nr-var:name".to_string(),
348-
VariableDefinition::new(String::default(), true, None, Some("Alice".to_string())),
344+
VariableDefinition::new(
345+
String::default(),
346+
true,
347+
None,
348+
Some("Alice ${UNTOUCHED}".to_string()),
349+
),
349350
),
350351
(
351352
"nr-var:age".to_string(),
352353
VariableDefinition::new(String::default(), true, None, Some(Number::from(30))),
353354
),
354355
]);
355356

356-
let input = "Hello ${nr-var:name}! You are ${nr-var:age} years old.".to_string();
357-
let expected_output = "Hello Alice! You are 30 years old.".to_string();
357+
let input =
358+
"Hello ${nr-var:name}! You are ${nr-var:age} years old. ${UNTOUCHED}".to_string();
359+
let expected_output =
360+
"Hello Alice ${UNTOUCHED}! You are 30 years old. ${UNTOUCHED}".to_string();
358361
let actual_output = template_string(input, &variables).unwrap();
359362
assert_eq!(actual_output, expected_output);
360363
}
@@ -499,7 +502,7 @@ mod tests {
499502
String::default(),
500503
true,
501504
None,
502-
Some("CHANGED-STRING".to_string()),
505+
Some("CHANGED-STRING ${UNTOUCHED}".to_string()),
503506
),
504507
),
505508
(
@@ -519,17 +522,19 @@ mod tests {
519522
${nr-var:change.me.string}: "Do not scape me"
520523
${nr-var:change.me.bool}: "Do not scape me"
521524
${nr-var:change.me.number}: "Do not scape me"
525+
test: ${UNTOUCHED}
522526
"#,
523527
)
524528
.unwrap();
525529
let expected_output: serde_yaml::Mapping = serde_yaml::from_str(
526530
r#"
527-
a_string: "CHANGED-STRING"
531+
a_string: "CHANGED-STRING ${UNTOUCHED}"
528532
a_boolean: true
529533
a_number: 42
530534
${nr-var:change.me.string}: "Do not scape me"
531535
${nr-var:change.me.bool}: "Do not scape me"
532536
${nr-var:change.me.number}: "Do not scape me"
537+
test: ${UNTOUCHED}
533538
"#,
534539
)
535540
.unwrap();
@@ -547,7 +552,7 @@ mod tests {
547552
String::default(),
548553
true,
549554
None,
550-
Some("CHANGED-STRING".to_string()),
555+
Some("CHANGED-STRING ${UNTOUCHED}".to_string()),
551556
),
552557
),
553558
(
@@ -564,15 +569,17 @@ mod tests {
564569
- ${nr-var:change.me.string}
565570
- ${nr-var:change.me.bool}
566571
- ${nr-var:change.me.number}
572+
- ${UNTOUCHED}
567573
- Do not scape me
568574
"#,
569575
)
570576
.unwrap();
571577
let expected_output: serde_yaml::Sequence = serde_yaml::from_str(
572578
r#"
573-
- CHANGED-STRING
579+
- CHANGED-STRING ${UNTOUCHED}
574580
- true
575581
- 42
582+
- ${UNTOUCHED}
576583
- Do not scape me
577584
"#,
578585
)
@@ -591,7 +598,7 @@ mod tests {
591598
String::default(),
592599
true,
593600
None,
594-
Some("CHANGED-STRING".to_string()),
601+
Some("CHANGED-STRING ${UNTOUCHED}".to_string()),
595602
),
596603
),
597604
(
@@ -651,35 +658,37 @@ mod tests {
651658
a_yaml: ${nr-var:change.me.yaml}
652659
another_yaml: ${nr-var:yaml.with.var.placeholder} # A variable inside another variable value is not expanded
653660
string_key: "here, the value ${nr-var:change.me.yaml} is encoded as string because it is not alone"
661+
last_one: ${UNTOUCHED}
654662
"#,
655663
)
656664
.unwrap();
657665
let expected_output: serde_yaml::Value = serde_yaml::from_str(
658666
r#"
659667
an_object:
660-
a_string: "CHANGED-STRING"
668+
a_string: "CHANGED-STRING ${UNTOUCHED}"
661669
a_boolean: true
662670
a_number: 42
663671
a_sequence:
664-
- "CHANGED-STRING"
672+
- "CHANGED-STRING ${UNTOUCHED}"
665673
- true
666674
- 42
667675
a_nested_object:
668676
with_nested_sequence:
669-
- a_string: "CHANGED-STRING"
677+
- a_string: "CHANGED-STRING ${UNTOUCHED}"
670678
- a_boolean: true
671679
- a_number: 42
672680
- a_yaml:
673681
key:
674682
value
675-
a_string: "CHANGED-STRING"
683+
a_string: "CHANGED-STRING ${UNTOUCHED}"
676684
a_boolean: true
677685
a_number: 42
678686
a_yaml:
679687
key: value
680688
another_yaml:
681689
"this.will.not.be.expanded": "${nr-var:change.me.string}" # A variable inside another other variable value is not expanded
682690
string_key: "here, the value key: value\n is encoded as string because it is not alone"
691+
last_one: ${UNTOUCHED}
683692
"#, // FIXME? Note line above, the "key: value\n" part was replaced!!
684693
)
685694
.unwrap();
@@ -943,19 +952,23 @@ mod tests {
943952
let neither_value_nor_default =
944953
VariableDefinition::new(String::default(), true, None::<String>, None::<String>);
945954

946-
let re = template_re();
947955
assert_eq!(
948956
"Value-${nr-var:other}".to_string(),
949-
replace(re, "${nr-var:any}-${nr-var:other}", "any", &value_var).unwrap()
957+
replace("${nr-var:any}", "${nr-var:any}-${nr-var:other}", &value_var).unwrap()
950958
);
951959
assert_eq!(
952960
"Default-${nr-var:other}".to_string(),
953-
replace(re, "${nr-var:any}-${nr-var:other}", "any", &default_var).unwrap()
961+
replace(
962+
"${nr-var:any}",
963+
"${nr-var:any}-${nr-var:other}",
964+
&default_var
965+
)
966+
.unwrap()
954967
);
955968
let key = assert_matches!(
956-
replace(re, "${nr-var:any}-x", "any", &neither_value_nor_default).unwrap_err(),
969+
replace("${nr-var:any}", "${nr-var:any}-x", &neither_value_nor_default).unwrap_err(),
957970
AgentTypeError::MissingTemplateKey(s) => s);
958-
assert_eq!("any".to_string(), key);
971+
assert_eq!("${nr-var:any}".to_string(), key);
959972
}
960973

961974
#[test]

0 commit comments

Comments
 (0)