Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Don't warn about home dir expansion for default profile paths #1558

Merged
merged 3 commits into from Jul 20, 2022
Merged

Don't warn about home dir expansion for default profile paths #1558

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
96a8108
Don't warn about home dir expansion for default profile paths
jdisanti Jul 20, 2022
5ee75e7
Update changelog
jdisanti Jul 20, 2022
d893554
Merge branch 'jdisanti-aggregate-prs' into jdisanti-long-way-home
jdisanti Jul 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions CHANGELOG.next.toml
View file
Open in desktop
Expand Up @@ -116,3 +116,13 @@ a version bump in all of them, but this should not be relied upon.
references = ["smithy-rs#1540"]
meta = { "breaking" = false, "tada" = false, "bug" = false }
author = "jdisanti"

[[aws-sdk-rust]]
message = """
Only emit a warning about failing to expand a `~` to the home
directory in a profile file's path if that path was explicitly
set (don't emit it for the default paths)
"""
references = ["smithy-rs#1558", "aws-sdk-rust#583"]
meta = { "breaking" = false, "tada" = false, "bug" = true }
author = "jdisanti"
81 changes: 36 additions & 45 deletions aws/rust-runtime/aws-config/src/profile/parser/source.rs
View file
Open in desktop
Expand Up @@ -8,7 +8,11 @@ use aws_types::os_shim_internal;
use std::borrow::Cow;
use std::io::ErrorKind;
use std::path::{Component, Path, PathBuf};
use tracing::Instrument;
use tracing::{warn, Instrument};

const HOME_EXPANSION_FAILURE_WARNING: &str =
"home directory expansion was requested (via `~` character) for the profile \
config file path, but no home directory could be determined";

