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

[WIP] Improve Tasking Manager Cloudformation template #5471

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

eternaltyro
Copy link
Contributor

The most important enhancement is to remove soon-to-be deprecated LaunchConfiguration pieces and replace with LaunchTemplates

  • Parameterise several values previously hardcoded in the template
    • AMI ID
    • Backend instance type
    • Load balancer TLS Policy
    • DNS Zone ID
  • Rename some parameters to reflect their meaning better
  • Improve parameter types to make validation easier
  • Add secrets to AWS Secrets Manager and add their ARN tail as params
    • DB credentials
    • SMTP credentials
    • OAuth2 credentials
    • TM Secret
  • Remove parameters replaced by Secrets Manager entries
  • Simplify condition names
  • Remove Launch Configuration and replace with Launch Templates
  • Move package install from user-data to Metadata section

The most important enhancement is to remove soon-to-be deprecated
LaunchConfiguration pieces and replace with LaunchTemplates

- Parameterise several values previously hardcoded in the template
  - AMI ID
  - Backend instance type
  - Load balancer TLS Policy
  - DNS Zone ID
- Rename some parameters to reflect their meaning better
- Improve parameter types to make validation easier
- Add secrets to AWS Secrets Manager and add their ARN tail as params
  - DB credentials
  - SMTP credentials
  - OAuth2 credentials
  - TM Secret
- Remove parameters replaced by Secrets Manager entries
- Simplify condition names
- Remove Launch Configuration and replace with Launch Templates
- Move package install from user-data to Metadata section
@eternaltyro
Copy link
Contributor Author

@AfiMaameDufie @dakotabenjamin There seems to be something wrong with the template in that the LaunchTemplate is "deleted" immediately after its created. Then the deployment fails with the AutoScalingGroup complaining that the referenced Launch Template is "not found". I am not sure if the LaunchTemplate is "deleted" or if the creation fails. Either way, we need to look into why this happens. My suspicion is that the Metadata section is causing trouble. But I can't be sure and I am not certain how to troubleshoot this effectively.

Copy link
Contributor

@AfiMaameDufie AfiMaameDufie left a comment

Choose a reason for hiding this comment

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

Had a look at the file in general then the Launch Template and the metadata specifically. Things look as it should. I also doubt that its an authorization issue. Could it be the AZ or region? Any errors or logs I can reference? @eternaltyro

@eternaltyro
Copy link
Contributor Author

Well, this is embarrassing. Apparently, cf.ref("BackendLaunchTemplate") returns the Template ID and not the Template Name. So that's my error. I'm fixing it now.

Copy link
Member

@dakotabenjamin dakotabenjamin left a comment

Choose a reason for hiding this comment

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

Made only one comment- love these changes.

What is left to move from WIP? I'll send final review once ready.

'export LC_CTYPE="en_US.UTF-8"',
'dpkg-reconfigure --frontend=noninteractive locales',
'sudo apt-get -y update',
'sudo DEBIAN_FRONTEND=noninteractive apt-get -y -o Dpkg::Options::="--force-confdef" -o Dpkg::Options::="--force-confold" dist-upgrade',
Copy link
Member

Choose a reason for hiding this comment

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

Is it advisable to run dist-upgrade?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No it's not. Will fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

- [MISC] Move default backend instance type to t3.small
- [MISC] Add us-east-1e to AZ list
- [ENHANCE] Upgrade PostgreSQL version to 13.x
- [ENHANCE] Create a parameter group and assign to RDS instance based on
  parameter group family
- [MISC] Move default datase instance type to db.t4g.xlarge
- [MISC] Rename Parameters to make them more meaningful
- [ENHANCE] Fix names of resources - instead of using cf.stackName, use a simpler
  one.
- [FIX] Switch to LaunchTemplateId from LaunchTemplateName
- [ENHANCE] Parameterize disk size, snapshot retention period, etc.,
  based on IsProduction condition
- [SECURITY] Add a more restricted S3 Bucket Policy for frontend bucket
- [ENHANCE] Add an Origin Access Control policy to CloudFront
- [ENHANCE] Add a custom cache policy to CloudFront
- [ENHANCE] Better defaults for CloudFront distribution
  - Enable http2
  - Enable IPV6
  - Add Route53 Recordset Group instead of a standalone RecordSet
    to add both A records and AAA records
  - Upgraded TLS Protocol Version option
The issue:
If CloudFront needs to connect to S3 - We add an S3Origin. However, if
the S3 bucket is configured as a website, it needs to be considered a
custom origin. And custom origins can't have OriginAccessControl.

The Solution:
Removed Origin Access Control.
@eternaltyro
Copy link
Contributor Author

@dakotabenjamin @AfiMaameDufie With the latest push, I have fixed almost all problems with CloudFormation and the infra. I will now have to validate everything about the new infra that has been created in my test.

- User data needed python to be installed in order to get cfn-init
  to work. Fixed that.
- Cloud init scripts are run as root by default. so removed sudo
@sonarcloud
Copy link

sonarcloud bot commented Dec 19, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 5 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants