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(appmesh): Move Client Policy from Virtual Service to backend structure #12943

Merged
merged 12 commits into from
Mar 12, 2021
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ export class AppMeshExtension extends ServiceExtension {
// Next update the app mesh config so that the local Envoy
// proxy on this service knows how to route traffic to
// nodes from the other service.
this.virtualNode.addBackend(otherAppMesh.virtualService);
this.virtualNode.addBackend(appmesh.Backend.virtualServiceBackend({ virtualService: otherAppMesh.virtualService }));
}

private routeSpec(weightedTargets: appmesh.WeightedTarget[], serviceName: string): appmesh.RouteSpec {
Expand Down
83 changes: 83 additions & 0 deletions packages/@aws-cdk/aws-appmesh/lib/shared-interfaces.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import * as cdk from '@aws-cdk/core';
import { CfnVirtualGateway, CfnVirtualNode } from './appmesh.generated';
import { ClientPolicy } from './client-policy';
import { IVirtualService } from './virtual-service';

// keep this import separate from other imports to reduce chance for merge conflicts with v2-main
// eslint-disable-next-line no-duplicate-imports, import/order
Expand Down Expand Up @@ -194,3 +196,84 @@ class FileAccessLog extends AccessLog {
}
}

/**
* Represents the properties needed to define backend defaults
*/
export interface BackendDefaults {
/**
* Client policy for backend defaults
*
* @default none
*/
readonly clientPolicy?: ClientPolicy;
}

/**
* Represents the properties needed to define a Virtual Service backend
*/
export interface VirtualServiceBackendOptions {
/**
* The Virtual Service this backend points to
*/
readonly virtualService: IVirtualService;

/**
* Client policy for the backend
*
* @default none
*/
readonly clientPolicy?: ClientPolicy;
}

/**
* Properties for a backend
*/
export interface BackendConfig {
/**
* Config for a Virtual Service backend
*/
readonly virtualServiceBackend: CfnVirtualNode.BackendProperty;
}


/**
* Contains static factory methods to create backends
*/
export abstract class Backend {
/**
* Construct a Virtual Service backend
*/
public static virtualServiceBackend(props: VirtualServiceBackendOptions): Backend {
alexbrjo marked this conversation as resolved.
Show resolved Hide resolved
return new VirtualServiceBackend(props.virtualService, props.clientPolicy);
}

/**
* Return backend config
*/
public abstract bind(_scope: Construct): BackendConfig;
}

/**
* Represents the properties needed to define a Virtual Service backend
*/
class VirtualServiceBackend extends Backend {

constructor (private readonly virtualService: IVirtualService,
private readonly clientPolicy: ClientPolicy | undefined) {
skinny85 marked this conversation as resolved.
Show resolved Hide resolved
super();
}

/**
* Return config for a Virtual Service backend
*/
public bind(_scope: Construct): BackendConfig {
return {
virtualServiceBackend: {
virtualService: {
virtualServiceName: this.virtualService.virtualServiceName,
clientPolicy: this.clientPolicy?.bind(_scope).clientPolicy,
},
},
};
}
}
9 changes: 5 additions & 4 deletions packages/@aws-cdk/aws-appmesh/lib/virtual-gateway.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import * as cdk from '@aws-cdk/core';
import { Construct } from 'constructs';
import { CfnVirtualGateway } from './appmesh.generated';
import { ClientPolicy } from './client-policy';
import { GatewayRoute, GatewayRouteBaseProps } from './gateway-route';
import { IMesh, Mesh } from './mesh';
import { AccessLog } from './shared-interfaces';
import { AccessLog, BackendDefaults } from './shared-interfaces';
import { VirtualGatewayListener, VirtualGatewayListenerConfig } from './virtual-gateway-listener';

/**
Expand Down Expand Up @@ -66,7 +65,7 @@ export interface VirtualGatewayBaseProps {
*
* @default - No Config
*/
readonly backendsDefaultClientPolicy?: ClientPolicy;
readonly backendDefaults?: BackendDefaults;
}

/**
Expand Down Expand Up @@ -180,7 +179,9 @@ export class VirtualGateway extends VirtualGatewayBase {
meshName: this.mesh.meshName,
spec: {
listeners: this.listeners.map(listener => listener.listener),
backendDefaults: props.backendsDefaultClientPolicy?.bind(this),
backendDefaults: props.backendDefaults !== undefined ? {
clientPolicy: props.backendDefaults?.clientPolicy?.bind(this).clientPolicy,
} : undefined,
alexbrjo marked this conversation as resolved.
Show resolved Hide resolved
logging: accessLogging !== undefined ? {
accessLog: accessLogging.virtualGatewayAccessLog,
} : undefined,
Expand Down
21 changes: 8 additions & 13 deletions packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
import * as cdk from '@aws-cdk/core';
import { Construct } from 'constructs';
import { CfnVirtualNode } from './appmesh.generated';
import { ClientPolicy } from './client-policy';
import { IMesh, Mesh } from './mesh';
import { ServiceDiscovery } from './service-discovery';
import { AccessLog } from './shared-interfaces';
import { AccessLog, BackendDefaults, Backend } from './shared-interfaces';
import { VirtualNodeListener, VirtualNodeListenerConfig } from './virtual-node-listener';
import { IVirtualService } from './virtual-service';

/**
* Interface which all VirtualNode based classes must implement
Expand Down Expand Up @@ -61,7 +59,7 @@ export interface VirtualNodeBaseProps {
*
* @default - No backends
*/
readonly backends?: IVirtualService[];
readonly backends?: Backend[];