/// In-memory source of profile data
pub(super) struct Source {
Expand Down Expand Up @@ -88,12 +92,12 @@ async fn load_config_file(
fs: &os_shim_internal::Fs,
environment: &os_shim_internal::Env,
) -> File {
let path = environment
let (path_is_default, path) = environment
.get(kind.override_environment_variable())
.map(Cow::Owned)
.map(|p| (false, Cow::Owned(p)))
.ok()
.unwrap_or_else(|| kind.default_path().into());
let expanded = expand_home(path.as_ref(), home_directory, environment);
.unwrap_or_else(|| (true, kind.default_path().into()));
let expanded = expand_home(path.as_ref(), path_is_default, home_directory);
if path != expanded.to_string_lossy() {
tracing::debug!(before = ?path, after = ?expanded, "home directory expanded");
}
Expand Down Expand Up @@ -134,8 +138,8 @@ async fn load_config_file(

fn expand_home(
path: impl AsRef<Path>,
path_is_default: bool,
home_dir: &Option<String>,
environment: &os_shim_internal::Env,
) -> PathBuf {
let path = path.as_ref();
let mut components = path.components();
Expand All @@ -150,15 +154,9 @@ fn expand_home(
dir.clone()
}
None => {
// Lambdas don't have home directories and emitting this warning is not helpful
// to users running the SDK from within a Lambda. This warning will be silenced
// if we determine that that is the case.
let is_likely_running_on_a_lambda =
check_is_likely_running_on_a_lambda(environment);
if !is_likely_running_on_a_lambda {
tracing::warn!(
"could not determine home directory but home expansion was requested"
);
// Only log a warning if the path was explicitly set by the customer.
if !path_is_default {
warn!(HOME_EXPANSION_FAILURE_WARNING);
}
// if we can't determine the home directory, just leave it as `~`
"~".into()
Expand All @@ -179,30 +177,25 @@ fn expand_home(
}
}

/// Returns true or false based on whether or not this code is likely running inside an AWS Lambda.
/// [Lambdas set many environment variables](https://docs.aws.amazon.com/lambda/latest/dg/configuration-envvars.html#configuration-envvars-runtime)
/// that we can check.
fn check_is_likely_running_on_a_lambda(environment: &aws_types::os_shim_internal::Env) -> bool {
// AWS_LAMBDA_FUNCTION_NAME – The name of the running Lambda function. Available both in Functions and Extensions
environment.get("AWS_LAMBDA_FUNCTION_NAME").is_ok()
}

#[cfg(test)]
mod tests {
use crate::profile::parser::source::{expand_home, load, load_config_file, FileKind};
use crate::profile::parser::source::{
expand_home, load, load_config_file, FileKind, HOME_EXPANSION_FAILURE_WARNING,
};
use aws_types::os_shim_internal::{Env, Fs};
use futures_util::FutureExt;
use serde::Deserialize;
use std::collections::HashMap;
use std::error::Error;
use std::fs;
use tracing_test::traced_test;

#[test]
fn only_expand_home_prefix() {
// ~ is only expanded as a single component (currently)
let path = "~aws/config";
let environment = Env::from_slice(&[]);
assert_eq!(
expand_home(&path, &None, &environment).to_str().unwrap(),
expand_home(&path, false, &None).to_str().unwrap(),
"~aws/config"
);
}
Expand Down Expand Up @@ -238,9 +231,6 @@ mod tests {
Ok(())
}

use futures_util::FutureExt;
use tracing_test::traced_test;

#[traced_test]
#[test]
fn logs_produced_default() {
Expand All @@ -260,14 +250,22 @@ mod tests {

#[traced_test]
#[test]
fn load_config_file_should_not_emit_warning_on_lambda() {
let env = Env::from_slice(&[("AWS_LAMBDA_FUNCTION_NAME", "someName")]);
fn load_config_file_should_not_emit_warning_when_path_not_explicitly_set() {
let env = Env::from_slice(&[]);
let fs = Fs::from_slice(&[]);

let _src = load_config_file(FileKind::Config, &None, &fs, &env).now_or_never();
assert!(!logs_contain(HOME_EXPANSION_FAILURE_WARNING));
}

#[traced_test]
#[test]
fn load_config_file_should_emit_warning_when_path_explicitly_set() {
let env = Env::from_slice(&[("AWS_CONFIG_FILE", "~/some/path")]);
let fs = Fs::from_slice(&[]);

let _src = load_config_file(FileKind::Config, &None, &fs, &env).now_or_never();
assert!(!logs_contain(
"could not determine home directory but home expansion was requested"
));
assert!(logs_contain(HOME_EXPANSION_FAILURE_WARNING));
}

async fn check(test_case: TestCase) {
Expand Down Expand Up @@ -302,9 +300,8 @@ mod tests {
#[cfg_attr(windows, ignore)]
fn test_expand_home() {
let path = "~/.aws/config";
let environment = Env::from_slice(&[]);
assert_eq!(
expand_home(&path, &Some("/user/foo".to_string()), &environment)
expand_home(&path, false, &Some("/user/foo".to_string()))
.to_str()
.unwrap(),
"/user/foo/.aws/config"
Expand All @@ -313,21 +310,16 @@ mod tests {

#[test]
fn expand_home_no_home() {
let environment = Env::from_slice(&[]);
// there is an edge case around expansion when no home directory exists
// if no home directory can be determined, leave the path as is
if !cfg!(windows) {
assert_eq!(
expand_home("~/config", &None, &environment)
.to_str()
.unwrap(),
expand_home("~/config", false, &None).to_str().unwrap(),
"~/config"
)
} else {
assert_eq!(
expand_home("~/config", &None, &environment)
.to_str()
.unwrap(),
expand_home("~/config", false, &None).to_str().unwrap(),
"~\\config"
)
}
Expand All @@ -338,9 +330,8 @@ mod tests {
#[cfg_attr(not(windows), ignore)]
fn test_expand_home_windows() {
let path = "~/.aws/config";
let environment = Env::from_slice(&[]);
assert_eq!(
expand_home(&path, &Some("C:\\Users\\name".to_string()), &environment)
expand_home(&path, true, &Some("C:\\Users\\name".to_string()),)
.to_str()
.unwrap(),
"C:\\Users\\name\\.aws\\config"
Expand Down