Skip to content

Commit

Permalink
Merge branch 'main' into checksum-callback
Browse files Browse the repository at this point in the history
  • Loading branch information
TingDaoK authored Jul 31, 2024
2 parents ccb46ee + 94e3342 commit 6698fc9
Show file tree
Hide file tree
Showing 11 changed files with 201 additions and 18 deletions.
19 changes: 16 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ on:
- 'main'

env:
BUILDER_VERSION: v0.9.59
BUILDER_VERSION: v0.9.62
BUILDER_SOURCE: releases
BUILDER_HOST: https://d19elf31gohf1l.cloudfront.net
PACKAGE_NAME: aws-c-s3
Expand Down Expand Up @@ -133,8 +133,21 @@ jobs:
run: |
python .\aws-c-s3\build\deps\aws-c-common\scripts\appverifier_ctest.py --build_directory .\aws-c-s3\build\aws-c-s3
osx:
runs-on: macos-13 # latest
macos:
runs-on: macos-14 # latest
steps:
- name: Checkout Sources
uses: actions/checkout@v4
- name: Build ${{ env.PACKAGE_NAME }} + consumers
run: |
python3 -m venv .venv
source .venv/bin/activate
python3 -c "from urllib.request import urlretrieve; urlretrieve('${{ env.BUILDER_HOST }}/${{ env.BUILDER_SOURCE }}/${{ env.BUILDER_VERSION }}/builder.pyz?run=${{ env.RUN }}', 'builder')"
chmod a+x builder
./builder build -p ${{ env.PACKAGE_NAME }} --cmake-extra=-DASSERT_LOCK_HELD=ON
macos-x64:
runs-on: macos-14-large # latest
steps:
- name: Checkout Sources
uses: actions/checkout@v4
Expand Down
17 changes: 17 additions & 0 deletions include/aws/s3/private/s3_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,10 @@ struct aws_s3_endpoint_options {
* If the transfer speed falls below the specified minimum_throughput_bytes_per_second, the operation is aborted.
*/
struct aws_http_connection_monitoring_options *monitoring_options;

/* An array of `struct aws_byte_cursor` of network interface names. */
const struct aws_byte_cursor *network_interface_names_array;
size_t num_network_interface_names;
};

