Bug: S3Model not compatible with ObjectRemoved events #1637

barreeeiroo · 2022-10-23T15:50:31Z

Expected Behaviour

After deleting an object from S3, if a Lambda catches this event, they should be able to parse it. This event name from S3 is ObjectDeleted:*.

Current Behaviour

A ValidationError is thrown because of the missing size and eTag attributes.

Records -> 0 -> s3 -> object -> size
  field required (type=value_error.missing)
Records -> 0 -> s3 -> object -> eTag
  field required (type=value_error.missing)

The ObjectDeleted event does not include those attributes.

Code snippet

parse(model=S3Model, event=json.loads(s3_message))

Payload Example: #1637 (comment)

Possible Solution

Update S3Object so that both size and eTag are optional:

class S3Object(BaseModel):
    key: str
    size: Optional[NonNegativeFloat]
    eTag: Optional[str]
    sequencer: str
    versionId: Optional[str]

Steps to Reproduce

Create a Lambda function and an S3 bucket
Trigger the Lambda function through S3 events, and enable the ObjectDeleted type
Delete an object from S3
S3Model will raise a ValidationError

AWS Lambda Powertools for Python version

latest

AWS Lambda function runtime

3.8

Packaging format used

PyPi

Debugging logs

No response

The text was updated successfully, but these errors were encountered:

boring-cyborg · 2022-10-23T15:50:33Z

Thanks for opening your first issue here! We'll come back to you as soon as we can.
In the meantime, check out the #python channel on our AWS Lambda Powertools Discord: Invite link

heitorlessa · 2022-10-23T17:48:03Z

@barreeeiroo thank you for flagging this - would you be able to send a quick fix PR marking them as Optional, please?

We're launching V2 tomorrow morning and we could include the fix in.

Otherwise we can make a V1 patch release afterwards.

Thank you!!

Fixes ObjectRemoved API event, as they don't include these attributes. aws-powertools#1637 Signed-off-by: Diego Barreiro <[email protected]>

barreeeiroo · 2022-10-23T18:07:11Z

@heitorlessa done, the PR should be available now: PR#1638. I couldn't really test the change right now, but if strictly needed I can fully test it tomorrow.

heitorlessa · 2022-10-23T18:11:48Z

@heitorlessa done, the PR should be available now: PR#1638. I couldn't really test the change right now, but if strictly needed I can fully test it tomorrow.

Thank you!! Do you happen to have a copy of the delete event?

If you do, please add to the issue. We can complete the tests before releasing it tomorrow (~10am Amsterdam time)

barreeeiroo · 2022-10-23T18:22:55Z

Sure, here it is:

{
  "eventVersion": "2.1",
  "eventSource": "aws:s3",
  "awsRegion": "us-west-2",
  "eventTime": "2022-10-23T15:37:51.707Z",
  "eventName": "ObjectRemoved:Delete",
  "userIdentity": {
    "principalId": "[REDACTED]"
  },
  "requestParameters": {
    "sourceIPAddress": "[REDACTED]"
  },
  "responseElements": {
    "x-amz-request-id": "[REDACTED]",
    "x-amz-id-2": "[REDACTED]"
  },
  "s3": {
    "s3SchemaVersion": "1.0",
    "configurationId": "[REDACTED]",
    "bucket": {
      "name": "[REDACTED]",
      "ownerIdentity": {
        "principalId": "[REDACTED]"
      },
      "arn": "[REDACTED]"
    },
    "object": {
      "key": "[REDACTED]",
      "sequencer": "[REDACTED]"
    }
  }
}

Seems like the rest of the payload is correct.

ran-isenberg · 2022-10-23T18:44:25Z

Thanks @barreeeiroo for finding and solving this!

@heitorlessa maybe we can add a validator that checks that they are indeed optional only if the event name is ObjectRemoved:Delete? For other S3object events, these fields are not optional, right?

ran-isenberg · 2022-10-23T18:52:38Z

@heitorlessa i think it's also a bug for the S3Object dataclass.

barreeeiroo · 2022-10-23T18:57:37Z

No problem @ran-isenberg! I also thought about adding a conditional validator like this:

