Skip to content

Migrate Infrastructure from Serverless Framework to AWS CDK#68

Merged
sannidhishukla merged 35 commits intodevfrom
migrate-to-aws-cdk
Jun 16, 2025
Merged

Migrate Infrastructure from Serverless Framework to AWS CDK#68
sannidhishukla merged 35 commits intodevfrom
migrate-to-aws-cdk

Conversation

@sannidhishukla
Copy link
Copy Markdown
Contributor

@sannidhishukla sannidhishukla commented Jun 10, 2025

Overview

This PR contains a number of changes related to migrating the Feedback Widget's AWS infrastructure from Serverless Framework to AWS CDK and moving it into our new Platform-specific AWS accounts

  1. Building out the resources using AWS
  2. Updating calls to the SSM parameter store to happen within the code instead of during resource creation so that plaintext values are not stuck in the CloudFormation template
  3. Removing the summary endpoint which would return an AI-generated summary of Feedback Widget comments. This function has not been used and its functionality can be replaced with using the NJ AI Assistant
  4. Upgrading the project from Node v18 to Node v22
  5. Greatly simplifying the file structure
  6. Update the README to be more up-to-date

For more in depth documentation on the changes and resources migrated over, see this document

Next Steps

  1. Create a GitHub action that handles deployment to the new AWS accounts
  2. Add more tests - many parts of this repository are completely untested. We will work on adding more tests as we make the switch over to our new RDS database
  3. Potentially setting up a connection to a dev Google Sheet to use in the interim while the databases are being set up. Not sure if this is strictly necessary at this point, though.

Comment on lines -78 to -109
### Project structure

The project code base is mainly located within the `src` folder. This folder is divided in:

- `functions` - containing code base and configuration for your lambda functions
- `libs` - containing shared code base between your lambdas

```
.
├── src
│ ├── functions # Lambda configuration and source code folder
│ │ ├── rating
│ │ │ ├── handler.ts # `rating` lambda source code
│ │ │ ├── index.ts # `rating` lambda Serverless configuration
│ │ │ ├── mock.json # `rating` lambda input parameter, if any, for local invocation
│ │ │ └── schema.ts # `rating` lambda input event JSON-Schema
│ │ │
│ │ └── index.ts # Import/export of all lambda configurations
│ │
│ └── libs # Lambda shared code
│ └── apiGateway.ts # API Gateway specific helpers
│ └── handlerResolver.ts # Sharable library for resolving lambda handlers
│ └── lambda.ts # Lambda middleware
├── package.json
├── serverless.ts # Serverless service file
├── tsconfig.json # Typescript compiler configuration
├── tsconfig.paths.json # Typescript paths
└── webpack.config.js # Webpack configuration
```

This project also contains a `scripts` folder, which contains post-processing Python scripts that help analyze the feedback data.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removing this for now. it will be hard to keep up-to-date. we can replace it if needed once all of the major changes forecasted for the feedback widget are complete.

Comment on lines -49 to -91
export async function getLastNComments(
sheetsClient: sheets_v4.Sheets,
n: number,
pageURL: string,
tabInfo
): Promise<string[][]> {
const { tabName, totalRowsRange, columnMap, isDefault } = tabInfo;
try {
const totalRows = await getTotalRows(sheetsClient, totalRowsRange);
if (totalRows < 2) return [];
let accumulatedComments = [];
let commentBatchEnd = totalRows;
const rightmostColumn = Object.keys(columnMap).reduce((a, b) =>
columnMap[a].index > columnMap[b].index ? a : b
);
while (accumulatedComments.length < n && commentBatchEnd > 1) {
const commentBatchStart = Math.max(commentBatchEnd - n + 1, 2);
const result = await sheetsClient.spreadsheets.values.get({
spreadsheetId: process.env.SHEET_ID,
range: `${tabName}!A${commentBatchStart}:${columnMap[rightmostColumn].column}${commentBatchEnd}`
});
let commentBatch = result.data.values ?? [];
if (commentBatch.length > 0) {
if (isDefault) {
commentBatch = commentBatch.filter(
(v) =>
v[columnMap.pageUrl.index].includes(pageURL) &&
v[columnMap.comment.index] !== undefined
);
} else {
commentBatch = commentBatch.filter(
(v) => !!v[columnMap.comment.index]
);
}
accumulatedComments = commentBatch.concat(accumulatedComments);
}
commentBatchEnd = commentBatchStart - 1;
}
return accumulatedComments;
} catch (e) {
throw Error(`Google Sheets API failed to get input data: ${e.message}`);
}
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed as this was only used by the deprecated /summary endpoint

Comment on lines -36 to -47
// only used within getLastNComments function
async function getTotalRows(sheetsClient: sheets_v4.Sheets, totalRowRange) {
try {
const result = await sheetsClient.spreadsheets.values.get({
spreadsheetId: process.env.SHEET_ID,
range: totalRowRange
});
return parseInt(result.data.values[0][0]);
} catch (e) {
throw Error(`Google Sheets API failed to get data size: ${e.message}`);
}
}
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed as this was only used by the deprecated /summary endpoint

Comment on lines +7 to +8
// eslint-disable-next-line @typescript-eslint/no-var-requires
const { getSsmParam } = require('./awsUtils');
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not sure why but the next line breaks everything if it's switched from require to import

@sannidhishukla sannidhishukla changed the base branch from main to dev June 11, 2025 14:39
Copy link
Copy Markdown

@casewalker casewalker left a comment

Choose a reason for hiding this comment

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

I just wanted to do a quick CDK check, things look pretty great! Left some comments, but great work regardless!

const { feedbackId, comment, pageURL, rating } = JSON.parse(
event.body
) as Comment;

Copy link
Copy Markdown
Contributor

@AnJuHyppolite AnJuHyppolite Jun 13, 2025

Choose a reason for hiding this comment

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

[boulder] Since comment is a required property, we should validate it before entering the try block by adding a guard clause that returns early if it's null or undefined. What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

just added this!

event: APIGatewayProxyEvent
): Promise<FeedbackResponse> => {
const { pageURL, rating } = JSON.parse(event.body) as Rating;

Copy link
Copy Markdown
Contributor

@AnJuHyppolite AnJuHyppolite Jun 13, 2025

Choose a reason for hiding this comment

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

[boulder] Since pageURL and rating are required properties, we should validate them before entering the try block by adding a guard clause that returns early if they're null or undefined. What do you think?

@AnJuHyppolite AnJuHyppolite self-requested a review June 13, 2025 15:38
Copy link
Copy Markdown
Contributor

@AnJuHyppolite AnJuHyppolite left a comment

Choose a reason for hiding this comment

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

Left a few comments. Great work, @sannidhishukla!

Copy link
Copy Markdown
Contributor

@ezhangy ezhangy left a comment

Choose a reason for hiding this comment

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

awesome job on this refactor! left some dusts, mostly on the README to clarify how to deploy the API and test it locally

README.md Outdated
2. Run `npm install` (on Node 22, as listed in `.nvmrc`) to install Node dependencies
3. Save the credentials from the `Innov-Platform-dev` AWS account to your `~/.aws/credentials` file
4. Run the API locally
5. In another terminal window, you can test sending commands like the following:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[dust] is it possible to add instructions on how to query the dev deployment (in Innov-Platform-dev) instead of from localhost:3000? i feel like being able to do so will be an important manual testing step before promoting to prod while we're still missing test coverage

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes! i added instructions on sending requests through the lambda console to test (which is what i've been doing)

@sannidhishukla sannidhishukla merged commit 3fe9fb7 into dev Jun 16, 2025
@sannidhishukla sannidhishukla deleted the migrate-to-aws-cdk branch June 16, 2025 21:47
@ezhangy ezhangy mentioned this pull request Aug 20, 2025
ezhangy added a commit that referenced this pull request Aug 26, 2025
@ezhangy ezhangy mentioned this pull request Aug 26, 2025
ezhangy added a commit that referenced this pull request Aug 26, 2025
Merges the following PRs to main: 
- #68 
- #70 
- #69 
- #72 
- #71 
- #73 
- #75 
- #77
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.

4 participants