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

chore(ci): add canary to layer deployment #1593

Merged
merged 2 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 3 additions & 1 deletion .github/scripts/setup_tmp_layer_files.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ npm init -y
npm i \
@aws-lambda-powertools/logger@$VERSION \
@aws-lambda-powertools/metrics@$VERSION \
@aws-lambda-powertools/tracer@$VERSION
@aws-lambda-powertools/tracer@$VERSION \
@aws-lambda-powertools/commons@$VERSION \
am29d marked this conversation as resolved.
Show resolved Hide resolved
@aws-lambda-powertools/parameters@$VERSION
rm -rf node_modules/@types \
package.json \
package-lock.json
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/reusable_deploy_layer_stack.yml
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ jobs:
path: ./cdk-layer-stack/* # NOTE: upload-artifact does not inherit working-directory setting.
if-no-files-found: error
retention-days: 1
- name: CDK deploy canary
run: npm run cdk -w layer -- deploy --app cdk.out --context region=${{ matrix.region }} 'CanaryStack' --require-approval never --verbose --outputs-file cdk-outputs.json
update_layer_arn_docs:
needs: deploy-cdk-stack
permissions:
Expand Down
7 changes: 7 additions & 0 deletions layers/bin/layers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import 'source-map-support/register';
import { App } from 'aws-cdk-lib';
import { LayerPublisherStack } from '../src/layer-publisher-stack';
import { CanaryStack } from 'layers/src/canary-stack';

const SSM_PARAM_LAYER_ARN = '/layers/powertools-layer-arn';

Expand All @@ -12,3 +13,9 @@ new LayerPublisherStack(app, 'LayerPublisherStack', {
layerName: 'AWSLambdaPowertoolsTypeScript',
ssmParameterLayerArn: SSM_PARAM_LAYER_ARN,
});

new CanaryStack(app, 'CanaryStack', {
powertoolsPackageVersion: app.node.tryGetContext('PowertoolsPackageVersion'),
dreamorosi marked this conversation as resolved.
Show resolved Hide resolved
ssmParameterLayerArn: SSM_PARAM_LAYER_ARN,
layerName: 'AWSLambdaPowertoolsCanaryTypeScript',
});
99 changes: 99 additions & 0 deletions layers/src/canary-stack.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
import { CustomResource, Duration, Stack, StackProps } from 'aws-cdk-lib';
import { Construct } from 'constructs';
import { LayerVersion, Runtime } from 'aws-cdk-lib/aws-lambda';
import { RetentionDays } from 'aws-cdk-lib/aws-logs';
import { v4 } from 'uuid';
import {
Effect,
ManagedPolicy,
PolicyStatement,
Role,
ServicePrincipal,
} from 'aws-cdk-lib/aws-iam';
import { Provider } from 'aws-cdk-lib/custom-resources';
import { StringParameter } from 'aws-cdk-lib/aws-ssm';
import path from 'path';
import { NodejsFunction } from 'aws-cdk-lib/aws-lambda-nodejs';

export interface CanaryStackProps extends StackProps {
readonly layerName: string;
readonly powertoolsPackageVersion: string;
readonly ssmParameterLayerArn: string;
}

export class CanaryStack extends Stack {
public constructor(scope: Construct, id: string, props: CanaryStackProps) {
super(scope, id, props);
const { layerName, powertoolsPackageVersion } = props;

const suffix = v4().substring(0, 5);

const layerArn = StringParameter.fromStringParameterAttributes(
this,
'LayerArn',
{
parameterName: props.ssmParameterLayerArn,
}
).stringValue;

// lambda function
const layer = [
LayerVersion.fromLayerVersionArn(this, 'powertools-layer', layerArn),
];

const executionRole = new Role(this, 'LambdaExecutionRole', {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: if we are defining the executionRole only to attach a policy below, maybe we can instead do:

canaryFunction.role?.attachInlinePolicy(
  new iam.Policy(this, 'list-buckets-policy', {
    statements: [
      new PolicyStatement({
        actions: ['lambda:GetFunction'],
        resources: ['*'],
        effect: Effect.ALLOW,
      })
    ],
  }),
);

This way we let CDK manage the role now & in future versions.

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 made me realise that the role is not required any more, so removing.

assumedBy: new ServicePrincipal('lambda.amazonaws.com'),
managedPolicies: [
ManagedPolicy.fromAwsManagedPolicyName(
'service-role/AWSLambdaBasicExecutionRole'
),
],
});

executionRole.addToPolicy(
new PolicyStatement({
actions: ['lambda:GetFunction'],
resources: ['*'],
effect: Effect.ALLOW,
})
);

const canaryFunction = new NodejsFunction(this, 'CanaryFunction', {
entry: path.join(__dirname, './canary/app.ts'),
handler: 'handler',
runtime: Runtime.NODEJS_18_X,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we / is it worth it to use different runtimes? Is it overkill to do so here?

If we decide to go with only one, should we use the oldest runtime instead? The reasoning behind using oldest is that we already develop using the latest, so it's more likely we'll have caught issues with that.

On the other hand, that might be a concern for e2e tests and not for this stage.

Thoughts?

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 am indifferent, the e2e tests for layer should ensure it works with all supported runtimes. Using oldest is a better choice as you mentioned.

functionName: `canary-${suffix}`,
timeout: Duration.seconds(30),
bundling: {
externalModules: [
// don't package these modules, we want to pull them from the layer
'aws-sdk',
'@aws-lambda-powertools/logger',
'@aws-lambda-powertools/metrics',
'@aws-lambda-powertools/tracer',
'@aws-lambda-powertools/parameters',
'@aws-lambda-powertools/commons',
],
},
role: executionRole,
environment: {
POWERTOOLS_SERVICE_NAME: 'canary',
POWERTOOLS_VERSION: powertoolsPackageVersion,
POWERTOOLS_LAYER_NAME: layerName,
},
layers: layer,
logRetention: RetentionDays.ONE_DAY,
});

// use custom resource to trigger the lambda function during the CFN deployment
const provider = new Provider(this, 'CanaryCustomResourceProvider', {
onEventHandler: canaryFunction,
logRetention: RetentionDays.ONE_DAY,
});

// random suffix forces recreation of the custom resource otherwise the custom resource will be reused from prevous deployment
new CustomResource(this, `CanaryCustomResource${suffix}`, {
serviceToken: provider.serviceToken,
});
}
}
20 changes: 20 additions & 0 deletions layers/src/canary/app.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { Context } from 'aws-lambda';
import { Logger } from '@aws-lambda-powertools/logger';
import { Tracer } from '@aws-lambda-powertools/tracer';
import { Metrics, MetricUnits } from '@aws-lambda-powertools/metrics';
import { SSMProvider } from '@aws-lambda-powertools/parameters/ssm';

const logger = new Logger();
const tracer = new Tracer();
const metrics = new Metrics({});
// eslint-disable-next-line @typescript-eslint/no-unused-vars
const ssmProvider = new SSMProvider();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const ssmProvider = new SSMProvider();
new SSMProvider();

Copy link
Contributor

Choose a reason for hiding this comment

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

So we can remove the Eslint ignore.

Copy link
Contributor

Choose a reason for hiding this comment

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

Alternatively, let's actually retrieve a parameter - I see that the stack has one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can fetch the layer ARN parameter, good point

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 changed my mind, I only create the provider. It'd add otherwise unnecessary complexity to change the tests with parameters.


export const handler = async (
_event: unknown,
_context: Context
): Promise<void> => {
logger.info('Hello, world!');
metrics.addMetric('MyMetric', MetricUnits.Count, 1);
tracer.annotateColdStart();
};