aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.68k stars 3.93k forks source link

(aws_events_targets) : CDK Stack incorrectly creates a "Lambda & Service role" when only "CloudWatchLogGroup" target is added. #25024

Closed agentgill closed 1 year ago

agentgill commented 1 year ago

Describe the bug

CDK Stack incorrectly creates a "Lambda & Service role" when only "CloudWatchLogGroup" target is added.

A lambda function and service role are created by CDK deploy with no trigger and target, however these resources should not be created and are not needed.

Example CDK Snippet - this code should only create an s3 bucket and an event bridge rule based on s3 notifications, create a target for CloudWatchLogGroup

from aws_cdk import (
    Stack,
    aws_s3 as s3,
    aws_events as events,
    aws_events_targets as targets,
    aws_logs as logs,
)
from constructs import Construct

class CdkTestCleanupStack(Stack):
    def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        log_group = logs.LogGroup(
            self,
            "s3_notifications_log",
        )

        bucket = s3.Bucket(
            self,
            "med_892",
            block_public_access=s3.BlockPublicAccess.BLOCK_ALL,
            encryption=s3.BucketEncryption.S3_MANAGED,
            event_bridge_enabled=False,
            versioned=True,
        )

        rule = events.Rule(
            self,
            "s3_notifications_rule",
            description="Send s3 Notitications to CloudWatch",
            event_pattern=events.EventPattern(
                source=["aws.s3"],
                detail_type=["AWS API Call via CloudTrail"],
                detail={
                    "eventSource": ["s3.amazonaws.com"],
                    "eventName": ["PutObject"],
                    "requestParameters": {
                        "bucketName": [bucket.bucket_name],
                    },
                },
            ),
            targets=None,
        )

        rule.add_target(targets.CloudWatchLogGroup(log_group))

Expected Behavior

Following resources are expected to be created:

Current Behavior

Following resources are to be created:

Reproduction Steps

Create new CDK 2 project using Python 2.73.0 (build 43e681e)

Use the following CDK code

from aws_cdk import (
    Stack,
    aws_s3 as s3,
    aws_events as events,
    aws_events_targets as targets,
    aws_logs as logs,
)
from constructs import Construct

class CdkTestCleanupStack(Stack):
    def __init__(self, scope: Construct, construct_id: str, **kwargs) -> None:
        super().__init__(scope, construct_id, **kwargs)

        log_group = logs.LogGroup(
            self,
            "s3_notifications_log",
        )

        bucket = s3.Bucket(
            self,
            "med_892",
            block_public_access=s3.BlockPublicAccess.BLOCK_ALL,
            encryption=s3.BucketEncryption.S3_MANAGED,
            event_bridge_enabled=False,
            versioned=True,
        )

        rule = events.Rule(
            self,
            "s3_notifications_rule",
            description="Send s3 Notitications to CloudWatch",
            event_pattern=events.EventPattern(
                source=["aws.s3"],
                detail_type=["AWS API Call via CloudTrail"],
                detail={
                    "eventSource": ["s3.amazonaws.com"],
                    "eventName": ["PutObject"],
                    "requestParameters": {
                        "bucketName": [bucket.bucket_name],
                    },
                },
            ),
            targets=None,
        )

        rule.add_target(targets.CloudWatchLogGroup(log_group))

Possible Solution

This creates unintended resources which are surplus to requirements and not used. Deleting any surplus resources creates drift issues which could be ignore, but none-the-less these leaves a CDK managed project in an unintended state.

if the resources are deleted as not needed it creates a CDK Deploy issue as resources are missing

Additional Information/Context

No response

CDK CLI Version

2.73.0 (build 43e681e)

Framework Version

Python

Node.js Version

v16.19.1

OS

Mac OS Venture 13.3.1

Language

Python

Language Version

3.11.2

Other information

No response

hoegertn commented 1 year ago

Could you please provide the lambda function and role that are created? I assume it is a custom resource to manage the LogGroup.

agentgill commented 1 year ago

Could you please provide the lambda function and role that are created? I assume it is a custom resource to manage the LogGroup.

Here is the Lambda function code (node 14) and policy at the bottom

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.handler = exports.forceSdkInstallation = exports.flatten = exports.PHYSICAL_RESOURCE_ID_REFERENCE = void 0;
/* eslint-disable no-console */
const child_process_1 = require("child_process");
const fs = require("fs");
const path_1 = require("path");
/**
 * Serialized form of the physical resource id for use in the operation parameters
 */
exports.PHYSICAL_RESOURCE_ID_REFERENCE = 'PHYSICAL:RESOURCEID:';
/**
 * Flattens a nested object
 *
 * @param object the object to be flattened
 * @returns a flat object with path as keys
 */
function flatten(object) {
    return Object.assign({}, ...function _flatten(child, path = []) {
        return [].concat(...Object.keys(child)
            .map(key => {
            const childKey = Buffer.isBuffer(child[key]) ? child[key].toString('utf8') : child[key];
            return typeof childKey === 'object' && childKey !== null
                ? _flatten(childKey, path.concat([key]))
                : ({ [path.concat([key]).join('.')]: childKey });
        }));
    }(object));
}
exports.flatten = flatten;
/**
 * Decodes encoded special values (physicalResourceId)
 */