/* global vtable, only used when mocking for tests */
Expand Down Expand Up @@ -313,6 +317,19 @@ struct aws_s3_client {
*/
struct aws_atomic_var upload_timeout_ms;

/*
* An aws_array_list<struct aws_string *> of network interface names.
*/
struct aws_array_list network_interface_names;
/*
* An array of `struct aws_byte_cursor` of network interface names. The cursors are over the strings in
* `network_interface_names` so that we can easily pass it to the connection manager without any processing. We need
* to create both `struct aws_array_list<struct aws_string *>` and the array of `struct aws_byte_cursor` since byte
* cursors are non-owning and we need to ensure that the underlying memory is valid as long as the client lives.
*/
struct aws_byte_cursor *network_interface_names_cursor_array;
size_t num_network_interface_names;

struct {
/* Number of overall requests currently being processed by the client. */
struct aws_atomic_var num_requests_in_flight;
Expand Down
35 changes: 25 additions & 10 deletions include/aws/s3/s3_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,9 @@ struct aws_s3_client_config {
enum aws_s3_meta_request_tls_mode tls_mode;

/* TLS Options to be used for each connection, if tls_mode is ENABLED. When compiling with BYO_CRYPTO, and tls_mode
* is ENABLED, this is required. Otherwise, this is optional. */
struct aws_tls_connection_options *tls_connection_options;
* is ENABLED, this is required. Otherwise, this is optional.
*/
const struct aws_tls_connection_options *tls_connection_options;

/**
* Required.
Expand All @@ -387,7 +388,7 @@ struct aws_s3_client_config {
*
* TODO: deprecate this structure from auth, introduce a new S3 specific one.
*/
struct aws_signing_config_aws *signing_config;
const struct aws_signing_config_aws *signing_config;

/**
* Optional.
Expand Down Expand Up @@ -445,7 +446,7 @@ struct aws_s3_client_config {
* If the connection_type is AWS_HPCT_HTTP_LEGACY, it will be converted to AWS_HPCT_HTTP_TUNNEL if tls_mode is
* ENABLED. Otherwise, it will be converted to AWS_HPCT_HTTP_FORWARD.
*/
struct aws_http_proxy_options *proxy_options;
const struct aws_http_proxy_options *proxy_options;

/**
* Optional.
Expand All @@ -454,7 +455,7 @@ struct aws_s3_client_config {
* configuration from environment.
* Only works when proxy_options is not set. If both are set, configuration from proxy_options is used.
*/
struct proxy_env_var_settings *proxy_ev_settings;
const struct proxy_env_var_settings *proxy_ev_settings;

/**
* Optional.
Expand All @@ -466,15 +467,15 @@ struct aws_s3_client_config {
* Optional.
* Set keepalive to periodically transmit messages for detecting a disconnected peer.
*/
struct aws_s3_tcp_keep_alive_options *tcp_keep_alive_options;
const struct aws_s3_tcp_keep_alive_options *tcp_keep_alive_options;

/**
* Optional.
* Configuration options for connection monitoring.
* If the transfer speed falls below the specified minimum_throughput_bytes_per_second, the operation is aborted.
* If set to NULL, default values are used.
*/
struct aws_http_connection_monitoring_options *monitoring_options;
const struct aws_http_connection_monitoring_options *monitoring_options;

/**
* Enable backpressure and prevent response data from downloading faster than you can handle it.
Expand Down Expand Up @@ -514,6 +515,20 @@ struct aws_s3_client_config {
*/
aws_s3express_provider_factory_fn *s3express_provider_override_factory;
void *factory_user_data;

/**
* THIS IS AN EXPERIMENTAL AND UNSTABLE API
* (Optional)
* An array of network interface names. The client will distribute the
* connections across network interface names provided in this array. If any interface name is invalid, goes down,
* or has any issues like network access, you will see connection failures.
*
* This option is only supported on Linux, MacOS, and platforms that have either SO_BINDTODEVICE or IP_BOUND_IF. It
* is not supported on Windows. `AWS_ERROR_PLATFORM_NOT_SUPPORTED` will be raised on unsupported platforms. On
* Linux, SO_BINDTODEVICE is used and requires kernel version >= 5.7 or root privileges.
*/
const struct aws_byte_cursor *network_interface_names_array;
size_t num_network_interface_names;
};

struct aws_s3_checksum_config {
Expand Down Expand Up @@ -558,7 +573,7 @@ struct aws_s3_checksum_config {
*
* If the response checksum was validated by client, the result will indicate which algorithm was picked.
*/
struct aws_array_list *validate_checksum_algorithms;
const struct aws_array_list *validate_checksum_algorithms;
};

/**
Expand Down Expand Up @@ -746,7 +761,7 @@ struct aws_s3_meta_request_options {
* - Both Host and Endpoint is set - Host header must match Authority of
* Endpoint uri. Port and Scheme from endpoint is used.
*/
struct aws_uri *endpoint;
const struct aws_uri *endpoint;

/**
* Optional.
Expand All @@ -764,7 +779,7 @@ struct aws_s3_meta_request_options {
* Set this hint to help the S3 client choose the best strategy for this particular file.
* This is just used as an estimate, so it's okay to provide an approximate value if the exact size is unknown.
*/
uint64_t *object_size_hint;
const uint64_t *object_size_hint;
};

/* Result details of a meta request.
Expand Down
1 change: 1 addition & 0 deletions source/s3.c
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ static void s_s3_request_type_info_init(struct aws_allocator *allocator) {

static void s_s3_request_type_info_clean_up(void) {
aws_hash_table_clean_up(&s_s3_operation_name_to_request_type_table);
AWS_ZERO_ARRAY(s_s3_request_type_info_array);
}

struct aws_string *aws_s3_request_type_to_operation_name_static_string(enum aws_s3_request_type type) {
Expand Down
49 changes: 48 additions & 1 deletion source/s3_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -579,6 +579,34 @@ struct aws_s3_client *aws_s3_client_new(
*((bool *)&client->enable_read_backpressure) = client_config->enable_read_backpressure;
*((size_t *)&client->initial_read_window) = client_config->initial_read_window;

client->num_network_interface_names = client_config->num_network_interface_names;
if (client_config->num_network_interface_names > 0) {
AWS_LOGF_DEBUG(
AWS_LS_S3_CLIENT,
"id=%p Client received network interface names array with length %zu.",
(void *)client,
client->num_network_interface_names);
aws_array_list_init_dynamic(
&client->network_interface_names,
client->allocator,
client_config->num_network_interface_names,
sizeof(struct aws_string *));
client->network_interface_names_cursor_array = aws_mem_calloc(
client->allocator, client_config->num_network_interface_names, sizeof(struct aws_byte_cursor));
for (size_t i = 0; i < client_config->num_network_interface_names; i++) {
struct aws_byte_cursor interface_name = client_config->network_interface_names_array[i];
struct aws_string *interface_name_str = aws_string_new_from_cursor(client->allocator, &interface_name);
aws_array_list_push_back(&client->network_interface_names, &interface_name_str);
client->network_interface_names_cursor_array[i] = aws_byte_cursor_from_string(interface_name_str);
AWS_LOGF_DEBUG(
AWS_LS_S3_CLIENT,
"id=%p network_interface_names_array[%zu]=" PRInSTR "",
(void *)client,
i,
AWS_BYTE_CURSOR_PRI(client->network_interface_names_cursor_array[i]));
}
}

return client;

on_error:
Expand Down Expand Up @@ -713,6 +741,15 @@ static void s_s3_client_finish_destroy_default(struct aws_s3_client *client) {
void *shutdown_user_data = client->shutdown_callback_user_data;

aws_s3_buffer_pool_destroy(client->buffer_pool);

aws_mem_release(client->allocator, client->network_interface_names_cursor_array);
for (size_t i = 0; i < client->num_network_interface_names; i++) {
struct aws_string *interface_name = NULL;
aws_array_list_get_at(&client->network_interface_names, &interface_name, i);
aws_string_destroy(interface_name);
}
aws_array_list_clean_up(&client->network_interface_names);

aws_mem_release(client->allocator, client);
client = NULL;

Expand Down Expand Up @@ -1048,6 +1085,8 @@ struct aws_s3_meta_request *aws_s3_client_make_meta_request(
.connect_timeout_ms = client->connect_timeout_ms,
.tcp_keep_alive_options = client->tcp_keep_alive_options,
.monitoring_options = &client->monitoring_options,
.network_interface_names_array = client->network_interface_names_cursor_array,
.num_network_interface_names = client->num_network_interface_names,
};

endpoint = aws_s3_endpoint_new(client->allocator, &endpoint_options);
Expand Down Expand Up @@ -2104,12 +2143,20 @@ static void s_s3_client_on_acquire_http_connection(
error_code,
aws_error_str(error_code));

if (error_code == AWS_IO_DNS_INVALID_NAME || error_code == AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE) {
if (error_code == AWS_IO_DNS_INVALID_NAME || error_code == AWS_IO_TLS_ERROR_NEGOTIATION_FAILURE ||
error_code == AWS_ERROR_PLATFORM_NOT_SUPPORTED || error_code == AWS_IO_SOCKET_INVALID_OPTIONS) {
/**
* Fall fast without retry
* - Invalid DNS name will not change after retry.
* - TLS negotiation is expensive and retry will not help in most case.
*/
AWS_LOGF_ERROR(
AWS_LS_S3_META_REQUEST,
"id=%p Meta request cannot recover from error %d (%s) while acquiring HTTP connection. (request=%p)",
(void *)meta_request,
error_code,
aws_error_str(error_code),
(void *)request);
goto error_fail;
}

Expand Down
14 changes: 11 additions & 3 deletions source/s3_endpoint.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ static struct aws_http_connection_manager *s_s3_endpoint_create_http_connection_
const struct proxy_env_var_settings *proxy_ev_settings,
uint32_t connect_timeout_ms,
const struct aws_s3_tcp_keep_alive_options *tcp_keep_alive_options,
const struct aws_http_connection_monitoring_options *monitoring_options);
const struct aws_http_connection_monitoring_options *monitoring_options,
const struct aws_byte_cursor *network_interface_names_array,
size_t num_network_interface_names);

