Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
5326284
Added BackendDefaults to Virtual Node
sshver Oct 29, 2020
e476ae7
Merge remote-tracking branch 'upstream/master' into feature/backendDe…
sshver Nov 3, 2020
dbc5870
Modeled TLSClientValidation to enforce trust in file or acm cetificate
sshver Nov 4, 2020
5b4c97b
Merge remote-tracking branch 'upstream/master' into feature/backendDe…
sshver Nov 10, 2020
b4b7f95
Added check to have only one backendDefault per virtual node
sshver Nov 11, 2020
ce35692
Merge remote-tracking branch 'upstream/master' into feature/backendDe…
sshver Nov 11, 2020
4aea038
adding backend validation context
sshver Nov 12, 2020
25208a4
created a seperate module validation-context
sshver Nov 12, 2020
5b260be
Created a seperate client-policy file as client policy wont be only s…
sshver Nov 18, 2020
a55a305
Merge remote-tracking branch 'upstream/master' into feature/backendDe…
sshver Nov 18, 2020
7ca64da
Updated comments and removed newlines
sshver Nov 18, 2020
01be91b
1. Addressed comments
sshver Nov 24, 2020
7e2ddbc
Merge remote-tracking branch 'upstream/master' into feature/backendDe…
sshver Nov 24, 2020
49824b1
Added skeleton L2 for ACMPCA and addressed comments
sshver Nov 30, 2020
0306a36
Merge remote-tracking branch 'upstream/master' into feature/backendDe…
sshver Nov 30, 2020
045d2a7
Fixed minor comments
sshver Dec 2, 2020
2cad882
Merge remote-tracking branch 'upstream/master' into feature/backendDe…
sshver Dec 2, 2020
20c0e96
Fixed comments
sshver Dec 3, 2020
d36fc42
Added validation check to ensure certificateAuthorityArns is not empty
sshver Dec 4, 2020
4e07587
Minor fix
sshver Dec 4, 2020
da25261
Merge remote-tracking branch 'upstream/master' into feature/backendDe…
sshver Dec 4, 2020
41d2c12
Updated README and fixed build errors
sshver Dec 4, 2020
d18fdc4
Merge remote-tracking branch 'upstream/master' into feature/backendDe…
sshver Dec 4, 2020
4ec4b21
Merge remote-tracking branch 'upstream/master' into feature/backendDe…
sshver Dec 4, 2020
8b0b863
Minor changes
sshver Dec 7, 2020
bd2d85e
Updated README.md
sshver Dec 7, 2020
8b707ce
Merge remote-tracking branch 'upstream/master' into feature/backendDe…
sshver Dec 7, 2020
de939c3
Merge branch 'master' into feature/backendDefaults
mergify[bot] Dec 7, 2020
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
83 changes: 83 additions & 0 deletions packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ export interface IVirtualNode extends cdk.IResource {
* Utility method to add Node Listeners for new or existing VirtualNodes
*/
addListeners(...listeners: VirtualNodeListener[]): void;

/**
* Utility method to add Default Backend Configuration for new or existing VirtualNodes
*/
addBackendDefaults(backendDefaults: BackendDefaults): void;
}

/**
Expand Down Expand Up @@ -97,8 +102,50 @@ export interface VirtualNodeBaseProps {
* @default - No access logging
*/
readonly accessLog?: AccessLog;

/**
* Default Configuration Virtual Node uses to communicate with Vritual Service
Copy link

Choose a reason for hiding this comment

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

typo: Vritual => Virtual

*
* @default - No Config
*/
readonly backendDefaults?: BackendDefaults;
}

/**
* Default Configuration for Virtual Nodes
Copy link

Choose a reason for hiding this comment

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

I think we should be more descriptive here.

Default configuration that is applied to all backends for the virtual node. Any configuration defined will be overwritten by configurations specified for a particular backend.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Added descriptive comment.

*/
export interface BackendDefaults {
/**
* TLS enforced if True.
Copy link

Choose a reason for hiding this comment

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

Maybe we can reword this to be more descriptive

TLS Connections with downstream server will always be enforced if True

Something to that effect.

*
* @default - none
Copy link

Choose a reason for hiding this comment

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

The default in our API is true, let's mirror that here

Copy link
Owner Author

Choose a reason for hiding this comment

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

set enforce to true by default.

*/
readonly enforce?: boolean

/**
* TLS enforced on these ports. If not specified it is enforced on all ports.
*
* @default - none
*/
readonly ports?: number[]

/**
* Certificate discovery method, ACM-PCA or Local file hosting.
*
* @default - none
*/
readonly certificateType: string;

/**
* Certificate File path or Certificate ARN.
*
* @default - none
*/
readonly certificate: string[];
Copy link

Choose a reason for hiding this comment

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

These should not be top level properties. These are specific to the client configuration with regard to TLS. There may be other types of backend defaults we could apply in the future.

You could model this more like

interface BackendDefaults {
  readonly tlsClientPolicy: TLSClientPolicy;
}

interface TLSClientPolicy {
  /**
   * @default - True
   */
  readonly enforce?: boolean;

  ...

  readonly validation: TLSClientValidation;
}

TLSClientValidation could be modeled as a bind class to enforce the trust is one of file, acmpca, or sds (in the future). We may want to accept the Certificate Authority construct from ACMPCA, which is not yet built. Adam can help with that, we'll just need to coordinate with him

We also want to be aware of the changes that are going to be made with aws/aws-app-mesh-roadmap#34 and be able to accommodate them easily

Copy link
Owner Author

Choose a reason for hiding this comment

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

updated the model.


}


