iliana / rust-crowbar

Wrapper to simplify writing AWS Lambda functions in Rust (using the Python execution environment)
https://docs.rs/crowbar
Apache License 2.0
197 stars 16 forks source link

[WIP] Add Data Fixtures for Lambda Event Types #31

Open naftulikay opened 6 years ago

naftulikay commented 6 years ago

Tests and documentation forthcoming. I am using these data structures for my serverless event bus in Rust using Crowbar.

My lambda handler looks something like this:

lambda!(|event, _context| {
    // dispatch event based on its type
    match serde_json::from_value::<Event>(event) {
        Ok(Event::Auth(event)) => Ok(to_value(&service::auth::route(&context, &event)).unwrap()),
        Ok(Event::CloudWatch(event)) => Ok(service::cloudwatch::route(&context, &event)),
        Ok(Event::Http(event)) => Ok(to_value(&service::http::route(&context, &event)).unwrap()),
        Ok(Event::Records(records)) => Ok(service::multi::route_all(&context, records.entries)),
        Ok(Event::Unknown(event)) => Ok(service::unknown::route(&context, &event)),
        Err(_) => {
            error!("Unable to convert event.");
            Ok(json!({ "statusCode": 400 ,"message": "Unable to convert event." }))
        }
    }
});

Thus, I do routing based on event type. For example, SNS:

pub fn route(context: &Context, record: &sns::Record) {
    info!("Received SNS Event: {}", record.event.message_id);

    match from_str(&record.event.message) {
        Ok(SnsEventKind::Ses(message)) => service::ses::receive(&context, &message),
        Ok(SnsEventKind::Unknown(value)) => warn!("Unknown JSON Event: {}", to_string_pretty(&value).unwrap()),
        Err(_) => error!("Unknown non-JSON event: {}", record.event.message),
    };
}

My multi-event router:

/// Dispatch multiple events.
pub fn route_all(context: &Context, events: Vec<Record>) -> Value {
    for event in events {
        match event {
            Record::S3(event_record) => s3::route(&context, &event_record),
            Record::Ses(event_record) => {
                // if it's an SES request/response, a response may be required, so only process one
                // of them if the type is RequestResponse
                match event_record.event.receipt.action {
                    Action::Lambda(ref action) if action.invocation_type == RequestResponse => {
                        return to_value(ses::filter(&context, &event_record)).unwrap();
                    },
                    _ => ses::route(&context, &event_record.event),
                };
            },
            Record::Sns(event_record) => sns::route(&context, &event_record),
            Record::Unknown(value) => {
                error!("Unknown event record type: {}", to_string_pretty(&value).unwrap())
            }
        };
    }

    Value::Null
}

This dispatching occurs in around 400-500 microseconds and works extremely well to make things type safe.

Obviously there's a lot here and it's not done. If @ilianaw or others have any input, RFC is open to comments :smiley:

iliana commented 6 years ago

I like this. :)

Can you add what has been done and what remains as checkboxes to the pull request description so we can see the path to this leaving a WIP state?

naftulikay commented 6 years ago

:tada: so happy that you're welcoming this PR. I will create a separate issue and track things there. There are so many different data types to support, so this PR will maybe only cover a subset with future on the way, hope that's okay.

iliana commented 6 years ago

Yeah, I don't have an issue with incremental development over time.

I'm hoping I'll have a chance to do a good read through of all of this soon.

naftulikay commented 6 years ago

@ilianaw can you please go turn on Travis CI for this project? I have updated with a build badge and Travis configuration for building and testing.

EDIT: Actually, I will submit Travis support in a separate PR.

naftulikay commented 6 years ago

Latest adds support for CloudWatch AutoScaling events.

softprops commented 6 years ago

I'd love to see this branch ship fast with additional followups to add other events types. Commented with extra thoughts on the checkbox ticket

naftulikay commented 6 years ago

I will start breaking this apart into a PR per service supported, I think that's probably the most prudent course of action.

One thing that is probably a really good idea is to find a way to efficiently nest CloudWatch style events and deserialize them more efficiently than I'm currently doing.

softprops commented 6 years ago

One thing that is probably a really good idea is to find a way to efficiently nest CloudWatch style events and deserialize them more efficiently than I'm currently doing.

Can you link to an example in this pull or sketch out in a comment what you mean?

iliana commented 6 years ago

Overall I'm happy with this, and thanks to @softprops for the additional review.

I'm fine with continuing with this PR, or breaking it apart as a PR per service (the latter would be great as its easier for others to help write the structs out).

I have two thoughts:

softprops commented 6 years ago

What happens when AWS inevitably adds another field to these objects? Does serde correctly ignore extra fields?

Yes. This used to not be true but is now a default of serde.

naftulikay commented 6 years ago

I apologize for my lateness, just getting back from vacation. I will get caught up this week.

iliana commented 6 years ago

No worries! ^_^;

naftulikay commented 6 years ago

TL;DR the events received from Lambda can come via a few sources:

Therefore, our enums for representing all kinds of incoming events should basically include these three, and then users can disambiguate between the implementations (ie cloudwatch::BatchEvent, cloudwatch::ApiCallEvent, apigateway::Request, apigateway::AuthRequest, etc.)

I will probably break this PR down into tiny pieces and make one PR for each service to add. I wholeheartedly agree with removing Option<T> wherever possible and using the default for things like Vec<T> and BTreeMap<K, V>.

softprops commented 6 years ago

Is there any way to help move this forward? I'd love to help