class S3Object(BaseModel):
    key: str
    size: Optional[NonNegativeFloat] # This needs to still be defined as Optional
    eTag: Optional[str] # This needs to still be defined as Optional
    sequencer: str
    versionId: Optional[str]

class S3RecordModel(BaseModel):
    eventVersion: str
    eventSource: Literal["aws:s3"]
    awsRegion: str
    eventTime: datetime
    eventName: str
    userIdentity: S3Identity
    requestParameters: S3RequestParameters
    responseElements: S3ResponseElements
    s3: S3Message
    glacierEventData: Optional[S3EventRecordGlacierEventData]

    @root_validator
    def validate_s3_object(cls, values):
        event_name = values.get('eventName')
        s3_object = values.get('s3').get('object')
        if 'ObjectRemoved' not in event_name:
            if not s3_object.get('size') or not s3_object.get('eTag'):
                raise ValueError('S3Object.size and S3Object.eTag are required for non-ObjectRemoved events')
        return values

But wasn't really sure about it as I couldn't find any AWS official documentation stating which attributes are required for each event type. Is there any document which lists all the attributes provided for each event type from S3?

heitorlessa · 2022-10-23T18:58:01Z

For now, let’s make them optional - it’s a two-way door (we can always add the validators back as it’s a non-breaking change). We need a better strategy for Parser. It’s starting to break customers more frequently. Maybe a rule for now could be to only add any additional Parser Models until we have either E2E or regular runs on its various configurations — e.g., MSK takes 30m to setup which isn’t suitable for 30m as it adds up quickly. @ran - if you’ve got ideas on how we can make Parser more accurate, despite not having official schemas from Lambda (actual root cause), could you kick off a RFC on this, please? Thanks a lot you both, truly.

…

On Sun, 23 Oct 2022 at 20:44, Ran Isenberg ***@***.***> wrote: Thanks @barreeeiroo <https://github.com/barreeeiroo> for finding and solving this! @heitorlessa <https://github.com/heitorlessa> maybe we can add a validator that checks that they are indeed optional only if the event name is ObjectRemoved:Delete? For other S3object events, these fields are not optional, right? — Reply to this email directly, view it on GitHub <#1637 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZPQBH2YPWWK5ZAVUXEZCLWEWBRHANCNFSM6AAAAAARMK6GCQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

ran-isenberg · 2022-10-23T19:00:06Z

@heitorlessa it's not just the parser in this case, also regular data classes are broken too.

ran-isenberg · 2022-10-23T19:00:38Z

No problem @ran-isenberg! I also thought about adding a conditional validator like this:

class S3Object(BaseModel):
    key: str
    size: Optional[NonNegativeFloat] # This needs to still be defined as Optional
    eTag: Optional[str] # This needs to still be defined as Optional
    sequencer: str
    versionId: Optional[str]

class S3RecordModel(BaseModel):
    eventVersion: str
    eventSource: Literal["aws:s3"]
    awsRegion: str
    eventTime: datetime
    eventName: str
    userIdentity: S3Identity
    requestParameters: S3RequestParameters
    responseElements: S3ResponseElements
    s3: S3Message
    glacierEventData: Optional[S3EventRecordGlacierEventData]

    @root_validator
    def validate_s3_object(cls, values):
        event_name = values.get('eventName')
        s3_object = values.get('s3').get('object')
        if 'ObjectRemoved' not in event_name:
            if not s3_object.get('size') or not s3_object.get('eTag'):
                raise ValueError('S3Object.size and S3Object.eTag are required for non-ObjectRemoved events')
        return values

But wasn't really sure about it as I couldn't find any AWS official documentation stating which attributes are required for each event type. Is there any document which lists all the attributes provided for each event type from S3?

I didnt find any official docs, otherwise i'd have the even name as a literal of closed event types list :(

heitorlessa · 2022-10-23T19:02:04Z

Yes, both. The difference is that Parser breaks at runtime, while data classes varies on how the property is retrieved (some has sentinel values).

…

