Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 19 additions & 13 deletions .github/actions/api-deploy-ecs/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,19 @@ runs:
aws-secret-access-key: ${{ inputs.aws_secret_access_key }}
aws-region: eu-west-2

- name: Fill in the new image ID in the Amazon ECS API task definition
id: task-def-api
- name: Render SDK API task definition
id: task-def-sdk-api
uses: aws-actions/amazon-ecs-render-task-definition@v1
with:
task-definition: ${{ inputs.aws_task_definitions_directory_path }}/ecs-task-definition-web.json
task-definition: ${{ inputs.aws_task_definitions_directory_path }}/ecs-task-definition-sdk-api.json
container-name: flagsmith-api
image: ${{ inputs.api_ecr_image_url }}

- name: Render Admin API task definition
id: task-def-admin-api
uses: aws-actions/amazon-ecs-render-task-definition@v1
with:
task-definition: ${{ inputs.aws_task_definitions_directory_path }}/ecs-task-definition-admin-api.json
container-name: flagsmith-api
image: ${{ inputs.api_ecr_image_url }}

Expand Down Expand Up @@ -93,21 +101,19 @@ runs:
}}'
shell: bash

- name: Deploy Amazon ECS web task definition
id: deploy-api-task-def
- name: Deploy Amazon ECS SDK API task definition
uses: aws-actions/amazon-ecs-deploy-task-definition@v2
with:
cluster: ${{ inputs.aws_ecs_cluster_name }}
service: ${{ inputs.aws_ecs_service_name }}
task-definition: ${{ steps.task-def-api.outputs.task-definition }}
task-definition: ${{ steps.task-def-sdk-api.outputs.task-definition }}

- name: Deploy Amazon ECS SDK service with same task definition
run: |
Copy link

Choose a reason for hiding this comment

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

Removed step ID breaks deployment verification step

High Severity

The old step id: deploy-api-task-def was removed when renaming the deploy step, but line 166 still references steps.deploy-api-task-def.outputs.task-definition-arn. This will resolve to an empty string, causing EXPECTED_TASK_DEF to be empty and the "Verify correct version is running" check to always fail, blocking every deployment.

Fix in Cursor Fix in Web

aws ecs update-service \
--cluster ${{ inputs.aws_ecs_cluster_name }} \
--service ${{ inputs.aws_ecs_sdk_service_name }} \
--task-definition ${{ steps.deploy-api-task-def.outputs.task-definition-arn }}
shell: bash
- name: Deploy Amazon ECS SDK service
uses: aws-actions/amazon-ecs-deploy-task-definition@v2
with:
cluster: ${{ inputs.aws_ecs_cluster_name }}
service: ${{ inputs.aws_ecs_sdk_service_name }}
task-definition: ${{ steps.task-def-admin-api.outputs.task-definition }}
Copy link

Choose a reason for hiding this comment

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

Admin and SDK task definitions deployed to wrong services

High Severity

The task definitions are swapped between services. The step "Deploy Amazon ECS SDK API task definition" deploys task-def-sdk-api to aws_ecs_service_name (the admin/main service), while "Deploy Amazon ECS SDK service" deploys task-def-admin-api to aws_ecs_sdk_service_name (the SDK service). The verification step at line 167 confirms aws_ecs_service_name is the admin service by naming its output RUNNING_ADMIN_API_TASK_DEF. This means the admin service gets the SDK config and the SDK service gets the admin config.

Additional Locations (1)

Fix in Cursor Fix in Web


