Skip to content

Commit f236a10

Browse files
rina23qrina23q
authored
Merge pull request #3372 from rina23q/fix/3369/tedge-connect-always-use-cn-as-device-id
fix: device.id is derived from device certificate if certificate exists
2 parents 17b838b + 83a82b2 commit f236a10

File tree

10 files changed

+272
-260
lines changed

10 files changed

+272
-260
lines changed

‎crates/common/tedge_config/src/tedge_config_cli/tedge_config.rs‎

Lines changed: 25 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -1030,65 +1030,52 @@ fn device_id(
10301030
device: &TEdgeConfigReaderDevice,
10311031
dto_value: &OptionalConfig<String>,
10321032
) -> Result<String, ReadError> {
1033-
match dto_value.or_none() {
1034-
Some(id) => Ok(id.clone()),
1035-
None => device_id_from_cert(&device.cert_path),
1033+
match (device_id_from_cert(&device.cert_path), dto_value.or_none()) {
1034+
(Ok(common_name), _) => Ok(common_name),
1035+
(Err(_), Some(dto_value)) => Ok(dto_value.to_string()),
1036+
(Err(err), None) => Err(err),
10361037
}
10371038
}
10381039

10391040
fn c8y_device_id(
10401041
c8y_device: &TEdgeConfigReaderC8yDevice,
10411042
dto_value: &OptionalConfig<String>,
10421043
) -> Result<String, ReadError> {
1043-
match dto_value.or_none() {
1044-
Some(id) => Ok(id.clone()),
1045-
None => device_id_from_cert(&c8y_device.cert_path),
1044+
match (
1045+
device_id_from_cert(&c8y_device.cert_path),
1046+
dto_value.or_none(),
1047+
) {
1048+
(Ok(common_name), _) => Ok(common_name),
1049+
(Err(_), Some(dto_value)) => Ok(dto_value.to_string()),
1050+
(Err(err), None) => Err(err),
10461051
}
10471052
}
10481053

10491054
fn az_device_id(
10501055
az_device: &TEdgeConfigReaderAzDevice,
10511056
dto_value: &OptionalConfig<String>,
10521057
) -> Result<String, ReadError> {
1053-
match dto_value.or_none() {
1054-
Some(id) => Ok(id.clone()),
1055-
None => device_id_from_cert(&az_device.cert_path),
1058+
match (
1059+
device_id_from_cert(&az_device.cert_path),
1060+
dto_value.or_none(),
1061+
) {
1062+
(Ok(common_name), _) => Ok(common_name),
1063+
(Err(_), Some(dto_value)) => Ok(dto_value.to_string()),
1064+
(Err(err), None) => Err(err),
10561065
}
10571066
}
10581067

10591068
fn aws_device_id(
10601069
aws_device: &TEdgeConfigReaderAwsDevice,
10611070
dto_value: &OptionalConfig<String>,
10621071
) -> Result<String, ReadError> {
1063-
match dto_value.or_none() {
1064-
Some(id) => Ok(id.clone()),
1065-
None => device_id_from_cert(&aws_device.cert_path),
1066-
}
1067-
}
1068-
1069-
pub fn explicit_device_id(
1070-
config_location: &TEdgeConfigLocation,
1071-
cloud: &Option<Cloud>,
1072-
) -> Option<String> {
1073-
let dto = config_location.load_dto_from_toml_and_env().ok()?;
1074-
1075-
match cloud {
1076-
None => dto.device.id.clone(),
1077-
Some(Cloud::C8y(profile)) => {
1078-
let key = profile.map(|name| name.to_string());
1079-
let c8y_dto = dto.c8y.try_get(key.as_deref(), "c8y").ok()?;
1080-
c8y_dto.device.id.clone()
1081-
}
1082-
Some(Cloud::Az(profile)) => {
1083-
let key = profile.map(|name| name.to_string());
1084-
let az_dto = dto.az.try_get(key.as_deref(), "az").ok()?;
1085-
az_dto.device.id.clone()
1086-
}
1087-
Some(Cloud::Aws(profile)) => {
1088-
let key = profile.map(|name| name.to_string());
1089-
let aws_dto = dto.aws.try_get(key.as_deref(), "aws").ok()?;
1090-
aws_dto.device.id.clone()
1091-
}
1072+
match (
1073+
device_id_from_cert(&aws_device.cert_path),
1074+
dto_value.or_none(),
1075+
) {
1076+
(Ok(common_name), _) => Ok(common_name),
1077+
(Err(_), Some(dto_value)) => Ok(dto_value.to_string()),
1078+
(Err(err), None) => Err(err),
10921079
}
10931080
}
10941081

‎crates/core/tedge/src/cli/certificate/cli.rs‎

Lines changed: 32 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,9 @@ use super::upload::*;
88
use anyhow::anyhow;
99
use camino::Utf8PathBuf;
1010
use clap::ValueHint;
11-
use tedge_config::explicit_device_id;
1211
use tedge_config::OptionalConfigError;
1312
use tedge_config::ProfileName;
1413
use tedge_config::TEdgeConfig;
15-
use tedge_config::TEdgeConfigLocation;
1614

1715
use crate::cli::common::Cloud;
1816
use crate::cli::common::CloudArg;
@@ -84,12 +82,11 @@ impl BuildCommand for TEdgeCertCli {
8482
let cloud: Option<Cloud> = cloud.map(<_>::try_into).transpose()?;
8583

8684
let cmd = CreateCertCmd {
87-
id: get_device_id(id, &config, &context.config_location, &cloud)?,
85+
id: get_device_id(id, &config, &cloud)?,
8886
cert_path: config.device_cert_path(cloud.as_ref())?.to_owned(),
8987
key_path: config.device_key_path(cloud.as_ref())?.to_owned(),
9088
user: user.to_owned(),
9189
group: group.to_owned(),
92-
config_location: context.config_location,
9390
};
9491
cmd.into_boxed()
9592
}
@@ -102,7 +99,7 @@ impl BuildCommand for TEdgeCertCli {
10299
let cloud: Option<Cloud> = cloud.map(<_>::try_into).transpose()?;
103100

104101
let cmd = CreateCsrCmd {
105-
id: get_device_id(id, &config, &context.config_location, &cloud)?,
102+
id: get_device_id(id, &config, &cloud)?,
106103
key_path: config.device_key_path(cloud.as_ref())?.to_owned(),
107104
// Use output file instead of csr_path from tedge config if provided
108105
csr_path: output_path.unwrap_or_else(|| config.device.csr_path.clone()),
@@ -194,32 +191,14 @@ pub enum UploadCertCli {
194191
fn get_device_id(
195192
id: Option<String>,
196193
config: &TEdgeConfig,
197-
config_location: &TEdgeConfigLocation,
198194
cloud: &Option<Cloud>,
199195
) -> Result<String, anyhow::Error> {
200196
match (id, config.device_id(cloud.as_ref()).ok()) {
201197
(None, None) => Err(anyhow!(
202198
"No device ID is provided. Use `--device-id <name>` option to specify the device ID."
203199
)),
204200
(None, Some(config_id)) => Ok(config_id.into()),
205-
(Some(input_id), None) => Ok(input_id),
206-
(Some(input_id), Some(config_id)) if input_id == config_id => Ok(input_id),
207-
(Some(input_id), Some(_config_id)) => {
208-
match explicit_device_id(config_location, &cloud.as_ref().map(Into::into)) {
209-
None => {
210-
// the cloud profile doesn't have its own device.id explicitly, so using the input id is fine
211-
Ok(input_id)
212-
}
213-
Some(explicit_id) => {
214-
Err(anyhow!(
215-
"`--device-id` option conflicts with tedge config settings.\n\
216-
Configured value: '{explicit_id}', but input: '{input_id}'\n\n\
217-
Please either update the configuration using `tedge config set <key> <new_id>`\n\
218-
or provide the correct value with the `--device-id` option."
219-
))
220-
}
221-
}
222-
}
201+
(Some(input_id), _) => Ok(input_id),
223202
}
224203
}
225204

@@ -328,51 +307,54 @@ mod tests {
328307
},
329308
"input"
330309
)]
331-
fn validate_get_device_id_returns_ok(
332-
input_id: Option<&str>,
333-
cloud_arg: Option<CloudArg>,
334-
toml: toml::Table,
335-
expected: &str,
336-
) {
337-
let cloud: Option<Cloud> = cloud_arg.map(<_>::try_into).transpose().unwrap();
338-
let ttd = TempTedgeDir::new();
339-
ttd.file("tedge.toml").with_toml_content(toml);
340-
let location = TEdgeConfigLocation::from_custom_root(ttd.path());
341-
let reader = location.load().unwrap();
342-
let id = input_id.map(|s| s.to_string());
343-
let result = get_device_id(id, &reader, &location, &cloud);
344-
assert_eq!(result.unwrap().as_str(), expected);
345-
}
346-
347-
#[test_case(
348-
None,
349-
None,
350-
toml::toml!{
351-
[device]
352-
}
353-
)]
354310
#[test_case(
355311
Some("input"),
356312
None,
357313
toml::toml!{
358314
[device]
359315
id = "test"
360-
}
316+
},
317+
"input"
361318
)]
362319
#[test_case(
363320
Some("input"),
364321
Some(CloudArg::C8y{ profile: None }),
365322
toml::toml!{
366323
[c8y.device]
367324
id = "c8y-test"
368-
}
325+
},
326+
"input"
369327
)]
370328
#[test_case(
371329
Some("input"),
372330
Some(CloudArg::C8y{ profile: Some("foo".parse().unwrap()) }),
373331
toml::toml!{
374332
[c8y.profiles.foo.device]
375333
id = "c8y-foo-test"
334+
},
335+
"input"
336+
)]
337+
fn validate_get_device_id_returns_ok(
338+
input_id: Option<&str>,
339+
cloud_arg: Option<CloudArg>,
340+
toml: toml::Table,
341+
expected: &str,
342+
) {
343+
let cloud: Option<Cloud> = cloud_arg.map(<_>::try_into).transpose().unwrap();
344+
let ttd = TempTedgeDir::new();
345+
ttd.file("tedge.toml").with_toml_content(toml);
346+
let location = TEdgeConfigLocation::from_custom_root(ttd.path());
347+
let reader = location.load().unwrap();
348+
let id = input_id.map(|s| s.to_string());
349+
let result = get_device_id(id, &reader, &cloud);
350+
assert_eq!(result.unwrap().as_str(), expected);
351+
}
352+
353+
#[test_case(
354+
None,
355+
None,
356+
toml::toml!{
357+
[device]
376358
}
377359
)]
378360
fn validate_get_device_id_returns_err(
@@ -386,7 +368,7 @@ mod tests {
386368
let location = TEdgeConfigLocation::from_custom_root(ttd.path());
387369
let reader = location.load().unwrap();
388370
let id = input_id.map(|s| s.to_string());
389-
let result = get_device_id(id, &reader, &location, &cloud);
371+
let result = get_device_id(id, &reader, &cloud);
390372
assert!(result.is_err());
391373
}
392374
}

