From 6890db3bd6a74495a075c6f796b26366f4b819c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Felipe=20=C3=81lvarez?= Date: Wed, 8 Apr 2026 17:49:04 +0200 Subject: [PATCH 1/9] chore: prepare system-identity cli code to nr-auth update --- .../src/cli/common/system_identity.rs | 111 +++++++++--------- agent-control/src/cli/k8s/system_identity.rs | 36 ++++-- agent-control/src/cli/on_host/config_gen.rs | 45 ++++--- 3 files changed, 112 insertions(+), 80 deletions(-) diff --git a/agent-control/src/cli/common/system_identity.rs b/agent-control/src/cli/common/system_identity.rs index 476e7190e1..36de4667a8 100644 --- a/agent-control/src/cli/common/system_identity.rs +++ b/agent-control/src/cli/common/system_identity.rs @@ -66,11 +66,25 @@ pub struct SystemIdentityArgs { pub organization_id: String, } +/// Valid data to create a SystemIdentity, represent [SystemIdentityArgs] after validation. +#[derive(Debug)] +pub struct SystemIdentitySpec { + pub system_identity_data: SystemIdentityData, + pub private_key_path: PathBuf, +} + +/// Defines whereas a SystemIdentity already exists or needs to be provisioned +#[derive(Debug)] +pub enum SystemIdentityData { + /// The Identity already exists + Existing { auth_client_id: String }, + /// The identity needs to be provisioned + Provision(ProvisioningMethod), +} + +/// Defines the supported provisioning methods System Identities #[derive(Debug)] pub enum ProvisioningMethod { - ExistingIdentity { - auth_client_id: String, - }, ParentToken { token: String, organization_id: String, @@ -82,13 +96,6 @@ pub enum ProvisioningMethod { }, } -/// Valid data to create a SystemIdentity, represent [SystemIdentityArgs] after validation. -#[derive(Debug)] -pub struct SystemIdentitySpec { - pub method: ProvisioningMethod, - pub private_key_path: PathBuf, -} - impl SystemIdentityArgs { /// Performs additional args validation (not covered by clap's arguments) and returns [SystemIdentityData] if /// validation was Ok. @@ -101,42 +108,49 @@ impl SystemIdentityArgs { })? .clone(); - let method = self.resolve_provisioning_method(&private_key_path)?; + let system_identity_data = self.resolve_provisioning_method(&private_key_path)?; Ok(SystemIdentitySpec { - method, + system_identity_data, private_key_path, }) } - fn resolve_provisioning_method(self, key_path: &Path) -> Result { + fn resolve_provisioning_method( + self, + private_key_path: &Path, + ) -> Result { if !self.auth_client_id.is_empty() { - if !key_path.exists() { + if !private_key_path.exists() { return Err( "when 'auth_client_id' is provided the 'auth_private_key_path' must also be provided and exist" .to_string(), ); } - return Ok(ProvisioningMethod::ExistingIdentity { + return Ok(SystemIdentityData::Existing { auth_client_id: self.auth_client_id, }); } if !self.auth_parent_token.is_empty() { self.require_org_and_parent_id("token based")?; - return Ok(ProvisioningMethod::ParentToken { - token: self.auth_parent_token, - organization_id: self.organization_id, - }); + return Ok(SystemIdentityData::Provision( + ProvisioningMethod::ParentToken { + token: self.auth_parent_token, + organization_id: self.organization_id, + }, + )); } if !self.auth_parent_client_secret.is_empty() { self.require_org_and_parent_id("client-secret based")?; - return Ok(ProvisioningMethod::ParentSecret { - secret: self.auth_parent_client_secret, - parent_client_id: self.auth_parent_client_id, - organization_id: self.organization_id, - }); + return Ok(SystemIdentityData::Provision( + ProvisioningMethod::ParentSecret { + secret: self.auth_parent_client_secret, + parent_client_id: self.auth_parent_client_id, + organization_id: self.organization_id, + }, + )); } Err( @@ -157,27 +171,23 @@ impl SystemIdentityArgs { /// Provides a key-based identity considering the supplied args. It returns the corresponding **client_id** as a String. pub fn provide_identity( - identity_input: &SystemIdentitySpec, + method: &ProvisioningMethod, region: Region, proxy_config: Option, key_creator: impl Creator, ) -> Result { let environment = NewRelicEnvironment::from(region); - build_identity(identity_input, environment, proxy_config, key_creator) + build_identity(method, environment, proxy_config, key_creator) } /// Helper to allow injecting testing urls when building the identity. fn build_identity( - identity_input: &SystemIdentitySpec, + method: &ProvisioningMethod, environment: NewRelicEnvironment, proxy_config: Option, key_creator: impl Creator, ) -> Result { - match &identity_input.method { - ProvisioningMethod::ExistingIdentity { auth_client_id } => { - info!("Using provided System Identity"); - Ok(auth_client_id.to_string()) - } + match &method { ProvisioningMethod::ParentToken { token, organization_id, @@ -407,18 +417,9 @@ pub mod tests { } #[test] - fn test_build_identity_already_provided() { + fn test_identity_already_provided() { let tempdir = TempDir::new().unwrap(); let auth_private_key_path = tempdir.path().join("private-key"); - // Expect no request because the identity was already provided - let environment = NewRelicEnvironment::Custom { - token_renewal_endpoint: "https://should-not-call.this" - .parse() - .expect("url should be valid"), - system_identity_creation_uri: "https://should-not-call.this" - .parse() - .expect("url should be valid"), - }; // Key file must exist when using ExistingIdentity fs::write(&auth_private_key_path, "").unwrap(); let identity_args = SystemIdentityArgs { @@ -428,12 +429,9 @@ pub mod tests { }; let identity_data = identity_args.validate().expect("validation should succeed"); - let mut creator_mock = MockCreator::new(); - creator_mock.expect_create().never(); - - let client_id = build_identity(&identity_data, environment, None, creator_mock) - .expect("no error expected"); - assert_eq!(client_id, "provided_client_id".to_string()); + assert_matches!(identity_data.system_identity_data, SystemIdentityData::Existing { auth_client_id } => { + assert_eq!(auth_client_id, "provided_client_id".to_string()); + }); } #[test] @@ -470,12 +468,15 @@ pub mod tests { }; let creator_mock = MockCreator::expecting_create_ok(); - let identity_data = identity_args.validate().expect("validation should succeed"); - let client_id = build_identity(&identity_data, environment, None, creator_mock) + let identity_spec = identity_args.validate().expect("validation should succeed"); + + assert_matches!(identity_spec.system_identity_data, SystemIdentityData::Provision(method) => { + let client_id = build_identity(&method, environment, None, creator_mock) .expect("no error expected"); + assert_eq!(client_id, "created_client_id".to_string()); + }); identity_mock.assert_calls(1); - assert_eq!(client_id, "created_client_id".to_string()); } #[test] @@ -520,13 +521,15 @@ pub mod tests { }; let creator_mock = MockCreator::expecting_create_ok(); - let identity_data = identity_args.validate().expect("validation should succeed"); - let client_id = build_identity(&identity_data, environment, None, creator_mock) + let identity_spec = identity_args.validate().expect("validation should succeed"); + assert_matches!(identity_spec.system_identity_data, SystemIdentityData::Provision(method) => { + let client_id = build_identity(&method, environment, None, creator_mock) .expect("no error expected"); + assert_eq!(client_id, "created_client_id".to_string()); + }); identity_mock.assert_calls(1); token_mock.assert_calls(1); - assert_eq!(client_id, "created_client_id".to_string()); } fn token_body(token: &str) -> String { diff --git a/agent-control/src/cli/k8s/system_identity.rs b/agent-control/src/cli/k8s/system_identity.rs index 6d4d60740a..6188d1856d 100644 --- a/agent-control/src/cli/k8s/system_identity.rs +++ b/agent-control/src/cli/k8s/system_identity.rs @@ -17,7 +17,10 @@ use crate::{ error::CliError, proxy_config::ProxyConfig, region::{Region, region_parser}, - system_identity::{SystemIdentityArgs, SystemIdentitySpec, provide_identity}, + system_identity::{ + ProvisioningMethod, SystemIdentityArgs, SystemIdentityData, SystemIdentitySpec, + provide_identity, + }, }, k8s::{errors::K8sCliError, utils::try_new_k8s_client}, }, @@ -94,7 +97,7 @@ fn provide_system_identity_secret( ) -> Result<(), K8sCliError> where F: Fn( - &SystemIdentitySpec, + &ProvisioningMethod, Region, Option, PublicKeyHolder, @@ -124,11 +127,22 @@ where )) })?; let pk_holder = PublicKeyHolder { public_key }; + let SystemIdentityData::Provision(provisioning_method) = &spec.identity.system_identity_data + else { + return Err(K8sCliError::Generic( + "existing identity is not supported in the k8s cli".to_string(), + )); + }; - let client_id = provide_identity_fn(&spec.identity, spec.region, spec.proxy_config, pk_holder) - .map_err(|err| { - K8sCliError::Generic(format!("failure registering the System Identity: {err}")) - })?; + let client_id = provide_identity_fn( + provisioning_method, + spec.region, + spec.proxy_config, + pk_holder, + ) + .map_err(|err| { + K8sCliError::Generic(format!("failure registering the System Identity: {err}")) + })?; let private_key = String::from_utf8(private_key).map_err(|err| { K8sCliError::Generic(format!( @@ -216,9 +230,13 @@ mod tests { secret_name: "test-secret".to_string(), region: Region::US, identity: SystemIdentitySpec { - method: ProvisioningMethod::ExistingIdentity { - auth_client_id: "test-client-id".to_string(), - }, + system_identity_data: SystemIdentityData::Provision( + ProvisioningMethod::ParentSecret { + secret: "secret".to_string(), + parent_client_id: "parent_client_id".to_string(), + organization_id: "org_id".to_string(), + }, + ), private_key_path: PathBuf::from("/test/key"), }, proxy_config: None, diff --git a/agent-control/src/cli/on_host/config_gen.rs b/agent-control/src/cli/on_host/config_gen.rs index 5fe5e1661a..89ea829425 100644 --- a/agent-control/src/cli/on_host/config_gen.rs +++ b/agent-control/src/cli/on_host/config_gen.rs @@ -4,7 +4,10 @@ use crate::cli::{ error::CliError, proxy_config::ProxyConfig, region::{Region, region_parser}, - system_identity::{SystemIdentityArgs, SystemIdentitySpec, provide_identity}, + system_identity::{ + ProvisioningMethod, SystemIdentityArgs, SystemIdentityData, SystemIdentitySpec, + provide_identity, + }, }, on_host::config_gen::config::{ AuthConfig, Config, FleetControl, LogConfig, Server, SignatureValidation, @@ -192,7 +195,7 @@ fn generate_config_and_system_identity( ) -> Result where F: Fn( - &SystemIdentitySpec, + &ProvisioningMethod, Region, Option, LocalCreator, @@ -200,18 +203,24 @@ where { let fleet_control = match ¶ms.fleet { FleetParams::FleetDisabled => None, - FleetParams::FleetEnabled { fleet_id, identity } => { + FleetParams::FleetEnabled { + fleet_id, + identity: identity_spec, + } => { let key_creator = LocalCreator::from(KeyPairGeneratorLocalConfig { key_type: KeyType::Rsa4096, - file_path: identity.private_key_path.clone(), + file_path: identity_spec.private_key_path.clone(), }); - let client_id = provide_identity_fn( - identity, - params.region, - params.proxy_config.clone(), - key_creator, - )?; + let client_id = match &identity_spec.system_identity_data { + SystemIdentityData::Existing { auth_client_id } => auth_client_id.to_string(), + SystemIdentityData::Provision(provisioning_method) => provide_identity_fn( + provisioning_method, + params.region, + params.proxy_config.clone(), + key_creator, + )?, + }; Some(FleetControl { endpoint: params.region.opamp_endpoint().to_string(), @@ -223,7 +232,7 @@ where token_url: params.region.token_renewal_endpoint().to_string(), client_id, provider: "local".to_string(), - private_key_path: identity.private_key_path.to_string_lossy().to_string(), + private_key_path: identity_spec.private_key_path.to_string_lossy().to_string(), }, }) } @@ -400,7 +409,7 @@ mod tests { } fn identity_provider_mock( - _: &SystemIdentitySpec, + _: &ProvisioningMethod, _: Region, _: Option, _: LocalCreator, @@ -419,11 +428,13 @@ mod tests { FleetParams::FleetEnabled { fleet_id: "test-fleet-id".to_string(), identity: SystemIdentitySpec { - method: ProvisioningMethod::ParentSecret { - secret: "parent-client-secret".to_string(), - parent_client_id: "parent-client-id".to_string(), - organization_id: "test-org-id".to_string(), - }, + system_identity_data: SystemIdentityData::Provision( + ProvisioningMethod::ParentSecret { + secret: "parent-client-secret".to_string(), + parent_client_id: "parent-client-id".to_string(), + organization_id: "test-org-id".to_string(), + }, + ), private_key_path: PathBuf::from("/path/to/key"), }, } From 84f13f9c74c9ac9c2109c201ebedb46b84580cd7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Felipe=20=C3=81lvarez?= Date: Thu, 9 Apr 2026 10:56:45 +0200 Subject: [PATCH 2/9] chore: bump nr-auth --- Cargo.lock | 4 +- agent-control/Cargo.toml | 2 +- .../src/cli/common/system_identity.rs | 69 +++++-------------- agent-control/src/cli/k8s/system_identity.rs | 38 ++++------ agent-control/src/cli/on_host/config_gen.rs | 56 ++++++++++----- 5 files changed, 73 insertions(+), 96 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bdbea7ed53..23ffbc1e8e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2952,8 +2952,8 @@ checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" [[package]] name = "nr-auth" -version = "0.3.0" -source = "git+https://github.com/newrelic/newrelic-auth-rs.git?tag=0.3.0#b4773eb96fbead5071e3b9acd792461ed7c8cf81" +version = "0.4.0" +source = "git+https://github.com/newrelic/newrelic-auth-rs.git?tag=0.4.0#2d936f25f6950dbec6de2ab39d3b1654711862af" dependencies = [ "base64 0.22.1", "chrono", diff --git a/agent-control/Cargo.toml b/agent-control/Cargo.toml index e0e737427b..2ae930a2e8 100644 --- a/agent-control/Cargo.toml +++ b/agent-control/Cargo.toml @@ -32,7 +32,7 @@ semver = { workspace = true, features = ["serde"] } chrono = { workspace = true } base64 = { version = "0.22.1" } # New Relic dependencies (external repos) -nr-auth = { git = "https://github.com/newrelic/newrelic-auth-rs.git", tag = "0.3.0" } +nr-auth = { git = "https://github.com/newrelic/newrelic-auth-rs.git", tag = "0.4.0" } opamp-client = { git = "https://github.com/newrelic/newrelic-opamp-rs.git", tag = "0.0.36" } # local dependencies fs = { path = "../fs" } diff --git a/agent-control/src/cli/common/system_identity.rs b/agent-control/src/cli/common/system_identity.rs index 36de4667a8..00fdd27e66 100644 --- a/agent-control/src/cli/common/system_identity.rs +++ b/agent-control/src/cli/common/system_identity.rs @@ -9,10 +9,10 @@ use nr_auth::{ TokenRetriever, authenticator::HttpAuthenticator, http::{client::HttpClient, config::HttpConfig}, - key::creator::Creator, + key::generation::PublicKeyPem, system_identity::{ - generator::L2SystemIdentityGenerator, iam_client::http::{HttpIAMClient, IAMAuthCredential}, + identity_creator::L2IdentityCreator, input_data::{SystemIdentityCreationMetadata, environment::NewRelicEnvironment}, }, token::{Token, TokenType}, @@ -73,7 +73,7 @@ pub struct SystemIdentitySpec { pub private_key_path: PathBuf, } -/// Defines whereas a SystemIdentity already exists or needs to be provisioned +/// Defines wether a SystemIdentity already exists or needs to be provisioned #[derive(Debug)] pub enum SystemIdentityData { /// The Identity already exists @@ -82,7 +82,7 @@ pub enum SystemIdentityData { Provision(ProvisioningMethod), } -/// Defines the supported provisioning methods System Identities +/// Defines the supported provisioning methods for System Identities #[derive(Debug)] pub enum ProvisioningMethod { ParentToken { @@ -174,10 +174,10 @@ pub fn provide_identity( method: &ProvisioningMethod, region: Region, proxy_config: Option, - key_creator: impl Creator, + pub_key: PublicKeyPem, ) -> Result { let environment = NewRelicEnvironment::from(region); - build_identity(method, environment, proxy_config, key_creator) + build_identity(method, environment, proxy_config, pub_key) } /// Helper to allow injecting testing urls when building the identity. @@ -185,9 +185,9 @@ fn build_identity( method: &ProvisioningMethod, environment: NewRelicEnvironment, proxy_config: Option, - key_creator: impl Creator, + pub_key: PublicKeyPem, ) -> Result { - match &method { + match method { ProvisioningMethod::ParentToken { token, organization_id, @@ -199,7 +199,7 @@ fn build_identity( organization_id.clone(), environment, http_client, - key_creator, + pub_key, ) } ProvisioningMethod::ParentSecret { @@ -219,7 +219,7 @@ fn build_identity( organization_id.clone(), environment, http_client, - key_creator, + pub_key, ) } } @@ -270,7 +270,7 @@ fn build_identity_from_token( organization_id: String, environment: NewRelicEnvironment, http_client: HttpClient, - key_creator: impl Creator, + pub_key: PublicKeyPem, ) -> Result { let system_identity_creation_metadata = SystemIdentityCreationMetadata { organization_id: organization_id.clone(), @@ -279,15 +279,10 @@ fn build_identity_from_token( }; let iam_client = HttpIAMClient::new(http_client, system_identity_creation_metadata.to_owned()); - let system_identity_generator = L2SystemIdentityGenerator { - iam_client: &iam_client, - key_creator, - }; - - let auth_credential = IAMAuthCredential::BearerToken(token.access_token().to_string()); + let auth_credentials = IAMAuthCredential::BearerToken(token.access_token().to_string()); - let result = system_identity_generator - .generate(&auth_credential) + let result = iam_client + .create_l2_system_identity(&auth_credentials, &pub_key) .map_err(|err| CliError::Command(format!("error generating the system identity: {err}")))?; Ok(result.client_id) @@ -318,38 +313,12 @@ pub mod tests { use assert_matches::assert_matches; use http::header::AUTHORIZATION; use httpmock::{Method::POST, MockServer}; - use mockall::mock; - use nr_auth::key::creator::PublicKeyPem; use rstest::rstest; use std::fs; use tempfile::TempDir; - use thiserror::Error; use super::*; - #[derive(Debug, Error)] - #[error("{0}")] - pub struct TestCreatorError(String); - - mock! { - pub Creator {} - impl Creator for Creator { - type Error = TestCreatorError; - fn create(&self) -> Result; - } - } - impl MockCreator { - fn expecting_create_ok() -> Self { - let mut creator = Self::new(); - creator - .expect_create() - .once() - .returning(|| Ok(PublicKeyPem::default())); - - creator - } - } - #[rstest] #[case::existing_identity(|| SystemIdentityArgs { auth_private_key_path: Some(std::env::current_dir().unwrap()), @@ -467,12 +436,11 @@ pub mod tests { .expect("url should be valid"), }; - let creator_mock = MockCreator::expecting_create_ok(); let identity_spec = identity_args.validate().expect("validation should succeed"); + let pub_key = b"mock-pub-key"; assert_matches!(identity_spec.system_identity_data, SystemIdentityData::Provision(method) => { - let client_id = build_identity(&method, environment, None, creator_mock) - .expect("no error expected"); + let client_id = build_identity(&method, environment, None, pub_key.to_vec()).expect("no error expected"); assert_eq!(client_id, "created_client_id".to_string()); }); @@ -520,11 +488,10 @@ pub mod tests { .expect("url should be valid"), }; - let creator_mock = MockCreator::expecting_create_ok(); let identity_spec = identity_args.validate().expect("validation should succeed"); + let pub_key = b"mock-public-key"; assert_matches!(identity_spec.system_identity_data, SystemIdentityData::Provision(method) => { - let client_id = build_identity(&method, environment, None, creator_mock) - .expect("no error expected"); + let client_id = build_identity(&method, environment, None, pub_key.to_vec()).expect("no error expected"); assert_eq!(client_id, "created_client_id".to_string()); }); diff --git a/agent-control/src/cli/k8s/system_identity.rs b/agent-control/src/cli/k8s/system_identity.rs index 6188d1856d..1e4de6edf5 100644 --- a/agent-control/src/cli/k8s/system_identity.rs +++ b/agent-control/src/cli/k8s/system_identity.rs @@ -1,8 +1,6 @@ -use std::convert::Infallible; - use kube::api::{DynamicObject, ObjectMeta}; use nr_auth::key::{ - creator::{Creator, KeyPair, KeyType, PublicKeyPem}, + generation::{KeyPair, KeyType, PublicKeyPem}, rsa::rsa, }; use serde_json::json; @@ -100,7 +98,7 @@ where &ProvisioningMethod, Region, Option, - PublicKeyHolder, + PublicKeyPem, ) -> Result, { let secret_object_key = K8sObjectKey { @@ -118,6 +116,16 @@ where } info!("Secret is not present, creating system identity"); + // Existing identities cannot be used here because the private key must be stored in the Secret, + // not on local filesystem. + let SystemIdentityData::Provision(provisioning_method) = &spec.identity.system_identity_data + else { + return Err(K8sCliError::Generic( + "the k8s cli requires provisioning a new System Identity; use --auth-parent-token or --auth-parent-client-secret instead of --auth-client-id" + .to_string(), + )); + }; + let KeyPair { private_key, public_key, @@ -126,19 +134,12 @@ where "failure building key-pair for System Identity: {err}" )) })?; - let pk_holder = PublicKeyHolder { public_key }; - let SystemIdentityData::Provision(provisioning_method) = &spec.identity.system_identity_data - else { - return Err(K8sCliError::Generic( - "existing identity is not supported in the k8s cli".to_string(), - )); - }; let client_id = provide_identity_fn( provisioning_method, spec.region, spec.proxy_config, - pk_holder, + public_key, ) .map_err(|err| { K8sCliError::Generic(format!("failure registering the System Identity: {err}")) @@ -201,19 +202,6 @@ fn secret_dynamic_object(object_key: K8sObjectKey<'_>, data: serde_json::Value) } } -/// Helper struct to hold a public key an use it as [Creator] for System Identity provisioning. -struct PublicKeyHolder { - public_key: PublicKeyPem, -} - -impl Creator for PublicKeyHolder { - type Error = Infallible; - - fn create(&self) -> Result { - Ok(self.public_key.clone()) - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/agent-control/src/cli/on_host/config_gen.rs b/agent-control/src/cli/on_host/config_gen.rs index 89ea829425..e106d38bd0 100644 --- a/agent-control/src/cli/on_host/config_gen.rs +++ b/agent-control/src/cli/on_host/config_gen.rs @@ -15,8 +15,8 @@ use crate::cli::{ }; use fs::file::{LocalFile, writer::FileWriter}; use nr_auth::key::{ - creator::KeyType, - local::{KeyPairGeneratorLocalConfig, LocalCreator}, + generation::{KeyType, PublicKeyPem}, + local::{LocalKeyPairGenerator, LocalKeyPairGeneratorConfig}, }; use std::{collections::HashMap, path::PathBuf}; use tracing::info; @@ -198,7 +198,7 @@ where &ProvisioningMethod, Region, Option, - LocalCreator, + PublicKeyPem, ) -> Result, { let fleet_control = match ¶ms.fleet { @@ -207,19 +207,17 @@ where fleet_id, identity: identity_spec, } => { - let key_creator = LocalCreator::from(KeyPairGeneratorLocalConfig { - key_type: KeyType::Rsa4096, - file_path: identity_spec.private_key_path.clone(), - }); - let client_id = match &identity_spec.system_identity_data { SystemIdentityData::Existing { auth_client_id } => auth_client_id.to_string(), - SystemIdentityData::Provision(provisioning_method) => provide_identity_fn( - provisioning_method, - params.region, - params.proxy_config.clone(), - key_creator, - )?, + SystemIdentityData::Provision(provisioning_method) => { + let pub_key = public_key_from_key_pair(identity_spec.private_key_path.clone())?; + provide_identity_fn( + provisioning_method, + params.region, + params.proxy_config.clone(), + pub_key, + )? + } }; Some(FleetControl { @@ -262,6 +260,19 @@ fn default_log_config() -> Option { } } +/// Helper to generate a key pair using the provided path for the private key and return the public key. +fn public_key_from_key_pair(private_key_path: PathBuf) -> Result { + let key_generator = LocalKeyPairGenerator::from(LocalKeyPairGeneratorConfig { + key_type: KeyType::Rsa4096, + file_path: private_key_path, + }); + key_generator.generate().map_err(|err| { + CliError::Command(format!( + "could not generate System Identity's key-pair; {err}" + )) + }) +} + #[cfg(test)] mod tests { use super::*; @@ -271,6 +282,7 @@ mod tests { use clap::{CommandFactory, FromArgMatches}; use rstest::rstest; use std::env::current_dir; + use tempfile::tempdir; impl Default for Params { fn default() -> Self { @@ -354,7 +366,16 @@ mod tests { #[case] proxy_config: Option, #[case] expected: String, ) { - let args = create_test_args(fleet_enabled, region, proxy_config); + let tmp = tempdir().unwrap(); + let private_key_path = tmp.path().join("private_key"); + let args = create_test_args( + fleet_enabled, + region, + proxy_config, + private_key_path.clone(), + ); + // Replacing hardcoded path in expectations because the private-key-path is dynamic + let expected = expected.replace("/path/to/key", &private_key_path.to_string_lossy()); let yaml = generate_config_and_system_identity(&args, identity_provider_mock) .expect("result expected to be OK"); @@ -412,7 +433,7 @@ mod tests { _: &ProvisioningMethod, _: Region, _: Option, - _: LocalCreator, + _: PublicKeyPem, ) -> Result { Ok("test-client-id".to_string()) } @@ -421,6 +442,7 @@ mod tests { fleet_disabled: bool, region: Region, proxy_config: Option, + private_key_path: PathBuf, ) -> Params { let fleet = if fleet_disabled { FleetParams::FleetDisabled @@ -435,7 +457,7 @@ mod tests { organization_id: "test-org-id".to_string(), }, ), - private_key_path: PathBuf::from("/path/to/key"), + private_key_path, }, } }; From 1d368dbc3d9bfad59324d8f7189170763eae5424 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Felipe=20=C3=81lvarez?= Date: Thu, 9 Apr 2026 12:22:35 +0200 Subject: [PATCH 3/9] fix: typo --- agent-control/src/cli/common/system_identity.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent-control/src/cli/common/system_identity.rs b/agent-control/src/cli/common/system_identity.rs index 00fdd27e66..9a68769846 100644 --- a/agent-control/src/cli/common/system_identity.rs +++ b/agent-control/src/cli/common/system_identity.rs @@ -73,7 +73,7 @@ pub struct SystemIdentitySpec { pub private_key_path: PathBuf, } -/// Defines wether a SystemIdentity already exists or needs to be provisioned +/// Defines whether a SystemIdentity already exists or needs to be provisioned #[derive(Debug)] pub enum SystemIdentityData { /// The Identity already exists From 007154060c8114763943313c0c672cc0661ba25e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Felipe=20=C3=81lvarez?= Date: Fri, 10 Apr 2026 09:18:08 +0200 Subject: [PATCH 4/9] chore: split types for system identity provision --- .../src/cli/common/system_identity.rs | 190 ++++-------------- agent-control/src/cli/k8s/system_identity.rs | 95 ++++----- agent-control/src/cli/on_host/config_gen.rs | 105 ++++++++-- 3 files changed, 163 insertions(+), 227 deletions(-) diff --git a/agent-control/src/cli/common/system_identity.rs b/agent-control/src/cli/common/system_identity.rs index 9a68769846..cacc902722 100644 --- a/agent-control/src/cli/common/system_identity.rs +++ b/agent-control/src/cli/common/system_identity.rs @@ -1,9 +1,6 @@ //! This module provides the functions to handle identity creation when setting up Agent Control. //! -use std::{ - path::{Path, PathBuf}, - time::Duration, -}; +use std::{path::PathBuf, time::Duration}; use nr_auth::{ TokenRetriever, @@ -25,7 +22,7 @@ use crate::cli::common::{error::CliError, proxy_config::ProxyConfig, region::Reg const DEFAULT_TIMEOUT: Duration = Duration::from_secs(3); const DEFAULT_RETRIES: u8 = 3; -/// Arguments required to provide or generate a system identity. +/// Arguments required to provision system identity. /// /// **Design note:** modelling the different provisioning methods as subcommands would be /// a more natural fit for clap (each subcommand carrying only its own required fields), @@ -37,16 +34,7 @@ const DEFAULT_RETRIES: u8 = 3; /// credentials, or accept a pre-obtained token) lives here rather than being duplicated /// in each installer. #[derive(Debug, Default, clap::Args)] -pub struct SystemIdentityArgs { - /// Path where the private key is stored or will be written. - #[arg(long)] - pub auth_private_key_path: Option, - - /// Client ID of an already-provisioned system identity. When non-empty, no identity - /// generation is performed. - #[arg(long, default_value_t)] - pub auth_client_id: String, - +pub struct ProvisionIdentityArgs { /// Client ID of the parent system identity, used to obtain a token or as metadata /// during identity generation. #[arg(long, default_value_t)] @@ -66,22 +54,6 @@ pub struct SystemIdentityArgs { pub organization_id: String, } -/// Valid data to create a SystemIdentity, represent [SystemIdentityArgs] after validation. -#[derive(Debug)] -pub struct SystemIdentitySpec { - pub system_identity_data: SystemIdentityData, - pub private_key_path: PathBuf, -} - -/// Defines whether a SystemIdentity already exists or needs to be provisioned -#[derive(Debug)] -pub enum SystemIdentityData { - /// The Identity already exists - Existing { auth_client_id: String }, - /// The identity needs to be provisioned - Provision(ProvisioningMethod), -} - /// Defines the supported provisioning methods for System Identities #[derive(Debug)] pub enum ProvisioningMethod { @@ -96,63 +68,24 @@ pub enum ProvisioningMethod { }, } -impl SystemIdentityArgs { - /// Performs additional args validation (not covered by clap's arguments) and returns [SystemIdentityData] if - /// validation was Ok. - pub fn validate(self) -> Result { - let private_key_path = self - .auth_private_key_path - .as_ref() - .ok_or_else(|| { - "'auth_private_key_path' needs to be set to register System Identity".to_string() - })? - .clone(); - - let system_identity_data = self.resolve_provisioning_method(&private_key_path)?; - - Ok(SystemIdentitySpec { - system_identity_data, - private_key_path, - }) - } - - fn resolve_provisioning_method( - self, - private_key_path: &Path, - ) -> Result { - if !self.auth_client_id.is_empty() { - if !private_key_path.exists() { - return Err( - "when 'auth_client_id' is provided the 'auth_private_key_path' must also be provided and exist" - .to_string(), - ); - } - return Ok(SystemIdentityData::Existing { - auth_client_id: self.auth_client_id, - }); - } - +impl ProvisionIdentityArgs { + pub fn validate(self) -> Result { if !self.auth_parent_token.is_empty() { self.require_org_and_parent_id("token based")?; - return Ok(SystemIdentityData::Provision( - ProvisioningMethod::ParentToken { - token: self.auth_parent_token, - organization_id: self.organization_id, - }, - )); + return Ok(ProvisioningMethod::ParentToken { + token: self.auth_parent_token, + organization_id: self.organization_id, + }); } if !self.auth_parent_client_secret.is_empty() { self.require_org_and_parent_id("client-secret based")?; - return Ok(SystemIdentityData::Provision( - ProvisioningMethod::ParentSecret { - secret: self.auth_parent_client_secret, - parent_client_id: self.auth_parent_client_id, - organization_id: self.organization_id, - }, - )); + return Ok(ProvisioningMethod::ParentSecret { + secret: self.auth_parent_client_secret, + parent_client_id: self.auth_parent_client_id, + organization_id: self.organization_id, + }); } - Err( "either 'auth_client_id', 'auth_parent_token' or 'auth_parent_secret' should be set to register System Identity" .to_string(), @@ -169,8 +102,8 @@ impl SystemIdentityArgs { } } -/// Provides a key-based identity considering the supplied args. It returns the corresponding **client_id** as a String. -pub fn provide_identity( +/// Creates a key-based identity considering the supplied args. It returns the corresponding **client_id** as a String. +pub fn create_identity( method: &ProvisioningMethod, region: Region, proxy_config: Option, @@ -314,106 +247,59 @@ pub mod tests { use http::header::AUTHORIZATION; use httpmock::{Method::POST, MockServer}; use rstest::rstest; - use std::fs; - use tempfile::TempDir; use super::*; #[rstest] - #[case::existing_identity(|| SystemIdentityArgs { - auth_private_key_path: Some(std::env::current_dir().unwrap()), - auth_client_id: "some-client-id".to_string(), - ..Default::default() - })] - #[case::parent_token(|| SystemIdentityArgs { - auth_private_key_path: Some(PathBuf::from("/some/path")), + #[case::parent_token(|| ProvisionIdentityArgs { auth_parent_token: "TOKEN".to_string(), auth_parent_client_id: "parent-id".to_string(), organization_id: "org-id".to_string(), ..Default::default() })] - #[case::parent_secret(|| SystemIdentityArgs { - auth_private_key_path: Some(PathBuf::from("/some/path")), + #[case::parent_secret(|| ProvisionIdentityArgs { auth_parent_client_secret: "SECRET".to_string(), auth_parent_client_id: "parent-id".to_string(), organization_id: "org-id".to_string(), ..Default::default() })] - fn test_validate(#[case] make_args: fn() -> SystemIdentityArgs) { + fn test_validate(#[case] make_args: fn() -> ProvisionIdentityArgs) { assert_matches!(make_args().validate(), Ok(_)); } #[rstest] - #[case::missing_private_key_path(|| SystemIdentityArgs { - auth_client_id: "some-client-id".to_string(), - ..Default::default() - })] - #[case::nonexistent_key_with_client_id(|| SystemIdentityArgs { - auth_private_key_path: Some(PathBuf::from("/does/not/exist")), - auth_client_id: "some-client-id".to_string(), - ..Default::default() - })] - #[case::token_missing_org_id(|| SystemIdentityArgs { - auth_private_key_path: Some(PathBuf::from("/some/path")), + #[case::token_missing_org_id(|| ProvisionIdentityArgs { auth_parent_token: "TOKEN".to_string(), auth_parent_client_id: "parent-id".to_string(), ..Default::default() })] - #[case::token_missing_parent_client_id(|| SystemIdentityArgs { - auth_private_key_path: Some(PathBuf::from("/some/path")), + #[case::token_missing_parent_client_id(|| ProvisionIdentityArgs { auth_parent_token: "TOKEN".to_string(), organization_id: "org-id".to_string(), ..Default::default() })] - #[case::secret_missing_org_id(|| SystemIdentityArgs { - auth_private_key_path: Some(PathBuf::from("/some/path")), + #[case::secret_missing_org_id(|| ProvisionIdentityArgs { auth_parent_client_secret: "SECRET".to_string(), auth_parent_client_id: "parent-id".to_string(), ..Default::default() })] - #[case::secret_missing_parent_client_id(|| SystemIdentityArgs { - auth_private_key_path: Some(PathBuf::from("/some/path")), + #[case::secret_missing_parent_client_id(|| ProvisionIdentityArgs { auth_parent_client_secret: "SECRET".to_string(), organization_id: "org-id".to_string(), ..Default::default() })] - #[case::no_method_provided(|| SystemIdentityArgs { - auth_private_key_path: Some(PathBuf::from("/some/path")), - ..Default::default() - })] - fn test_validate_errors(#[case] make_args: fn() -> SystemIdentityArgs) { + #[case::no_method_provided(|| ProvisionIdentityArgs::default())] + fn test_validate_errors(#[case] make_args: fn() -> ProvisionIdentityArgs) { assert_matches!(make_args().validate(), Err(_)); } - #[test] - fn test_identity_already_provided() { - let tempdir = TempDir::new().unwrap(); - let auth_private_key_path = tempdir.path().join("private-key"); - // Key file must exist when using ExistingIdentity - fs::write(&auth_private_key_path, "").unwrap(); - let identity_args = SystemIdentityArgs { - auth_private_key_path: Some(auth_private_key_path.clone()), - auth_client_id: "provided_client_id".to_string(), - ..Default::default() - }; - let identity_data = identity_args.validate().expect("validation should succeed"); - - assert_matches!(identity_data.system_identity_data, SystemIdentityData::Existing { auth_client_id } => { - assert_eq!(auth_client_id, "provided_client_id".to_string()); - }); - } - #[test] fn test_build_identity_with_token() { - let tempdir = TempDir::new().unwrap(); - let auth_private_key_path = tempdir.path().join("private-key"); - let server = MockServer::start(); - let identity_args = SystemIdentityArgs { + let identity_args = ProvisionIdentityArgs { auth_parent_client_id: "parent-client-id".to_string(), auth_parent_token: "TOKEN".to_string(), - auth_private_key_path: Some(auth_private_key_path.clone()), organization_id: "test-org-id".to_string(), ..Default::default() }; @@ -436,28 +322,23 @@ pub mod tests { .expect("url should be valid"), }; - let identity_spec = identity_args.validate().expect("validation should succeed"); + let method = identity_args.validate().expect("validation should succeed"); let pub_key = b"mock-pub-key"; - assert_matches!(identity_spec.system_identity_data, SystemIdentityData::Provision(method) => { - let client_id = build_identity(&method, environment, None, pub_key.to_vec()).expect("no error expected"); - assert_eq!(client_id, "created_client_id".to_string()); - }); + let client_id = build_identity(&method, environment, None, pub_key.to_vec()) + .expect("no error expected"); + assert_eq!(client_id, "created_client_id".to_string()); identity_mock.assert_calls(1); } #[test] fn test_build_identity_client_secret() { - let tempdir = TempDir::new().unwrap(); - let auth_private_key_path = tempdir.path().join("private-key"); - let server = MockServer::start(); - let identity_args = SystemIdentityArgs { + let identity_args = ProvisionIdentityArgs { auth_parent_client_id: "parent-client-id".to_string(), auth_parent_client_secret: "client-secret-value".to_string(), - auth_private_key_path: Some(auth_private_key_path.clone()), organization_id: "test-org-id".to_string(), ..Default::default() }; @@ -488,12 +369,11 @@ pub mod tests { .expect("url should be valid"), }; - let identity_spec = identity_args.validate().expect("validation should succeed"); + let method = identity_args.validate().expect("validation should succeed"); let pub_key = b"mock-public-key"; - assert_matches!(identity_spec.system_identity_data, SystemIdentityData::Provision(method) => { - let client_id = build_identity(&method, environment, None, pub_key.to_vec()).expect("no error expected"); - assert_eq!(client_id, "created_client_id".to_string()); - }); + let client_id = build_identity(&method, environment, None, pub_key.to_vec()) + .expect("no error expected"); + assert_eq!(client_id, "created_client_id".to_string()); identity_mock.assert_calls(1); token_mock.assert_calls(1); diff --git a/agent-control/src/cli/k8s/system_identity.rs b/agent-control/src/cli/k8s/system_identity.rs index 1e4de6edf5..04d90567d8 100644 --- a/agent-control/src/cli/k8s/system_identity.rs +++ b/agent-control/src/cli/k8s/system_identity.rs @@ -15,10 +15,7 @@ use crate::{ error::CliError, proxy_config::ProxyConfig, region::{Region, region_parser}, - system_identity::{ - ProvisioningMethod, SystemIdentityArgs, SystemIdentityData, SystemIdentitySpec, - provide_identity, - }, + system_identity::{ProvisionIdentityArgs, ProvisioningMethod, create_identity}, }, k8s::{errors::K8sCliError, utils::try_new_k8s_client}, }, @@ -41,7 +38,7 @@ pub struct Args { /// Identity configuration #[command(flatten)] - identity: SystemIdentityArgs, + identity: ProvisionIdentityArgs, /// Proxy configuration #[command(flatten)] @@ -53,7 +50,7 @@ pub struct Args { pub struct IdentityRegistrationSpec { secret_name: String, region: Region, - identity: SystemIdentitySpec, + identity_provisioning_method: ProvisioningMethod, proxy_config: Option, } @@ -68,7 +65,7 @@ impl Args { Ok(IdentityRegistrationSpec { secret_name: self.secret_name, region: self.region, - identity: self.identity.validate()?, + identity_provisioning_method: self.identity.validate()?, proxy_config: self.proxy_config, }) } @@ -80,7 +77,7 @@ pub fn register_system_identity( spec: IdentityRegistrationSpec, ) -> Result<(), K8sCliError> { let k8s_client = try_new_k8s_client()?; - provide_system_identity_secret(namespace, spec, &k8s_client, provide_identity) + provide_system_identity_secret(namespace, spec, &k8s_client, create_identity) } /// Helper function to implement [register_system_identity] while allowing the usage of mocks for the k8s client @@ -91,7 +88,7 @@ fn provide_system_identity_secret( namespace: &str, spec: IdentityRegistrationSpec, k8s_client: &SyncK8sClient, - provide_identity_fn: F, + create_identity: F, ) -> Result<(), K8sCliError> where F: Fn( @@ -116,16 +113,6 @@ where } info!("Secret is not present, creating system identity"); - // Existing identities cannot be used here because the private key must be stored in the Secret, - // not on local filesystem. - let SystemIdentityData::Provision(provisioning_method) = &spec.identity.system_identity_data - else { - return Err(K8sCliError::Generic( - "the k8s cli requires provisioning a new System Identity; use --auth-parent-token or --auth-parent-client-secret instead of --auth-client-id" - .to_string(), - )); - }; - let KeyPair { private_key, public_key, @@ -135,8 +122,8 @@ where )) })?; - let client_id = provide_identity_fn( - provisioning_method, + let client_id = create_identity( + &spec.identity_provisioning_method, spec.region, spec.proxy_config, public_key, @@ -210,22 +197,17 @@ mod tests { use assert_matches::assert_matches; use clap::{CommandFactory, FromArgMatches}; use rstest::rstest; - use std::{env::current_dir, path::PathBuf, sync::Arc}; + use std::sync::Arc; impl Default for IdentityRegistrationSpec { fn default() -> Self { IdentityRegistrationSpec { secret_name: "test-secret".to_string(), region: Region::US, - identity: SystemIdentitySpec { - system_identity_data: SystemIdentityData::Provision( - ProvisioningMethod::ParentSecret { - secret: "secret".to_string(), - parent_client_id: "parent_client_id".to_string(), - organization_id: "org_id".to_string(), - }, - ), - private_key_path: PathBuf::from("/test/key"), + identity_provisioning_method: ProvisioningMethod::ParentSecret { + secret: "secret".to_string(), + parent_client_id: "parent_client_id".to_string(), + organization_id: "org_id".to_string(), }, proxy_config: None, } @@ -233,44 +215,39 @@ mod tests { } #[rstest] - #[case::token_based_identity( - || String::from("--secret-name s --region us --auth-private-key-path /some/path --auth-parent-token TOKEN --auth-parent-client-id id --organization-id org-id") - )] - #[case::client_secret_based_identity( - || String::from("--secret-name s --region us --auth-private-key-path /some/path --auth-parent-client-secret SECRET --auth-parent-client-id id --organization-id org-id") - )] - fn test_args_validation(#[case] args: fn() -> String) { + #[case::token_based_identity(String::from( + "--secret-name s --region us --auth-parent-token TOKEN --auth-parent-client-id id --organization-id org-id" + ))] + #[case::client_secret_based_identity(String::from( + "--secret-name s --region us --auth-parent-client-secret SECRET --auth-parent-client-id id --organization-id org-id" + ))] + fn test_args_validation(#[case] args: String) { let cmd = Args::command().no_binary_name(true); let matches = cmd - .try_get_matches_from(args().split_ascii_whitespace()) + .try_get_matches_from(args.split_ascii_whitespace()) .expect("arguments should be valid"); let args = Args::from_arg_matches(&matches).expect("should create the struct back"); assert!(args.validate().is_ok()); } #[rstest] - #[case::missing_private_key_path( - || String::from("--secret-name s --region us --auth-client-id some-id") - )] - #[case::no_identity_method( - || format!("--secret-name s --region us --auth-private-key-path {}", current_dir().unwrap().display()) - )] - #[case::missing_org_id_with_token( - || String::from("--secret-name s --region us --auth-private-key-path /p --auth-parent-token TOKEN --auth-parent-client-id id") - )] - #[case::missing_parent_client_id_with_secret( - || String::from("--secret-name s --region us --auth-private-key-path /p --auth-parent-client-secret SECRET --organization-id org-id") - )] - #[case::missing_org_id_with_secret( - || String::from("--secret-name s --region us --auth-private-key-path /p --auth-parent-client-secret SECRET --auth-parent-client-id id") - )] - #[case::invalid_proxy_config( - || String::from("--secret-name s --region us --auth-private-key-path /p --auth-parent-client-secret SECRET --auth-parent-client-id id --organization-id org-id --proxy-url https::/invalid") - )] - fn test_args_validation_errors(#[case] args: fn() -> String) { + #[case::no_identity_method(String::from("--secret-name s --region us"))] + #[case::missing_org_id_with_token(String::from( + "--secret-name s --region us --auth-parent-token TOKEN --auth-parent-client-id id" + ))] + #[case::missing_parent_client_id_with_secret(String::from( + "--secret-name s --region us --auth-parent-client-secret SECRET --organization-id org-id" + ))] + #[case::missing_org_id_with_secret(String::from( + "--secret-name s --region us --auth-parent-client-secret SECRET --auth-parent-client-id id" + ))] + #[case::invalid_proxy_config(String::from( + "--secret-name s --region us --auth-parent-client-secret SECRET --auth-parent-client-id id --organization-id org-id --proxy-url https::/invalid" + ))] + fn test_args_validation_errors(#[case] args: String) { let cmd = Args::command().no_binary_name(true); let matches = cmd - .try_get_matches_from(args().split_ascii_whitespace()) + .try_get_matches_from(args.split_ascii_whitespace()) .expect("arguments should be valid"); let args = Args::from_arg_matches(&matches).expect("should create the struct back"); assert_matches!(args.validate(), Err(_)); diff --git a/agent-control/src/cli/on_host/config_gen.rs b/agent-control/src/cli/on_host/config_gen.rs index e106d38bd0..f0c10ae0dc 100644 --- a/agent-control/src/cli/on_host/config_gen.rs +++ b/agent-control/src/cli/on_host/config_gen.rs @@ -4,10 +4,7 @@ use crate::cli::{ error::CliError, proxy_config::ProxyConfig, region::{Region, region_parser}, - system_identity::{ - ProvisioningMethod, SystemIdentityArgs, SystemIdentityData, SystemIdentitySpec, - provide_identity, - }, + system_identity::{ProvisionIdentityArgs, ProvisioningMethod, create_identity}, }, on_host::config_gen::config::{ AuthConfig, Config, FleetControl, LogConfig, Server, SignatureValidation, @@ -18,7 +15,10 @@ use nr_auth::key::{ generation::{KeyType, PublicKeyPem}, local::{LocalKeyPairGenerator, LocalKeyPairGeneratorConfig}, }; -use std::{collections::HashMap, path::PathBuf}; +use std::{ + collections::HashMap, + path::{Path, PathBuf}, +}; use tracing::info; pub mod config; @@ -45,9 +45,18 @@ pub struct Args { #[arg(long, default_value_t)] fleet_id: String, + /// Path where the private key is stored or will be written. + #[arg(long)] + pub auth_private_key_path: Option, + + /// Client ID of an already-provisioned system identity. When non-empty, no identity + /// generation is performed. + #[arg(long, default_value_t)] + pub auth_client_id: String, + /// Identity configuration #[command(flatten)] - identity: SystemIdentityArgs, + identity: ProvisionIdentityArgs, /// Proxy configuration #[command(flatten)] @@ -62,6 +71,24 @@ pub struct Args { env_vars_file_path: Option, } +/// Valid data to create a SystemIdentity, represent [SystemIdentityArgs] after validation. +#[derive(Debug)] +pub struct SystemIdentitySpec { + /// Data to get or create the System Identity to be used by Agent Control + pub system_identity_data: SystemIdentityData, + /// Path where the corresponding private key needs to be read from or written to + pub private_key_path: PathBuf, +} + +/// Defines whether a SystemIdentity already exists or needs to be provisioned +#[derive(Debug)] +pub enum SystemIdentityData { + /// The Identity already exists + Existing { auth_client_id: String }, + /// The identity needs to be provisioned + Provision(ProvisioningMethod), +} + /// Represents fleet parameters to generate configuration depending of it its enabled or not. #[derive(Debug)] pub enum FleetParams { @@ -93,9 +120,25 @@ impl Args { if self.fleet_id.is_empty() { return Err(String::from("'fleet_id' should be set when enabling fleet")); } + let private_key_path = self + .auth_private_key_path + .as_ref() + .ok_or_else(|| { + "'auth_private_key_path' needs to be set to register System Identity" + .to_string() + })? + .clone(); + FleetParams::FleetEnabled { fleet_id: self.fleet_id, - identity: self.identity.validate()?, + identity: SystemIdentitySpec { + system_identity_data: Self::identity_data( + &private_key_path, + self.auth_client_id, + self.identity, + )?, + private_key_path, + }, } }; if let Some(proxy_config) = self.proxy_config.clone() @@ -112,6 +155,24 @@ impl Args { fleet: fleet_inputs, }) } + + /// Helper to build [SystemIdentityData] from args + fn identity_data( + private_key_path: &Path, + auth_client_id: String, + identity_args: ProvisionIdentityArgs, + ) -> Result { + if !auth_client_id.is_empty() { + if !private_key_path.exists() { + return Err( + "when 'auth_client_id' is provided, 'auth_private_key_path' must exist" + .to_string(), + ); + } + return Ok(SystemIdentityData::Existing { auth_client_id }); + } + Ok(SystemIdentityData::Provision(identity_args.validate()?)) + } } /// Generates: @@ -128,7 +189,7 @@ pub fn generate(params: Params) -> Result<(), CliError> { fn write_config_and_generate_system_identity(params: &Params) -> Result<(), CliError> { info!("Generating Agent Control configuration"); - let yaml = generate_config_and_system_identity(params, provide_identity)?; + let yaml = generate_config_and_system_identity(params, create_identity)?; LocalFile.write(¶ms.output_path, yaml).map_err(|err| { CliError::Command(format!( @@ -191,7 +252,7 @@ fn generate_env_var_config(params: &Params) -> Result { /// Generates the configuration according to args using the provided function to generate the identity. fn generate_config_and_system_identity( params: &Params, - provide_identity_fn: F, + create_identity: F, ) -> Result where F: Fn( @@ -211,7 +272,7 @@ where SystemIdentityData::Existing { auth_client_id } => auth_client_id.to_string(), SystemIdentityData::Provision(provisioning_method) => { let pub_key = public_key_from_key_pair(identity_spec.private_key_path.clone())?; - provide_identity_fn( + create_identity( provisioning_method, params.region, params.proxy_config.clone(), @@ -301,9 +362,6 @@ mod tests { #[case::fleet_disabled( || String::from("--fleet-disabled --output-path /some/path --region us") )] - #[case::identity_already_provided( - || format!("--output-path /some/path --region us --fleet-id some-id --auth-private-key-path {} --auth-client-id some-client-id", pwd()) - )] #[case::token_based_identity( || format!("--output-path /some/path --region us --fleet-id some-id --auth-private-key-path {} --auth-parent-token TOKEN --auth-parent-client-id id --organization-id org-id", pwd()) )] @@ -319,6 +377,27 @@ mod tests { assert_matches!(args.validate(), Ok(_)); } + #[test] + fn test_identity_already_provided() { + let args_definition = format!( + "--output-path /some/path --region us --fleet-id some-id --auth-private-key-path {} --auth-client-id some-client-id", + pwd() + ); + let cmd = Args::command().no_binary_name(true); + let matches = cmd + .try_get_matches_from(args_definition.split_ascii_whitespace()) + .expect("arguments should be valid"); + let args = Args::from_arg_matches(&matches).expect("should create the struct back"); + assert_matches!(args.validate(), Ok(params) => { + assert_matches!(params.fleet, FleetParams::FleetEnabled { fleet_id, identity } => { + assert_eq!(fleet_id, "some-id".to_string()); + assert_matches!(identity.system_identity_data, SystemIdentityData::Existing { auth_client_id } => { + assert_eq!(auth_client_id, "some-client-id".to_string()); + }) + }) + }) + } + #[rstest] #[case::missing_identity_creation_method( || format!("--output-path /some/path --region us --auth-private-key-path {}", pwd()) From 0f161fa3d354a608b4029ef2876b6c26f98cdd71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Felipe=20=C3=81lvarez?= Date: Fri, 10 Apr 2026 13:45:45 +0200 Subject: [PATCH 5/9] chore: address feedback --- agent-control/src/cli/common/system_identity.rs | 8 ++++---- agent-control/src/cli/on_host/config_gen.rs | 12 +++++++++++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/agent-control/src/cli/common/system_identity.rs b/agent-control/src/cli/common/system_identity.rs index cacc902722..a74ff0679f 100644 --- a/agent-control/src/cli/common/system_identity.rs +++ b/agent-control/src/cli/common/system_identity.rs @@ -251,20 +251,20 @@ pub mod tests { use super::*; #[rstest] - #[case::parent_token(|| ProvisionIdentityArgs { + #[case::parent_token(ProvisionIdentityArgs { auth_parent_token: "TOKEN".to_string(), auth_parent_client_id: "parent-id".to_string(), organization_id: "org-id".to_string(), ..Default::default() })] - #[case::parent_secret(|| ProvisionIdentityArgs { + #[case::parent_secret(ProvisionIdentityArgs { auth_parent_client_secret: "SECRET".to_string(), auth_parent_client_id: "parent-id".to_string(), organization_id: "org-id".to_string(), ..Default::default() })] - fn test_validate(#[case] make_args: fn() -> ProvisionIdentityArgs) { - assert_matches!(make_args().validate(), Ok(_)); + fn test_validate(#[case] make_args: ProvisionIdentityArgs) { + assert_matches!(make_args.validate(), Ok(_)); } #[rstest] diff --git a/agent-control/src/cli/on_host/config_gen.rs b/agent-control/src/cli/on_host/config_gen.rs index f0c10ae0dc..35dfa9369f 100644 --- a/agent-control/src/cli/on_host/config_gen.rs +++ b/agent-control/src/cli/on_host/config_gen.rs @@ -271,7 +271,17 @@ where let client_id = match &identity_spec.system_identity_data { SystemIdentityData::Existing { auth_client_id } => auth_client_id.to_string(), SystemIdentityData::Provision(provisioning_method) => { - let pub_key = public_key_from_key_pair(identity_spec.private_key_path.clone())?; + let pub_key = LocalKeyPairGenerator::from(LocalKeyPairGeneratorConfig { + key_type: KeyType::Rsa4096, + file_path: identity_spec.private_key_path.clone(), + }) + .generate() + .map_err(|err| { + CliError::Command(format!( + "could not generate System Identity's key-pair; {err}" + )) + })?; + create_identity( provisioning_method, params.region, From cee90f15c8b696bda2c1edd2f0306f2d493d6937 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Felipe=20=C3=81lvarez?= Date: Fri, 10 Apr 2026 13:48:57 +0200 Subject: [PATCH 6/9] chore: remove unused code --- agent-control/src/cli/on_host/config_gen.rs | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/agent-control/src/cli/on_host/config_gen.rs b/agent-control/src/cli/on_host/config_gen.rs index 35dfa9369f..23bfff8646 100644 --- a/agent-control/src/cli/on_host/config_gen.rs +++ b/agent-control/src/cli/on_host/config_gen.rs @@ -331,19 +331,6 @@ fn default_log_config() -> Option { } } -/// Helper to generate a key pair using the provided path for the private key and return the public key. -fn public_key_from_key_pair(private_key_path: PathBuf) -> Result { - let key_generator = LocalKeyPairGenerator::from(LocalKeyPairGeneratorConfig { - key_type: KeyType::Rsa4096, - file_path: private_key_path, - }); - key_generator.generate().map_err(|err| { - CliError::Command(format!( - "could not generate System Identity's key-pair; {err}" - )) - }) -} - #[cfg(test)] mod tests { use super::*; From 72e53c0582d6c8aee719a16da5cbb910854337b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Felipe=20=C3=81lvarez?= Date: Mon, 13 Apr 2026 11:48:18 +0200 Subject: [PATCH 7/9] chore: update third-party-notices --- THIRD_PARTY_NOTICES.md | 45 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/THIRD_PARTY_NOTICES.md b/THIRD_PARTY_NOTICES.md index b01c00e4f0..c60a995272 100644 --- a/THIRD_PARTY_NOTICES.md +++ b/THIRD_PARTY_NOTICES.md @@ -478,6 +478,20 @@ Distributed under the following license(s): * MIT +## const-random + +Distributed under the following license(s): + +* MIT +* Apache-2.0 + +## const-random-macro + +Distributed under the following license(s): + +* MIT +* Apache-2.0 + ## const_format Distributed under the following license(s): @@ -588,6 +602,12 @@ Distributed under the following license(s): * MIT * Apache-2.0 +## crunchy + +Distributed under the following license(s): + +* MIT + ## crypto-common Distributed under the following license(s): @@ -714,6 +734,13 @@ Distributed under the following license(s): * MIT * Apache-2.0 +## dlv-list + +Distributed under the following license(s): + +* MIT +* Apache-2.0 + ## downcast Distributed under the following license(s): @@ -1677,6 +1704,12 @@ Distributed under the following license(s): * MIT +## ordered-multimap + +Distributed under the following license(s): + +* MIT + ## parking Distributed under the following license(s): @@ -2028,6 +2061,12 @@ Distributed under the following license(s): * MIT * Apache-2.0 +## rust-ini + +Distributed under the following license(s): + +* MIT + ## rust_decimal Distributed under the following license(s): @@ -2455,6 +2494,12 @@ Distributed under the following license(s): * MIT * Apache-2.0 +## tiny-keccak + +Distributed under the following license(s): + +* CC0-1.0 + ## tinystr Distributed under the following license(s): From 98148db331d3bba7e449073e8dcd49bf6268e46b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Felipe=20=C3=81lvarez?= Date: Mon, 13 Apr 2026 15:51:26 +0200 Subject: [PATCH 8/9] chore: address PR feedback --- .../src/cli/common/system_identity.rs | 31 +++++++++---------- agent-control/src/cli/k8s/system_identity.rs | 15 +++------ agent-control/src/cli/on_host/config_gen.rs | 25 ++++++--------- 3 files changed, 29 insertions(+), 42 deletions(-) diff --git a/agent-control/src/cli/common/system_identity.rs b/agent-control/src/cli/common/system_identity.rs index a74ff0679f..383c75ba77 100644 --- a/agent-control/src/cli/common/system_identity.rs +++ b/agent-control/src/cli/common/system_identity.rs @@ -56,7 +56,7 @@ pub struct ProvisionIdentityArgs { /// Defines the supported provisioning methods for System Identities #[derive(Debug)] -pub enum ProvisioningMethod { +pub enum ParentAuthMethod { ParentToken { token: String, organization_id: String, @@ -69,10 +69,10 @@ pub enum ProvisioningMethod { } impl ProvisionIdentityArgs { - pub fn validate(self) -> Result { + pub fn validate(self) -> Result { if !self.auth_parent_token.is_empty() { self.require_org_and_parent_id("token based")?; - return Ok(ProvisioningMethod::ParentToken { + return Ok(ParentAuthMethod::ParentToken { token: self.auth_parent_token, organization_id: self.organization_id, }); @@ -80,14 +80,14 @@ impl ProvisionIdentityArgs { if !self.auth_parent_client_secret.is_empty() { self.require_org_and_parent_id("client-secret based")?; - return Ok(ProvisioningMethod::ParentSecret { + return Ok(ParentAuthMethod::ParentSecret { secret: self.auth_parent_client_secret, parent_client_id: self.auth_parent_client_id, organization_id: self.organization_id, }); } Err( - "either 'auth_client_id', 'auth_parent_token' or 'auth_parent_secret' should be set to register System Identity" + "either 'auth_parent_token' or 'auth_parent_secret' should be set to create a System Identity" .to_string(), ) } @@ -104,24 +104,24 @@ impl ProvisionIdentityArgs { /// Creates a key-based identity considering the supplied args. It returns the corresponding **client_id** as a String. pub fn create_identity( - method: &ProvisioningMethod, + parent_auth_method: &ParentAuthMethod, region: Region, proxy_config: Option, pub_key: PublicKeyPem, ) -> Result { let environment = NewRelicEnvironment::from(region); - build_identity(method, environment, proxy_config, pub_key) + build_identity(parent_auth_method, environment, proxy_config, pub_key) } /// Helper to allow injecting testing urls when building the identity. fn build_identity( - method: &ProvisioningMethod, + parent_auth_method: &ParentAuthMethod, environment: NewRelicEnvironment, proxy_config: Option, pub_key: PublicKeyPem, ) -> Result { - match method { - ProvisioningMethod::ParentToken { + match parent_auth_method { + ParentAuthMethod::ParentToken { token, organization_id, } => { @@ -135,7 +135,7 @@ fn build_identity( pub_key, ) } - ProvisioningMethod::ParentSecret { + ParentAuthMethod::ParentSecret { secret, parent_client_id, organization_id, @@ -184,12 +184,9 @@ fn get_auth_token( let authenticator = HttpAuthenticator::new(http_client.clone(), environment.token_renewal_endpoint()); - let token_retriever = TokenRetrieverWithCache::new_with_secret( - parent_client_id.clone(), - authenticator, - secret.into(), - ) - .with_retries(DEFAULT_RETRIES); + let token_retriever = + TokenRetrieverWithCache::new_with_secret(parent_client_id, authenticator, secret.into()) + .with_retries(DEFAULT_RETRIES); token_retriever.retrieve().map_err(|err| { CliError::Command(format!( diff --git a/agent-control/src/cli/k8s/system_identity.rs b/agent-control/src/cli/k8s/system_identity.rs index 04d90567d8..477bca41a2 100644 --- a/agent-control/src/cli/k8s/system_identity.rs +++ b/agent-control/src/cli/k8s/system_identity.rs @@ -15,7 +15,7 @@ use crate::{ error::CliError, proxy_config::ProxyConfig, region::{Region, region_parser}, - system_identity::{ProvisionIdentityArgs, ProvisioningMethod, create_identity}, + system_identity::{ParentAuthMethod, ProvisionIdentityArgs, create_identity}, }, k8s::{errors::K8sCliError, utils::try_new_k8s_client}, }, @@ -50,7 +50,7 @@ pub struct Args { pub struct IdentityRegistrationSpec { secret_name: String, region: Region, - identity_provisioning_method: ProvisioningMethod, + identity_provisioning_method: ParentAuthMethod, proxy_config: Option, } @@ -91,12 +91,7 @@ fn provide_system_identity_secret( create_identity: F, ) -> Result<(), K8sCliError> where - F: Fn( - &ProvisioningMethod, - Region, - Option, - PublicKeyPem, - ) -> Result, + F: Fn(&ParentAuthMethod, Region, Option, PublicKeyPem) -> Result, { let secret_object_key = K8sObjectKey { name: &spec.secret_name, @@ -192,7 +187,7 @@ fn secret_dynamic_object(object_key: K8sObjectKey<'_>, data: serde_json::Value) #[cfg(test)] mod tests { use super::*; - use crate::cli::common::system_identity::ProvisioningMethod; + use crate::cli::common::system_identity::ParentAuthMethod; use crate::k8s::client::MockSyncK8sClient; use assert_matches::assert_matches; use clap::{CommandFactory, FromArgMatches}; @@ -204,7 +199,7 @@ mod tests { IdentityRegistrationSpec { secret_name: "test-secret".to_string(), region: Region::US, - identity_provisioning_method: ProvisioningMethod::ParentSecret { + identity_provisioning_method: ParentAuthMethod::ParentSecret { secret: "secret".to_string(), parent_client_id: "parent_client_id".to_string(), organization_id: "org_id".to_string(), diff --git a/agent-control/src/cli/on_host/config_gen.rs b/agent-control/src/cli/on_host/config_gen.rs index 23bfff8646..f4fb688cf1 100644 --- a/agent-control/src/cli/on_host/config_gen.rs +++ b/agent-control/src/cli/on_host/config_gen.rs @@ -4,7 +4,7 @@ use crate::cli::{ error::CliError, proxy_config::ProxyConfig, region::{Region, region_parser}, - system_identity::{ProvisionIdentityArgs, ProvisioningMethod, create_identity}, + system_identity::{ParentAuthMethod, ProvisionIdentityArgs, create_identity}, }, on_host::config_gen::config::{ AuthConfig, Config, FleetControl, LogConfig, Server, SignatureValidation, @@ -47,12 +47,12 @@ pub struct Args { /// Path where the private key is stored or will be written. #[arg(long)] - pub auth_private_key_path: Option, + auth_private_key_path: Option, /// Client ID of an already-provisioned system identity. When non-empty, no identity /// generation is performed. #[arg(long, default_value_t)] - pub auth_client_id: String, + auth_client_id: String, /// Identity configuration #[command(flatten)] @@ -86,7 +86,7 @@ pub enum SystemIdentityData { /// The Identity already exists Existing { auth_client_id: String }, /// The identity needs to be provisioned - Provision(ProvisioningMethod), + Provision(ParentAuthMethod), } /// Represents fleet parameters to generate configuration depending of it its enabled or not. @@ -255,12 +255,7 @@ fn generate_config_and_system_identity( create_identity: F, ) -> Result where - F: Fn( - &ProvisioningMethod, - Region, - Option, - PublicKeyPem, - ) -> Result, + F: Fn(&ParentAuthMethod, Region, Option, PublicKeyPem) -> Result, { let fleet_control = match ¶ms.fleet { FleetParams::FleetDisabled => None, @@ -270,7 +265,7 @@ where } => { let client_id = match &identity_spec.system_identity_data { SystemIdentityData::Existing { auth_client_id } => auth_client_id.to_string(), - SystemIdentityData::Provision(provisioning_method) => { + SystemIdentityData::Provision(parent_auth_method) => { let pub_key = LocalKeyPairGenerator::from(LocalKeyPairGeneratorConfig { key_type: KeyType::Rsa4096, file_path: identity_spec.private_key_path.clone(), @@ -283,7 +278,7 @@ where })?; create_identity( - provisioning_method, + parent_auth_method, params.region, params.proxy_config.clone(), pub_key, @@ -335,7 +330,7 @@ fn default_log_config() -> Option { mod tests { use super::*; use crate::agent_control::config::AgentControlConfig; - use crate::cli::common::system_identity::ProvisioningMethod; + use crate::cli::common::system_identity::ParentAuthMethod; use assert_matches::assert_matches; use clap::{CommandFactory, FromArgMatches}; use rstest::rstest; @@ -506,7 +501,7 @@ mod tests { } fn identity_provider_mock( - _: &ProvisioningMethod, + _: &ParentAuthMethod, _: Region, _: Option, _: PublicKeyPem, @@ -527,7 +522,7 @@ mod tests { fleet_id: "test-fleet-id".to_string(), identity: SystemIdentitySpec { system_identity_data: SystemIdentityData::Provision( - ProvisioningMethod::ParentSecret { + ParentAuthMethod::ParentSecret { secret: "parent-client-secret".to_string(), parent_client_id: "parent-client-id".to_string(), organization_id: "test-org-id".to_string(), From 432f42ee6458e44716f2f1871dc6057ea68974dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christian=20Felipe=20=C3=81lvarez?= Date: Mon, 13 Apr 2026 16:24:31 +0200 Subject: [PATCH 9/9] chore: remove missing public field --- agent-control/src/cli/on_host/config_gen.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent-control/src/cli/on_host/config_gen.rs b/agent-control/src/cli/on_host/config_gen.rs index f4fb688cf1..a05de5f3bd 100644 --- a/agent-control/src/cli/on_host/config_gen.rs +++ b/agent-control/src/cli/on_host/config_gen.rs @@ -75,9 +75,9 @@ pub struct Args { #[derive(Debug)] pub struct SystemIdentitySpec { /// Data to get or create the System Identity to be used by Agent Control - pub system_identity_data: SystemIdentityData, + system_identity_data: SystemIdentityData, /// Path where the corresponding private key needs to be read from or written to - pub private_key_path: PathBuf, + private_key_path: PathBuf, } /// Defines whether a SystemIdentity already exists or needs to be provisioned