QoL updates ✅ #24

alismx · 2024-10-03T14:45:23Z

DEVOPS PULL REQUEST

Related Issue

resolve Bug: unable to apply due to missing IAM permissions. #29
resolve Validate or enforce that S3 buckets and ECR repository names are lowercase prior to plan and apply #28
resolve Internet gateway should be optional #23
starts Enables autoscaling for DIBBS pipeline apps #34
resolve Create a tfstate module #36
resolve Use terraform workspace jargon #35

Changes Proposed

Use terraform workspace jargon #35 Replace references to environment to use workspace instead to keep things consistent with jargon used by terraform
Create a tfstate module #36 Move tfstate related terraform into a module to better encapsulate those resources
Internet gateway should be optional #23 Refine the usage of the internal variable to better setup resources in private subnets and prevent the creation of certain VPC resources
Internet gateway should be optional #23 Setup VPC endpoints to allow resources in private subnets access to other AWS resources
Internet gateway should be optional #23 Bug: unable to apply due to missing IAM permissions. #29 Add additional permissions to the OIDC policies so that VPC endpoints and logs can be managed
Update setup script to handle setting the AWS provider
Validate or enforce that S3 buckets and ECR repository names are lowercase prior to plan and apply #28 Add validation to some terraform variables to prevent resources in AWS from throwing errors because a name is too long or contain characters that are not allowed
Enables autoscaling for DIBBS pipeline apps #34 Update the service_data to include min_capacity and max_capacity so that we can handle autoscaling

rin-skylight · 2024-10-18T19:05:55Z

.github/workflows/deployment.yaml


 on:
 workflow_dispatch:
 inputs:
- environment:
- description: 'The environment to deploy to'
+ workspace:


Non-breaking note: this will be a point of divergence from the Azure repositories, as that side does not leverage TF workspaces as a feature. There should be documentation to explain the discrepancy or choice of architecture somewhere.

I do think we should consider a discussion on whether workspaces or discrete folders are preferred. I tend to be wholly against workspaces for some of the reasons that Hashicorp outlines here: https://developer.hashicorp.com/terraform/cli/workspaces#use-cases If the partners you're speaking with overwhelmingly use workspaces, then we shouldn't reinvent the wheel...but, I would be very resistant to using this same approach with Azure.

😬 This repo has used workspaces from the beginning; these changes are just around language to eliminate confusion, which seems successful! We should set up a call to discuss this.

rin-skylight · 2024-10-18T19:08:04Z

.github/workflows/deployment.yaml

@@ -1,29 +1,35 @@
-name: Deploy to ECS
+name: Terraform (Plan||Apply)


I feel extremely squidgy about putting plan and apply together in the same action. If we mess up a conditional somewhere, damage can result. Is there a reason why these actions are not maintained separately, for safety reasons?

I don't mind separating it out.

rin-skylight · 2024-10-18T19:09:18Z

.github/workflows/deployment.yaml

@@ -45,23 +51,32 @@ jobs:
 with:
 role-to-assume: ${{ secrets.AWS_ROLE_ARN }}
 role-session-name: githubDeploymentWorkflow
- aws-region: ${{ env.aws_region }}
+ aws-region: ${{ secrets.AWS_REGION }}


Can I ask why we're making this a secret? You've given a default value in the documentation already...does obscuring this value provide us any benefit?

rin-skylight · 2024-10-18T19:10:20Z

terraform/implementation/ecs/_variable.tf

@@ -4,6 +4,17 @@ variable "availability_zones" {
 default = ["us-east-1a", "us-east-1b", "us-east-1c"]
 }

