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.

Sign up for GitHub

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: update all 'onXxx' methods to be CloudWatch Events #2609

Merged
merged 6 commits into from
May 23, 2019
+1,091 −286
Merged

fix: update all 'onXxx' methods to be CloudWatch Events #2609

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4908eb0
fix: update all 'onXxx' methods to be CloudWatch Events
rix0rrr May 22, 2019
048333f
Merge remote-tracking branch 'origin/master' into huijbers/events-on-…
rix0rrr May 23, 2019
432fa2f
Fix awslint
rix0rrr May 23, 2019
ecf226f
Merge remote-tracking branch 'origin/master' into huijbers/events-on-…
rix0rrr May 23, 2019
482d15e
Fix config uses
rix0rrr May 23, 2019
3be7222
Update readme
rix0rrr May 23, 2019
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
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-applicationautoscaling/lib/step-scaling-policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export class StepScalingPolicy extends cdk.Construct {
evaluationPeriods: 1,
threshold,
});
this.lowerAlarm.onAlarm(this.lowerAction);
this.lowerAlarm.addAlarmAction(this.lowerAction);
}

if (alarms.upperAlarmIntervalIndex !== undefined) {
Expand Down Expand Up @@ -138,7 +138,7 @@ export class StepScalingPolicy extends cdk.Construct {
evaluationPeriods: 1,
threshold,
});
this.upperAlarm.onAlarm(this.upperAction);
this.upperAlarm.addAlarmAction(this.upperAction);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-autoscaling/lib/auto-scaling-group.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ abstract class AutoScalingGroupBase extends Resource implements IAutoScalingGrou
/**
* Send a message to either an SQS queue or SNS topic when instances launch or terminate
*/
public onLifecycleTransition(id: string, props: BasicLifecycleHookProps): LifecycleHook {
public addLifecycleHook(id: string, props: BasicLifecycleHookProps): LifecycleHook {
return new LifecycleHook(this, `LifecycleHook${id}`, {
autoScalingGroup: this,
...props
Expand Down Expand Up @@ -692,7 +692,7 @@ export interface IAutoScalingGroup extends IResource {
/**
* Send a message to either an SQS queue or SNS topic when instances launch or terminate
*/
onLifecycleTransition(id: string, props: BasicLifecycleHookProps): LifecycleHook;
addLifecycleHook(id: string, props: BasicLifecycleHookProps): LifecycleHook;

/**
* Scale out or in based on time
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-autoscaling/lib/step-scaling-policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ export class StepScalingPolicy extends cdk.Construct {
evaluationPeriods: 1,
threshold,
});
this.lowerAlarm.onAlarm(this.lowerAction);
this.lowerAlarm.addAlarmAction(this.lowerAction);
}

if (alarms.upperAlarmIntervalIndex !== undefined) {
Expand Down Expand Up @@ -139,7 +139,7 @@ export class StepScalingPolicy extends cdk.Construct {
evaluationPeriods: 1,
threshold,
});
this.upperAlarm.onAlarm(this.upperAction);
this.upperAlarm.addAlarmAction(this.upperAction);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-autoscaling/test/test.lifecyclehooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ export = {
});

// WHEN
asg.onLifecycleTransition('Transition', {
asg.addLifecycleHook('Transition', {
notificationTarget: new FakeNotificationTarget(),
lifecycleTransition: autoscaling.LifecycleTransition.InstanceLaunching,
defaultResult: autoscaling.DefaultResult.Abandon,
Expand Down
68 changes: 68 additions & 0 deletions packages/@aws-cdk/aws-cloudtrail/SAMPLE-EVENTS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# Some sample CloudTrail events

For reference.


## S3

PutObject

{
"eventSource": "s3.amazonaws.com",
"resources": [
{
"ARN": "arn:aws:s3:::BUCKETNAME/OBJECTKEY",
"type": "AWS::S3::Object"
},
{
"accountId": "123456789012",
"ARN": "arn:aws:s3:::BUCKETNAME",
"type": "AWS::S3::Bucket"
}
],
"eventTime": "2019-05-22T08:38:05Z",
"userAgent": "[aws-cli/1.16.96 Python/2.7.12 Linux/4.4.0-146-generic botocore/1.12.86]",
"readOnly": false,
"recipientAccountId": "123456789012",
"awsRegion": "eu-west-1",
"requestID": "CF9748DFDC5FB0A4",
"additionalEventData": {
"CipherSuite": "ECDHE-RSA-AES128-GCM-SHA256",
"bytesTransferredOut": 0,
"AuthenticationMethod": "AuthHeader",
"x-amz-id-2": "hRJMAs5p4ALZIabP4ATIL53npWU61+N6LYWj02gdQtR0ymKSySzVXUSZx7ydv7tRJwk+XMaPerM=",
"bytesTransferredIn": 197,
"SignatureVersion": "SigV4"
},
"eventType": "AwsApiCall",
"eventID": "3074546e-1bfa-4973-8502-b1bb4d0bda1a",
"eventVersion": "1.05",
"eventName": "PutObject",
"sourceIPAddress": "1.2.3.4",
"userIdentity": {
"accountId": "123456789012",
"type": "AssumedRole",
"principalId": "AROAJBNCAL3UTR5C42U4M:user-SomeRole",
"accessKeyId": "AZYCAIJERO6H7",
"sessionContext": {
"attributes": {
"mfaAuthenticated": "false",
"creationDate": "2019-05-22T08:10:21Z"
},
"sessionIssuer": {
"accountId": "123456789012",
"type": "Role",
"principalId": "AROAJBNCAL3UTR5C42U4M",
"userName": "SomeRole",
"arn": "arn:aws:iam::123456789012:role/SomeRole"
}
},
"arn": "arn:aws:sts::123456789012:assumed-role/SomeRole/user-SomeRole"
},
"responseElements": null,
"requestParameters": {
"bucketName": "BUCKETNAME",
"Host": "BUCKETNAME.s3.eu-west-1.amazonaws.com",
"key": "OBJECTKEY"
}
}
14 changes: 8 additions & 6 deletions packages/@aws-cdk/aws-cloudtrail/lib/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,14 +212,16 @@ export class Trail extends Resource {
}

/**
* Create an event rule for when an event is recorded by any trail.
* Create an event rule for when an event is recorded by any Trail in the account.
*
* Note that the event doesn't necessarily have to come from this
* trail. Be sure to filter the event properly using an event pattern.
* Note that the event doesn't necessarily have to come from this Trail, it can
* be captured from any one.
*
* Be sure to filter the event further down using an event pattern.
*/
public onEvent(name: string, target?: events.IRuleTarget, options?: events.RuleProps) {
const rule = new events.Rule(this, name, options);
rule.addTarget(target);
public onCloudTrailEvent(id: string, options: events.OnEventOptions): events.Rule {
const rule = new events.Rule(this, id, options);
rule.addTarget(options.target);
rule.addEventPattern({
detailType: ['AWS API Call via CloudTrail']
});
Expand Down
12 changes: 7 additions & 5 deletions packages/@aws-cdk/aws-cloudtrail/test/test.cloudtrail.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,11 +197,13 @@ export = {
const trail = new Trail(stack, 'MyAmazingCloudTrail', { managementEvents: ReadWriteType.WriteOnly });

// WHEN
trail.onEvent('DoEvents', {
bind: () => ({
arn: 'arn',
id: 'myid'
})
trail.onCloudTrailEvent('DoEvents', {
target: {
bind: () => ({
arn: 'arn',
id: 'myid'
})
}
});

// THEN
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-cloudwatch/lib/alarm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export class Alarm extends Resource implements IAlarm {
*
* Typically the ARN of an SNS topic or ARN of an AutoScaling policy.
*/
public onAlarm(...actions: IAlarmAction[]) {
public addAlarmAction(...actions: IAlarmAction[]) {
if (this.alarmActionArns === undefined) {
this.alarmActionArns = [];
}
Expand All @@ -168,7 +168,7 @@ export class Alarm extends Resource implements IAlarm {
*
* Typically the ARN of an SNS topic or ARN of an AutoScaling policy.
*/
public onInsufficientData(...actions: IAlarmAction[]) {
public addInsufficientDataAction(...actions: IAlarmAction[]) {
if (this.insufficientDataActionArns === undefined) {
this.insufficientDataActionArns = [];
}
Expand All @@ -181,7 +181,7 @@ export class Alarm extends Resource implements IAlarm {
*
* Typically the ARN of an SNS topic or ARN of an AutoScaling policy.
*/
public onOk(...actions: IAlarmAction[]) {
public addOkAction(...actions: IAlarmAction[]) {
if (this.okActionArns === undefined) {
this.okActionArns = [];
}
Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-cloudwatch/test/test.alarm.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,9 +72,9 @@ export = {
evaluationPeriods: 2
});

alarm.onAlarm(new TestAlarmAction('A'));
alarm.onInsufficientData(new TestAlarmAction('B'));
alarm.onOk(new TestAlarmAction('C'));
alarm.addAlarmAction(new TestAlarmAction('A'));
alarm.addInsufficientDataAction(new TestAlarmAction('B'));
alarm.addOkAction(new TestAlarmAction('C'));

// THEN
expect(stack).to(haveResource('AWS::CloudWatch::Alarm', {
Expand Down
68 changes: 39 additions & 29 deletions packages/@aws-cdk/aws-codebuild/lib/project.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,13 @@ export interface IProject extends IResource, iam.IGrantable {
/** The IAM service Role of this Project. Undefined for imported Projects. */
readonly role?: iam.IRole;

/**
* Defines a CloudWatch event rule triggered when something happens with this project.
*
* @see https://docs.aws.amazon.com/codebuild/latest/userguide/sample-build-notifications.html
*/
onEvent(id: string, options: events.OnEventOptions): events.Rule;

/**
* Defines a CloudWatch event rule triggered when the build project state
* changes. You can filter specific build status events using an event
Expand All @@ -57,30 +64,30 @@ export interface IProject extends IResource, iam.IGrantable {
*
* @see https://docs.aws.amazon.com/codebuild/latest/userguide/sample-build-notifications.html
*/
onStateChange(name: string, target?: events.IRuleTarget, options?: events.RuleProps): events.Rule;
onStateChange(id: string, options: events.OnEventOptions): events.Rule;

/**
* Defines a CloudWatch event rule that triggers upon phase change of this
* build project.
*
* @see https://docs.aws.amazon.com/codebuild/latest/userguide/sample-build-notifications.html
*/
onPhaseChange(name: string, target?: events.IRuleTarget, options?: events.RuleProps): events.Rule;
onPhaseChange(id: string, options: events.OnEventOptions): events.Rule;

/**
* Defines an event rule which triggers when a build starts.
*/
onBuildStarted(name: string, target?: events.IRuleTarget, options?: events.RuleProps): events.Rule;
onBuildStarted(id: string, options: events.OnEventOptions): events.Rule;

/**
* Defines an event rule which triggers when a build fails.
*/
onBuildFailed(name: string, target?: events.IRuleTarget, options?: events.RuleProps): events.Rule;
onBuildFailed(id: string, options: events.OnEventOptions): events.Rule;

/**
* Defines an event rule which triggers when a build completes successfully.
*/
onBuildSucceeded(name: string, target?: events.IRuleTarget, options?: events.RuleProps): events.Rule;
onBuildSucceeded(id: string, options: events.OnEventOptions): events.Rule;

/**
* @returns a CloudWatch metric associated with this build project.
Expand Down Expand Up @@ -157,6 +164,23 @@ abstract class ProjectBase extends Resource implements IProject {
/** The IAM service Role of this Project. */
public abstract readonly role?: iam.IRole;

/**
* Defines a CloudWatch event rule triggered when something happens with this project.
*
* @see https://docs.aws.amazon.com/codebuild/latest/userguide/sample-build-notifications.html
*/
public onEvent(id: string, options: events.OnEventOptions): events.Rule {
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess I missed this in the previous PR, but it's now or never: I kind of think that "target" shouldn't be under "Options", so I can do onEvent('foo', target).

It also maps more cleanly to the fact that you pass in options to events.Rule and then addTarget separately.

I would also say that events.RuleProps should contain targets (plural) to allow adding targets declaratively...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was bivaricating on that. I found the API very ugly that way.

Hmm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, here's why I changed the signature: the signature with 3 arguments becomes very ugly if you have a complex target (such as an ECS task or CodeBuild project), or a complex payload (from a JSON message), and then also configuration after it.

It would look something like this:

source.onEvent('MyEvent', rule.addTarget(new targets.EcsEc2Task({
  cluster,
  taskDefinition,
  taskCount: 1,
  containerOverrides: [{
    containerName: 'TheContainer',
    environment: [{
      name: 'I_WAS_TRIGGERED',
      value: 'From CloudWatch Events'
    }]
  }]
}), {
  eventDescription: 'hello'
  eventPattern: {
    field: ['value']
  }
});

vs:

source.onEvent('MyEvent', {
  eventDescription: 'hello'
  eventPattern: {
    field: ['value']
  },
  target: new targets.EcsEc2Task({
    cluster,
    taskDefinition,
    taskCount: 1,
    containerOverrides: [{
        containerName: 'TheContainer',
        environment: [{
        name: 'I_WAS_TRIGGERED',
        value: 'From CloudWatch Events'
        }]
    }]
  })
});

I feel the bad case is a lot worse for the first API, where the 2nd API fixes that, and the regular case isn't THAT much worse.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, I think obj.method('id', { ...props... }) is more consistent with most of our other APIs.

const rule = new events.Rule(this, id, options);
rule.addTarget(options.target);
rule.addEventPattern({
source: ['aws.codebuild'],
detail: {
'project-name': [this.projectName]
}
});
return rule;
}

/**
* Defines a CloudWatch event rule triggered when the build project state
* changes. You can filter specific build status events using an event
Expand All @@ -182,17 +206,10 @@ abstract class ProjectBase extends Resource implements IProject {
*
* @see https://docs.aws.amazon.com/codebuild/latest/userguide/sample-build-notifications.html
*/
public onStateChange(name: string, target?: events.IRuleTarget, options?: events.RuleProps) {
const rule = new events.Rule(this, name, options);
rule.addTarget(target);
public onStateChange(id: string, options: events.OnEventOptions) {
const rule = this.onEvent(id, options);
rule.addEventPattern({
source: ['aws.codebuild'],
detailType: ['CodeBuild Build State Change'],
detail: {
'project-name': [
this.projectName
]
}
});
return rule;
}
Expand All @@ -203,17 +220,10 @@ abstract class ProjectBase extends Resource implements IProject {
*
* @see https://docs.aws.amazon.com/codebuild/latest/userguide/sample-build-notifications.html
*/
public onPhaseChange(name: string, target?: events.IRuleTarget, options?: events.RuleProps) {
const rule = new events.Rule(this, name, options);
rule.addTarget(target);
public onPhaseChange(id: string, options: events.OnEventOptions) {
const rule = this.onEvent(id, options);
rule.addEventPattern({
source: ['aws.codebuild'],
detailType: ['CodeBuild Build Phase Change'],
detail: {
'project-name': [
this.projectName
]
}
});
return rule;
}
Expand All @@ -224,8 +234,8 @@ abstract class ProjectBase extends Resource implements IProject {
* To access fields from the event in the event target input,
* use the static fields on the `StateChangeEvent` class.
*/
public onBuildStarted(name: string, target?: events.IRuleTarget, options?: events.RuleProps) {
const rule = this.onStateChange(name, target, options);
public onBuildStarted(id: string, options: events.OnEventOptions) {
const rule = this.onStateChange(id, options);
rule.addEventPattern({
detail: {
'build-status': ['IN_PROGRESS']
Expand All @@ -240,8 +250,8 @@ abstract class ProjectBase extends Resource implements IProject {
* To access fields from the event in the event target input,
* use the static fields on the `StateChangeEvent` class.
*/
public onBuildFailed(name: string, target?: events.IRuleTarget, options?: events.RuleProps) {
const rule = this.onStateChange(name, target, options);
public onBuildFailed(id: string, options: events.OnEventOptions) {
const rule = this.onStateChange(id, options);
rule.addEventPattern({
detail: {
'build-status': ['FAILED']
Expand All @@ -256,8 +266,8 @@ abstract class ProjectBase extends Resource implements IProject {
* To access fields from the event in the event target input,
* use the static fields on the `StateChangeEvent` class.
*/
public onBuildSucceeded(name: string, target?: events.IRuleTarget, options?: events.RuleProps) {
const rule = this.onStateChange(name, target, options);
public onBuildSucceeded(id: string, options: events.OnEventOptions) {
const rule = this.onStateChange(id, options);
rule.addEventPattern({
detail: {
'build-status': ['SUCCEEDED']
Expand Down
10 changes: 5 additions & 5 deletions packages/@aws-cdk/aws-codebuild/test/test.codebuild.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -973,11 +973,11 @@ export = {
source: new codebuild.CodePipelineSource()
});

project.onBuildFailed('OnBuildFailed');
project.onBuildSucceeded('OnBuildSucceeded');
project.onPhaseChange('OnPhaseChange');
project.onStateChange('OnStateChange');
project.onBuildStarted('OnBuildStarted');
project.onBuildFailed('OnBuildFailed', { target: { bind: () => ({ arn: 'ARN', id: 'ID' }) }});
project.onBuildSucceeded('OnBuildSucceeded', { target: { bind: () => ({ arn: 'ARN', id: 'ID' }) }});
project.onPhaseChange('OnPhaseChange', { target: { bind: () => ({ arn: 'ARN', id: 'ID' }) }});
project.onStateChange('OnStateChange', { target: { bind: () => ({ arn: 'ARN', id: 'ID' }) }});
project.onBuildStarted('OnBuildStarted', { target: { bind: () => ({ arn: 'ARN', id: 'ID' }) }});

expect(stack).to(haveResource('AWS::Events::Rule', {
"EventPattern": {
Expand Down
Loading