Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
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
165 changes: 165 additions & 0 deletions packages/@aws-cdk/aws-appmesh/lib/virtual-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,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 @@ -98,6 +103,144 @@ 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 that is applied to all backends for the virtual node.
* Any configuration defined will be overwritten by configurations specified for a particular backend.
*/
export interface BackendDefaults {
/**
* Client policy for TLS
*/
readonly tlsClientPolicy: TLSClientPolicyProps;
}

/**
* Properties with respect to TLS backend default.
*/
export interface TLSClientPolicyProps {
Copy link

@dfezzie dfezzie Nov 5, 2020

Choose a reason for hiding this comment

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

So the Props name is reserved for class constructors. (Found this out in a PR I have out now)

Let's have this named TLSClientPolicyOptions

/**
* 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 - True
*/
readonly enforce?: boolean;

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

/**
* To enforce the trust is one of file, acmpca, or sds.
Copy link

Choose a reason for hiding this comment

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

Since we haven't introduced SDS yet, let's remove the reference here

We can also reword this to

Policy used to determine if the TLS certificate the server presents is accepted

*
* @default - none
*/
readonly validation: TLSClientValidation;
Copy link

Choose a reason for hiding this comment

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

Needs to be optional

Copy link
Owner Author

Choose a reason for hiding this comment

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

Shouldn't it be required ? I found this reference doc where in it says this field in required

}

/**
* Defines the TLS validation context trust.
*/
export abstract class TLSClientValidation {
/**
* TLS validation context trust for a local file
*/
public static fileTrustValidation(props: FileTrustProps): TLSClientValidation {
Copy link

Choose a reason for hiding this comment

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

We could shorten this to fileTrust(...)

Copy link

Choose a reason for hiding this comment

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

We could also potentially accept just the path to the certificate chain instead of having a props interface. Makes it simpler to use for customers, but has the drawback of not being as extensible in the future, but there are plenty of ways around that.

return new FileTrust(props);
}

/**
* TLS validation context trust for AWS Certicate Manager (ACM) certificate.
Copy link

Choose a reason for hiding this comment

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

I want to specify this is acm-pca somehow, but it doesn't flow great :P You have any ideas?

*/
public static acmTrustValidation(props: ACMTrustProps): TLSClientValidation {
return new ACMTrust(props);
}

/**
* Returns Trust context based on trust type.
*/
public abstract bind(scope: cdk.Construct): CfnVirtualNode.TlsValidationContextProperty;
Copy link

Choose a reason for hiding this comment

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

Bind should return a Config object here

}

/**
* ACM Trust Properties
*/
export interface ACMTrustProps {
/**
* Amazon Resource Name of the Certificates
Copy link

Choose a reason for hiding this comment

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

We should specify ACM PCA here

Amazon Resource Names (ARN) of trusted ACM Private Certificate Authorities

*/
readonly certificateAuthorityArns: string[];
Copy link

Choose a reason for hiding this comment

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

this should be an array of ACM Private Certificate Authorities. I think I mentioned this in the previous rev, since acm-pca is not modeled in CDK yet this is tricky. Will need to coordinate with Adam on this.

}

/**
* File Trust Properties
*/
export interface FileTrustProps {
/**
* Path to the Certificate Chain file on the file system where the Envoy is deployed.
*/
readonly certificateChain: string;
}

/**
* Represents a Transport Layer Security (TLS) validation context trust for a local file
*/
export class FileTrust extends TLSClientValidation {
/**
* Path to the Certificate Chain file on the file system where the Envoy is deployed.
*/
readonly certificateChain: string;

constructor(props: FileTrustProps) {
super();
this.certificateChain = props.certificateChain;
}

public bind(_scope: cdk.Construct): CfnVirtualNode.TlsValidationContextProperty {
return {
trust: {
file: {
certificateChain: this.certificateChain,
},
},
};
}
}

/**
* Represents a TLS validation context trust for an AWS Certicate Manager (ACM) certificate.
*/
export class ACMTrust extends TLSClientValidation {
Copy link

Choose a reason for hiding this comment

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

This does not need to be exported. Same for above

/**
* Amazon Resource Name of the Certificates
*/
readonly certificateAuthorityArns: string[];

constructor(props: ACMTrustProps) {
super();
this.certificateAuthorityArns = props.certificateAuthorityArns;
}

public bind(_scope: cdk.Construct): CfnVirtualNode.TlsValidationContextProperty {
return {
trust: {
acm: {
certificateAuthorityArns: this.certificateAuthorityArns,
},
},
};
}
}

/**
Expand All @@ -123,6 +266,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 @@ -137,6 +281,23 @@ abstract class VirtualNodeBase extends cdk.Resource implements IVirtualNode {
}
}

/**
* Adds Default Backend Configuration for virtual node to communicate with Virtual Services.
*/
public addBackendDefaults(backendDefaults: BackendDefaults) {
// eslint-disable-next-line no-console
console.log(backendDefaults.tlsClientPolicy.enforce);
this.backendDefaults.push({
clientPolicy: {
tls: {
enforce: backendDefaults.tlsClientPolicy.enforce === undefined ? true : backendDefaults.tlsClientPolicy.enforce,
Copy link

Choose a reason for hiding this comment

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

I think enforce: backendDefaults.tlsClientPolicy.enforce ?? true would work here?

validation: backendDefaults.tlsClientPolicy.validation.bind(this),
ports: backendDefaults.tlsClientPolicy.ports ? backendDefaults.tlsClientPolicy.ports : undefined,
Copy link

Choose a reason for hiding this comment

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

This could be simplified to just backendDefaults.tlsClientPolicy.ports since it accepts undefined.

},
},
});
}

/**
* Utility method to add an inbound listener for this virtual node
*/
Expand Down Expand Up @@ -234,6 +395,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 @@ -242,6 +406,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-file"
}
}
}
}
}
},
"Backends": [
{
"VirtualService": {
Expand Down Expand Up @@ -739,6 +753,20 @@
]
},
"Spec": {
"BackendDefaults": {
"ClientPolicy": {
"TLS": {
"Enforce": true,
"Validation": {
"Trust": {
"File": {
"CertificateChain": "path-to-file"
}
}
}
}
}
},
"Listeners": [
{
"HealthCheck": {
Expand Down
15 changes: 15 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,13 @@ const node2 = mesh.addVirtualNode('node2', {
unhealthyThreshold: 2,
},
},
backendDefaults: {
tlsClientPolicy: {
validation: appmesh.TLSClientValidation.fileTrustValidation({
certificateChain: 'path-to-file',
}),
},
},
backends: [
new appmesh.VirtualService(stack, 'service-3', {
virtualServiceName: 'service3.domain.local',
Expand All @@ -98,6 +105,14 @@ const node3 = mesh.addVirtualNode('node3', {
accessLog: appmesh.AccessLog.fromFilePath('/dev/stdout'),
});

node3.addBackendDefaults({
tlsClientPolicy: {
validation: appmesh.TLSClientValidation.fileTrustValidation({
certificateChain: 'path-to-file',
}),
},
});

router.addRoute('route-2', {
routeTargets: [
{
Expand Down
62 changes: 62 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,69 @@ 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({
tlsClientPolicy: {
validation: appmesh.TLSClientValidation.fileTrustValidation({
certificateChain: 'path-to-certificate',
}),
ports: [8080, 8081],
},
});

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