feat: support multiple configs (NR-424837) (#1708)

* feat: support multiple configs

* refactor: remove optional configuration in OpampRemoteConfig

Author: gsanchezgavier

agent-control/src/agent_control.rs

@@ -431,7 +431,7 @@ where
             .into());
         }
 
-        let remote_config_value = opamp_remote_config.get_unique()?;
+        let remote_config_value = opamp_remote_config.get_default()?;
 
         let new_dynamic_config = if remote_config_value.is_empty() {
             // Use the local configuration if the content of the remote config is empty.
@@ -583,7 +583,9 @@ mod tests {
     use crate::health::with_start_time::HealthWithStartTime;
     use crate::opamp::client_builder::tests::MockStartedOpAMPClient;
     use crate::opamp::remote_config::hash::{ConfigState, Hash};
-    use crate::opamp::remote_config::{ConfigurationMap, OpampRemoteConfig};
+    use crate::opamp::remote_config::{
+        ConfigurationMap, DEFAULT_AGENT_CONFIG_IDENTIFIER, OpampRemoteConfig,
+    };
     use crate::sub_agent::collection::StartedSubAgents;
     use crate::sub_agent::error::SubAgentBuilderError;
     use crate::sub_agent::identity::AgentIdentity;
@@ -705,10 +707,10 @@ agents:
                 AgentID::AgentControl,
                 hash,
                 ConfigState::Applying,
-                Some(ConfigurationMap::new(HashMap::from([(
-                    "".to_string(),
+                ConfigurationMap::new(HashMap::from([(
+                    DEFAULT_AGENT_CONFIG_IDENTIFIER.to_string(),
                     s.to_string(),
-                )]))),
+                )])),
             )
         }
 

agent-control/src/opamp/callbacks.rs

@@ -91,20 +91,20 @@ where
             }
         };
 
-        let config_map: Option<ConfigurationMap> = match msg_remote_config.config {
+        let config_map: ConfigurationMap = match msg_remote_config.config {
             Some(msg_config_map) => msg_config_map
                 .try_into()
                 .inspect_err(|err: &OpampRemoteConfigError| {
                     state = ConfigState::Failed {
                         error_message: format!("Invalid remote config format: {err}"),
                     };
                 })
-                .ok(),
+                .unwrap_or_default(),
             None => {
                 state = ConfigState::Failed {
                     error_message: "Config missing".into(),
                 };
-                None
+                ConfigurationMap::default()
             }
         };
 
@@ -264,7 +264,9 @@ pub(crate) mod tests {
     use crate::opamp::remote_config::signature::{
         ED25519, SIGNATURE_CUSTOM_CAPABILITY, SIGNATURE_CUSTOM_MESSAGE_TYPE,
     };
-    use crate::opamp::remote_config::{ConfigurationMap, OpampRemoteConfig};
+    use crate::opamp::remote_config::{
+        ConfigurationMap, DEFAULT_AGENT_CONFIG_IDENTIFIER, OpampRemoteConfig,
+    };
     use opamp_client::opamp::proto::{AgentConfigFile, AgentConfigMap, AgentRemoteConfig};
     use std::collections::HashMap;
     use std::time::Duration;
@@ -338,15 +340,15 @@ pub(crate) mod tests {
         let (valid_remote_config_map, expected_remote_config_map) = (
             AgentConfigMap {
                 config_map: HashMap::from([(
-                    "my-config".to_string(),
+                    DEFAULT_AGENT_CONFIG_IDENTIFIER.to_string(),
                     AgentConfigFile {
                         body: "enable_proces_metrics: true".as_bytes().to_vec(),
                         content_type: "".to_string(),
                     },
                 )]),
             },
             ConfigurationMap::new(HashMap::from([(
-                "my-config".to_string(),
+                DEFAULT_AGENT_CONFIG_IDENTIFIER.to_string(),
                 "enable_proces_metrics: true".to_string(),
             )])),
         );
@@ -356,7 +358,7 @@ pub(crate) mod tests {
             opamp_msg: Option<MessageData>, // using option here to allow taking the ownership of the MessageData which cannot be cloned.
             expected_remote_config_hash: Hash,
             expected_remote_config_state: ConfigState,
-            expected_remote_config_config_map: Option<ConfigurationMap>,
+            expected_remote_config_config_map: ConfigurationMap,
             expected_signature: Option<Signatures>,
         }
         impl TestCase {
@@ -403,7 +405,7 @@ pub(crate) mod tests {
                     }),
                     ..Default::default()
                 }),
-                expected_remote_config_config_map: Some(expected_remote_config_map.clone()),
+                expected_remote_config_config_map: expected_remote_config_map.clone(),
                 expected_remote_config_hash: Hash::from(valid_hash),
                 expected_remote_config_state: ConfigState::Applying,
                 expected_signature: None,
@@ -425,7 +427,7 @@ pub(crate) mod tests {
                     }),
                     ..Default::default()
                 }),
