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(logs): move log destinations into 'aws-logs-destinations' #2655

Merged
merged 9 commits into from
May 30, 2019
Merged
Show file tree
Hide file tree
Changes from 3 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
70 changes: 2 additions & 68 deletions packages/@aws-cdk/aws-kinesis/lib/stream.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import iam = require('@aws-cdk/aws-iam');
import kms = require('@aws-cdk/aws-kms');
import logs = require('@aws-cdk/aws-logs');
import { Construct, HashedAddressingScheme, IResource, Resource } from '@aws-cdk/cdk';
import { Construct, IResource, Resource } from '@aws-cdk/cdk';
import { CfnStream } from './kinesis.generated';

export interface IStream extends IResource, logs.ILogSubscriptionDestination {
export interface IStream extends IResource {
/**
* The ARN of the stream.
*
Expand Down Expand Up @@ -102,11 +101,6 @@ abstract class StreamBase extends Resource implements IStream {
*/
public abstract readonly encryptionKey?: kms.IKey;

/**
* The role that can be used by CloudWatch logs to write to this stream
*/
private cloudWatchLogsRole?: iam.Role;

/**
* Grant write permissions for this stream and its contents to an IAM
* principal (Role/Group/User).
Expand Down Expand Up @@ -164,66 +158,6 @@ abstract class StreamBase extends Resource implements IStream {
return ret;
}

public logSubscriptionDestination(sourceLogGroup: logs.ILogGroup): logs.LogSubscriptionDestination {
// Following example from https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/SubscriptionFilters.html#DestinationKinesisExample
if (!this.cloudWatchLogsRole) {
// Create a role to be assumed by CWL that can write to this stream and pass itself.
this.cloudWatchLogsRole = new iam.Role(this, 'CloudWatchLogsCanPutRecords', {
assumedBy: new iam.ServicePrincipal(`logs.${this.node.stack.region}.amazonaws.com`)
});
this.cloudWatchLogsRole.addToPolicy(new iam.PolicyStatement().addAction('kinesis:PutRecord').addResource(this.streamArn));
this.cloudWatchLogsRole.addToPolicy(new iam.PolicyStatement().addAction('iam:PassRole').addResource(this.cloudWatchLogsRole.roleArn));
}

// We've now made it possible for CloudWatch events to write to us. In case the LogGroup is in a
// different account, we must add a Destination in between as well.
const sourceStack = sourceLogGroup.node.stack;
const thisStack = this.node.stack;

// Case considered: if both accounts are undefined, we can't make any assumptions. Better
// to assume we don't need to do anything special.
const sameAccount = sourceStack.env.account === thisStack.env.account;

if (!sameAccount) {
return this.crossAccountLogSubscriptionDestination(sourceLogGroup);
}

return { arn: this.streamArn, role: this.cloudWatchLogsRole };
}

/**
* Generate a CloudWatch Logs Destination and return the properties in the form o a subscription destination
*/
private crossAccountLogSubscriptionDestination(sourceLogGroup: logs.ILogGroup): logs.LogSubscriptionDestination {
const sourceLogGroupConstruct: Construct = sourceLogGroup as any;
const sourceStack = sourceLogGroupConstruct.node.stack;
const thisStack = this.node.stack;

if (!sourceStack.env.account || !thisStack.env.account) {
throw new Error('SubscriptionFilter stack and Destination stack must either both have accounts defined, or both not have accounts');
}

// Take some effort to construct a unique ID for the destination that is unique to the
// combination of (stream, loggroup).
const uniqueId = new HashedAddressingScheme().allocateAddress([
sourceLogGroupConstruct.node.path.replace('/', ''),
sourceStack.env.account!
]);

// The destination lives in the target account
const dest = new logs.CrossAccountDestination(this, `CWLDestination${uniqueId}`, {
targetArn: this.streamArn,
role: this.cloudWatchLogsRole!
});

dest.addToPolicy(new iam.PolicyStatement()
.addAction('logs:PutSubscriptionFilter')
.addAwsAccountPrincipal(sourceStack.env.account)
.addAllResources());

return dest.logSubscriptionDestination(sourceLogGroup);
}

private grant(grantee: iam.IGrantable, ...actions: string[]) {
return iam.Grant.addToPrincipal({
grantee,
Expand Down
13 changes: 13 additions & 0 deletions packages/@aws-cdk/aws-kinesis/test/test.stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,19 @@ export = {

test.done();
},

'stream from attributes'(test: Test) {
const stack = new Stack();

const s = Stream.fromStreamAttributes(stack, 'MyStream', {
streamArn: 'arn:aws:kinesis:region:account-id:stream/stream-name'
});

test.equals(s.streamArn, 'arn:aws:kinesis:region:account-id:stream/stream-name');

test.done();
},

"uses explicit shard count"(test: Test) {
const stack = new Stack();

Expand Down
83 changes: 0 additions & 83 deletions packages/@aws-cdk/aws-kinesis/test/test.subscriptiondestination.ts

This file was deleted.

2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-lambda-event-sources/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
"@aws-cdk/aws-kinesis": "^0.32.0",
"@aws-cdk/aws-lambda": "^0.32.0",
"@aws-cdk/aws-s3": "^0.32.0",
"@aws-cdk/aws-s3-notifications": "^0.32.0",
"@aws-cdk/aws-sns": "^0.32.0",
"@aws-cdk/aws-sqs": "^0.32.0",
"@aws-cdk/cdk": "^0.32.0"
Expand All @@ -82,6 +83,7 @@
"@aws-cdk/aws-kinesis": "^0.32.0",
"@aws-cdk/aws-lambda": "^0.32.0",
"@aws-cdk/aws-s3": "^0.32.0",
"@aws-cdk/aws-s3-notifications": "^0.32.0",
"@aws-cdk/aws-sns": "^0.32.0",
"@aws-cdk/aws-sqs": "^0.32.0",
"@aws-cdk/cdk": "^0.32.0"
Expand Down
25 changes: 1 addition & 24 deletions packages/@aws-cdk/aws-lambda/lib/function-base.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import cloudwatch = require('@aws-cdk/aws-cloudwatch');
import ec2 = require('@aws-cdk/aws-ec2');
import iam = require('@aws-cdk/aws-iam');
import logs = require('@aws-cdk/aws-logs');
import s3n = require('@aws-cdk/aws-s3-notifications');
import cdk = require('@aws-cdk/cdk');
import { IResource, Resource } from '@aws-cdk/cdk';
Expand All @@ -10,7 +9,7 @@ import { EventSourceMapping, EventSourceMappingOptions } from './event-source-ma
import { CfnPermission } from './lambda.generated';
import { Permission } from './permission';

export interface IFunction extends IResource, logs.ILogSubscriptionDestination,
export interface IFunction extends IResource,
s3n.IBucketNotificationDestination, ec2.IConnectable, iam.IGrantable {

/**
Expand Down Expand Up @@ -158,11 +157,6 @@ export abstract class FunctionBase extends Resource implements IFunction {
*/
protected _connections?: ec2.Connections;

/**
* Indicates if the policy that allows CloudWatch logs to publish to this lambda has been added.
*/
private logSubscriptionDestinationPolicyAddedFor: string[] = [];

/**
* Adds a permission to the Lambda resource policy.
* @param id The id ƒor the permission construct
Expand Down Expand Up @@ -253,23 +247,6 @@ export abstract class FunctionBase extends Resource implements IFunction {
});
}

public logSubscriptionDestination(sourceLogGroup: logs.ILogGroup): logs.LogSubscriptionDestination {
const arn = sourceLogGroup.logGroupArn;

if (this.logSubscriptionDestinationPolicyAddedFor.indexOf(arn) === -1) {
// NOTE: the use of {AWS::Region} limits this to the same region, which shouldn't really be an issue,
// since the Lambda must be in the same region as the SubscriptionFilter anyway.
//
// (Wildcards in principals are unfortunately not supported.
this.addPermission('InvokedByCloudWatchLogs', {
principal: new iam.ServicePrincipal(`logs.${this.node.stack.region}.amazonaws.com`),
sourceArn: arn
});
this.logSubscriptionDestinationPolicyAddedFor.push(arn);
}
return { arn: this.functionArn };
}

/**
* Allows this Lambda to be used as a destination for bucket notifications.
* Use `bucket.onEvent(lambda)` to subscribe.
Expand Down
44 changes: 0 additions & 44 deletions packages/@aws-cdk/aws-lambda/test/test.subscriptiondestination.ts

This file was deleted.

16 changes: 16 additions & 0 deletions packages/@aws-cdk/aws-logs-destinations/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
*.js
tsconfig.json
tslint.json
*.js.map
*.d.ts
*.generated.ts
dist
lib/generated/resources.ts
.jsii

.LAST_BUILD
.nyc_output
coverage
.nycrc
.LAST_PACKAGE
*.snk
18 changes: 18 additions & 0 deletions packages/@aws-cdk/aws-logs-destinations/.npmignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Don't include original .ts files when doing `npm pack`
*.ts
!*.d.ts
coverage
.nyc_output
*.tgz

dist
.LAST_PACKAGE
.LAST_BUILD
!*.js

# Include .jsii
!.jsii

*.snk

*.tsbuildinfo
Loading