Skip to content

fix(lambda): improve validation errors for lambda functions #32323

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

Merged
merged 2 commits into from
Dec 7, 2024
Merged

fix(lambda): improve validation errors for lambda functions #32323

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f2088b9
feat: improve validation errors for lambda functions
mrgrain Nov 29, 2024
3958233
Merge branch 'main' into mrgrain/feat/improved-validation-errors-lambda
mergify[bot] Dec 7, 2024
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
87 changes: 44 additions & 43 deletions packages/aws-cdk-lib/aws-lambda/lib/function.ts
View file
Open in desktop
Copy link
Contributor

@kaizencc kaizencc Dec 6, 2024

Choose a reason for hiding this comment

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

is this fix only in lambda.function as a POC for now and can/should eventually replace our errors all over aws-cdk?

Copy link
Contributor Author

@mrgrain mrgrain Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, this is a soft-launch POC. I'll add a few more modules later and than towards mid January we can roll this out wholesale (a lot of grunt work).

Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import * as logs from '../../aws-logs';
import * as sns from '../../aws-sns';
import * as sqs from '../../aws-sqs';
import { Annotations, ArnFormat, CfnResource, Duration, FeatureFlags, Fn, IAspect, Lazy, Names, Size, Stack, Token } from '../../core';
import { ValidationError } from '../../core/lib/errors';
import { LAMBDA_RECOGNIZE_LAYER_VERSION } from '../../cx-api';