function decodeSpecialValues(object, physicalResourceId) {
    return JSON.parse(JSON.stringify(object), (_k, v) => {
        switch (v) {
            case exports.PHYSICAL_RESOURCE_ID_REFERENCE:
                return physicalResourceId;
            default:
                return v;
        }
    });
}
/**
 * Filters the keys of an object.
 */
function filterKeys(object, pred) {
    return Object.entries(object)
        .reduce((acc, [k, v]) => pred(k)
        ? { ...acc, [k]: v }
        : acc, {});
}
let latestSdkInstalled = false;
function forceSdkInstallation() {
    latestSdkInstalled = false;
}
exports.forceSdkInstallation = forceSdkInstallation;
/**
 * Installs latest AWS SDK v2
 */
function installLatestSdk() {
    console.log('Installing latest AWS SDK v2');
    // Both HOME and --prefix are needed here because /tmp is the only writable location
    (0, child_process_1.execSync)('HOME=/tmp npm install aws-sdk@2 --production --no-package-lock --no-save --prefix /tmp');
    latestSdkInstalled = true;
}
// no currently patched services
const patchedServices = [];
/**
 * Patches the AWS SDK by loading service models in the same manner as the actual SDK
 */
function patchSdk(awsSdk) {
    const apiLoader = awsSdk.apiLoader;
    patchedServices.forEach(({ serviceName, apiVersions }) => {
        const lowerServiceName = serviceName.toLowerCase();
        if (!awsSdk.Service.hasService(lowerServiceName)) {
            apiLoader.services[lowerServiceName] = {};
            awsSdk[serviceName] = awsSdk.Service.defineService(lowerServiceName, apiVersions);
        }
        else {
            awsSdk.Service.addVersions(awsSdk[serviceName], apiVersions);
        }
        apiVersions.forEach(apiVersion => {
            Object.defineProperty(apiLoader.services[lowerServiceName], apiVersion, {
                get: function get() {
                    const modelFilePrefix = `aws-sdk-patch/${lowerServiceName}-${apiVersion}`;
                    const model = JSON.parse(fs.readFileSync((0, path_1.join)(__dirname, `${modelFilePrefix}.service.json`), 'utf-8'));
                    model.paginators = JSON.parse(fs.readFileSync((0, path_1.join)(__dirname, `${modelFilePrefix}.paginators.json`), 'utf-8')).pagination;
                    return model;
                },
                enumerable: true,
                configurable: true,
            });
        });
    });
    return awsSdk;
}
/* eslint-disable @typescript-eslint/no-require-imports, import/no-extraneous-dependencies */
async function handler(event, context) {
    try {
        let AWS;
        if (!latestSdkInstalled && event.ResourceProperties.InstallLatestAwsSdk === 'true') {
            try {
                installLatestSdk();
                AWS = require('/tmp/node_modules/aws-sdk');
            }
            catch (e) {
                console.log(`Failed to install latest AWS SDK v2: ${e}`);
                AWS = require('aws-sdk'); // Fallback to pre-installed version
            }
        }
        else if (latestSdkInstalled) {
            AWS = require('/tmp/node_modules/aws-sdk');
        }
        else {
            AWS = require('aws-sdk');
        }
        try {
            AWS = patchSdk(AWS);
        }
        catch (e) {
            console.log(`Failed to patch AWS SDK: ${e}. Proceeding with the installed copy.`);
        }
        console.log(JSON.stringify({ ...event, ResponseURL: '...' }));
        console.log('AWS SDK VERSION: ' + AWS.VERSION);
        event.ResourceProperties.Create = decodeCall(event.ResourceProperties.Create);
        event.ResourceProperties.Update = decodeCall(event.ResourceProperties.Update);
        event.ResourceProperties.Delete = decodeCall(event.ResourceProperties.Delete);
        // Default physical resource id
        let physicalResourceId;
        switch (event.RequestType) {
            case 'Create':
                physicalResourceId = event.ResourceProperties.Create?.physicalResourceId?.id ??
                    event.ResourceProperties.Update?.physicalResourceId?.id ??
                    event.ResourceProperties.Delete?.physicalResourceId?.id ??
                    event.LogicalResourceId;
                break;
            case 'Update':
            case 'Delete':
                physicalResourceId = event.ResourceProperties[event.RequestType]?.physicalResourceId?.id ?? event.PhysicalResourceId;
                break;
        }
        let flatData = {};
        let data = {};
        const call = event.ResourceProperties[event.RequestType];
        if (call) {
            let credentials;
            if (call.assumedRoleArn) {
                const timestamp = (new Date()).getTime();
                const params = {
                    RoleArn: call.assumedRoleArn,
                    RoleSessionName: `${timestamp}-${physicalResourceId}`.substring(0, 64),
                };
                credentials = new AWS.ChainableTemporaryCredentials({
                    params: params,
                    stsConfig: { stsRegionalEndpoints: 'regional' },
                });
            }
            if (!Object.prototype.hasOwnProperty.call(AWS, call.service)) {
                throw Error(`Service ${call.service} does not exist in AWS SDK version ${AWS.VERSION}.`);
            }
            const awsService = new AWS[call.service]({
                apiVersion: call.apiVersion,
                credentials: credentials,
                region: call.region,
            });
            try {
                const response = await awsService[call.action](call.parameters && decodeSpecialValues(call.parameters, physicalResourceId)).promise();
                flatData = {
                    apiVersion: awsService.config.apiVersion,
                    region: awsService.config.region,
                    ...flatten(response),
                };
                let outputPaths;
                if (call.outputPath) {
                    outputPaths = [call.outputPath];
                }
                else if (call.outputPaths) {
                    outputPaths = call.outputPaths;
                }
                if (outputPaths) {
                    data = filterKeys(flatData, startsWithOneOf(outputPaths));
                }
                else {
                    data = flatData;
                }
            }
            catch (e) {
                if (!call.ignoreErrorCodesMatching || !new RegExp(call.ignoreErrorCodesMatching).test(e.code)) {
                    throw e;
                }
            }
            if (call.physicalResourceId?.responsePath) {
                physicalResourceId = flatData[call.physicalResourceId.responsePath];
            }
        }
        await respond('SUCCESS', 'OK', physicalResourceId, data);
    }
    catch (e) {
        console.log(e);
        await respond('FAILED', e.message || 'Internal Error', context.logStreamName, {});
    }
    function respond(responseStatus, reason, physicalResourceId, data) {
        const responseBody = JSON.stringify({
            Status: responseStatus,
            Reason: reason,
            PhysicalResourceId: physicalResourceId,
            StackId: event.StackId,
            RequestId: event.RequestId,
            LogicalResourceId: event.LogicalResourceId,
            NoEcho: false,
            Data: data,
        });
        console.log('Responding', responseBody);
        // eslint-disable-next-line @typescript-eslint/no-require-imports
        const parsedUrl = require('url').parse(event.ResponseURL);
        const requestOptions = {
            hostname: parsedUrl.hostname,
            path: parsedUrl.path,
            method: 'PUT',
            headers: {
                'content-type': '',
                'content-length': Buffer.byteLength(responseBody, 'utf8'),
            },
        };
        return new Promise((resolve, reject) => {
            try {
                // eslint-disable-next-line @typescript-eslint/no-require-imports
                const request = require('https').request(requestOptions, resolve);
                request.on('error', reject);
                request.write(responseBody);
                request.end();
            }
            catch (e) {
                reject(e);
            }
        });
    }
}
exports.handler = handler;
function decodeCall(call) {
    if (!call) {
        return undefined;
    }
    return JSON.parse(call);
}
function startsWithOneOf(searchStrings) {
    return function (string) {
        for (const searchString of searchStrings) {
            if (string.startsWith(searchString)) {
                return true;
            }
        }
        return false;
    };
}
//# sourceMappingURL=data:application/json;base64