static void s_s3_endpoint_http_connection_manager_shutdown_callback(void *user_data);

Expand Down Expand Up @@ -109,7 +111,9 @@ struct aws_s3_endpoint *aws_s3_endpoint_new(
options->proxy_ev_settings,
options->connect_timeout_ms,
options->tcp_keep_alive_options,
options->monitoring_options);
options->monitoring_options,
options->network_interface_names_array,
options->num_network_interface_names);

if (endpoint->http_connection_manager == NULL) {
goto error_cleanup;
Expand Down Expand Up @@ -137,7 +141,9 @@ static struct aws_http_connection_manager *s_s3_endpoint_create_http_connection_
const struct proxy_env_var_settings *proxy_ev_settings,
uint32_t connect_timeout_ms,
const struct aws_s3_tcp_keep_alive_options *tcp_keep_alive_options,
const struct aws_http_connection_monitoring_options *monitoring_options) {
const struct aws_http_connection_monitoring_options *monitoring_options,
const struct aws_byte_cursor *network_interface_names_array,
size_t num_network_interface_names) {

AWS_PRECONDITION(endpoint);
AWS_PRECONDITION(client_bootstrap);
Expand Down Expand Up @@ -175,6 +181,8 @@ static struct aws_http_connection_manager *s_s3_endpoint_create_http_connection_
manager_options.shutdown_complete_callback = s_s3_endpoint_http_connection_manager_shutdown_callback;
manager_options.shutdown_complete_user_data = endpoint;
manager_options.proxy_ev_settings = proxy_ev_settings;
manager_options.network_interface_names_array = network_interface_names_array;
manager_options.num_network_interface_names = num_network_interface_names;
if (monitoring_options != NULL) {
manager_options.monitoring_options = monitoring_options;
}
Expand Down
2 changes: 2 additions & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ file(GLOB TEST_SRC "*.c")
file(GLOB TEST_HDRS "*.h")
file(GLOB TESTS ${TEST_HDRS} ${TEST_SRC})

add_test_case(test_s3_library_init_cleanup_init_cleanup)
add_test_case(test_s3_copy_http_message)
add_test_case(test_s3_message_util_assign_body)
add_test_case(test_s3_ranged_get_object_message_new)
Expand Down Expand Up @@ -298,6 +299,7 @@ add_net_test_case(test_s3_list_bucket_valid)
# Tests against local mock server
if(ENABLE_MOCK_SERVER_TESTS)
add_net_test_case(multipart_upload_mock_server)
add_net_test_case(multipart_upload_with_network_interface_names_mock_server)
add_net_test_case(multipart_upload_checksum_with_retry_mock_server)
add_net_test_case(multipart_download_checksum_with_retry_mock_server)
add_net_test_case(async_internal_error_from_complete_multipart_mock_server)
Expand Down
60 changes: 60 additions & 0 deletions tests/s3_mock_server_tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,66 @@ TEST_CASE(multipart_upload_mock_server) {
return AWS_OP_SUCCESS;
}

TEST_CASE(multipart_upload_with_network_interface_names_mock_server) {
(void)ctx;

struct aws_s3_tester tester;
ASSERT_SUCCESS(aws_s3_tester_init(allocator, &tester));
struct aws_byte_cursor *interface_names_array = aws_mem_calloc(allocator, 2, sizeof(struct aws_byte_cursor));
char *localhost_interface = "\0";
#if defined(AWS_OS_APPLE)
localhost_interface = "lo0";
#else
localhost_interface = "lo";
#endif
interface_names_array[0] = aws_byte_cursor_from_c_str(localhost_interface);
interface_names_array[1] = aws_byte_cursor_from_c_str(localhost_interface);

struct aws_s3_tester_client_options client_options = {
.part_size = MB_TO_BYTES(5),
.tls_usage = AWS_S3_TLS_DISABLED,
.network_interface_names_array = interface_names_array,
.num_network_interface_names = 2,
};

struct aws_s3_client *client = NULL;
ASSERT_SUCCESS(aws_s3_tester_client_new(&tester, &client_options, &client));

struct aws_byte_cursor object_path = aws_byte_cursor_from_c_str("/default");

struct aws_s3_tester_meta_request_options put_options = {
.allocator = allocator,
.meta_request_type = AWS_S3_META_REQUEST_TYPE_PUT_OBJECT,
.client = client,
.checksum_algorithm = AWS_SCA_CRC32,
.validate_get_response_checksum = false,
.put_options =
{
.object_size_mb = 10,
.object_path_override = object_path,
},
.mock_server = true,
.validate_type = AWS_S3_TESTER_VALIDATE_TYPE_NO_VALIDATE,
};
struct aws_s3_meta_request_test_results out_results;
aws_s3_meta_request_test_results_init(&out_results, allocator);
ASSERT_SUCCESS(aws_s3_tester_send_meta_request_with_options(&tester, &put_options, &out_results));
if (out_results.finished_error_code != 0) {
#if !defined(AWS_OS_APPLE) && !defined(AWS_OS_LINUX)
if (out_results.finished_error_code == AWS_ERROR_PLATFORM_NOT_SUPPORTED) {
return AWS_OP_SKIP;
}
#endif
ASSERT_TRUE(false, "aws_s3_tester_send_meta_request_with_options(() failed");
}
aws_s3_meta_request_test_results_clean_up(&out_results);
aws_s3_client_release(client);
aws_s3_tester_clean_up(&tester);
aws_mem_release(allocator, interface_names_array);

return AWS_OP_SUCCESS;
}

TEST_CASE(multipart_upload_checksum_with_retry_mock_server) {
(void)ctx;
/**
Expand Down
Loading

0 comments on commit 6698fc9

Please sign in to comment.