On Sun, 23 Oct 2022 at 21:00, Ran Isenberg ***@***.***> wrote: @heitorlessa <https://github.com/heitorlessa> it's not just the parser in this case, also regular data classes are broken too. — Reply to this email directly, view it on GitHub <#1637 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZPQBFNEMRMWYJAPH3ZXTDWEWDMBANCNFSM6AAAAAARMK6GCQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

ran-isenberg · 2022-10-23T19:02:19Z

@barreeeiroo yeah that makes sense! nice one.
I'll open a PR with tests in a few min.

heitorlessa · 2022-10-23T19:06:51Z

No problem @ran-isenberg! I also thought about adding a conditional validator like this:
class S3Object(BaseModel):
key: str
size: Optional[NonNegativeFloat] # This needs to still be defined as Optional
eTag: Optional[str] # This needs to still be defined as Optional
sequencer: str
versionId: Optional[str]
class S3RecordModel(BaseModel):
eventVersion: str
eventSource: Literal["aws:s3"]
awsRegion: str
eventTime: datetime
eventName: str
userIdentity: S3Identity
requestParameters: S3RequestParameters
responseElements: S3ResponseElements
s3: S3Message
glacierEventData: Optional[S3EventRecordGlacierEventData]
@root_validator
def validate_s3_object(cls, values):
    event_name = values.get('eventName')
    s3_object = values.get('s3').get('object')
    if 'ObjectRemoved' not in event_name:
        if not s3_object.get('size') or not s3_object.get('eTag'):
            raise ValueError('S3Object.size and S3Object.eTag are required for non-ObjectRemoved events')
    return values
But wasn't really sure about it as I couldn't find any AWS official documentation stating which attributes are required for each event type. Is there any document which lists all the attributes provided for each event type from S3?

I didnt find any official docs, otherwise i'd have the even name as a literal of closed event types list :(

+1. It's what I shared earlier - the root cause is not having an event schema. We derived based on events we trigger - our models can only be as good as our tests and "contracts" being respected by their original producers.

So far we've noticed a few variations that end up making one or more properties to be either omitted or have different casing:

Event Source default settings
Event Source custom settings (FIFO, different action like Deleted here)
Event Source connected with another event source (SNS->SQS->Lambda is slightly different than SNS->Lambda)

heitorlessa · 2022-10-23T19:47:43Z

Sorry it’s Sunday, so we’ll look into tomorrow after V2 launch. I’m not comfortable with the validators - I can’t be confident this won’t break in another unexpected way. Diego had a PR but it’s missing test. Let’s be mindful and foster first-time contributors, please. I’ll wake up earlier as we’ve had too many last minute papercuts for the launch last Friday - we’ve got to launch tomorrow morning the earliest and need to reduce deltas. Thanks everyone for the patience and more so for messaging this late on a Sunday. Have a great rest and we crack on tomorrow after launch

…

On Sun, 23 Oct 2022 at 21:08, Ran Isenberg ***@***.***> wrote: after installing it manually it now says: "E ImportError: cannot import name 'aws_apigatewayv2_alpha' from 'aws_cdk' (/aws-lambda-powertools-python/.venv/lib/python3.9/site-packages/aws_cdk/ *init*.py)" — Reply to this email directly, view it on GitHub <#1637 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZPQBDFWQ5ADWBDZPSFQQDWEWEMPANCNFSM6AAAAAARMK6GCQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

ran-isenberg · 2022-10-23T19:51:02Z

@heitorlessa of course. It's 11PM here, didnt expect you to merge it :)
I didnt know how to add Diego's PR to mine, but you can merge his code and then mine so his commit is added.

The validator does not break older tests and it's very specific to the delete event.

barreeeiroo · 2022-10-23T20:29:46Z

For reference, providing this link to this AWS page: https://docs.aws.amazon.com/filegateway/latest/files3/refresh-cache.html
It shows an example payload in Step 15 for ObjectRemoved where they show the object having only eTag and key attributes.

Fixes ObjectRemoved API event, as they don't include these attributes. aws-powertools#1637 Signed-off-by: Diego Barreiro <[email protected]>

heitorlessa · 2022-10-24T06:24:53Z

Releasing v2.0.0a4 pre-release: https://github.com/awslabs/aws-lambda-powertools-python/actions/runs/3310473745

We should make the official launch at 10am CET (Amsterdam time)