Role has AWS Managed Policy - [AWSLambdaBasicExecutionRole] Customer Inline Policy is this

{
    "Version": "2012-10-17",
    "Statement": [
        {
            "Action": [
                "logs:DeleteResourcePolicy",
                "logs:PutResourcePolicy"
            ],
            "Resource": "*",
            "Effect": "Allow"
        }
    ]
}
pahud commented 1 year ago

When you add a CloudWatchLogGroup as your event target with this:

rule.add_target(targets.CloudWatchLogGroup(log_group))

Under the covers, a LogGroupResourcePolicy custom resource will be created for you which contains a lambda function and an IAM role generated for this function.

agentgill commented 1 year ago

@pahud thanks for details here, if I am correct this is not an normal operational lambda, but only for assisting with the standing up of Log Group Resources. It's surplus to my normal service operation. I do wonder if there is way to get more control over this resource within the CDK project directly versus just having it be created by some background custom service. Feels a bit like anti-pattern, but that could be me. Thanks for responding.

peterwoodworth commented 1 year ago

Custom resources are a pattern offered by CloudFormation which allow us to implement functionalities that might be buggy or nonexistent on CloudFormation's end (at least at the time we created them). Our custom resources are considered stable solutions in our non-alpha libraries, so I wouldn't be hesitant to use them just due to the appearance of them 🙂

If you wish to adjust CDK's implementation of it, I can only recommend dropping down to the l1 layer if possible, or implementing your own custom resource. It's more trouble than it's worth to try to modify the custom resource we provide

You can read more about the custom resource framework here if you're interested. tl;dr Providers (in the form of a lambda function typically) will be ready to receive a payload during stack creation, update or delete. This payload will specify whether the custom resource should be created, updated, or deleted, and the custom resource will probably be configured to do something different depending on which type of event it receives. This is basically how official Cfn resources work too, except that isn't being done as part of a lambda function in your own account

agentgill commented 1 year ago

@peterwoodworth thanks for sharing this info, greatly appreciated

peterwoodworth commented 1 year ago

No problem @agentgill, ping me if you have any further questions 🙂

github-actions[bot] commented 1 year ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.