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(examples): update api-websocket-lambda-dynamodb example #146

Merged
merged 12 commits into from
Aug 23, 2024

Conversation

corymhall
Copy link
Contributor

This PR makes a couple of updates to the api-websocket-lambda-dynamodb
example test

  • Updates the Lambda runtime to NODEJS_LATEST
    • This also requires switching from aws-sdk (v2) to
      @aws-sdk/client-* (v3)
  • Updates the CDK code to use L2s instead of L1s
  • Adds validation to the test to ensure that the code actually works (it
    didn't)

closes #144, re #145

This PR makes a couple of updates to the api-websocket-lambda-dynamodb
example test

- Updates the Lambda runtime to `NODEJS_LATEST`
  - This also requires switching from `aws-sdk` (v2) to
    `@aws-sdk/client-*` (v3)
- Updates the CDK code to use L2s instead of L1s
- Adds validation to the test to ensure that the code actually works (it
  didn't)

closes #144, re #145
@corymhall corymhall self-assigned this Aug 21, 2024
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The individual package.json files are not needed since the Lambda functions are not bundling any dependencies.

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 kept getting flaky test failures with this one so I just decided to update it as part of this PR.

The example that this had of creating a LB/Target group via pulumi and then the rest of the resources as CDK L1s doesn't really make sense. At that point it just makes more sense to create the entire thing as pulumi resources since they are the same level of abstraction.

This still shows an example of taking pulumi outputs (the vpc info) and using that as inputs to CDK constructs.

@@ -90,8 +101,6 @@ func TestS3ObjectLambda(t *testing.T) {
}

func TestEC2Instance(t *testing.T) {
t.Skipf("skipping due to missing support for `AWS::EC2::SecurityGroup`")
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 works now!

// It shouldn't be necessary to retry this, but my testing was very inconsistent, sometimes the `Dial`
// triggered the connect and sometimes it didn't (according to the docs it shouldn't be possible to establish
// a connection without triggering $connect)
func isConnected(ctx context.Context, t *testing.T, table string, c *websocket.Conn, client *dynamodb.Client, canRetry bool) error {
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 thought this was going to be easy, but it ended up being extremely annoying/difficult 😖

It might not even be worth it at this point...

@@ -444,7 +444,6 @@ export function mapToAwsResource(
const policy = new aws.iam.Policy(
logicalId,
{
name: rawProps.PolicyName,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

CloudFormation requires that a name be set so CDK sets a name automatically, but if not set then pulumi will auto name it which is better.

This fixes some inter-test conflicts.

@corymhall corymhall requested review from t0yv0 and flostadler August 21, 2024 19:58
Copy link
Contributor

@flostadler flostadler left a comment

Choose a reason for hiding this comment

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

LGTM

filters: [
{
name: 'opt-in-status',
values: ['opt-in-not-required'],
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! :D

// });
const azs = pulumi.output(
aws
.getAvailabilityZones({
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: you could do aws.getAvailabilityZonesOutput().names.

@@ -47,7 +45,7 @@ export class Ec2CdkStack extends pulumicdk.Stack {

// Use Latest Amazon Linux Image - CPU Type ARM64
const ami = new ec2.AmazonLinuxImage({
generation: ec2.AmazonLinuxGeneration.AMAZON_LINUX_2,
generation: ec2.AmazonLinuxGeneration.AMAZON_LINUX_2023,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

@corymhall corymhall merged commit 8825bca into main Aug 23, 2024
8 checks passed
@corymhall corymhall deleted the corymhall/update-nodejs-lambda branch August 23, 2024 13:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update api-websocket-lambda-dynamodb test
2 participants