apiclient: address code review suggestions

Author: Anish Cheraku

sources/api/apiclient/Cargo.toml

@@ -50,6 +50,7 @@ tokio-tungstenite = { workspace = true, features = ["connect"] }
 toml.workspace = true
 unindent.workspace = true
 url.workspace = true
+aws-smithy-runtime-api.workspace = true
 
 [build-dependencies]
 generate-readme.workspace = true

sources/api/apiclient/src/apply.rs

@@ -1,20 +1,15 @@
 //! 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_smithy_runtime_api::client::result::SdkError;
-use aws_sdk_secretsmanager::operation::get_secret_value::GetSecretValueError;
 use aws_sdk_secretsmanager::config::http::HttpResponse as SdkHttpResponse;
-use aws_sdk_ssm::operation::get_parameter::GetParameterError;
-use crate::uri_resolver::{StdinUri, FileUri, HttpUri, S3Uri, UriResolver, SecretsManagerUri, SsmUri};
 use crate::rando;
-use futures::future::{join, ready, TryFutureExt};
+use futures::future::{join, ready};
 use futures::stream::{self, StreamExt};
 use reqwest::Url;
 use serde::de::{Deserialize, IntoDeserializer};
-use snafu::{futures::try_future::TryFutureExt as SnafuTryFutureExt, OptionExt, ResultExt};
+use snafu::{OptionExt, ResultExt};
 use std::convert::TryFrom;
 use std::path::Path;
-use tokio::io::AsyncReadExt;
 
 
 /// Reads settings in TOML or JSON format from files at the requested URIs (or from stdin, if given
@@ -73,43 +68,47 @@ where
 }
 
 /// Retrieves the given source location and returns the result in a String.
-pub async fn get(input: &str) -> Result<String> {
-    let resolver = select_resolver(input)?;
+async fn get<S>(input_source: S) -> Result<String>
+where
+    S: AsRef<str>,
+{
+    let resolver = select_resolver(input_source.as_ref())?;
     resolver.resolve().await
 }
 
+
 /// Choose which UriResolver applies to `input` (stdin, file://, http(s):// or s3://).
-fn select_resolver(input: &str) -> Result<Box<dyn UriResolver>> {
-    // 1) "-" → stdin
-    if let Ok(r) = StdinUri::try_from(input) {
+fn select_resolver(input: &str) -> Result<Box<dyn crate::uri_resolver::UriResolver>> {
+    // "-" → stdin
+    if let Ok(r) = crate::uri_resolver::StdinUri::try_from(input) {
         return Ok(Box::new(r));
     }
 
-    // 2) parse as a URL
+    // parse as a URL
     let url = Url::parse(input).context(error::UriSnafu { input_source: input.to_string() })?;
 
-    // 3) file://
-    if let Ok(r) = FileUri::try_from(&url) {
+    // file://
+    if let Ok(r) = crate::uri_resolver::FileUri::try_from(&url) {
         return Ok(Box::new(r));
     }
 
-    // 4) http(s)://
-    if let Ok(r) = HttpUri::try_from(url.clone()) {
+    // http(s)://
+    if let Ok(r) = crate::uri_resolver::HttpUri::try_from(url.clone()) {
         return Ok(Box::new(r));
     }
 
-    // 5) s3://
-    if let Ok(r) = S3Uri::try_from(url.clone()) {
+    // s3://
+    if let Ok(r) = crate::uri_resolver::S3Uri::try_from(url.clone()) {
         return Ok(Box::new(r));
     }
 
-    // 6) secretsmanager://
-    if let Ok(r) = SecretsManagerUri::try_from(input) {
+    // secretsmanager://
+    if let Ok(r) = crate::uri_resolver::SecretsManagerUri::try_from(input) {
         return Ok(Box::new(r));
     }
 
-     // 6) ssm://
-    if let Ok(r) = SsmUri::try_from(input) {
+    // ssm://
+    if let Ok(r) = crate::uri_resolver::SsmUri::try_from(input) {
         return Ok(Box::new(r));
     }
 
@@ -155,6 +154,10 @@ 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)]
@@ -237,6 +240,24 @@ pub(crate) mod 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 },
 
@@ -246,6 +267,20 @@ pub(crate) mod error {
         #[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 },