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.

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

[v2] S3 expires timestamp port #8935

Merged
merged 9 commits into from
Oct 7, 2024
Merged
5 changes: 5 additions & 0 deletions .changes/next-release/enhancement-s3-74544.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "enhancement",
"category": "``s3``",
"description": "Adds logic to gracefully handle invalid timestamps returned in the Expires header."
}
17 changes: 17 additions & 0 deletions awscli/bcdoc/restdoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,23 @@ def push_write(self, s):
"""
self._writes.append(s)

def find_last_write(self, content):
aemous marked this conversation as resolved.
Show resolved Hide resolved
"""
Returns the index of the last occurrence of the content argument
in the stack, or returns None if content is not on the stack.
"""
try:
return len(self._writes) - self._writes[::-1].index(content) - 1
except ValueError:
return None

def insert_write(self, index, content):
"""
Inserts the content argument to the stack directly before the
supplied index.
"""
self._writes.insert(index, content)

def getvalue(self):
"""
Returns the current content of the document as a string.
Expand Down
8 changes: 8 additions & 0 deletions awscli/botocore/data/s3/2006-03-01/service-2.sdk-extras.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"version": 1.0,
"merge": {
"shapes": {
"Expires":{"type":"timestamp"}
}
}
}
3 changes: 3 additions & 0 deletions awscli/botocore/docs/bcdoc/restdoc.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@ def get_section(self, name):
"""Retrieve a section"""
return self._structure[name]

def has_section(self, name):
return name in self._structure

def delete_section(self, name):
"""Delete a section"""
del self._structure[name]
Expand Down
8 changes: 8 additions & 0 deletions awscli/botocore/endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -218,9 +218,17 @@ def _do_get_response(self, request, operation_model, context):
history_recorder.record('HTTP_RESPONSE', http_response_record_dict)

