feat(appmesh): add support for AWS AppMesh #2299

yandy-r · 2019-04-15T15:22:06Z

This PR allows one to work with AWS AppMesh at higher level L2 construct without needing to dive into the CFN layer for most use case.

This allows one to be able to.

Create a service mesh
Create virtual services and add them too the mesh
Add virtual nodes to the mesh
Add virtual routers to the mesh
Add virtual routes pointing to these nodes
- Should be able to add HTTP and TCP routes
Know which mesh a route(r) is attached to
Know which router a Node is attached to
Know which backends nodes talk to

Fixes #2297

Pull Request Checklist

Testing
- Unit test added (prefer not to modify an existing test, otherwise, it's probably a breaking change)
- CLI change?: coordinate update of integration tests with team
- cdk-init template change?: coordinated update of integration tests with team
Docs
- jsdocs: All public APIs documented
- README: README and/or documentation topic updated
Title and Description
- Change type: title prefixed with fix, feat will appear in changelog
- Title: use lower-case and doesn't end with a period
- Breaking?: last paragraph: "BREAKING CHANGE: <describe what changed + link for details>"
- Issues: Indicate issues fixed via: "Fixes #xxx" or "Closes #xxx"
Sensitive Modules (requires 2 PR approvers)
- IAM Policy Document (in @aws-cdk/aws-iam)
- EC2 Security Groups and ACLs (in @aws-cdk/aws-ec2)
- Grant APIs (only if not based on official documentation with a reference)

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

This was being worked on in separate repo, being merged here

There's no need to pass the entire mesh as a reference around. Only the meshName is necssary for most things as it is unique and what the api takes for reference as well.

Only the namespaceName was required, making things more complex than necessary. This was changed in favor of just passing down the namespace name.

adding a virtual router, there was no way to name this router, now there is with this update

Also change name and type fo properties to better align with official API

This adds the ability for the service to point to a VirtualRouter or a VirtualNode for single point of communication

The readme now has examples of mesh actions.

meshArn as import prop is unnecessary and redundant, as only the meshName is important.

Create a method to allow the addition of a sinble backend at a time to the VirtualNodes created

Stale

rix0rrr · 2019-08-09T13:25:55Z

Okay, I think I've got the PR in good shape technically, updated to latest AWSCL standards.

Have not really reviewed for content and usability, since I don't know anything about AppMesh. I'm good to ship it and iterate later since it will start out @experimental anyway, but maybe someone with some knowledge on the domain can chip in whether this is fit for purpose? @SoManyHs maybe?

If no further comments, removing the do-not-merge label should trigger the automerge.

SoManyHs · 2019-08-09T18:02:00Z

@rix0rrr Yes, I'll take a look.

rix0rrr · 2019-08-19T09:20:21Z

@SoManyHs did you? :)

mergify · 2019-08-20T14:02:14Z

Pull Request Checklist

Testing

Unit test added (prefer to add a new test rather than modify existing tests)
CLI change? Re-run/add CLI integration tests

Documentation

Inline docs: make sure all public APIs are documented (copy & paste from official AWS docs)
README: update module README
Design: for significant features, follow the design process

Title uses the format type(scope): text

Type: fix, feat, refactor go into CHANGELOG, chore is hidden
Scope: name of the module without the aws- or cdk- prefix or postfix (e.g. s3 instead of aws-s3-deployment)
Style: use all lower-case, do not end with a period

Description

Rationale: describe rationale of change and approach taken
Issues: Indicate issues fixed via: fixes #xxx or closes #xxx
Breaking?: last paragraph: BREAKING CHANGE: <describe what changed + link for details>

Sensitive Modules (requires 2 PR approvers)

IAM document library (in @aws-cdk/aws-iam)
EC2 security groups and ACLs (in @aws-cdk/aws-ec2)
Grant APIs (if not based on official documentation with a reference)

SoManyHs

Great work! A few questions/nits. Thanks for contributing!

packages/@aws-cdk/aws-appmesh/README.md

SoManyHs · 2019-08-21T19:42:53Z

packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts

