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

fix(app-mesh): Missing port property in gRPC routers matchers #25868

Merged
merged 16 commits into from
Jun 14, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,20 @@ router.addRoute('route-7', {
}),
});

router.addRoute('route-8', {
routeSpec: appmesh.RouteSpec.grpc({
weightedTargets: [
{
virtualNode: node4,
weight: 10,
},
],
match: {
port: 1234,
},
}),
});

const gateway = mesh.addVirtualGateway('gateway1', {
accessLog: appmesh.AccessLog.fromFilePath('/dev/stdout'),
virtualGatewayName: 'gateway1',
Expand Down Expand Up @@ -429,3 +443,12 @@ gateway.addGatewayRoute('gateway1-route-grpc-2', {
},
}),
});

gateway.addGatewayRoute('gateway1-route-grpc-3', {
routeSpec: appmesh.GatewayRouteSpec.grpc({
routeTarget: virtualService,
match: {
port: 1234,
},
}),
});
22 changes: 21 additions & 1 deletion packages/aws-cdk-lib/aws-appmesh/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -673,6 +673,26 @@ router.addRoute('route-grpc-retry', {
});
```

Add a gRPC route that matches based on port:

```ts
declare const router: appmesh.VirtualRouter;
declare const node: appmesh.VirtualNode;

router.addRoute('route-grpc-port', {
routeSpec: appmesh.RouteSpec.grpc({
weightedTargets: [
{
virtualNode: node,
},
],
match: {
port: 1234,
},
}),
});
```

Add a gRPC route with timeout:

```ts
Expand Down Expand Up @@ -779,7 +799,7 @@ gateway.addGatewayRoute('gateway-route-http', {
});
```

For gRPC-based gateway routes, the `match` field can be used to match on service name, host name, and metadata.
For gRPC-based gateway routes, the `match` field can be used to match on service name, host name, port and metadata.

```ts
declare const gateway: appmesh.VirtualGateway;
Expand Down
8 changes: 8 additions & 0 deletions packages/aws-cdk-lib/aws-appmesh/lib/gateway-route-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,13 @@ export interface GrpcGatewayRouteMatch {
* @default true
*/
readonly rewriteRequestHostname?: boolean;

/**
* The port to match from the request.
*
* @default - do not match on port
*/
readonly port?: number;
}