protocol = operation_model.metadata['protocol']
customized_response_dict = {}
self._event_emitter.emit(
f"before-parse.{service_id}.{operation_model.name}",
operation_model=operation_model,
response_dict=response_dict,
customized_response_dict=customized_response_dict,
)
parser = self._response_parser_factory.create_parser(protocol)
parsed_response = parser.parse(
response_dict, operation_model.output_shape)
parsed_response.update(customized_response_dict)
# Do a second parsing pass to pick up on any modeled error fields
# NOTE: Ideally, we would push this down into the parser classes but
# they currently have no reference to the operation or service model
Expand Down
67 changes: 66 additions & 1 deletion awscli/botocore/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -1056,6 +1056,69 @@ def customize_endpoint_resolver_builtins(
builtins[EndpointResolverBuiltins.AWS_S3_FORCE_PATH_STYLE] = False


def handle_expires_header(
operation_model, response_dict, customized_response_dict, **kwargs
):
if _has_expires_shape(operation_model.output_shape):
if expires_value := response_dict.get('headers', {}).get('Expires'):
customized_response_dict['ExpiresString'] = expires_value
try:
utils.parse_timestamp(expires_value)
except (ValueError, RuntimeError):
logger.warning(
f'Failed to parse the "Expires" member as a timestamp: {expires_value}. '
f'The unparsed value is available in the response under "ExpiresString".'
)
del response_dict['headers']['Expires']


def _has_expires_shape(shape):
if not shape:
return False
return any(
member_shape.name == 'Expires'
and member_shape.serialization.get('name') == 'Expires'
for member_shape in shape.members.values()
)


def document_expires_shape(section, event_name, **kwargs):
# Updates the documentation for S3 operations that include the 'Expires' member
# in their response structure. Documents a synthetic member 'ExpiresString' and
# includes a deprecation notice for 'Expires'.
if 'response-example' in event_name:
if not section.has_section('structure-value'):
return
parent = section.get_section('structure-value')
if not parent.has_section('Expires'):
return
param_line = parent.get_section('Expires')
param_line.add_new_section('ExpiresString')
new_param_line = param_line.get_section('ExpiresString')
new_param_line.write("'ExpiresString': 'string',")
new_param_line.style.new_line()
elif 'response-params' in event_name:
if not section.has_section('Expires'):
return
param_section = section.get_section('Expires')
# Add a deprecation notice for the "Expires" param
doc_section = param_section.get_section('param-documentation')
doc_section.style.start_note()
doc_section.write(
'This member has been deprecated. Please use ``ExpiresString`` instead.'
)
doc_section.style.end_note()
# Document the "ExpiresString" param
new_param_section = param_section.add_new_section('ExpiresString')
new_param_section.style.new_paragraph()
new_param_section.write('- **ExpiresString** *(string) --*')
new_param_section.style.indent()
new_param_section.style.new_paragraph()
new_param_section.write(
'The raw, unparsed value of the ``Expires`` field.'
)


# This is a list of (event_name, handler).
# When a Session is created, everything in this list will be
# automatically registered with that Session.
Expand All @@ -1079,7 +1142,7 @@ def customize_endpoint_resolver_builtins(
('after-call.ec2.GetConsoleOutput', decode_console_output),
('after-call.cloudformation.GetTemplate', json_decode_template_body),
('after-call.s3.GetBucketLocation', parse_get_bucket_location),

('before-parse.s3.*', handle_expires_header),
('before-parameter-build', generate_idempotent_uuid),

('before-parameter-build.s3', validate_bucket_name),
Expand All @@ -1102,6 +1165,8 @@ def customize_endpoint_resolver_builtins(
('before-parameter-build.s3-control', remove_accid_host_prefix_from_model),
('docs.*.s3.CopyObject.complete-section', document_copy_source_form),
('docs.*.s3.UploadPartCopy.complete-section', document_copy_source_form),
('docs.response-example.s3.*.complete-section', document_expires_shape),
('docs.response-params.s3.*.complete-section', document_expires_shape),
('before-endpoint-resolution.s3', customize_endpoint_resolver_builtins),

('before-call.s3', add_expect_header),
Expand Down
33 changes: 33 additions & 0 deletions awscli/customizations/s3events.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,20 @@ def register_event_stream_arg(event_handlers):
event_handlers.register(
'building-argument-table.s3api.select-object-content',
add_event_stream_output_arg)

event_handlers.register_last(
'doc-output.s3api.select-object-content',
replace_event_stream_docs
)


def register_document_expires_string(event_handlers):
event_handlers.register_last(
'doc-output.s3api',
document_expires_string
)


def add_event_stream_output_arg(argument_table, operation_model,
session, **kwargs):
argument_table['outfile'] = S3SelectStreamOutputArgument(
Expand All @@ -57,6 +65,31 @@ def replace_event_stream_docs(help_command, **kwargs):
"object content is written to the specified outfile.\n")


def document_expires_string(help_command, **kwargs):
aemous marked this conversation as resolved.
Show resolved Hide resolved
doc = help_command.doc
expires_field_idx = doc.find_last_write('Expires -> (timestamp)')

if expires_field_idx is None:
return

deprecation_note_and_expires_string = [
f'\n\n\n{" " * doc.style.indentation * doc.style.indent_width}',
'.. note::',
f'\n\n\n{" " * (doc.style.indentation + 1) * doc.style.indent_width}',
'This member has been deprecated. Please use `ExpiresString` instead.\n',
f'\n\n{" " * doc.style.indentation * doc.style.indent_width}',
f'\n\n{" " * doc.style.indentation * doc.style.indent_width}',
'ExpiresString -> (string)\n\n',
'\tThe raw, unparsed value of the ``Expires`` field.',
f'\n\n{" " * doc.style.indentation * doc.style.indent_width}'
]

for idx, write in enumerate(deprecation_note_and_expires_string):
# We add 4 to the index of the expires field name because each
# field in the output section consists of exactly 4 elements.
doc.insert_write(expires_field_idx + idx + 4, write)


class S3SelectStreamOutputArgument(CustomArgument):
_DOCUMENT_AS_REQUIRED = True

Expand Down
3 changes: 2 additions & 1 deletion awscli/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
from awscli.customizations.waiters import register_add_waiters
from awscli.customizations.opsworkscm import register_alias_opsworks_cm
from awscli.customizations.servicecatalog import register_servicecatalog_commands
from awscli.customizations.s3events import register_event_stream_arg
from awscli.customizations.s3events import register_event_stream_arg, register_document_expires_string
from awscli.customizations.sessionmanager import register_ssm_session
from awscli.customizations.logs import register_logs_commands
from awscli.customizations.devcommands import register_dev_commands
Expand Down Expand Up @@ -191,6 +191,7 @@ def awscli_initialize(event_handlers):
register_history_mode(event_handlers)
register_history_commands(event_handlers)
register_event_stream_arg(event_handlers)
register_document_expires_string(event_handlers)
dlm_initialize(event_handlers)
register_ssm_session(event_handlers)
register_logs_commands(event_handlers)
Expand Down
36 changes: 36 additions & 0 deletions tests/functional/botocore/test_s3.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import re

import pytest
from dateutil.tz import tzutc

from tests import (
create_session, mock, temporary_file, unittest,
Expand Down Expand Up @@ -1223,6 +1224,41 @@ def test_500_error_with_non_xml_body(self):
self.assertEqual(len(http_stubber.requests), 2)


class TestS3ExpiresHeaderResponse(BaseS3OperationTest):
def test_valid_expires_value_in_response(self):
expires_value = "Thu, 01 Jan 1970 00:00:00 GMT"
mock_headers = {'expires': expires_value}
s3 = self.session.create_client("s3")
with ClientHTTPStubber(s3) as http_stubber:
http_stubber.add_response(headers=mock_headers)
response = s3.get_object(Bucket='mybucket', Key='mykey')
self.assertEqual(
response.get('Expires'),
datetime.datetime(1970, 1, 1, tzinfo=tzutc()),
)
self.assertEqual(response.get('ExpiresString'), expires_value)

def test_invalid_expires_value_in_response(self):
expires_value = "Invalid Date"
mock_headers = {'expires': expires_value}
warning_msg = 'Failed to parse the "Expires" member as a timestamp'
s3 = self.session.create_client("s3")
with self.assertLogs('botocore.handlers', level='WARNING') as log:
with ClientHTTPStubber(s3) as http_stubber:
http_stubber.add_response(headers=mock_headers)
response = s3.get_object(Bucket='mybucket', Key='mykey')
self.assertNotIn(
'expires',
response.get('ResponseMetadata').get('HTTPHeaders'),
)
self.assertNotIn('Expires', response)
self.assertEqual(response.get('ExpiresString'), expires_value)
self.assertTrue(
any(warning_msg in entry for entry in log.output),
f'Expected warning message not found in logs. Logs: {log.output}',
)


class TestWriteGetObjectResponse(BaseS3ClientConfigurationTest):
def create_stubbed_s3_client(self, **kwargs):
client = self.create_s3_client(**kwargs)
Expand Down
27 changes: 27 additions & 0 deletions tests/unit/bcdoc/test_document.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@

class TestReSTDocument(unittest.TestCase):

def _write_array(self, doc, arr):
for elt in arr:
doc.write(elt)

def test_write(self):
doc = ReSTDocument()
doc.write('foo')
Expand All @@ -36,6 +40,29 @@ def test_writeln(self):
doc.writeln('foo')
self.assertEqual(doc.getvalue(), b'foo\n')

def test_find_last_write(self):
doc = ReSTDocument()
self._write_array(doc, ['a', 'b', 'c', 'd', 'e'])
expected_index = 0
self.assertEqual(doc.find_last_write('a'), expected_index)

def test_find_last_write_duplicates(self):
doc = ReSTDocument()
self._write_array(doc, ['a', 'b', 'c', 'a', 'e'])
expected_index = 3
self.assertEqual(doc.find_last_write('a'), expected_index)

def test_find_last_write_not_found(self):
doc = ReSTDocument()
self._write_array(doc, ['a', 'b', 'c', 'd', 'e'])
self.assertIsNone(doc.find_last_write('f'))

def test_insert_write(self):
doc = ReSTDocument()
self._write_array(doc, ['foo', 'bar'])
doc.insert_write(1, 'baz')
self.assertEqual(doc.getvalue(), b'foobazbar')

def test_include_doc_string(self):
doc = ReSTDocument()
doc.include_doc_string('<p>this is a <code>test</code></p>')
Expand Down
16 changes: 10 additions & 6 deletions tests/unit/botocore/test_endpoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,13 +76,16 @@ def setUp(self):
def tearDown(self):
self.factory_patch.stop()

def get_emitter_responses(self, num_retries=0, sleep_time=0):
def get_emitter_responses(self, num_retries=0, sleep_time=0, num_events=4):
emitter_responses = []
# We emit the following events:
# 1. request-created
# 2. before-send
# 3. before-parse (may not be emitted if certain errors are thrown)
# 4. response-received
response_request_emitter_responses = [
[(None, None)], # emit() response for request-created
[(None, None)], # emit() response for before-send
[(None, None)], # emit() response for response-received
]
[(None, None)] # emit() response for each emitted event
] * num_events
for _ in range(num_retries):
emitter_responses.extend(response_request_emitter_responses)
# emit() response for retry for sleep time
Expand Down Expand Up @@ -195,14 +198,15 @@ def test_retry_events_can_alter_behavior(self):
expected_events=[
'request-created.ec2.DescribeInstances',
'before-send.ec2.DescribeInstances',
'before-parse.ec2.DescribeInstances',
'response-received.ec2.DescribeInstances',
'needs-retry.ec2.DescribeInstances',
] * 2
)

def test_retry_on_socket_errors(self):
self.event_emitter.emit.side_effect = self.get_emitter_responses(
num_retries=1)
num_retries=1, num_events=3)
self.http_session.send.side_effect = HTTPClientError(error='wrapped')
with self.assertRaises(HTTPClientError):
self.endpoint.make_request(self._operation, request_dict())
Expand Down
Loading
Loading