/**
Expand Down Expand Up @@ -917,16 +918,16 @@ export class Function extends FunctionBase {

if (props.functionName && !Token.isUnresolved(props.functionName)) {
if (props.functionName.length > 64) {
throw new Error(`Function name can not be longer than 64 characters but has ${props.functionName.length} characters.`);
throw new ValidationError(`Function name can not be longer than 64 characters but has ${props.functionName.length} characters.`, this);
}
if (!/^[a-zA-Z0-9-_]+$/.test(props.functionName)) {
throw new Error(`Function name ${props.functionName} can contain only letters, numbers, hyphens, or underscores with no spaces.`);
throw new ValidationError(`Function name ${props.functionName} can contain only letters, numbers, hyphens, or underscores with no spaces.`, this);
}
}

if (props.description && !Token.isUnresolved(props.description)) {
if (props.description.length > 256) {
throw new Error(`Function description can not be longer than 256 characters but has ${props.description.length} characters.`);
throw new ValidationError(`Function description can not be longer than 256 characters but has ${props.description.length} characters.`, this);
}
}

Expand All @@ -951,10 +952,10 @@ export class Function extends FunctionBase {
const config = props.filesystem.config;
if (!Token.isUnresolved(config.localMountPath)) {
if (!/^\/mnt\/[a-zA-Z0-9-_.]+$/.test(config.localMountPath)) {
throw new Error(`Local mount path should match with ^/mnt/[a-zA-Z0-9-_.]+$ but given ${config.localMountPath}.`);
throw new ValidationError(`Local mount path should match with ^/mnt/[a-zA-Z0-9-_.]+$ but given ${config.localMountPath}.`, this);
}
if (config.localMountPath.length > 160) {
throw new Error(`Local mount path can not be longer than 160 characters but has ${config.localMountPath.length} characters.`);
throw new ValidationError(`Local mount path can not be longer than 160 characters but has ${config.localMountPath.length} characters.`, this);
}
}
if (config.policies) {
Expand Down Expand Up @@ -1019,16 +1020,16 @@ export class Function extends FunctionBase {
}

if (props.architecture && props.architectures !== undefined) {
throw new Error('Either architecture or architectures must be specified but not both.');
throw new ValidationError('Either architecture or architectures must be specified but not both.', this);
}
if (props.architectures && props.architectures.length > 1) {
throw new Error('Only one architecture must be specified.');
throw new ValidationError('Only one architecture must be specified.', this);
}
this._architecture = props.architecture ?? (props.architectures && props.architectures[0]);

if (props.ephemeralStorageSize && !props.ephemeralStorageSize.isUnresolved()
&& (props.ephemeralStorageSize.toMebibytes() < 512 || props.ephemeralStorageSize.toMebibytes() > 10240)) {
throw new Error(`Ephemeral storage size must be between 512 and 10240 MB, received ${props.ephemeralStorageSize}.`);
throw new ValidationError(`Ephemeral storage size must be between 512 and 10240 MB, received ${props.ephemeralStorageSize}.`, this);
}

const resource: CfnFunction = new CfnFunction(this, 'Resource', {
Expand Down Expand Up @@ -1096,7 +1097,7 @@ export class Function extends FunctionBase {

if (props.layers) {
if (props.runtime === Runtime.FROM_IMAGE) {
throw new Error('Layers are not supported for container image functions');
throw new ValidationError('Layers are not supported for container image functions', this);
}

this.addLayers(...props.layers);
Expand All @@ -1109,7 +1110,7 @@ export class Function extends FunctionBase {
// Log retention
if (props.logRetention) {
if (props.logGroup) {
throw new Error('CDK does not support setting logRetention and logGroup');
throw new ValidationError('CDK does not support setting logRetention and logGroup', this);
}
const logRetention = new logs.LogRetention(this, 'LogRetention', {
logGroupName: `/aws/lambda/${this.functionName}`,
Expand Down Expand Up @@ -1137,7 +1138,7 @@ export class Function extends FunctionBase {

if (props.filesystem) {
if (!props.vpc) {
throw new Error('Cannot configure \'filesystem\' without configuring a VPC.');
throw new ValidationError('Cannot configure \'filesystem\' without configuring a VPC.', this);
}
const config = props.filesystem.config;
if (config.dependency) {
Expand Down Expand Up @@ -1201,7 +1202,7 @@ export class Function extends FunctionBase {
'LAMBDA_RUNTIME_DIR',
];
if (reservedEnvironmentVariables.includes(key)) {
throw new Error(`${key} environment variable is reserved by the lambda runtime and can not be set manually. See https://docs.aws.amazon.com/lambda/latest/dg/configuration-envvars.html`);
throw new ValidationError(`${key} environment variable is reserved by the lambda runtime and can not be set manually. See https://docs.aws.amazon.com/lambda/latest/dg/configuration-envvars.html`, this);
}
this.environment[key] = { value, ...options };
return this;
Expand All @@ -1214,24 +1215,24 @@ export class Function extends FunctionBase {
*/
private getLoggingConfig(props: FunctionProps): CfnFunction.LoggingConfigProperty | undefined {
if (props.logFormat && props.loggingFormat) {
throw new Error('Only define LogFormat or LoggingFormat, not both.');
throw new ValidationError('Only define LogFormat or LoggingFormat, not both.', this);
}

if (props.applicationLogLevel && props.applicationLogLevelV2) {
throw new Error('Only define applicationLogLevel or applicationLogLevelV2, not both.');
throw new ValidationError('Only define applicationLogLevel or applicationLogLevelV2, not both.', this);
}

if (props.systemLogLevel && props.systemLogLevelV2) {
throw new Error('Only define systemLogLevel or systemLogLevelV2, not both.');
throw new ValidationError('Only define systemLogLevel or systemLogLevelV2, not both.', this);
}

if (props.applicationLogLevel || props.applicationLogLevelV2 || props.systemLogLevel || props.systemLogLevelV2) {
if (props.logFormat !== LogFormat.JSON && props.loggingFormat === undefined) {
throw new Error(`To use ApplicationLogLevel and/or SystemLogLevel you must set LogFormat to '${LogFormat.JSON}', got '${props.logFormat}'.`);
throw new ValidationError(`To use ApplicationLogLevel and/or SystemLogLevel you must set LogFormat to '${LogFormat.JSON}', got '${props.logFormat}'.`, this);
}

if (props.loggingFormat !== LoggingFormat.JSON && props.logFormat === undefined) {
throw new Error(`To use ApplicationLogLevel and/or SystemLogLevel you must set LoggingFormat to '${LoggingFormat.JSON}', got '${props.loggingFormat}'.`);
throw new ValidationError(`To use ApplicationLogLevel and/or SystemLogLevel you must set LoggingFormat to '${LoggingFormat.JSON}', got '${props.loggingFormat}'.`, this);
}
}

Expand Down Expand Up @@ -1268,7 +1269,7 @@ export class Function extends FunctionBase {
*/
public invalidateVersionBasedOn(x: string) {
if (Token.isUnresolved(x)) {
throw new Error('invalidateVersionOn: input may not contain unresolved tokens');
throw new ValidationError('invalidateVersionOn: input may not contain unresolved tokens', this);
}
this.hashMixins.push(x);
}
Expand All @@ -1283,11 +1284,11 @@ export class Function extends FunctionBase {
public addLayers(...layers: ILayerVersion[]): void {
for (const layer of layers) {
if (this._layers.length === 5) {
throw new Error('Unable to add layer: this lambda function already uses 5 layers.');
throw new ValidationError('Unable to add layer: this lambda function already uses 5 layers.', this);
}
if (layer.compatibleRuntimes && !layer.compatibleRuntimes.find(runtime => runtime.runtimeEquals(this.runtime))) {
const runtimes = layer.compatibleRuntimes.map(runtime => runtime.name).join(', ');
throw new Error(`This lambda function uses a runtime that is incompatible with this layer (${this.runtime.name} is not in [${runtimes}])`);
throw new ValidationError(`This lambda function uses a runtime that is incompatible with this layer (${this.runtime.name} is not in [${runtimes}])`, this);
}

// Currently no validations for compatible architectures since Lambda service
Expand Down Expand Up @@ -1398,8 +1399,8 @@ export class Function extends FunctionBase {
}
const envKeys = Object.keys(this.environment);
if (envKeys.length !== 0) {
throw new Error(`The function ${this.node.path} contains environment variables [${envKeys}] and is not compatible with Lambda@Edge. \
Environment variables can be marked for removal when used in Lambda@Edge by setting the \'removeInEdge\' property in the \'addEnvironment()\' API.`);
throw new ValidationError(`The function ${this.node.path} contains environment variables [${envKeys}] and is not compatible with Lambda@Edge. \
Environment variables can be marked for removal when used in Lambda@Edge by setting the \'removeInEdge\' property in the \'addEnvironment()\' API.`, this);
}

return;
Expand Down Expand Up @@ -1435,19 +1436,19 @@ Environment variables can be marked for removal when used in Lambda@Edge by sett
}

if (props.runtime === Runtime.FROM_IMAGE) {
throw new Error("ADOT Lambda layer can't be configured with container image package type");
throw new ValidationError("ADOT Lambda layer can't be configured with container image package type", this);
}

// This is not the complete list of incompatible runtimes and layer types. We are only
// checking for common mistakes on a best-effort basis.
if (this.runtime === Runtime.GO_1_X) {
throw new Error('Runtime go1.x is not supported by the ADOT Lambda Go SDK');
throw new ValidationError('Runtime go1.x is not supported by the ADOT Lambda Go SDK', this);
}

// The Runtime is Python and Adot is set it requires a different EXEC_WRAPPER than the other code bases.
if (this.runtime.family === RuntimeFamily.PYTHON &&
props.adotInstrumentation.execWrapper.valueOf() !== AdotLambdaExecWrapper.INSTRUMENT_HANDLER) {
throw new Error('Python Adot Lambda layer requires AdotLambdaExecWrapper.INSTRUMENT_HANDLER');
throw new ValidationError('Python Adot Lambda layer requires AdotLambdaExecWrapper.INSTRUMENT_HANDLER', this);
}

this.addLayers(LayerVersion.fromLayerVersionArn(this, 'AdotLayer', props.adotInstrumentation.layerVersion._bind(this).arn));
Expand Down Expand Up @@ -1510,47 +1511,47 @@ Environment variables can be marked for removal when used in Lambda@Edge by sett
*/
private configureVpc(props: FunctionProps): CfnFunction.VpcConfigProperty | undefined {
if (props.securityGroup && props.securityGroups) {
throw new Error('Only one of the function props, securityGroup or securityGroups, is allowed');
throw new ValidationError('Only one of the function props, securityGroup or securityGroups, is allowed', this);
}

const hasSecurityGroups = props.securityGroups && props.securityGroups.length > 0;
if (!props.vpc) {
if (props.allowAllOutbound !== undefined) {
throw new Error('Cannot configure \'allowAllOutbound\' without configuring a VPC');
throw new ValidationError('Cannot configure \'allowAllOutbound\' without configuring a VPC', this);
}
if (props.securityGroup) {
throw new Error('Cannot configure \'securityGroup\' without configuring a VPC');
throw new ValidationError('Cannot configure \'securityGroup\' without configuring a VPC', this);
}
if (hasSecurityGroups) {
throw new Error('Cannot configure \'securityGroups\' without configuring a VPC');
throw new ValidationError('Cannot configure \'securityGroups\' without configuring a VPC', this);
}
if (props.vpcSubnets) {
throw new Error('Cannot configure \'vpcSubnets\' without configuring a VPC');
throw new ValidationError('Cannot configure \'vpcSubnets\' without configuring a VPC', this);
}
if (props.ipv6AllowedForDualStack) {
throw new Error('Cannot configure \'ipv6AllowedForDualStack\' without configuring a VPC');
throw new ValidationError('Cannot configure \'ipv6AllowedForDualStack\' without configuring a VPC', this);
}
if (props.allowAllIpv6Outbound !== undefined) {
throw new Error('Cannot configure \'allowAllIpv6Outbound\' without configuring a VPC');
throw new ValidationError('Cannot configure \'allowAllIpv6Outbound\' without configuring a VPC', this);
}
return undefined;
}

if (props.allowAllOutbound !== undefined) {
if (props.securityGroup) {
throw new Error('Configure \'allowAllOutbound\' directly on the supplied SecurityGroup.');
throw new ValidationError('Configure \'allowAllOutbound\' directly on the supplied SecurityGroup.', this);
}
if (hasSecurityGroups) {
throw new Error('Configure \'allowAllOutbound\' directly on the supplied SecurityGroups.');
throw new ValidationError('Configure \'allowAllOutbound\' directly on the supplied SecurityGroups.', this);
}
}

if (props.allowAllIpv6Outbound !== undefined) {
if (props.securityGroup) {
throw new Error('Configure \'allowAllIpv6Outbound\' directly on the supplied SecurityGroup.');
throw new ValidationError('Configure \'allowAllIpv6Outbound\' directly on the supplied SecurityGroup.', this);
}
if (hasSecurityGroups) {
throw new Error('Configure \'allowAllIpv6Outbound\' directly on the supplied SecurityGroups.');
throw new ValidationError('Configure \'allowAllIpv6Outbound\' directly on the supplied SecurityGroups.', this);
}
}

Expand Down Expand Up @@ -1585,8 +1586,8 @@ Environment variables can be marked for removal when used in Lambda@Edge by sett
const publicSubnetIds = new Set(props.vpc.publicSubnets.map(s => s.subnetId));
for (const subnetId of selectedSubnets.subnetIds) {
if (publicSubnetIds.has(subnetId) && !allowPublicSubnet) {
throw new Error('Lambda Functions in a public subnet can NOT access the internet. ' +
'If you are aware of this limitation and would still like to place the function in a public subnet, set `allowPublicSubnet` to true');
throw new ValidationError('Lambda Functions in a public subnet can NOT access the internet. ' +
'If you are aware of this limitation and would still like to place the function in a public subnet, set `allowPublicSubnet` to true', this);
}
}
this.node.addDependency(selectedSubnets.internetConnectivityEstablished);
Expand Down Expand Up @@ -1622,15 +1623,15 @@ Environment variables can be marked for removal when used in Lambda@Edge by sett
Annotations.of(this).addWarningV2('@aws-cdk/aws-lambda:snapStartRequirePublish', 'SnapStart only support published Lambda versions. Ignore if function already have published versions');

if (!props.runtime.supportsSnapStart) {
throw new Error(`SnapStart currently not supported by runtime ${props.runtime.name}`);
throw new ValidationError(`SnapStart currently not supported by runtime ${props.runtime.name}`, this);
}

if (props.filesystem) {
throw new Error('SnapStart is currently not supported using EFS');
throw new ValidationError('SnapStart is currently not supported using EFS', this);
}

if (props.ephemeralStorageSize && props.ephemeralStorageSize?.toMebibytes() > 512) {
throw new Error('SnapStart is currently not supported using more than 512 MiB Ephemeral Storage');
throw new ValidationError('SnapStart is currently not supported using more than 512 MiB Ephemeral Storage', this);
}

return props.snapStart._render();
Expand All @@ -1648,7 +1649,7 @@ Environment variables can be marked for removal when used in Lambda@Edge by sett
throw Error('deadLetterQueue defined but deadLetterQueueEnabled explicitly set to false');
}
if (props.deadLetterTopic && (props.deadLetterQueue || props.deadLetterQueueEnabled !== undefined)) {
throw new Error('deadLetterQueue and deadLetterTopic cannot be specified together at the same time');
throw new ValidationError('deadLetterQueue and deadLetterTopic cannot be specified together at the same time', this);
}

let deadLetterQueue: sqs.IQueue | sns.ITopic;
Expand Down Expand Up @@ -1698,7 +1699,7 @@ Environment variables can be marked for removal when used in Lambda@Edge by sett

private validateProfiling(props: FunctionProps) {
if (!props.runtime.supportsCodeGuruProfiling) {
throw new Error(`CodeGuru profiling is not supported by runtime ${props.runtime.name}`);
throw new ValidationError(`CodeGuru profiling is not supported by runtime ${props.runtime.name}`, this);
Copy link
Contributor

@kaizencc kaizencc Dec 6, 2024

Choose a reason for hiding this comment

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

[this question is not specific to this one instance, and is non-blocking, just for my own benefit]

when would there be a use case for the construct passed in to be anything other than this?
would it make more sense to add a method directly on Construct directly so we can instead do something like this.throwValidationError(...)

Copy link
Contributor Author

@mrgrain mrgrain Dec 6, 2024

Choose a reason for hiding this comment

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

I think this is a good idea and worth considering later. For aws-cdk-lib we really should have helpers available that also take into account tokens. That would give us a similar experience (albeit limited to this repo). I've considered this out-of-scope for this project.

}
if (props.environment && (props.environment.AWS_CODEGURU_PROFILER_GROUP_NAME
|| props.environment.AWS_CODEGURU_PROFILER_GROUP_ARN
Expand Down
Loading
Loading