[prpixie]

Summary

Moves BuildBot from the testing Application Load Balancer to the internal one, and puts an API Gateway HTTP API in front of it, created by the stack. Clients call the API Gateway custom domain for ApiHost. It forwards only the six application routes through a stack-owned VPC Link to the internal ALB listener, which routes to the ECS service as before.

The change also restructures cloudformation.json into the shared template format (#427) and sets the ECS service name explicitly.

Resolved values are unchanged from main; the shared-format work only moves where they are declared.

Changes

1. Shared template format (#427)

• Parameters are declared only where .github/actions/cloudformation-deploy/action.yml passes them: ApplicationName, ApplicationContainer, ApplicationVersion, ApiHost, LoadBalancerHostName, LoadBalancerPriority and AwsContainerRegistry.
• Everything else lives in a Settings mapping, which replaces Deployment.
• Secrets use the long-form Fn::Sub, with a Fn::FindInMap for the Secrets Manager ARN, because the short form cannot read a mapping.

2. Internal load balancer

• ListenerRule moves to the internal listener (arn:aws:elasticloadbalancing:eu-west-1:117769150821:listener/app/internal/fbd77c1a39a60f7f/e01c11e315c3ff6e) at priority 50, taken from LoadBalancerPriority.
• TargetGroup no longer has a Name, so CloudFormation can replace it. The ECS service's LoadBalancers block points at the replacement.

3. API Gateway HTTP API

• VPC Link: a stack-owned AWS::ApiGatewayV2::VpcLink on the service's subnets, with its own security group. The only rule is egress TCP 443 to 172.31.0.0/16, with no ingress.
• Integration: one HTTP_PROXY integration over the VPC Link to the internal listener. It overwrites Host with LoadBalancerHostName and X-Client-Ip with $context.identity.sourceIp, so a client cannot spoof either. TLS is verified against buildbot.funfair.io (TlsConfig.ServerNameToVerify).
• Routes: six routes and no catch-all: GET /ping, POST /github/ping, POST /github/push, POST /github/status, POST /watchtower/deploy and POST /cloudformation/deploy. Any other path gets API Gateway's own 404.
• Stage: $default, with auto-deploy.
• Custom domain: a regional domain for ApiHost using the existing ACM certificate, plus an API mapping to the stage.
• DNS: a stack-owned Route 53 A alias record in funfair.io. pointing at the custom domain. It has DeletionPolicy and UpdateReplacePolicy set to Retain. Route 53 record sets cannot be imported, so the existing buildbot.funfair.io record is deleted by hand before the first deploy (see the Deploy checklist).

4. X-Client-Ip in the webhook rejection log

• GitHubWebhookSignatureMiddleware reads the X-Client-Ip header and logs it as ClientIp, sanitised like the other client-supplied values. The rejection message gains client ip: {ClientIp}.
• Behind the gateway, X-Forwarded-For only holds the VPC Link's private address, so X-Client-Ip is the real caller.

5. ECS ServiceName fix

• ECSService now sets ServiceName to ApplicationName explicitly.

6. README

• New Deployment section covering the gateway routes, the 10 MB payload limit, X-Client-Ip, authentication guidance (use the Authorization header, never X-Api-Key), the VPC Link's 60-day inactivity limit, and the DNS cut-over steps.

Deploy checklist

For the owner:

• Delete the existing buildbot.funfair.io record just before the first deploy, keeping its values (the alias target and EvaluateTargetHealth, plus any AAAA record for the name). The README has the commands.
• The first deploy replaces the ECS service, the target group and the listener rule. The old and new services briefly overlap while this happens.
• If the deploy rolls back, delete the retained buildbot.funfair.io record again before retrying, because the Retain policy leaves it in place and the retry would otherwise fail.
• Check webhook deliveries afterwards: valid deliveries should return 204 through the gateway, and an unsigned delivery should return the app's 401.

Closes #421
Closes #427

[credfeto]

Roslyn analyzer findings

Source	Rule	Level	File	Line	Suppressed	Message
DotNet	IL2026	error	src/BuildBot/Helpers/Endpoints.CloudFormation.cs	26	yes	Using member 'Microsoft.AspNetCore.Builder.EndpointRouteBuilderExtensions.MapPost(IEndpointRouteBuilder, String, Delegate)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. This API may perform reflection on the supplied delegate and its parameters. These types may be trimmed if not directly referenced.
DotNet	CA2000	error	src/BuildBot/Helpers/ServerStartup.cs	124	yes	Call System.IDisposable.Dispose on object created by 'CreateLogger()' before all references to it are out of scope
DotNet	IL2026	error	src/BuildBot/Helpers/Endpoints.Watchtower.cs	19	yes	Using member 'Microsoft.AspNetCore.Builder.EndpointRouteBuilderExtensions.MapPost(IEndpointRouteBuilder, String, Delegate)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. This API may perform reflection on the supplied delegate and its parameters. These types may be trimmed if not directly referenced.
DotNet	IL2026	error	src/BuildBot/Helpers/Endpoints.Test.cs	22	yes	Using member 'Microsoft.AspNetCore.Builder.EndpointRouteBuilderExtensions.MapGet(IEndpointRouteBuilder, String, Delegate)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. This API may perform reflection on the supplied delegate and its parameters. These types may be trimmed if not directly referenced.
DotNet	IL2026	error	src/BuildBot/obj/Release/net11.0/Microsoft.AspNetCore.Http.RequestDelegateGenerator/Microsoft.AspNetCore.Http.RequestDelegateGenerator.RequestDelegateGenerator/GeneratedRouteBuilderExtensions.g.cs	1014	yes	Using member 'Microsoft.AspNetCore.Http.HttpResponseJsonExtensions.WriteAsJsonAsync(HttpResponse, TValue, JsonSerializerOptions, CancellationToken)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. JSON serialization and deserialization might require types that cannot be statically analyzed. Use the overload that takes a JsonTypeInfo or JsonSerializerContext, or make sure all of the required types are preserved.
DotNet	IL2026	error	src/BuildBot/Helpers/Endpoints.GitHub.cs	34	yes	Using member 'Microsoft.AspNetCore.Builder.EndpointRouteBuilderExtensions.MapPost(IEndpointRouteBuilder, String, Delegate)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. This API may perform reflection on the supplied delegate and its parameters. These types may be trimmed if not directly referenced.
DotNet	IL2026	error	src/BuildBot/Helpers/Endpoints.GitHub.cs	44	yes	Using member 'Microsoft.AspNetCore.Builder.EndpointRouteBuilderExtensions.MapPost(IEndpointRouteBuilder, String, Delegate)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. This API may perform reflection on the supplied delegate and its parameters. These types may be trimmed if not directly referenced.
DotNet	IL2026	error	src/BuildBot/Helpers/Endpoints.GitHub.cs	54	yes	Using member 'Microsoft.AspNetCore.Builder.EndpointRouteBuilderExtensions.MapPost(IEndpointRouteBuilder, String, Delegate)' which has 'RequiresUnreferencedCodeAttribute' can break functionality when trimming application code. This API may perform reflection on the supplied delegate and its parameters. These types may be trimmed if not directly referenced.

[credfeto]

Live-chat answers from the repository owner:

q12 yes q13 yes q14 yes i've already answered that q15 yes

Resolved:

• Q12: the DOCKER_CONTAINER_REGISTRY_RELEASE / _PRE_RELEASE variables hold 117769150821.dkr.ecr.eu-west-1.amazonaws.com, so the image URI is unchanged.
• Q13: the task security groups allow the internal ALB on 8080.
• Q14: priorities 50 and 60 are free on the internal listener (already covered by Q7/Q10 on Move to the internal load balancer behind an API Gateway #421).
• Q15: this PR also fixes the ECS service Name -> ServiceName property, as its own change after the API Gateway change. It replaces the service on the next deploy.

[credfeto]

Repository owner in live chat: "q18 yes". The three subnets' AZs are supported for VPC Link V2 in eu-west-1. Q19 (whether funfair.io. is the only hosted zone with that name) is still open.

[credfeto]

Repository owner in live chat: "q19 there's ONLY one zone". funfair.io. is the only hosted zone with that name, so looking the zone up by name is safe.

[credfeto]

Correcting the PR body: the previous body said the Route 53 record would be imported into the stack. That is no longer the plan. CloudFormation cannot import AWS::Route53::RecordSet, so per the owner's answer to Q16 (b) (see #421), the existing buildbot.funfair.io record is deleted by hand just before the first deploy and the stack creates and owns the record, with Retain as its deletion policy. The body now states this, together with the Deploy checklist.

[credfeto]

Simplify clean - advancing to code review

[credfeto]

Coding Researcher

Question 1: Does HTTP API (API Gateway v2) access logging to CloudWatch need the account-level API Gateway CloudWatch role (AWS::ApiGateway::Account / cloudWatchRoleArn), as REST APIs do?

Outcome: No (high confidence). HTTP APIs use CloudWatch Logs vended-log delivery. The service principal delivery.logs.amazonaws.com writes the logs, and an automatically created log-group resource policy grants it access, so there is no IAM role. The principal that deploys the stack needs logs:CreateLogDelivery, logs:PutResourcePolicy, logs:UpdateLogDelivery, logs:DeleteLogDelivery, logs:CreateLogGroup, logs:DescribeResourcePolicies, logs:GetLogDelivery and logs:ListLogDeliveries on *. It also needs logs:DescribeLogGroups (with DescribeLogStreams, GetLogEvents and FilterLogEvents) on log-group:*. A known failure mode is the 5,120-character limit on account-level CloudWatch Logs resource policies in accounts with many vended-log deliveries. Sources: https://docs.aws.amazon.com/apigateway/latest/developerguide/http-api-logging.html, https://docs.aws.amazon.com/apigateway/latest/developerguide/set-up-logging.html, https://docs.aws.amazon.com/AmazonCloudWatch/latest/logs/AWS-logs-infrastructure-CWL.html

Question 2: Does AWS::ApiGatewayV2::Stage AccessLogSettings.DestinationArn accept Fn::GetAtt <LogGroup>.Arn, which ends in :*?

Outcome: Yes (medium-high confidence). AWS's SAM AWS::Serverless::HttpApi reference example passes !GetAtt AccessLogs.Arn straight through to a v2 Stage's AccessLogSettings. Source: https://docs.aws.amazon.com/serverless-application-model/latest/developerguide/sam-resource-httpapi.html

[credfeto]

Coverage ratchet passed - advancing to Human Review (.NET overall 99.2%, equal to main)

[credfeto]

AI Review Loop: reviewed 4191813

[credfeto]

Super-linter summary

Language	Validation result
CLOUDFORMATION	Pass ✅
JSON	Pass ✅

All files and directories linted successfully

For more information, see the GitHub Actions workflow run

Powered by Super-linter