heitorlessa · 2022-10-24T11:05:14Z

Released in V2 now: https://github.com/awslabs/aws-lambda-powertools-python/releases/tag/v2.0.0

github-actions · 2022-10-24T11:05:34Z

⚠️COMMENT VISIBILITY WARNING⚠️

This issue is now closed. Please be mindful that future comments are hard for our team to see.

If you need more assistance, please either tag a team member or open a new issue that references this one.

If you wish to keep having a conversation with other community members under this issue feel free to do so.

barreeeiroo added bug Something isn't working triage Pending triage from maintainers labels Oct 23, 2022

barreeeiroo changed the title ~~Bug: S3Model not compatible with ObjectRemoved eventName~~ Bug: S3Model not compatible with ObjectRemoved events Oct 23, 2022

heitorlessa added area/parser and removed triage Pending triage from maintainers labels Oct 23, 2022

barreeeiroo mentioned this issue Oct 23, 2022

fix(parser): S3Model Object Deleted omits size and eTag attr #1638

Merged

5 tasks

ran-isenberg mentioned this issue Oct 23, 2022

fix: parser S3Model not compatible with ObjectRemoved events #1639

Closed

heitorlessa added the pending-release Fix or implementation already in dev waiting to be released label Oct 24, 2022

heitorlessa removed the pending-release Fix or implementation already in dev waiting to be released label Oct 24, 2022

heitorlessa closed this as completed Oct 24, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug: S3Model not compatible with ObjectRemoved events #1637

Bug: S3Model not compatible with ObjectRemoved events #1637

barreeeiroo commented Oct 23, 2022 •

edited

Loading

boring-cyborg bot commented Oct 23, 2022

heitorlessa commented Oct 23, 2022

barreeeiroo commented Oct 23, 2022

heitorlessa commented Oct 23, 2022

barreeeiroo commented Oct 23, 2022

ran-isenberg commented Oct 23, 2022

ran-isenberg commented Oct 23, 2022

barreeeiroo commented Oct 23, 2022

heitorlessa commented Oct 23, 2022 via email

ran-isenberg commented Oct 23, 2022

ran-isenberg commented Oct 23, 2022

heitorlessa commented Oct 23, 2022 via email

ran-isenberg commented Oct 23, 2022

heitorlessa commented Oct 23, 2022

heitorlessa commented Oct 23, 2022 via email

ran-isenberg commented Oct 23, 2022 •

edited

Loading

barreeeiroo commented Oct 23, 2022

heitorlessa commented Oct 24, 2022

heitorlessa commented Oct 24, 2022

github-actions bot commented Oct 24, 2022

Bug: S3Model not compatible with ObjectRemoved events #1637

Bug: S3Model not compatible with ObjectRemoved events #1637

Comments

barreeeiroo commented Oct 23, 2022 • edited Loading

Expected Behaviour

Current Behaviour

Code snippet

Possible Solution

Steps to Reproduce

AWS Lambda Powertools for Python version

AWS Lambda function runtime

Packaging format used

Debugging logs

boring-cyborg bot commented Oct 23, 2022

heitorlessa commented Oct 23, 2022

barreeeiroo commented Oct 23, 2022

heitorlessa commented Oct 23, 2022

barreeeiroo commented Oct 23, 2022

ran-isenberg commented Oct 23, 2022

ran-isenberg commented Oct 23, 2022

barreeeiroo commented Oct 23, 2022

heitorlessa commented Oct 23, 2022 via email

ran-isenberg commented Oct 23, 2022

ran-isenberg commented Oct 23, 2022

heitorlessa commented Oct 23, 2022 via email

ran-isenberg commented Oct 23, 2022

heitorlessa commented Oct 23, 2022

heitorlessa commented Oct 23, 2022 via email

ran-isenberg commented Oct 23, 2022 • edited Loading

barreeeiroo commented Oct 23, 2022

heitorlessa commented Oct 24, 2022

heitorlessa commented Oct 24, 2022

github-actions bot commented Oct 24, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

barreeeiroo commented Oct 23, 2022 •

edited

Loading

ran-isenberg commented Oct 23, 2022 •

edited

Loading