/**
* The properties used when creating a new VirtualNode
*/
Expand All @@ -122,6 +169,7 @@ abstract class VirtualNodeBase extends cdk.Resource implements IVirtualNode {

protected readonly backends = new Array<CfnVirtualNode.BackendProperty>();
protected readonly listeners = new Array<CfnVirtualNode.ListenerProperty>();
protected readonly backendDefaults = new Array<CfnVirtualNode.BackendDefaultsProperty>();

/**
* Add a Virtual Services that this node is expected to send outbound traffic to
Expand All @@ -136,6 +184,37 @@ abstract class VirtualNodeBase extends cdk.Resource implements IVirtualNode {
}
}

/**
* Adds Default Backend Configuration for virtual node to communicate with Virtual Services.
*/
public addBackendDefaults(backendDefaults: BackendDefaults) {
if (Object.keys(backendDefaults).length!==0) {
const enforce = backendDefaults.enforce || false;
const ports = backendDefaults.ports || undefined;
const certificateType = backendDefaults.certificateType || 'acm';
const certificate = backendDefaults.certificate || [];
this.backendDefaults.push({
clientPolicy: {
tls: {
enforce: enforce,
ports: ports,
validation: {
trust: certificateType === 'acm' ? {
acm: {
certificateAuthorityArns: certificate,
},
} : {
file: {
certificateChain: certificate[0],
},
},
},
},
},
});
}
}

/**
* Utility method to add an inbound listener for this virtual node
*/
Expand Down Expand Up @@ -259,6 +338,9 @@ export class VirtualNode extends VirtualNodeBase {

this.addBackends(...props.backends || []);
this.addListeners(...props.listener ? [props.listener] : []);
if (props.backendDefaults) {
this.addBackendDefaults(props.backendDefaults);
}
const accessLogging = props.accessLog?.bind(this);

const node = new CfnVirtualNode(this, 'Resource', {
Expand All @@ -267,6 +349,7 @@ export class VirtualNode extends VirtualNodeBase {
spec: {
backends: cdk.Lazy.anyValue({ produce: () => this.backends }, { omitEmptyArray: true }),
listeners: cdk.Lazy.anyValue({ produce: () => this.listeners }, { omitEmptyArray: true }),
backendDefaults: cdk.Lazy.anyValue({ produce: () => this.backendDefaults[0] }, { omitEmptyArray: true }),
serviceDiscovery: {
dns: props.dnsHostName !== undefined ? { hostname: props.dnsHostName } : undefined,
awsCloudMap: props.cloudMapService !== undefined ? {
Expand Down
28 changes: 28 additions & 0 deletions packages/@aws-cdk/aws-appmesh/test/integ.mesh.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,20 @@
]
},
"Spec": {
"BackendDefaults": {
"ClientPolicy": {
"TLS": {
"Enforce": true,
"Validation": {
"Trust": {
"File": {
"CertificateChain": "Path to Certificate"
}
}
}
}
}
},
"Backends": [
{
"VirtualService": {
Expand Down Expand Up @@ -739,6 +753,20 @@
]
},
"Spec": {
"BackendDefaults": {
"ClientPolicy": {
"TLS": {
"Enforce": true,
"Validation": {
"Trust": {
"File": {
"CertificateChain": "Path to Certificate"
}
}
}
}
}
},
"Listeners": [
{
"HealthCheck": {
Expand Down
11 changes: 11 additions & 0 deletions packages/@aws-cdk/aws-appmesh/test/integ.mesh.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,11 @@ const node2 = mesh.addVirtualNode('node2', {
unhealthyThreshold: 2,
},
},
backendDefaults: {
enforce: true,
certificate: ['Path to Certificate'],
certificateType: 'file',
},
backends: [
new appmesh.VirtualService(stack, 'service-3', {
virtualServiceName: 'service3.domain.local',
Expand All @@ -98,6 +103,12 @@ const node3 = mesh.addVirtualNode('node3', {
accessLog: appmesh.AccessLog.fromFilePath('/dev/stdout'),
});

node3.addBackendDefaults({
enforce: true,
certificate: ['Path to Certificate'],
certificateType: 'file',
});

router.addRoute('route-2', {
routeTargets: [
{
Expand Down
57 changes: 57 additions & 0 deletions packages/@aws-cdk/aws-appmesh/test/test.virtual-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,64 @@ export = {
},
}),
);
test.done();
},
},
'when a default backend configuration is added': {
'should add the backend default configuration to the resource'(test: Test) {
// GIVEN
const stack = new cdk.Stack();

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

const node = mesh.addVirtualNode('test-node', {
dnsHostName: 'test',
});

node.addListeners({
portMapping: {
port: 8081,
protocol: appmesh.Protocol.TCP,
},
});

node.addBackendDefaults({
certificateType: 'file',
certificate: ['path to certificate'],
});

// THEN
expect(stack).to(
haveResourceLike('AWS::AppMesh::VirtualNode', {
Spec: {
Listeners: [
{
PortMapping: {
Port: 8081,
Protocol: 'tcp',
},
},
],
BackendDefaults: {
ClientPolicy: {
TLS: {
Enforce: false,
Validation: {
Trust: {
File: {
CertificateChain: 'path to certificate',
},
},
},
},
},
},
},
}),
);
test.done();
},
},
Expand Down