Skip to content

Commit

Permalink
Fix Multiple Shutdown Callback for Profile with STS Code Path (#251)
Browse files Browse the repository at this point in the history
  • Loading branch information
@waahm7
waahm7 authored Sep 12, 2024
1 parent e930f1a commit d5e6eb0
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 24 deletions.
14 changes: 9 additions & 5 deletions source/credentials_provider_profile.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,11 +344,15 @@ static struct aws_credentials_provider *s_create_sts_based_provider(
"static: source_profile set to %s",
aws_string_c_str(aws_profile_property_get_value(source_profile_property)));

struct aws_credentials_provider_profile_options profile_provider_options = *options;
profile_provider_options.profile_name_override =
aws_byte_cursor_from_string(aws_profile_property_get_value(source_profile_property));
/* reuse profile collection instead of reading it again */
profile_provider_options.profile_collection_cached = merged_profiles;
struct aws_credentials_provider_profile_options profile_provider_options = {
.bootstrap = options->bootstrap,
.profile_name_override =
aws_byte_cursor_from_string(aws_profile_property_get_value(source_profile_property)),
/* reuse profile collection instead of reading it again */
.profile_collection_cached = merged_profiles,
.tls_ctx = options->tls_ctx,
.function_table = options->function_table,
};
sts_options.creds_provider =
s_credentials_provider_new_profile_internal(allocator, &profile_provider_options, source_profiles_table);

Expand Down
73 changes: 54 additions & 19 deletions tests/credentials_provider_sts_tests.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ struct aws_mock_sts_tester {

bool fail_connection;

int provider_shutdown_callback_count;

struct aws_event_loop_group *el_group;

struct aws_host_resolver *resolver;
Expand All @@ -78,16 +80,40 @@ static void s_on_connection_manager_shutdown_complete(void *user_data) {
aws_condition_variable_notify_one(&s_tester.signal);
}

static bool s_has_tester_received_shutdown_callback(void *user_data) {
static bool s_has_tester_received_connection_manager_shutdown_callback(void *user_data) {
(void)user_data;

return s_tester.mocked_connection_manager_shutdown_callback_count ==
s_tester.expected_connection_manager_shutdown_callback_count;
}

static void s_aws_wait_for_connection_manager_shutdown_callback(void) {
aws_mutex_lock(&s_tester.lock);
aws_condition_variable_wait_pred(
&s_tester.signal, &s_tester.lock, s_has_tester_received_connection_manager_shutdown_callback, NULL);
aws_mutex_unlock(&s_tester.lock);
}

static void s_on_provider_shutdown(void *user_data) {
(void)user_data;

aws_mutex_lock(&s_tester.lock);
s_tester.provider_shutdown_callback_count++;
aws_mutex_unlock(&s_tester.lock);

aws_condition_variable_notify_one(&s_tester.signal);
}

static bool s_has_tester_received_provider_shutdown_callback(void *user_data) {
(void)user_data;

return s_tester.provider_shutdown_callback_count;
}

static void s_aws_wait_for_provider_shutdown_callback(void) {
aws_mutex_lock(&s_tester.lock);
aws_condition_variable_wait_pred(&s_tester.signal, &s_tester.lock, s_has_tester_received_shutdown_callback, NULL);
aws_condition_variable_wait_pred(
&s_tester.signal, &s_tester.lock, s_has_tester_received_provider_shutdown_callback, NULL);
aws_mutex_unlock(&s_tester.lock);
}

Expand Down Expand Up @@ -508,7 +534,7 @@ static int s_credentials_provider_sts_direct_config_succeeds_fn(struct aws_alloc
s_tester.mocked_requests[0].body.len);

aws_credentials_provider_release(sts_provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
aws_credentials_provider_release(static_provider);
ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

Expand Down Expand Up @@ -561,7 +587,7 @@ static int s_credentials_provider_sts_direct_config_with_external_id_succeeds_fn
s_tester.mocked_requests[0].body.len);

aws_credentials_provider_release(sts_provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
aws_credentials_provider_release(static_provider);
ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

Expand Down Expand Up @@ -658,7 +684,7 @@ static int s_credentials_provider_sts_direct_config_with_region_succeeds_fn(
s_tester.mocked_requests[0].body.len);

aws_credentials_provider_release(sts_provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
aws_credentials_provider_release(static_provider);
ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

Expand Down Expand Up @@ -741,7 +767,7 @@ static int s_credentials_provider_sts_direct_config_with_default_region_succeeds
s_tester.mocked_requests[0].body.len);

aws_credentials_provider_release(sts_provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
aws_credentials_provider_release(static_provider);
ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

Expand Down Expand Up @@ -829,7 +855,7 @@ static int s_credentials_provider_sts_direct_config_with_region_from_config_succ
s_tester.mocked_requests[0].body.len);

aws_credentials_provider_release(sts_provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
aws_credentials_provider_release(static_provider);
aws_file_delete(config_file_str);
aws_string_destroy(config_file_str);
Expand Down Expand Up @@ -915,7 +941,7 @@ static int s_credentials_provider_sts_direct_config_succeeds_after_retry_fn(
s_tester.mocked_requests[0].body.len);

aws_credentials_provider_release(sts_provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
aws_credentials_provider_release(static_provider);
ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

Expand Down Expand Up @@ -999,7 +1025,7 @@ static int s_credentials_provider_sts_direct_config_invalid_doc_fn(struct aws_al
s_tester.mocked_requests[0].body.len);

aws_credentials_provider_release(sts_provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
aws_credentials_provider_release(static_provider);
ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

Expand Down Expand Up @@ -1046,7 +1072,7 @@ static int s_credentials_provider_sts_direct_config_connection_failed_fn(struct
ASSERT_NULL(s_tester.credentials);

aws_credentials_provider_release(sts_provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
aws_credentials_provider_release(static_provider);
ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

Expand Down Expand Up @@ -1096,7 +1122,7 @@ static int s_credentials_provider_sts_direct_config_service_fails_fn(struct aws_
ASSERT_NULL(s_tester.credentials);

aws_credentials_provider_release(sts_provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
aws_credentials_provider_release(static_provider);
ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

Expand Down Expand Up @@ -1166,6 +1192,10 @@ static int s_credentials_provider_sts_from_profile_config_with_chain_fn(struct a
.profile_name_override = aws_byte_cursor_from_c_str("roletest"),
.bootstrap = s_tester.bootstrap,
.function_table = &s_mock_function_table,
.shutdown_options =
{
.shutdown_callback = s_on_provider_shutdown,
},
};
int expected_num_requests = 3;
for (int i = 0; i < expected_num_requests; i++) {
Expand Down Expand Up @@ -1223,7 +1253,12 @@ static int s_credentials_provider_sts_from_profile_config_with_chain_fn(struct a
}

aws_credentials_provider_release(provider);
s_aws_wait_for_connection_manager_shutdown_callback();
s_aws_wait_for_provider_shutdown_callback();
/* There used to be a bug that triggered the shutdown callback multiple times. Sleep for a few seconds
* and validate that we don't trigger the shutdown callback multiple times */
aws_thread_current_sleep(3000000000);
ASSERT_INT_EQUALS(1, s_tester.provider_shutdown_callback_count);
ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

return AWS_OP_SUCCESS;
Expand Down Expand Up @@ -1331,7 +1366,7 @@ static int s_credentials_provider_sts_from_profile_config_with_ecs_credentials_s
}

aws_credentials_provider_release(provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
aws_string_destroy(relative_uri_str);
aws_string_destroy(config_file_str);
aws_string_destroy(creds_file_str);
Expand Down Expand Up @@ -1443,7 +1478,7 @@ static int s_credentials_provider_sts_from_profile_config_with_chain_and_profile
}

aws_credentials_provider_release(provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

return AWS_OP_SUCCESS;
Expand Down Expand Up @@ -1514,7 +1549,7 @@ static int s_credentials_provider_sts_from_profile_config_with_chain_and_partial
ASSERT_NULL(s_tester.credentials);
ASSERT_INT_EQUALS(s_tester.error_code, AWS_AUTH_SIGNING_NO_CREDENTIALS);
aws_credentials_provider_release(provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

return AWS_OP_SUCCESS;
Expand Down Expand Up @@ -1607,7 +1642,7 @@ static int s_credentials_provider_sts_from_self_referencing_profile_fn(struct aw
}

aws_credentials_provider_release(provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

return AWS_OP_SUCCESS;
Expand Down Expand Up @@ -1813,7 +1848,7 @@ static int s_credentials_provider_sts_from_profile_config_succeeds(
s_tester.mocked_requests[0].body.len);

aws_credentials_provider_release(provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

return AWS_OP_SUCCESS;
Expand Down Expand Up @@ -1885,7 +1920,7 @@ static int s_credentials_provider_sts_from_profile_config_with_external_id_fn(
s_tester.mocked_requests[0].body.len);

aws_credentials_provider_release(provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

return AWS_OP_SUCCESS;
Expand Down Expand Up @@ -1982,7 +2017,7 @@ static int s_credentials_provider_sts_from_profile_config_environment_succeeds_f
s_tester.mocked_requests[0].body.len);

aws_credentials_provider_release(provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();

ASSERT_SUCCESS(s_aws_sts_tester_cleanup());

Expand Down Expand Up @@ -2105,7 +2140,7 @@ static int s_credentials_provider_sts_cache_expiration_conflict(struct aws_alloc

aws_credentials_provider_release(cached_provider);
aws_credentials_provider_release(sts_provider);
s_aws_wait_for_provider_shutdown_callback();
s_aws_wait_for_connection_manager_shutdown_callback();
aws_credentials_provider_release(static_provider);

ASSERT_SUCCESS(s_aws_sts_tester_cleanup());
Expand Down

0 comments on commit d5e6eb0

Please sign in to comment.