Skip to content

Commit bde79d2

Browse files
DavSanchezDavSanchez
authored
refactor: rework AgentID type (#1561)
* refactor: rework agent_id type * fix: remove unused agent id * style: merge String and &str impls
1 parent 3382c26 commit bde79d2

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

58 files changed

+303
-302
lines changed

‎agent-control/src/agent_control.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ where
157157
(self.health_checker_builder)(self.start_time).map(|health_checker| {
158158
debug!("Starting Agent Control health-checker");
159159
spawn_health_checker(
160-
AgentID::new_agent_control_id(),
160+
AgentID::AgentControl,
161161
health_checker,
162162
self.agent_control_internal_publisher.clone(),
163163
self.initial_config.health_check.interval,
@@ -668,7 +668,7 @@ agents:
668668
let config = self
669669
.dyn_config_store
670670
.values_repository
671-
.get_remote_config(&AgentID::new_agent_control_id())
671+
.get_remote_config(&AgentID::AgentControl)
672672
.expect("Error getting the remote config for Agent control")
673673
.expect("No remote config found for Agent Control");
674674
f(config)
@@ -683,7 +683,7 @@ agents:
683683
fn build_ac_remote_config(&self, s: &str) -> OpampRemoteConfig {
684684
let hash = Hash::new(s);
685685
OpampRemoteConfig::new(
686-
AgentID::new_agent_control_id(),
686+
AgentID::AgentControl,
687687
hash,
688688
ConfigState::Applying,
689689
Some(ConfigurationMap::new(HashMap::from([(
@@ -748,7 +748,7 @@ agents:
748748
values
749749
.into_iter()
750750
.map(|(id, at)| {
751-
let agent_id = AgentID::new(id).unwrap();
751+
let agent_id = AgentID::try_from(id).unwrap();
752752
let agent_type_id = AgentTypeID::try_from(at).unwrap();
753753
AgentIdentity {
754754
id: agent_id,
@@ -832,7 +832,7 @@ agents:
832832
// store as local
833833
self.sa_dynamic_config_store
834834
.values_repository
835-
.store_local(&AgentID::new_agent_control_id(), &as_yaml)
835+
.store_local(&AgentID::AgentControl, &as_yaml)
836836
.unwrap();
837837
// load the dynamic part
838838
let dyn_config = self.sa_dynamic_config_store.load().unwrap();

‎agent-control/src/agent_control/agent_id.rs‎

Lines changed: 38 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,22 @@
11
use crate::agent_control::defaults::AGENT_CONTROL_ID;
22
use serde::{Deserialize, Serialize};
33
use std::fmt::Display;
4-
use std::ops::Deref;
54
use std::path::Path;
65
use thiserror::Error;
76

87
const AGENT_ID_MAX_LENGTH: usize = 32;
98

109
#[derive(Debug, Deserialize, Serialize, PartialEq, Clone, Hash, Eq)]
1110
#[serde(try_from = "String")]
11+
#[serde(into = "String")]
1212
/// AgentID is a unique identifier for any agent, including agent-control.
1313
/// It must contain 32 characters at most, contain alphanumeric characters or dashes only,
1414
/// start with alphabetic, and end with alphanumeric,
1515
/// following [RFC 1035 Label names](https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#rfc-1035-label-names).
16-
pub struct AgentID(String);
16+
pub enum AgentID {
17+
AgentControl,
18+
SubAgent(String),
19+
}
1720

1821
#[derive(Error, Debug)]
1922
pub enum AgentIDError {
@@ -26,19 +29,13 @@ pub enum AgentIDError {
2629
}
2730

2831
impl AgentID {
29-
pub fn new(str: &str) -> Result<Self, AgentIDError> {
30-
Self::try_from(str.to_string())
31-
}
32-
// For agent control ID we need to skip validation
33-
pub fn new_agent_control_id() -> Self {
34-
Self(AGENT_CONTROL_ID.to_string())
35-
}
36-
pub fn get(&self) -> String {
37-
String::from(&self.0)
38-
}
39-
pub fn is_agent_control_id(&self) -> bool {
40-
self.0.eq(AGENT_CONTROL_ID)
32+
pub fn as_str(&self) -> &str {
33+
match self {
34+
AgentID::AgentControl => AGENT_CONTROL_ID,
35+
AgentID::SubAgent(id) => id,
36+
}
4137
}
38+
4239
/// Checks if a string reference has valid format to build an [AgentID].
4340
/// It follows [RFC 1035 Label names](https://kubernetes.io/docs/concepts/overview/working-with-objects/names/#rfc-1035-label-names),
4441
/// and sets a shorter maximum length to avoid issues when the agent-id is used to compose names.
@@ -53,37 +50,43 @@ impl AgentID {
5350

5451
impl TryFrom<String> for AgentID {
5552
type Error = AgentIDError;
56-
fn try_from(str: String) -> Result<Self, Self::Error> {
57-
if str.eq(AGENT_CONTROL_ID) {
58-
return Err(AgentIDError::Reserved(AGENT_CONTROL_ID.to_string()));
59-
}
60-
61-
if AgentID::is_valid_format(&str) {
62-
Ok(AgentID(str))
53+
fn try_from(input: String) -> Result<Self, Self::Error> {
54+
if input.eq_ignore_ascii_case(AGENT_CONTROL_ID) {
55+
Err(AgentIDError::Reserved(input))
56+
} else if AgentID::is_valid_format(&input) {
57+
Ok(AgentID::SubAgent(input))
6358
} else {
6459
Err(AgentIDError::InvalidFormat)
6560
}
6661
}
6762
}
6863

69-
impl Deref for AgentID {
70-
type Target = str;
71-
72-
fn deref(&self) -> &Self::Target {
73-
&self.0
64+
impl TryFrom<&str> for AgentID {
65+
type Error = AgentIDError;
66+
fn try_from(input: &str) -> Result<Self, Self::Error> {
67+
Self::try_from(input.to_string())
7468
}
7569
}
7670

77-
impl AsRef<Path> for AgentID {
78-
fn as_ref(&self) -> &Path {
79-
// TODO: define how AgentID should be converted to a Path here.
80-
Path::new(&self.0)
71+
impl From<AgentID> for String {
72+
fn from(val: AgentID) -> Self {
73+
match val {
74+
AgentID::AgentControl => AGENT_CONTROL_ID.to_string(),
75+
AgentID::SubAgent(id) => id,
76+
}
8177
}
8278
}
8379

8480
impl Display for AgentID {
8581
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
86-
write!(f, "{}", self.0.as_str())
82+
write!(f, "{}", self.as_str())
83+
}
84+
}
85+
86+
impl AsRef<Path> for AgentID {
87+
fn as_ref(&self) -> &Path {
88+
// TODO: define how AgentID should be converted to a Path here.
89+
Path::new(self.as_str())
8790
}
8891
}
8992

@@ -94,17 +97,16 @@ pub(crate) mod tests {
9497

9598
impl Default for AgentID {
9699
fn default() -> Self {
97-
AgentID::new("default").unwrap()
100+
AgentID::try_from("default").unwrap()
98101
}
99102
}
100103

101104
#[test]
102105
fn agent_control_id() {
103-
let agent_id = AgentID::new_agent_control_id();
104-
assert_eq!(agent_id.get(), AGENT_CONTROL_ID);
105-
assert!(agent_id.is_agent_control_id());
106+
let agent_id = AgentID::AgentControl;
107+
assert_eq!(agent_id.as_str(), AGENT_CONTROL_ID);
106108

107-
AgentID::new(AGENT_CONTROL_ID).unwrap_err();
109+
AgentID::try_from(AGENT_CONTROL_ID).unwrap_err();
108110
}
109111
#[test]
110112
fn agent_id_validator() {

‎agent-control/src/agent_control/config.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -648,7 +648,7 @@ k8s:
648648
#[test]
649649
fn test_sub_agent_removal_diff_with_removal() {
650650
let old_sub_agents = helper_get_agent_list();
651-
let agent_id_to_remove = AgentID::new("infra-agent").unwrap();
651+
let agent_id_to_remove = AgentID::try_from("infra-agent").unwrap();
652652
let mut new_sub_agents = old_sub_agents.clone();
653653
new_sub_agents.remove(&agent_id_to_remove);
654654

@@ -668,14 +668,14 @@ k8s:
668668

669669
assert_eq!(diff.len(), 2);
670670
assert!(diff.contains(&(
671-
&AgentID::new("infra-agent").unwrap(),
671+
&AgentID::try_from("infra-agent").unwrap(),
672672
&SubAgentConfig {
673673
agent_type:
674674
AgentTypeID::try_from("newrelic/com.newrelic.infrastructure:0.0.1").unwrap(),
675675
},
676676
)));
677677
assert!(diff.contains(&(
678-
&AgentID::new("nrdot").unwrap(),
678+
&AgentID::try_from("nrdot").unwrap(),
679679
&SubAgentConfig {
680680
agent_type:
681681
AgentTypeID::try_from("newrelic/io.opentelemetry.collector:0.0.1").unwrap(),
@@ -699,7 +699,7 @@ k8s:
699699
////////////////////////////////////////////////////////////////////////////////////
700700

701701
pub fn infra_identity() -> AgentIdentity {
702-
let id = AgentID::new("infra-agent").unwrap();
702+
let id = AgentID::try_from("infra-agent").unwrap();
703703
let agent_type_id =
704704
AgentTypeID::try_from("newrelic/com.newrelic.infrastructure:0.0.1").unwrap();
705705
AgentIdentity { id, agent_type_id }
@@ -716,7 +716,7 @@ k8s:
716716
}
717717

718718
pub fn nrdot_identity() -> AgentIdentity {
719-
let id = AgentID::new("nrdot").unwrap();
719+
let id = AgentID::try_from("nrdot").unwrap();
720720
let agent_type_id =
721721
AgentTypeID::try_from("newrelic/io.opentelemetry.collector:0.0.1").unwrap();
722722
AgentIdentity { id, agent_type_id }

‎agent-control/src/agent_control/config_repository/repository.rs‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,9 @@ pub(crate) mod tests {
6767

6868
impl AgentControlDynamicConfigRepository for InMemoryAgentControlDynamicConfigRepository {
6969
fn load(&self) -> Result<AgentControlDynamicConfig, AgentControlConfigError> {
70-
let config = self.values_repository.load_remote_fallback_local(
71-
&AgentID::new_agent_control_id(),
72-
&Capabilities::default(),
73-
)?;
70+
let config = self
71+
.values_repository
72+
.load_remote_fallback_local(&AgentID::AgentControl, &Capabilities::default())?;
7473
Ok(config
7574
.unwrap_or_default()
7675
.get_yaml_config()
@@ -81,25 +80,25 @@ pub(crate) mod tests {
8180
fn store(&self, config: &RemoteConfig) -> Result<(), AgentControlConfigError> {
8281
Ok(self
8382
.values_repository
84-
.store_remote(&AgentID::new_agent_control_id(), config)?)
83+
.store_remote(&AgentID::AgentControl, config)?)
8584
}
8685

8786
fn update_state(&self, state: ConfigState) -> Result<(), AgentControlConfigError> {
8887
Ok(self
8988
.values_repository
90-
.update_state(&AgentID::new_agent_control_id(), state)?)
89+
.update_state(&AgentID::AgentControl, state)?)
9190
}
9291

9392
fn get_remote_config(&self) -> Result<Option<RemoteConfig>, AgentControlConfigError> {
9493
Ok(self
9594
.values_repository
96-
.get_remote_config(&AgentID::new_agent_control_id())?)
95+
.get_remote_config(&AgentID::AgentControl)?)
9796
}
9897

9998
fn delete(&self) -> Result<(), AgentControlConfigError> {
10099
Ok(self
101100
.values_repository
102-
.delete_remote(&AgentID::new_agent_control_id())?)
101+
.delete_remote(&AgentID::AgentControl)?)
103102
}
104103
}
105104
}

‎agent-control/src/agent_control/config_repository/store.rs‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ where
7878
Self {
7979
config_builder,
8080
values_repository,
81-
agent_control_id: AgentID::new_agent_control_id(),
81+
agent_control_id: AgentID::AgentControl,
8282
agent_control_capabilities: default_capabilities(),
8383
}
8484
}
@@ -190,7 +190,7 @@ pub(crate) mod tests {
190190
.unwrap();
191191

192192
config_repository
193-
.store_local(&AgentID::new_agent_control_id(), &local_config)
193+
.store_local(&AgentID::AgentControl, &local_config)
194194
.unwrap();
195195

196196
let store = AgentControlConfigStore::new(Arc::new(config_repository));
@@ -216,7 +216,7 @@ pub(crate) mod tests {
216216
.unwrap();
217217

218218
config_repository
219-
.store_local(&AgentID::new_agent_control_id(), &local_config)
219+
.store_local(&AgentID::AgentControl, &local_config)
220220
.unwrap();
221221
let remote_config: YAMLConfig = r#"
222222
agents:
@@ -228,10 +228,7 @@ pub(crate) mod tests {
228228
.unwrap();
229229

230230
config_repository
231-
.store_remote(
232-
&AgentID::new_agent_control_id(),
233-
&remote_config.clone().into(),
234-
)
231+
.store_remote(&AgentID::AgentControl, &remote_config.clone().into())
235232
.unwrap();
236233

237234
let store = AgentControlConfigStore::new(Arc::new(config_repository));
@@ -240,7 +237,7 @@ pub(crate) mod tests {
240237
let expected = AgentControlConfig {
241238
dynamic: AgentControlDynamicConfig {
242239
agents: HashMap::from([(
243-
AgentID::new("rolldice").unwrap(),
240+
AgentID::try_from("rolldice").unwrap(),
244241
SubAgentConfig {
245242
agent_type: AgentTypeID::try_from("namespace/name:0.0.2").unwrap(),
246243
},
@@ -267,7 +264,7 @@ pub(crate) mod tests {
267264
.unwrap();
268265

269266
config_repository
270-
.store_local(&AgentID::new_agent_control_id(), &local_config)
267+
.store_local(&AgentID::AgentControl, &local_config)
271268
.unwrap();
272269

273270
// We set the environment variable with the `__` separator which will create the nested
@@ -281,7 +278,7 @@ pub(crate) mod tests {
281278
let expected = AgentControlConfig {
282279
dynamic: AgentControlDynamicConfig {
283280
agents: HashMap::from([(
284-
AgentID::new("rolldice").unwrap(),
281+
AgentID::try_from("rolldice").unwrap(),
285282
SubAgentConfig {
286283
agent_type: AgentTypeID::try_from("namespace/name:0.0.2").unwrap(),
287284
},
@@ -312,7 +309,7 @@ pub(crate) mod tests {
312309
.unwrap();
313310

314311
config_repository
315-
.store_local(&AgentID::new_agent_control_id(), &local_config)
312+
.store_local(&AgentID::AgentControl, &local_config)
316313
.unwrap();
317314

318315
// We set the environment variable with the `__` separator which will create the nested
@@ -326,7 +323,7 @@ pub(crate) mod tests {
326323
let expected = AgentControlConfig {
327324
dynamic: AgentControlDynamicConfig {
328325
agents: HashMap::from([(
329-
AgentID::new("overrideme").unwrap(),
326+
AgentID::try_from("overrideme").unwrap(),
330327
SubAgentConfig {
331328
agent_type: AgentTypeID::try_from("namespace/from.env:0.0.2").unwrap(),
332329
},
@@ -356,7 +353,7 @@ pub(crate) mod tests {
356353
.unwrap();
357354

358355
config_repository
359-
.store_local(&AgentID::new_agent_control_id(), &local_config)
356+
.store_local(&AgentID::AgentControl, &local_config)
360357
.unwrap();
361358

362359
let store = AgentControlConfigStore::new(Arc::new(config_repository));

‎agent-control/src/agent_control/http_server/status.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -382,18 +382,18 @@ pub mod tests {
382382
sub_agents: SubAgentsStatus(
383383
[
384384
(
385-
AgentID::new("agent-id-1").unwrap(),
385+
AgentID::try_from("agent-id-1").unwrap(),
386386
SubAgentStatus {
387-
agent_id: AgentID::new("agent-id-1").unwrap(),
387+
agent_id: AgentID::try_from("agent-id-1").unwrap(),
388388
agent_type: AgentTypeID::try_from("ns/some.type:1.2.3").unwrap(),
389389
agent_start_time_unix_nano: 0,
390390
health_info: None,
391391
},
392392
),
393393
(
394-
AgentID::new("agent-id-2").unwrap(),
394+
AgentID::try_from("agent-id-2").unwrap(),
395395
SubAgentStatus {
396-
agent_id: AgentID::new("agent-id-2").unwrap(),
396+
agent_id: AgentID::try_from("agent-id-2").unwrap(),
397397
agent_type: AgentTypeID::try_from("ns/some.type:1.2.3").unwrap(),
398398
agent_start_time_unix_nano: 0,
399399
health_info: Some(HealthInfo {
@@ -406,9 +406,9 @@ pub mod tests {
406406
},
407407
),
408408
(
409-
AgentID::new("agent-id-3").unwrap(),
409+
AgentID::try_from("agent-id-3").unwrap(),
410410
SubAgentStatus {
411-
agent_id: AgentID::new("agent-id-3").unwrap(),
411+
agent_id: AgentID::try_from("agent-id-3").unwrap(),
412412
agent_type: AgentTypeID::try_from("ns/some.type:1.2.3").unwrap(),
413413
agent_start_time_unix_nano: 0,
414414
health_info: Some(HealthInfo {

0 commit comments

Comments
 (0)