-                expected_remote_config_config_map: None,
+                expected_remote_config_config_map: ConfigurationMap::default(),
                 expected_remote_config_hash:  Hash::from(valid_hash),
                 expected_remote_config_state: ConfigState::Failed {
                     error_message: "Invalid remote config format: invalid UTF-8 sequence: `invalid utf-8 sequence of 1 bytes from index 0`".into(),
@@ -441,7 +443,7 @@ pub(crate) mod tests {
                     }),
                     ..Default::default()
                 }),
-                expected_remote_config_config_map: None,
+                expected_remote_config_config_map: ConfigurationMap::default(),
                 expected_remote_config_hash: Hash::from(valid_hash),
                 expected_remote_config_state: ConfigState::Failed {
                     error_message: "Config missing".into(),
@@ -457,7 +459,7 @@ pub(crate) mod tests {
                     }),
                     ..Default::default()
                 }),
-                expected_remote_config_config_map: None,
+                expected_remote_config_config_map: ConfigurationMap::default(),
                 expected_remote_config_hash: Hash::default(),
                 expected_remote_config_state: ConfigState::Failed {
                     error_message: "Config missing".into(),
@@ -473,7 +475,7 @@ pub(crate) mod tests {
                     }),
                     ..Default::default()
                 }),
-                expected_remote_config_config_map: Some(expected_remote_config_map.clone()),
+                expected_remote_config_config_map: expected_remote_config_map.clone(),
                 expected_remote_config_hash: Hash::default(),
                 expected_remote_config_state: ConfigState::Failed {
                     error_message: "Invalid hash: invalid utf-8 sequence of 1 bytes from index 0".into(),
@@ -490,22 +492,23 @@ pub(crate) mod tests {
                     custom_message: Some(CustomMessage {
                         capability: SIGNATURE_CUSTOM_CAPABILITY.to_string(),
                         r#type: SIGNATURE_CUSTOM_MESSAGE_TYPE.to_string(),
-                        data: r#"{
-                            "unique": [{
+                        data: serde_json::json!({
+                            DEFAULT_AGENT_CONFIG_IDENTIFIER: [{
                                 "signature": "fake config",
                                 "signingAlgorithm": "ED25519",
                                 "keyId": "fake keyid"
                             }]
-                        }"#
+                        })
+                        .to_string()
                         .as_bytes()
                         .to_vec(),
                     }),
                     ..Default::default()
                 }),
-                expected_remote_config_config_map: Some(expected_remote_config_map.clone()),
+                expected_remote_config_config_map: expected_remote_config_map.clone(),
                 expected_remote_config_hash: Hash::from(valid_hash),
                 expected_remote_config_state: ConfigState::Applying,
-                expected_signature: Some(Signatures::new_unique(
+                expected_signature: Some(Signatures::new_default(
                     "fake config",
                     ED25519,
                     "fake keyid",
@@ -533,7 +536,7 @@ pub(crate) mod tests {
                     }),
                     ..Default::default()
                 }),
-                expected_remote_config_config_map: Some(expected_remote_config_map.clone()),
+                expected_remote_config_config_map: expected_remote_config_map.clone(),
                 expected_remote_config_hash: Hash::from(valid_hash),
                 expected_remote_config_state: ConfigState::Applying,
                 expected_signature: None,
@@ -560,7 +563,7 @@ pub(crate) mod tests {
                     }),
                     ..Default::default()
                 }),
-                expected_remote_config_config_map: Some(expected_remote_config_map.clone()),
+                expected_remote_config_config_map: expected_remote_config_map.clone(),
                 expected_remote_config_hash: Hash::from(valid_hash),
                 expected_remote_config_state: ConfigState::Applying,
                 expected_signature: None,
@@ -579,7 +582,7 @@ pub(crate) mod tests {
                     }),
                     ..Default::default()
                 }),
