fix: support relative path args when using tedge cert#3564
fix: support relative path args when using tedge cert#3564reubenmiller merged 3 commits intothin-edge:mainfrom
Conversation
Robot Results
|
| tedge configure MQTT client authentication | ||
| Execute Command tedge config set mqtt.client.auth.cert_file client.crt | ||
| Execute Command tedge config set mqtt.client.auth.key_file client.key | ||
| Execute Command tedge config set mqtt.client.auth.cert_file "$(pwd)/client.crt" |
There was a problem hiding this comment.
I thought that relative paths would still be accepted in the CLI, but its absolute form (after resolving it) would be stored in the TOML file.
There was a problem hiding this comment.
tedge config set expects absolute paths and doesn't try to convert a relative path into an absolute form that would be stored in the TOML file.
There was a problem hiding this comment.
Here a proposal to implement this idea to normalize a path configured using tedge config set
rina23q
left a comment
There was a problem hiding this comment.
The changes really make sense. CLI accepts both absolute and relative paths while tedge config accepts only absolute.
368ec48 to
1ce240b
Compare
albinsuresh
left a comment
There was a problem hiding this comment.
LGTM. Just one minor remark on an error message.
| Ok(AbsolutePath(path)) | ||
| } | ||
| let Ok(Ok(pwd)) = std::env::current_dir().map(Utf8PathBuf::try_from) else { | ||
| return Err(InvalidAbsolutePath(path)); |
There was a problem hiding this comment.
Minor:
| return Err(InvalidAbsolutePath(path)); | |
| return Err(InvalidRelativePath(path)); |
Else the error message will be slightly misleading, right? As the issue is not at all with the input path but from the derived pwd.
There was a problem hiding this comment.
I changed the error message to "Cannot be converted to an absolute path" but let the error type name unchanged because it makes sense for AbsolutePath::try_new to fail with InvalidAbsolutePath.
See 4134c9e
| fn clean_utf8_path(path: &Utf8Path) -> Utf8PathBuf { | ||
| Utf8PathBuf::from(path_clean::clean(path.as_str())) | ||
| // unwrap is safe because clean returns an utf8 path when given an utf8 path | ||
| Utf8PathBuf::try_from(path_clean::clean(path.as_std_path())).unwrap() |
There was a problem hiding this comment.
I'm assuming that we're using path_clean::clean() instead of std::path::absolute() because our MSRV is still 1.78 and the latter is only available from 1.79.
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
Signed-off-by: Didier Wenzek <didier.wenzek@free.fr>
4134c9e to
4e350e1
Compare
Proposed changes
Allow users to provide a relative path when using
tedge certsub-commands acreate-csrorshow.tedge config setto make sure no relative paths is stored in the TOML config (because used later from any working directory)tedge config setto store cannonalized paths when given relative pathstedge certthe check that was preventing a path argument to be relative.Types of changes
Paste Link to the issue
#3547
Checklist
just prepare-devonce)just formatas mentioned in CODING_GUIDELINESjust checkas mentioned in CODING_GUIDELINESFurther comments