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(events): AwsApi warns if service does not exist #13352

Merged
merged 9 commits into from Jun 4, 2021
2 changes: 2 additions & 0 deletions package.json
Expand Up @@ -97,6 +97,8 @@
"@aws-cdk/core/minimatch/**",
"@aws-cdk/cx-api/semver",
"@aws-cdk/cx-api/semver/**",
"@aws-cdk/aws-events-targets/aws-sdk",
"@aws-cdk/aws-events-targets/aws-sdk/**",
"@aws-cdk/yaml-cfn/yaml",
"@aws-cdk/yaml-cfn/yaml/**",
"aws-cdk-lib/@balena/dockerignore",
Expand Down
15 changes: 15 additions & 0 deletions packages/@aws-cdk/aws-events-targets/lib/aws-api.ts
Expand Up @@ -2,6 +2,7 @@ import * as path from 'path';
import * as events from '@aws-cdk/aws-events';
import * as iam from '@aws-cdk/aws-iam';
import * as lambda from '@aws-cdk/aws-lambda';
import { Annotations } from '@aws-cdk/core';
import { metadata } from './sdk-api-metadata.generated';
import { addLambdaPermission } from './util';

Expand Down Expand Up @@ -89,6 +90,8 @@ export class AwsApi implements events.IRuleTarget {
lambdaPurpose: 'AWS',
});

checkServiceExists(this.props.service, handler);

if (this.props.policyStatement) {
handler.addToRolePolicy(this.props.policyStatement);
} else {
Expand Down Expand Up @@ -117,6 +120,18 @@ export class AwsApi implements events.IRuleTarget {
}
}

/**
* Check if the given service exists in the AWS SDK. If not, a warning will be raised.
* @param service Service name
*/
function checkServiceExists(service: string, handler: lambda.SingletonFunction) {
const sdkService = awsSdkMetadata[service.toLowerCase()];
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we sure this is correct for services like SQS and such? I'm not too familiar with the contents of this map.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This map is generated in buildtools/gen.js. It is built from https://github.com/aws/aws-sdk-js/blob/master/apis/metadata.json

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ping @rix0rrr

if (!sdkService) {
Annotations.of(handler).addWarning(`Service ${service} does not exist in the AWS SDK. Check the list of available \
services and actions from https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/index.html`);
}
}

/**
* Transform SDK service/action to IAM action using metadata from aws-sdk module.
*/
Expand Down
38 changes: 32 additions & 6 deletions packages/@aws-cdk/aws-events-targets/test/aws-api/aws-api.test.ts
@@ -1,4 +1,4 @@
import { countResources, expect, haveResource } from '@aws-cdk/assert-internal';
import { countResources, expect as cdkExpect, haveResource, SynthUtils } from '@aws-cdk/assert-internal';
import * as events from '@aws-cdk/aws-events';
import * as iam from '@aws-cdk/aws-iam';
import { Stack } from '@aws-cdk/core';
Expand Down Expand Up @@ -32,7 +32,7 @@ test('use AwsApi as an event rule target', () => {
}));

// THEN
expect(stack).to(haveResource('AWS::Events::Rule', {
cdkExpect(stack).to(haveResource('AWS::Events::Rule', {
Targets: [
{
Arn: {
Expand Down Expand Up @@ -73,9 +73,9 @@ test('use AwsApi as an event rule target', () => {
}));

// Uses a singleton function
expect(stack).to(countResources('AWS::Lambda::Function', 1));
cdkExpect(stack).to(countResources('AWS::Lambda::Function', 1));

expect(stack).to(haveResource('AWS::IAM::Policy', {
cdkExpect(stack).to(haveResource('AWS::IAM::Policy', {
PolicyDocument: {
Statement: [
{
Expand Down Expand Up @@ -112,7 +112,7 @@ test('with policy statement', () => {
}));

// THEN
expect(stack).to(haveResource('AWS::Events::Rule', {
cdkExpect(stack).to(haveResource('AWS::Events::Rule', {
Targets: [
{
Arn: {
Expand All @@ -130,7 +130,7 @@ test('with policy statement', () => {
],
}));

expect(stack).to(haveResource('AWS::IAM::Policy', {
cdkExpect(stack).to(haveResource('AWS::IAM::Policy', {
PolicyDocument: {
Statement: [
{
Expand All @@ -143,3 +143,29 @@ test('with policy statement', () => {
},
}));
});

test('with service not in AWS SDK', () => {
// GIVEN
const stack = new Stack();
const rule = new events.Rule(stack, 'Rule', {
schedule: events.Schedule.expression('rate(15 minutes)'),
});
const awsApi = new targets.AwsApi({
service: 'no-such-service',
action: 'no-such-action',
policyStatement: new iam.PolicyStatement({
actions: ['s3:GetObject'],
resources: ['resource'],
}),
});

// WHEN
rule.addTarget(awsApi);

// THEN
const assembly = SynthUtils.synthesize(stack);
expect(assembly.messages.length).toBe(1);
const message = assembly.messages[0];
expect(message.entry.type).toBe('aws:cdk:warning');
expect(message.entry.data).toBe('Service no-such-service does not exist in the AWS SDK. Check the list of available services and actions from https://docs.aws.amazon.com/AWSJavaScriptSDK/latest/index.html');
});