-
Notifications
You must be signed in to change notification settings - Fork 1
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
Linting, security scans and docs 🥇 #1
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
name: Terraform Linting | ||
on: | ||
pull_request: | ||
push: | ||
branches: | ||
- main | ||
|
||
concurrency: | ||
group: ${{ github.workflow }}-${{ github.ref }} | ||
cancel-in-progress: true | ||
|
||
jobs: | ||
tflint: | ||
runs-on: ${{ matrix.os }} | ||
|
||
strategy: | ||
matrix: | ||
os: [ubuntu-latest] | ||
|
||
steps: | ||
- uses: actions/checkout@v4 | ||
name: Checkout source code | ||
|
||
- uses: actions/cache@v4 | ||
name: Cache plugin dir | ||
with: | ||
path: ~/.tflint.d/plugins | ||
key: ${{ matrix.os }}-tflint-${{ hashFiles('.tflint.hcl') }} | ||
|
||
- uses: terraform-linters/setup-tflint@v4 | ||
name: Setup TFLint | ||
with: | ||
tflint_version: v0.52.0 | ||
|
||
- name: Show version | ||
run: tflint --version | ||
|
||
- name: Init TFLint | ||
run: tflint --init | ||
# If rate limiting becomes an issue, setup a GitHub token and enable it as an environment variable | ||
# env: | ||
# https://github.com/terraform-linters/tflint/blob/master/docs/user-guide/plugins.md#avoiding-rate-limiting | ||
# GITHUB_TOKEN: ${{ github.token }} | ||
|
||
- name: Run TFLint | ||
run: tflint -f compact |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
name: Terraform Security Scan | ||
|
||
on: | ||
pull_request: | ||
push: | ||
branches: | ||
- main | ||
|
||
concurrency: | ||
group: ${{ github.workflow }}-${{ github.ref }} | ||
cancel-in-progress: true | ||
|
||
jobs: | ||
trivy: | ||
runs-on: ubuntu-latest | ||
|
||
steps: | ||
- name: Checkout code | ||
uses: actions/checkout@v4 | ||
|
||
- name: Run tflint | ||
uses: ghcr.io/terraform-linters/tflint | ||
|
||
- name: Run Trivy vulnerability scanner | ||
uses: aquasecurity/[email protected] | ||
with: | ||
scan-type: 'fs' | ||
scan-ref: . | ||
scanners: 'vuln,secret,config' | ||
ignore-unfixed: false | ||
exit-code: '1' | ||
format: 'table' | ||
severity: 'CRITICAL,HIGH' |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
plugin "terraform" { | ||
enabled = true | ||
preset = "recommended" | ||
} | ||
|
||
plugin "aws" { | ||
enabled = true | ||
version = "0.34.0" | ||
source = "github.com/terraform-linters/tflint-ruleset-aws" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,22 +6,21 @@ resource "random_string" "s3_viewer" { | |
|
||
locals { | ||
registry_url = var.disable_ecr == false ? "${data.aws_caller_identity.current.account_id}.dkr.ecr.${var.region}.amazonaws.com" : "ghcr.io/cdcgov/phdi" | ||
registry_auth = data.aws_ecr_authorization_token.this.proxy_endpoint | ||
registry_username = data.aws_ecr_authorization_token.this.user_name | ||
registry_password = data.aws_ecr_authorization_token.this.password | ||
service_data = { | ||
service_data = var.service_data == {} ? { | ||
ecr-viewer = { | ||
short_name = "ecrv", | ||
fargate_cpu = 1024, | ||
fargate_memory = 2048, | ||
min_capacity = 1 | ||
max_capacity = 5 | ||
min_capacity = 1, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, no. This is going to be one of these stylistic areas that gives me fits. Is this JSON or YAML here? I've got a similar structure in |
||
max_capacity = 5, | ||
app_image = var.disable_ecr == false ? "${terraform.workspace}-ecr-viewer" : "ecr-viewer", | ||
app_version = var.phdi_version, | ||
container_port = 3000, | ||
host_port = 3000, | ||
public = true | ||
registry_url = local.registry_url | ||
public = true, | ||
registry_url = local.registry_url, | ||
env_vars = [ | ||
{ | ||
name = "AWS_REGION", | ||
|
@@ -61,64 +60,64 @@ locals { | |
short_name = "fhirc", | ||
fargate_cpu = 1024, | ||
fargate_memory = 2048, | ||
min_capacity = 1 | ||
max_capacity = 5 | ||
min_capacity = 1, | ||
max_capacity = 5, | ||
app_image = var.disable_ecr == false ? "${terraform.workspace}-fhir-converter" : "fhir-converter", | ||
app_version = var.phdi_version, | ||
container_port = 8080, | ||
host_port = 8080, | ||
public = false | ||
registry_url = local.registry_url | ||
public = false, | ||
registry_url = local.registry_url, | ||
env_vars = [] | ||
}, | ||
ingestion = { | ||
short_name = "inge", | ||
fargate_cpu = 1024, | ||
fargate_memory = 2048, | ||
min_capacity = 1 | ||
max_capacity = 5 | ||
min_capacity = 1, | ||
max_capacity = 5, | ||
app_image = var.disable_ecr == false ? "${terraform.workspace}-ingestion" : "ingestion", | ||
app_version = var.phdi_version, | ||
container_port = 8080, | ||
host_port = 8080, | ||
public = false | ||
registry_url = local.registry_url | ||
public = false, | ||
registry_url = local.registry_url, | ||
env_vars = [] | ||
}, | ||
validation = { | ||
short_name = "vali", | ||
fargate_cpu = 1024, | ||
fargate_memory = 2048, | ||
min_capacity = 1 | ||
max_capacity = 5 | ||
min_capacity = 1, | ||
max_capacity = 5, | ||
app_image = var.disable_ecr == false ? "${terraform.workspace}-validation" : "validation", | ||
app_version = var.phdi_version, | ||
container_port = 8080, | ||
host_port = 8080, | ||
public = false | ||
registry_url = local.registry_url | ||
public = false, | ||
registry_url = local.registry_url, | ||
env_vars = [] | ||
}, | ||
trigger-code-reference = { | ||
short_name = "trigcr", | ||
fargate_cpu = 1024, | ||
fargate_memory = 2048, | ||
min_capacity = 1 | ||
max_capacity = 5 | ||
min_capacity = 1, | ||
max_capacity = 5, | ||
app_image = var.disable_ecr == false ? "${terraform.workspace}-trigger-code-reference" : "trigger-code-reference", | ||
app_version = var.phdi_version, | ||
container_port = 8080, | ||
host_port = 8080, | ||
public = false | ||
registry_url = local.registry_url | ||
public = false, | ||
registry_url = local.registry_url, | ||
env_vars = [] | ||
}, | ||
message-parser = { | ||
short_name = "msgp", | ||
fargate_cpu = 1024, | ||
fargate_memory = 2048, | ||
min_capacity = 1 | ||
max_capacity = 5 | ||
min_capacity = 1, | ||
max_capacity = 5, | ||
app_image = var.disable_ecr == false ? "${terraform.workspace}-message-parser" : "message-parser", | ||
app_version = var.phdi_version, | ||
container_port = 8080, | ||
|
@@ -131,14 +130,14 @@ locals { | |
short_name = "orch", | ||
fargate_cpu = 1024, | ||
fargate_memory = 2048, | ||
min_capacity = 1 | ||
max_capacity = 5 | ||
min_capacity = 1, | ||
max_capacity = 5, | ||
app_image = var.disable_ecr == false ? "${terraform.workspace}-orchestration" : "orchestration", | ||
app_version = var.phdi_version, | ||
container_port = 8080, | ||
host_port = 8080, | ||
public = true | ||
registry_url = local.registry_url | ||
public = true, | ||
registry_url = local.registry_url, | ||
env_vars = [ | ||
{ | ||
name = "OTEL_METRICS", | ||
|
@@ -174,14 +173,13 @@ locals { | |
} | ||
] | ||
} | ||
} | ||
} : var.service_data | ||
local_name = "${var.project}-${var.owner}-${terraform.workspace}" | ||
|
||
# service_data = var.service_data == {} ? local.default_service_data : local.default_service_data | ||
appmesh_name = var.appmesh_name == "" ? local.local_name : var.appmesh_name | ||
cloudmap_namespace_name = var.cloudmap_namespace_name == "" ? local.local_name : var.cloudmap_namespace_name | ||
cloudmap_service_name = var.cloudmap_service_name == "" ? local.local_name : var.cloudmap_service_name | ||
ecs_alb_name = var.ecs_alb_name == "" ? "${local.local_name}" : var.ecs_alb_name | ||
ecs_alb_name = var.ecs_alb_name == "" ? local.local_name : var.ecs_alb_name | ||
ecs_alb_tg_name = var.ecs_alb_tg_name == "" ? local.local_name : var.ecs_alb_tg_name | ||
ecs_task_execution_role_name = var.ecs_task_execution_role_name == "" ? "${local.local_name}-tern" : var.ecs_task_execution_role_name | ||
ecs_task_role_name = var.ecs_task_role_name == "" ? "${local.local_name}-trn" : var.ecs_task_role_name | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small item, non-blocking: would it be more legible for us to use the
coalesce
function here, so the last part of the ternary isn't dangling off the end of the block a few dozen lines down?More info here: https://developer.hashicorp.com/terraform/language/functions/coalesce
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! I looked at
coalesce,
which seems like a good win for readability. 🔺I also took a peak at
merge
, I could see using it to allow for more refined control of the service_data object without needing to pass in the entire blob. ( like maybe I just want the orchestration service to have more memory) That said, it will not work for today, so I'll file that away in the maybe someday column.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After testing coalesce, I got some very weird behavior. The most consistent behavior came from checking the
service_data
object with this:I'd like to use this version; it's consistent and solves the issue of
var.service_data
getting lost at the end.