/**
* Initial listener for the virtual node
Expand All @@ -82,7 +80,7 @@ export interface VirtualNodeBaseProps {
*
* @default - No Config
*/
readonly backendsDefaultClientPolicy?: ClientPolicy;
readonly backendDefaults?: BackendDefaults;
}

/**
Expand Down Expand Up @@ -185,7 +183,9 @@ export class VirtualNode extends VirtualNodeBase {
spec: {
backends: cdk.Lazy.anyValue({ produce: () => this.backends }, { omitEmptyArray: true }),
listeners: cdk.Lazy.anyValue({ produce: () => this.listeners.map(listener => listener.listener) }, { omitEmptyArray: true }),
backendDefaults: props.backendsDefaultClientPolicy?.bind(this),
backendDefaults: props.backendDefaults !== undefined ? {
clientPolicy: props.backendDefaults?.clientPolicy?.bind(this).clientPolicy,
} : undefined,
alexbrjo marked this conversation as resolved.
Show resolved Hide resolved
serviceDiscovery: {
dns: serviceDiscovery?.dns,
awsCloudMap: serviceDiscovery?.cloudmap,
Expand Down Expand Up @@ -214,13 +214,8 @@ export class VirtualNode extends VirtualNodeBase {
/**
* Add a Virtual Services that this node is expected to send outbound traffic to
*/
public addBackend(virtualService: IVirtualService) {
this.backends.push({
virtualService: {
virtualServiceName: virtualService.virtualServiceName,
clientPolicy: virtualService.clientPolicy?.bind(this).clientPolicy,
},
});
public addBackend(backend: Backend) {
this.backends.push(backend.bind(this).virtualServiceBackend);
}
}

Expand Down
24 changes: 0 additions & 24 deletions packages/@aws-cdk/aws-appmesh/lib/virtual-service.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as cdk from '@aws-cdk/core';
import { Construct } from 'constructs';
import { CfnVirtualService } from './appmesh.generated';
import { ClientPolicy } from './client-policy';
import { IMesh, Mesh } from './mesh';
import { IVirtualNode } from './virtual-node';
import { IVirtualRouter } from './virtual-router';
Expand All @@ -28,11 +27,6 @@ export interface IVirtualService extends cdk.IResource {
* The Mesh which the VirtualService belongs to
*/
readonly mesh: IMesh;

/**
* Client policy for this Virtual Service
*/
readonly clientPolicy?: ClientPolicy;
}

/**
Expand All @@ -50,13 +44,6 @@ export interface VirtualServiceProps {
*/
readonly virtualServiceName?: string;

/**
* Client policy for this Virtual Service
*
* @default - none
*/
readonly clientPolicy?: ClientPolicy;

