feat(ec2): add support for vpc endpoints #2104

jogold · 2019-03-27T15:43:52Z

Add support for both gateway and interface VPC endpoints. Static members are
exposed for all AWS service endpoints.

As gateway endpoints reference route tables, they currently cannot be added to
imported VPC networks.

BREAKING CHANGE: subnetIds is now replaced by selectSubnets which returns an object containing subnetIds.
BREAKING CHANGE: vpnRoutePropagation now expects a SubnetSelection[]

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.

Add support for both gateway and interface VPC endpoints. Static members are exposed for all AWS service endpoints. As gateway endpoints reference route tables, they currently cannot be added to imported VPC networks.

RomainMuller

Would be nice if @rix0rrr could also review this.

RomainMuller · 2019-03-28T10:11:58Z

packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts

+ * A VPC endpoint service.
+ */
+export class VpcEndpointService {
+ constructor(public readonly name: string, public readonly type: VpcEndpointType) {}


This looks like it should be an interface?

RomainMuller · 2019-03-28T10:14:46Z

packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts

+ * A VPC endpoint AWS service.
+ */
+export class VpcEndpointAwsService extends VpcEndpointService {
+ public static readonly SageMakeNotebook = new VpcEndpointAwsService('sagemaker', VpcEndpointType.Interface, 'aws.sagemaker');


SageMakerNotebook └─ missing 'r'

RomainMuller · 2019-03-28T10:15:38Z

packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts

+ public static readonly CodePipeline = new VpcEndpointAwsService('codepipeline');
+ public static readonly Config = new VpcEndpointAwsService('config');
+ public static readonly DynamoDb = new VpcEndpointAwsService('dynamodb', VpcEndpointType.Gateway);
+ public static readonly Ec2 = new VpcEndpointAwsService('ec2');


Somehow I would rather name those EC2, ECR, SNS, ... The PascalCased name hurts my eyes.

Agree with you but when looking at other packages I find lots of PascalCased names:
https://github.com/awslabs/aws-cdk/blob/7001f7723cde611f0d8cd9c182f92120b09579c3/packages/%40aws-cdk/aws-sns/lib/subscription.ts#L89-L93
https://github.com/awslabs/aws-cdk/blob/7001f7723cde611f0d8cd9c182f92120b09579c3/packages/%40aws-cdk/aws-applicationautoscaling/lib/scalable-target.ts#L227-L246

Hmm. I guess consistency is good. 🤷🏻‍♂️
Thanks for pointing those out 👍🏻

Pascal case is our standard. Otherwise jsii languages cannot delineate words when they convert cases.

RomainMuller · 2019-03-28T10:16:10Z

packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts

+}
+
+/**
+ * A VPC endpoint AWS service.


Would say A VPC endpoint for an AWS service

An AWS VPC endpoint service? This is a service not an endpoint...

I kinda think of those as "services that provide VPC endpoints"... But maybe my mental model is wrong here... This is actually why I was calling for @rix0rrr to have a look, too... I feel he might be a little more into this domain than I am.

RomainMuller · 2019-03-28T10:17:47Z

packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts

+ *
+ * @default private subnets
+ */
+ readonly subnets?: SubnetSelection[]


Does this really need to be an array?

For a VPC with isolated and private subnets, the user could want to add endpoint routing to the route tables of both subnet types.

See the same kind of discussion here for VPN routing #1899 (comment).

If possible, I would like to refactor here to also use a SubnetSelection[]:
https://github.com/awslabs/aws-cdk/blob/7001f7723cde611f0d8cd9c182f92120b09579c3/packages/%40aws-cdk/aws-ec2/lib/vpc.ts#L141-L147

I wonder if it wouldn't be a better approach to make the SubnetSelection able to represent "private + isolated", don't you think?

You mean changing the SubnetSelection interface to:

export interface SubnetSelection { readonly subnetType?: SubnetType[]; readonly subnetName?: string[]; }

We also have to support a list of subnetNames if the user created the VpcNetwork with a specific subnetConfiguration

+1 on extending SubnetSelection. That's our go-to

packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts

eladb · 2019-03-28T19:45:17Z

packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts

+/**
+ * A VPC endpoint service.
+ */
+export interface VpcEndpointService {


This needs to be an IVpcEndpointService (if something implements it begins with an "I").

eladb · 2019-03-28T19:46:02Z

packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts

+ public static readonly CodePipeline = new VpcEndpointAwsService('codepipeline');
+ public static readonly Config = new VpcEndpointAwsService('config');
+ public static readonly DynamoDb = new VpcEndpointAwsService('dynamodb', VpcEndpointType.Gateway);
+ public static readonly Ec2 = new VpcEndpointAwsService('ec2');


Pascal case is our standard. Otherwise jsii languages cannot delineate words when they convert cases.

eladb · 2019-03-28T19:46:37Z

packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts

+ public readonly type: VpcEndpointType;
+
+ constructor(name: string, type?: VpcEndpointType, prefix?: string) {
+ this.name = `${prefix || 'com.amazonaws'}.${cdk.Aws.region}.${name}`;


Do we need to use some URL suffix token that's partition-aware (it's @RomainMuller's turf...)

This is AWS::URLSuffix but reversed... don't know how does this look like for China regions...

@RomainMuller our resident expert on the topic, thoughts?

It's probably going to be different yet again for noncommercial regions (one of those things that looks like a domain name but really isn't). I think this needs to be in RegionInfo.

Well all the documentation says is that those are formatted as com.amazonaws.<region>.<service>... And I couldn't find a reference table with more concrete information. So at this point I'd be willing to assume those are always formatted like so until proven wrong...

eladb · 2019-03-28T19:47:23Z

packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts

+ *
+ * @default private subnets
+ */
+ readonly subnets?: SubnetSelection[]


+1 on extending SubnetSelection. That's our go-to

eladb · 2019-03-28T19:48:50Z

packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts

@@ -266,7 +287,7 @@ export abstract class VpcNetworkBase extends Construct implements IVpcNetwork {
 /**
 * Return the subnets appropriate for the placement strategy
 */
- protected subnets(selection: SubnetSelection = {}): IVpcSubnet[] {
+ public subnets(selection: SubnetSelection = {}): IVpcSubnet[] {


Should we rename this to selectSubnets (to reflect SubnetSelection)

eladb · 2019-03-28T19:49:45Z

packages/@aws-cdk/aws-ec2/test/integ.vpc-endpoint.lit.ts

+const app = new cdk.App();
+const stack = new cdk.Stack(app, 'aws-cdk-ec2-vpc-endpoint');
+
+/// !show


Can we put this in a subclass of stack so that first argument is always this instead of stack (which is non-idiomatic). I know we have many examples like that, but we should start somewhere...

eladb · 2019-03-28T19:50:54Z

packages/@aws-cdk/aws-ec2/test/integ.vpc-endpoint.lit.ts

+ }
+});
+
+const dynamoDbEndpoint = vpc.addGatewayEndpoint('DynamoDbEndpoint', {


indicate that this is just an alternative API

eladb · 2019-03-28T19:51:38Z

packages/@aws-cdk/aws-ec2/test/integ.vpc-endpoint.lit.ts

+ service: ec2.VpcEndpointAwsService.EcrDocker
+});
+
+ecrDockerEndpoint.connections.allowFromAnyIPv4(new ec2.TcpPort(443));


explain why this is needed

eladb · 2019-03-28T19:52:23Z

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

@@ -353,3 +353,8 @@ const vpnConnection = vpc.addVpnConnection('Dynamic', {
 });
 const state = vpnConnection.metricTunnelState();
 ```
+
+### VPC endpoints
+VPC gateway and interface endpoints can be added to a VPC:


Copy & paste some official docs that explain what VPC endpoints are and why would I want to use them

BREAKING CHANGE: `SubnetSelection` now expects either a list of subnet types (`subnetTypes`) or a list of subnet names (`subnetNames`) BREAKING CHANGE: `vpnRoutePropagation` now expects a `SubnetSelection` BREAKING CHANGE: `vpcSubnets` in `ClusterProps` in `@aws-cdk/aws-eks` now expects a `SubnetSelection`

jogold · 2019-03-29T07:56:30Z

Just discovered that some interface endpoints also have a policy document (e.g. CodeCommit), this needs further investigation because the CF documentation is not clear (https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ec2-vpcendpoint.html#cfn-ec2-vpcendpoint-policydocument)

jogold · 2019-04-01T14:44:11Z

Strange... this build (https://travis-ci.org/awslabs/aws-cdk/builds/514128209#L1573) should be failing during the build phase but it's not (lerna success - @aws-cdk/aws-codebuild https://travis-ci.org/awslabs/aws-cdk/builds/514128209#L1702). Ultimately it fails during the test phase for decdk.

eladb

Looks good to me!

@PaulMaddox @leepa any chance you guys can take a quick look?

eladb · 2019-04-03T07:35:26Z

packages/@aws-cdk/aws-ec2/lib/vpc-endpoint.ts

+ });
+
+ this.vpcEndpointId = endpoint.vpcEndpointId;
+ this.creationTimestamp = endpoint.vpcEndpointCreationTimestamp;


Please use the vpcEndpoint prefix. It hints that this is a deploy-time attribute (soon more formally)

eladb · 2019-04-03T07:37:17Z

packages/@aws-cdk/aws-ec2/lib/vpc-ref.ts

@@ -71,6 +72,11 @@ export interface IVpcNetwork extends IConstruct {
 */
 subnetIds(selection?: SubnetSelection): string[];

+ /**
+ * Return the subnets appropriate for the placement strategy


Can we deprecate subnetIds? seems like those can be mapped from the subnet objects?

If we still need it let’s rename to selectSubnetIds but I would just deprecate.

You mean adding @deprecated in JSDoc and removing all calls to this method?

Nooo don't!

I've put effort into precisely into deprecating access to subnet objects and just getting IDs instead, because that is basically all the class consumers ever need from this class anyway. The previous interface was wider than anyone needed, and uncomfortable for imported VPCs.

Can you elaborate why returning IDs is a better API? It is in contradiction with one of our design tents to use strong types instead of IDs.

First of all: convenience. A subnet is never what a consumer needs. It always needs a list of subnet IDs, to pass to CloudFormation. Why would you make a consumer grub around in the innards of my data structures, to do exactly the same operation in all places where it's called? I might as well do that for you.

Second of all: it allows us more flexilibity around exports and imports. The function:

function subnetIds(): string[];

Can be satisfied by a { Fn::ImportValue: Export } or a { Ref: Parameter }. The function:

function subnets(): ISubnet[]

Can not. Because of this, it's super convoluted to export/import VPCs across stacks today. It could all be dramatically simplified if we hadn't painted ourselves into a corner with an API that promises too much, for no good reason because all consumers do is map across those subnets to get the IDs anyway!

And if we're invoking design principles: it violates the Law of Demeter. The VPC is your abstraction, and you shouldn't depend on how it's implemented.

rix0rrr · 2019-04-03T09:22:57Z

Sorry, I apparently should have paid attention to this PR earlier. Why do we need access to subnet objects? I just spent #2025 limiting access to it.

rix0rrr · 2019-04-03T09:26:03Z

Also, I don't agree with the change of subnetType => subnetTypes. This is quite a big breaking change for existing consumers. What are the use cases for selecting multiple subnet types, and can we not achieve them another way?

rix0rrr

Apart from the VPC endpoint support, I don't like the API-complicating changes that are being introduced here to VpcNetwork. Please tell me why they are necessary.

jogold · 2019-04-03T09:33:35Z

@rix0rrr here are a few examples where selecting multiple subnet types are needed:

VPC subnets for AWS EKS clusters (already uses a SubnetSelection[])
Route propagation for VPN routes
Routing for VPC gateway endpoints

My first proposal was to simply use a SubnetSelection[] for those cases. see #2104 (comment). Then @eladb and @RomainMuller came with the idea of extending the subnet selection mechanism.

I let you guys decide on this.

PS: it's pretty easy to revert

jogold · 2019-04-08T09:57:03Z

For the route tables, I don't think you need to get the route tables out, do you?

We need to specify the route table ids in AWS::EC2::VPCEndpoint (https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ec2-vpcendpoint.html#cfn-ec2-vpcendpoint-routetableids). Do you suggest adding public selectSubnetRouteTableIds?

rix0rrr · 2019-04-08T11:18:02Z

Do you suggest adding public selectSubnetRouteTableIds?

No, I would suggest adding an addEndpoint() method. Again, tell, don't ask.

rix0rrr · 2019-04-08T11:25:55Z

RDS for Multi AZ

How were you thinking about this? DBInstace.MultiAZ seems to be a true/false property. So it's specifically for non-multi AZ instances that you would need an AZ, no?

rix0rrr · 2019-04-08T11:30:30Z

By the way, something else I would also be very okay with if we want to model the result of subnet selection as an object, is something like this:

const selection = vpc.selectSubnets({ subnetType: Public });
const ids = selection.subnetIds;
const azs = selection.availabilityZones;

// And even
selection.addRoute(...);
selection.addGateway(...);
selection.addInterface(...);

jogold · 2019-04-08T12:15:35Z

How were you thinking about this? DBInstace.MultiAZ seems to be a true/false property. So it's specifically for non-multi AZ instances that you would need an AZ, no?

I was thinking about improving this:
https://github.com/awslabs/aws-cdk/blob/0f6ce563f40ff8909401ba478c98cd7731a333b5/packages/%40aws-cdk/aws-rds/lib/cluster.ts#L240-L243

same use case here:
https://github.com/awslabs/aws-cdk/blob/fd1729977c969a6b22c0b5ffe66d8b71115b1501/packages/%40aws-cdk/aws-ec2/lib/vpc-endpoint.ts#L392-L396

rix0rrr · 2019-04-08T12:29:01Z

I think these are not created equal. In the first case, I feel like the comment is a little misleading.

For a constructed VPC (as they exist today), the property is correct by construction: there will be exactly one returned subnet per AZ.
For an imported VPC this might not be the case, but also the property is untestable in the general case: the AZs could be unknown at construction time, they could be { Fn::Select: [0, { Fn::GetAZs }]} or even { Ref: AZParameter1 }, and there's no way you could test at construction time whether the requirement is satisfied.

What I'm saying is, the comment makes things sound more dire than they actually are. If we wanted this feature, the right place to enforce this functionality would be at the VPC level:

// Make VpcNetwork guarantee this property if it can
const ids = vpc.selectSubnetIds({ ...props.subnets, atLeast2UniqueAZs: true });

But this only becomes relevant if subnet selection becomes more expressive than it is today. Today 1 subnet would map to 1 AZ, and at least 2 of one maps to at least 2 of the other.

For the VPC Endpoint case, the story is similar. Either the property you're looking for (exactly one subnet per AZ... though maybe the actual constraint is at most one subnet per AZ?) is either guaranteed by construction, or is untestable anyway.

jogold · 2019-04-08T12:53:12Z

For the VPC Endpoint case, the story is similar. Either the property you're looking for (exactly one subnet per AZ... though maybe the actual constraint is at most one subnet per AZ?) is either guaranteed by construction, or is untestable anyway.

This is not always guaranteed by construction, see the following test case I wrote https://github.com/awslabs/aws-cdk/blob/fd1729977c969a6b22c0b5ffe66d8b71115b1501/packages/%40aws-cdk/aws-ec2/test/test.vpc-endpoint.ts#L297

From the console experience, I think it is one subnet per AZ (if I specify 2 subnets then I need those to be in 2 different AZs, availabilityZones.length must equal subnetIds.length no?)

No, I would suggest adding an addEndpoint() method.

This method has been implemented at the VpcNetwork level (see addInterfaceEndpoint and addGatewayEndpoint)

In order to go forward with this PR, could you have a look at the latest changes from today and tell me what you would like to be adapted?

rix0rrr · 2019-04-08T13:16:58Z

This is not always guaranteed by construction, see the following test case I wrote

Oh, you are right of course, for some reason I was thinking about selecting subnets by name and missed this case. In any case, I still think the solution is to put the selection logic inside VpcNetwork. Maybe add something like a oneSubnetPerAz?: boolean to the selection.

This method has been implemented at the VpcNetwork level (see addInterfaceEndpoint and addGatewayEndpoint)

Well that is perfect. I only went down this discussion because of the following exchange:

For the route tables, I don't think you need to get the route tables out, do you?

We need to specify the route table ids in AWS::EC2::VPCEndpoint

Your response made me assume we needed the route table IDs outside the VPC construct. If route table IDs are an implementation detail used inside the VPC to implement the functions you mentioned before, we don't have a problem at all.

could you have a look at the latest changes from today

Will do.

Explicitly deal with the fact that not all IVpcSubnets are VpcSubnets.

rix0rrr · 2019-04-08T14:15:49Z

How about the change that I just pushed? I think it's an okay compromise. All I ask is that we treat the subnets as one group of things, to keep the API surface small and not rely too much on implementation details. It does deprecate selectSubnetIds() again, which is a thread I didn't pull all the way through.

Also added dealing with the case where the VpcSubnets would be imported, which wasn't deal with before as far as I could see, all IVpcSubnets were cast to VpcSubnets and their routeTableId attribute read, but that cast was not necessarily valid.

rix0rrr · 2019-04-08T14:17:11Z

Right now I did the oneSubnetPerAz: true trick to make sure that worked out correctly, which magically restricts the selection. If that is undesirable we can turn it into an assertion as well (throw if the selection matches multiple subnet groups).

jogold · 2019-04-08T15:57:15Z

@rix0rrr added a fix to onePerAz because it was overwritten in reifySelectionDefaults.

Do you want to leave the calls to selectSubnetIds in the other packages? Renaming subnetIds to selectSubnetIds is already a breaking change so we are actually deprecating something that did not exist...

rix0rrr · 2019-04-08T16:31:55Z

Do you want to leave the calls to selectSubnetIds in the other packages

If you're volunteering to update them, not at all :)

rix0rrr · 2019-04-08T16:32:13Z

Also thanks for fixing my hasty updates

jogold · 2019-04-08T17:36:04Z

Do you want to leave the calls to selectSubnetIds in the other packages

If you're volunteering to update them, not at all :)

Will update, this also means that subnetIds gets removed (not deprecated) from aws-ec2, right?

feat(ec2): add support for vpc endpoints

9052eb6

Add support for both gateway and interface VPC endpoints. Static members are exposed for all AWS service endpoints. As gateway endpoints reference route tables, they currently cannot be added to imported VPC networks.

jogold requested a review from a team as a code owner March 27, 2019 15:43

jogold added 2 commits March 28, 2019 09:21

Merge branch 'master' into vpc-endpoint

3e794db

Add readonly

746507e

RomainMuller reviewed Mar 28, 2019

View reviewed changes

jogold added 2 commits March 28, 2019 11:42

Fix typo

2a29faa

Convert VpcEndpointService to interface

a8e827e

RomainMuller added @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud gap labels Mar 28, 2019

eladb reviewed Mar 28, 2019

View reviewed changes

jogold added 3 commits March 28, 2019 22:04

IVpcEndpointService, JSDoc, README and integ test

65293e4

Merge branch 'master' into vpc-endpoint

3a3f896

jogold requested a review from SoManyHs as a code owner March 28, 2019 22:57

jogold added 6 commits March 29, 2019 10:51

Default port range and naming

625c5f4

Add policy support for interface endpoints

b07e2cb

Fix build

5c3e0d2

Merge branch 'master' into vpc-endpoint

a6f47c5

Merge branch 'master' into vpc-endpoint

5293965

Integrate codebuild vpc support

1688c67

jogold requested a review from skinny85 as a code owner April 1, 2019 13:24

eladb reviewed Apr 3, 2019

View reviewed changes

jogold added 2 commits April 3, 2019 09:53

Rename creationTimestamp to vpcEndpointCreationTimestamp

af46e05

Deprecate subnetIds

a3978aa

rix0rrr requested changes Apr 3, 2019

View reviewed changes

jogold added 2 commits April 8, 2019 13:05

Use subnetSelection[] and selectSubnetIds

8101b6b

Merge branch 'master' into vpc-endpoint

fd17299

Reinstate selectSubnets() but return a SOA instead of an AOS

f3811d6

Explicitly deal with the fact that not all IVpcSubnets are VpcSubnets.

jogold added 3 commits April 8, 2019 17:05

Fix calls to subnetInternetDependencies

394cd4e

Error message

c0fcd02

Fix onePerAz

6651dae

jogold added 2 commits April 8, 2019 19:45

Add test for gateway endpoint with imported VPC

5f883b8

Remove calls to selectSubnetIds

f83acd9

rix0rrr approved these changes Apr 9, 2019

View reviewed changes

rix0rrr merged commit bbb3f34 into aws:master Apr 9, 2019

jogold deleted the vpc-endpoint branch April 9, 2019 09:59

jogold mentioned this pull request Apr 14, 2019

VPC Endpoints of type Gateway #773

Closed

jogold mentioned this pull request Jul 31, 2019

Adding VPC Gateway Endpoint to VPC created fromLookup fails #3472

Closed

5 tasks

This was referenced Dec 12, 2019

[Snyk] Upgrade @aws-cdk/cdk from 0.22.0 to 0.35.0 MechanicalRock/scdk#3

Closed

[Snyk] Upgrade @aws-cdk/cdk from 0.24.1 to 0.35.0 MechanicalRock/cdk-constructs#8

Open

[Snyk] Upgrade @aws-cdk/cdk from 0.24.1 to 0.35.0 MechanicalRock/account-reaper#6

Open

feat(ec2): add support for vpc endpoints #2104

feat(ec2): add support for vpc endpoints #2104

Conversation

jogold commented Mar 27, 2019 • edited Loading

Pull Request Checklist

RomainMuller left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RomainMuller Mar 28, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jogold commented Mar 29, 2019 • edited Loading

jogold commented Apr 1, 2019 • edited Loading

eladb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rix0rrr commented Apr 3, 2019

rix0rrr commented Apr 3, 2019

rix0rrr left a comment

Choose a reason for hiding this comment

jogold commented Apr 3, 2019 • edited Loading

jogold commented Apr 8, 2019

rix0rrr commented Apr 8, 2019

rix0rrr commented Apr 8, 2019

rix0rrr commented Apr 8, 2019 • edited Loading

jogold commented Apr 8, 2019

rix0rrr commented Apr 8, 2019 • edited Loading

jogold commented Apr 8, 2019

rix0rrr commented Apr 8, 2019 • edited Loading

rix0rrr commented Apr 8, 2019

rix0rrr commented Apr 8, 2019

jogold commented Apr 8, 2019

rix0rrr commented Apr 8, 2019

rix0rrr commented Apr 8, 2019

jogold commented Apr 8, 2019

jogold commented Mar 27, 2019 •

edited

Loading

RomainMuller Mar 28, 2019 •

edited

Loading

jogold commented Mar 29, 2019 •

edited

Loading

jogold commented Apr 1, 2019 •

edited

Loading

jogold commented Apr 3, 2019 •

edited

Loading

rix0rrr commented Apr 8, 2019 •

edited

Loading

rix0rrr commented Apr 8, 2019 •

edited

Loading

rix0rrr commented Apr 8, 2019 •

edited

Loading