adress code review suggestions #2

Author: Anish Cheraku

sources/Cargo.lock

@@ -16,7 +16,6 @@ fips = ["tls", "aws-lc-rs/fips", "rustls/fips"]
 
 [dependencies]
 async-trait.workspace = true
-aws-smithy-types.workspace = true
 aws-config.workspace = true
 aws-lc-rs = { workspace = true, optional = true, features = ["bindgen"] }
 aws-sdk-s3.workspace = true
@@ -37,14 +36,15 @@ log.workspace = true
 models.workspace = true
 nix.workspace = true
 rand = { workspace = true, features = ["default"] }
-reqwest.workspace = true 
+reqwest.workspace = true
 retry-read.workspace = true
 rustls = { workspace = true, optional = true }
 serde = { workspace = true, features = ["derive"] }
 serde_json.workspace = true
 signal-hook.workspace = true
 simplelog.workspace = true
 snafu = { workspace = true, features = ["futures"] }
+test-case = "*"
 tokio = { workspace = true, features = ["fs", "io-std", "io-util", "macros", "rt-multi-thread", "time"] }
 tokio-tungstenite = { workspace = true, features = ["connect"] }
 toml.workspace = true

sources/api/apiclient/Cargo.toml

@@ -1,7 +1,7 @@
 //! This module allows application of settings from URIs or stdin.  The inputs are expected to be
 //! TOML settings files, in the same format as user data, or the JSON equivalent.  The inputs are
 //! pulled and applied to the API server in a single transaction.
-use aws_sdk_secretsmanager::config::http::HttpResponse as SdkHttpResponse;
+use crate::apply::error::ResolverFailureSnafu;
 use crate::rando;
 use futures::future::{join, ready};
 use futures::stream::{self, StreamExt};
@@ -11,7 +11,6 @@ use snafu::{OptionExt, ResultExt};
 use std::convert::TryFrom;
 use std::path::Path;
 
-
 /// Reads settings in TOML or JSON format from files at the requested URIs (or from stdin, if given
 /// "-"), then commits them in a single transaction and applies them to the system.
 pub async fn apply<P>(socket_path: P, input_sources: Vec<String>) -> Result<()>
@@ -67,53 +66,65 @@ where
     Ok(())
 }
 
+/// Holds the raw input string and (if it parses) the URL.
+pub struct SettingsInput {
+    pub input: String,
+    pub parsed_url: Option<Url>,
+}
+impl SettingsInput {
+    pub(crate) fn new(input: impl Into<String>) -> Self {
+        let input = input.into();
+        let parsed_url = Url::parse(&input).ok();
+        SettingsInput { input, parsed_url }
+    }
+}
+
 /// Retrieves the given source location and returns the result in a String.
 async fn get<S>(input_source: S) -> Result<String>
 where
     S: AsRef<str>,
 {
-    let resolver = select_resolver(input_source.as_ref())?;
-    resolver.resolve().await
+    let settings = SettingsInput::new(input_source.as_ref());
+    let resolver = select_resolver(&settings)?;
+    resolver.resolve().await.context(ResolverFailureSnafu)
 }
 
