feat(appsync): add Elasticsearch data source - fixes #6063

[bartcallant]

This is a PR which adds Elasticsearch as a data source for AppSync.

I mentioned in the #6063 there are several default request/response templates for AppSync. They are all included except for the Get all documents within a 20 mile radius one. Not sure if that one should be included, just let me know. Then I can add it as well.

Also updated the integration test to include an Elasticsearch data resolved.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

AWS CodeBuild CI Report

• Result: FAILED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[aws-cdk-automation]

AWS CodeBuild CI Report

• Result: FAILED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[aws-cdk-automation]

AWS CodeBuild CI Report

• Result: FAILED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[aws-cdk-automation]

AWS CodeBuild CI Report

• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[aws-cdk-automation]

AWS CodeBuild CI Report

• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[aws-cdk-automation]

AWS CodeBuild CI Report

• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[MrArnoldPalmer]

@bartcallant I'm tempted to hold off on merging this until there is an Elasticsearch L2 construct. That way addElasticsearchDataSource can accept the es "Index" L2 as an argument which is the ideal interface and what CDK overall strives for. Otherwise this doesn't match the other data source apis and will either require breaking changes or a continued difference to other data sources.

That being said, App Sync constructs are still experimental and a breaking change in the future isn't the worst thing in the world. @shivlaks @bartcallant what do you think?

[skinny85]

@bartcallant I'm tempted to hold off on merging this until there is an Elasticsearch L2 construct. That way addElasticsearchDataSource can accept the es "Index" L2 as an argument which is the ideal interface and what CDK overall strives for. Otherwise this doesn't match the other data source apis and will either require breaking changes or a continued difference to other data sources.

That being said, App Sync constructs are still experimental and a breaking change in the future isn't the worst thing in the world. @shivlaks @bartcallant what do you think?

How about creating, in the @aws-cdk/aws-elasticsearch package, a super-simple IIndex interface with indexName and indexArn properties, and a super-simple Index class, with just a fromIndexName and/or fromIndexArn method, and use that in this package? We do something similar with CodeDeploy ECS Deployment Groups, which are not supported in CloudFormation yet.

[MrArnoldPalmer]

I'm definitely on board with that considering we are doing the same elsewhere. Thanks @skinny85 😄

[MrArnoldPalmer]

@bartcallant let us know if the above makes sense and you want to move forward with this.

[shivlaks]

I'm definitely on board with that considering we are doing the same elsewhere. Thanks @skinny85 😄

I was also unaware that we do this elsewhere. It sounds like a nice compromise in this situation.

[bartcallant]

You've got a point with the consistency between the adding of datasources.

Is there a timeline when a L2 construct could be introduced? (I could not find any issue/pr)
Waiting util that would be introduces might still take w while.
The idea of the simple L2 constructs are something I'm definitely a fan of.

Personally I think those should be done through a separate PR, no? Should there be an issue first where we can correctly discuss the implementation details?

[skinny85]

Personally I think those should be done through a separate PR, no?

I think it's completely fine to introduce the simple L2 in the same PR that it will be used in. Makes a lot of sense, actually (PRs generally should be atomic and self-contained).

[MrArnoldPalmer]

@bartcallant If we wanted to create a fully fleshed out L2 for ES I'd agree. If we are only creating a minimal interface for an Index to pass around to other constructs (this being the first) I think we can do it in this PR.

@skinny85 am I right in the understanding that the IIndex interface and Index class + fromIndex(Name|Arn) methods need not be a fully functional L2? The ones you reference in CodeDeploy are just shells but that is because Cfn doesn't have matching resources right?

[skinny85]

@skinny85 am I right in the understanding that the IIndex interface and Index class + fromIndex(Name|Arn) methods need not be a fully functional L2?

Yes, absolutely. They just need to be classes / interfaces that you can use for integrations. When it's time to build an entire L2 for ElasticSearch, you'll simply fill the existing shells with more functionality.

[bartcallant]

I will try to implement this tonight or tomorrow evening.

[MrArnoldPalmer]

@bartcallant gonna close this for now. Reopen when you're ready. If we or someone else happens to pick this up before that I'll reach out for review feedback.