fix(parser): make etag optional field on S3 notification events

[benjamingorman]

Issue number: #4156

Summary

etag is not guaranteed to be present on an S3 event (if it's a DeleteObject event for example) so this MR changes the type of etag on S3EventNotificationObjectModel to be Optional.

Changes

Please provide a summary of what's being changed

• Modified S3EventNotificationObjectModel
• Modified S3EventBridgeNotificationObject

User experience

Please share what the user experience looks like before and after this change

Before: S3 DeleteObject event fails to parse, with a pydantic validation error

After: S3 DeleteObject event is parsed correctly

Checklist

If your change doesn't seem to apply, please leave them unchecked.

• Meet tenets criteria
• I have performed a self-review of this change
• Changes have been tested
• [] Changes are documented
• PR title follows conventional commit semantics

Is this a breaking change?

It simply reflects the reality of the event in AWS.

RFC issue number:

Checklist:

• Migration process documented
• Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

[boring-cyborg]

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.31%. Comparing base (e14e768) to head (0fe964d).
Report is 407 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4173      +/-   ##
===========================================
- Coverage    96.38%   96.31%   -0.07%     
===========================================
  Files          214      217       +3     
  Lines        10030    10437     +407     
  Branches      1846     1930      +84     
===========================================
+ Hits          9667    10052     +385     
- Misses         259      270      +11     
- Partials       104      115      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[leandrodamascena]

Thanks for submitting the PR @benjamingorman! I think we need to change other fields like size, version_id and sequencer in the event source data class. They are already optional in Parser and based on the payload you sent they are not present.

Let me fix this and push a commit.

[am29d]

Make also sure to remove the etag and size from the test event in

powertools-lambda-python/tests/events/s3EventBridgeNotificationObjectDeletedEvent.json

Line 20 in fe26d1c

"etag": "4e68adba0abe2dc8653dc3354e14c01d",

[heitorlessa]

Looking...

[heitorlessa]

Pushed the fixes for tests and used sane defaults for etag (S3->EventBridge) and eTag (S3->Lambda/SNS/SQS).

Improved docstrings to explain when that's the case. Pending CI and we can merge.

[dreamorosi]

For reference, I have tested the events with an actual stack, these are two examples of events that trigger a function:

{
    "version": "0",
    "id": "1d0e793f-a560-c0b2-9f0f-f0046cf8d051",
    "detail-type": "Object Created",
    "source": "aws.s3",
    "account": "536254204126",
    "time": "2024-05-07T09:31:21Z",
    "region": "eu-west-1",
    "resources": [
        "arn:aws:s3:::s3teststack-mybucketf68f3ff0-gscrrokfnvnw"
    ],
    "detail": {
        "version": "0",
        "bucket": {
            "name": "s3teststack-mybucketf68f3ff0-gscrrokfnvnw"
        },
        "object": {
            "key": "surprised-pikachu.gif",
            "size": 2638774,
            "etag": "fb21c5a0ff18e29aab890d1d1f6d6e64",
            "sequencer": "006639F4E9AAD27BAD"
        },
        "request-id": "ACJYMRYT9T7HS3KE",
        "requester": "536254204126",
        "source-ip-address": "15.248.3.15",
        "reason": "PutObject"
    }
}

and

{
    "version": "0",
    "id": "46bf2718-5e27-a74d-ec6b-2d95d4e21844",
    "detail-type": "Object Deleted",
    "source": "aws.s3",
    "account": "536254204126",
    "time": "2024-05-07T09:31:52Z",
    "region": "eu-west-1",
    "resources": [
        "arn:aws:s3:::s3teststack-mybucketf68f3ff0-gscrrokfnvnw"
    ],
    "detail": {
        "version": "0",
        "bucket": {
            "name": "s3teststack-mybucketf68f3ff0-gscrrokfnvnw"
        },
        "object": {
            "key": "surprised-pikachu.gif",
            "sequencer": "006639F508B96A7690"
        },
        "request-id": "11K82SGF9X2HBEKD",
        "requester": "536254204126",
        "source-ip-address": "15.248.3.15",
        "reason": "DeleteObject",
        "deletion-type": "Permanently Deleted"
    }
}

[heitorlessa]

checking shortly, just updating the main issue

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
15.7% Duplication on New Code

See analysis details on SonarCloud

[boring-cyborg]

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!