+/// Choose which UriResolver applies to `input` (stdin, file://, http(s)://, s3://, secretsmanager://, and ssm://).
+fn select_resolver(input: &SettingsInput) -> Result<Box<dyn crate::uri_resolver::UriResolver>> {
+    use crate::uri_resolver;
 
-/// Choose which UriResolver applies to `input` (stdin, file://, http(s):// or s3://).
-fn select_resolver(input: &str) -> Result<Box<dyn crate::uri_resolver::UriResolver>> {
-    // "-" → stdin
-    if let Ok(r) = crate::uri_resolver::StdinUri::try_from(input) {
+    // stdin ("-")
+    if let Ok(r) = uri_resolver::StdinUri::try_from(input) {
         return Ok(Box::new(r));
     }
 
-    // parse as a URL
-    let url = Url::parse(input).context(error::UriSnafu { input_source: input.to_string() })?;
-
     // file://
-    if let Ok(r) = crate::uri_resolver::FileUri::try_from(&url) {
+    if let Ok(r) = uri_resolver::FileUri::try_from(input) {
         return Ok(Box::new(r));
     }
 
     // http(s)://
-    if let Ok(r) = crate::uri_resolver::HttpUri::try_from(url.clone()) {
+    if let Ok(r) = uri_resolver::HttpUri::try_from(input) {
         return Ok(Box::new(r));
     }
 
     // s3://
-    if let Ok(r) = crate::uri_resolver::S3Uri::try_from(url.clone()) {
+    if let Ok(r) = uri_resolver::S3Uri::try_from(input) {
         return Ok(Box::new(r));
     }
 
     // secretsmanager://
-    if let Ok(r) = crate::uri_resolver::SecretsManagerUri::try_from(input) {
+    if let Ok(r) = uri_resolver::SecretsManagerUri::try_from(input) {
         return Ok(Box::new(r));
     }
 
     // ssm://
-    if let Ok(r) = crate::uri_resolver::SsmUri::try_from(input) {
+    if let Ok(r) = uri_resolver::SsmUri::try_from(input) {
         return Ok(Box::new(r));
     }
 
     error::NoResolverSnafu {
-        input_source: input.to_string(),
+        input_source: input.input.clone(),
     }
     .fail()
 }
@@ -152,12 +163,6 @@ fn format_change(input: &str, input_source: &str) -> Result<String> {
 }
 
 pub(crate) mod error {
-    use aws_sdk_secretsmanager::operation::get_secret_value::GetSecretValueError;
-    use aws_sdk_ssm::operation::get_parameter::GetParameterError;
-    use crate::apply::SdkHttpResponse;
-    use aws_smithy_runtime_api::client::result::SdkError;
-    use aws_sdk_s3::operation::get_object::GetObjectError;
-    //use aws_smithy_http::result::SdkHttpResponse;
     use snafu::Snafu;
 
     #[derive(Debug, Snafu)]
@@ -171,15 +176,6 @@ pub(crate) mod error {
             source: Box<crate::Error>,
         },
 
-        #[snafu(display("Failed to read given file '{}': {}", input_source, source))]
-        FileRead {
-            input_source: String,
-            source: std::io::Error,
-        },
-
-        #[snafu(display("Given invalid file URI '{}'", input_source))]
-        FileUri { input_source: String },
-
         #[snafu(display("No URI resolver found for '{}'", input_source))]
         NoResolver { input_source: String },
 
@@ -237,77 +233,9 @@ pub(crate) mod error {
             source: reqwest::Error,
         },
 
-        #[snafu(display("Invalid S3 URI '{}': missing bucket name", input_source))]
-        S3UriMissingBucket { input_source: String },
-
-        #[snafu(display("Failed to perform HTTP GET to '{}': {}", uri, source))]
-        HttpRequest {
-            uri: String,
-            source: reqwest::Error,
-        },
-
-        #[snafu(display("Non-success HTTP status from '{}': {}", uri, status))]
-        HttpStatus {
-            uri: String,
-            status: reqwest::StatusCode,
-        },
-
-        #[snafu(display("Failed to read HTTP response body from '{}': {}", uri, source))]
-        HttpBody {
-            uri: String,
-            source: reqwest::Error,
-        },
-
         #[snafu(display("Given invalid file URI '{}'", input_source))]
         InvalidFileUri { input_source: String },
 
-        #[snafu(display("Given HTTP(S) URI '{}'", input_source))]
-        InvalidHTTPUri { input_source: String },
-
-        #[snafu(display("Failed to read standard input: {}", source))]
-        StdinRead { source: std::io::Error },
-
-        #[snafu(display("Failed to read S3 object body '{bucket}/{key}': {}", source))]
-        S3Body {
-            bucket: String,
-            key: String,
-            source: aws_sdk_s3::primitives::ByteStreamError,
-        },
-
-        #[snafu(display("Failed to fetch S3 object '{bucket}/{key}': {}", source))]
-        S3Get {
-            bucket: String,
-            key: String,
-            source: SdkError<GetObjectError, SdkHttpResponse>,
-        },
-
-        #[snafu(display("Invalid S3 URI scheme for '{}', expected s3://", input_source))]
-        S3UriScheme { input_source: String },
-
-        #[snafu(display("Invalid Secrets Manager URI scheme for '{}', expected secretsmanager://", input_source))]
-        SecretsManagerUri { input_source: String },
-
-        #[snafu(display("Invalid SSM URI scheme for '{}', expected ssm://", input_source))]
-        SsmUri { input_source: String },
-
-        #[snafu(display("Failed to fetch secret '{}' from Secrets Manager: {}", secret_id, source))]
-        SecretsManagerGet {
-            secret_id: String,
-            source: aws_sdk_secretsmanager::error::SdkError<GetSecretValueError>,
-        },
-
-        #[snafu(display("Secrets Manager secret '{}' did not return a string value", secret_id))]
-        SecretsManagerStringMissing { secret_id: String },
-
-        #[snafu(display("Failed to fetch parameter '{}' from SSM: {}", parameter_name, source))]
-        SsmGetParameter {
-            parameter_name: String,
-            source: aws_sdk_ssm::error::SdkError<GetParameterError>,
-        },
-        
-        #[snafu(display("SSM parameter '{}' did not return a string value", parameter_name))]
-        SsmParameterMissing { parameter_name: String },
-
         #[snafu(display(
             "Failed to translate TOML from '{}' to JSON for API: {}",
             input_source,
@@ -323,7 +251,13 @@ pub(crate) mod error {
             input_source: String,
             source: url::ParseError,
         },
+
+        #[snafu(display("Resolver failed: {}", source))]
+        ResolverFailure { source: crate::uri_resolver::ResolverError },
+
     }
+
 }
 pub use error::Error;
 pub type Result<T> = std::result::Result<T, error::Error>;
+