/**
* The VirtualNode or VirtualRouter which the VirtualService uses as its provider
*/
Expand Down Expand Up @@ -90,7 +77,6 @@ export class VirtualService extends cdk.Resource implements IVirtualService {
return new class extends cdk.Resource implements IVirtualService {
readonly virtualServiceName = attrs.virtualServiceName;
readonly mesh = attrs.mesh;
readonly clientPolicy = attrs.clientPolicy;
readonly virtualServiceArn = cdk.Stack.of(this).formatArn({
service: 'appmesh',
resource: `mesh/${attrs.mesh.meshName}/virtualService`,
Expand All @@ -114,14 +100,11 @@ export class VirtualService extends cdk.Resource implements IVirtualService {
*/
public readonly mesh: IMesh;

public readonly clientPolicy?: ClientPolicy;

constructor(scope: Construct, id: string, props: VirtualServiceProps) {
super(scope, id, {
physicalName: props.virtualServiceName || cdk.Lazy.string({ produce: () => cdk.Names.uniqueId(this) }),
});

this.clientPolicy = props.clientPolicy;
const providerConfig = props.virtualServiceProvider.bind(this);
this.mesh = providerConfig.mesh;

Expand Down Expand Up @@ -160,13 +143,6 @@ export interface VirtualServiceAttributes {
* The Mesh which the VirtualService belongs to
*/
readonly mesh: IMesh;

/**
* Client policy for this Virtual Service
*
* @default - none
*/
readonly clientPolicy?: ClientPolicy;
}

/**
Expand Down
8 changes: 4 additions & 4 deletions packages/@aws-cdk/aws-appmesh/test/integ.mesh.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,7 @@
"VirtualService": {
"VirtualServiceName": {
"Fn::GetAtt": [
"service27C65CF7D",
alexbrjo marked this conversation as resolved.
Show resolved Hide resolved
"service2B2862B6B",
"VirtualServiceName"
]
}
Expand Down Expand Up @@ -711,7 +711,7 @@
"VirtualService": {
"VirtualServiceName": {
"Fn::GetAtt": [
"service3859EB104",
"service3529E2EB1",
"VirtualServiceName"
]
}
Expand Down Expand Up @@ -969,7 +969,7 @@
"VirtualServiceName": "service1.domain.local"
}
},
"service27C65CF7D": {
"service2B2862B6B": {
"Type": "AWS::AppMesh::VirtualService",
"Properties": {
"MeshName": {
Expand All @@ -982,7 +982,7 @@
"VirtualServiceName": "service2.domain.local"
}
},
"service3859EB104": {
"service3529E2EB1": {
"Type": "AWS::AppMesh::VirtualService",
"Properties": {
"MeshName": {
Expand Down
47 changes: 28 additions & 19 deletions packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,16 @@ const virtualService = new appmesh.VirtualService(stack, 'service', {
virtualServiceName: 'service1.domain.local',
});

const virtualService2 = new appmesh.VirtualService(stack, 'service2', {
virtualServiceProvider: appmesh.VirtualServiceProvider.none(mesh),
virtualServiceName: 'service2.domain.local',
});

const virtualService3 = new appmesh.VirtualService(stack, 'service3', {
virtualServiceProvider: appmesh.VirtualServiceProvider.none(mesh),
virtualServiceName: 'service3.domain.local',
});

const node = mesh.addVirtualNode('node', {
serviceDiscovery: appmesh.ServiceDiscovery.dns(`node1.${namespace.namespaceName}`),
listeners: [appmesh.VirtualNodeListener.http({
Expand All @@ -36,16 +46,14 @@ const node = mesh.addVirtualNode('node', {
path: '/check-path',
},
})],
backends: [
virtualService,
],
backends: [appmesh.Backend.virtualServiceBackend({
virtualService: virtualService,
})],
});

node.addBackend(new appmesh.VirtualService(stack, 'service-2', {
virtualServiceName: 'service2.domain.local',
virtualServiceProvider: appmesh.VirtualServiceProvider.none(mesh),
}),
);
node.addBackend(appmesh.Backend.virtualServiceBackend({
virtualService: virtualService2,
}));

router.addRoute('route-1', {
routeSpec: appmesh.RouteSpec.http({
Expand Down Expand Up @@ -78,15 +86,14 @@ const node2 = mesh.addVirtualNode('node2', {
unhealthyThreshold: 2,
},
})],
backendsDefaultClientPolicy: appmesh.ClientPolicy.fileTrust({
certificateChain: 'path/to/cert',
}),
backends: [
new appmesh.VirtualService(stack, 'service-3', {
virtualServiceName: 'service3.domain.local',
virtualServiceProvider: appmesh.VirtualServiceProvider.none(mesh),
backendDefaults: {
clientPolicy: appmesh.ClientPolicy.fileTrust({
certificateChain: 'path/to/cert',
}),
],
},
backends: [appmesh.Backend.virtualServiceBackend({
virtualService: virtualService3,
})],
});

const node3 = mesh.addVirtualNode('node3', {
Expand All @@ -102,9 +109,11 @@ const node3 = mesh.addVirtualNode('node3', {
unhealthyThreshold: 2,
},
})],
backendsDefaultClientPolicy: appmesh.ClientPolicy.fileTrust({
certificateChain: 'path-to-certificate',
}),
backendDefaults: {
clientPolicy: appmesh.ClientPolicy.fileTrust({
certificateChain: 'path-to-certificate',
}),
},
accessLog: appmesh.AccessLog.fromFilePath('/dev/stdout'),
});

Expand Down
2 changes: 0 additions & 2 deletions packages/@aws-cdk/aws-appmesh/test/test.health-check.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@ export = {
// THEN
test.doesNotThrow(() => toThrow(min));
test.doesNotThrow(() => toThrow(max));
// falsy, falls back to portMapping.port
// test.throws(() => toThrow(min - 1), /below the minimum threshold/);
test.throws(() => toThrow(max + 1), /above the maximum threshold/);

test.done();
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-appmesh/test/test.mesh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -270,9 +270,9 @@ export = {
listeners: [appmesh.VirtualNodeListener.http({
port: 8080,
})],
backends: [
service1,
],
backends: [appmesh.Backend.virtualServiceBackend({
virtualService: service1,
})],
});

// THEN
Expand Down
Loading