Terraform for mock AMAPI for loading testing - #48919
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Terraform infrastructure to support Android MDM load testing by deploying an internal Android AMAPI mock service and wiring loadtest Fleet to use it as a proxy endpoint (optionally forwarding real-device traffic to Google via a Secrets Manager credential).
Changes:
- Add a shared Secrets Manager secret for Android Google service account credentials and expose it via remote state output.
- Set
FLEET_DEV_ANDROID_PROXY_ENDPOINTin loadtest infra to point at the internal ALB. - Introduce a new
android_amapi_mockTerraform stack + Dockerfile to build/push/runcmd/android-amapi-mockbehind the internal ALB with/v1/*and/mock/*routing.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| infrastructure/loadtesting/terraform/shared/output.tf | Exports the Android Google credentials secret via shared remote state. |
| infrastructure/loadtesting/terraform/shared/enroll.tf | Adds a Secrets Manager secret resource intended for Android Google credentials. |
| infrastructure/loadtesting/terraform/infra/locals.tf | Sets FLEET_DEV_ANDROID_PROXY_ENDPOINT for the loadtest Fleet deployment. |
| infrastructure/loadtesting/terraform/docker/android-amapi-mock.Dockerfile | Builds android-amapi-mock from the Fleet repo for ECS deployment. |
| infrastructure/loadtesting/terraform/android_amapi_mock/variables.tf | Adds module inputs for git tag and forwarding toggle. |
| infrastructure/loadtesting/terraform/android_amapi_mock/outputs.tf | Outputs the internal URL to reach the mock via the internal ALB. |
| infrastructure/loadtesting/terraform/android_amapi_mock/main.tf | Provisions ECR image build/push, IAM policy for secrets, ECS service, and ALB routing rules for the mock. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| FLEET_MDM_SSO_RATE_LIMIT_PER_MINUTE = "500" | ||
| FLEET_SERVER_GZIP_RESPONSES = "true" | ||
|
|
||
| FLEET_DEV_ANDROID_PROXY_ENDPOINT = "http://${resource.aws_lb.internal.dns_name}/" |
There was a problem hiding this comment.
If Android MDM isn't configured in the DB, Fleet never makes AMAPI calls.
If Android MDM IS configured but the mock isn't deployed, the ALB returns a 404 or routes to Fleet which returns a 404 on /v1/*. Fleet's profile reconciliation cron retries every 30 seconds, no crash
|
@rfairburn Would you please review this when you have a moment? Thanks! |
…terraform # Conflicts: # cmd/android-amapi-mock/handlers.go # cmd/osquery-perf/android_agent.go # cmd/osquery-perf/osquery_perf/stats.go
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdded Terraform infrastructure for an Android AMAPI mock load-testing service. The configuration builds and publishes a Docker image, deploys it to ECS Fargate, configures logging and network access, and routes ALB requests for 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cmd/android-amapi-mock/handlers.go (1)
382-390: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse
continueinstead ofreturnwhen an application has no install operations.If
certIDsis empty for the current application, the function returns immediately. This skips any remaining applications inreq.Applicationsthat might contain install-type certificate templates.🐛 Proposed fix
if len(certIDs) == 0 { - return + continue }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/android-amapi-mock/handlers.go` around lines 382 - 390, In the application-processing loop around CertificateTemplateIDs, replace the early return when certIDs is empty with continue so only the current application is skipped. Preserve processing for subsequent applications in req.Applications.
🧹 Nitpick comments (5)
infrastructure/loadtesting/terraform/shared/output.tf (1)
29-31: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winMark this output as
sensitivefor consistency.This output exposes the full
aws_secretsmanager_secretresource, including its ARN. Sibling outputs that expose infrastructure identifiers ininfra/outputs.tf(for exampleecs_cluster,security_groups,ecs_arn) are markedsensitive = true. Apply the same convention here to avoid the ARN surfacing in plain CLI/CI output and logs.🔒️ Proposed fix
output "android_google_credentials" { + sensitive = true value = aws_secretsmanager_secret.android_google_credentials }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infrastructure/loadtesting/terraform/shared/output.tf` around lines 29 - 31, Mark the android_google_credentials output as sensitive by adding sensitive = true to its output block, matching the convention used by sibling infrastructure outputs while preserving its existing value.infrastructure/loadtesting/terraform/docker/android-amapi-mock.Dockerfile (2)
4-4: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winQuote the
$TAGshell variable.
RUN git clone -b $TAG --depth=1 ...expands$TAGunquoted in the shell. Quote it to avoid word-splitting/glob expansion and to guard against shell metacharacters in the build argument.🔒️ Proposed fix
-RUN git clone -b $TAG --depth=1 --no-tags --progress --no-recurse-submodules https://github.com/fleetdm/fleet.git +RUN git clone -b "$TAG" --depth=1 --no-tags --progress --no-recurse-submodules https://github.com/fleetdm/fleet.git🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infrastructure/loadtesting/terraform/docker/android-amapi-mock.Dockerfile` at line 4, Quote the TAG variable in the git clone command so its expansion is passed as a single argument without word splitting or glob expansion; update the existing clone invocation while preserving all other options.
1-1: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winConsider bumping the pinned Go and Alpine patch versions.
Line 1 pins
golang:1.26.4-alpine3.23. go1.26.4 (released 2026-06-02) includes security fixes to the crypto/x509, mime, and net/textproto packages, but a newer patch exists: Go 1.26.5 fixes a high-severity os.Root symlink-escape bug on Unix and a crypto/tls Encrypted Client Hello privacy leak.Line 7 pins
alpine:3.23.4. A newer Alpine patch is also available: Alpine 3.22.5, 3.23.5 released on 2026-06-21.Since this Dockerfile builds from a
sha256digest, bumping the version tag also requires updating the digest. Consider tracking the latest patch releases for both base images.Also applies to: 7-7
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infrastructure/loadtesting/terraform/docker/android-amapi-mock.Dockerfile` at line 1, Update the pinned base image versions in the Dockerfile to the latest requested Go and Alpine patch releases, and replace the existing sha256 digest with the digest matching the new golang:1.26.5-alpine3.23.5 image. Apply the same version and digest update to the corresponding Alpine pin on the referenced line.infrastructure/loadtesting/terraform/android_amapi_mock/main.tf (2)
9-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winVerify the pinned
kreuzwerker/dockerprovider version.The
dockerprovider is pinned at exactly3.6.2. The current major line has moved to v4, with4.5.0as the latest published version.Version 4.5.0 (latest) is the latest release of
kreuzwerker/docker, and the v3-to-v4 migration guide documents breaking changes. Confirm whether3.6.2is an intentional pin (v3 API dependency) or simply stale, and check whether a newer v3.x patch (bug fixes) is preferable to jumping straight to v4.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infrastructure/loadtesting/terraform/android_amapi_mock/main.tf` around lines 9 - 12, Review the `kreuzwerker/docker` provider constraint in the Terraform `docker` block and verify whether version 3.6.2 is required by v3 API dependencies. If the v3 API is still required, update to the newest compatible v3.x patch; otherwise assess the documented v3-to-v4 breaking changes and upgrade to the current v4 release only after confirming compatibility.
229-252: 🚀 Performance & Scalability | 🔵 TrivialConsider autoscaling for the load-testing mock service.
aws_ecs_service.android_amapi_mocksetsdesired_count = 1with a single Fargate task atcpu = 256/memory = 512(Line 186-187). Since the PR's goal is to simulate Android devices "at scale," a single small mock instance risks becoming the bottleneck during high-volume enrollment/sync/command load tests, which would skew results toward the mock rather than the Fleet server under test.Consider adding an
aws_appautoscaling_target/policyfor this service, or at minimum exposedesired_countand task sizing as variables so load-test runs can scale the mock independently.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@infrastructure/loadtesting/terraform/android_amapi_mock/main.tf` around lines 229 - 252, Make aws_ecs_service.android_amapi_mock independently scalable for load tests by exposing its desired_count and task CPU/memory settings as variables, or by adding ECS Service Application Auto Scaling with an aws_appautoscaling_target and policy. Ensure the service task definition and service configuration consume the configurable sizing values while preserving current defaults.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/android-amapi-mock/handlers.go`:
- Around line 392-398: Replace the loop over store.byESID with a lookup of the
single device identified by hostUUID using store.getByESID, then assign certIDs
while locking only that device’s mutex. Preserve the existing behavior for the
matching device and avoid updating any other device’s PendingCertificates.
In `@cmd/osquery-perf/android_agent.go`:
- Around line 433-438: Add a finite timeout to the HTTP requests created in
getCertificateTemplate and updateCertificateStatus, replacing reliance on
http.DefaultClient’s unlimited timeout. Apply the same timeout behavior to both
the GET certificate request and the PUT status-update request, while preserving
existing request handling and error propagation.
- Around line 202-212: Update the error branches for registerWithProxy() and
sendEnrollment() in the agent setup flow to call
a.stats.IncrementAndroidErrors() before returning, while preserving the existing
logging and return behavior.
In `@infrastructure/loadtesting/terraform/android_amapi_mock/main.tf`:
- Around line 164-213: The aws_iam_role_policy.android_mock_secrets currently
grants secret access through the shared ecs_execution_arn role; replace this
with a task-specific execution role for
aws_ecs_task_definition.android_amapi_mock, or an equivalent secret resource
policy restricted to that task’s dedicated role. Update the task definition’s
execution_role_arn and policy attachment so enabling google forwarding never
grants access to other tasks using the shared role.
---
Outside diff comments:
In `@cmd/android-amapi-mock/handlers.go`:
- Around line 382-390: In the application-processing loop around
CertificateTemplateIDs, replace the early return when certIDs is empty with
continue so only the current application is skipped. Preserve processing for
subsequent applications in req.Applications.
---
Nitpick comments:
In `@infrastructure/loadtesting/terraform/android_amapi_mock/main.tf`:
- Around line 9-12: Review the `kreuzwerker/docker` provider constraint in the
Terraform `docker` block and verify whether version 3.6.2 is required by v3 API
dependencies. If the v3 API is still required, update to the newest compatible
v3.x patch; otherwise assess the documented v3-to-v4 breaking changes and
upgrade to the current v4 release only after confirming compatibility.
- Around line 229-252: Make aws_ecs_service.android_amapi_mock independently
scalable for load tests by exposing its desired_count and task CPU/memory
settings as variables, or by adding ECS Service Application Auto Scaling with an
aws_appautoscaling_target and policy. Ensure the service task definition and
service configuration consume the configurable sizing values while preserving
current defaults.
In `@infrastructure/loadtesting/terraform/docker/android-amapi-mock.Dockerfile`:
- Line 4: Quote the TAG variable in the git clone command so its expansion is
passed as a single argument without word splitting or glob expansion; update the
existing clone invocation while preserving all other options.
- Line 1: Update the pinned base image versions in the Dockerfile to the latest
requested Go and Alpine patch releases, and replace the existing sha256 digest
with the digest matching the new golang:1.26.5-alpine3.23.5 image. Apply the
same version and digest update to the corresponding Alpine pin on the referenced
line.
In `@infrastructure/loadtesting/terraform/shared/output.tf`:
- Around line 29-31: Mark the android_google_credentials output as sensitive by
adding sensitive = true to its output block, matching the convention used by
sibling infrastructure outputs while preserving its existing value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c9d4e21a-10a3-4690-b382-8439f4f3dd77
📒 Files selected for processing (9)
cmd/android-amapi-mock/handlers.gocmd/osquery-perf/android_agent.goinfrastructure/loadtesting/terraform/android_amapi_mock/main.tfinfrastructure/loadtesting/terraform/android_amapi_mock/outputs.tfinfrastructure/loadtesting/terraform/android_amapi_mock/variables.tfinfrastructure/loadtesting/terraform/docker/android-amapi-mock.Dockerfileinfrastructure/loadtesting/terraform/infra/locals.tfinfrastructure/loadtesting/terraform/shared/enroll.tfinfrastructure/loadtesting/terraform/shared/output.tf
| resource "aws_iam_role_policy" "android_mock_secrets" { | ||
| count = var.enable_google_forwarding ? 1 : 0 | ||
| name = "${local.customer}-android-mock-secrets" | ||
| role = basename(data.terraform_remote_state.infra.outputs.ecs_execution_arn) | ||
| policy = jsonencode({ | ||
| Version = "2012-10-17" | ||
| Statement = [ | ||
| { | ||
| Effect = "Allow" | ||
| Action = ["secretsmanager:GetSecretValue"] | ||
| Resource = [data.terraform_remote_state.shared.outputs.android_google_credentials.arn] | ||
| } | ||
| ] | ||
| }) | ||
| } | ||
|
|
||
| # ---- ECS Task Definition ---- | ||
|
|
||
| resource "aws_ecs_task_definition" "android_amapi_mock" { | ||
| family = "${local.customer}-android-mock" | ||
| requires_compatibilities = ["FARGATE"] | ||
| network_mode = "awsvpc" | ||
| cpu = 256 | ||
| memory = 512 | ||
| execution_role_arn = data.terraform_remote_state.infra.outputs.ecs_execution_arn | ||
| task_role_arn = data.terraform_remote_state.infra.outputs.ecs_arn | ||
|
|
||
| container_definitions = jsonencode([ | ||
| { | ||
| name = "android-amapi-mock" | ||
| image = docker_registry_image.android_amapi_mock.name | ||
| essential = true | ||
|
|
||
| portMappings = [ | ||
| { | ||
| containerPort = 9999 | ||
| protocol = "tcp" | ||
| } | ||
| ] | ||
|
|
||
| command = ["--listen", ":9999"] | ||
|
|
||
| environment = [] | ||
|
|
||
| secrets = var.enable_google_forwarding ? [ | ||
| { | ||
| name = "GOOGLE_CREDENTIALS" | ||
| valueFrom = data.terraform_remote_state.shared.outputs.android_google_credentials.arn | ||
| } | ||
| ] : [] |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Scope the secret-read grant to this task instead of the shared execution role.
aws_iam_role_policy.android_mock_secrets attaches an inline policy directly to basename(data.terraform_remote_state.infra.outputs.ecs_execution_arn). This ARN is exported from the "infra" module as module.loadtest.byo-db.byo-ecs.execution_iam_role_arn, which is the execution role shared by the primary load-test ECS service, not a role scoped to this mock task alone.
When enable_google_forwarding is true, every task using that shared execution role — including the main Fleet load-test server — gains secretsmanager:GetSecretValue on the Android Google credentials secret. This broadens access beyond the mock task and violates least privilege. Create or request a dedicated execution role for the android-amapi-mock task, or scope this permission through a resource policy on the secret restricted to the specific task, rather than modifying a role owned by another Terraform state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@infrastructure/loadtesting/terraform/android_amapi_mock/main.tf` around lines
164 - 213, The aws_iam_role_policy.android_mock_secrets currently grants secret
access through the shared ecs_execution_arn role; replace this with a
task-specific execution role for aws_ecs_task_definition.android_amapi_mock, or
an equivalent secret resource policy restricted to that task’s dedicated role.
Update the task definition’s execution_role_arn and policy attachment so
enabling google forwarding never grants access to other tasks using the shared
role.
There was a problem hiding this comment.
We would need a dedicated execution role for this which is non-trivial. This is a load testing environment. I think we can let this go.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
infrastructure/loadtesting/terraform/android_amapi_mock/outputs.tf:4
mock_urlis intended to be used asFLEET_DEV_ANDROID_PROXY_ENDPOINT, which is passed directly togoogle.golang.org/api/option.WithEndpoint. The docs and existing infra locals set this with a trailing/; without it, URL joining can produce incorrect request paths depending on how the client constructs endpoints.
output "mock_url" {
description = "Internal URL for the Android AMAPI mock (use as FLEET_DEV_ANDROID_PROXY_ENDPOINT and --android_proxy_address)"
value = "http://${data.terraform_remote_state.infra.outputs.internal_alb_dns_name}"
}
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
infrastructure/loadtesting/terraform/android_amapi_mock/outputs.tf:3
mock_urlis documented as a drop-in value forFLEET_DEV_ANDROID_PROXY_ENDPOINT, which elsewhere is shown with a trailing/(and Fleet’s default proxy endpoint includes one). Adding the trailing slash here avoids subtle URL-join differences if callers append paths.
value = "http://${data.terraform_remote_state.infra.outputs.internal_alb_dns_name}"
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (2)
infrastructure/loadtesting/terraform/android_amapi_mock/main.tf:177
GOOGLE_CREDENTIALSsecret is encrypted with the shared CMK (aws_kms_key.mainin shared). Granting onlysecretsmanager:GetSecretValueis typically insufficient for ECS runtime secret retrieval with a customer-managed KMS key; the execution role also needskms:Decrypt(and usuallykms:DescribeKey) on the CMK. Without it, tasks may fail at startup with KMS access denied whenenable_google_forwardingis true.
policy = jsonencode({
Version = "2012-10-17"
Statement = [
{
Effect = "Allow"
Action = ["secretsmanager:GetSecretValue"]
Resource = [data.terraform_remote_state.shared.outputs.android_google_credentials.arn]
}
infrastructure/loadtesting/terraform/android_amapi_mock/outputs.tf:3
FLEET_DEV_ANDROID_PROXY_ENDPOINT/option.WithEndpointexpects a base endpoint that ends with/(the default ishttps://fleetdm.com/api/android/, and docs examples also include a trailing slash). This output omits the trailing/, which can lead to incorrect request URL resolution depending on the client. Consider adding the trailing slash for consistency withinfra/locals.tf.
value = "http://${data.terraform_remote_state.infra.outputs.internal_alb_dns_name}"
|
@rfairburn I cleaned up this PR and removed all the merge conflicts from main. It should have only the relevant terraform changes needed to the load testing environment to spin up the additional android management mock api required to perform android load testing. Let me know if you have any concerns. |
Related issue: Resolves #26225
Testing
Summary by CodeRabbit