+variable "internal" {
+ description = "Internal"


We can do better for a description. 🙂

Thanks for catching this oversight.

rin-skylight · 2024-10-18T19:18:20Z

terraform/modules/ecs/_variable.tf

 type = bool
- description = "Flag to determine if the ALB is public (intended for external access) or private (only intended to be accessed within your AWS VPC)."
+ description = "Flag to determine if the several AWS resources are public (intended for external access, public internet) or private (only intended to be accessed within your AWS VPC or avaiable with other means, a transit gateway for example)."


This description can likely be copy-pasted into the concern I commented on above!

rin-skylight · 2024-10-18T19:20:00Z

terraform/modules/ecs/alb.tf

@@ -2,9 +2,9 @@
 # trivy:ignore:AVD-AWS-0053


Is this suppression still valid?

rin-skylight · 2024-10-18T19:21:22Z

terraform/modules/ecs/ecs.tf

@@ -43,7 +43,7 @@ resource "aws_ecs_service" "this" {
 name = each.key
 cluster = aws_ecs_cluster.dibbs_app_cluster.id
 task_definition = each.value.arn
- desired_count = local.service_data[each.key].app_count
+ desired_count = local.service_data[each.key].min_capacity


Non-blocking...do AWS services support scaling down to 0 with a rapid restart back to 1 if the container is hit? This could save some money, but may have a small performance impact. Just a thought!

I'm not sure, but I can look into it. That would be added in a follow-up PR. Changing to min_capacity is only a bit of setup to get us closer to adding autoscaling.

rin-skylight · 2024-10-18T19:22:21Z

terraform/modules/oidc/_variable.tf

 }

 variable "owner" {
 description = "The owner of the project"
 type = string
 default = "skylight"
+ validation {
+ condition = can(regex("^[[:alnum:]]{1,8}$", var.owner))
+ error_message = "owner must be 8 characters or less, all lowerspace with no special characters or spaces"


"lowercase"?

Hah, I misunderstood your comment. My thought was, yeah, lowercase... 😆

I'll fix the typo.

rin-skylight · 2024-10-18T19:22:51Z

terraform/modules/tfstate/_variable.tf

+ default = "skylight"
+ validation {
+ condition = can(regex("^[[:alnum:]]{1,8}$", var.owner))
+ error_message = "owner must be 8 characters/numbers or less, all lowerspace with no special characters or spaces"


"lowercase"?

alismx force-pushed the alis/private_subnet_updates branch 2 times, most recently from f46d10d to a2f5711 Compare October 9, 2024 15:39

update private subnets so they can pull from ecr

b80e6eb

alismx force-pushed the alis/private_subnet_updates branch 3 times, most recently from 1a8f2f9 to 4749dd5 Compare October 15, 2024 19:01

add tfstate module

0798ca3

alismx force-pushed the alis/private_subnet_updates branch from 4749dd5 to 0798ca3 Compare October 15, 2024 19:03

alismx added 2 commits October 15, 2024 14:44

update setup to use a module for state and oidc management additions

8282e97

update docs

6f74a20

alismx force-pushed the alis/private_subnet_updates branch 5 times, most recently from 16cbd35 to f1d3625 Compare October 15, 2024 22:37

update deployment to be more configurable via secrets and variables

41f77a9

alismx force-pushed the alis/private_subnet_updates branch 6 times, most recently from 65b96f5 to be9551c Compare October 16, 2024 21:16

add a plan workflow

27d72e8

alismx force-pushed the alis/private_subnet_updates branch 2 times, most recently from cadbd71 to 80ae92e Compare October 17, 2024 18:22

Add max and min capacity for future autoscaling options

e0393f9

alismx force-pushed the alis/private_subnet_updates branch from 80ae92e to e0393f9 Compare October 17, 2024 18:58

alismx changed the title ~~update private subnets so they can pull from ecr~~ QoL updates ✅ Oct 18, 2024

alismx requested review from rin-skylight and shanice-skylight October 18, 2024 17:47

alismx marked this pull request as ready for review October 18, 2024 18:28

rin-skylight reviewed Oct 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

QoL updates ✅ #24

QoL updates ✅ #24

alismx commented Oct 3, 2024 •

edited

Loading

rin-skylight Oct 18, 2024

rin-skylight Oct 18, 2024

alismx Oct 18, 2024 •

edited

Loading

rin-skylight Oct 18, 2024

alismx Oct 18, 2024

rin-skylight Oct 18, 2024

rin-skylight Oct 18, 2024

alismx Oct 18, 2024

rin-skylight Oct 18, 2024

rin-skylight Oct 18, 2024

rin-skylight Oct 18, 2024

alismx Oct 18, 2024

rin-skylight Oct 18, 2024

alismx Oct 18, 2024 •

edited

Loading

rin-skylight Oct 18, 2024

		@@ -1,29 +1,35 @@
		name: Deploy to ECS
		name: Terraform (Plan\|\|Apply)

QoL updates ✅ #24

Are you sure you want to change the base?

QoL updates ✅ #24

Conversation

alismx commented Oct 3, 2024 • edited Loading

DEVOPS PULL REQUEST

Related Issue

Changes Proposed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alismx Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alismx Oct 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alismx commented Oct 3, 2024 •

edited

Loading

alismx Oct 18, 2024 •

edited

Loading

alismx Oct 18, 2024 •

edited

Loading