Add sdk_ua_app_id in addition to sdk-ua-app-id in profile file support

CHANGELOG.next.toml

@@ -41,3 +41,9 @@ message = "Fix error message when `credentials-sso` feature is not enabled on `a
 references = ["smithy-rs#2722", "aws-sdk-rust#703"]
 meta = { "breaking" = false, "tada" = false, "bug" = true }
 author = "rcoh"
+
+[[aws-sdk-rust]]
+message = "The AppName property can now be set with `sdk_ua_app_id` in profile files. The old field, `sdk-ua-app-id`, is maintained for backards compatibility."
+references = ["smithy-rs#2724"]
+meta = { "breaking" = false, "tada" = false, "bug" = false }
+author = "rcoh"

aws/rust-runtime/aws-config/src/default_provider/app_name.rs

@@ -3,49 +3,85 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-use crate::environment::app_name::EnvironmentVariableAppNameProvider;
-use crate::profile::app_name;
 use crate::provider_config::ProviderConfig;
-use aws_types::app_name::AppName;
+use crate::standard_property::{PropertyResolutionError, StandardProperty};
+use aws_smithy_types::error::display::DisplayErrorContext;
+use aws_types::app_name::{AppName, InvalidAppName};
 
 /// Default App Name Provider chain
 ///
 /// This provider will check the following sources in order:
