Refactoring get logic

Author: Anish Cheraku

sources/Cargo.lock

@@ -15,6 +15,9 @@ tls = ["dep:rustls", "dep:aws-lc-rs", "reqwest/rustls-tls-native-roots"]
 fips = ["tls", "aws-lc-rs/fips", "rustls/fips"]
 
 [dependencies]
+async-trait = "0.1"
+aws-smithy-types.workspace = true
+aws-config.workspace = true
 aws-lc-rs = { workspace = true, optional = true, features = ["bindgen"] }
 base64.workspace = true
 constants.workspace = true
@@ -31,7 +34,7 @@ log.workspace = true
 models.workspace = true
 nix.workspace = true
 rand = { workspace = true, features = ["default"] }
-reqwest.workspace = true
+reqwest = {features = ["blocking", "json"], workspace = true }
 retry-read.workspace = true
 rustls = { workspace = true, optional = true }
 serde = { workspace = true, features = ["derive"] }

sources/api/apiclient/Cargo.toml

@@ -2,15 +2,18 @@
 //! 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 crate::uri_resolver::{StdinUri, FileUri, HttpUri, UriResolver};
+use crate::apply::error::{UriSnafu, NoResolverSnafu};
 use crate::rando;
 use futures::future::{join, ready, TryFutureExt};
 use futures::stream::{self, StreamExt};
 use reqwest::Url;
 use serde::de::{Deserialize, IntoDeserializer};
 use snafu::{futures::try_future::TryFutureExt as SnafuTryFutureExt, OptionExt, ResultExt};
-use std::path::Path;
+use std::{convert::TryFrom, path::Path};
 use tokio::io::AsyncReadExt;
 
+
 /// 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,46 +70,35 @@ where
 }
 
 /// Retrieves the given source location and returns the result in a String.