‎crates/core/tedge/src/cli/certificate/create.rs‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ use std::fs::File;
1111
use std::fs::OpenOptions;
1212
use std::io::prelude::*;
1313
use std::path::Path;
14-
use tedge_config::TEdgeConfigLocation;
1514
use tedge_utils::paths::set_permission;
1615
use tedge_utils::paths::validate_parent_dir_exists;
1716

@@ -29,9 +28,6 @@ pub struct CreateCertCmd {
2928
/// The owner of the private key
3029
pub user: String,
3130
pub group: String,
32-
33-
/// The tedge.toml file location, required to access to TEdgeConfigDto
34-
pub config_location: TEdgeConfigLocation,
3531
}
3632

3733
impl Command for CreateCertCmd {
@@ -194,7 +190,6 @@ mod tests {
194190
key_path: key_path.clone(),
195191
user: "mosquitto".to_string(),
196192
group: "mosquitto".to_string(),
197-
config_location: TEdgeConfigLocation::from_custom_root(dir.path()),
198193
};
199194

200195
assert_matches!(
@@ -224,7 +219,6 @@ mod tests {
224219
key_path: key_path.clone(),
225220
user: "mosquitto".to_string(),
226221
group: "mosquitto".to_string(),
227-
config_location: TEdgeConfigLocation::from_custom_root(dir.path()),
228222
};
229223

230224
assert!(cmd
@@ -248,7 +242,6 @@ mod tests {
248242
key_path,
249243
user: "mosquitto".to_string(),
250244
group: "mosquitto".to_string(),
251-
config_location: TEdgeConfigLocation::from_custom_root(dir.path()),
252245
};
253246

254247
let cert_error = cmd
@@ -269,7 +262,6 @@ mod tests {
269262
key_path,
270263
user: "mosquitto".to_string(),
271264
group: "mosquitto".to_string(),
272-
config_location: TEdgeConfigLocation::from_custom_root(dir.path()),
273265
};
274266

275267
let cert_error = cmd

‎crates/core/tedge/src/cli/certificate/create_csr.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,6 @@ mod tests {
7171
use crate::CreateCertCmd;
7272
use assert_matches::assert_matches;
7373
use std::path::Path;
74-
use tedge_config::TEdgeConfigLocation;
7574
use tempfile::*;
7675
use x509_parser::der_parser::asn1_rs::FromDer;
7776
use x509_parser::nom::AsBytes;
@@ -114,7 +113,6 @@ mod tests {
114113
key_path: key_path.clone(),
115114
user: "mosquitto".to_string(),
116115
group: "mosquitto".to_string(),
117-
config_location: TEdgeConfigLocation::from_custom_root(dir.path()),
118116
};
119117

120118
// create private key and public cert with standard command

‎crates/core/tedge/src/cli/certificate/renew.rs‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,6 @@ mod tests {
5959
use std::path::Path;
6060
use std::thread::sleep;
6161
use std::time::Duration;
62-
use tedge_config::TEdgeConfigLocation;
6362
use tempfile::*;
6463

6564
#[test]
@@ -74,7 +73,6 @@ mod tests {
7473
key_path: key_path.clone(),
7574
user: "mosquitto".to_string(),
7675
group: "mosquitto".to_string(),
77-
config_location: TEdgeConfigLocation::from_custom_root(dir.path()),
7876
};
7977

8078
// First create both cert and key

0 commit comments

Comments
 (0)