Refactoring `apiclient apply`'s get logic and `apiclient apply` support for S3, SSM, and Secrets Manager URIs #554

anishcheraku · 2025-06-20T22:32:05Z

Issue number: 4208

Description of changes: Refactored the get logic for apiclient apply, implemented Structs for each of the resolver types, and added support for S3, SSM, Secrets Manager URIs as well as SSM and Secrets Manager ARNs to apiclient apply

Testing done: Manual Integration Testing for fetching and modifying configuration data from all sources (Stdin, File, HTTP/HTTPS, S3, SSM, Secrets Manager). Added Unit Tests to verify that the correct resolver is selected, positive and negative test cases to verify that URIs/ARNs are parsed correctly/throw an error, test cases to verify that S3 URIs with special characters and special URIs will be parsed correctly, and added a Unit Test to verify that files are parsed properly using Tempfile.

Terms of contribution:

By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.

jmt-lab · 2025-06-24T22:55:03Z

sources/api/apiclient/src/uri_resolver.rs

+use reqwest::Url;
+use crate::apply::{Error, Result};
+use crate::apply::error::{
+    FileReadSnafu, FileUriSnafu, ReqwestSnafu, StdinReadSnafu, S3UriMissingBucketSnafu, InvalidFileUriSnafu, InvalidHTTPUriSnafu, S3UriSchemeSnafu


Why do we have both FileUriSnafu and InvalidFileUriSnafu
which is a bit confusing? Can we differentiate these error variants?

Fixed - Got rid of InvalidFileUriSnafu because it did the same thing as FileUriSnafu and to maintain consistency with other resolvers.

jmt-lab · 2025-06-24T22:55:21Z

sources/api/apiclient/src/uri_resolver.rs

+            .await
+            .context(ReqwestSnafu {
+                uri: self.url.to_string(),
+                method: "GET".to_string(),


You don't need .to_string() here

Fixed - Addressing this in another comment: #554 (comment)

jmt-lab · 2025-06-24T22:56:03Z

sources/api/apiclient/src/uri_resolver.rs

+#[async_trait]
+impl UriResolver for S3Uri {
+    async fn resolve(&self) -> Result<String> {
+        // still unimplemented


We generally don't want to merge a PR with unimplemented methods like this, but since this is a draft I guess its alright for now

Agree with Jarrett, but as another tip, in rust you can use the unimplemented!() macro in functions to avoid having to "make stuff up" Like this:

async fn resolve(&self) -> Result<String> { unimplemented!() }

Instead of having to make up an Err to return, unimplemented!() just does what it takes to make the compiler happy (it panics, lol)

Fixed - I've now implemented the resolve() method completely, so I have removed the unimplemented comment.

jmt-lab · 2025-06-24T22:56:50Z

sources/api/apiclient/src/uri_resolver.rs

@@ -0,0 +1,159 @@
+// src/uri_resolver.rs


This module is lacking alot of documentation, especially a module level devinition, maybe addint something like:

//! URI resolution for different protocols. //! //! This module provides a trait-based approach to resolving different types of URIs //! including stdin (`-`), file paths (`file://`), HTTP(S) URLs, and S3 URIs.

Try to imagine what someone reading this file for the first time would want.

The module-level docstring is your opportunity to explain how the abstractions/types in here relate to eachother.

If you prime programmers with the right information here, they will be able to read and comprehend your code much faster.

With more sparse information, you basically make the programmer guess at how the implementation works. Then they have to read the code to be absolutely certain (or what often times happen is that they don't do this, and then are surprised when their mental model doesn't totally match the actual code)

Fixed - Added a module-level docstring to the top of uri_resolver.rs

jmt-lab · 2025-06-24T22:57:18Z

sources/api/apiclient/src/apply.rs

-            .context(error::StdinReadSnafu)
-            .await?;
-        return Ok(output);
+pub async fn get(input: &str) -> Result<String> {


Is there a reason the visibility on this method needs to be bumped to pub?

Fixed - the function is no longer pub. It was temporarily bumped to pub in the PR because one of the variables didn't have the correct scope.

jmt-lab · 2025-06-24T23:00:21Z

sources/api/apiclient/src/apply.rs

+    let url = Url::parse(input).context(error::UriSnafu { input_source: input.to_string() })?;
+
+    // 3) file://
+    if let Ok(r) = FileUri::try_from(url.clone()) {


I wonder if it might be better to use a prefix strip and then match instead of trying to directly form the Uri type directly and failing out, but its not a real blocker here.

Fixed - fixed this issue in another comment: #554 (comment)

anishcheraku · 2025-06-30T22:05:10Z

Pushed new Resolver for secretsmanager:// and added error handling for Secrets Manager. (reverted in next commit)

anishcheraku · 2025-06-30T22:08:03Z

Reversed the changes I just made for secretsmanager://

jmt-lab

Looks like something went awry with the rebase packages/kubernetes-1.26 files are showing up here.

jmt-lab · 2025-06-30T23:23:59Z

sources/api/apiclient/src/apply.rs

@@ -83,11 +84,16 @@ fn select_resolver(input: &str) -> Result<Box<dyn UriResolver>> {
        return Ok(Box::new(r));
    }

+    // 6) secretsmanager://


These numberings look out of order, I used to do numbering too but now i've learned better to not do numbering as it prevents edits from creating inconsistencies.

Fixed - Removed the numberings and fixed the order.

cbgbt

Nice work!

cbgbt · 2025-07-01T19:46:06Z

sources/api/apiclient/src/apply.rs

+}
+
+/// Choose which UriResolver applies to `input` (stdin, file://, http(s):// or s3://).
+fn select_resolver(input: &str) -> Result<Box<dyn UriResolver>> {


This function is fine, but I somewhat prefer an implementation where we have a static list of resolvers, and the resolver selection iterates through the list, calling an interface that each resolver exposes to decide if that resolver is capable of handling this input.

This makes it a little easier to reason about how to add more resolvers, rather than having to keep the stateful "if/else" ladder in your mind.

trait ResolverSelector { fn try_create_resolver(input: &str) -> Option<Box<dyn UriResolver>>; } const RESOLVERS: &[&dyn ResolverSelector] = &[ &StdioResolverSelector, &FileResolverSelector, // etc ]; struct FileResolverSelector; impl ResolverSelector for FileResolverSelector { fn try_create_resolver(input: &str) -> Option<Box<dyn UriResolver>> { let input_uri = Url::parse(&input).ok()?; // This maps the "Result" response into an "Option" // try to make a FileResolver or return None if we can't } }

cbgbt · 2025-07-01T19:47:18Z

sources/api/apiclient/src/uri_resolver.rs

+
+use reqwest::Url;
+use crate::apply::{Error, Result};
+use crate::apply::error::{


We shouldn't import Snafu's like this.

The "preferred" way is, in each function that needs these Snafu context selectors, do something like this:

fn something_that_can_fail() -> Result<(), ErrorType> { use crate::apply::error::*; // now the selectors are in scope just for this function }

One reason is that this import list basically becomes a chore to maintain.

Fixed - Used the preferred format for crate::uri_resolver as well as crate::apply.

cbgbt · 2025-07-01T19:48:52Z

sources/api/apiclient/src/uri_resolver.rs

+                method: "GET".to_string(),
+            })?
+            .error_for_status()
+            .context(ReqwestSnafu {


These shouldn't all be ReqwestSnafu. We want a different error variant to explain the failure mode more succinctly (e.g. an error variant for failing to get text() from a response body should be different than the error variant for failing to make a web request at all)

Fixed - replaced ReqwestSnafus with HttpRequest, HttpStatus, and HttpBody.

cbgbt · 2025-07-01T19:50:09Z

sources/api/apiclient/src/uri_resolver.rs

+    }
+}
+
+/// s3://bucket/key URLs (stub; add aws-sdk-s3 later)


This is useful information to include in a docstring, but you should start with a human-readable description first. Something like:

/// Uri Resolver that fetches content from S3 /// /// This resolver accepts input of the form s3://bucket/key and translates this into /// authenticated AWS S3 get requests using the standard AWS credential resolution mechanism

Fixed - added a description for each resolver.

cbgbt · 2025-07-01T19:51:26Z

sources/api/apiclient/src/uri_resolver.rs

+#[async_trait]
+impl UriResolver for S3Uri {
+    async fn resolve(&self) -> Result<String> {
+        // still unimplemented


Agree with Jarrett, but as another tip, in rust you can use the unimplemented!() macro in functions to avoid having to "make stuff up" Like this:

async fn resolve(&self) -> Result<String> { unimplemented!() }

Instead of having to make up an Err to return, unimplemented!() just does what it takes to make the compiler happy (it panics, lol)

cbgbt · 2025-07-01T19:52:30Z

sources/api/apiclient/src/uri_resolver.rs

@@ -0,0 +1,159 @@
+// src/uri_resolver.rs


Try to imagine what someone reading this file for the first time would want.

The module-level docstring is your opportunity to explain how the abstractions/types in here relate to eachother.

If you prime programmers with the right information here, they will be able to read and comprehend your code much faster.

With more sparse information, you basically make the programmer guess at how the implementation works. Then they have to read the code to be absolutely certain (or what often times happen is that they don't do this, and then are surprised when their mental model doesn't totally match the actual code)

cbgbt · 2025-07-01T19:56:51Z

sources/api/apiclient/src/uri_resolver.rs

+        use aws_config::{self, BehaviorVersion, Region};
+        use aws_sdk_secretsmanager;
+
+        // 1) load AWS config (region/account via env)


In general, comments which exactly say what the code below does aren't a great idea. The newline separation you have in this function is good, and you can basically skim each newline-separated section to figure out the flow of the function.

I think a better approach would be to hoist a lot of this information into the docstring:

/// Resolves incoming source artifacts by fetching from AWS Secrets Manager /// /// This function uses the typical AWS credentials resolution mechanism to authenticate.

Docstrings like this are useful at two levels:

People calling this function don't need to read the code to understand what's going to happen

People who need to change this code can just read the docstring and be primed to understand what they're about to read, much like how I mentioned with module-level docstrings.

So when is it more useful to have inner comments?

If the discrete actions of the function are hard to figure out by skimming each newline-separated section (this can also be a sign that you should refactor things, but like all things it's a tradeoff decision)

If the what of the code is clear, but not the why (e.g. Imagine a function much like this one, but it reads AWS configurations from a separate file. You would probably need to add a comment there explaining why you are doing that)

sources/api/apiclient/src/apply.rs

sources/api/apiclient/src/uri_resolver.rs

cbgbt · 2025-07-08T22:33:42Z

Twoliter.toml

-release-version = "9.1.0"
+release-version = "8.2.0-1"


We'll need to make sure we pull these changes out prior to merging.

Fixed - Rebased my code on top of the bottlerocket-os:develop branch and stopped tracking changes to Twoliter.toml.

cbgbt · 2025-07-08T22:34:29Z

sources/api/apiclient/Cargo.toml

@@ -31,7 +37,7 @@ log.workspace = true
 models.workspace = true
 nix.workspace = true
 rand = { workspace = true, features = ["default"] }
-reqwest.workspace = true
+reqwest.workspace = true 


nit:

Suggested change

reqwest.workspace = true

reqwest.workspace = true

Fixed - Removed the extra space

cbgbt · 2025-07-08T22:35:50Z

sources/api/apiclient/Cargo.toml

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


I don't think we actually want to directly take a dependency here. Most SDK crates re-export the error types they create.

Fixed - This dependency is no longer being used

It appears to still be present in Cargo.toml

cbgbt · 2025-07-08T22:36:20Z

sources/api/apiclient/src/apply.rs

+}
+
+
+/// Choose which UriResolver applies to `input` (stdin, file://, http(s):// or s3://).


This appears to be out-of-date.

Fixed - Updated the comment to reflect the new resolvers that were added.

cbgbt · 2025-07-08T22:41:44Z

sources/api/apiclient/src/apply.rs

+        #[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>,
+        },


Instead of expanding the top-level error type with all of these error variants, I think it would be better to instead make a ResolverError type in the uri_resolver module.

We could then add Resolver as a specific type here. This would make it easier to determine at what point in the program each error variant can occur.

Fixed - Moved all error variants associated with the resolvers into uri_resolver.rs into a new ResolverError enum. Additionally, the functions now return a new type ResolverResult, so it is easier to associate errors with their sources.

cbgbt · 2025-07-08T22:56:06Z

sources/api/apiclient/src/apply.rs

+    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;


Please remove unused code.

Fixed - I have removed the unused code

cbgbt · 2025-07-08T23:03:36Z

sources/api/apiclient/src/apply.rs

+    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) {


I might suggest adding use crate::uri_resolver so we don't have to fully specify crate:: each time here.

Fixed - Imported the uri_resolver crate, so that I no longer have to specify crate:: each time

cbgbt · 2025-07-08T23:06:19Z

sources/api/apiclient/src/apply.rs

+
+
+/// Choose which UriResolver applies to `input` (stdin, file://, http(s):// or s3://).
+fn select_resolver(input: &str) -> Result<Box<dyn crate::uri_resolver::UriResolver>> {


Can we add unit tests to assert that the correct resolver is selected?

I also think we really need tests to understand what happens when characters in the S3 keyspace are used which aren't supported by URL standard. We should add tests for these cases, as it seems like it's entirely up to our parser implementation to decide what to do given this current implementation.

Fixed - I have added unit tests to assert that the correct resolver was selected.

Fixed - After trying out 4 different try_from() implementations for the S3Uri, I have found one that works on all sorts of URIs that have special characters, and ones that don't follow the URL standard. I have also added unit tests for testing different S3 URIs that have special characters and don't follow the URL standard.

cbgbt · 2025-07-08T23:08:36Z

sources/api/apiclient/src/uri_resolver.rs

+    parameter_name: String,
+}
+
+impl TryFrom<&str> for SsmUri {


I think we're still supporting TryFrom on multiple types. Does it make sense to unify on each resolver accepting &str types and allowing them each to parse?

This is technically less efficient than the current setup, but it would make it easier for us to test our Resolver selector.

Another option could be to do something like this:

struct SettingsInput { input: String, 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(); Self { input, parsed_url } } }

Then you'd change select_resolver to be like this:

fn select_resolver(input: &SettingsInput) -> Result<Box<dyn crate::uri_resolver::UriResolver>> { // ... }

And you could make it so that each resolver had a uniform TryFrom impl on &SettingsInput.

Doing this would let you parse the Url exactly once and standardize the TryFrom. Resolvers that require a Url could basically return an error if the parsed_url is None.

Fixed - TryFrom now takes &SettingsInput as its input, and determines what input type is required for each resolver. Also, only one type (&SettingsInput) is passed into all of the resolvers.

sources/api/apiclient/src/uri_resolver.rs

cbgbt · 2025-07-08T23:54:12Z

I think the build failed because the Cargo.lock file is not in-sync with your dependency closure?

You can fix this locally by going into the sources directory and doing cargo check.

sources/api/apiclient/Cargo.toml

sources/api/apiclient/src/apply.rs

sources/api/apiclient/src/uri_resolver.rs

sam-berning · 2025-07-15T19:10:07Z

sources/api/apiclient/src/uri_resolver.rs

+            SecretsManagerUriSnafu { input_source: input.input.as_str().to_string() }
+        );
+        // strip the prefix and ensure there's actually an ID
+        let id = &input.input.as_str()["secretsmanager://".len()..];


nit: you could use .strip_prefix("secretsmanager://") here, which would be easier to read

Fixed - replaced ["secretsmanager://".len()..] with .strip_prefix("secretsmanager://"). Also rewrote some of the code in the try_from() method to account for changing the type of id.

let's update this in the TryFrom implementation for SsmUri as well. I think the S3 resolver's TryFrom implementation is the cleanest of the AWS resolvers as well, can you have all three of them align with that implementation?

Fixed - All TryFrom implementations are significantly cleaner and parsing logic for SSM and Secrets Manager align similarly with the parsing logic in S3Uri's TryFrom implementation.

sources/api/apiclient/src/uri_resolver.rs

sam-berning · 2025-07-24T20:42:39Z

sources/api/apiclient/src/uri_resolver.rs

+impl UriResolver for HttpUri {
+    async fn resolve(&self) -> ResolverResult<String> {
+        use resolver_error::*;
+        const MAX_SIZE_BYTES: u64 = 100 * 1024 * 1024;


You should share this value between the two resolvers that use it, so we can configure it in one place

Fixed - MAX_SIZE_BYTE is now defined at the top of the uri_resolver crate, and has a comment explaining its purpose.

sources/api/apiclient/src/uri_resolver.rs

cbgbt · 2025-07-25T19:24:59Z

sources/api/apiclient/Cargo.toml

@@ -39,11 +45,14 @@ serde_json.workspace = true
 signal-hook.workspace = true
 simplelog.workspace = true
 snafu = { workspace = true, features = ["futures"] }
+test-case.workspace = true


This should be listed under [dev-dependencies] instead of [dependencies]

Fixed - moved test-case.workspace to [dev-dependencies].

cbgbt · 2025-07-25T19:25:11Z

sources/api/apiclient/Cargo.toml

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


It appears to still be present in Cargo.toml

cbgbt · 2025-07-25T19:26:26Z

sources/api/apiclient/src/apply.rs

+        let parsed_url = Url::parse(&input).ok();
+        SettingsInput { input, parsed_url }


Wouldn't be a bad idea to emit a debug!() message if the URL parse fails that includes the error message from that failure. That would help us debug cases where the URL parser fails when we would expect it to succeed.

Fixed - a log::debug! message is now logged whenever the File or HTTP URI resolvers fails to parse a URL correctly.

I'd rather see the debug message logged here where we still have access to the error message that would have been returned by Url::parse().

The error will only be meaningful in the File/HTTP URI cases, but the idea is that the message will be logged at debug, allowing it to be ignored in cases where it isn't relevant.

cbgbt · 2025-07-25T23:13:25Z

sources/api/apiclient/src/apply.rs

+/// 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;
+
+    // stdin ("-")
+    if let Ok(r) = uri_resolver::StdinUri::try_from(input) {
+        return Ok(Box::new(r));
    }


The repetition in this function is still hard for me to thumbs up. I have a simple suggestion to improve this via macros:

/// Macro to try multiple settings resolver types in sequence, returning the first one that succeeds. macro_rules! try_resolvers { ($input:expr, $($resolver_type:ty),+ $(,)?) => { $( if let Ok(r) = <$resolver_type>::try_from($input) { return Ok(Box::new(r)); } )+ }; } /// 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::*; try_resolvers!( input, StdinUri, FileUri, HttpUri, S3Uri, SecretsManagerArn, SecretsManagerUri, SsmArn, SsmUri, ); error::NoResolverSnafu { input_source: input.input.clone(), } .fail() }

We should also probably emit a debug!-level log message when we select a resolver saying which one we are using.

Fixed - Made both changes.

cbgbt · 2025-07-25T23:44:39Z

sources/api/apiclient/src/apply.rs

+    #[test_case("-",                    TypeId::of::<crate::uri_resolver::StdinUri>();         "stdin")]
+    #[test_case("file:///tmp/folder",      TypeId::of::<crate::uri_resolver::FileUri>();          "file")]
+    #[test_case("http://amazon.com",   TypeId::of::<crate::uri_resolver::HttpUri>();          "http")]
+    #[test_case("https://amazon.com",  TypeId::of::<crate::uri_resolver::HttpUri>();          "https")]
+    #[test_case("s3://mybucket/path",   TypeId::of::<crate::uri_resolver::S3Uri>();            "s3")]
+    #[test_case("secretsmanager://sec", TypeId::of::<crate::uri_resolver::SecretsManagerUri>(); "secrets")]
+    #[test_case("ssm://param",          TypeId::of::<crate::uri_resolver::SsmUri>();           "ssmUri")]
+    #[test_case("arn:aws:ssm:<region>:<account_id>:parameter/<name>",          TypeId::of::<crate::uri_resolver::SsmArn>();           "ssmArn")]
+    #[test_case("arn:aws:secretsmanager:<region>:<account-id>:secret:<secret-id>",          TypeId::of::<crate::uri_resolver::SecretsManagerArn>();           "secretsmanagerArn")]


The spacing on these is very odd. You should either consistently use one space after each comma, or modify these to look like this:

#[test_case( "-", TypeId::of::<crate::uri_resolver::StdinUri>() ; "stdin")]

Fixed - The spacing is now consistent for all test cases.

cbgbt · 2025-07-26T00:06:39Z

sources/api/apiclient/src/uri_resolver.rs

+        // 3) Construct
+        Ok(SsmArn {
+            region: arn.region,
+            parameter_name: input.input.to_string(), // still use the full `/parameter/...` segment


The comment and parameter_name are both incorrect. This contains the full ARN.

Fixed - Fixed both variable names for SecretsManagerArn and SsmArn to be more descriptive of what they actually contain (full_arn).

cbgbt · 2025-07-26T00:08:02Z

sources/api/apiclient/src/uri_resolver.rs

+impl UriResolver for SsmArn {
+    async fn resolve(&self) -> ResolverResult<String> {
+        use resolver_error::*;
+        // 1) Load default config, then override region & credentials


Comment is not correct, we aren't changing credentials.

Fixed - Forgot to update the comment. The original comment mentioned overwriting the credentials because that was my first approach, but then decided to use IAM permissions to access parameters and secrets instead (doesn't require overriding credentials).

cbgbt · 2025-07-26T00:10:35Z

sources/api/apiclient/src/uri_resolver.rs

+#[derive(Debug, Snafu)]
+#[snafu(module)]
+pub enum ResolverError {
+    #[snafu(display("Invalid ARN '{}': {}", input_source, reason))]
+    InvalidArnFormat {


I'd prefer having this be composed of smaller Resolver-specific types like FileUriResolverError, S3ResolverError, SsmResolverError, etc would improve maintainability.

sources/api/apiclient/src/uri_resolver.rs

cbgbt

Very nice work! I have a few comments about logging, but otherwise this LGTM.

cbgbt · 2025-07-29T18:44:55Z

sources/api/apiclient/src/apply.rs

+        let parsed_url = Url::parse(&input).ok();
+        SettingsInput { input, parsed_url }


I'd rather see the debug message logged here where we still have access to the error message that would have been returned by Url::parse().

The error will only be meaningful in the File/HTTP URI cases, but the idea is that the message will be logged at debug, allowing it to be ignored in cases where it isn't relevant.

cbgbt · 2025-07-29T18:48:09Z

sources/api/apiclient/src/uri_resolver.rs

+        let url = {
+            if let None = &input.parsed_url {
+                log::debug!("Failed to parse HTTP URI '{}'", input.input);
+            }
+            input.parsed_url.clone()
+        }.context(FileUriSnafu {
+            input_source: input.input.as_str(),
+        })?;


debug! is the wrong log level for a failure here. We expect the URI to have parsed properly, so an error here would be error! and not debug!.

I'd be OK with removing the logging in this case in favor of just always debug logging at the top level as mentioned in the other comment.

Fixed - updated the debug!() statement to be thrown whenever there is an error parsing the Url for all parsed Urls, not just in the File/HTTP URI cases.

cbgbt · 2025-07-29T18:48:45Z

sources/api/apiclient/Cargo.toml

@@ -44,6 +50,10 @@ tokio-tungstenite = { workspace = true, features = ["connect"] }
 toml.workspace = true
 unindent.workspace = true
 url.workspace = true
+tempfile.workspace = true


Should this also be a dev dependency?

Fixed - It should be under [dev-dependencies] since its only used to test the File parser.

anishcheraku · 2025-08-01T22:39:53Z

^ Force pushed to fix commit messages and remove unnecessary changes.

anishcheraku force-pushed the dev branch from 1a0ce92 to 9ee333d Compare June 20, 2025 23:24

jmt-lab reviewed Jun 24, 2025

View reviewed changes

anishcheraku force-pushed the dev branch from 9ee333d to 204d896 Compare June 30, 2025 22:03

anishcheraku force-pushed the dev branch from 204d896 to fc47105 Compare June 30, 2025 22:07

jmt-lab reviewed Jun 30, 2025

View reviewed changes

cbgbt reviewed Jul 1, 2025

View reviewed changes

sam-berning reviewed Jul 1, 2025

View reviewed changes

sources/api/apiclient/src/apply.rs Outdated Show resolved Hide resolved

sources/api/apiclient/src/uri_resolver.rs Outdated Show resolved Hide resolved

anishcheraku changed the title ~~Refactoring apiclient apply's get logic and apiclient apply support for S3 URIs~~ Refactoring apiclient apply's get logic and apiclient apply support for S3, SSM, and Secrets Manager URIs Jul 2, 2025

cbgbt reviewed Jul 8, 2025

View reviewed changes

anishcheraku force-pushed the dev branch from 96a557b to 398d757 Compare July 14, 2025 23:10

sam-berning reviewed Jul 15, 2025

View reviewed changes

anishcheraku force-pushed the dev branch from 5006d80 to 018cef9 Compare July 17, 2025 23:08

sam-berning reviewed Jul 24, 2025

View reviewed changes

cbgbt reviewed Jul 26, 2025

View reviewed changes

cbgbt marked this pull request as ready for review July 29, 2025 05:56

cbgbt marked this pull request as draft July 29, 2025 05:56

cbgbt approved these changes Jul 29, 2025

View reviewed changes

Anish Cheraku added 2 commits July 29, 2025 22:35

apiclient: refactor get logic in apply.rs

8a6cce5

apiclient: add support for s3:// URIs

e2d3aa2

anishcheraku force-pushed the dev branch 2 times, most recently from 05ba52f to 0faab71 Compare August 1, 2025 18:20

Anish Cheraku added 3 commits August 1, 2025 22:35

apiclient: add support for secretsmanager:// URIs

ed78c98

apiclient: add support for ssm:// URIs

4c97027

apiclient: add support for SSM and Secrets Manager ARNs

b2f62e7

anishcheraku force-pushed the dev branch from 0faab71 to b2f62e7 Compare August 1, 2025 22:38

		}


		/// Choose which UriResolver applies to `input` (stdin, file://, http(s):// or s3://).



		/// Choose which UriResolver applies to `input` (stdin, file://, http(s):// or s3://).
		fn select_resolver(input: &str) -> Result<Box<dyn crate::uri_resolver::UriResolver>> {

		let parsed_url = Url::parse(&input).ok();
		SettingsInput { input, parsed_url }

		release-version = "9.1.0"
		release-version = "8.2.0-1"

Refactoring apiclient apply's get logic and apiclient apply support for S3, SSM, and Secrets Manager URIs #554

Are you sure you want to change the base?

Refactoring apiclient apply's get logic and apiclient apply support for S3, SSM, and Secrets Manager URIs #554

Uh oh!

Conversation

anishcheraku commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anishcheraku Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anishcheraku Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anishcheraku Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anishcheraku Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anishcheraku Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anishcheraku Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anishcheraku commented Jun 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

anishcheraku commented Jun 30, 2025

Uh oh!

jmt-lab left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anishcheraku Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbgbt left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anishcheraku Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anishcheraku Jul 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Refactoring `apiclient apply`'s get logic and `apiclient apply` support for S3, SSM, and Secrets Manager URIs #554

Refactoring `apiclient apply`'s get logic and `apiclient apply` support for S3, SSM, and Secrets Manager URIs #554

anishcheraku commented Jun 20, 2025 •

edited

Loading

anishcheraku Jul 2, 2025 •

edited

Loading

anishcheraku Jul 2, 2025 •

edited

Loading

anishcheraku Jul 2, 2025 •

edited

Loading

anishcheraku Jul 2, 2025 •

edited

Loading

anishcheraku Jul 2, 2025 •

edited

Loading

anishcheraku Jul 2, 2025 •

edited

Loading

anishcheraku commented Jun 30, 2025 •

edited

Loading

anishcheraku Jul 2, 2025 •

edited

Loading

anishcheraku Jul 2, 2025 •

edited

Loading

anishcheraku Jul 2, 2025 •

edited

Loading

anishcheraku Jul 2, 2025 •

edited

Loading

anishcheraku Jul 9, 2025 •

edited

Loading

anishcheraku Jul 9, 2025 •

edited

Loading

anishcheraku Jul 9, 2025 •

edited

Loading