Skip to content

Commit

Permalink
fix(logs): move log destinations into 'aws-logs-destinations' (#2655)
Browse files Browse the repository at this point in the history
In accordance with new guidelines, we're centralizing cross-service
integrations into their own package. In this case, centralizing
CloudWatch Logs Subscription Destinations into
@aws-cdk/aws-logs-destinations.

Fixes #2444.

BREAKING CHANGE: using a Lambda or Kinesis Stream as CloudWatch log subscription destination now requires an integration object from the `@aws-cdk/aws-logs-destinations` package.
  • Loading branch information
rix0rrr authored and Elad Ben-Israel committed May 30, 2019
1 parent 38a9894 commit 01601c2
Show file tree
Hide file tree
Showing 22 changed files with 5,516 additions and 237 deletions.
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.

26 changes: 1 addition & 25 deletions packages/@aws-cdk/aws-lambda/lib/function-base.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,13 @@
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 { IResource, Resource } from '@aws-cdk/cdk';
import { IEventSource } from './event-source';
import { EventSourceMapping, EventSourceMappingOptions } from './event-source-mapping';
import { CfnPermission } from './lambda.generated';
import { Permission } from './permission';

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

/**
* Logical ID of this Function.
Expand Down Expand Up @@ -156,11 +154,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 @@ -251,23 +244,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 };
}

/**
* Adds an event source to this function.
*
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

0 comments on commit 01601c2

Please sign in to comment.