-                expected_remote_config_config_map: Some(expected_remote_config_map.clone()),
+                expected_remote_config_config_map: expected_remote_config_map.clone(),
                 expected_remote_config_hash: Hash::from(valid_hash),
                 expected_remote_config_state: ConfigState::Failed {
                     error_message: "Invalid remote config signature format: expected value at line 1 column 1".into(),

agent-control/src/opamp/remote_config.rs

@@ -12,6 +12,9 @@ pub mod report;
 pub mod signature;
 pub mod validators;
 
+/// Identifier key for the primary agent configuration within the OpAMP [opamp_client::opamp::proto::AgentConfigMap].
+pub const DEFAULT_AGENT_CONFIG_IDENTIFIER: &str = "configAgent";
+
 /// This structure represents the remote configuration that we would retrieve from a server via OpAMP.
 /// Contains identifying metadata and the actual configuration values
 #[derive(Debug, PartialEq, Clone)]
@@ -20,7 +23,7 @@ pub struct OpampRemoteConfig {
     pub hash: Hash,
     pub state: ConfigState,
     signatures: Option<Signatures>,
-    config_map: Option<ConfigurationMap>,
+    config_map: ConfigurationMap,
 }
 
 #[derive(Error, Debug, Clone, PartialEq)]
@@ -41,7 +44,7 @@ impl OpampRemoteConfig {
         agent_id: AgentID,
         hash: Hash,
         state: ConfigState,
-        config_map: Option<ConfigurationMap>,
+        config_map: ConfigurationMap,
     ) -> Self {
         Self {
             agent_id,
@@ -57,57 +60,32 @@ impl OpampRemoteConfig {
             ..self
         }
     }
-    //TODO : This is temporal as when there is only one conf item we should receive an empty string as key
-    pub fn get_unique(&self) -> Result<&str, OpampRemoteConfigError> {
-        let config_map = self
-            .config_map
-            .as_ref()
-            .ok_or(OpampRemoteConfigError::InvalidConfig(
-                self.hash.to_string(),
-                "missing config".to_string(),
-            ))?;
 
-        match config_map.0.len() {
-            0 => Err(OpampRemoteConfigError::InvalidConfig(
-                self.hash.to_string(),
-                "empty config map".to_string(),
-            )),
-            1 => Ok(config_map
-                .0
-                .values()
-                .next()
-                .expect("at least one config has been provided")),
-            _ => Err(OpampRemoteConfigError::InvalidConfig(
+    /// Get configuration value at the
+    pub fn get_default(&self) -> Result<&str, OpampRemoteConfigError> {
+        self.config_map
+            .0
+            .get(DEFAULT_AGENT_CONFIG_IDENTIFIER)
+            .map(|s| s.as_str())
+            .ok_or(OpampRemoteConfigError::InvalidConfig(
                 self.hash.to_string(),
-                "too many config items".to_string(),
-            )),
-        }
+                "missing default config".to_string(),
+            ))
     }
 
-    // gets the config signature if it exists. It fails if there are multiple signatures.
-    pub fn get_unique_signature(&self) -> Result<Option<SignatureData>, OpampRemoteConfigError> {
+    /// Get the signature data for the default config
+    pub fn get_default_signature(&self) -> Result<Option<SignatureData>, OpampRemoteConfigError> {
         if let Some(signatures) = &self.signatures {
-            match signatures.len() {
-                0 => Err(OpampRemoteConfigError::InvalidConfig(
-                    self.hash.to_string(),
-                    "empty signature".to_string(),
-                )),
-                1 => Ok(Some(
-                    signatures
-                        .iter()
-                        .next()
-                        // assumes that the sigunature corresponds to the unique config item
-                        .map(|(_, signature)| signature.clone())
-                        .ok_or(OpampRemoteConfigError::InvalidConfig(
-                            self.hash.to_string(),
-                            "getting unique signature".to_string(),
-                        ))?,
-                )),
-                _ => Err(OpampRemoteConfigError::InvalidConfig(
-                    self.hash.to_string(),
-                    "too many signature items".to_string(),
-                )),
-            }
+            Ok(Some(
+                signatures
+                    .signatures
+                    .get(DEFAULT_AGENT_CONFIG_IDENTIFIER)
+                    .cloned()
+                    .ok_or(OpampRemoteConfigError::InvalidConfig(
+                        self.hash.to_string(),
+                        "missing signature for default config".to_string(),
+                    ))?,
+            ))
         } else {
             // Agent control config is not signed
             Ok(None)

agent-control/src/opamp/remote_config/signature.rs

@@ -143,7 +143,7 @@ fn parse_rsa_algorithm(algo: &str) -> Option<SigningAlgorithm> {
 ///     capability: "com.newrelic.security.configSignature"
 ///     type: "newrelicRemoteConfigSignature"
 ///     data: {
-///           "3936250589": [{
+///           "agentConfig": [{
 ///                 "checksum":  "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08",
 ///                 "checksumAlgorithm":  "SHA256",
 ///                 "signature":  "nppw2CuZg+YO5MsEoNOsHlgHxF7qAwWPli37NGXAr5isfP1jUTSJcLi0l7k9lNlpbq31GF9DZ0JQBZhoGS0j+sDjvirKSb7yXdqj6JcZ8sxax7KWAnk5QPiwLHFA1kGmszVJ/ccbwtVozG46FvKedcc3X5RME/HGdJupKBe3UzmJawL0xs9jNY+9519CL+CpbkBl/WgCvrIUhTNZv5TUHK23hMD+kz1Brf60pW7MQVtsyClOllsb6WhAsSXdhkpSCJ+96ZGyYywUlvx3/vkBM5a7q4IWqiPM4U0LPZDMQJQCCpxWV3T7cnIR1Ye2yYUqJHs9vfKmTWeBKH2Tb5FgpQ==",
@@ -167,7 +167,7 @@ fn parse_rsa_algorithm(algo: &str) -> Option<SigningAlgorithm> {
 /// use crate::newrelic_agent_control::opamp::remote_config::signature::Signatures;
 ///
 /// let data= r#"{
-///      "3936250589": [
+///      "agentConfig": [
 ///         {
 ///            "signature":  "some signature",
 ///            "signingAlgorithm": "UNSUPPORTED",
@@ -187,7 +187,7 @@ fn parse_rsa_algorithm(algo: &str) -> Option<SigningAlgorithm> {
 /// }"#.as_bytes().to_vec();
 ///
 /// let signatures: Signatures = serde_json::from_slice(&data).unwrap();
-/// let (_, signature) = signatures.iter().next().unwrap();
+/// let (_, signature) = signatures.signatures.iter().next().unwrap();
 /// assert_eq!(signature.signing_algorithm.as_ref(), "ED25519");
 /// ```
 #[derive(Debug, Serialize, PartialEq, Clone)]
@@ -290,8 +290,8 @@ impl SignatureData {
     }
 }
 
-/// CRC32 checksum of the config data. Currently this field is ignored since the current implementation of RemoteConfig
-/// doesn't support multiple config items.
+/// Configuration identifier that corresponds to a specific remote configuration.
+/// This key links signature data to its associated configuration in the remote config map.
 pub type ConfigID = String;
 
 #[derive(Error, Debug, Clone, PartialEq)]
@@ -306,18 +306,6 @@ pub enum SignatureError {
     UnsupportedAlgorithm(String),
 }
 
-impl Signatures {
-    pub fn is_empty(&self) -> bool {
-        self.signatures.is_empty()
-    }
-    pub fn len(&self) -> usize {
-        self.signatures.len()
-    }
-    pub fn iter(&self) -> impl Iterator<Item = (&ConfigID, &SignatureData)> {
-        self.signatures.iter()
-    }
-}
-
 impl TryFrom<&CustomMessage> for Signatures {
     type Error = SignatureError;
 
@@ -340,6 +328,7 @@ impl TryFrom<&CustomMessage> for Signatures {
 mod tests {
     use super::SignatureData;
     use super::Signatures;
+    use crate::opamp::remote_config::DEFAULT_AGENT_CONFIG_IDENTIFIER;
     use crate::opamp::remote_config::signature::ECDSA_P256_SHA256;
     use crate::opamp::remote_config::signature::ECDSA_P256_SHA384;
     use crate::opamp::remote_config::signature::ED25519;
@@ -348,10 +337,10 @@ mod tests {
     use std::collections::HashMap;
 
     impl Signatures {
-        pub fn new_unique(signature: &str, signing_algorithm: &str, key_id: &str) -> Self {
+        pub fn new_default(signature: &str, signing_algorithm: &str, key_id: &str) -> Self {
             Self {
                 signatures: HashMap::from([(
-                    "unique".to_string(),
+                    DEFAULT_AGENT_CONFIG_IDENTIFIER.to_string(),
                     SignatureData::new(signature, signing_algorithm, key_id),
                 )]),
             }
@@ -388,7 +377,7 @@ mod tests {
             fn run(self) {
                 let signatures = Signatures::try_from(&self.custom_message)
                     .unwrap_or_else(|err| panic!("case: {} - {}", self.name, err));
-                let (_, signature) = signatures.iter().next().unwrap();
+                let (_, signature) = signatures.signatures.iter().next().unwrap();
                 assert_eq!(signature.signing_algorithm, self.algorithm);
             }
         }