Skip to content

Commit e727c2d

Browse files
Merge pull request #3525 from didier-wenzek/fix/cert-renew-exit-code
fix: tedge cert renew returning success exit code on error
2 parents 8d5f4ca + f34c00a commit e727c2d

File tree

8 files changed

+101
-66
lines changed

8 files changed

+101
-66
lines changed

crates/core/c8y_api/src/http_proxy.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,17 +69,19 @@ impl C8yEndPoint {
6969
c8y_profile: Option<&str>,
7070
) -> Result<Self, C8yEndPointConfigError> {
7171
let c8y_config = tedge_config.c8y.try_get(c8y_profile)?;
72-
let c8y_host = c8y_config.proxy.client.host.to_string();
73-
let c8y_mqtt_host = c8y_host.clone();
7472
let auth_proxy_addr = c8y_config.proxy.client.host.clone();
7573
let auth_proxy_port = c8y_config.proxy.client.port;
7674
let auth_proxy_protocol = c8y_config
7775
.proxy
7876
.cert_path
7977
.or_none()
8078
.map_or(Protocol::Http, |_| Protocol::Https);
79+
let c8y_host = format!(
80+
"{}://{auth_proxy_addr}:{auth_proxy_port}",
81+
auth_proxy_protocol.as_str()
82+
);
83+
let c8y_mqtt_host = c8y_host.clone();
8184
let proxy = ProxyUrlGenerator::new(auth_proxy_addr, auth_proxy_port, auth_proxy_protocol);
82-
8385
Ok(C8yEndPoint {
8486
c8y_host,
8587
c8y_mqtt_host,

crates/core/tedge/src/cli/certificate/c8y/download.rs

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ impl Command for DownloadCertCmd {
7373

7474
impl DownloadCertCmd {
7575
async fn download_device_certificate(&self) -> Result<(), Error> {
76-
let (common_name, security_token) = self.get_registration_data()?;
76+
let (common_name, security_token) = self.get_registration_data().await?;
7777
if self.generate_csr {
7878
create_device_csr(
7979
common_name.clone(),
@@ -129,25 +129,30 @@ impl DownloadCertCmd {
129129
/// Prompt the user for the device id and the security token
130130
///
131131
/// - unless already set on the command line or using env variables.
132-
fn get_registration_data(&self) -> Result<(String, String), std::io::Error> {
133-
let device_id = if self.device_id.is_empty() {
134-
print!("Enter device id: ");
135-
std::io::stdout().flush()?;
136-
let mut input = String::new();
137-
std::io::stdin().read_line(&mut input)?;
138-
input.trim_end_matches(['\n', '\r']).to_string()
139-
} else {
140-
self.device_id.clone()
141-
};
142-
143-
// Read the security token from /dev/tty
144-
let security_token = if self.security_token.is_empty() {
145-
rpassword::read_password_from_tty(Some("Enter security token: "))?
146-
} else {
147-
self.security_token.clone()
148-
};
149-
150-
Ok((device_id, security_token))
132+
async fn get_registration_data(&self) -> Result<(String, String), std::io::Error> {
133+
let self_device_id = self.device_id.clone();
134+
let self_security_token = self.security_token.clone();
135+
tokio::task::spawn_blocking(move || {
136+
let device_id = if self_device_id.is_empty() {
137+
print!("Enter device id: ");
138+
std::io::stdout().flush()?;
139+
let mut input = String::new();
140+
std::io::stdin().read_line(&mut input)?;
141+
input.trim_end_matches(['\n', '\r']).to_string()
142+
} else {
143+
self_device_id
144+
};
145+
146+
// Read the security token from /dev/tty
147+
let security_token = if self_security_token.is_empty() {
148+
rpassword::read_password_from_tty(Some("Enter security token: "))?
149+
} else {
150+
self_security_token
151+
};
152+
153+
Ok((device_id, security_token))
154+
})
155+
.await?
151156
}
152157

153158
/// Post the device CSR

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

Lines changed: 19 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@ use crate::cli::certificate::c8y::create_device_csr;
33
use crate::cli::certificate::c8y::read_csr_from_file;
44
use crate::cli::certificate::c8y::store_device_cert;
55
use crate::command::Command;
6-
use crate::error;
76
use crate::get_webpki_error_from_reqwest;
87
use crate::log::MaybeFancy;
8+
use anyhow::anyhow;
99
use anyhow::Error;
1010
use c8y_api::http_proxy::C8yEndPoint;
1111
use camino::Utf8PathBuf;
@@ -47,7 +47,10 @@ pub struct RenewCertCmd {
4747
#[async_trait::async_trait]
4848
impl Command for RenewCertCmd {
4949
fn description(&self) -> String {
50-
format!("Renew the device certificate from {}", self.c8y_url())
50+
format!(
51+
"renew the device certificate via Cumulocity HTTP proxy {}",
52+
self.c8y_url()
53+
)
5154
}
5255

5356
async fn execute(&self) -> Result<(), MaybeFancy<Error>> {
@@ -89,29 +92,23 @@ impl RenewCertCmd {
8992
let url = Url::parse(&url)?;
9093
let result = self.post_device_csr(&http, &url, &csr).await;
9194
match result {
92-
Ok(response) if response.status() == StatusCode::OK => {
93-
if let Ok(cert) = response.text().await {
95+
Ok(response) if response.status() == StatusCode::OK => match response.text().await {
96+
Ok(cert) => {
9497
store_device_cert(&self.cert_path, cert).await?;
95-
return Ok(());
98+
Ok(())
9699
}
97-
error!("Fail to extract a certificate from the response returned by {url}");
98-
}
99-
Ok(response) => {
100-
error!(
101-
"The device certificate cannot be renewed from {url}:\n\t{} {}",
102-
response.status(),
103-
response.text().await.unwrap_or("".to_string())
104-
);
105-
}
106-
Err(err) => {
107-
error!(
108-
"Fail to connect to {url}: {:?}",
109-
get_webpki_error_from_reqwest(err)
110-
)
111-
}
100+
Err(err) => Err(anyhow!(
101+
"Fail to extract a certificate from the response: {err}"
102+
)),
103+
},
104+
Ok(response) => Err(anyhow!(
105+
"The request failed with {}:\n\t{}",
106+
response.status(),
107+
response.text().await.unwrap_or("".to_string())
108+
)),
109+
Err(err) => Err(Error::new(get_webpki_error_from_reqwest(err))
110+
.context(format!("Fail to connect to Cumulocity HTTP proxy {url}"))),
112111
}
113-
114-
Ok(())
115112
}
116113

117114
/// Post the device CSR

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,13 @@ pub async fn reuse_private_key(key_path: &Utf8PathBuf) -> Result<KeyKind, std::i
166166
tokio::fs::read_to_string(key_path)
167167
.await
168168
.map(|keypair_pem| KeyKind::Reuse { keypair_pem })
169+
.or_else(|err| {
170+
if key_path.exists() {
171+
Err(err)
172+
} else {
173+
Ok(KeyKind::New)
174+
}
175+
})
169176
}
170177

171178
async fn persist_private_key(

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,10 @@ impl CreateCsrCmd {
4747
let csr_path = &self.csr_path;
4848
let key_path = &self.key_path;
4949

50-
let previous_key = reuse_private_key(key_path).await.unwrap_or(KeyKind::New);
50+
let previous_key = reuse_private_key(key_path)
51+
.await
52+
.map_err(|e| CertError::IoError(e).key_context(key_path.clone()))?;
53+
5154
let cert =
5255
KeyCertPair::new_certificate_sign_request(&self.csr_template, id, &previous_key)?;
5356

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

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ pub enum CertError {
2525
)]
2626
CertificateNotFound { path: Utf8PathBuf },
2727

28+
#[error("I/O error accessing the certificate: {path:?}")]
29+
CertificateIoError {
30+
path: Utf8PathBuf,
31+
#[source]
32+
source: std::io::Error,
33+
},
34+
2835
#[error(
2936
r#"No private key has been attached to that device.
3037
Missing file: {path:?}
@@ -41,6 +48,13 @@ pub enum CertError {
4148
)]
4249
KeyAlreadyExists { path: Utf8PathBuf },
4350

51+
#[error("I/O error accessing the private key: {path:?}")]
52+
KeyIoError {
53+
path: Utf8PathBuf,
54+
#[source]
55+
source: std::io::Error,
56+
},
57+
4458
#[error(transparent)]
4559
ConfigError(#[from] crate::ConfigError),
4660

@@ -62,15 +76,15 @@ pub enum CertError {
6276
#[error(transparent)]
6377
CertificateError(#[from] certificate::CertificateError),
6478

65-
#[error(
66-
r#"Certificate read error at: {1:?}
67-
Run `tedge cert create` if you want to create a new certificate."#
68-
)]
69-
CertificateReadFailed(#[source] std::io::Error, String),
70-
7179
#[error(transparent)]
7280
PathsError(#[from] PathsError),
7381

82+
#[error("Connection error: {0}")]
83+
ReqwestConnect(String),
84+
85+
#[error("Request time out")]
86+
ReqwestTimeout,
87+
7488
#[error(transparent)]
7589
ReqwestError(#[from] reqwest::Error),
7690

@@ -100,10 +114,10 @@ impl CertError {
100114
/// Improve the error message in case the error in a IO error on the certificate file.
101115
pub fn cert_context(self, path: Utf8PathBuf) -> CertError {
102116
match self {
103-
CertError::IoError(ref err) => match err.kind() {
117+
CertError::IoError(err) => match err.kind() {
104118
std::io::ErrorKind::AlreadyExists => CertError::CertificateAlreadyExists { path },
105119
std::io::ErrorKind::NotFound => CertError::CertificateNotFound { path },
106-
_ => self,
120+
_ => CertError::CertificateIoError { path, source: err },
107121
},
108122
_ => self,
109123
}
@@ -112,10 +126,10 @@ impl CertError {
112126
/// Improve the error message in case the error in a IO error on the private key file.
113127
pub fn key_context(self, path: Utf8PathBuf) -> CertError {
114128
match self {
115-
CertError::IoError(ref err) => match err.kind() {
129+
CertError::IoError(err) => match err.kind() {
116130
std::io::ErrorKind::AlreadyExists => CertError::KeyAlreadyExists { path },
117131
std::io::ErrorKind::NotFound => CertError::KeyNotFound { path },
118-
_ => self,
132+
_ => CertError::KeyIoError { path, source: err },
119133
},
120134
_ => self,
121135
}
@@ -153,6 +167,16 @@ pub fn get_webpki_error_from_reqwest(err: reqwest::Error) -> CertError {
153167
{
154168
CertError::CertificateError(tls_error)
155169
} else {
156-
CertError::ReqwestError(err) // any other Error type than `hyper::Error`
170+
// any other Error type than `hyper::Error`
171+
if err.is_connect() {
172+
match err.source().and_then(|err| err.source()) {
173+
Some(io_error) => CertError::ReqwestConnect(format!("{io_error}")),
174+
None => CertError::ReqwestError(err),
175+
}
176+
} else if err.is_timeout() {
177+
CertError::ReqwestTimeout
178+
} else {
179+
CertError::ReqwestError(err)
180+
}
157181
}
158182
}

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
pub use self::cli::TEdgeCertCli;
2-
use std::path::Path;
2+
use camino::Utf8Path;
33
use tokio::io::AsyncReadExt;
44

55
mod c8y;
@@ -15,10 +15,12 @@ pub use self::cli::*;
1515
pub use self::create::*;
1616
pub use self::error::*;
1717

18-
pub(crate) async fn read_cert_to_string(path: impl AsRef<Path>) -> Result<String, CertError> {
18+
pub(crate) async fn read_cert_to_string(path: impl AsRef<Utf8Path>) -> Result<String, CertError> {
1919
let mut file = tokio::fs::File::open(path.as_ref()).await.map_err(|err| {
20-
let path = path.as_ref().display().to_string();
21-
CertError::CertificateReadFailed(err, path)
20+
CertError::CertificateIoError {
21+
source: err,
22+
path: path.as_ref().to_owned(),
23+
}
2224
})?;
2325
let mut content = String::new();
2426
file.read_to_string(&mut content).await?;

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,6 @@ impl RenewCertCmd {
3939
let key_path = &self.key_path;
4040
let id = certificate_cn(cert_path).await?;
4141

42-
// Remove only certificate
43-
tokio::fs::remove_file(&self.cert_path)
44-
.await
45-
.map_err(|e| CertError::IoError(e).cert_context(self.cert_path.clone()))?;
46-
4742
// Re-create the certificate from the key, with new validity
4843
let previous_key = reuse_private_key(key_path)
4944
.await

0 commit comments

Comments
 (0)