From bcd9c8101f21a4a4fdaa2f46cba532b5944328db Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Wed, 4 Sep 2024 16:29:40 -0700 Subject: [PATCH 1/4] Add regression test --- tests/credentials_provider_process_tests.c | 81 ++++++++++------------ 1 file changed, 35 insertions(+), 46 deletions(-) diff --git a/tests/credentials_provider_process_tests.c b/tests/credentials_provider_process_tests.c index c761ea8e..68afcbc9 100644 --- a/tests/credentials_provider_process_tests.c +++ b/tests/credentials_provider_process_tests.c @@ -144,6 +144,16 @@ AWS_STATIC_STRING_FROM_LITERAL( "\"Expiration\":\"2020-02-25T06:03:31Z\"}'"); #endif +AWS_STATIC_STRING_FROM_LITERAL( + s_test_command_with_logging_on_stderr, + "(" + "echo 'Logging on stderr' >&2" + " && " + "echo '{\"Version\": 1, \"AccessKeyId\": \"AccessKey123\", " + "\"SecretAccessKey\": \"SecretAccessKey321\", \"SessionToken\":\"TokenSuccess\", " + "\"Expiration\":\"2020-02-25T06:03:31Z\"}'" + ")"); + AWS_STATIC_STRING_FROM_LITERAL(s_bad_test_command, "/i/dont/know/what/is/this/command"); AWS_STATIC_STRING_FROM_LITERAL(s_bad_command_output, "echo \"Hello, World!\""); @@ -262,15 +272,14 @@ static int s_credentials_provider_process_new_failed(struct aws_allocator *alloc } AWS_TEST_CASE(credentials_provider_process_new_failed, s_credentials_provider_process_new_failed); -static int s_credentials_provider_process_bad_command(struct aws_allocator *allocator, void *ctx) { - (void)ctx; +static int s_test_command_expect_failure(struct aws_allocator *allocator, const struct aws_string *command) { s_aws_process_tester_init(allocator); struct aws_byte_buf content_buf; struct aws_byte_buf existing_content = aws_byte_buf_from_c_str(aws_string_c_str(s_process_config_file_contents)); aws_byte_buf_init_copy(&content_buf, allocator, &existing_content); - struct aws_byte_cursor cursor = aws_byte_cursor_from_string(s_bad_test_command); + struct aws_byte_cursor cursor = aws_byte_cursor_from_string(command); ASSERT_TRUE(aws_byte_buf_append_dynamic(&content_buf, &cursor) == AWS_OP_SUCCESS); cursor = aws_byte_cursor_from_c_str("\n"); ASSERT_TRUE(aws_byte_buf_append_dynamic(&content_buf, &cursor) == AWS_OP_SUCCESS); @@ -304,49 +313,16 @@ static int s_credentials_provider_process_bad_command(struct aws_allocator *allo s_aws_process_tester_cleanup(); return 0; } + +static int s_credentials_provider_process_bad_command(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + return s_test_command_expect_failure(allocator, s_bad_test_command); +} AWS_TEST_CASE(credentials_provider_process_bad_command, s_credentials_provider_process_bad_command); static int s_credentials_provider_process_incorrect_command_output(struct aws_allocator *allocator, void *ctx) { (void)ctx; - - s_aws_process_tester_init(allocator); - - struct aws_byte_buf content_buf; - struct aws_byte_buf existing_content = aws_byte_buf_from_c_str(aws_string_c_str(s_process_config_file_contents)); - aws_byte_buf_init_copy(&content_buf, allocator, &existing_content); - struct aws_byte_cursor cursor = aws_byte_cursor_from_string(s_bad_command_output); - ASSERT_TRUE(aws_byte_buf_append_dynamic(&content_buf, &cursor) == AWS_OP_SUCCESS); - cursor = aws_byte_cursor_from_c_str("\n"); - ASSERT_TRUE(aws_byte_buf_append_dynamic(&content_buf, &cursor) == AWS_OP_SUCCESS); - - struct aws_string *config_file_contents = aws_string_new_from_array(allocator, content_buf.buffer, content_buf.len); - ASSERT_TRUE(config_file_contents != NULL); - aws_byte_buf_clean_up(&content_buf); - - s_aws_process_test_init_config_profile(allocator, config_file_contents); - aws_string_destroy(config_file_contents); - - struct aws_credentials_provider_process_options options = { - .shutdown_options = - { - .shutdown_callback = s_on_shutdown_complete, - .shutdown_user_data = NULL, - }, - .profile_to_use = aws_byte_cursor_from_string(s_credentials_process_profile), - }; - struct aws_credentials_provider *provider = aws_credentials_provider_new_process(allocator, &options); - ASSERT_NOT_NULL(provider); - aws_credentials_provider_get_credentials(provider, s_get_credentials_callback, NULL); - - s_aws_wait_for_credentials_result(); - - ASSERT_TRUE(s_tester.has_received_credentials_callback == true); - ASSERT_TRUE(s_tester.credentials == NULL); - - aws_credentials_provider_release(provider); - s_aws_wait_for_provider_shutdown_callback(); - s_aws_process_tester_cleanup(); - return 0; + return s_test_command_expect_failure(allocator, s_bad_command_output); } AWS_TEST_CASE( credentials_provider_process_incorrect_command_output, @@ -367,15 +343,13 @@ static int s_verify_credentials(struct aws_credentials *credentials) { return AWS_OP_SUCCESS; } -static int s_credentials_provider_process_basic_success(struct aws_allocator *allocator, void *ctx) { - (void)ctx; - +static int s_test_command_expect_success(struct aws_allocator *allocator, const struct aws_string *command) { s_aws_process_tester_init(allocator); struct aws_byte_buf content_buf; struct aws_byte_buf existing_content = aws_byte_buf_from_c_str(aws_string_c_str(s_process_config_file_contents)); aws_byte_buf_init_copy(&content_buf, allocator, &existing_content); - struct aws_byte_cursor cursor = aws_byte_cursor_from_string(s_test_command); + struct aws_byte_cursor cursor = aws_byte_cursor_from_string(command); ASSERT_TRUE(aws_byte_buf_append_dynamic(&content_buf, &cursor) == AWS_OP_SUCCESS); cursor = aws_byte_cursor_from_c_str("\n"); ASSERT_TRUE(aws_byte_buf_append_dynamic(&content_buf, &cursor) == AWS_OP_SUCCESS); @@ -409,8 +383,23 @@ static int s_credentials_provider_process_basic_success(struct aws_allocator *al s_aws_process_tester_cleanup(); return 0; } + +static int s_credentials_provider_process_basic_success(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + return s_test_command_expect_success(allocator, s_test_command); +} AWS_TEST_CASE(credentials_provider_process_basic_success, s_credentials_provider_process_basic_success); +/* Test that stderr is ignored, if the process otherwise succeeds with exit code 0 and valid JSON to stdout. + * Once upon a time stderr and stdout were merged, and mundane logging to stderr would break things. */ +static int s_credentials_provider_process_success_ignores_stderr(struct aws_allocator *allocator, void *ctx) { + (void)ctx; + return s_test_command_expect_success(allocator, s_test_command_with_logging_on_stderr); +} +AWS_TEST_CASE( + credentials_provider_process_success_ignores_stderr, + s_credentials_provider_process_success_ignores_stderr); + static int s_credentials_provider_process_basic_success_from_profile_provider( struct aws_allocator *allocator, void *ctx) { From 030e37702a5826785ac8da7e2f35cce1eae22c05 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Wed, 4 Sep 2024 16:44:49 -0700 Subject: [PATCH 2/4] forgot a file --- tests/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 40235cf8..a520a9f2 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -113,6 +113,7 @@ add_test_case(credentials_provider_process_new_failed) add_test_case(credentials_provider_process_bad_command) add_test_case(credentials_provider_process_incorrect_command_output) add_test_case(credentials_provider_process_basic_success) +add_test_case(credentials_provider_process_success_ignores_stderr) add_test_case(credentials_provider_process_basic_success_from_profile_provider) add_test_case(credentials_provider_process_basic_success_cached) From c8c3a26c75ece57a0b6cae59c1433e9cfc0fd2a9 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Wed, 4 Sep 2024 17:00:07 -0700 Subject: [PATCH 3/4] Windows version of command --- tests/credentials_provider_process_tests.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/credentials_provider_process_tests.c b/tests/credentials_provider_process_tests.c index 68afcbc9..57b00634 100644 --- a/tests/credentials_provider_process_tests.c +++ b/tests/credentials_provider_process_tests.c @@ -144,6 +144,17 @@ AWS_STATIC_STRING_FROM_LITERAL( "\"Expiration\":\"2020-02-25T06:03:31Z\"}'"); #endif +#ifdef _WIN32 +AWS_STATIC_STRING_FROM_LITERAL( + s_test_command_with_logging_on_stderr, + "(" + "echo Logging on stderr >&2" + " && " + "echo {\"Version\": 1, \"AccessKeyId\": \"AccessKey123\", " + "\"SecretAccessKey\": \"SecretAccessKey321\", \"SessionToken\":\"TokenSuccess\", " + "\"Expiration\":\"2020-02-25T06:03:31Z\"}" + ")"); +#else AWS_STATIC_STRING_FROM_LITERAL( s_test_command_with_logging_on_stderr, "(" @@ -153,6 +164,7 @@ AWS_STATIC_STRING_FROM_LITERAL( "\"SecretAccessKey\": \"SecretAccessKey321\", \"SessionToken\":\"TokenSuccess\", " "\"Expiration\":\"2020-02-25T06:03:31Z\"}'" ")"); +#endif AWS_STATIC_STRING_FROM_LITERAL(s_bad_test_command, "/i/dont/know/what/is/this/command"); AWS_STATIC_STRING_FROM_LITERAL(s_bad_command_output, "echo \"Hello, World!\""); From ec7d04317d3b0666841ac1285a402f114b289854 Mon Sep 17 00:00:00 2001 From: Michael Graeb Date: Wed, 4 Sep 2024 20:55:26 -0700 Subject: [PATCH 4/4] stop redirecting stderr to stdout --- source/credentials_provider_process.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/source/credentials_provider_process.c b/source/credentials_provider_process.c index d043ff39..a78c661a 100644 --- a/source/credentials_provider_process.c +++ b/source/credentials_provider_process.c @@ -152,7 +152,22 @@ static void s_check_or_get_with_profile_config( } } -static struct aws_byte_cursor s_stderr_redirect_to_stdout = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL(" 2>&1"); +/* Redirect stderr to /dev/null + * As of Sep 2024, aws_process_run() can only capture stdout, and the + * process's stderr goes into the stderr of the application that launched it. + * Some credentials-processes log to stderr during normal operation. + * To prevent this from polluting the application's stderr, + * we redirect the credential-process's stderr into oblivion. + * + * It would be better to fix aws_process_run() so it captures stderr as well, + * and logging it if the process fails. This is recommended by the SEP: + * > SDKs SHOULD make this error message accessible to the customer. */ +#ifdef _WIN32 +static struct aws_byte_cursor s_stderr_redirect_to_devnull = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL(" 2> nul"); +#else +static struct aws_byte_cursor s_stderr_redirect_to_devnull = AWS_BYTE_CUR_INIT_FROM_STRING_LITERAL(" 2> /dev/null"); +#endif + static struct aws_string *s_get_command( struct aws_allocator *allocator, const struct aws_credentials_provider_process_options *options) { @@ -190,7 +205,7 @@ static struct aws_string *s_get_command( goto on_finish; } - if (aws_byte_buf_append_dynamic(&command_buf, &s_stderr_redirect_to_stdout)) { + if (aws_byte_buf_append_dynamic(&command_buf, &s_stderr_redirect_to_devnull)) { goto on_finish; }