Bug: S3Model not compatible with ObjectRemoved events

[barreeeiroo]

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

1. Create a Lambda function and an S3 bucket
2. Trigger the Lambda function through S3 events, and enable the ObjectDeleted type
3. Delete an object from S3
4. 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

[boring-cyborg]

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]

@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!!

[barreeeiroo]

@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]

@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]

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]

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]

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

[barreeeiroo]

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]

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: ***@***.***>