+        backends: this.backends,
+        listeners: this.listeners,
+        serviceDiscovery: {
+          dns: {


Are we always expecting this to be DNS? Is there a way to allow AWSCloudMapServiceDiscovery as well?

You're right, there should be.

packages/@aws-cdk/aws-appmesh/lib/virtual-service.ts

packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts

SoManyHs · 2019-08-21T20:22:43Z

packages/@aws-cdk/aws-appmesh/test/test.route.ts

+    const stack2 = new cdk.Stack();
+    appmesh.Route.fromRouteName(stack2, 'imported-route', mesh.meshName, router.virtualRouterName, route.routeName);
+
+    // Nothing to do with imported route yet


I'm confused -- what are we expecting to assert?

A lack of exceptions I suppose :)

yandy-r · 2019-08-22T12:51:38Z

Thank you all for picking up the slack, it was going to be difficult for me to get this here with everything going on. Thanks again!

… supported, fix tests

mergify · 2019-08-22T15:30:26Z

Codebuild (Continuous Integration) build failed for current commits. Please check log and resolve before PR is merged.

Yandy Ramirez added 30 commits April 13, 2019 23:42

initial add of appmesh construct

31fc983

This was being worked on in separate repo, being merged here

change pssing of mesh to just meshName

a97d7ff

There's no need to pass the entire mesh as a reference around. Only the meshName is necssary for most things as it is unique and what the api takes for reference as well.

change needing reference to entire INamespace

2751c78

Only the namespaceName was required, making things more complex than necessary. This was changed in favor of just passing down the namespace name.

remove unnecessary interface

c592be3

fix: remove unnecessary conditional check

40251c5

fix: use id instead of hostname for nodename

15aa098

doc: finish documenting mesh class

d518594

doc: begin documenting virtual service class

336efef

fix: use only virtual router name

e6742e0

doc: update mesh documentation

a1140a5

doc: document virtual service class

e1e012c

doc: document virtual node class

c067fe5

doc: update mesh documentation with DocumentThis

5fe3902

fix: update to use virtualRouterName

d9bc64f

adding a virtual router, there was no way to name this router, now there is with this update

doc: document router class

ae073ee

feat(router): create ability to add >1 routes

7ec5e1f

fix: remove redundant route interface

0b78eec

doc: begin documenting VirtualRoute

7dd83c9

Also change name and type fo properties to better align with official API

fix: add virtual route name identifier

d3876b5

doc: finish documenting the virtual route class

3e040df

feat: change import style to require

b66e213

feat(virtualService): add ability for router or node provider

26c8332

This adds the ability for the service to point to a VirtualRouter or a VirtualNode for single point of communication

feat: update readme as documentation

0159c42

The readme now has examples of mesh actions.

fix: remove meshArn as import prop

5001c7f

meshArn as import prop is unnecessary and redundant, as only the meshName is important.

fix: correct a typo in mesh filter spec

c631058

fix: add error checking for empty portMappings

3d3a6fd

feat(node): create ability to add single backend

a61b1ee

Create a method to allow the addition of a sinble backend at a time to the VirtualNodes created

feat: add check for empty array properties

dd5e802

feat: wrote unit tests for mesh

8cb4cf1

feat: wrote unit tests for virtual node

5ebbc91

yandy-r requested a review from SoManyHs as a code owner August 8, 2019 10:31

rix0rrr added 2 commits August 8, 2019 13:09

Update names and import/export conventions

729d722

Make awslint happy

8755ee4

rix0rrr added the large-review label Aug 9, 2019

rix0rrr added 2 commits August 9, 2019 12:18

Physical names and logical IDs

d4f69a0

Merge remote-tracking branch 'origin/master' into pr/IPyandy/2299

3ce1ec3

rix0rrr requested a review from RomainMuller August 9, 2019 10:34

rix0rrr added 2 commits August 9, 2019 13:51

Remove deprecated files

d389faa

Removing another obsoleted package

e3a6226

rix0rrr added the pr/do-not-merge This PR should not be merged at this time. label Aug 9, 2019

rix0rrr previously approved these changes Aug 9, 2019

View reviewed changes

rix0rrr changed the title ~~feat(appmesh): Add L2 Construct for AWS AppMesh~~ feat(appmesh): add support for AWS AppMesh Aug 20, 2019

mergify bot added the automerge-enabled label Aug 20, 2019

SoManyHs reviewed Aug 21, 2019

View reviewed changes

Merge remote-tracking branch 'origin/master' into IPyandy/aws-appmesh

0307c16

rix0rrr dismissed their stale review via 0307c16 August 22, 2019 09:50

Fix typos, simplify addBackend().

36e7618

Fix up various details of idiomacity, make it so only one listener is…

46eb29f

… supported, fix tests

Fix build

c374999

rix0rrr approved these changes Aug 23, 2019

View reviewed changes

rix0rrr merged commit 98863f9 into aws:master Aug 23, 2019

yandy-r deleted the IPyandy/aws-appmesh branch August 23, 2019 12:48

subfuzion mentioned this pull request Oct 4, 2019

Needs to support routing traffic to multiple services subfuzion/enable-appmesh#3

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(appmesh): add support for AWS AppMesh #2299

feat(appmesh): add support for AWS AppMesh #2299

yandy-r commented Apr 15, 2019 •

edited

Loading

rix0rrr commented Aug 9, 2019

SoManyHs commented Aug 9, 2019

rix0rrr commented Aug 19, 2019

mergify bot commented Aug 20, 2019

SoManyHs left a comment

SoManyHs Aug 21, 2019

rix0rrr Aug 22, 2019

SoManyHs Aug 21, 2019

rix0rrr Aug 22, 2019

yandy-r commented Aug 22, 2019

mergify bot commented Aug 22, 2019

feat(appmesh): add support for AWS AppMesh #2299

feat(appmesh): add support for AWS AppMesh #2299

Conversation

yandy-r commented Apr 15, 2019 • edited Loading

Pull Request Checklist

rix0rrr commented Aug 9, 2019

SoManyHs commented Aug 9, 2019

rix0rrr commented Aug 19, 2019

mergify bot commented Aug 20, 2019

Pull Request Checklist

SoManyHs left a comment

Choose a reason for hiding this comment

SoManyHs Aug 21, 2019

Choose a reason for hiding this comment

rix0rrr Aug 22, 2019

Choose a reason for hiding this comment

SoManyHs Aug 21, 2019

Choose a reason for hiding this comment

rix0rrr Aug 22, 2019

Choose a reason for hiding this comment

yandy-r commented Aug 22, 2019

mergify bot commented Aug 22, 2019

yandy-r commented Apr 15, 2019 •

edited

Loading