-async fn get<S>(input_source: S) -> Result<String>
-where
-    S: Into<String>,
-{
-    let input_source = input_source.into();
-
-    // Read from stdin if "-" was given.
-    if input_source == "-" {
-        let mut output = String::new();
-        tokio::io::stdin()
-            .read_to_string(&mut output)
-            .context(error::StdinReadSnafu)
-            .await?;
-        return Ok(output);
+pub async fn get(input: &str) -> Result<String> {
+    let resolver = select_resolver(input)?;
+    resolver.resolve().await
+}
+
+/// Choose which UriResolver applies to `input` (stdin, file://, http(s)://).
+fn select_resolver(input: &str) -> Result<Box<dyn UriResolver>> {
+    // 1) "-" → stdin
+    if let Ok(r) = StdinUri::try_from(input) {
+        return Ok(Box::new(r));
     }
 
-    // Otherwise, the input should be a URI; parse it to know what kind.
-    // Until reqwest handles file:// URIs: https://github.com/seanmonstar/reqwest/issues/178
-    let uri = Url::parse(&input_source).context(error::UriSnafu {
-        input_source: &input_source,
-    })?;
-    if uri.scheme() == "file" {
-        // Turn the URI to a file path, and return a future that reads it.
-        let path = uri.to_file_path().ok().context(error::FileUriSnafu {
-            input_source: &input_source,
-        })?;
-        tokio::fs::read_to_string(path)
-            .context(error::FileReadSnafu { input_source })
-            .await
-    } else {
-        // Return a future that contains the text of the (non-file) URI.
-        reqwest::get(uri)
-            .and_then(|response| ready(response.error_for_status()))
-            .and_then(|response| response.text())
-            .context(error::ReqwestSnafu {
-                uri: input_source,
-                method: "GET",
-            })
-            .await
+    // 2) parse as a URL
+    let url = Url::parse(input).context(UriSnafu { input_source: input.to_string() })?;
+
+    // 3) file://
+    if let Ok(r) = FileUri::try_from(url.clone()) {
+        return Ok(Box::new(r));
     }
+
+    // 4) http(s)://
+    if let Ok(r) = HttpUri::try_from(url.clone()) {
+        return Ok(Box::new(r));
+    }
+
+    NoResolverSnafu {
+        input_source: input.to_string(),
+    }
+    .fail()
 }
 
 /// Takes a string of TOML or JSON settings data and reserializes
@@ -142,11 +134,11 @@ fn format_change(input: &str, input_source: &str) -> Result<String> {
     serde_json::to_string(&json_inner).context(error::JsonSerializeSnafu { input_source })
 }
 
-mod error {
+pub(crate) mod error {
     use snafu::Snafu;
 
     #[derive(Debug, Snafu)]
-    #[snafu(visibility(pub(super)))]
+    #[snafu(visibility(pub(crate)))]
     pub enum Error {
         #[snafu(display("Failed to commit combined settings to '{}': {}", uri, source))]
         CommitApply {
@@ -164,6 +156,9 @@ mod error {
         #[snafu(display("Given invalid file URI '{}'", input_source))]
         FileUri { input_source: String },
 
+        #[snafu(display("No URI resolver for '{}'", input_source))]
+        NoResolver { input_source: String },
+
         #[snafu(display(
             "Input '{}' is not valid TOML or JSON.  (TOML error: {})  (JSON error: {})",
             input_source,
@@ -218,6 +213,12 @@ mod error {
             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 },
 

sources/api/apiclient/src/apply.rs

@@ -27,6 +27,7 @@ pub mod reboot;
 pub mod report;
 pub mod set;
 pub mod update;
+pub mod uri_resolver;
 
 mod error {
     use snafu::Snafu;

sources/api/apiclient/src/lib.rs

@@ -0,0 +1,124 @@
+// src/uri_resolver.rs
+
+use async_trait::async_trait;
+use snafu::{ensure, ResultExt, OptionExt};
+use std::{convert::TryFrom, path::PathBuf};
+use tokio::io::AsyncReadExt;
+
+use reqwest::Url;
+use crate::apply::{Error, Result};
+use crate::apply::error::{
+    FileReadSnafu, FileUriSnafu, ReqwestSnafu, StdinReadSnafu, InvalidFileUriSnafu, InvalidHTTPUriSnafu
+};
+
+/// Anything that can fetch itself as a UTF-8 `String`.
+#[async_trait]
+pub trait UriResolver {
+    /// Fetches the contents of this URI as a `String`.
+    async fn resolve(&self) -> Result<String>;
+}
+
+/// “-” → standard input
+pub struct StdinUri;
+
+impl TryFrom<&str> for StdinUri {
+    type Error = ();
+
+    fn try_from(s: &str) -> std::result::Result<Self, Self::Error> {
+        if s == "-" {
+            Ok(StdinUri)
+        } else {
+            Err(())
+        }
+    }
+}
+
+#[async_trait]
+impl UriResolver for StdinUri {
+    async fn resolve(&self) -> Result<String> {
+        let mut buf = String::new();
+        tokio::io::stdin()
+            .read_to_string(&mut buf)
+            .await
+            .context(StdinReadSnafu)?;
+        Ok(buf)
+    }
+}
+
+/// file:// URLs map to local filesystem paths
+pub struct FileUri {
+    path: PathBuf,
+}
+
+impl TryFrom<Url> for FileUri {
+    type Error = Error;
+
+    fn try_from(url: Url) -> std::result::Result<Self, Self::Error> {
+        // only accept file://
+        ensure!(
+            url.scheme() == "file",
+            InvalidFileUriSnafu { input_source: url.to_string() }
+        );
+
+        // convert to PathBuf or error
+        let path = url
+            .to_file_path()
+            .ok()
+            .context(FileUriSnafu { input_source: url.to_string() })?;
+
+        Ok(FileUri { path })
+    }
+}
+
+#[async_trait]
+impl UriResolver for FileUri {
+    async fn resolve(&self) -> Result<String> {
+        tokio::fs::read_to_string(&self.path)
+            .await
+            .context(FileReadSnafu {
+                input_source: self.path.to_string_lossy().into_owned(),
+            })
+    }
+}
+
+/// http:// or https:// URLs
+pub struct HttpUri {
+    url: Url,
+}
+
+impl TryFrom<Url> for HttpUri {
+    type Error = Error;
+
+    fn try_from(url: Url) -> std::result::Result<Self, Self::Error> {
+        ensure!(
+            url.scheme() == "http" || url.scheme() == "https",
+            InvalidHTTPUriSnafu { input_source: url.to_string() }
+        );
+        Ok(HttpUri { url })
+    }
+}
+
+#[async_trait]
+impl UriResolver for HttpUri {
+    async fn resolve(&self) -> Result<String> {
+        let resp = reqwest::get(self.url.clone())
+            .await
+            .context(ReqwestSnafu {
+                uri: self.url.to_string(),
+                method: "GET".to_string(),
+            })?
+            .error_for_status()
+            .context(ReqwestSnafu {
+                uri: self.url.to_string(),
+                method: "GET".to_string(),
+            })?;
+
+        resp.text()
+            .await
+            .context(ReqwestSnafu {
+                uri: self.url.to_string(),
+                method: "GET".to_string(),
+            })
+    }
+}
+

sources/api/apiclient/src/uri_resolver.rs

@@ -26,6 +26,13 @@ license-files = [
     { path = "LICENSE-MIT", hash = 0xfeb1e4a7 },
 ]
 
+[clarify.aws-sdk-s3]
+expression = "Apache-2.0"
+license-files = [
+    { path = "LICENSE", hash = 0x1b71bb24 }
+]
+skip-files = [ "tests/blns/LICENSE" ] # we do not vend this as it's just used in s3's tests
+
 [clarify.backtrace-sys]
 # backtrace-sys is MIT/Apache-2.0, libbacktrace is BSD-3-Clause
 expression = "(MIT OR Apache-2.0) AND BSD-3-Clause"