# The DynamoDB Identity Migrator uses the same task definition as the SQL schema migrator but overrides the container definition
# with the new django execute target
Expand Down
310 changes: 310 additions & 0 deletions infrastructure/aws/production/ecs-task-definition-admin-api.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,310 @@
{
"family": "flagsmith-api",
"networkMode": "awsvpc",
"executionRoleArn": "arn:aws:iam::084060095745:role/task-exec-role-741a7e3",
"taskRoleArn": "arn:aws:iam::084060095745:role/task-exec-role-741a7e3",
"containerDefinitions": [
{
"name": "flagsmith-api",
"command": [
"serve"
],
"cpu": 0,
"portMappings": [
{
"containerPort": 8000,
"hostPort": 8000,
"protocol": "tcp"
}
Copy link

Choose a reason for hiding this comment

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

New task definitions missing Prometheus port 9100

Medium Severity

Both new task definition files only expose port 8000 but set PROMETHEUS_ENABLED to "True". Every other task definition in the codebase (production SDK API, staging admin API, both task processors) exposes port 9100 alongside 8000 for Prometheus metrics scraping. Without port 9100, Prometheus won't be able to collect metrics from the production admin API or staging SDK API despite the feature being enabled.

Additional Locations (1)

Fix in Cursor Fix in Web

],
"healthCheck": {
"command": [
"CMD-SHELL",
"flagsmith healthcheck tcp"
],
"interval": 10,
"timeout": 2,
"retries": 5,
"startPeriod": 5
},
"essential": true,
"environment": [
{
"name": "LOG_LEVEL",
"value": "INFO"
},
{
"name": "AWS_REGION",
"value": "eu-west-2"
},
{
"name": "AWS_DEFAULT_REGION",
"value": "eu-west-2"
},
{
"name": "DJANGO_ALLOWED_HOSTS",
"value": "*"
},
{
"name": "DJANGO_SETTINGS_MODULE",
"value": "app.settings.production"
},
{
"name": "CACHE_FLAGS_SECONDS",
"value": "10"
},
{
"name": "CACHE_PROJECT_SEGMENTS_SECONDS",
"value": "10"
},
{
"name": "CACHE_ENVIRONMENT_SEGMENTS_SECONDS",
"value": "30"
},
{
"name": "CHARGEBEE_SITE",
"value": "flagsmith"
},
{
"name": "DJANGO_SETTINGS_MODULE",
"value": "app.settings.production"
},
{
"name": "ENABLE_CHARGEBEE",
"value": "True"
},
{
"name": "ENABLE_TELEMETRY",
"value": "False"
},
{
"name": "ENVIRONMENT",
"value": "production"
},
{
"name": "ENVIRONMENTS_TABLE_NAME_DYNAMO",
"value": "flagsmith_environments"
},
{
"name": "ENVIRONMENTS_V2_TABLE_NAME_DYNAMO",
"value": "flagsmith_environments_v2"
},
{
"name": "ENABLE_FE_E2E",
"value": "True"
},
{
"name": "GITHUB_CLIENT_ID",
"value": "b706a0da3e9d3115ea9d"
},
{
"name": "GUNICORN_WORKERS",
"value": "3"
},
{
"name": "GUNICORN_THREADS",
"value": "15"
},
{
"name": "GUNICORN_KEEP_ALIVE",
"value": "60"
},
{
"name": "IDENTITIES_TABLE_NAME_DYNAMO",
"value": "flagsmith_identities"
},
{
"name": "INFLUXDB_BUCKET",
"value": "api_prod"
},
{
"name": "INFLUXDB_ORG",
"value": "ben.rometsch@bullet-train.io"
},
{
"name": "INFLUXDB_URL",
"value": "https://eu-central-1-1.aws.cloud2.influxdata.com"
},
{
"name": "DEFAULT_THROTTLE_CLASSES",
"value": "core.throttling.UserRateThrottle"
},
{
"name": "USER_THROTTLE_RATE",
"value": "10/sec"
},
{
"name": "OAUTH_CLIENT_ID",
"value": "232959427810-br6ltnrgouktp0ngsbs04o14ueb9rch0.apps.googleusercontent.com"
},
{
"name": "PROJECT_METADATA_TABLE_NAME_DYNAMO",
"value": "flagsmith_project_metadata"
},
{
"name": "SECURE_PROXY_SSL_HEADER_NAME",
"value": "HTTP_CLOUDFRONT_FORWARDED_PROTO"
},
{
"name": "SENDER_EMAIL",
"value": "Flagsmith <support@flagsmith.com>"
},
{
"name": "SLACK_CLIENT_ID",
"value": "937916178726.1924685747446"
},
{
"name": "IDENTITY_MIGRATION_EVENT_BUS_NAME",
"value": "identity_migration-d46ed1a"
},
{
"name": "ENVIRONMENTS_API_KEY_TABLE_NAME_DYNAMO",
"value": "flagsmith_environment_api_key"
},
{
"name": "CACHE_BAD_ENVIRONMENTS_SECONDS",
"value": "60"
},
{
"name": "FEATURE_EVALUATION_CACHE_SECONDS",
"value": "300"
},
{
"name": "USE_CACHE_FOR_USAGE_DATA",
"value": "True"
},
{
"name": "API_USAGE_CACHE_SECONDS",
"value": "300"
},
{
"name": "EDGE_RELEASE_DATETIME",
"value": "2022-06-07T10:00:00Z"
},
{
"name": "CACHE_ENVIRONMENT_DOCUMENT_SECONDS",
"value": "60"
},
{
"name": "TASK_RUN_METHOD",
"value": "TASK_PROCESSOR"
},
{
"name": "SSE_SERVER_BASE_URL",
"value": "https://origin.realtime.flagsmith.com"
},
{
"name": "ENABLE_HUBSPOT_LEAD_TRACKING",
"value": "True"
},
{
"name": "GITHUB_APP_ID",
"value": "811209"
},
{
"name": "FLAGSMITH_ON_FLAGSMITH_SERVER_OFFLINE_MODE",
"value": "False"
},
{
"name": "PROMETHEUS_ENABLED",
"value": "True"
},
{
"name": "FLAGSMITH_ON_FLAGSMITH_SERVER_API_URL",
"value": "https://edge.api.flagsmith.com/api/v1/"
},
{
"name": "SEGMENT_RULES_CONDITIONS_EXPLICIT_ORDERING_ENABLED",
"value": "True"
}
],
"secrets": [
{
"name": "CHARGEBEE_API_KEY",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:CHARGEBEE_API_KEY::"
},
{
"name": "DATABASE_URL",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:DATABASE_URL::"
},
{
"name": "DJANGO_SECRET_KEY",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:DJANGO_SECRET_KEY::"
},
{
"name": "E2E_TEST_AUTH_TOKEN",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:E2E_TEST_AUTH_TOKEN::"
},
{
"name": "FORCE_SENTRY_TRACE_KEY",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:FORCE_SENTRY_TRACE_KEY::"
},
{
"name": "GITHUB_CLIENT_SECRET",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:GITHUB_CLIENT_SECRET::"
},
{
"name": "INFLUXDB_TOKEN",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:INFLUXDB_TOKEN::"
},
{
"name": "OAUTH_CLIENT_SECRET",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:OAUTH_CLIENT_SECRET::"
},
{
"name": "SENDGRID_API_KEY",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:SENDGRID_API_KEY::"
},
{
"name": "SENTRY_SDK_DSN",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:SENTRY_SDK_DSN::"
},
{
"name": "SLACK_CLIENT_SECRET",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:SLACK_CLIENT_SECRET::"
},
{
"name": "EDGE_REQUEST_SIGNING_KEY",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:EDGE_REQUEST_SIGNING_KEY::"
},
{
"name": "SSE_AUTHENTICATION_TOKEN",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:SSE_AUTHENTICATION_TOKEN::"
},
{
"name": "GITHUB_WEBHOOK_SECRET",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:GITHUB_WEBHOOK_SECRET::"
},
{
"name": "GITHUB_PEM",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:GITHUB_PEM-E1Ot8p"
},
{
"name": "HUBSPOT_ACCESS_TOKEN",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:HUBSPOT_ACCESS_TOKEN::"
},
{
"name": "FLAGSMITH_ON_FLAGSMITH_SERVER_KEY",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:FLAGSMITH_ON_FLAGSMITH_SERVER_KEY::"
},
{
"name": "PYLON_IDENTITY_VERIFICATION_SECRET",
"valueFrom": "arn:aws:secretsmanager:eu-west-2:084060095745:secret:ECS-API-LxUiIQ:PYLON_IDENTITY_VERIFICATION_SECRET::"
}
],
"logConfiguration": {
"logDriver": "awslogs",
"options": {
"awslogs-group": "flagsmith-fargate-eu-west-2-e815bfd",
"awslogs-region": "eu-west-2",
"awslogs-stream-prefix": "awslogs-flagsmith"
}
}
}
],
"requiresCompatibilities": [
"FARGATE"
],
"cpu": "1024",
"memory": "2048"
}
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@
},
{
"name": "DASHBOARD_ENDPOINTS_SENTRY_TRACE_SAMPLE_RATE",
"value": "0.002"
"value": "0"
Copy link

Choose a reason for hiding this comment

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

Dashboard sentry trace rate set to zero despite discussion

Medium Severity

DASHBOARD_ENDPOINTS_SENTRY_TRACE_SAMPLE_RATE was changed from "0.002" to "0", completely disabling dashboard endpoint sentry tracing on the SDK API. The PR discussion explicitly concluded this value should be left as it was, but it remains set to "0" in the current diff.

Fix in Cursor Fix in Web

},
{
"name": "SLACK_CLIENT_ID",
Expand Down
Loading
Loading