Skip to content

Commit 6905e70

Browse files
authored
fix(sse/access-logs): handle deleted object (#6175)
1 parent 1010bce commit 6905e70

2 files changed

Lines changed: 74 additions & 1 deletion

File tree

api/sse/sse_service.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import boto3
88
import gnupg # type: ignore[import-untyped]
9+
from botocore.exceptions import ClientError
910
from django.conf import settings
1011

1112
from sse import tasks
@@ -61,7 +62,16 @@ def stream_access_logs() -> Generator[SSEAccessLogs, None, None]:
6162
bucket = boto3.resource("s3").Bucket(settings.AWS_SSE_LOGS_BUCKET_NAME)
6263

6364
for log_file in bucket.objects.all():
64-
encrypted_body = log_file.get()["Body"].read()
65+
try:
66+
encrypted_body = log_file.get()["Body"].read()
67+
except ClientError as e:
68+
if e.response["Error"]["Code"] == "NoSuchKey":
69+
logger.warning(
70+
"Log file %s has already been deleted, skipping", log_file.key
71+
)
72+
continue
73+
raise
74+
6575
decrypted_body = gpg.decrypt(encrypted_body)
6676

6777
reader = csv.reader(StringIO(decrypted_body.data.decode()))

api/tests/unit/sse/test_sse_service.py

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import boto3
22
import pytest
3+
from botocore.exceptions import ClientError
34
from django.conf import settings
45
from moto import mock_s3 # type: ignore[import-untyped]
56
from pytest_lazyfixture import lazy_fixture # type: ignore[import-untyped]
@@ -160,3 +161,65 @@ def test_stream_access_logs(mocker: MockerFixture, aws_credentials: None) -> Non
160161

161162
# And, bucket is now empty
162163
assert "Contents" not in s3_client.list_objects(Bucket=bucket_name)
164+
165+
166+
def test_stream_access_logs_handles_deleted_files(
167+
mocker: MockerFixture, aws_credentials: None
168+
) -> None:
169+
# Given - Mock bucket with objects where first raises NoSuchKey
170+
mock_obj_1 = mocker.MagicMock()
171+
mock_obj_1.key = "file1"
172+
mock_obj_1.get.side_effect = ClientError(
173+
{"Error": {"Code": "NoSuchKey"}}, "GetObject"
174+
)
175+
176+
mock_obj_2 = mocker.MagicMock()
177+
mock_obj_2.key = "file2"
178+
mock_obj_2.get.return_value = {"Body": mocker.MagicMock(read=lambda: b"data2")}
179+
180+
mock_bucket = mocker.MagicMock()
181+
mock_bucket.objects.all.return_value = [mock_obj_1, mock_obj_2]
182+
183+
mocker.patch(
184+
"sse.sse_service.boto3.resource"
185+
).return_value.Bucket.return_value = mock_bucket
186+
mocker.patch(
187+
"sse.sse_service.gnupg.GPG"
188+
).return_value.decrypt.return_value = mocker.MagicMock(
189+
data=b"2023-11-27T06:42:47+0000,test_key"
190+
)
191+
mocked_logger = mocker.patch("sse.sse_service.logger")
192+
193+
# When
194+
logs = list(stream_access_logs())
195+
196+
# Then - Should skip first file with warning and process second
197+
assert len(logs) == 1
198+
mocked_logger.warning.assert_called_once_with(
199+
"Log file %s has already been deleted, skipping", "file1"
200+
)
201+
202+
203+
def test_stream_access_logs_reraises_non_nosuchkey_errors(
204+
mocker: MockerFixture, aws_credentials: None
205+
) -> None:
206+
# Given - Mock bucket with object that raises AccessDenied error
207+
mock_obj = mocker.MagicMock()
208+
mock_obj.key = "file1"
209+
mock_obj.get.side_effect = ClientError(
210+
{"Error": {"Code": "AccessDenied"}}, "GetObject"
211+
)
212+
213+
mock_bucket = mocker.MagicMock()
214+
mock_bucket.objects.all.return_value = [mock_obj]
215+
216+
mocker.patch(
217+
"sse.sse_service.boto3.resource"
218+
).return_value.Bucket.return_value = mock_bucket
219+
mocker.patch("sse.sse_service.gnupg.GPG")
220+
221+
# When/Then - Should re-raise the ClientError
222+
with pytest.raises(ClientError) as exc_info:
223+
list(stream_access_logs())
224+
225+
assert exc_info.value.response["Error"]["Code"] == "AccessDenied"

0 commit comments

Comments
 (0)