-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
CloudWatch Logs: new library #307
Merged
Merged
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
352cc82
CloudWatch Logs: new library
8ad77fe
Address review comments
41baec6
Import library as namespace for all cross-library imports
bf6bc4c
Addressing more review comments
d1fc7af
Fix broken import paths
c2d2727
Review comments
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
import { Construct, Output, PolicyStatement, Token } from '@aws-cdk/core'; | ||
import { IIdentityResource } from '@aws-cdk/iam'; | ||
import { Arn, AwsRegion, Construct, FnConcat, HashedAddressingScheme, Output, | ||
PolicyStatement, ServicePrincipal, Stack, Token } from '@aws-cdk/core'; | ||
import { IIdentityResource, Role } from '@aws-cdk/iam'; | ||
import * as kms from '@aws-cdk/kms'; | ||
import logs = require('@aws-cdk/logs'); | ||
import { cloudformation, StreamArn } from './kinesis.generated'; | ||
|
||
/** | ||
|
@@ -37,7 +39,7 @@ export interface StreamRefProps { | |
* StreamRef.import(this, 'MyImportedStream', ref); | ||
* | ||
*/ | ||
export abstract class StreamRef extends Construct { | ||
export abstract class StreamRef extends Construct implements logs.ILogSubscriptionDestination { | ||
/** | ||
* Creates a Stream construct that represents an external stream. | ||
* | ||
|
@@ -55,11 +57,21 @@ export abstract class StreamRef extends Construct { | |
*/ | ||
public abstract readonly streamArn: StreamArn; | ||
|
||
/** | ||
* The name of the stream | ||
*/ | ||
public abstract readonly streamName: StreamName; | ||
|
||
/** | ||
* Optional KMS encryption key associated with this stream. | ||
*/ | ||
public abstract readonly encryptionKey?: kms.EncryptionKeyRef; | ||
|
||
/** | ||
* The role that can be used by CloudWatch logs to write to this stream | ||
*/ | ||
private cloudWatchLogsRole?: Role; | ||
|
||
/** | ||
* Exports this stream from the stack. | ||
*/ | ||
|
@@ -159,6 +171,61 @@ export abstract class StreamRef extends Construct { | |
); | ||
} | ||
|
||
public logSubscriptionDestination(sourceLogGroup: logs.LogGroup): 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 Role(this, 'CloudWatchLogsCanPutRecords', { | ||
assumedBy: new ServicePrincipal(new FnConcat('logs.', new AwsRegion(), '.amazonaws.com')), | ||
}); | ||
this.cloudWatchLogsRole.addToPolicy(new PolicyStatement().addAction('kinesis:PutRecord').addResource(this.streamArn)); | ||
this.cloudWatchLogsRole.addToPolicy(new 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 = Stack.find(sourceLogGroup); | ||
const thisStack = Stack.find(this); | ||
|
||
// 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 { arn: this.streamArn, role: this.cloudWatchLogsRole }; | ||
} | ||
|
||
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'); | ||
} | ||
|
||
return this.crossAccountLogSubscriptionDestination(sourceLogGroup); | ||
} | ||
|
||
/** | ||
* Generate a CloudWatch Logs Destination and return the properties in the form o a subscription destination | ||
*/ | ||
private crossAccountLogSubscriptionDestination(sourceLogGroup: logs.LogGroup): logs.LogSubscriptionDestination { | ||
const sourceStack = Stack.find(sourceLogGroup); | ||
|
||
// 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([sourceLogGroup.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 PolicyStatement() | ||
.addAction('logs:PutSubscriptionFilter') | ||
.addAwsAccountPrincipal(sourceStack.env.account) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if account is undefined? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Then we assume both accounts will be undefined and the If one is undefined and the other is not, we're very much SOL. |
||
.addAllResources()); | ||
|
||
return dest.logSubscriptionDestination(sourceLogGroup); | ||
} | ||
|
||
private grant(identity: IIdentityResource, actions: { streamActions: string[], keyActions: string[] }) { | ||
identity.addToPolicy(new PolicyStatement() | ||
.addResource(this.streamArn) | ||
|
@@ -307,12 +374,16 @@ export class StreamName extends Token {} | |
|
||
class ImportedStreamRef extends StreamRef { | ||
public readonly streamArn: StreamArn; | ||
public readonly streamName: StreamName; | ||
public readonly encryptionKey?: kms.EncryptionKeyRef; | ||
|
||
constructor(parent: Construct, name: string, props: StreamRefProps) { | ||
super(parent, name); | ||
|
||
this.streamArn = props.streamArn; | ||
// Get the name from the ARN | ||
this.streamName = Arn.parseToken(props.streamArn).resourceName; | ||
|
||
if (props.encryptionKey) { | ||
this.encryptionKey = kms.EncryptionKeyRef.import(parent, 'Key', props.encryptionKey); | ||
} else { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
80 changes: 80 additions & 0 deletions
80
packages/@aws-cdk/kinesis/test/test.subscriptiondestination.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,80 @@ | ||
import { expect, haveResource } from '@aws-cdk/assert'; | ||
import { Stack } from '@aws-cdk/core'; | ||
import { FilterPattern, LogGroup, SubscriptionFilter } from '@aws-cdk/logs'; | ||
import { Test } from 'nodeunit'; | ||
import { Stream } from '../lib'; | ||
|
||
export = { | ||
'stream can be subscription destination'(test: Test) { | ||
// GIVEN | ||
const stack = new Stack(); | ||
const stream = new Stream(stack, 'MyStream'); | ||
const logGroup = new LogGroup(stack, 'LogGroup'); | ||
|
||
// WHEN | ||
new SubscriptionFilter(stack, 'Subscription', { | ||
logGroup, | ||
destination: stream, | ||
filterPattern: FilterPattern.allEvents() | ||
}); | ||
|
||
// THEN: subscription target is Stream | ||
expect(stack).to(haveResource('AWS::Logs::SubscriptionFilter', { | ||
DestinationArn: { "Fn::GetAtt": [ "MyStream5C050E93", "Arn" ] }, | ||
RoleArn: { "Fn::GetAtt": [ "MyStreamCloudWatchLogsCanPutRecords58498490", "Arn" ] }, | ||
})); | ||
|
||
// THEN: we have a role to write to the Lambda | ||
expect(stack).to(haveResource('AWS::IAM::Role', { | ||
AssumeRolePolicyDocument: { | ||
Statement: [{ | ||
Action: "sts:AssumeRole", | ||
Principal: { Service: { "Fn::Join": ["", ["logs.", {Ref: "AWS::Region"}, ".amazonaws.com"]] }} | ||
}], | ||
} | ||
})); | ||
|
||
expect(stack).to(haveResource('AWS::IAM::Policy', { | ||
PolicyDocument: { | ||
Statement: [ | ||
{ | ||
Action: "kinesis:PutRecord", | ||
Effect: "Allow", | ||
Resource: { "Fn::GetAtt": [ "MyStream5C050E93", "Arn" ] } | ||
}, | ||
{ | ||
Action: "iam:PassRole", | ||
Effect: "Allow", | ||
Resource: { "Fn::GetAtt": [ "MyStreamCloudWatchLogsCanPutRecords58498490", "Arn" ] } | ||
} | ||
], | ||
} | ||
})); | ||
|
||
test.done(); | ||
}, | ||
|
||
'cross-account stream can be subscription destination with Destination'(test: Test) { | ||
// GIVEN | ||
const sourceStack = new Stack(undefined, undefined, { env: { account: '12345' }}); | ||
const logGroup = new LogGroup(sourceStack, 'LogGroup'); | ||
|
||
const destStack = new Stack(undefined, undefined, { env: { account: '67890' }}); | ||
const stream = new Stream(destStack, 'MyStream'); | ||
|
||
// WHEN | ||
new SubscriptionFilter(sourceStack, 'Subscription', { | ||
logGroup, | ||
destination: stream, | ||
filterPattern: FilterPattern.allEvents() | ||
}); | ||
|
||
// THEN: the source stack has a Destination object that the subscription points to | ||
expect(destStack).to(haveResource('AWS::Logs::Destination', { | ||
TargetArn: { "Fn::GetAtt": [ "MyStream5C050E93", "Arn" ] }, | ||
RoleArn: { "Fn::GetAtt": [ "MyStreamCloudWatchLogsCanPutRecords58498490", "Arn" ] }, | ||
})); | ||
|
||
test.done(); | ||
} | ||
}; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify (and maybe worth a comment in the code): will this Just Work across regions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will never work across regions, I don't think. There are very few things that work across regions.
In fact, this code will also not work today--but it will magically start working once we implement cross-stack references!