Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for AppSync events. #187

Merged
merged 6 commits into from
Mar 2, 2021
Merged

Conversation

DwayneCoussement
Copy link
Contributor

Add support for AppSync.

Motivation:

AppSync is a common AWS Service, so it might be nice to support OOB support.

Modifications:

This is a purely additive change to support AWS AppSync. An AppSync.Request and AppSync.Response was added.

Result:

You'll be able to listen to AppSync requests as follows:

let myHandler = { (context: Lambda.Context, event: AppSync.Request, callback: (Result<AppSync.Response<MyCodableModel>, Error>) -> Void) in

}
let myHandler = { (context: Lambda.Context, event: AppSync.Request, callback: (Result<JSONStringResponse, Error>) -> Void) in

}

@swift-server-bot
Copy link

Can one of the admins verify this patch?

3 similar comments
@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

@swift-server-bot
Copy link

Can one of the admins verify this patch?

Sources/AWSLambdaEvents/AppSync.swift Outdated Show resolved Hide resolved
Tests/AWSLambdaEventsTests/AppSyncTests.swift Outdated Show resolved Hide resolved
Tests/AWSLambdaEventsTests/AppSyncTests.swift Outdated Show resolved Hide resolved
Tests/AWSLambdaEventsTests/AppSyncTests.swift Outdated Show resolved Hide resolved
Tests/AWSLambdaEventsTests/AppSyncTests.swift Outdated Show resolved Hide resolved
Tests/AWSLambdaEventsTests/AppSyncTests.swift Outdated Show resolved Hide resolved
Tests/AWSLambdaEventsTests/AppSyncTests.swift Outdated Show resolved Hide resolved
Tests/AWSLambdaEventsTests/AppSyncTests.swift Outdated Show resolved Hide resolved
Tests/AWSLambdaEventsTests/AppSyncTests.swift Outdated Show resolved Hide resolved
Tests/AWSLambdaEventsTests/AppSyncTests.swift Outdated Show resolved Hide resolved
Co-authored-by: Laurent Gaches <laurent@binimo.com>
Copy link
Member

@fabianfett fabianfett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @DwayneCoussement for the pr and thanks @lgaches for the first round of reviews!

@DwayneCoussement would you mind running swiftformat . once on your code (please make sure you are in the project directory before you run it)? This should make the soundness check succeed.

Further I left a small number of comments. Do you know of any documentation of the AppSync Lambda Event types that we can link in the top of the file? Comparable to what we have done here:

// https://docs.aws.amazon.com/lambda/latest/dg/with-ddb.html
public struct DynamoDB {
public struct Event: Decodable {
public let records: [EventRecord]

Sources/AWSLambdaEvents/AppSync.swift Outdated Show resolved Hide resolved
Sources/AWSLambdaEvents/AppSync.swift Outdated Show resolved Hide resolved
Sources/AWSLambdaEvents/AppSync.swift Outdated Show resolved Hide resolved
@DwayneCoussement
Copy link
Contributor Author

Thanks @DwayneCoussement for the pr and thanks @lgaches for the first round of reviews!

@DwayneCoussement would you mind running swiftformat . once on your code (please make sure you are in the project directory before you run it)? This should make the soundness check succeed.

Further I left a small number of comments. Do you know of any documentation of the AppSync Lambda Event types that we can link in the top of the file? Comparable to what we have done here:

// https://docs.aws.amazon.com/lambda/latest/dg/with-ddb.html
public struct DynamoDB {
public struct Event: Decodable {
public let records: [EventRecord]

@lgaches also thanks for the suggestions and first code review. @fabianfett, thank you for the second review, I've applied the requested changes as well! :)

@fabianfett
Copy link
Member

@swift-server-bot test this please

2 similar comments
@tomerd
Copy link
Contributor

tomerd commented Feb 26, 2021

@swift-server-bot test this please

@tomerd
Copy link
Contributor

tomerd commented Feb 26, 2021

@swift-server-bot test this please

}

public let identity: Identity?
public struct Identity: Codable {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation in https://docs.aws.amazon.com/appsync/latest/devguide/resolver-context-reference.html#aws-appsync-resolver-context-reference-identity says:

Identity

The identity section contains information about the caller. The shape of this section depends on the authorization type of your AWS AppSync API.

For more information about this section and how it can be used, see Security.

API_KEY authorization
The identity field isn’t populated.

AWS_IAM authorization
The identity has the following shape:

{
    "accountId" : "string",
    "cognitoIdentityPoolId" : "string",
    "cognitoIdentityId" : "string",
    "sourceIp" : ["string"],
    "username" : "string", // IAM user principal
    "userArn" : "string",
    "cognitoIdentityAuthType" : "string", // authenticated/unauthenticated based on the identity type
    "cognitoIdentityAuthProvider" : "string" // the auth provider that was used to obtain the credentials
}

AMAZON_COGNITO_USER_POOLS authorization
The identity has the following shape:

{
    "sub" : "uuid",
    "issuer" : "string",
    "username" : "string"
    "claims" : { ... },
    "sourceIp" : ["x.x.x.x"],
    "defaultAuthStrategy" : "string"
}

Should we model this differently?

Copy link
Contributor Author

@DwayneCoussement DwayneCoussement Feb 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good point! I've updated the model slightly here to take this all into account. Some updates were needed for AWS_IAM authorization. Please see the latest commit

@fabianfett
Copy link
Member

@swift-server-bot test this please

1 similar comment
@tomerd
Copy link
Contributor

tomerd commented Mar 2, 2021

@swift-server-bot test this please

Copy link
Contributor

@tomerd tomerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm @fabianfett ?

@fabianfett
Copy link
Member

LGTM

@tomerd tomerd merged commit e33e4af into swift-server:main Mar 2, 2021
@DwayneCoussement DwayneCoussement deleted the appsync branch March 3, 2021 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants