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

Feature: AWS cloud infrastructure 🚀 #921

Open
wants to merge 22 commits into
base: dev
Choose a base branch
from

Conversation

chriswilty
Copy link
Member

@chriswilty chriswilty commented Jul 26, 2024

Description

And here it is! Cloud infra has sat on this feature branch for a while now, but I finally got around to adding a pipeline for automatic deployment of the production app on merging to main.

Note that there are likely some minor tasks still to come, but the pipeline was deployed and tested as an independent "dev" application, which was available at dev.spylogic.ai, and I verified it did not interfere with any existing prod resources.

Resolves #920

Screenshots

Best I can do right now is to show the CloudFormation stacks for our prod deployment:

image

Notes

After merging to main, I will need to manually deploy the prod pipeline to our AWS account, after which, the pipeline will self-update and deploy all application resources on subsequent merges to main branch.

Concerns

The stack constructs are not unit tested, so there is a chance that untested changes to the CDK application stacks could cause the pipeline to break. For this reason, it is advised to test any stack changes by synthesizing locally and then deploying the application stacks without the pipeline (and then destroying them after testing). I have added info to the README about this, plus some extra package scripts to help with these tasks.

Checklist

Have you done the following?

  • Linked the relevant Issue
  • Added tests
  • Ensured the workflow steps are passing

@chriswilty chriswilty force-pushed the feature/aws-cloud-infrastructure branch from 45744e0 to c303852 Compare July 26, 2024 13:59
@chriswilty
Copy link
Member Author

@pmarsh-scottlogic, @dhinrichs-scottlogic, @gsproston-scottlogic, @heatherlogan-scottlogic, @kclark-scottlogic, @AAloshine-scottlogic

This one may or may not be interesting to you! It's the culmination of the cloud infrastructure work resulting in our spylogic.ai application deployed in AWS. Once this is merged into main branch and the pipeline deployed to AWS, all future merges to main will trigger the pipeline to deploy the updated application.

There were some shenanigans around authorization, so I might even give a quick presentation to the Cloud COP if I have time.

@chriswilty
Copy link
Member Author

@kscott-scottlogic FYI

@chriswilty chriswilty force-pushed the feature/aws-cloud-infrastructure branch 2 times, most recently from 9fee899 to d70d549 Compare August 13, 2024 07:27

/*
Fargate service defines task running our backend container.
Note: cannot use ApplicationLoadBalancedFargateService construct as that adds a
Copy link
Collaborator

@kscott-scottlogic kscott-scottlogic Aug 16, 2024

Choose a reason for hiding this comment

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

I'm not sure but you might be able to use the openListener property on the ApplicationLoadBalancedFargateService construct to make its listener private. Also not sure how configurable that listener would be.
But the approach below works as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that didn't work nicely for me... One problem is around the Security Groups that construct generates, but additionally it creates a default rule on the listener that you cannot remove, and we want our default rule to deny access. So, I needed to do things the hard way 🙄

deleteExisting: true,
comment: `DNS A Record for UI host (${stage})`,
});
new AaaaRecord(this, generateResourceId('aaaarecord-cfront'), {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the AaaaRecord used for?

Copy link
Member Author

Choose a reason for hiding this comment

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

IPv6 access. CF distributions are dual stack by default, and I saw no reason to remove that.

Copy link
Collaborator

@kscott-scottlogic kscott-scottlogic left a comment

Choose a reason for hiding this comment

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

The api, auth and ui stack look good to me. Just a couple of minor comments, but I don't see anything that needs changing.

@chriswilty
Copy link
Member Author

The api, auth and ui stack look good to me. Just a couple of minor comments, but I don't see anything that needs changing.

Thanks! I would like to add some extra rate-limiting protections, but that will add extra costs so it's on the back burner for now. I was hoping to be able to reduce costs by removing the NAT Instance in favour of an IPv6 egress gateway for our container, but it turns out api.openai.com does not have an IPv6 address yet so that's a no-go 🤬

@chriswilty chriswilty force-pushed the feature/aws-cloud-infrastructure branch 9 times, most recently from f84ac05 to 6aca3b4 Compare September 13, 2024 12:36
@chriswilty
Copy link
Member Author

chriswilty commented Sep 13, 2024

Update: 13/09/2024

I've narrowed down the permissions for CloudFormation now, so that the pipeline no longer runs with full AdministratorAccess. I've created a set of IAM managed policies: see cloud/permissions/README.md

@chriswilty chriswilty force-pushed the feature/aws-cloud-infrastructure branch from 6aca3b4 to 978df68 Compare September 13, 2024 14:17
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.

AWS Cloud Infrastructure
2 participants