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

feat(iot-actions): add SNS publish action #18839

Merged
merged 19 commits into from
Feb 9, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
22 changes: 22 additions & 0 deletions packages/@aws-cdk/aws-iot-actions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Currently supported are:
- Put records to Kinesis Data stream
- Put records to Kinesis Data Firehose stream
- Send messages to SQS queues
- Publish messages on SNS topics

## Republish a message to another MQTT topic

Expand Down Expand Up @@ -256,3 +257,24 @@ const topicRule = new iot.TopicRule(this, 'TopicRule', {
],
});
```

## Publish messages on an SNS topic

The code snippet below creates and AWS IoT Rule that publishes messages to an SNS topic when it is triggered:

```ts
import * as sns from '@aws-cdk/aws-sns';

const topic = new sns.Topic(this, 'MyTopic');

const topicRule = new iot.TopicRule(this, 'TopicRule', {
sql: iot.IotSql.fromStringAsVer20160323(
"SELECT topic(2) as device_id, year, month, day FROM 'device/+/data'",
),
actions: [
new actions.SnsTopicAction(topic, {
messageFormat: actions.SnsActionMessageFormat.JSON, // optional property, default is SnsActionMessageFormat.RAW
}),
],
});
```
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-iot-actions/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@ export * from './kinesis-put-record-action';
export * from './lambda-function-action';
export * from './s3-put-object-action';
export * from './sqs-queue-action';
export * from './sns-topic-action';
75 changes: 75 additions & 0 deletions packages/@aws-cdk/aws-iot-actions/lib/sns-topic-action.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import * as iam from '@aws-cdk/aws-iam';
import * as iot from '@aws-cdk/aws-iot';
import * as sns from '@aws-cdk/aws-sns';
import { CommonActionProps } from '.';
import { singletonActionRole } from './private/role';

/**
* SNS topic action message format options.
*/
export enum SnsActionMessageFormat {
/**
* RAW message format.
*/
RAW = 'RAW',

/**
* JSON message format.
*/
JSON = 'JSON'
}

/**
* Configuration options for the SNS topic action.
*/
export interface SnsTopicActionProps extends CommonActionProps {
/**
* The message format of the message to publish.
*
* SNS uses this setting to determine if the payload should be parsed and relevant platform-specific bits of the payload should be extracted.
* @see https://docs.aws.amazon.com/sns/latest/dg/sns-message-and-json-formats.html
*
* @default SnsActionMessageFormat.RAW
*/
readonly messageFormat?: SnsActionMessageFormat;
}

/**
* The action to write the data from an MQTT message to an Amazon SNS topic.
*
* @see https://docs.aws.amazon.com/iot/latest/developerguide/sns-rule-action.html
*/
export class SnsTopicAction implements iot.IAction {
private readonly role?: iam.IRole;
private readonly topic: sns.ITopic;
private readonly messageFormat?: SnsActionMessageFormat;

/**
* @param topic The Amazon SNS topic to publish data on. Must not be a FIFO topic.
* @param props Properties to configure the action.
*/
constructor(topic: sns.ITopic, props: SnsTopicActionProps = {}) {
if (topic.fifo) {
throw Error('IoT Rule actions cannot be used with FIFO SNS Topics, please pass a non-FIFO Topic instead');
}

this.topic = topic;
this.role = props.role;
this.messageFormat = props.messageFormat;
}

bind(rule: iot.ITopicRule): iot.ActionConfig {
Copy link
Contributor Author

@AdamVD AdamVD Feb 5, 2022

Choose a reason for hiding this comment

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

Limitation here is we don't check if the SNS topic is encrypted and adjust the role or KMS resource policy accordingly. ITopic doesn't expose encryption details so I'm not sure how we'd perform this check (unlike IQueue.encryptionMasterKey). The current SQS queue action has the same limitation.

Copy link
Contributor

Choose a reason for hiding this comment

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

What if we used the grantPublish() method of ITopic, instead of creating the Policy directly?

If that method doesn't handle permissions to the Key correctly, then that's a bug in the SNS library!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right, I should use the grantPublish() method. Unfortunately, it looks like this is another limitation of the current SNS topic implementation. The grant method doesn't take encryption into account.

From topic-base.ts:

  /**
   * Grant topic publishing permissions to the given identity
   */
  public grantPublish(grantee: iam.IGrantable) {
    return iam.Grant.addToPrincipalOrResource({
      grantee,
      actions: ['sns:Publish'],
      resourceArns: [this.topicArn],
      resource: this,
    });
  }

Compare this against queue-base.ts:

  public grantSendMessages(grantee: iam.IGrantable) {
    const ret = this.grant(grantee,
      'sqs:SendMessage',
      'sqs:GetQueueAttributes',
      'sqs:GetQueueUrl');

    if (this.encryptionMasterKey) {
      // kms:Decrypt necessary to execute grantsendMessages to an SSE enabled SQS queue
      this.encryptionMasterKey.grantEncryptDecrypt(grantee);
    }
    return ret;
  }

This is concisely described in #18387, and can be assumed to affect all other SNS topic integrations (e.g. #16271, #11121) unless a separate workaround was implemented. I'd be happy to look into this and and try to bring it in-line with what SQS queues do, but that probably shouldn't be a part of this PR. What do you think @skinny85?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

With that said, changing this implementation to use grantPublish() means that no further changes will be required once #18387 is fixed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with everything you said 🙂. Ideally, this would be a separate PR that only touches the SNS module, but if you'd rather do it all in one, because that will be easier for you - go for it. You're fixing bugs in our project, I won't be too harsh 😉.

const role = this.role ?? singletonActionRole(rule);
this.topic.grantPublish(role);

return {
configuration: {
sns: {
targetArn: this.topic.topicArn,
roleArn: role.roleArn,
messageFormat: this.messageFormat,
},
},
};
}
}
2 changes: 2 additions & 0 deletions packages/@aws-cdk/aws-iot-actions/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
"@aws-cdk/aws-lambda": "0.0.0",
"@aws-cdk/aws-logs": "0.0.0",
"@aws-cdk/aws-s3": "0.0.0",
"@aws-cdk/aws-sns": "0.0.0",
"@aws-cdk/aws-sqs": "0.0.0",
"@aws-cdk/core": "0.0.0",
"case": "1.6.3",
Expand All @@ -110,6 +111,7 @@
"@aws-cdk/aws-lambda": "0.0.0",
"@aws-cdk/aws-logs": "0.0.0",
"@aws-cdk/aws-s3": "0.0.0",
"@aws-cdk/aws-sns": "0.0.0",
"@aws-cdk/aws-sqs": "0.0.0",
"@aws-cdk/core": "0.0.0",
"constructs": "^3.3.69"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
{
"Resources": {
"TopicRule40A4EA44": {
"Type": "AWS::IoT::TopicRule",
"Properties": {
"TopicRulePayload": {
"Actions": [
{
"Sns": {
"RoleArn": {
"Fn::GetAtt": [
"TopicRuleTopicRuleActionRole246C4F77",
"Arn"
]
},
"TargetArn": {
"Ref": "MyTopic86869434"
}
}
}
],
"AwsIotSqlVersion": "2016-03-23",
"Sql": "SELECT topic(2) as device_id, year, month, day FROM 'device/+/data'"
}
}
},
"TopicRuleTopicRuleActionRole246C4F77": {
"Type": "AWS::IAM::Role",
"Properties": {
"AssumeRolePolicyDocument": {
"Statement": [
{
"Action": "sts:AssumeRole",
"Effect": "Allow",
"Principal": {
"Service": "iot.amazonaws.com"
}
}
],
"Version": "2012-10-17"
}
}
},
"TopicRuleTopicRuleActionRoleDefaultPolicy99ADD687": {
"Type": "AWS::IAM::Policy",
"Properties": {
"PolicyDocument": {
"Statement": [
{
"Action": "sns:Publish",
"Effect": "Allow",
"Resource": {
"Ref": "MyTopic86869434"
}
}
],
"Version": "2012-10-17"
},
"PolicyName": "TopicRuleTopicRuleActionRoleDefaultPolicy99ADD687",
"Roles": [
{
"Ref": "TopicRuleTopicRuleActionRole246C4F77"
}
]
}
},
"MyTopic86869434": {
"Type": "AWS::SNS::Topic"
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
* Stack verification steps:
* * aws sns subscribe --topic-arn "arn:aws:sns:<region>:<account>:test-stack-MyTopic86869434-10F6E3DMK3E5P" --protocol email --notification-endpoint <email-addr>
* * confirm subscription from email
* * echo '{"message": "hello world"}' > testfile.txt
* * aws iot-data publish --topic device/mydevice/data --qos 1 --payload fileb://testfile.txt
* * verify that an email was sent from the SNS
* * rm testfile.txt
*/
/// !cdk-integ pragma:ignore-assets
import * as iot from '@aws-cdk/aws-iot';
import * as sns from '@aws-cdk/aws-sns';
import * as cdk from '@aws-cdk/core';
import * as actions from '../../lib';

class TestStack extends cdk.Stack {
constructor(scope: cdk.App, id: string, props?: cdk.StackProps) {
super(scope, id, props);

const topicRule = new iot.TopicRule(this, 'TopicRule', {
sql: iot.IotSql.fromStringAsVer20160323(
"SELECT topic(2) as device_id, year, month, day FROM 'device/+/data'",
),
});

const snsTopic = new sns.Topic(this, 'MyTopic');
topicRule.addAction(new actions.SnsTopicAction(snsTopic));
}
}

const app = new cdk.App();
new TestStack(app, 'sns-topic-action-test-stack');
app.synth();
103 changes: 103 additions & 0 deletions packages/@aws-cdk/aws-iot-actions/test/sns/sns-topic-action.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
import { Match, Template } from '@aws-cdk/assertions';
import * as iam from '@aws-cdk/aws-iam';
import * as iot from '@aws-cdk/aws-iot';
import * as sns from '@aws-cdk/aws-sns';
import * as cdk from '@aws-cdk/core';
import * as actions from '../../lib';

const SNS_TOPIC_ARN = 'arn:aws:sns::123456789012:test-topic';

let stack: cdk.Stack;
let topicRule: iot.TopicRule;
let snsTopic: sns.ITopic;

beforeEach(() => {
stack = new cdk.Stack();
topicRule = new iot.TopicRule(stack, 'MyTopicRule', {
sql: iot.IotSql.fromStringAsVer20160323("SELECT topic(2) as device_id FROM 'device/+/data'"),
});
snsTopic = sns.Topic.fromTopicArn(stack, 'MySnsTopic', SNS_TOPIC_ARN);
});

test('Default SNS topic action', () => {
// WHEN
topicRule.addAction(new actions.SnsTopicAction(snsTopic));

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::IoT::TopicRule', {
TopicRulePayload: {
Actions: [{
Sns: {
RoleArn: { 'Fn::GetAtt': ['MyTopicRuleTopicRuleActionRoleCE2D05DA', 'Arn'] },
TargetArn: SNS_TOPIC_ARN,
},
}],
},
});

Template.fromStack(stack).hasResourceProperties('AWS::IAM::Role', {
AssumeRolePolicyDocument: {
Statement: [{
Action: 'sts:AssumeRole',
Effect: 'Allow',
Principal: { Service: 'iot.amazonaws.com' },
}],
},
});

Template.fromStack(stack).hasResourceProperties('AWS::IAM::Policy', {
PolicyDocument: {
Statement: [{
Action: 'sns:Publish',
Effect: 'Allow',
Resource: SNS_TOPIC_ARN,
}],
},
Roles: [{ Ref: 'MyTopicRuleTopicRuleActionRoleCE2D05DA' }],
});
});

test('Can set messageFormat', () => {
// WHEN
topicRule.addAction(new actions.SnsTopicAction(snsTopic, {
messageFormat: actions.SnsActionMessageFormat.JSON,
}));

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::IoT::TopicRule', {
TopicRulePayload: {
Actions: [
Match.objectLike({ Sns: { MessageFormat: 'JSON' } }),
],
},
});
});

test('Can set role', () => {
// GIVEN
const roleArn = 'arn:aws:iam::123456789012:role/testrole';
const role = iam.Role.fromRoleArn(stack, 'MyRole', roleArn);

// WHEN
topicRule.addAction(new actions.SnsTopicAction(snsTopic, {
role,
}));

// THEN
Template.fromStack(stack).hasResourceProperties('AWS::IoT::TopicRule', {
TopicRulePayload: {
Actions: [
Match.objectLike({ Sns: { RoleArn: roleArn } }),
],
},
});
});

test('Action with FIFO topic throws error', () => {
// GIVEN
const snsFifoTopic = sns.Topic.fromTopicArn(stack, 'MyFifoTopic', `${SNS_TOPIC_ARN}.fifo`);

expect(() => {
topicRule.addAction(new actions.SnsTopicAction(snsFifoTopic));
}).toThrowError('IoT Rule actions cannot be used with FIFO SNS Topics, please pass a non-FIFO Topic instead');
});
9 changes: 9 additions & 0 deletions packages/@aws-cdk/aws-sns/lib/topic-base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ export interface ITopic extends IResource, notifications.INotificationRuleTarget
*/
readonly topicName: string;

/**
* Whether this topic is an Amazon SNS FIFO queue. If false, this is a standard topic.
*
* @attribute
*/
readonly fifo: boolean;

/**
* Subscribe some endpoint to this topic
*/
Expand Down Expand Up @@ -56,6 +63,8 @@ export abstract class TopicBase extends Resource implements ITopic {

public abstract readonly topicName: string;

public abstract readonly fifo: boolean;

/**
* Controls automatic creation of policy objects.
*
Expand Down
3 changes: 3 additions & 0 deletions packages/@aws-cdk/aws-sns/lib/topic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export class Topic extends TopicBase {
class Import extends TopicBase {
public readonly topicArn = topicArn;
public readonly topicName = Stack.of(scope).splitArn(topicArn, ArnFormat.NO_RESOURCE_NAME).resource;
public readonly fifo = this.topicName.endsWith('.fifo');
protected autoCreatePolicy: boolean = false;
}

Expand All @@ -72,6 +73,7 @@ export class Topic extends TopicBase {

public readonly topicArn: string;
public readonly topicName: string;
public readonly fifo: boolean;

protected readonly autoCreatePolicy: boolean = true;

Expand Down Expand Up @@ -110,5 +112,6 @@ export class Topic extends TopicBase {
resource: this.physicalName,
});
this.topicName = this.getResourceNameAttribute(resource.attrTopicName);
this.fifo = props.fifo || false;
}
}
Loading