/**
Expand Down Expand Up @@ -365,6 +372,7 @@ class GrpcGatewayRouteSpec extends GatewayRouteSpec {
match: {
serviceName: this.match.serviceName,
hostname: this.match.hostname?.bind(scope).hostnameMatch,
port: this.match.port,
metadata: metadataMatch?.map(metadata => metadata.bind(scope).headerMatch),
},
action: {
Expand Down
4 changes: 2 additions & 2 deletions packages/aws-cdk-lib/aws-appmesh/lib/private/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export function validateGrpcMatchArrayLength(metadata?: HeaderMatch[]): void {
* This is the helper method to validate at least one of gRPC route match type is defined.
*/
export function validateGrpcRouteMatch(match: GrpcRouteMatch): void {
if (match.serviceName === undefined && match.metadata === undefined && match.methodName === undefined) {
if (match.serviceName === undefined && match.metadata === undefined && match.methodName === undefined && match.port === undefined) {
throw new Error('At least one gRPC route match property must be provided');
}
}
Expand All @@ -135,7 +135,7 @@ export function validateGrpcRouteMatch(match: GrpcRouteMatch): void {
* This is the helper method to validate at least one of gRPC gateway route match type is defined.
*/
export function validateGrpcGatewayRouteMatch(match: GrpcGatewayRouteMatch): void {
if (match.serviceName === undefined && match.metadata === undefined && match.hostname === undefined) {
if (match.serviceName === undefined && match.metadata === undefined && match.hostname === undefined && match.port === undefined) {
throw new Error('At least one gRPC gateway route match property beside rewriteRequestHostname must be provided');
}
}
9 changes: 9 additions & 0 deletions packages/aws-cdk-lib/aws-appmesh/lib/route-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@ export interface GrpcRouteMatch {
* @default - do not match on method name
*/
readonly methodName?: string;

/**
* The port to match from the request.
*
* @default - do not match on port
*/
readonly port?: number;
}

/**
Expand Down Expand Up @@ -551,6 +558,7 @@ class GrpcRouteSpec extends RouteSpec {
const serviceName = this.match.serviceName;
const methodName = this.match.methodName;
const metadata = this.match.metadata;
const port = this.match.port;

validateGrpcRouteMatch(this.match);
validateGrpcMatchArrayLength(metadata);
Expand All @@ -569,6 +577,7 @@ class GrpcRouteSpec extends RouteSpec {
serviceName: serviceName,
methodName: methodName,
metadata: metadata?.map(singleMetadata => singleMetadata.bind(scope).headerMatch),
port: port,
},
timeout: renderTimeout(this.timeout),
retryPolicy: this.retryPolicy ? renderGrpcRetryPolicy(this.retryPolicy) : undefined,
Expand Down
71 changes: 71 additions & 0 deletions packages/aws-cdk-lib/aws-appmesh/test/gateway-route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -549,6 +549,77 @@ describe('gateway route', () => {
});
});

describe('with port match', () => {
test('should match based on port', () => {
// GIVEN
const stack = new cdk.Stack();

// WHEN
const mesh = new appmesh.Mesh(stack, 'mesh', {
meshName: 'test-mesh',
});

const virtualGateway = new appmesh.VirtualGateway(stack, 'gateway-1', {
listeners: [appmesh.VirtualGatewayListener.http()],
mesh: mesh,
});

const virtualService = new appmesh.VirtualService(stack, 'vs-1', {
virtualServiceProvider: appmesh.VirtualServiceProvider.none(mesh),
virtualServiceName: 'target.local',
});

// Add an HTTP Route
virtualGateway.addGatewayRoute('gateway-http-route', {
routeSpec: appmesh.GatewayRouteSpec.http({
routeTarget: virtualService,
match: {
hostname: appmesh.GatewayRouteHostnameMatch.exactly('example.com'),
},
}),
gatewayRouteName: 'gateway-http-route',
});

virtualGateway.addGatewayRoute('gateway-grpc-route', {
routeSpec: appmesh.GatewayRouteSpec.grpc({
routeTarget: virtualService,
match: {
port: 1234,
},
}),
gatewayRouteName: 'gateway-grpc-route',
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::AppMesh::GatewayRoute', {
GatewayRouteName: 'gateway-http-route',
Spec: {
HttpRoute: {
Match: {
Hostname: {
Exact: 'example.com',
},
},
Action: {
Rewrite: Match.absent(),
},
},
},
});

Template.fromStack(stack).hasResourceProperties('AWS::AppMesh::GatewayRoute', {
GatewayRouteName: 'gateway-grpc-route',
Spec: {
GrpcRoute: {
Match: {
Port: 1234,
},
},
},
});
});
});

describe('with metadata match', () => {
test('should match based on metadata', () => {
// GIVEN
Expand Down
35 changes: 35 additions & 0 deletions packages/aws-cdk-lib/aws-appmesh/test/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,41 @@ describe('route', () => {
});
});

test('should have grpc route matcher with port when specified', () => {
// GIVEN
const stack = new cdk.Stack();
const mesh = new appmesh.Mesh(stack, 'mesh', {
meshName: 'test-mesh',
});
const router = new appmesh.VirtualRouter(stack, 'router', {
mesh,
});
const virtualNode = mesh.addVirtualNode('test-node', {
serviceDiscovery: appmesh.ServiceDiscovery.dns('test'),
listeners: [appmesh.VirtualNodeListener.http()],
});

// WHEN
router.addRoute('test-grpc-route', {
routeSpec: appmesh.RouteSpec.grpc({
weightedTargets: [{ virtualNode }],
match: { port: 1234 },
}),
});

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::AppMesh::Route', {
Spec: {
GrpcRoute: {
Match: {
Port: 1234,
},
},
},
RouteName: 'test-grpc-route',
});
});

test('grpc retry events are Match.absent() when specified as an empty array', () => {
// GIVEN
const stack = new cdk.Stack();
Expand Down
30 changes: 15 additions & 15 deletions yarn.lock
Copy link
Contributor

Choose a reason for hiding this comment

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

yarn.lock shouldn't need to be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated! 😄

Original file line number Diff line number Diff line change
Expand Up @@ -6984,7 +6984,7 @@ jsii-reflect@1.82.0, jsii-reflect@^1.82.0:
oo-ascii-tree "^1.82.0"
yargs "^16.2.0"

jsii-rosetta@1.82.0, jsii-rosetta@^1.82.0:
jsii-rosetta@^1.82.0:
version "1.82.0"
resolved "https://registry.npmjs.org/jsii-rosetta/-/jsii-rosetta-1.82.0.tgz#8680af6223ada85a247731e036cf4fc3220b90e3"
integrity sha512-3R/+fYm1gIUp6HEBzRyvHgCxKy1BZB2ZqO+0zYvgi7LEUEsvn7OBUDI7NoVe6/Bqp8DKOxzUYm4xe8jJ6Gr0og==
Expand Down Expand Up @@ -7021,43 +7021,43 @@ jsii-rosetta@~5.0.8:
workerpool "^6.4.0"
yargs "^17.7.2"

jsii@1.81.0, jsii@^1.81.0:
version "1.81.0"
resolved "https://registry.npmjs.org/jsii/-/jsii-1.81.0.tgz#144daeacfa41660d04b9c695f21c17d3ffd58515"
integrity sha512-m5BQlNJGHFFwxUxZwagSqqMCS1wbt/0fKeEr+j7WWsG1lhu3fUxANYukUAeQEPJAPWuv0kAbqMaxj/4HSgd5RA==
jsii@1.82.0, jsii@^1.82.0:
version "1.82.0"
resolved "https://registry.npmjs.org/jsii/-/jsii-1.82.0.tgz#dbc5d61853619a0fb7653539663832ae1c2bf2a5"
integrity sha512-EMOjpWCiRYCo5ZzndGXsSsMrBSj4sa7njzEcFWryVnXj6reVP2axwsa3AXC/xCf4/PjchESxGDbSieX7cP9A+A==
dependencies:
"@jsii/check-node" "1.81.0"
"@jsii/spec" "^1.81.0"
"@jsii/check-node" "1.82.0"
"@jsii/spec" "^1.82.0"
case "^1.6.3"
chalk "^4"
fast-deep-equal "^3.1.3"
fs-extra "^10.1.0"
log4js "^6.9.1"
semver "^7.5.0"
semver "^7.5.1"
semver-intersect "^1.4.0"
sort-json "^2.0.1"
spdx-license-list "^6.6.0"
typescript "~3.9.10"
yargs "^16.2.0"

jsii@1.82.0:
version "1.82.0"
resolved "https://registry.npmjs.org/jsii/-/jsii-1.82.0.tgz#dbc5d61853619a0fb7653539663832ae1c2bf2a5"
integrity sha512-EMOjpWCiRYCo5ZzndGXsSsMrBSj4sa7njzEcFWryVnXj6reVP2axwsa3AXC/xCf4/PjchESxGDbSieX7cP9A+A==
jsii@5.0.x:
version "5.0.10"
resolved "https://registry.npmjs.org/jsii/-/jsii-5.0.10.tgz#0bf853e02e4c21ab88fc6c73ec2dc01a9d1156f9"
integrity sha512-AIcxUr+9qmv7SipMP8ZyvAT1EFlAzd9JAu+qc6SLlKvx1qxQe1O2wvcg3jOaOpdrbEHNYHXJ7VorztAtRcXX9A==
dependencies:
"@jsii/check-node" "1.82.0"
"@jsii/spec" "^1.82.0"
case "^1.6.3"
chalk "^4"
downlevel-dts "^0.11.0"
fast-deep-equal "^3.1.3"
fs-extra "^10.1.0"
log4js "^6.9.1"
semver "^7.5.1"
semver-intersect "^1.4.0"
sort-json "^2.0.1"
spdx-license-list "^6.6.0"
typescript "~3.9.10"
yargs "^16.2.0"
typescript "~5.0.4"
yargs "^17.7.2"

jsii@~5.0.8:
version "5.0.8"
Expand Down