-/// 1. [Environment variables](EnvironmentVariableAppNameProvider)
-/// 2. [Profile file](crate::profile::app_name::ProfileFileAppNameProvider)
+/// 1. Environment variables: `AWS_SDK_UA_APP_ID`
+/// 2. Profile files from the key `sdk_ua_app_id`
+///
+#[doc = include_str!("../profile/location_of_profile_files.md")]
+///
+/// # Examples
+///
+/// **Loads "my-app" as the app name**
+/// ```ini
+/// [default]
+/// sdk_ua_app_id = my-app
+/// ```
+///
+/// **Loads "my-app" as the app name _if and only if_ the `AWS_PROFILE` environment variable
+/// is set to `other`.**
+/// ```ini
+/// [profile other]
+/// sdk_ua_app_id = my-app
+/// ```
 pub fn default_provider() -> Builder {
     Builder::default()
 }
 
 /// Default provider builder for [`AppName`]
 #[derive(Debug, Default)]
 pub struct Builder {
-    env_provider: EnvironmentVariableAppNameProvider,
-    profile_file: app_name::Builder,
+    provider_config: ProviderConfig,
 }
 
 impl Builder {
     #[doc(hidden)]
     /// Configure the default chain
     ///
     /// Exposed for overriding the environment when unit-testing providers
-    pub fn configure(mut self, configuration: &ProviderConfig) -> Self {
-        self.env_provider = EnvironmentVariableAppNameProvider::new_with_env(configuration.env());
-        self.profile_file = self.profile_file.configure(configuration);
-        self
+    pub fn configure(self, configuration: &ProviderConfig) -> Self {
+        Self {
+            provider_config: configuration.clone(),
+        }
     }
 
     /// Override the profile name used by this provider
     pub fn profile_name(mut self, name: &str) -> Self {
-        self.profile_file = self.profile_file.profile_name(name);
+        self.provider_config = self.provider_config.with_profile_name(name.to_string());
         self
     }
 
+    async fn fallback_app_name(
+        &self,
+    ) -> Result<Option<AppName>, PropertyResolutionError<InvalidAppName>> {
+        StandardProperty::new()
+            .profile("sdk-ua-app-id")
+            .validate(&self.provider_config, |name| AppName::new(name.to_string()))
+            .await
+    }
+
     /// Build an [`AppName`] from the default chain
     pub async fn app_name(self) -> Option<AppName> {
-        self.env_provider
-            .app_name()
-            .or(self.profile_file.build().app_name().await)
+        let standard = StandardProperty::new()
+            .env("AWS_SDK_UA_APP_ID")
+            .profile("sdk_ua_app_id")
+            .validate(&self.provider_config, |name| AppName::new(name.to_string()))
+            .await;
+        let with_fallback = match standard {
+            Ok(None) => self.fallback_app_name().await,
+            other => other,
+        };
+
+        with_fallback.map_err(
+                |err| tracing::warn!(err = %DisplayErrorContext(&err), "invalid value for App Name setting"),
+            )
+            .unwrap_or(None)
     }
 }
 
@@ -80,7 +116,7 @@ mod tests {
     // test that overriding profile_name on the root level is deprecated
     #[tokio::test]
     async fn profile_name_override() {
-        let fs = Fs::from_slice(&[("test_config", "[profile custom]\nsdk-ua-app-id = correct")]);
+        let fs = Fs::from_slice(&[("test_config", "[profile custom]\nsdk_ua_app_id = correct")]);
         let conf = crate::from_env()
             .configure(
                 ProviderConfig::empty()
@@ -101,6 +137,23 @@ mod tests {
 
     #[tokio::test]
     async fn load_from_profile() {
+        let fs = Fs::from_slice(&[("test_config", "[default]\nsdk_ua_app_id = correct")]);
+        let env = Env::from_slice(&[("AWS_CONFIG_FILE", "test_config")]);
+        let app_name = Builder::default()
+            .configure(
+                &ProviderConfig::empty()
+                    .with_fs(fs)
+                    .with_env(env)
+                    .with_http_connector(no_traffic_connector()),
+            )
+            .app_name()
+            .await;
+
+        assert_eq!(Some(AppName::new("correct").unwrap()), app_name);
+    }
+
+    #[tokio::test]
+    async fn load_from_profile_old_name() {
         let fs = Fs::from_slice(&[("test_config", "[default]\nsdk-ua-app-id = correct")]);
         let env = Env::from_slice(&[("AWS_CONFIG_FILE", "test_config")]);
         let app_name = Builder::default()

aws/rust-runtime/aws-config/src/environment/app_name.rs

@@ -9,10 +9,12 @@ use aws_types::os_shim_internal::Env;
 
 /// Load an app name from the `AWS_SDK_UA_APP_ID` environment variable.
 #[derive(Debug, Default)]
+#[deprecated(note = "This is unused and will be removed in a future release.")]
 pub struct EnvironmentVariableAppNameProvider {
     env: Env,
 }
 
+#[allow(deprecated)]
 impl EnvironmentVariableAppNameProvider {
     /// Create a new `EnvironmentVariableAppNameProvider`
     pub fn new() -> Self {
@@ -42,30 +44,3 @@ impl EnvironmentVariableAppNameProvider {
         }
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use crate::environment::EnvironmentVariableAppNameProvider;
-    use aws_types::app_name::AppName;
-    use aws_types::os_shim_internal::Env;
-    use std::collections::HashMap;
-
-    #[test]
-    fn env_var_not_set() {
-        let provider = EnvironmentVariableAppNameProvider::new_with_env(Env::from(HashMap::new()));
-        assert_eq!(None, provider.app_name());
-    }
-
-    #[test]
-    fn env_var_set() {
-        let provider = EnvironmentVariableAppNameProvider::new_with_env(Env::from(
-            vec![("AWS_SDK_UA_APP_ID".to_string(), "something".to_string())]
-                .into_iter()
-                .collect::<HashMap<String, String>>(),
-        ));
-        assert_eq!(
-            Some(AppName::new("something").unwrap()),
-            provider.app_name()
-        );
-    }
-}

aws/rust-runtime/aws-config/src/environment/mod.rs

@@ -8,7 +8,6 @@
 /// Load app name from the environment
 pub mod app_name;
 
-pub use app_name::EnvironmentVariableAppNameProvider;
 use std::error::Error;
 use std::fmt::{Display, Formatter};
 

aws/rust-runtime/aws-config/src/profile/app_name.rs

@@ -33,10 +33,14 @@ use aws_types::app_name::AppName;
 ///
 /// This provider is part of the [default app name provider chain](crate::default_provider::app_name).
 #[derive(Debug, Default)]
+#[deprecated(
+    note = "This is unused and is deprecated for backwards compatibility. It will be removed in a future release."
+)]
 pub struct ProfileFileAppNameProvider {
     provider_config: ProviderConfig,
 }
 
+#[allow(deprecated)]
 impl ProfileFileAppNameProvider {
     /// Create a new [ProfileFileAppNameProvider}
     ///
@@ -67,12 +71,14 @@ impl ProfileFileAppNameProvider {
 
 /// Builder for [ProfileFileAppNameProvider]
 #[derive(Debug, Default)]
+#[allow(deprecated)]
 pub struct Builder {
     config: Option<ProviderConfig>,
     profile_override: Option<String>,
     profile_files: Option<ProfileFiles>,
 }
 
+#[allow(deprecated)]
 impl Builder {
     /// Override the configuration for this provider
     pub fn configure(mut self, config: &ProviderConfig) -> Self {
@@ -87,6 +93,7 @@ impl Builder {
     }
 
     /// Build a [ProfileFileAppNameProvider] from this builder
+    #[allow(deprecated)]
     pub fn build(self) -> ProfileFileAppNameProvider {
         let conf = self
             .config
@@ -97,77 +104,3 @@ impl Builder {
         }
     }
 }
-
-#[cfg(test)]
-mod tests {
-    use super::ProfileFileAppNameProvider;
-    use crate::provider_config::ProviderConfig;
-    use crate::test_case::no_traffic_connector;
-    use aws_sdk_sts::config::AppName;
-    use aws_types::os_shim_internal::{Env, Fs};
-    use tracing_test::traced_test;
-
-    fn provider_config(config_contents: &str) -> ProviderConfig {
-        let fs = Fs::from_slice(&[("test_config", config_contents)]);
-        let env = Env::from_slice(&[("AWS_CONFIG_FILE", "test_config")]);
-        ProviderConfig::empty()
-            .with_fs(fs)
-            .with_env(env)
-            .with_http_connector(no_traffic_connector())
-    }
-
-    fn default_provider(config_contents: &str) -> ProfileFileAppNameProvider {
-        ProfileFileAppNameProvider::builder()
-            .configure(&provider_config(config_contents))
-            .build()
-    }
-
-    #[tokio::test]
-    async fn no_app_name() {
-        assert_eq!(None, default_provider("[default]\n").app_name().await);
-    }
-
-    #[tokio::test]
-    async fn app_name_default_profile() {
-        assert_eq!(
-            Some(AppName::new("test").unwrap()),
-            default_provider("[default]\nsdk-ua-app-id = test")
-                .app_name()
-                .await
-        );
-    }
-
-    #[tokio::test]
-    async fn app_name_other_profiles() {
-        let config = "\
-            [default]\n\
-            sdk-ua-app-id = test\n\
-            \n\
-            [profile other]\n\
-            sdk-ua-app-id = bar\n
-        ";
-        assert_eq!(
-            Some(AppName::new("bar").unwrap()),
-            ProfileFileAppNameProvider::builder()
-                .profile_name("other")
-                .configure(&provider_config(config))
-                .build()
-                .app_name()
-                .await
-        );
-    }
-
-    #[traced_test]
-    #[tokio::test]
-    async fn invalid_app_name() {
-        assert_eq!(
-            None,
-            default_provider("[default]\nsdk-ua-app-id = definitely invalid")
-                .app_name()
-                .await
-        );
-        assert!(logs_contain(
-            "`sdk-ua-app-id` property `definitely invalid` was invalid"
-        ));
-    }
-}