From 8f888097680dbb6c8cb432fc329c59fbbd0d24ce Mon Sep 17 00:00:00 2001 From: John Simons Date: Wed, 9 Sep 2026 08:24:30 +1000 Subject: [PATCH 01/14] Add GitHub Actions workflows and provisioning scripts for cloud database tests Introduces two workflows and supporting PowerShell scripts to run the persistence and acceptance test suites against managed cloud databases (Azure SQL, Azure Database for PostgreSQL, Aurora PostgreSQL, RDS SQL Server). The test workflow provisions a fresh database per run, executes the relevant test projects against it, and tears it down regardless of outcome. A nightly cleanup workflow sweeps any resources tagged `sc-cloud-test` that are older than four hours. --- .../cloud-database-tests-cleanup.yml | 113 +++++++++++++++ .github/workflows/cloud-database-tests.yml | 131 ++++++++++++++++++ tools/cloud/aurora-postgresql.ps1 | 95 +++++++++++++ tools/cloud/azure-postgresql.ps1 | 64 +++++++++ tools/cloud/azure-sql.ps1 | 73 ++++++++++ tools/cloud/common.ps1 | 109 +++++++++++++++ tools/cloud/rds-sqlserver.ps1 | 90 ++++++++++++ tools/cloud/verify-sqlserver.cs | 52 +++++++ 8 files changed, 727 insertions(+) create mode 100644 .github/workflows/cloud-database-tests-cleanup.yml create mode 100644 .github/workflows/cloud-database-tests.yml create mode 100644 tools/cloud/aurora-postgresql.ps1 create mode 100644 tools/cloud/azure-postgresql.ps1 create mode 100644 tools/cloud/azure-sql.ps1 create mode 100644 tools/cloud/common.ps1 create mode 100644 tools/cloud/rds-sqlserver.ps1 create mode 100644 tools/cloud/verify-sqlserver.cs diff --git a/.github/workflows/cloud-database-tests-cleanup.yml b/.github/workflows/cloud-database-tests-cleanup.yml new file mode 100644 index 0000000000..974022efc8 --- /dev/null +++ b/.github/workflows/cloud-database-tests-cleanup.yml @@ -0,0 +1,113 @@ +name: Cloud database tests cleanup +on: + schedule: + - cron: '0 3 * * *' + workflow_dispatch: +env: + DOTNET_NOLOGO: true +defaults: + run: + shell: pwsh +jobs: + azure: + name: Azure + runs-on: ubuntu-latest + steps: + - name: Check for secrets + env: + SECRETS_AVAILABLE: ${{ secrets.SECRETS_AVAILABLE }} + run: exit $(If ($env:SECRETS_AVAILABLE -eq 'true') { 0 } Else { 1 }) + - name: Azure login + uses: azure/login@v3.0.2 + with: + creds: ${{ secrets.AZURE_CLOUD_TEST_CREDENTIALS }} + - name: Delete leaked resource groups + run: | + $cutoff = [DateTimeOffset]::UtcNow.AddHours(-4).ToUnixTimeSeconds() + $groups = az group list --tag sc-cloud-test=true --query '[].{name:name, created:tags.created}' --output json | ConvertFrom-Json + + if (-not $groups) { + Write-Output 'Nothing tagged sc-cloud-test is left in this subscription.' + return + } + + foreach ($group in $groups) { + if (-not $group.created) { + Write-Warning "Resource group $($group.name) has no created tag, so its age is unknown. Leaving it, delete it by hand." + continue + } + + if ([long]$group.created -ge $cutoff) { + Write-Output "Leaving $($group.name), it belongs to a run that may still be going." + continue + } + + Write-Output "Deleting $($group.name)" + try { + az group delete --name $group.name --yes --no-wait --only-show-errors + } + catch { + Write-Warning "Could not delete $($group.name): $($_.Exception.Message)" + } + } + + aws: + name: AWS + runs-on: ubuntu-latest + steps: + - name: Check for secrets + env: + SECRETS_AVAILABLE: ${{ secrets.SECRETS_AVAILABLE }} + run: exit $(If ($env:SECRETS_AVAILABLE -eq 'true') { 0 } Else { 1 }) + - name: Setup AWS environment variables + run: | + echo "AWS_REGION=${{ secrets.AWS_REGION }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append + echo "AWS_ACCESS_KEY_ID=${{ secrets.AWS_ACCESS_KEY_ID }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append + echo "AWS_SECRET_ACCESS_KEY=${{ secrets.AWS_SECRET_ACCESS_KEY }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append + - name: Delete leaked RDS resources and security groups + run: | + $cutoff = [DateTimeOffset]::UtcNow.AddHours(-4).ToUnixTimeSeconds() + $tagged = aws resourcegroupstaggingapi get-resources --tag-filters Key=sc-cloud-test,Values=true --output json | ConvertFrom-Json + + $stale = @() + foreach ($resource in $tagged.ResourceTagMappingList) { + $created = ($resource.Tags | Where-Object { $_.Key -eq 'created' }).Value + + if (-not $created) { + Write-Warning "$($resource.ResourceARN) has no created tag, so its age is unknown. Leaving it, delete it by hand." + continue + } + + if ([long]$created -ge $cutoff) { + Write-Output "Leaving $($resource.ResourceARN), it belongs to a run that may still be going." + continue + } + + $stale += $resource.ResourceARN + } + + if (-not $stale) { + Write-Output 'Nothing stale is tagged sc-cloud-test in this account.' + return + } + + # Instances first, then the clusters that hold them, then the security groups the instances + # were using. A security group still in use refuses to go, and is picked up by the next run. + $order = @(':db:', ':cluster:', 'security-group/') + + foreach ($pattern in $order) { + foreach ($arn in $stale | Where-Object { $_ -like "*$pattern*" }) { + $identifier = ($arn -split '[:/]')[-1] + Write-Output "Deleting $arn" + try { + switch -Wildcard ($arn) { + '*:db:*' { aws rds delete-db-instance --db-instance-identifier $identifier --skip-final-snapshot --delete-automated-backups --no-cli-pager --output none; break } + '*:cluster:*' { aws rds delete-db-cluster --db-cluster-identifier $identifier --skip-final-snapshot --no-cli-pager --output none; break } + '*security-group/*' { aws ec2 delete-security-group --group-id $identifier --output none; break } + } + } + catch { + Write-Warning "Could not delete ${arn}: $($_.Exception.Message)" + } + } + } diff --git a/.github/workflows/cloud-database-tests.yml b/.github/workflows/cloud-database-tests.yml new file mode 100644 index 0000000000..7dc8a603eb --- /dev/null +++ b/.github/workflows/cloud-database-tests.yml @@ -0,0 +1,131 @@ +name: Cloud database tests +on: + workflow_dispatch: + inputs: + target: + description: Which managed database to test against + required: true + default: all + type: choice + options: + - all + - azure-sql + - azure-postgresql + - aurora-postgresql + - rds-sqlserver + push: + tags: + - '[0-9]+.[0-9]+.[0-9]+' + - '[0-9]+.[0-9]+.[0-9]+-*' +env: + DOTNET_NOLOGO: true +defaults: + run: + shell: pwsh +concurrency: + # Deliberately not cancel-in-progress: cancelling a run mid-flight risks skipping the teardown + # step and leaving billable cloud resources behind. + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: false +jobs: + plan: + name: Plan + runs-on: ubuntu-latest + outputs: + targets: ${{ steps.select.outputs.targets }} + steps: + - name: Select targets + id: select + run: | + $all = @( + @{ target = 'azure-sql'; provider = 'SqlServer' } + @{ target = 'azure-postgresql'; provider = 'PostgreSql' } + @{ target = 'aurora-postgresql'; provider = 'PostgreSql' } + @{ target = 'rds-sqlserver'; provider = 'SqlServer' } + ) + + # A tag push has no input, and runs everything. + $requested = '${{ inputs.target }}' + $targets = if (-not $requested -or $requested -eq 'all') { $all } else { $all | Where-Object { $_.target -eq $requested } } + + if (-not $targets) { + throw "No cloud target named '$requested'." + } + + $targets | ForEach-Object { Write-Output "Will test against $($_.target)" } + + $matrix = @{ include = @($targets) } + "targets=$(ConvertTo-Json -InputObject $matrix -Compress -Depth 4)" | Out-File -FilePath $Env:GITHUB_OUTPUT -Encoding utf8 -Append + + test: + name: ${{ matrix.target }} + needs: plan + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: ${{ fromJSON(needs.plan.outputs.targets) }} + # The two suites that exercise the persister. Named outright rather than selected by test + # category, because the category also carries the transport tests, which test the queue + # transport against its own connection string and have no business running here. + env: + PERSISTENCE_PROJECT: src/ServiceControl.Persistence.Tests.${{ matrix.provider }}/ServiceControl.Persistence.Tests.${{ matrix.provider }}.csproj + ACCEPTANCE_PROJECT: src/ServiceControl.AcceptanceTests.${{ matrix.provider }}/ServiceControl.AcceptanceTests.${{ matrix.provider }}.csproj + steps: + - name: Check for secrets + env: + SECRETS_AVAILABLE: ${{ secrets.SECRETS_AVAILABLE }} + run: exit $(If ($env:SECRETS_AVAILABLE -eq 'true') { 0 } Else { 1 }) + - name: Checkout + uses: actions/checkout@v7.0.1 + with: + fetch-depth: 0 + - name: Setup .NET SDK + uses: actions/setup-dotnet@v6.0.0 + with: + global-json-file: global.json + - name: Determine runtime version + run: | + # Read settings from Custom.Build.props + [xml]$xml = Get-Content ./src/Custom.Build.props + $runtimeVersion = $xml.selectNodes('/Project/PropertyGroup/RuntimeFrameworkVersion').InnerText + if (-not ($runtimeVersion)) { + throw "Missing RuntimeFrameworkVersion setting in Custom.Build.props" + } + echo "RuntimeVersion=$runtimeVersion" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append + - name: Install .NET Runtime + shell: bash + run: | + ./tools/dotnet-install.sh --skip-non-versioned-files --install-dir /usr/share/dotnet --runtime dotnet --version ${{ env.RuntimeVersion }} + ./tools/dotnet-install.sh --skip-non-versioned-files --install-dir /usr/share/dotnet --runtime aspnetcore --version ${{ env.RuntimeVersion }} + - name: Build + id: build + background: true + run: | + dotnet build $Env:PERSISTENCE_PROJECT --configuration Release + dotnet build $Env:ACCEPTANCE_PROJECT --configuration Release + - name: Azure login + uses: azure/login@v3.0.2 + if: startsWith(matrix.target, 'azure-') + with: + creds: ${{ secrets.AZURE_CLOUD_TEST_CREDENTIALS }} + - name: Setup AWS environment variables + if: startsWith(matrix.target, 'aurora-') || startsWith(matrix.target, 'rds-') + run: | + echo "AWS_REGION=${{ secrets.AWS_REGION }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append + echo "AWS_ACCESS_KEY_ID=${{ secrets.AWS_ACCESS_KEY_ID }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append + echo "AWS_SECRET_ACCESS_KEY=${{ secrets.AWS_SECRET_ACCESS_KEY }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append + # Unlike ci.yml, which holds its cloud resources back until the build is done, provisioning + # starts first: it is the long pole here at anywhere from 3 to 25 minutes, and the teardown + # step runs on failure regardless. + - name: Provision database + run: ./tools/cloud/${{ matrix.target }}.ps1 -Action Provision -Name sc-ct-${{ github.run_id }} + - name: Wait for build + wait: build + - name: Run tests + run: ./tools/run-tests.ps1 -Projects "$Env:PERSISTENCE_PROJECT`n$Env:ACCEPTANCE_PROJECT" -MaxParallel 2 + env: + ServiceControl_TESTS_FILTER: ${{ matrix.provider }} + PARTICULARSOFTWARE_LICENSE: ${{ secrets.LICENSETEXT }} + - name: Tear down database + if: always() + run: ./tools/cloud/${{ matrix.target }}.ps1 -Action Teardown -Name sc-ct-${{ github.run_id }} diff --git a/tools/cloud/aurora-postgresql.ps1 b/tools/cloud/aurora-postgresql.ps1 new file mode 100644 index 0000000000..464943b62f --- /dev/null +++ b/tools/cloud/aurora-postgresql.ps1 @@ -0,0 +1,95 @@ +# Provisions and tears down an Aurora PostgreSQL cluster for one run of the cloud database tests. + +param( + [Parameter(Mandatory)][ValidateSet('Provision', 'Teardown')][string]$Action, + [Parameter(Mandatory)][string]$Name, + [string]$DatabaseName = 'servicecontrol' +) + +. $PSScriptRoot/common.ps1 + +$instance = "$Name-1" +$adminUser = 'sctestadmin' + +if ($Action -eq 'Teardown') { + # Without waiting for the deletions to finish: an RDS instance takes minutes to disappear, and + # holding the job open for that costs more than the scheduled cleanup workflow does. + Invoke-Teardown "Deleting instance $instance" { + aws rds delete-db-instance --db-instance-identifier $instance --skip-final-snapshot --delete-automated-backups --no-cli-pager --output none + } + + Invoke-Teardown "Deleting cluster $Name" { + aws rds delete-db-cluster --db-cluster-identifier $Name --skip-final-snapshot --no-cli-pager --output none + } + + # Will refuse while the instance still holds it, which is the normal case. The cleanup workflow + # sweeps up whatever is left. + Invoke-Teardown "Deleting security group $Name" { + $groupId = aws ec2 describe-security-groups --filters "Name=group-name,Values=$Name" --query 'SecurityGroups[0].GroupId' --output text + if ($groupId -and $groupId -ne 'None') { + aws ec2 delete-security-group --group-id $groupId --output none + } + } + + return +} + +$password = New-AdminPassword +$runnerIp = Get-RunnerIpAddress +$created = Get-CreatedTimestamp + +$vpcId = aws ec2 describe-vpcs --filters 'Name=isDefault,Values=true' --query 'Vpcs[0].VpcId' --output text +if (-not $vpcId -or $vpcId -eq 'None') { + throw 'This AWS account has no default VPC in this region, so there is no public subnet group for the cluster to use.' +} + +Write-Step "Creating security group $Name in $vpcId, allowing $runnerIp on 5432" +$groupId = aws ec2 create-security-group ` + --group-name $Name ` + --description 'ServiceControl cloud database tests' ` + --vpc-id $vpcId ` + --tag-specifications "ResourceType=security-group,Tags=[{Key=sc-cloud-test,Value=true},{Key=run-id,Value=$Name},{Key=created,Value=$created}]" ` + --query GroupId --output text + +aws ec2 authorize-security-group-ingress ` + --group-id $groupId ` + --protocol tcp ` + --port 5432 ` + --cidr "$runnerIp/32" ` + --output none + +# The engine version is left to the AWS default so that this does not break every time a pinned +# minor version is retired. +Write-Step "Creating Aurora PostgreSQL cluster $Name" +aws rds create-db-cluster ` + --db-cluster-identifier $Name ` + --engine aurora-postgresql ` + --master-username $adminUser ` + --master-user-password $password ` + --database-name $DatabaseName ` + --vpc-security-group-ids $groupId ` + --no-deletion-protection ` + --backup-retention-period 1 ` + --tags "Key=sc-cloud-test,Value=true" "Key=run-id,Value=$Name" "Key=created,Value=$created" ` + --no-cli-pager --output none + +Write-Step "Creating instance $instance" +aws rds create-db-instance ` + --db-instance-identifier $instance ` + --db-cluster-identifier $Name ` + --engine aurora-postgresql ` + --db-instance-class db.t4g.medium ` + --publicly-accessible ` + --tags "Key=sc-cloud-test,Value=true" "Key=run-id,Value=$Name" "Key=created,Value=$created" ` + --no-cli-pager --output none + +Write-Step 'Waiting for the instance to become available' +aws rds wait db-instance-available --db-instance-identifier $instance + +$endpoint = aws rds describe-db-clusters --db-cluster-identifier $Name --query 'DBClusters[0].Endpoint' --output text + +# Trust Server Certificate because RDS presents an Amazon CA that is not in the runner's trust store. +# The connection is still encrypted; only the certificate chain goes unverified. +$connectionString = "Host=$endpoint;Port=5432;Database=$DatabaseName;Username=$adminUser;Password=$password;Ssl Mode=Require;Trust Server Certificate=true;Timeout=60" + +Set-PersistenceConnectionString -Provider PostgreSql -ConnectionString $connectionString diff --git a/tools/cloud/azure-postgresql.ps1 b/tools/cloud/azure-postgresql.ps1 new file mode 100644 index 0000000000..2a1bee0dfc --- /dev/null +++ b/tools/cloud/azure-postgresql.ps1 @@ -0,0 +1,64 @@ +# Provisions and tears down an Azure Database for PostgreSQL flexible server for one run of the +# cloud database tests. +# +# Everything lives in a single resource group named after the run, so teardown is one call and a +# partly provisioned run cleans up as completely as a successful one. + +param( + [Parameter(Mandatory)][ValidateSet('Provision', 'Teardown')][string]$Action, + [Parameter(Mandatory)][string]$Name, + [string]$Location = 'eastus2', + [string]$DatabaseName = 'servicecontrol' +) + +. $PSScriptRoot/common.ps1 + +$resourceGroup = $Name +$server = $Name +$adminUser = 'sctestadmin' + +if ($Action -eq 'Teardown') { + Invoke-Teardown "Deleting resource group $resourceGroup" { + az group delete --name $resourceGroup --yes --no-wait --only-show-errors + } + return +} + +$password = New-AdminPassword +$runnerIp = Get-RunnerIpAddress +$created = Get-CreatedTimestamp + +Write-Step "Creating resource group $resourceGroup in $Location" +az group create ` + --name $resourceGroup ` + --location $Location ` + --tags sc-cloud-test=true "run-id=$Name" "created=$created" ` + --only-show-errors --output none + +# Burstable B1ms is the cheapest tier, and the suites are latency bound rather than CPU bound. +# --public-access opens the firewall to just this runner as part of creation. +Write-Step "Creating PostgreSQL flexible server $server, allowing $runnerIp" +az postgres flexible-server create ` + --name $server ` + --resource-group $resourceGroup ` + --location $Location ` + --admin-user $adminUser ` + --admin-password $password ` + --tier Burstable ` + --sku-name Standard_B1ms ` + --storage-size 32 ` + --version 16 ` + --public-access $runnerIp ` + --yes ` + --only-show-errors --output none + +Write-Step "Creating database $DatabaseName" +az postgres flexible-server db create ` + --database-name $DatabaseName ` + --resource-group $resourceGroup ` + --server-name $server ` + --only-show-errors --output none + +$connectionString = "Host=$server.postgres.database.azure.com;Port=5432;Database=$DatabaseName;Username=$adminUser;Password=$password;Ssl Mode=Require;Timeout=60" + +Set-PersistenceConnectionString -Provider PostgreSql -ConnectionString $connectionString diff --git a/tools/cloud/azure-sql.ps1 b/tools/cloud/azure-sql.ps1 new file mode 100644 index 0000000000..c46e386196 --- /dev/null +++ b/tools/cloud/azure-sql.ps1 @@ -0,0 +1,73 @@ +# Provisions and tears down an Azure SQL Database for one run of the cloud database tests. +# +# Everything lives in a single resource group named after the run, so teardown is one call and a +# partly provisioned run cleans up as completely as a successful one. + +param( + [Parameter(Mandatory)][ValidateSet('Provision', 'Teardown')][string]$Action, + [Parameter(Mandatory)][string]$Name, + [string]$Location = 'eastus2', + [string]$DatabaseName = 'servicecontrol' +) + +. $PSScriptRoot/common.ps1 + +$resourceGroup = $Name +$server = $Name +$adminUser = 'sctestadmin' + +if ($Action -eq 'Teardown') { + # One call takes the server, the database and the firewall rule with it. --no-wait because + # nothing later in the run depends on the deletion having finished. + Invoke-Teardown "Deleting resource group $resourceGroup" { + az group delete --name $resourceGroup --yes --no-wait --only-show-errors + } + return +} + +$password = New-AdminPassword +$runnerIp = Get-RunnerIpAddress +$created = Get-CreatedTimestamp + +Write-Step "Creating resource group $resourceGroup in $Location" +az group create ` + --name $resourceGroup ` + --location $Location ` + --tags sc-cloud-test=true "run-id=$Name" "created=$created" ` + --only-show-errors --output none + +Write-Step "Creating SQL server $server" +az sql server create ` + --name $server ` + --resource-group $resourceGroup ` + --location $Location ` + --admin-user $adminUser ` + --admin-password $password ` + --only-show-errors --output none + +Write-Step "Allowing $runnerIp through the server firewall" +az sql server firewall-rule create ` + --name github-runner ` + --resource-group $resourceGroup ` + --server $server ` + --start-ip-address $runnerIp ` + --end-ip-address $runnerIp ` + --only-show-errors --output none + +# S0 is the cheapest tier that still provisions in a couple of minutes. Local backup redundancy +# because the database does not outlive the run. +Write-Step "Creating database $DatabaseName" +az sql db create ` + --name $DatabaseName ` + --resource-group $resourceGroup ` + --server $server ` + --service-objective S0 ` + --backup-storage-redundancy Local ` + --only-show-errors --output none + +$connectionString = "Server=tcp:$server.database.windows.net,1433;Initial Catalog=$DatabaseName;User ID=$adminUser;Password=$password;Encrypt=True;TrustServerCertificate=False;Connect Timeout=60" + +Write-Step 'Verifying the database is reachable and has Full-Text Search' +Invoke-SqlServerVerification -ConnectionString $connectionString + +Set-PersistenceConnectionString -Provider SqlServer -ConnectionString $connectionString diff --git a/tools/cloud/common.ps1 b/tools/cloud/common.ps1 new file mode 100644 index 0000000000..91f6dc225a --- /dev/null +++ b/tools/cloud/common.ps1 @@ -0,0 +1,109 @@ +# Shared helpers for the cloud database provisioning scripts in this folder. Dot-sourced, not run +# directly. + +# Native commands (az, aws) fail by returning a non-zero exit code rather than throwing, and a +# provisioning script that carries on after a failed step leaves half-built infrastructure behind. +$PSNativeCommandUseErrorActionPreference = $true +$ErrorActionPreference = 'Stop' + +function Write-Step { + param([Parameter(Mandatory)][string]$Message) + + Write-Output "==> $Message" +} + +# Satisfies the complexity rules of all four services at once, and avoids the characters that would +# have to be escaped in a connection string or are rejected outright by RDS (/ " @ and space). +function New-AdminPassword { + $upper = 'ABCDEFGHJKLMNPQRSTUVWXYZ' + $lower = 'abcdefghijkmnpqrstuvwxyz' + $digit = '23456789' + $symbol = '!#$%*()-_+' + $all = $upper + $lower + $digit + $symbol + + $characters = @( + Get-Random -InputObject $upper.ToCharArray() + Get-Random -InputObject $lower.ToCharArray() + Get-Random -InputObject $digit.ToCharArray() + Get-Random -InputObject $symbol.ToCharArray() + ) + $characters += 1..24 | ForEach-Object { Get-Random -InputObject $all.ToCharArray() } + + $password = -join ($characters | Sort-Object { Get-Random }) + + # Straight to stdout rather than through the output stream, which the caller is capturing as the + # return value. Everything the password is later embedded in, the connection string included, is + # redacted along with it. + [Console]::WriteLine("::add-mask::$password") + + return $password +} + +# The servers are reachable from the internet, so each one is firewalled to the single address this +# job connects from. +function Get-RunnerIpAddress { + try { + return (Invoke-RestMethod -Uri 'https://api.ipify.org' -TimeoutSec 30).Trim() + } + catch { + throw "Could not determine the runner's public IP address, which is needed to open the database firewall to it. $($_.Exception.Message)" + } +} + +# Neither an Azure resource group nor an EC2 security group records when it was created, so the +# scheduled cleanup workflow needs the provisioning scripts to stamp it. Epoch seconds because both +# clouds restrict the characters a tag value may contain. +function Get-CreatedTimestamp { + return [DateTimeOffset]::UtcNow.ToUnixTimeSeconds().ToString() +} + +function Set-PersistenceConnectionString { + param( + [Parameter(Mandatory)][ValidateSet('SqlServer', 'PostgreSql')][string]$Provider, + [Parameter(Mandatory)][string]$ConnectionString + ) + + $name = "ServiceControl_Persistence_${Provider}_ConnectionString" + + if (-not $Env:GITHUB_ENV) { + Write-Step "GITHUB_ENV is not set, so $name was not exported. Set it yourself to run the tests against this server." + return + } + + "$name=$ConnectionString" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf8 -Append + Write-Step "Exported $name" +} + +# Creates the test database if the service could not, and fails the run if the server has no +# Full-Text Search. +function Invoke-SqlServerVerification { + param([Parameter(Mandatory)][string]$ConnectionString) + + # Run from this folder: `dotnet run .cs` picks up any project file in the working + # directory, and by this point the repo root holds the tests.proj that select-test-projects.ps1 + # generated. + Push-Location $PSScriptRoot + try { + dotnet run ./verify-sqlserver.cs -- $ConnectionString + } + finally { + Pop-Location + } +} + +# Teardown runs even when provisioning failed part way, so it has to tolerate resources that were +# never created. +function Invoke-Teardown { + param( + [Parameter(Mandatory)][string]$Description, + [Parameter(Mandatory)][scriptblock]$Action + ) + + Write-Step $Description + try { + & $Action + } + catch { + Write-Warning "$Description failed: $($_.Exception.Message). The scheduled cloud-database-cleanup workflow will pick up anything left behind." + } +} diff --git a/tools/cloud/rds-sqlserver.ps1 b/tools/cloud/rds-sqlserver.ps1 new file mode 100644 index 0000000000..b5aa78ef95 --- /dev/null +++ b/tools/cloud/rds-sqlserver.ps1 @@ -0,0 +1,90 @@ +# Provisions and tears down an RDS SQL Server instance for one run of the cloud database tests. +# +# This is the slowest of the four targets to provision, around 15 to 25 minutes, so the workflow +# starts it before waiting on the build. + +param( + [Parameter(Mandatory)][ValidateSet('Provision', 'Teardown')][string]$Action, + [Parameter(Mandatory)][string]$Name, + [string]$DatabaseName = 'servicecontrol', + # Web edition is the cheapest licence-included option. Full-Text Search availability is what + # decides whether it is usable; verify-sqlserver.cs fails the run with a clear message if this + # edition turns out not to have it, in which case move to sqlserver-se. + [string]$Engine = 'sqlserver-web' +) + +. $PSScriptRoot/common.ps1 + +$adminUser = 'sctestadmin' + +if ($Action -eq 'Teardown') { + Invoke-Teardown "Deleting instance $Name" { + aws rds delete-db-instance --db-instance-identifier $Name --skip-final-snapshot --delete-automated-backups --no-cli-pager --output none + } + + Invoke-Teardown "Deleting security group $Name" { + $groupId = aws ec2 describe-security-groups --filters "Name=group-name,Values=$Name" --query 'SecurityGroups[0].GroupId' --output text + if ($groupId -and $groupId -ne 'None') { + aws ec2 delete-security-group --group-id $groupId --output none + } + } + + return +} + +$password = New-AdminPassword +$runnerIp = Get-RunnerIpAddress +$created = Get-CreatedTimestamp + +$vpcId = aws ec2 describe-vpcs --filters 'Name=isDefault,Values=true' --query 'Vpcs[0].VpcId' --output text +if (-not $vpcId -or $vpcId -eq 'None') { + throw 'This AWS account has no default VPC in this region, so there is no public subnet group for the instance to use.' +} + +Write-Step "Creating security group $Name in $vpcId, allowing $runnerIp on 1433" +$groupId = aws ec2 create-security-group ` + --group-name $Name ` + --description 'ServiceControl cloud database tests' ` + --vpc-id $vpcId ` + --tag-specifications "ResourceType=security-group,Tags=[{Key=sc-cloud-test,Value=true},{Key=run-id,Value=$Name},{Key=created,Value=$created}]" ` + --query GroupId --output text + +aws ec2 authorize-security-group-ingress ` + --group-id $groupId ` + --protocol tcp ` + --port 1433 ` + --cidr "$runnerIp/32" ` + --output none + +# No automated backups and no standby: the instance does not outlive the run, and both slow +# provisioning down. +Write-Step "Creating RDS SQL Server instance $Name" +aws rds create-db-instance ` + --db-instance-identifier $Name ` + --engine $Engine ` + --db-instance-class db.t3.small ` + --allocated-storage 20 ` + --master-username $adminUser ` + --master-user-password $password ` + --vpc-security-group-ids $groupId ` + --license-model license-included ` + --publicly-accessible ` + --no-multi-az ` + --backup-retention-period 0 ` + --tags "Key=sc-cloud-test,Value=true" "Key=run-id,Value=$Name" "Key=created,Value=$created" ` + --no-cli-pager --output none + +Write-Step 'Waiting for the instance to become available' +aws rds wait db-instance-available --db-instance-identifier $Name + +$endpoint = aws rds describe-db-instances --db-instance-identifier $Name --query 'DBInstances[0].Endpoint.Address' --output text + +# Trust Server Certificate because RDS presents an Amazon CA that is not in the runner's trust store. +# The connection is still encrypted; only the certificate chain goes unverified. +$connectionString = "Server=tcp:$endpoint,1433;Initial Catalog=$DatabaseName;User ID=$adminUser;Password=$password;Encrypt=True;TrustServerCertificate=True;Connect Timeout=60" + +# RDS cannot create a user database as part of the instance, unlike every other target here. +Write-Step "Creating database $DatabaseName and verifying Full-Text Search" +Invoke-SqlServerVerification -ConnectionString $connectionString + +Set-PersistenceConnectionString -Provider SqlServer -ConnectionString $connectionString diff --git a/tools/cloud/verify-sqlserver.cs b/tools/cloud/verify-sqlserver.cs new file mode 100644 index 0000000000..d7961f1d60 --- /dev/null +++ b/tools/cloud/verify-sqlserver.cs @@ -0,0 +1,52 @@ +#:package Microsoft.Data.SqlClient@6.1.1 + +// Creates the ServiceControl test database if the service could not create it during provisioning, +// and fails the run if the server has no Full-Text Search. Message search is not optional, so an +// instance without it would otherwise fail every search test twenty minutes later with a much less +// obvious error. + +using Microsoft.Data.SqlClient; + +if (args.Length != 1) +{ + Console.Error.WriteLine("usage: dotnet run verify-sqlserver.cs -- "); + return 1; +} + +var builder = new SqlConnectionStringBuilder(args[0]); +var database = builder.InitialCatalog; + +builder.InitialCatalog = "master"; + +try +{ + await using var master = new SqlConnection(builder.ConnectionString); + await master.OpenAsync(); + + await using (var create = master.CreateCommand()) + { + create.CommandText = $"IF DB_ID(N'{database}') IS NULL CREATE DATABASE [{database}]"; + create.CommandTimeout = 300; + await create.ExecuteNonQueryAsync(); + } + + await using (var fullText = master.CreateCommand()) + { + fullText.CommandText = "SELECT CONVERT(int, ISNULL(SERVERPROPERTY('IsFullTextInstalled'), 0))"; + if ((int)(await fullText.ExecuteScalarAsync())! != 1) + { + Console.Error.WriteLine($"{builder.DataSource} does not have SQL Server Full-Text Search installed, which ServiceControl requires. On RDS, check that the edition supports it and that the option group includes it."); + return 1; + } + } +} +catch (SqlException e) +{ + // Without the stack trace, which says nothing useful about a server that is unreachable or + // rejecting the login. + Console.Error.WriteLine($"Could not prepare {builder.DataSource}: {e.Message}"); + return 1; +} + +Console.WriteLine($"Database '{database}' is present on {builder.DataSource} and Full-Text Search is installed."); +return 0; From c16911b60b9f3799de25875d722a1a426c059dac Mon Sep 17 00:00:00 2001 From: John Simons Date: Wed, 9 Sep 2026 13:09:22 +1000 Subject: [PATCH 02/14] Enable the PR to be tested for now --- .github/workflows/cloud-database-tests.yml | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/.github/workflows/cloud-database-tests.yml b/.github/workflows/cloud-database-tests.yml index 7dc8a603eb..f612ca6e5d 100644 --- a/.github/workflows/cloud-database-tests.yml +++ b/.github/workflows/cloud-database-tests.yml @@ -1,5 +1,12 @@ name: Cloud database tests on: + pull_request: + push: + tags: + - '[0-9]+.[0-9]+.[0-9]+' + - '[0-9]+.[0-9]+.[0-9]+-*' + branches: + - john/cloud-ci-tests workflow_dispatch: inputs: target: @@ -13,10 +20,6 @@ on: - azure-postgresql - aurora-postgresql - rds-sqlserver - push: - tags: - - '[0-9]+.[0-9]+.[0-9]+' - - '[0-9]+.[0-9]+.[0-9]+-*' env: DOTNET_NOLOGO: true defaults: From ec4fddd8f99fd5bd108488b4dffc2ffddd92c71c Mon Sep 17 00:00:00 2001 From: John Simons Date: Wed, 9 Sep 2026 15:14:37 +1000 Subject: [PATCH 03/14] Use current Azure credentials --- .github/workflows/cloud-database-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cloud-database-tests.yml b/.github/workflows/cloud-database-tests.yml index f612ca6e5d..7d72f2b43c 100644 --- a/.github/workflows/cloud-database-tests.yml +++ b/.github/workflows/cloud-database-tests.yml @@ -110,7 +110,7 @@ jobs: uses: azure/login@v3.0.2 if: startsWith(matrix.target, 'azure-') with: - creds: ${{ secrets.AZURE_CLOUD_TEST_CREDENTIALS }} + creds: ${{ secrets.AZURE_ACI_CREDENTIALS }} - name: Setup AWS environment variables if: startsWith(matrix.target, 'aurora-') || startsWith(matrix.target, 'rds-') run: | From 63d1eac4efc9e81e1ae7a58d20088064d2dc6b52 Mon Sep 17 00:00:00 2001 From: John Simons Date: Thu, 10 Sep 2026 09:46:28 +1000 Subject: [PATCH 04/14] Replace PowerShell provisioning scripts with a self-tearing-down GitHub Actions composite action The PowerShell scripts required an explicit teardown step in every workflow that used them, which could be skipped or forgotten if a job failed early. Replacing them with a JavaScript action (node24) allows declaring a post step, which GitHub runs automatically at job end regardless of outcome, making teardown impossible to omit. The action also folds in the stale security group sweep that was previously handled by a nightly cleanup workflow, so that workflow is removed. Each run cleans up what earlier runs left behind, keeping the account self-healing without a scheduled job. Other notable changes: Aurora PostgreSQL moves from db.t4g.medium to db.r6g.large and Azure PostgreSQL from a burstable B1ms to a General Purpose D4ds_v5 to avoid throttling mid-run; the run name now includes the attempt number so a retried run does not collide with its predecessor's still-deleting resources; and a verify-postgresql.cs script is added alongside the existing verify-sqlserver.cs to wait for PostgreSQL servers to become reachable before handing off to the test run. --- .github/actions/cloud-database/action.yml | 16 +++ .../cloud-database/aurora-postgresql.mts | 79 ++++++++++++ .github/actions/cloud-database/aws.mts | 79 ++++++++++++ .../cloud-database/azure-postgresql.mts | 59 +++++++++ .github/actions/cloud-database/azure-sql.mts | 62 ++++++++++ .github/actions/cloud-database/azure.mts | 27 +++++ .github/actions/cloud-database/common.mts | 109 +++++++++++++++++ .github/actions/cloud-database/index.mts | 49 ++++++++ .../actions/cloud-database/rds-sqlserver.mts | 72 +++++++++++ .../cloud-database/verify-postgresql.cs | 47 ++++++++ .../cloud-database/verify-sqlserver.cs | 66 ++++++++++ .../cloud-database-tests-cleanup.yml | 113 ------------------ .github/workflows/cloud-database-tests.yml | 18 ++- tools/cloud/aurora-postgresql.ps1 | 95 --------------- tools/cloud/azure-postgresql.ps1 | 64 ---------- tools/cloud/azure-sql.ps1 | 73 ----------- tools/cloud/common.ps1 | 109 ----------------- tools/cloud/rds-sqlserver.ps1 | 90 -------------- tools/cloud/verify-sqlserver.cs | 52 -------- 19 files changed, 677 insertions(+), 602 deletions(-) create mode 100644 .github/actions/cloud-database/action.yml create mode 100644 .github/actions/cloud-database/aurora-postgresql.mts create mode 100644 .github/actions/cloud-database/aws.mts create mode 100644 .github/actions/cloud-database/azure-postgresql.mts create mode 100644 .github/actions/cloud-database/azure-sql.mts create mode 100644 .github/actions/cloud-database/azure.mts create mode 100644 .github/actions/cloud-database/common.mts create mode 100644 .github/actions/cloud-database/index.mts create mode 100644 .github/actions/cloud-database/rds-sqlserver.mts create mode 100644 .github/actions/cloud-database/verify-postgresql.cs create mode 100644 .github/actions/cloud-database/verify-sqlserver.cs delete mode 100644 .github/workflows/cloud-database-tests-cleanup.yml delete mode 100644 tools/cloud/aurora-postgresql.ps1 delete mode 100644 tools/cloud/azure-postgresql.ps1 delete mode 100644 tools/cloud/azure-sql.ps1 delete mode 100644 tools/cloud/common.ps1 delete mode 100644 tools/cloud/rds-sqlserver.ps1 delete mode 100644 tools/cloud/verify-sqlserver.cs diff --git a/.github/actions/cloud-database/action.yml b/.github/actions/cloud-database/action.yml new file mode 100644 index 0000000000..33dc575957 --- /dev/null +++ b/.github/actions/cloud-database/action.yml @@ -0,0 +1,16 @@ +name: Cloud database provisioner +description: Provisions a managed database for the run, and deletes it again when the job ends +inputs: + target: + description: Which managed database to provision (azure-sql, azure-postgresql, aurora-postgresql or rds-sqlserver) + required: true + name: + description: Name for the resources this run creates, which teardown uses to find them again + required: true +runs: + # A JavaScript action, not a composite one, because only this kind can declare a post step. + # Teardown is that post step, so it cannot be left out of a workflow or skipped by a failure + # earlier in the job. + using: node24 + main: index.mts + post: index.mts diff --git a/.github/actions/cloud-database/aurora-postgresql.mts b/.github/actions/cloud-database/aurora-postgresql.mts new file mode 100644 index 0000000000..e7f1332ce5 --- /dev/null +++ b/.github/actions/cloud-database/aurora-postgresql.mts @@ -0,0 +1,79 @@ +// Provisions and tears down an Aurora PostgreSQL cluster for one run of the cloud database tests. + +import { step, run, capture, newAdminPassword, runnerIpAddress, setPersistenceConnectionString, verifyDatabase, teardownStep } from './common.mts'; +import * as aws from './aws.mts'; + +const databaseName = 'servicecontrol'; +const adminUser = 'sctestadmin'; +const port = 5432; + +async function provision(name: string): Promise { + aws.removeStaleSecurityGroups(); + + const password = newAdminPassword(); + const runnerIp = await runnerIpAddress(); + const { groupId, created } = aws.createSecurityGroup(name); + aws.allowRunner(groupId, port, runnerIp); + + const tags = aws.tags(name, created); + const instance = `${name}-1`; + + // The engine version is left to the AWS default so that this does not break every time a pinned + // minor version is retired. + step(`Creating Aurora PostgreSQL cluster ${name}`); + run('aws', ['rds', 'create-db-cluster', + '--db-cluster-identifier', name, + '--engine', 'aurora-postgresql', + '--master-username', adminUser, + '--master-user-password', password, + '--database-name', databaseName, + '--vpc-security-group-ids', groupId, + '--no-deletion-protection', + '--backup-retention-period', '1', + ...tags, + '--no-cli-pager']); + + // One instance and no replicas: the cluster does not outlive the job. db.r6g.large rather than a + // burstable class, so the run is not throttled part way through. + step(`Creating instance ${instance}`); + run('aws', ['rds', 'create-db-instance', + '--db-instance-identifier', instance, + '--db-cluster-identifier', name, + '--engine', 'aurora-postgresql', + '--db-instance-class', 'db.r6g.large', + '--publicly-accessible', + ...tags, + '--no-cli-pager']); + + step('Waiting for the instance to become available'); + run('aws', ['rds', 'wait', 'db-instance-available', '--db-instance-identifier', instance]); + + const endpoint = capture('aws', ['rds', 'describe-db-clusters', '--db-cluster-identifier', name, '--query', 'DBClusters[0].Endpoint', '--output', 'text']); + + // Trust Server Certificate because RDS presents an Amazon CA that is not in the runner's trust + // store. The connection is still encrypted; only the certificate chain goes unverified. + const connectionString = `Host=${endpoint};Port=${port};Database=${databaseName};Username=${adminUser};Password=${password};Ssl Mode=Require;Trust Server Certificate=true;Timeout=60`; + + step('Waiting until the database accepts connections'); + verifyDatabase('PostgreSql', connectionString); + + setPersistenceConnectionString('PostgreSql', connectionString); +} + +function teardown(name: string): void { + const instance = `${name}-1`; + + // Without waiting for the deletions to finish: an RDS instance takes minutes to disappear, and + // holding the job open for that costs more than letting a later run sweep up does. + teardownStep(`Deleting instance ${instance}`, () => + run('aws', ['rds', 'delete-db-instance', '--db-instance-identifier', instance, '--skip-final-snapshot', '--delete-automated-backups', '--no-cli-pager'])); + + teardownStep(`Deleting cluster ${name}`, () => + run('aws', ['rds', 'delete-db-cluster', '--db-cluster-identifier', name, '--skip-final-snapshot', '--no-cli-pager'])); + + // Will refuse while the instance still holds it, which is the normal case. removeStaleSecurityGroups + // on a later run is what actually clears it. + teardownStep(`Deleting security group ${name}`, () => aws.deleteSecurityGroup(name)); +} + +export { provision, teardown }; diff --git a/.github/actions/cloud-database/aws.mts b/.github/actions/cloud-database/aws.mts new file mode 100644 index 0000000000..eb8f7c4447 --- /dev/null +++ b/.github/actions/cloud-database/aws.mts @@ -0,0 +1,79 @@ +// Helpers shared by the two AWS targets. + +import { step, run, capture, captureJson } from './common.mts'; + +function defaultVpcId(): string { + const vpcId = capture('aws', ['ec2', 'describe-vpcs', '--filters', 'Name=isDefault,Values=true', '--query', 'Vpcs[0].VpcId', '--output', 'text']); + + if (!vpcId || vpcId === 'None') { + throw new Error('This AWS account has no default VPC in this region, so there is no public subnet group for the database to use.'); + } + + return vpcId; +} + +// An EC2 security group does not record when it was created, so this tag is what the stale sweep +// judges age by. Epoch seconds because AWS restricts the characters a tag value may contain. +function createdTimestamp(): string { + return Math.floor(Date.now() / 1000).toString(); +} + +function createSecurityGroup(name: string): { groupId: string; created: string } { + const vpcId = defaultVpcId(); + const created = createdTimestamp(); + + step(`Creating security group ${name} in ${vpcId}`); + const groupId = capture('aws', [ + 'ec2', 'create-security-group', + '--group-name', name, + '--description', 'ServiceControl cloud database tests', + '--vpc-id', vpcId, + '--tag-specifications', `ResourceType=security-group,Tags=[{Key=sc-cloud-test,Value=true},{Key=run-id,Value=${name}},{Key=created,Value=${created}}]`, + '--query', 'GroupId', '--output', 'text' + ]); + + return { groupId, created }; +} + +function allowRunner(groupId: string, port: number, runnerIp: string): void { + step(`Allowing ${runnerIp} on ${port}`); + run('aws', ['ec2', 'authorize-security-group-ingress', '--group-id', groupId, '--protocol', 'tcp', '--port', String(port), '--cidr', `${runnerIp}/32`, '--no-cli-pager']); +} + +function deleteSecurityGroup(name: string): void { + const groupId = capture('aws', ['ec2', 'describe-security-groups', '--filters', `Name=group-name,Values=${name}`, '--query', 'SecurityGroups[0].GroupId', '--output', 'text']); + + if (groupId && groupId !== 'None') { + run('aws', ['ec2', 'delete-security-group', '--group-id', groupId, '--no-cli-pager']); + } +} + +// A security group cannot be deleted while an instance still holds it, and an RDS instance takes +// minutes to go, so a run can almost never delete its own. Each run clears out the ones earlier runs +// left instead, which keeps this self healing without a scheduled workflow. +function removeStaleSecurityGroups(): void { + const cutoff = Math.floor(Date.now() / 1000) - 4 * 60 * 60; + const tagged = captureJson('aws', ['ec2', 'describe-security-groups', '--filters', 'Name=tag:sc-cloud-test,Values=true', '--no-cli-pager', '--output', 'json']); + + for (const group of tagged.SecurityGroups) { + const created = group.Tags?.find((tag: { Key: string; Value: string }) => tag.Key === 'created')?.Value; + + if (!created || Number(created) >= cutoff) { + continue; + } + + try { + run('aws', ['ec2', 'delete-security-group', '--group-id', group.GroupId, '--no-cli-pager']); + step(`Deleted stale security group ${group.GroupName}`); + } catch { + // Still attached to an instance that has not finished deleting. A later run will get it. + step(`Stale security group ${group.GroupName} is still in use, leaving it for a later run`); + } + } +} + +function tags(name: string, created: string): string[] { + return ['--tags', 'Key=sc-cloud-test,Value=true', `Key=run-id,Value=${name}`, `Key=created,Value=${created}`]; +} + +export { createSecurityGroup, allowRunner, deleteSecurityGroup, removeStaleSecurityGroups, tags }; diff --git a/.github/actions/cloud-database/azure-postgresql.mts b/.github/actions/cloud-database/azure-postgresql.mts new file mode 100644 index 0000000000..7891db709e --- /dev/null +++ b/.github/actions/cloud-database/azure-postgresql.mts @@ -0,0 +1,59 @@ +// Provisions and tears down an Azure Database for PostgreSQL flexible server for one run of the +// cloud database tests. + +import { step, run, newAdminPassword, runnerIpAddress, setPersistenceConnectionString, verifyDatabase, teardownStep } from './common.mts'; +import * as azure from './azure.mts'; + +const databaseName = 'servicecontrol'; +const adminUser = 'sctestadmin'; + +async function provision(name: string): Promise { + const password = newAdminPassword(); + const runnerIp = await runnerIpAddress(); + const location = azure.location(); + const tags = azure.tags(name); + + // General Purpose with 4 vCores rather than a burstable tier, so the run is not throttled part + // way through. No high availability and no geo-redundant backup: nothing here outlives the job. + // --public-access opens the firewall to just this runner as part of creation. + step(`Creating PostgreSQL flexible server ${name} in ${azure.resourceGroup} (${location}), allowing ${runnerIp}`); + run('az', ['postgres', 'flexible-server', 'create', + '--name', name, + '--resource-group', azure.resourceGroup, + '--location', location, + '--admin-user', adminUser, + '--admin-password', password, + '--tier', 'GeneralPurpose', + '--sku-name', 'Standard_D4ds_v5', + '--storage-size', '128', + '--version', '16', + '--high-availability', 'Disabled', + '--geo-redundant-backup', 'Disabled', + '--public-access', runnerIp, + ...tags, + '--yes', + '--only-show-errors', '--output', 'none']); + + step(`Creating database ${databaseName}`); + run('az', ['postgres', 'flexible-server', 'db', 'create', + '--database-name', databaseName, + '--resource-group', azure.resourceGroup, + '--server-name', name, + '--only-show-errors', '--output', 'none']); + + const connectionString = `Host=${name}.postgres.database.azure.com;Port=5432;Database=${databaseName};Username=${adminUser};Password=${password};Ssl Mode=Require;Timeout=60`; + + step('Waiting until the database accepts connections'); + verifyDatabase('PostgreSql', connectionString); + + setPersistenceConnectionString('PostgreSql', connectionString); +} + +function teardown(name: string): void { + // The resource group is shared and long lived, so only the server goes. Its databases and + // firewall rules are children and go with it. + teardownStep(`Deleting PostgreSQL flexible server ${name}`, () => + run('az', ['postgres', 'flexible-server', 'delete', '--name', name, '--resource-group', azure.resourceGroup, '--yes', '--only-show-errors'])); +} + +export { provision, teardown }; diff --git a/.github/actions/cloud-database/azure-sql.mts b/.github/actions/cloud-database/azure-sql.mts new file mode 100644 index 0000000000..bb33e4ea34 --- /dev/null +++ b/.github/actions/cloud-database/azure-sql.mts @@ -0,0 +1,62 @@ +// Provisions and tears down an Azure SQL Database for one run of the cloud database tests. + +import { step, run, newAdminPassword, runnerIpAddress, setPersistenceConnectionString, verifyDatabase, teardownStep } from './common.mts'; +import * as azure from './azure.mts'; + +const databaseName = 'servicecontrol'; +const adminUser = 'sctestadmin'; + +async function provision(name: string): Promise { + const password = newAdminPassword(); + const runnerIp = await runnerIpAddress(); + const location = azure.location(); + const tags = azure.tags(name); + + step(`Creating SQL server ${name} in ${azure.resourceGroup} (${location})`); + run('az', ['sql', 'server', 'create', + '--name', name, + '--resource-group', azure.resourceGroup, + '--location', location, + '--admin-user', adminUser, + '--admin-password', password, + ...tags, + '--only-show-errors', '--output', 'none']); + + step(`Allowing ${runnerIp} through the server firewall`); + run('az', ['sql', 'server', 'firewall-rule', 'create', + '--name', 'github-runner', + '--resource-group', azure.resourceGroup, + '--server', name, + '--start-ip-address', runnerIp, + '--end-ip-address', runnerIp, + '--only-show-errors', '--output', 'none']); + + // General Purpose with 4 vCores: the suites are DDL heavy and run several assemblies at once, and + // a DTU tier that size would spend the run throttled. Local backup redundancy because nothing + // here outlives the job. + step(`Creating database ${databaseName}`); + run('az', ['sql', 'db', 'create', + '--name', databaseName, + '--resource-group', azure.resourceGroup, + '--server', name, + '--service-objective', 'GP_Gen5_4', + '--backup-storage-redundancy', 'Local', + ...tags, + '--only-show-errors', '--output', 'none']); + + const connectionString = `Server=tcp:${name}.database.windows.net,1433;Initial Catalog=${databaseName};User ID=${adminUser};Password=${password};Encrypt=True;TrustServerCertificate=False;Connect Timeout=60`; + + step('Waiting until the database accepts connections'); + verifyDatabase('SqlServer', connectionString); + + setPersistenceConnectionString('SqlServer', connectionString); +} + +function teardown(name: string): void { + // The resource group is shared and long lived, so only the server goes. Its databases and + // firewall rules are children and go with it. + teardownStep(`Deleting SQL server ${name}`, () => + run('az', ['sql', 'server', 'delete', '--name', name, '--resource-group', azure.resourceGroup, '--yes', '--only-show-errors'])); +} + +export { provision, teardown }; diff --git a/.github/actions/cloud-database/azure.mts b/.github/actions/cloud-database/azure.mts new file mode 100644 index 0000000000..77f5542769 --- /dev/null +++ b/.github/actions/cloud-database/azure.mts @@ -0,0 +1,27 @@ +// Helpers shared by the two Azure targets. + +import { capture } from './common.mts'; + +const resourceGroup = 'GitHubActions-RG'; + +// Created, Package and RunnerOS are required on every resource in the subscription. RunId is ours, +// and is what tells one run's resources from another's. +function tags(runId: string): string[] { + const created = new Date().toISOString().slice(0, 10); + + return ['--tags', `Created=${created}`, 'Package=ServiceControl', 'RunnerOS=Linux', `RunId=${runId}`]; +} + +// The subscription only grants write access to one long lived resource group, so resources are +// created inside it and deleted individually rather than by dropping a group per run. +function location(): string { + const value = capture('az', ['group', 'show', '--name', resourceGroup, '--query', 'location', '--output', 'tsv']); + + if (!value) { + throw new Error(`Could not read the location of resource group ${resourceGroup}.`); + } + + return value; +} + +export { resourceGroup, tags, location }; diff --git a/.github/actions/cloud-database/common.mts b/.github/actions/cloud-database/common.mts new file mode 100644 index 0000000000..4e828d2604 --- /dev/null +++ b/.github/actions/cloud-database/common.mts @@ -0,0 +1,109 @@ +// Shared helpers for the target modules beside this file. + +import { execFileSync } from 'node:child_process'; +import { appendFileSync } from 'node:fs'; +import { randomInt } from 'node:crypto'; + +export type Provider = 'SqlServer' | 'PostgreSql'; +export type Target = 'azure-sql' | 'azure-postgresql' | 'aurora-postgresql' | 'rds-sqlserver'; + +function step(message: string): void { + console.log(`==> ${message}`); +} + +// Both CLIs report failure by exiting non-zero, and execFileSync turns that into a throw, so a +// provision that fails half way does not carry on building on top of the failure. +function run(command: string, args: string[]): void { + execFileSync(command, args, { stdio: 'inherit' }); +} + +function capture(command: string, args: string[]): string { + return execFileSync(command, args, { encoding: 'utf8' }).trim(); +} + +function captureJson(command: string, args: string[]): any { + return JSON.parse(capture(command, args)); +} + +// Satisfies the complexity rules of all four services at once, and avoids the characters that would +// have to be escaped in a connection string or are rejected outright by RDS (/ " @ and space). +function newAdminPassword(): string { + const upper = 'ABCDEFGHJKLMNPQRSTUVWXYZ'; + const lower = 'abcdefghijkmnpqrstuvwxyz'; + const digit = '23456789'; + const symbol = '!#$%*()-_+'; + const all = upper + lower + digit + symbol; + + const pick = (set: string) => set[randomInt(set.length)]; + const characters = [pick(upper), pick(lower), pick(digit), pick(symbol)]; + + while (characters.length < 28) { + characters.push(pick(all)); + } + + for (let i = characters.length - 1; i > 0; i--) { + const j = randomInt(i + 1); + [characters[i], characters[j]] = [characters[j], characters[i]]; + } + + const password = characters.join(''); + + // Before it can reach a log line. Everything the password is later embedded in, the connection + // string included, is redacted along with it. + console.log(`::add-mask::${password}`); + + return password; +} + +// The servers are reachable from the internet, so each one is firewalled to the single address this +// job connects from. +async function runnerIpAddress(): Promise { + const response = await fetch('https://api.ipify.org', { signal: AbortSignal.timeout(30000) }); + + if (!response.ok) { + throw new Error(`Could not determine the runner's public IP address: ${response.status} ${response.statusText}`); + } + + return (await response.text()).trim(); +} + +function setPersistenceConnectionString(provider: Provider, connectionString: string): void { + const name = `ServiceControl_Persistence_${provider}_ConnectionString`; + appendFileSync(process.env.GITHUB_ENV, `${name}=${connectionString}\n`); + step(`Exported ${name}`); +} + +// Waits until the database is really reachable. The provisioning CLIs return before that is +// necessarily true, and the alternative to checking here is the test run discovering it far less +// clearly twenty minutes later. +function verifyDatabase(provider: Provider, connectionString: string): void { + const script = provider === 'SqlServer' ? './verify-sqlserver.cs' : './verify-postgresql.cs'; + + // From this folder, because `dotnet run .cs` picks up any project file in the working + // directory and the repo root holds a tests.proj by this point. + execFileSync('dotnet', ['run', script, '--', connectionString], { cwd: import.meta.dirname, stdio: 'inherit' }); +} + +// Teardown runs even when provisioning failed part way, so it has to tolerate resources that were +// never created. +function teardownStep(description: string, action: () => void): void { + step(description); + + try { + action(); + } catch (error) { + console.log(`::warning::${description} failed: ${(error as Error).message}`); + } +} + +export { + step, + run, + capture, + captureJson, + newAdminPassword, + runnerIpAddress, + setPersistenceConnectionString, + verifyDatabase, + teardownStep +}; diff --git a/.github/actions/cloud-database/index.mts b/.github/actions/cloud-database/index.mts new file mode 100644 index 0000000000..71335f05e2 --- /dev/null +++ b/.github/actions/cloud-database/index.mts @@ -0,0 +1,49 @@ +// Provisions a managed database on the way in, and deletes it on the way out. GitHub runs this same +// file twice: once as the step itself, and once as the job's post step, which is what makes the +// teardown impossible to forget. +// +// No dependencies on purpose. @actions/core would have to be either committed as node_modules or +// bundled by a build step, and everything used here is a documented workflow command or environment +// variable that the toolkit itself is a wrapper over. + +import { appendFileSync } from 'node:fs'; +import type { Target } from './common.mts'; + +const targets: Target[] = ['azure-sql', 'azure-postgresql', 'aurora-postgresql', 'rds-sqlserver']; + +function fail(message: string): never { + console.log(`::error::${message}`); + process.exit(1); +} + +function input(name: string): string { + return process.env[`INPUT_${name.toUpperCase()}`] || fail(`The ${name} input is required.`); +} + +async function main() { + const target = input('target'); + const name = input('name'); + + if (!targets.includes(target as Target)) { + fail(`'${target}' is not a cloud database target. Expected one of ${targets.join(', ')}.`); + } + + const database = await import(`./${target}.mts`); + + // GitHub sets STATE_ variables in the post step from whatever the main step wrote to GITHUB_STATE. + if (process.env.STATE_provisioning) { + database.teardown(name); + return; + } + + // Recorded before provisioning rather than after, so that a run which fails half way through + // creating its resources still tears down the ones it did create. + appendFileSync(process.env.GITHUB_STATE, `provisioning=${target}\n`); + await database.provision(name); +} + +main().catch(error => { + // The CLI output has already been streamed, so only the summary is worth adding. + console.log(`::error::${(error as Error).message}`); + process.exit(1); +}); diff --git a/.github/actions/cloud-database/rds-sqlserver.mts b/.github/actions/cloud-database/rds-sqlserver.mts new file mode 100644 index 0000000000..4f05375ee4 --- /dev/null +++ b/.github/actions/cloud-database/rds-sqlserver.mts @@ -0,0 +1,72 @@ +// Provisions and tears down an RDS SQL Server instance for one run of the cloud database tests. +// +// This is the slowest of the four targets to provision, around 15 to 25 minutes, so the workflow +// starts it before waiting on the build. + +import { step, run, capture, newAdminPassword, runnerIpAddress, setPersistenceConnectionString, verifyDatabase, teardownStep } from './common.mts'; +import * as aws from './aws.mts'; + +const databaseName = 'servicecontrol'; +const adminUser = 'sctestadmin'; +const port = 1433; + +// Web edition is the cheapest licence-included option. Full-Text Search availability is what decides +// whether it is usable; verify-sqlserver.cs fails the run with a clear message if this edition turns +// out not to have it, in which case move to sqlserver-se. +const engine = 'sqlserver-web'; + +async function provision(name: string): Promise { + aws.removeStaleSecurityGroups(); + + const password = newAdminPassword(); + const runnerIp = await runnerIpAddress(); + const { groupId, created } = aws.createSecurityGroup(name); + aws.allowRunner(groupId, port, runnerIp); + + // db.m5.large on gp3, because a t3.small would be throttled by both CPU credits and gp2 burst + // balance part way through the run. No automated backups and no standby: the instance does not + // outlive the job, and both slow provisioning down. + step(`Creating RDS SQL Server instance ${name}`); + run('aws', ['rds', 'create-db-instance', + '--db-instance-identifier', name, + '--engine', engine, + '--db-instance-class', 'db.m5.large', + '--allocated-storage', '100', + '--storage-type', 'gp3', + '--master-username', adminUser, + '--master-user-password', password, + '--vpc-security-group-ids', groupId, + '--license-model', 'license-included', + '--publicly-accessible', + '--no-multi-az', + '--backup-retention-period', '0', + ...aws.tags(name, created), + '--no-cli-pager']); + + step('Waiting for the instance to become available'); + run('aws', ['rds', 'wait', 'db-instance-available', '--db-instance-identifier', name]); + + const endpoint = capture('aws', ['rds', 'describe-db-instances', '--db-instance-identifier', name, '--query', 'DBInstances[0].Endpoint.Address', '--output', 'text']); + + // Trust Server Certificate because RDS presents an Amazon CA that is not in the runner's trust + // store. The connection is still encrypted; only the certificate chain goes unverified. + const connectionString = `Server=tcp:${endpoint},${port};Initial Catalog=${databaseName};User ID=${adminUser};Password=${password};Encrypt=True;TrustServerCertificate=True;Connect Timeout=60`; + + // RDS cannot create a user database as part of the instance, unlike every other target here, so + // this both creates it and waits for the server to accept connections. + step(`Creating database ${databaseName} and verifying Full-Text Search`); + verifyDatabase('SqlServer', connectionString); + + setPersistenceConnectionString('SqlServer', connectionString); +} + +function teardown(name: string): void { + teardownStep(`Deleting instance ${name}`, () => + run('aws', ['rds', 'delete-db-instance', '--db-instance-identifier', name, '--skip-final-snapshot', '--delete-automated-backups', '--no-cli-pager'])); + + // Will refuse while the instance still holds it, which is the normal case. removeStaleSecurityGroups + // on a later run is what actually clears it. + teardownStep(`Deleting security group ${name}`, () => aws.deleteSecurityGroup(name)); +} + +export { provision, teardown }; diff --git a/.github/actions/cloud-database/verify-postgresql.cs b/.github/actions/cloud-database/verify-postgresql.cs new file mode 100644 index 0000000000..fe162bcd80 --- /dev/null +++ b/.github/actions/cloud-database/verify-postgresql.cs @@ -0,0 +1,47 @@ +#:package Npgsql@10.0.3 + +// Waits until the server actually accepts connections on the test database. +// +// The provisioning CLIs report a server as available before it is necessarily reachable, so this +// retries rather than taking the first refusal as final. Without it the first thing to touch a +// half-ready server would be the test run, which reports the problem far less clearly. + +using Npgsql; + +if (args.Length != 1) +{ + Console.Error.WriteLine("usage: dotnet run verify-postgresql.cs -- "); + return 1; +} + +var builder = new NpgsqlConnectionStringBuilder(args[0]); +var deadline = DateTime.UtcNow.AddMinutes(10); +var attempt = 0; + +while (true) +{ + attempt++; + + try + { + await using var connection = new NpgsqlConnection(builder.ConnectionString); + await connection.OpenAsync(); + + await using var command = connection.CreateCommand(); + command.CommandText = "SELECT current_database()"; + var database = (string)(await command.ExecuteScalarAsync())!; + + Console.WriteLine($"Database '{database}' is reachable on {builder.Host}, after {attempt} attempt(s)."); + return 0; + } + catch (Exception e) when (e is NpgsqlException or TimeoutException && DateTime.UtcNow < deadline) + { + Console.WriteLine($"{builder.Host} is not ready yet (attempt {attempt}): {e.Message.Split('\n')[0]}"); + await Task.Delay(TimeSpan.FromSeconds(10)); + } + catch (Exception e) when (e is NpgsqlException or TimeoutException) + { + Console.Error.WriteLine($"Gave up waiting for {builder.Host} after {attempt} attempts: {e.Message}"); + return 1; + } +} diff --git a/.github/actions/cloud-database/verify-sqlserver.cs b/.github/actions/cloud-database/verify-sqlserver.cs new file mode 100644 index 0000000000..f741909586 --- /dev/null +++ b/.github/actions/cloud-database/verify-sqlserver.cs @@ -0,0 +1,66 @@ +#:package Microsoft.Data.SqlClient@6.1.1 + +// Waits until the server actually accepts connections, creates the test database if the service +// could not create it during provisioning, and fails the run if the server has no Full-Text Search. +// +// The provisioning CLIs report a database as created before it is necessarily reachable, and an +// Azure firewall rule takes a moment to propagate, so this retries rather than taking the first +// refusal as final. Message search is not optional either, so a server without Full-Text Search is +// better caught here than twenty minutes later in the search tests. + +using Microsoft.Data.SqlClient; + +if (args.Length != 1) +{ + Console.Error.WriteLine("usage: dotnet run verify-sqlserver.cs -- "); + return 1; +} + +var builder = new SqlConnectionStringBuilder(args[0]); +var database = builder.InitialCatalog; + +builder.InitialCatalog = "master"; + +var deadline = DateTime.UtcNow.AddMinutes(10); +var attempt = 0; + +while (true) +{ + attempt++; + + try + { + await using var master = new SqlConnection(builder.ConnectionString); + await master.OpenAsync(); + + await using (var create = master.CreateCommand()) + { + create.CommandText = $"IF DB_ID(N'{database}') IS NULL CREATE DATABASE [{database}]"; + create.CommandTimeout = 300; + await create.ExecuteNonQueryAsync(); + } + + await using (var fullText = master.CreateCommand()) + { + fullText.CommandText = "SELECT CONVERT(int, ISNULL(SERVERPROPERTY('IsFullTextInstalled'), 0))"; + if ((int)(await fullText.ExecuteScalarAsync())! != 1) + { + Console.Error.WriteLine($"{builder.DataSource} does not have SQL Server Full-Text Search installed, which ServiceControl requires. On RDS, check that the edition supports it and that the option group includes it."); + return 1; + } + } + + Console.WriteLine($"Database '{database}' is present on {builder.DataSource} and Full-Text Search is installed, after {attempt} attempt(s)."); + return 0; + } + catch (SqlException e) when (DateTime.UtcNow < deadline) + { + Console.WriteLine($"{builder.DataSource} is not ready yet (attempt {attempt}): {e.Message.Split('\n')[0]}"); + await Task.Delay(TimeSpan.FromSeconds(10)); + } + catch (SqlException e) + { + Console.Error.WriteLine($"Gave up waiting for {builder.DataSource} after {attempt} attempts: {e.Message}"); + return 1; + } +} diff --git a/.github/workflows/cloud-database-tests-cleanup.yml b/.github/workflows/cloud-database-tests-cleanup.yml deleted file mode 100644 index 974022efc8..0000000000 --- a/.github/workflows/cloud-database-tests-cleanup.yml +++ /dev/null @@ -1,113 +0,0 @@ -name: Cloud database tests cleanup -on: - schedule: - - cron: '0 3 * * *' - workflow_dispatch: -env: - DOTNET_NOLOGO: true -defaults: - run: - shell: pwsh -jobs: - azure: - name: Azure - runs-on: ubuntu-latest - steps: - - name: Check for secrets - env: - SECRETS_AVAILABLE: ${{ secrets.SECRETS_AVAILABLE }} - run: exit $(If ($env:SECRETS_AVAILABLE -eq 'true') { 0 } Else { 1 }) - - name: Azure login - uses: azure/login@v3.0.2 - with: - creds: ${{ secrets.AZURE_CLOUD_TEST_CREDENTIALS }} - - name: Delete leaked resource groups - run: | - $cutoff = [DateTimeOffset]::UtcNow.AddHours(-4).ToUnixTimeSeconds() - $groups = az group list --tag sc-cloud-test=true --query '[].{name:name, created:tags.created}' --output json | ConvertFrom-Json - - if (-not $groups) { - Write-Output 'Nothing tagged sc-cloud-test is left in this subscription.' - return - } - - foreach ($group in $groups) { - if (-not $group.created) { - Write-Warning "Resource group $($group.name) has no created tag, so its age is unknown. Leaving it, delete it by hand." - continue - } - - if ([long]$group.created -ge $cutoff) { - Write-Output "Leaving $($group.name), it belongs to a run that may still be going." - continue - } - - Write-Output "Deleting $($group.name)" - try { - az group delete --name $group.name --yes --no-wait --only-show-errors - } - catch { - Write-Warning "Could not delete $($group.name): $($_.Exception.Message)" - } - } - - aws: - name: AWS - runs-on: ubuntu-latest - steps: - - name: Check for secrets - env: - SECRETS_AVAILABLE: ${{ secrets.SECRETS_AVAILABLE }} - run: exit $(If ($env:SECRETS_AVAILABLE -eq 'true') { 0 } Else { 1 }) - - name: Setup AWS environment variables - run: | - echo "AWS_REGION=${{ secrets.AWS_REGION }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append - echo "AWS_ACCESS_KEY_ID=${{ secrets.AWS_ACCESS_KEY_ID }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append - echo "AWS_SECRET_ACCESS_KEY=${{ secrets.AWS_SECRET_ACCESS_KEY }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append - - name: Delete leaked RDS resources and security groups - run: | - $cutoff = [DateTimeOffset]::UtcNow.AddHours(-4).ToUnixTimeSeconds() - $tagged = aws resourcegroupstaggingapi get-resources --tag-filters Key=sc-cloud-test,Values=true --output json | ConvertFrom-Json - - $stale = @() - foreach ($resource in $tagged.ResourceTagMappingList) { - $created = ($resource.Tags | Where-Object { $_.Key -eq 'created' }).Value - - if (-not $created) { - Write-Warning "$($resource.ResourceARN) has no created tag, so its age is unknown. Leaving it, delete it by hand." - continue - } - - if ([long]$created -ge $cutoff) { - Write-Output "Leaving $($resource.ResourceARN), it belongs to a run that may still be going." - continue - } - - $stale += $resource.ResourceARN - } - - if (-not $stale) { - Write-Output 'Nothing stale is tagged sc-cloud-test in this account.' - return - } - - # Instances first, then the clusters that hold them, then the security groups the instances - # were using. A security group still in use refuses to go, and is picked up by the next run. - $order = @(':db:', ':cluster:', 'security-group/') - - foreach ($pattern in $order) { - foreach ($arn in $stale | Where-Object { $_ -like "*$pattern*" }) { - $identifier = ($arn -split '[:/]')[-1] - Write-Output "Deleting $arn" - try { - switch -Wildcard ($arn) { - '*:db:*' { aws rds delete-db-instance --db-instance-identifier $identifier --skip-final-snapshot --delete-automated-backups --no-cli-pager --output none; break } - '*:cluster:*' { aws rds delete-db-cluster --db-cluster-identifier $identifier --skip-final-snapshot --no-cli-pager --output none; break } - '*security-group/*' { aws ec2 delete-security-group --group-id $identifier --output none; break } - } - } - catch { - Write-Warning "Could not delete ${arn}: $($_.Exception.Message)" - } - } - } diff --git a/.github/workflows/cloud-database-tests.yml b/.github/workflows/cloud-database-tests.yml index 7d72f2b43c..e38eebcd65 100644 --- a/.github/workflows/cloud-database-tests.yml +++ b/.github/workflows/cloud-database-tests.yml @@ -118,10 +118,19 @@ jobs: echo "AWS_ACCESS_KEY_ID=${{ secrets.AWS_ACCESS_KEY_ID }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append echo "AWS_SECRET_ACCESS_KEY=${{ secrets.AWS_SECRET_ACCESS_KEY }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append # Unlike ci.yml, which holds its cloud resources back until the build is done, provisioning - # starts first: it is the long pole here at anywhere from 3 to 25 minutes, and the teardown - # step runs on failure regardless. + # starts first: it is the long pole here at anywhere from 3 to 25 minutes, and the database is + # torn down by this action's post step whatever the job does afterwards. + # + # It has to come after the login steps above. Post steps run in reverse, so this one's teardown + # runs before azure/login's post step signs the CLI out from under it. - name: Provision database - run: ./tools/cloud/${{ matrix.target }}.ps1 -Action Provision -Name sc-ct-${{ github.run_id }} + uses: ./.github/actions/cloud-database + with: + target: ${{ matrix.target }} + # The server is what isolates one run from another, so this has to be unique per run. It + # includes the attempt because run_id does not change when a run is retried, and the + # previous attempt's server may still be there or still be deleting. + name: sc-ct-${{ github.run_id }}-${{ github.run_attempt }} - name: Wait for build wait: build - name: Run tests @@ -129,6 +138,3 @@ jobs: env: ServiceControl_TESTS_FILTER: ${{ matrix.provider }} PARTICULARSOFTWARE_LICENSE: ${{ secrets.LICENSETEXT }} - - name: Tear down database - if: always() - run: ./tools/cloud/${{ matrix.target }}.ps1 -Action Teardown -Name sc-ct-${{ github.run_id }} diff --git a/tools/cloud/aurora-postgresql.ps1 b/tools/cloud/aurora-postgresql.ps1 deleted file mode 100644 index 464943b62f..0000000000 --- a/tools/cloud/aurora-postgresql.ps1 +++ /dev/null @@ -1,95 +0,0 @@ -# Provisions and tears down an Aurora PostgreSQL cluster for one run of the cloud database tests. - -param( - [Parameter(Mandatory)][ValidateSet('Provision', 'Teardown')][string]$Action, - [Parameter(Mandatory)][string]$Name, - [string]$DatabaseName = 'servicecontrol' -) - -. $PSScriptRoot/common.ps1 - -$instance = "$Name-1" -$adminUser = 'sctestadmin' - -if ($Action -eq 'Teardown') { - # Without waiting for the deletions to finish: an RDS instance takes minutes to disappear, and - # holding the job open for that costs more than the scheduled cleanup workflow does. - Invoke-Teardown "Deleting instance $instance" { - aws rds delete-db-instance --db-instance-identifier $instance --skip-final-snapshot --delete-automated-backups --no-cli-pager --output none - } - - Invoke-Teardown "Deleting cluster $Name" { - aws rds delete-db-cluster --db-cluster-identifier $Name --skip-final-snapshot --no-cli-pager --output none - } - - # Will refuse while the instance still holds it, which is the normal case. The cleanup workflow - # sweeps up whatever is left. - Invoke-Teardown "Deleting security group $Name" { - $groupId = aws ec2 describe-security-groups --filters "Name=group-name,Values=$Name" --query 'SecurityGroups[0].GroupId' --output text - if ($groupId -and $groupId -ne 'None') { - aws ec2 delete-security-group --group-id $groupId --output none - } - } - - return -} - -$password = New-AdminPassword -$runnerIp = Get-RunnerIpAddress -$created = Get-CreatedTimestamp - -$vpcId = aws ec2 describe-vpcs --filters 'Name=isDefault,Values=true' --query 'Vpcs[0].VpcId' --output text -if (-not $vpcId -or $vpcId -eq 'None') { - throw 'This AWS account has no default VPC in this region, so there is no public subnet group for the cluster to use.' -} - -Write-Step "Creating security group $Name in $vpcId, allowing $runnerIp on 5432" -$groupId = aws ec2 create-security-group ` - --group-name $Name ` - --description 'ServiceControl cloud database tests' ` - --vpc-id $vpcId ` - --tag-specifications "ResourceType=security-group,Tags=[{Key=sc-cloud-test,Value=true},{Key=run-id,Value=$Name},{Key=created,Value=$created}]" ` - --query GroupId --output text - -aws ec2 authorize-security-group-ingress ` - --group-id $groupId ` - --protocol tcp ` - --port 5432 ` - --cidr "$runnerIp/32" ` - --output none - -# The engine version is left to the AWS default so that this does not break every time a pinned -# minor version is retired. -Write-Step "Creating Aurora PostgreSQL cluster $Name" -aws rds create-db-cluster ` - --db-cluster-identifier $Name ` - --engine aurora-postgresql ` - --master-username $adminUser ` - --master-user-password $password ` - --database-name $DatabaseName ` - --vpc-security-group-ids $groupId ` - --no-deletion-protection ` - --backup-retention-period 1 ` - --tags "Key=sc-cloud-test,Value=true" "Key=run-id,Value=$Name" "Key=created,Value=$created" ` - --no-cli-pager --output none - -Write-Step "Creating instance $instance" -aws rds create-db-instance ` - --db-instance-identifier $instance ` - --db-cluster-identifier $Name ` - --engine aurora-postgresql ` - --db-instance-class db.t4g.medium ` - --publicly-accessible ` - --tags "Key=sc-cloud-test,Value=true" "Key=run-id,Value=$Name" "Key=created,Value=$created" ` - --no-cli-pager --output none - -Write-Step 'Waiting for the instance to become available' -aws rds wait db-instance-available --db-instance-identifier $instance - -$endpoint = aws rds describe-db-clusters --db-cluster-identifier $Name --query 'DBClusters[0].Endpoint' --output text - -# Trust Server Certificate because RDS presents an Amazon CA that is not in the runner's trust store. -# The connection is still encrypted; only the certificate chain goes unverified. -$connectionString = "Host=$endpoint;Port=5432;Database=$DatabaseName;Username=$adminUser;Password=$password;Ssl Mode=Require;Trust Server Certificate=true;Timeout=60" - -Set-PersistenceConnectionString -Provider PostgreSql -ConnectionString $connectionString diff --git a/tools/cloud/azure-postgresql.ps1 b/tools/cloud/azure-postgresql.ps1 deleted file mode 100644 index 2a1bee0dfc..0000000000 --- a/tools/cloud/azure-postgresql.ps1 +++ /dev/null @@ -1,64 +0,0 @@ -# Provisions and tears down an Azure Database for PostgreSQL flexible server for one run of the -# cloud database tests. -# -# Everything lives in a single resource group named after the run, so teardown is one call and a -# partly provisioned run cleans up as completely as a successful one. - -param( - [Parameter(Mandatory)][ValidateSet('Provision', 'Teardown')][string]$Action, - [Parameter(Mandatory)][string]$Name, - [string]$Location = 'eastus2', - [string]$DatabaseName = 'servicecontrol' -) - -. $PSScriptRoot/common.ps1 - -$resourceGroup = $Name -$server = $Name -$adminUser = 'sctestadmin' - -if ($Action -eq 'Teardown') { - Invoke-Teardown "Deleting resource group $resourceGroup" { - az group delete --name $resourceGroup --yes --no-wait --only-show-errors - } - return -} - -$password = New-AdminPassword -$runnerIp = Get-RunnerIpAddress -$created = Get-CreatedTimestamp - -Write-Step "Creating resource group $resourceGroup in $Location" -az group create ` - --name $resourceGroup ` - --location $Location ` - --tags sc-cloud-test=true "run-id=$Name" "created=$created" ` - --only-show-errors --output none - -# Burstable B1ms is the cheapest tier, and the suites are latency bound rather than CPU bound. -# --public-access opens the firewall to just this runner as part of creation. -Write-Step "Creating PostgreSQL flexible server $server, allowing $runnerIp" -az postgres flexible-server create ` - --name $server ` - --resource-group $resourceGroup ` - --location $Location ` - --admin-user $adminUser ` - --admin-password $password ` - --tier Burstable ` - --sku-name Standard_B1ms ` - --storage-size 32 ` - --version 16 ` - --public-access $runnerIp ` - --yes ` - --only-show-errors --output none - -Write-Step "Creating database $DatabaseName" -az postgres flexible-server db create ` - --database-name $DatabaseName ` - --resource-group $resourceGroup ` - --server-name $server ` - --only-show-errors --output none - -$connectionString = "Host=$server.postgres.database.azure.com;Port=5432;Database=$DatabaseName;Username=$adminUser;Password=$password;Ssl Mode=Require;Timeout=60" - -Set-PersistenceConnectionString -Provider PostgreSql -ConnectionString $connectionString diff --git a/tools/cloud/azure-sql.ps1 b/tools/cloud/azure-sql.ps1 deleted file mode 100644 index c46e386196..0000000000 --- a/tools/cloud/azure-sql.ps1 +++ /dev/null @@ -1,73 +0,0 @@ -# Provisions and tears down an Azure SQL Database for one run of the cloud database tests. -# -# Everything lives in a single resource group named after the run, so teardown is one call and a -# partly provisioned run cleans up as completely as a successful one. - -param( - [Parameter(Mandatory)][ValidateSet('Provision', 'Teardown')][string]$Action, - [Parameter(Mandatory)][string]$Name, - [string]$Location = 'eastus2', - [string]$DatabaseName = 'servicecontrol' -) - -. $PSScriptRoot/common.ps1 - -$resourceGroup = $Name -$server = $Name -$adminUser = 'sctestadmin' - -if ($Action -eq 'Teardown') { - # One call takes the server, the database and the firewall rule with it. --no-wait because - # nothing later in the run depends on the deletion having finished. - Invoke-Teardown "Deleting resource group $resourceGroup" { - az group delete --name $resourceGroup --yes --no-wait --only-show-errors - } - return -} - -$password = New-AdminPassword -$runnerIp = Get-RunnerIpAddress -$created = Get-CreatedTimestamp - -Write-Step "Creating resource group $resourceGroup in $Location" -az group create ` - --name $resourceGroup ` - --location $Location ` - --tags sc-cloud-test=true "run-id=$Name" "created=$created" ` - --only-show-errors --output none - -Write-Step "Creating SQL server $server" -az sql server create ` - --name $server ` - --resource-group $resourceGroup ` - --location $Location ` - --admin-user $adminUser ` - --admin-password $password ` - --only-show-errors --output none - -Write-Step "Allowing $runnerIp through the server firewall" -az sql server firewall-rule create ` - --name github-runner ` - --resource-group $resourceGroup ` - --server $server ` - --start-ip-address $runnerIp ` - --end-ip-address $runnerIp ` - --only-show-errors --output none - -# S0 is the cheapest tier that still provisions in a couple of minutes. Local backup redundancy -# because the database does not outlive the run. -Write-Step "Creating database $DatabaseName" -az sql db create ` - --name $DatabaseName ` - --resource-group $resourceGroup ` - --server $server ` - --service-objective S0 ` - --backup-storage-redundancy Local ` - --only-show-errors --output none - -$connectionString = "Server=tcp:$server.database.windows.net,1433;Initial Catalog=$DatabaseName;User ID=$adminUser;Password=$password;Encrypt=True;TrustServerCertificate=False;Connect Timeout=60" - -Write-Step 'Verifying the database is reachable and has Full-Text Search' -Invoke-SqlServerVerification -ConnectionString $connectionString - -Set-PersistenceConnectionString -Provider SqlServer -ConnectionString $connectionString diff --git a/tools/cloud/common.ps1 b/tools/cloud/common.ps1 deleted file mode 100644 index 91f6dc225a..0000000000 --- a/tools/cloud/common.ps1 +++ /dev/null @@ -1,109 +0,0 @@ -# Shared helpers for the cloud database provisioning scripts in this folder. Dot-sourced, not run -# directly. - -# Native commands (az, aws) fail by returning a non-zero exit code rather than throwing, and a -# provisioning script that carries on after a failed step leaves half-built infrastructure behind. -$PSNativeCommandUseErrorActionPreference = $true -$ErrorActionPreference = 'Stop' - -function Write-Step { - param([Parameter(Mandatory)][string]$Message) - - Write-Output "==> $Message" -} - -# Satisfies the complexity rules of all four services at once, and avoids the characters that would -# have to be escaped in a connection string or are rejected outright by RDS (/ " @ and space). -function New-AdminPassword { - $upper = 'ABCDEFGHJKLMNPQRSTUVWXYZ' - $lower = 'abcdefghijkmnpqrstuvwxyz' - $digit = '23456789' - $symbol = '!#$%*()-_+' - $all = $upper + $lower + $digit + $symbol - - $characters = @( - Get-Random -InputObject $upper.ToCharArray() - Get-Random -InputObject $lower.ToCharArray() - Get-Random -InputObject $digit.ToCharArray() - Get-Random -InputObject $symbol.ToCharArray() - ) - $characters += 1..24 | ForEach-Object { Get-Random -InputObject $all.ToCharArray() } - - $password = -join ($characters | Sort-Object { Get-Random }) - - # Straight to stdout rather than through the output stream, which the caller is capturing as the - # return value. Everything the password is later embedded in, the connection string included, is - # redacted along with it. - [Console]::WriteLine("::add-mask::$password") - - return $password -} - -# The servers are reachable from the internet, so each one is firewalled to the single address this -# job connects from. -function Get-RunnerIpAddress { - try { - return (Invoke-RestMethod -Uri 'https://api.ipify.org' -TimeoutSec 30).Trim() - } - catch { - throw "Could not determine the runner's public IP address, which is needed to open the database firewall to it. $($_.Exception.Message)" - } -} - -# Neither an Azure resource group nor an EC2 security group records when it was created, so the -# scheduled cleanup workflow needs the provisioning scripts to stamp it. Epoch seconds because both -# clouds restrict the characters a tag value may contain. -function Get-CreatedTimestamp { - return [DateTimeOffset]::UtcNow.ToUnixTimeSeconds().ToString() -} - -function Set-PersistenceConnectionString { - param( - [Parameter(Mandatory)][ValidateSet('SqlServer', 'PostgreSql')][string]$Provider, - [Parameter(Mandatory)][string]$ConnectionString - ) - - $name = "ServiceControl_Persistence_${Provider}_ConnectionString" - - if (-not $Env:GITHUB_ENV) { - Write-Step "GITHUB_ENV is not set, so $name was not exported. Set it yourself to run the tests against this server." - return - } - - "$name=$ConnectionString" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf8 -Append - Write-Step "Exported $name" -} - -# Creates the test database if the service could not, and fails the run if the server has no -# Full-Text Search. -function Invoke-SqlServerVerification { - param([Parameter(Mandatory)][string]$ConnectionString) - - # Run from this folder: `dotnet run .cs` picks up any project file in the working - # directory, and by this point the repo root holds the tests.proj that select-test-projects.ps1 - # generated. - Push-Location $PSScriptRoot - try { - dotnet run ./verify-sqlserver.cs -- $ConnectionString - } - finally { - Pop-Location - } -} - -# Teardown runs even when provisioning failed part way, so it has to tolerate resources that were -# never created. -function Invoke-Teardown { - param( - [Parameter(Mandatory)][string]$Description, - [Parameter(Mandatory)][scriptblock]$Action - ) - - Write-Step $Description - try { - & $Action - } - catch { - Write-Warning "$Description failed: $($_.Exception.Message). The scheduled cloud-database-cleanup workflow will pick up anything left behind." - } -} diff --git a/tools/cloud/rds-sqlserver.ps1 b/tools/cloud/rds-sqlserver.ps1 deleted file mode 100644 index b5aa78ef95..0000000000 --- a/tools/cloud/rds-sqlserver.ps1 +++ /dev/null @@ -1,90 +0,0 @@ -# Provisions and tears down an RDS SQL Server instance for one run of the cloud database tests. -# -# This is the slowest of the four targets to provision, around 15 to 25 minutes, so the workflow -# starts it before waiting on the build. - -param( - [Parameter(Mandatory)][ValidateSet('Provision', 'Teardown')][string]$Action, - [Parameter(Mandatory)][string]$Name, - [string]$DatabaseName = 'servicecontrol', - # Web edition is the cheapest licence-included option. Full-Text Search availability is what - # decides whether it is usable; verify-sqlserver.cs fails the run with a clear message if this - # edition turns out not to have it, in which case move to sqlserver-se. - [string]$Engine = 'sqlserver-web' -) - -. $PSScriptRoot/common.ps1 - -$adminUser = 'sctestadmin' - -if ($Action -eq 'Teardown') { - Invoke-Teardown "Deleting instance $Name" { - aws rds delete-db-instance --db-instance-identifier $Name --skip-final-snapshot --delete-automated-backups --no-cli-pager --output none - } - - Invoke-Teardown "Deleting security group $Name" { - $groupId = aws ec2 describe-security-groups --filters "Name=group-name,Values=$Name" --query 'SecurityGroups[0].GroupId' --output text - if ($groupId -and $groupId -ne 'None') { - aws ec2 delete-security-group --group-id $groupId --output none - } - } - - return -} - -$password = New-AdminPassword -$runnerIp = Get-RunnerIpAddress -$created = Get-CreatedTimestamp - -$vpcId = aws ec2 describe-vpcs --filters 'Name=isDefault,Values=true' --query 'Vpcs[0].VpcId' --output text -if (-not $vpcId -or $vpcId -eq 'None') { - throw 'This AWS account has no default VPC in this region, so there is no public subnet group for the instance to use.' -} - -Write-Step "Creating security group $Name in $vpcId, allowing $runnerIp on 1433" -$groupId = aws ec2 create-security-group ` - --group-name $Name ` - --description 'ServiceControl cloud database tests' ` - --vpc-id $vpcId ` - --tag-specifications "ResourceType=security-group,Tags=[{Key=sc-cloud-test,Value=true},{Key=run-id,Value=$Name},{Key=created,Value=$created}]" ` - --query GroupId --output text - -aws ec2 authorize-security-group-ingress ` - --group-id $groupId ` - --protocol tcp ` - --port 1433 ` - --cidr "$runnerIp/32" ` - --output none - -# No automated backups and no standby: the instance does not outlive the run, and both slow -# provisioning down. -Write-Step "Creating RDS SQL Server instance $Name" -aws rds create-db-instance ` - --db-instance-identifier $Name ` - --engine $Engine ` - --db-instance-class db.t3.small ` - --allocated-storage 20 ` - --master-username $adminUser ` - --master-user-password $password ` - --vpc-security-group-ids $groupId ` - --license-model license-included ` - --publicly-accessible ` - --no-multi-az ` - --backup-retention-period 0 ` - --tags "Key=sc-cloud-test,Value=true" "Key=run-id,Value=$Name" "Key=created,Value=$created" ` - --no-cli-pager --output none - -Write-Step 'Waiting for the instance to become available' -aws rds wait db-instance-available --db-instance-identifier $Name - -$endpoint = aws rds describe-db-instances --db-instance-identifier $Name --query 'DBInstances[0].Endpoint.Address' --output text - -# Trust Server Certificate because RDS presents an Amazon CA that is not in the runner's trust store. -# The connection is still encrypted; only the certificate chain goes unverified. -$connectionString = "Server=tcp:$endpoint,1433;Initial Catalog=$DatabaseName;User ID=$adminUser;Password=$password;Encrypt=True;TrustServerCertificate=True;Connect Timeout=60" - -# RDS cannot create a user database as part of the instance, unlike every other target here. -Write-Step "Creating database $DatabaseName and verifying Full-Text Search" -Invoke-SqlServerVerification -ConnectionString $connectionString - -Set-PersistenceConnectionString -Provider SqlServer -ConnectionString $connectionString diff --git a/tools/cloud/verify-sqlserver.cs b/tools/cloud/verify-sqlserver.cs deleted file mode 100644 index d7961f1d60..0000000000 --- a/tools/cloud/verify-sqlserver.cs +++ /dev/null @@ -1,52 +0,0 @@ -#:package Microsoft.Data.SqlClient@6.1.1 - -// Creates the ServiceControl test database if the service could not create it during provisioning, -// and fails the run if the server has no Full-Text Search. Message search is not optional, so an -// instance without it would otherwise fail every search test twenty minutes later with a much less -// obvious error. - -using Microsoft.Data.SqlClient; - -if (args.Length != 1) -{ - Console.Error.WriteLine("usage: dotnet run verify-sqlserver.cs -- "); - return 1; -} - -var builder = new SqlConnectionStringBuilder(args[0]); -var database = builder.InitialCatalog; - -builder.InitialCatalog = "master"; - -try -{ - await using var master = new SqlConnection(builder.ConnectionString); - await master.OpenAsync(); - - await using (var create = master.CreateCommand()) - { - create.CommandText = $"IF DB_ID(N'{database}') IS NULL CREATE DATABASE [{database}]"; - create.CommandTimeout = 300; - await create.ExecuteNonQueryAsync(); - } - - await using (var fullText = master.CreateCommand()) - { - fullText.CommandText = "SELECT CONVERT(int, ISNULL(SERVERPROPERTY('IsFullTextInstalled'), 0))"; - if ((int)(await fullText.ExecuteScalarAsync())! != 1) - { - Console.Error.WriteLine($"{builder.DataSource} does not have SQL Server Full-Text Search installed, which ServiceControl requires. On RDS, check that the edition supports it and that the option group includes it."); - return 1; - } - } -} -catch (SqlException e) -{ - // Without the stack trace, which says nothing useful about a server that is unreachable or - // rejecting the login. - Console.Error.WriteLine($"Could not prepare {builder.DataSource}: {e.Message}"); - return 1; -} - -Console.WriteLine($"Database '{database}' is present on {builder.DataSource} and Full-Text Search is installed."); -return 0; From b692ac29e231953af17feb3bbb9ace557d8ba293 Mon Sep 17 00:00:00 2001 From: John Simons Date: Thu, 10 Sep 2026 10:32:38 +1000 Subject: [PATCH 05/14] Fix branch name --- .github/workflows/cloud-database-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/cloud-database-tests.yml b/.github/workflows/cloud-database-tests.yml index e38eebcd65..bc9736db08 100644 --- a/.github/workflows/cloud-database-tests.yml +++ b/.github/workflows/cloud-database-tests.yml @@ -6,7 +6,7 @@ on: - '[0-9]+.[0-9]+.[0-9]+' - '[0-9]+.[0-9]+.[0-9]+-*' branches: - - john/cloud-ci-tests + - john/cloud_ci_tests workflow_dispatch: inputs: target: From 5c006ab7675fa1b0fc4055382b3880406ddcac88 Mon Sep 17 00:00:00 2001 From: John Simons Date: Thu, 10 Sep 2026 11:01:16 +1000 Subject: [PATCH 06/14] Fix AWS default VPC creation, SQL Server verify retry scope, and Azure PostgreSQL CLI flag Three independent fixes to the cloud database action: - AWS: instead of failing when a region has no default VPC, create one. RDS needs the default VPC's subnet group for publicly accessible instances, and the account may not have one in every region. Concurrent jobs racing to create it are handled by re-reading after the call. An IAM policy document is added alongside as a reference for the required permissions. - SQL Server verify script: only the initial TCP connection is retried. The previous loop retried database creation and Full-Text Search checks too, turning a real server-side failure into a silent ten-minute wait. The post-connection work is moved outside the retry loop, and `sys.databases` replaces `DB_ID` to handle Azure SQL's logical master database correctly. - Azure PostgreSQL: remove `--high-availability Disabled` because the CLI version on the runner does not accept the flag. --- .../cloud-database/aws-iam-policy.json | 58 +++++++++++++++ .github/actions/cloud-database/aws.mts | 32 +++++++- .../cloud-database/azure-postgresql.mts | 4 +- .../cloud-database/verify-postgresql.cs | 6 +- .../cloud-database/verify-sqlserver.cs | 73 ++++++++++++------- 5 files changed, 136 insertions(+), 37 deletions(-) create mode 100644 .github/actions/cloud-database/aws-iam-policy.json diff --git a/.github/actions/cloud-database/aws-iam-policy.json b/.github/actions/cloud-database/aws-iam-policy.json new file mode 100644 index 0000000000..b18fe9f0bb --- /dev/null +++ b/.github/actions/cloud-database/aws-iam-policy.json @@ -0,0 +1,58 @@ +{ + "Version": "2012-10-17", + "Statement": [ + { + "Sid": "ReadVpcAndSecurityGroups", + "Effect": "Allow", + "Action": [ + "ec2:DescribeVpcs", + "ec2:DescribeSecurityGroups", + "ec2:DescribeSubnets", + "ec2:DescribeAvailabilityZones" + ], + "Resource": "*" + }, + { + "Sid": "CreateDefaultVpcOncePerRegion", + "Effect": "Allow", + "Action": [ + "ec2:CreateDefaultVpc" + ], + "Resource": "*" + }, + { + "Sid": "ManageTestSecurityGroups", + "Effect": "Allow", + "Action": [ + "ec2:CreateSecurityGroup", + "ec2:CreateTags", + "ec2:AuthorizeSecurityGroupIngress", + "ec2:DeleteSecurityGroup" + ], + "Resource": "*" + }, + { + "Sid": "ReadRds", + "Effect": "Allow", + "Action": [ + "rds:DescribeDBInstances", + "rds:DescribeDBClusters", + "rds:DescribeDBSubnetGroups" + ], + "Resource": "*" + }, + { + "Sid": "ManageTestDatabases", + "Effect": "Allow", + "Action": [ + "rds:CreateDBInstance", + "rds:CreateDBCluster", + "rds:DeleteDBInstance", + "rds:DeleteDBCluster", + "rds:DeleteDBInstanceAutomatedBackup", + "rds:AddTagsToResource" + ], + "Resource": "*" + } + ] +} diff --git a/.github/actions/cloud-database/aws.mts b/.github/actions/cloud-database/aws.mts index eb8f7c4447..383f7f3128 100644 --- a/.github/actions/cloud-database/aws.mts +++ b/.github/actions/cloud-database/aws.mts @@ -2,14 +2,38 @@ import { step, run, capture, captureJson } from './common.mts'; -function defaultVpcId(): string { +function findDefaultVpc(): string | null { const vpcId = capture('aws', ['ec2', 'describe-vpcs', '--filters', 'Name=isDefault,Values=true', '--query', 'Vpcs[0].VpcId', '--output', 'text']); - if (!vpcId || vpcId === 'None') { - throw new Error('This AWS account has no default VPC in this region, so there is no public subnet group for the database to use.'); + return vpcId && vpcId !== 'None' ? vpcId : null; +} + +// RDS places a publicly accessible instance in the default VPC's subnet group, and an account has at +// most one default VPC per region. It is account infrastructure rather than this run's, so it is +// created when missing and never deleted. +function defaultVpcId(): string { + const existing = findDefaultVpc(); + + if (existing) { + return existing; + } + + step('This region has no default VPC, creating one'); + + try { + run('aws', ['ec2', 'create-default-vpc', '--no-cli-pager']); + } catch { + // Both AWS targets provision at the same time, so the other job may have created it between + // the lookup above and this call. The re-read below settles who won. + } + + const created = findDefaultVpc(); + + if (!created) { + throw new Error('This region has no default VPC and one could not be created. Check that the credentials allow ec2:CreateDefaultVpc.'); } - return vpcId; + return created; } // An EC2 security group does not record when it was created, so this tag is what the stale sweep diff --git a/.github/actions/cloud-database/azure-postgresql.mts b/.github/actions/cloud-database/azure-postgresql.mts index 7891db709e..e72d3a77a9 100644 --- a/.github/actions/cloud-database/azure-postgresql.mts +++ b/.github/actions/cloud-database/azure-postgresql.mts @@ -14,7 +14,8 @@ async function provision(name: string): Promise { const tags = azure.tags(name); // General Purpose with 4 vCores rather than a burstable tier, so the run is not throttled part - // way through. No high availability and no geo-redundant backup: nothing here outlives the job. + // way through. High availability is left at its default of disabled rather than passed + // explicitly, because the CLI on the runner does not accept --high-availability. // --public-access opens the firewall to just this runner as part of creation. step(`Creating PostgreSQL flexible server ${name} in ${azure.resourceGroup} (${location}), allowing ${runnerIp}`); run('az', ['postgres', 'flexible-server', 'create', @@ -27,7 +28,6 @@ async function provision(name: string): Promise { '--sku-name', 'Standard_D4ds_v5', '--storage-size', '128', '--version', '16', - '--high-availability', 'Disabled', '--geo-redundant-backup', 'Disabled', '--public-access', runnerIp, ...tags, diff --git a/.github/actions/cloud-database/verify-postgresql.cs b/.github/actions/cloud-database/verify-postgresql.cs index fe162bcd80..f8359a6975 100644 --- a/.github/actions/cloud-database/verify-postgresql.cs +++ b/.github/actions/cloud-database/verify-postgresql.cs @@ -2,9 +2,9 @@ // Waits until the server actually accepts connections on the test database. // -// The provisioning CLIs report a server as available before it is necessarily reachable, so this -// retries rather than taking the first refusal as final. Without it the first thing to touch a -// half-ready server would be the test run, which reports the problem far less clearly. +// Only the connection is retried. The provisioning CLIs report a server as available before it is +// necessarily reachable, so a refused connection is worth waiting out. Anything the server actually +// answers is a real result, and retrying it would just turn a clear failure into a ten minute one. using Npgsql; diff --git a/.github/actions/cloud-database/verify-sqlserver.cs b/.github/actions/cloud-database/verify-sqlserver.cs index f741909586..b1879b1ed6 100644 --- a/.github/actions/cloud-database/verify-sqlserver.cs +++ b/.github/actions/cloud-database/verify-sqlserver.cs @@ -1,12 +1,12 @@ #:package Microsoft.Data.SqlClient@6.1.1 -// Waits until the server actually accepts connections, creates the test database if the service -// could not create it during provisioning, and fails the run if the server has no Full-Text Search. +// Waits until the server accepts connections, creates the test database if the provisioning CLI +// could not, and fails the run if the server has no Full-Text Search. // -// The provisioning CLIs report a database as created before it is necessarily reachable, and an -// Azure firewall rule takes a moment to propagate, so this retries rather than taking the first -// refusal as final. Message search is not optional either, so a server without Full-Text Search is -// better caught here than twenty minutes later in the search tests. +// Only the connection is retried. The provisioning CLIs report a server as ready before it is +// necessarily reachable, and an Azure firewall rule takes a moment to propagate, so a refused +// connection is worth waiting out. Everything after that is a real answer from the server, and +// retrying it would just turn a clear failure into a ten minute one. using Microsoft.Data.SqlClient; @@ -23,6 +23,7 @@ var deadline = DateTime.UtcNow.AddMinutes(10); var attempt = 0; +SqlConnection master; while (true) { @@ -30,32 +31,13 @@ try { - await using var master = new SqlConnection(builder.ConnectionString); + master = new SqlConnection(builder.ConnectionString); await master.OpenAsync(); - - await using (var create = master.CreateCommand()) - { - create.CommandText = $"IF DB_ID(N'{database}') IS NULL CREATE DATABASE [{database}]"; - create.CommandTimeout = 300; - await create.ExecuteNonQueryAsync(); - } - - await using (var fullText = master.CreateCommand()) - { - fullText.CommandText = "SELECT CONVERT(int, ISNULL(SERVERPROPERTY('IsFullTextInstalled'), 0))"; - if ((int)(await fullText.ExecuteScalarAsync())! != 1) - { - Console.Error.WriteLine($"{builder.DataSource} does not have SQL Server Full-Text Search installed, which ServiceControl requires. On RDS, check that the edition supports it and that the option group includes it."); - return 1; - } - } - - Console.WriteLine($"Database '{database}' is present on {builder.DataSource} and Full-Text Search is installed, after {attempt} attempt(s)."); - return 0; + break; } catch (SqlException e) when (DateTime.UtcNow < deadline) { - Console.WriteLine($"{builder.DataSource} is not ready yet (attempt {attempt}): {e.Message.Split('\n')[0]}"); + Console.WriteLine($"{builder.DataSource} is not reachable yet (attempt {attempt}): {e.Message.Split('\n')[0]}"); await Task.Delay(TimeSpan.FromSeconds(10)); } catch (SqlException e) @@ -64,3 +46,38 @@ return 1; } } + +await using (master) +{ + // sys.databases rather than DB_ID: on Azure SQL the master database is a logical one, and + // DB_ID returns null for a database that is sitting right there on the same server. + bool exists; + await using (var lookup = master.CreateCommand()) + { + lookup.CommandText = "SELECT COUNT(*) FROM sys.databases WHERE name = @database"; + lookup.Parameters.AddWithValue("@database", database); + exists = (int)(await lookup.ExecuteScalarAsync())! > 0; + } + + if (!exists) + { + Console.WriteLine($"Creating database '{database}'."); + await using var create = master.CreateCommand(); + create.CommandText = $"CREATE DATABASE [{database}]"; + create.CommandTimeout = 300; + await create.ExecuteNonQueryAsync(); + } + + await using (var fullText = master.CreateCommand()) + { + fullText.CommandText = "SELECT CONVERT(int, ISNULL(SERVERPROPERTY('IsFullTextInstalled'), 0))"; + if ((int)(await fullText.ExecuteScalarAsync())! != 1) + { + Console.Error.WriteLine($"{builder.DataSource} does not have SQL Server Full-Text Search installed, which ServiceControl requires. On RDS, check that the edition supports it and that the option group includes it."); + return 1; + } + } +} + +Console.WriteLine($"Database '{database}' is present on {builder.DataSource} and Full-Text Search is installed, after {attempt} connection attempt(s)."); +return 0; From a4fc241ba8dea00c1bb3ede233e095343e990bbe Mon Sep 17 00:00:00 2001 From: John Simons Date: Thu, 10 Sep 2026 11:26:15 +1000 Subject: [PATCH 07/14] Fix RDS provisioning by creating a dedicated DB subnet group and using a fixed AWS region RDS requires a subnet group spanning at least two availability zones, but regions do not reliably have a default one even when a default VPC exists. This adds a `dbSubnetGroupName` helper that creates and reuses a named subnet group (`servicecontrol-cloud-tests`) as shared account infrastructure, matching the same concurrent-creation pattern already used for the default VPC. The workflow also switches RDS targets to a fixed region (`us-east-2`) rather than the shared `AWS_REGION` secret, which is at its VPC ceiling and cannot host publicly accessible RDS instances. A `tryRun` helper is added to `common.mts` to capture CLI failure output without throwing, allowing callers to distinguish a race-condition loss from a genuine error. --- .../cloud-database/aurora-postgresql.mts | 1 + .../cloud-database/aws-iam-policy.json | 1 + .github/actions/cloud-database/aws.mts | 51 +++++++++++++++---- .github/actions/cloud-database/common.mts | 13 +++++ .../actions/cloud-database/rds-sqlserver.mts | 1 + .github/workflows/cloud-database-tests.yml | 6 ++- 6 files changed, 62 insertions(+), 11 deletions(-) diff --git a/.github/actions/cloud-database/aurora-postgresql.mts b/.github/actions/cloud-database/aurora-postgresql.mts index e7f1332ce5..d1408c0fe0 100644 --- a/.github/actions/cloud-database/aurora-postgresql.mts +++ b/.github/actions/cloud-database/aurora-postgresql.mts @@ -28,6 +28,7 @@ async function provision(name: string): Promise { '--master-user-password', password, '--database-name', databaseName, '--vpc-security-group-ids', groupId, + '--db-subnet-group-name', aws.dbSubnetGroupName(), '--no-deletion-protection', '--backup-retention-period', '1', ...tags, diff --git a/.github/actions/cloud-database/aws-iam-policy.json b/.github/actions/cloud-database/aws-iam-policy.json index b18fe9f0bb..032603043c 100644 --- a/.github/actions/cloud-database/aws-iam-policy.json +++ b/.github/actions/cloud-database/aws-iam-policy.json @@ -47,6 +47,7 @@ "Action": [ "rds:CreateDBInstance", "rds:CreateDBCluster", + "rds:CreateDBSubnetGroup", "rds:DeleteDBInstance", "rds:DeleteDBCluster", "rds:DeleteDBInstanceAutomatedBackup", diff --git a/.github/actions/cloud-database/aws.mts b/.github/actions/cloud-database/aws.mts index 383f7f3128..39aa9b3d13 100644 --- a/.github/actions/cloud-database/aws.mts +++ b/.github/actions/cloud-database/aws.mts @@ -1,6 +1,6 @@ // Helpers shared by the two AWS targets. -import { step, run, capture, captureJson } from './common.mts'; +import { step, run, tryRun, capture, captureJson } from './common.mts'; function findDefaultVpc(): string | null { const vpcId = capture('aws', ['ec2', 'describe-vpcs', '--filters', 'Name=isDefault,Values=true', '--query', 'Vpcs[0].VpcId', '--output', 'text']); @@ -20,17 +20,14 @@ function defaultVpcId(): string { step('This region has no default VPC, creating one'); - try { - run('aws', ['ec2', 'create-default-vpc', '--no-cli-pager']); - } catch { - // Both AWS targets provision at the same time, so the other job may have created it between - // the lookup above and this call. The re-read below settles who won. - } - + // Both AWS targets provision at the same time, so the other job may have created it between the + // lookup above and this call. The re-read below settles who won, which is why the failure is + // held rather than thrown. + const failure = tryRun('aws', ['ec2', 'create-default-vpc', '--no-cli-pager']); const created = findDefaultVpc(); if (!created) { - throw new Error('This region has no default VPC and one could not be created. Check that the credentials allow ec2:CreateDefaultVpc.'); + throw new Error(`This region has no default VPC and one could not be created: ${failure}`); } return created; @@ -42,6 +39,40 @@ function createdTimestamp(): string { return Math.floor(Date.now() / 1000).toString(); } +// RDS needs a subnet group spanning at least two availability zones. A region does not reliably +// have one called "default", even where a default VPC exists, so this owns one rather than betting +// on RDS creating it. Like the default VPC it is account infrastructure: created when missing, +// never deleted, and shared by concurrent runs. +const subnetGroup = 'servicecontrol-cloud-tests'; + +function dbSubnetGroupName(): string { + const existing = capture('aws', ['rds', 'describe-db-subnet-groups', '--query', `length(DBSubnetGroups[?DBSubnetGroupName=='${subnetGroup}'])`, '--output', 'text', '--no-cli-pager']); + + if (existing !== '0') { + return subnetGroup; + } + + const subnets = capture('aws', ['ec2', 'describe-subnets', '--filters', `Name=vpc-id,Values=${defaultVpcId()}`, 'Name=default-for-az,Values=true', '--query', 'Subnets[].SubnetId', '--output', 'text']).split(/\s+/).filter(Boolean); + + if (subnets.length < 2) { + throw new Error(`The default VPC has ${subnets.length} default subnet(s), and RDS needs at least two availability zones.`); + } + + step(`Creating DB subnet group ${subnetGroup} across ${subnets.length} subnets`); + const failure = tryRun('aws', ['rds', 'create-db-subnet-group', + '--db-subnet-group-name', subnetGroup, + '--db-subnet-group-description', 'ServiceControl cloud database tests', + '--subnet-ids', ...subnets, + '--no-cli-pager']); + + // The other AWS target provisions at the same time and may have created it in between. + if (failure && capture('aws', ['rds', 'describe-db-subnet-groups', '--query', `length(DBSubnetGroups[?DBSubnetGroupName=='${subnetGroup}'])`, '--output', 'text', '--no-cli-pager']) === '0') { + throw new Error(`Could not create the DB subnet group: ${failure}`); + } + + return subnetGroup; +} + function createSecurityGroup(name: string): { groupId: string; created: string } { const vpcId = defaultVpcId(); const created = createdTimestamp(); @@ -100,4 +131,4 @@ function tags(name: string, created: string): string[] { return ['--tags', 'Key=sc-cloud-test,Value=true', `Key=run-id,Value=${name}`, `Key=created,Value=${created}`]; } -export { createSecurityGroup, allowRunner, deleteSecurityGroup, removeStaleSecurityGroups, tags }; +export { dbSubnetGroupName, createSecurityGroup, allowRunner, deleteSecurityGroup, removeStaleSecurityGroups, tags }; diff --git a/.github/actions/cloud-database/common.mts b/.github/actions/cloud-database/common.mts index 4e828d2604..c6e69a8f9e 100644 --- a/.github/actions/cloud-database/common.mts +++ b/.github/actions/cloud-database/common.mts @@ -17,6 +17,18 @@ function run(command: string, args: string[]): void { execFileSync(command, args, { stdio: 'inherit' }); } +// Runs a command that is allowed to fail, returning the reason it gave when it does. Reporting the +// CLI's own message beats guessing at the cause in an error string. +function tryRun(command: string, args: string[]): string | null { + try { + execFileSync(command, args, { stdio: ['ignore', 'inherit', 'pipe'], encoding: 'utf8' }); + return null; + } catch (error) { + const stderr = String((error as { stderr?: unknown }).stderr ?? '').trim(); + return stderr.split('\n').filter(Boolean).pop() ?? (error as Error).message; + } +} + function capture(command: string, args: string[]): string { return execFileSync(command, args, { encoding: 'utf8' }).trim(); } @@ -99,6 +111,7 @@ function teardownStep(description: string, action: () => void): void { export { step, run, + tryRun, capture, captureJson, newAdminPassword, diff --git a/.github/actions/cloud-database/rds-sqlserver.mts b/.github/actions/cloud-database/rds-sqlserver.mts index 4f05375ee4..f6571389b6 100644 --- a/.github/actions/cloud-database/rds-sqlserver.mts +++ b/.github/actions/cloud-database/rds-sqlserver.mts @@ -36,6 +36,7 @@ async function provision(name: string): Promise { '--master-username', adminUser, '--master-user-password', password, '--vpc-security-group-ids', groupId, + '--db-subnet-group-name', aws.dbSubnetGroupName(), '--license-model', 'license-included', '--publicly-accessible', '--no-multi-az', diff --git a/.github/workflows/cloud-database-tests.yml b/.github/workflows/cloud-database-tests.yml index bc9736db08..d9e239c732 100644 --- a/.github/workflows/cloud-database-tests.yml +++ b/.github/workflows/cloud-database-tests.yml @@ -22,6 +22,10 @@ on: - rds-sqlserver env: DOTNET_NOLOGO: true + # Its own region rather than the AWS_REGION secret the SQS tests use: that one is at its VPC + # ceiling, so RDS has no default VPC to place a publicly accessible instance in. us-east-2 already + # has one, has VPC headroom, and offers both instance classes these targets ask for. + AWS_DATABASE_REGION: us-east-2 defaults: run: shell: pwsh @@ -114,7 +118,7 @@ jobs: - name: Setup AWS environment variables if: startsWith(matrix.target, 'aurora-') || startsWith(matrix.target, 'rds-') run: | - echo "AWS_REGION=${{ secrets.AWS_REGION }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append + echo "AWS_REGION=${{ env.AWS_DATABASE_REGION }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append echo "AWS_ACCESS_KEY_ID=${{ secrets.AWS_ACCESS_KEY_ID }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append echo "AWS_SECRET_ACCESS_KEY=${{ secrets.AWS_SECRET_ACCESS_KEY }}" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf-8 -Append # Unlike ci.yml, which holds its cloud resources back until the build is done, provisioning From 2be4050ed107848d892d32d02694fd929d34c387 Mon Sep 17 00:00:00 2001 From: John Simons Date: Thu, 10 Sep 2026 11:42:44 +1000 Subject: [PATCH 08/14] Fix resource name collisions when Aurora PostgreSQL and RDS SQL Server run concurrently Both AWS targets run at the same time using the same run name, so their security groups and RDS identifiers collide. Aurora PostgreSQL now appends `-aurora` to its resource names and SQL Server appends `-mssql`, making them distinct within the same run. Also fixes the Azure PostgreSQL database creation command, which requires `--name` rather than `--database-name` for the `flexible-server db create` subcommand. --- .../cloud-database/aurora-postgresql.mts | 29 ++++++++++++------- .../cloud-database/azure-postgresql.mts | 4 ++- .../actions/cloud-database/rds-sqlserver.mts | 28 ++++++++++++------ 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/.github/actions/cloud-database/aurora-postgresql.mts b/.github/actions/cloud-database/aurora-postgresql.mts index d1408c0fe0..bdbf0d0b7c 100644 --- a/.github/actions/cloud-database/aurora-postgresql.mts +++ b/.github/actions/cloud-database/aurora-postgresql.mts @@ -7,22 +7,29 @@ const databaseName = 'servicecontrol'; const adminUser = 'sctestadmin'; const port = 5432; +// Both AWS targets run at once with the same run name, so each needs its own prefix or they collide +// on the security group and on RDS identifiers. +function resources(name: string) { + return { base: `${name}-aurora`, instance: `${name}-aurora-1` }; +} + async function provision(name: string): Promise { + const { base, instance } = resources(name); + aws.removeStaleSecurityGroups(); const password = newAdminPassword(); const runnerIp = await runnerIpAddress(); - const { groupId, created } = aws.createSecurityGroup(name); + const { groupId, created } = aws.createSecurityGroup(base); aws.allowRunner(groupId, port, runnerIp); - const tags = aws.tags(name, created); - const instance = `${name}-1`; + const tags = aws.tags(base, created); // The engine version is left to the AWS default so that this does not break every time a pinned // minor version is retired. - step(`Creating Aurora PostgreSQL cluster ${name}`); + step(`Creating Aurora PostgreSQL cluster ${base}`); run('aws', ['rds', 'create-db-cluster', - '--db-cluster-identifier', name, + '--db-cluster-identifier', base, '--engine', 'aurora-postgresql', '--master-username', adminUser, '--master-user-password', password, @@ -39,7 +46,7 @@ async function provision(name: string): Promise { step(`Creating instance ${instance}`); run('aws', ['rds', 'create-db-instance', '--db-instance-identifier', instance, - '--db-cluster-identifier', name, + '--db-cluster-identifier', base, '--engine', 'aurora-postgresql', '--db-instance-class', 'db.r6g.large', '--publicly-accessible', @@ -49,7 +56,7 @@ async function provision(name: string): Promise { step('Waiting for the instance to become available'); run('aws', ['rds', 'wait', 'db-instance-available', '--db-instance-identifier', instance]); - const endpoint = capture('aws', ['rds', 'describe-db-clusters', '--db-cluster-identifier', name, '--query', 'DBClusters[0].Endpoint', '--output', 'text']); + const endpoint = capture('aws', ['rds', 'describe-db-clusters', '--db-cluster-identifier', base, '--query', 'DBClusters[0].Endpoint', '--output', 'text']); // Trust Server Certificate because RDS presents an Amazon CA that is not in the runner's trust // store. The connection is still encrypted; only the certificate chain goes unverified. @@ -62,19 +69,19 @@ async function provision(name: string): Promise { } function teardown(name: string): void { - const instance = `${name}-1`; + const { base, instance } = resources(name); // Without waiting for the deletions to finish: an RDS instance takes minutes to disappear, and // holding the job open for that costs more than letting a later run sweep up does. teardownStep(`Deleting instance ${instance}`, () => run('aws', ['rds', 'delete-db-instance', '--db-instance-identifier', instance, '--skip-final-snapshot', '--delete-automated-backups', '--no-cli-pager'])); - teardownStep(`Deleting cluster ${name}`, () => - run('aws', ['rds', 'delete-db-cluster', '--db-cluster-identifier', name, '--skip-final-snapshot', '--no-cli-pager'])); + teardownStep(`Deleting cluster ${base}`, () => + run('aws', ['rds', 'delete-db-cluster', '--db-cluster-identifier', base, '--skip-final-snapshot', '--no-cli-pager'])); // Will refuse while the instance still holds it, which is the normal case. removeStaleSecurityGroups // on a later run is what actually clears it. - teardownStep(`Deleting security group ${name}`, () => aws.deleteSecurityGroup(name)); + teardownStep(`Deleting security group ${base}`, () => aws.deleteSecurityGroup(base)); } export { provision, teardown }; diff --git a/.github/actions/cloud-database/azure-postgresql.mts b/.github/actions/cloud-database/azure-postgresql.mts index e72d3a77a9..e64c3c2afe 100644 --- a/.github/actions/cloud-database/azure-postgresql.mts +++ b/.github/actions/cloud-database/azure-postgresql.mts @@ -35,8 +35,10 @@ async function provision(name: string): Promise { '--only-show-errors', '--output', 'none']); step(`Creating database ${databaseName}`); + // --name, not --database-name: that is what this command asks for, whatever the server-level + // commands use. run('az', ['postgres', 'flexible-server', 'db', 'create', - '--database-name', databaseName, + '--name', databaseName, '--resource-group', azure.resourceGroup, '--server-name', name, '--only-show-errors', '--output', 'none']); diff --git a/.github/actions/cloud-database/rds-sqlserver.mts b/.github/actions/cloud-database/rds-sqlserver.mts index f6571389b6..19f9f28d98 100644 --- a/.github/actions/cloud-database/rds-sqlserver.mts +++ b/.github/actions/cloud-database/rds-sqlserver.mts @@ -15,20 +15,28 @@ const port = 1433; // out not to have it, in which case move to sqlserver-se. const engine = 'sqlserver-web'; +// Both AWS targets run at once with the same run name, so each needs its own prefix or they collide +// on the security group and on RDS identifiers. +function instanceName(name: string): string { + return `${name}-mssql`; +} + async function provision(name: string): Promise { + const instance = instanceName(name); + aws.removeStaleSecurityGroups(); const password = newAdminPassword(); const runnerIp = await runnerIpAddress(); - const { groupId, created } = aws.createSecurityGroup(name); + const { groupId, created } = aws.createSecurityGroup(instance); aws.allowRunner(groupId, port, runnerIp); // db.m5.large on gp3, because a t3.small would be throttled by both CPU credits and gp2 burst // balance part way through the run. No automated backups and no standby: the instance does not // outlive the job, and both slow provisioning down. - step(`Creating RDS SQL Server instance ${name}`); + step(`Creating RDS SQL Server instance ${instance}`); run('aws', ['rds', 'create-db-instance', - '--db-instance-identifier', name, + '--db-instance-identifier', instance, '--engine', engine, '--db-instance-class', 'db.m5.large', '--allocated-storage', '100', @@ -41,13 +49,13 @@ async function provision(name: string): Promise { '--publicly-accessible', '--no-multi-az', '--backup-retention-period', '0', - ...aws.tags(name, created), + ...aws.tags(instance, created), '--no-cli-pager']); step('Waiting for the instance to become available'); - run('aws', ['rds', 'wait', 'db-instance-available', '--db-instance-identifier', name]); + run('aws', ['rds', 'wait', 'db-instance-available', '--db-instance-identifier', instance]); - const endpoint = capture('aws', ['rds', 'describe-db-instances', '--db-instance-identifier', name, '--query', 'DBInstances[0].Endpoint.Address', '--output', 'text']); + const endpoint = capture('aws', ['rds', 'describe-db-instances', '--db-instance-identifier', instance, '--query', 'DBInstances[0].Endpoint.Address', '--output', 'text']); // Trust Server Certificate because RDS presents an Amazon CA that is not in the runner's trust // store. The connection is still encrypted; only the certificate chain goes unverified. @@ -62,12 +70,14 @@ async function provision(name: string): Promise { } function teardown(name: string): void { - teardownStep(`Deleting instance ${name}`, () => - run('aws', ['rds', 'delete-db-instance', '--db-instance-identifier', name, '--skip-final-snapshot', '--delete-automated-backups', '--no-cli-pager'])); + const instance = instanceName(name); + + teardownStep(`Deleting instance ${instance}`, () => + run('aws', ['rds', 'delete-db-instance', '--db-instance-identifier', instance, '--skip-final-snapshot', '--delete-automated-backups', '--no-cli-pager'])); // Will refuse while the instance still holds it, which is the normal case. removeStaleSecurityGroups // on a later run is what actually clears it. - teardownStep(`Deleting security group ${name}`, () => aws.deleteSecurityGroup(name)); + teardownStep(`Deleting security group ${instance}`, () => aws.deleteSecurityGroup(instance)); } export { provision, teardown }; From c1c91e17a3376b9523dab2f0745f513e62237568 Mon Sep 17 00:00:00 2001 From: John Simons Date: Thu, 10 Sep 2026 17:55:24 +1000 Subject: [PATCH 09/14] Switch Azure SQL tier from General Purpose to Business Critical MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit General Purpose stores its transaction log on remote storage, capping log throughput at roughly one-tenth of what Business Critical provides on local SSD. The test suites create, index, and drop a schema per test around 550 times, so log throughput is the binding constraint—not CPU. A run on General Purpose measured 9 to 21 times slower than a local container and timed out. Business Critical keeps data and log on local SSD and resolves the bottleneck. --- .github/actions/cloud-database/azure-sql.mts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/actions/cloud-database/azure-sql.mts b/.github/actions/cloud-database/azure-sql.mts index bb33e4ea34..b14a443326 100644 --- a/.github/actions/cloud-database/azure-sql.mts +++ b/.github/actions/cloud-database/azure-sql.mts @@ -31,15 +31,17 @@ async function provision(name: string): Promise { '--end-ip-address', runnerIp, '--only-show-errors', '--output', 'none']); - // General Purpose with 4 vCores: the suites are DDL heavy and run several assemblies at once, and - // a DTU tier that size would spend the run throttled. Local backup redundancy because nothing - // here outlives the job. + // Business Critical, not General Purpose. The suites create, index and drop a schema per test, + // roughly 550 times, so the limit they hit is transaction log throughput rather than CPU. That + // is capped an order of magnitude lower on General Purpose, whose log is remote, and a run there + // measured 9 to 21 times slower than a local container and timed out. Business Critical keeps + // data and log on local SSD. Local backup redundancy because nothing here outlives the job. step(`Creating database ${databaseName}`); run('az', ['sql', 'db', 'create', '--name', databaseName, '--resource-group', azure.resourceGroup, '--server', name, - '--service-objective', 'GP_Gen5_4', + '--service-objective', 'BC_Gen5_4', '--backup-storage-redundancy', 'Local', ...tags, '--only-show-errors', '--output', 'none']); From acc81d5959ae6a79cb672e207c8199f4c7430f72 Mon Sep 17 00:00:00 2001 From: John Simons Date: Thu, 10 Sep 2026 18:21:06 +1000 Subject: [PATCH 10/14] Upsize all cloud database instances and rename azure-sql target to azure-sqlserver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All four managed database targets are upgraded to larger instance classes (db.r6g.2xlarge for Aurora PostgreSQL, db.m5.2xlarge for RDS SQL Server, Standard_D8ds_v5 with 512 GB storage for Azure PostgreSQL, BC_Gen5_8 for Azure SQL Server) to avoid CPU and IO throttling mid-run. The test suites are IO bound, and premium SSD IOPS on Azure PostgreSQL scale with disk size, so the storage is intentionally oversized. The azure-sql target is renamed to azure-sqlserver throughout—action inputs, TypeScript types, the workflow dispatch options, and the provisioner file itself—to match the naming convention of the other SQL Server target (rds-sqlserver). --- .github/actions/cloud-database/action.yml | 2 +- .github/actions/cloud-database/aurora-postgresql.mts | 4 ++-- .github/actions/cloud-database/azure-postgresql.mts | 9 +++++---- .../{azure-sql.mts => azure-sqlserver.mts} | 2 +- .github/actions/cloud-database/common.mts | 2 +- .github/actions/cloud-database/index.mts | 2 +- .github/actions/cloud-database/rds-sqlserver.mts | 6 +++--- .github/workflows/cloud-database-tests.yml | 4 ++-- 8 files changed, 16 insertions(+), 15 deletions(-) rename .github/actions/cloud-database/{azure-sql.mts => azure-sqlserver.mts} (98%) diff --git a/.github/actions/cloud-database/action.yml b/.github/actions/cloud-database/action.yml index 33dc575957..d814b114ab 100644 --- a/.github/actions/cloud-database/action.yml +++ b/.github/actions/cloud-database/action.yml @@ -2,7 +2,7 @@ name: Cloud database provisioner description: Provisions a managed database for the run, and deletes it again when the job ends inputs: target: - description: Which managed database to provision (azure-sql, azure-postgresql, aurora-postgresql or rds-sqlserver) + description: Which managed database to provision (azure-sqlserver, azure-postgresql, aurora-postgresql or rds-sqlserver) required: true name: description: Name for the resources this run creates, which teardown uses to find them again diff --git a/.github/actions/cloud-database/aurora-postgresql.mts b/.github/actions/cloud-database/aurora-postgresql.mts index bdbf0d0b7c..5be074e9b0 100644 --- a/.github/actions/cloud-database/aurora-postgresql.mts +++ b/.github/actions/cloud-database/aurora-postgresql.mts @@ -41,14 +41,14 @@ async function provision(name: string): Promise { ...tags, '--no-cli-pager']); - // One instance and no replicas: the cluster does not outlive the job. db.r6g.large rather than a + // One instance and no replicas: the cluster does not outlive the job. db.r6g.2xlarge rather than a // burstable class, so the run is not throttled part way through. step(`Creating instance ${instance}`); run('aws', ['rds', 'create-db-instance', '--db-instance-identifier', instance, '--db-cluster-identifier', base, '--engine', 'aurora-postgresql', - '--db-instance-class', 'db.r6g.large', + '--db-instance-class', 'db.r6g.2xlarge', '--publicly-accessible', ...tags, '--no-cli-pager']); diff --git a/.github/actions/cloud-database/azure-postgresql.mts b/.github/actions/cloud-database/azure-postgresql.mts index e64c3c2afe..b6d63a62e9 100644 --- a/.github/actions/cloud-database/azure-postgresql.mts +++ b/.github/actions/cloud-database/azure-postgresql.mts @@ -13,8 +13,9 @@ async function provision(name: string): Promise { const location = azure.location(); const tags = azure.tags(name); - // General Purpose with 4 vCores rather than a burstable tier, so the run is not throttled part - // way through. High availability is left at its default of disabled rather than passed + // General Purpose with 8 vCores rather than a burstable tier, so the run is not throttled part + // way through. The disk is oversized on purpose: premium SSD IOPS scale with its size here, and + // the suites are IO bound rather than short of space. High availability is left at its default of disabled rather than passed // explicitly, because the CLI on the runner does not accept --high-availability. // --public-access opens the firewall to just this runner as part of creation. step(`Creating PostgreSQL flexible server ${name} in ${azure.resourceGroup} (${location}), allowing ${runnerIp}`); @@ -25,8 +26,8 @@ async function provision(name: string): Promise { '--admin-user', adminUser, '--admin-password', password, '--tier', 'GeneralPurpose', - '--sku-name', 'Standard_D4ds_v5', - '--storage-size', '128', + '--sku-name', 'Standard_D8ds_v5', + '--storage-size', '512', '--version', '16', '--geo-redundant-backup', 'Disabled', '--public-access', runnerIp, diff --git a/.github/actions/cloud-database/azure-sql.mts b/.github/actions/cloud-database/azure-sqlserver.mts similarity index 98% rename from .github/actions/cloud-database/azure-sql.mts rename to .github/actions/cloud-database/azure-sqlserver.mts index b14a443326..1bce6f200a 100644 --- a/.github/actions/cloud-database/azure-sql.mts +++ b/.github/actions/cloud-database/azure-sqlserver.mts @@ -41,7 +41,7 @@ async function provision(name: string): Promise { '--name', databaseName, '--resource-group', azure.resourceGroup, '--server', name, - '--service-objective', 'BC_Gen5_4', + '--service-objective', 'BC_Gen5_8', '--backup-storage-redundancy', 'Local', ...tags, '--only-show-errors', '--output', 'none']); diff --git a/.github/actions/cloud-database/common.mts b/.github/actions/cloud-database/common.mts index c6e69a8f9e..6b49587151 100644 --- a/.github/actions/cloud-database/common.mts +++ b/.github/actions/cloud-database/common.mts @@ -5,7 +5,7 @@ import { appendFileSync } from 'node:fs'; import { randomInt } from 'node:crypto'; export type Provider = 'SqlServer' | 'PostgreSql'; -export type Target = 'azure-sql' | 'azure-postgresql' | 'aurora-postgresql' | 'rds-sqlserver'; +export type Target = 'azure-sqlserver' | 'azure-postgresql' | 'aurora-postgresql' | 'rds-sqlserver'; function step(message: string): void { console.log(`==> ${message}`); diff --git a/.github/actions/cloud-database/index.mts b/.github/actions/cloud-database/index.mts index 71335f05e2..e171d02fbe 100644 --- a/.github/actions/cloud-database/index.mts +++ b/.github/actions/cloud-database/index.mts @@ -9,7 +9,7 @@ import { appendFileSync } from 'node:fs'; import type { Target } from './common.mts'; -const targets: Target[] = ['azure-sql', 'azure-postgresql', 'aurora-postgresql', 'rds-sqlserver']; +const targets: Target[] = ['azure-sqlserver', 'azure-postgresql', 'aurora-postgresql', 'rds-sqlserver']; function fail(message: string): never { console.log(`::error::${message}`); diff --git a/.github/actions/cloud-database/rds-sqlserver.mts b/.github/actions/cloud-database/rds-sqlserver.mts index 19f9f28d98..e8b5c0c8be 100644 --- a/.github/actions/cloud-database/rds-sqlserver.mts +++ b/.github/actions/cloud-database/rds-sqlserver.mts @@ -31,14 +31,14 @@ async function provision(name: string): Promise { const { groupId, created } = aws.createSecurityGroup(instance); aws.allowRunner(groupId, port, runnerIp); - // db.m5.large on gp3, because a t3.small would be throttled by both CPU credits and gp2 burst - // balance part way through the run. No automated backups and no standby: the instance does not + // db.m5.2xlarge on gp3, because a burstable class would run out of both CPU credits and gp2 + // burst balance part way through the run. No automated backups and no standby: the instance does not // outlive the job, and both slow provisioning down. step(`Creating RDS SQL Server instance ${instance}`); run('aws', ['rds', 'create-db-instance', '--db-instance-identifier', instance, '--engine', engine, - '--db-instance-class', 'db.m5.large', + '--db-instance-class', 'db.m5.2xlarge', '--allocated-storage', '100', '--storage-type', 'gp3', '--master-username', adminUser, diff --git a/.github/workflows/cloud-database-tests.yml b/.github/workflows/cloud-database-tests.yml index d9e239c732..9446358dbc 100644 --- a/.github/workflows/cloud-database-tests.yml +++ b/.github/workflows/cloud-database-tests.yml @@ -16,7 +16,7 @@ on: type: choice options: - all - - azure-sql + - azure-sqlserver - azure-postgresql - aurora-postgresql - rds-sqlserver @@ -45,7 +45,7 @@ jobs: id: select run: | $all = @( - @{ target = 'azure-sql'; provider = 'SqlServer' } + @{ target = 'azure-sqlserver'; provider = 'SqlServer' } @{ target = 'azure-postgresql'; provider = 'PostgreSql' } @{ target = 'aurora-postgresql'; provider = 'PostgreSql' } @{ target = 'rds-sqlserver'; provider = 'SqlServer' } From 0620391bcc9861fca129c83efefc585163640359 Mon Sep 17 00:00:00 2001 From: John Simons Date: Thu, 10 Sep 2026 18:53:00 +1000 Subject: [PATCH 11/14] Handle DependencyViolation when deleting AWS security groups RDS instances take minutes to finish deleting and hold their security group until then. Rather than failing, the deletion attempt now treats DependencyViolation as an expected transient condition and leaves the group for removeStaleSecurityGroups to clean up on a later run. Any other error is still re-thrown. --- .github/actions/cloud-database/aws.mts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/.github/actions/cloud-database/aws.mts b/.github/actions/cloud-database/aws.mts index 39aa9b3d13..2984422e71 100644 --- a/.github/actions/cloud-database/aws.mts +++ b/.github/actions/cloud-database/aws.mts @@ -98,9 +98,24 @@ function allowRunner(groupId: string, port: number, runnerIp: string): void { function deleteSecurityGroup(name: string): void { const groupId = capture('aws', ['ec2', 'describe-security-groups', '--filters', `Name=group-name,Values=${name}`, '--query', 'SecurityGroups[0].GroupId', '--output', 'text']); - if (groupId && groupId !== 'None') { - run('aws', ['ec2', 'delete-security-group', '--group-id', groupId, '--no-cli-pager']); + if (!groupId || groupId === 'None') { + return; } + + const failure = tryRun('aws', ['ec2', 'delete-security-group', '--group-id', groupId, '--no-cli-pager']); + + if (!failure) { + return; + } + + // The normal outcome, not a problem: the database that used the group takes minutes to finish + // deleting and holds it until then. removeStaleSecurityGroups clears it on a later run. + if (failure.includes('DependencyViolation')) { + step(`Security group ${name} is still held by a database that is deleting, leaving it for a later run to sweep up`); + return; + } + + throw new Error(failure); } // A security group cannot be deleted while an instance still holds it, and an RDS instance takes From 1be145b55c4ac1132a2ec76c3c738a08ee287ee5 Mon Sep 17 00:00:00 2001 From: John Simons Date: Fri, 11 Sep 2026 07:33:24 +1000 Subject: [PATCH 12/14] Clean-up --- .github/actions/cloud-database/action.yml | 3 --- .../cloud-database/aurora-postgresql.mts | 8 -------- .github/actions/cloud-database/aws.mts | 8 -------- .../actions/cloud-database/azure-postgresql.mts | 12 ------------ .../actions/cloud-database/azure-sqlserver.mts | 9 --------- .github/actions/cloud-database/azure.mts | 4 ---- .github/actions/cloud-database/common.mts | 10 +--------- .github/actions/cloud-database/index.mts | 11 ----------- .../actions/cloud-database/rds-sqlserver.mts | 17 ----------------- .../actions/cloud-database/verify-postgresql.cs | 4 ---- .../actions/cloud-database/verify-sqlserver.cs | 5 ----- 11 files changed, 1 insertion(+), 90 deletions(-) diff --git a/.github/actions/cloud-database/action.yml b/.github/actions/cloud-database/action.yml index d814b114ab..57375c4938 100644 --- a/.github/actions/cloud-database/action.yml +++ b/.github/actions/cloud-database/action.yml @@ -8,9 +8,6 @@ inputs: description: Name for the resources this run creates, which teardown uses to find them again required: true runs: - # A JavaScript action, not a composite one, because only this kind can declare a post step. - # Teardown is that post step, so it cannot be left out of a workflow or skipped by a failure - # earlier in the job. using: node24 main: index.mts post: index.mts diff --git a/.github/actions/cloud-database/aurora-postgresql.mts b/.github/actions/cloud-database/aurora-postgresql.mts index 5be074e9b0..177edb9849 100644 --- a/.github/actions/cloud-database/aurora-postgresql.mts +++ b/.github/actions/cloud-database/aurora-postgresql.mts @@ -7,8 +7,6 @@ const databaseName = 'servicecontrol'; const adminUser = 'sctestadmin'; const port = 5432; -// Both AWS targets run at once with the same run name, so each needs its own prefix or they collide -// on the security group and on RDS identifiers. function resources(name: string) { return { base: `${name}-aurora`, instance: `${name}-aurora-1` }; } @@ -41,8 +39,6 @@ async function provision(name: string): Promise { ...tags, '--no-cli-pager']); - // One instance and no replicas: the cluster does not outlive the job. db.r6g.2xlarge rather than a - // burstable class, so the run is not throttled part way through. step(`Creating instance ${instance}`); run('aws', ['rds', 'create-db-instance', '--db-instance-identifier', instance, @@ -71,16 +67,12 @@ async function provision(name: string): Promise { function teardown(name: string): void { const { base, instance } = resources(name); - // Without waiting for the deletions to finish: an RDS instance takes minutes to disappear, and - // holding the job open for that costs more than letting a later run sweep up does. teardownStep(`Deleting instance ${instance}`, () => run('aws', ['rds', 'delete-db-instance', '--db-instance-identifier', instance, '--skip-final-snapshot', '--delete-automated-backups', '--no-cli-pager'])); teardownStep(`Deleting cluster ${base}`, () => run('aws', ['rds', 'delete-db-cluster', '--db-cluster-identifier', base, '--skip-final-snapshot', '--no-cli-pager'])); - // Will refuse while the instance still holds it, which is the normal case. removeStaleSecurityGroups - // on a later run is what actually clears it. teardownStep(`Deleting security group ${base}`, () => aws.deleteSecurityGroup(base)); } diff --git a/.github/actions/cloud-database/aws.mts b/.github/actions/cloud-database/aws.mts index 2984422e71..8c803932c4 100644 --- a/.github/actions/cloud-database/aws.mts +++ b/.github/actions/cloud-database/aws.mts @@ -33,16 +33,10 @@ function defaultVpcId(): string { return created; } -// An EC2 security group does not record when it was created, so this tag is what the stale sweep -// judges age by. Epoch seconds because AWS restricts the characters a tag value may contain. function createdTimestamp(): string { return Math.floor(Date.now() / 1000).toString(); } -// RDS needs a subnet group spanning at least two availability zones. A region does not reliably -// have one called "default", even where a default VPC exists, so this owns one rather than betting -// on RDS creating it. Like the default VPC it is account infrastructure: created when missing, -// never deleted, and shared by concurrent runs. const subnetGroup = 'servicecontrol-cloud-tests'; function dbSubnetGroupName(): string { @@ -108,8 +102,6 @@ function deleteSecurityGroup(name: string): void { return; } - // The normal outcome, not a problem: the database that used the group takes minutes to finish - // deleting and holds it until then. removeStaleSecurityGroups clears it on a later run. if (failure.includes('DependencyViolation')) { step(`Security group ${name} is still held by a database that is deleting, leaving it for a later run to sweep up`); return; diff --git a/.github/actions/cloud-database/azure-postgresql.mts b/.github/actions/cloud-database/azure-postgresql.mts index b6d63a62e9..8869c2f983 100644 --- a/.github/actions/cloud-database/azure-postgresql.mts +++ b/.github/actions/cloud-database/azure-postgresql.mts @@ -1,6 +1,3 @@ -// Provisions and tears down an Azure Database for PostgreSQL flexible server for one run of the -// cloud database tests. - import { step, run, newAdminPassword, runnerIpAddress, setPersistenceConnectionString, verifyDatabase, teardownStep } from './common.mts'; import * as azure from './azure.mts'; @@ -13,11 +10,6 @@ async function provision(name: string): Promise { const location = azure.location(); const tags = azure.tags(name); - // General Purpose with 8 vCores rather than a burstable tier, so the run is not throttled part - // way through. The disk is oversized on purpose: premium SSD IOPS scale with its size here, and - // the suites are IO bound rather than short of space. High availability is left at its default of disabled rather than passed - // explicitly, because the CLI on the runner does not accept --high-availability. - // --public-access opens the firewall to just this runner as part of creation. step(`Creating PostgreSQL flexible server ${name} in ${azure.resourceGroup} (${location}), allowing ${runnerIp}`); run('az', ['postgres', 'flexible-server', 'create', '--name', name, @@ -36,8 +28,6 @@ async function provision(name: string): Promise { '--only-show-errors', '--output', 'none']); step(`Creating database ${databaseName}`); - // --name, not --database-name: that is what this command asks for, whatever the server-level - // commands use. run('az', ['postgres', 'flexible-server', 'db', 'create', '--name', databaseName, '--resource-group', azure.resourceGroup, @@ -53,8 +43,6 @@ async function provision(name: string): Promise { } function teardown(name: string): void { - // The resource group is shared and long lived, so only the server goes. Its databases and - // firewall rules are children and go with it. teardownStep(`Deleting PostgreSQL flexible server ${name}`, () => run('az', ['postgres', 'flexible-server', 'delete', '--name', name, '--resource-group', azure.resourceGroup, '--yes', '--only-show-errors'])); } diff --git a/.github/actions/cloud-database/azure-sqlserver.mts b/.github/actions/cloud-database/azure-sqlserver.mts index 1bce6f200a..be454cfd82 100644 --- a/.github/actions/cloud-database/azure-sqlserver.mts +++ b/.github/actions/cloud-database/azure-sqlserver.mts @@ -1,5 +1,3 @@ -// Provisions and tears down an Azure SQL Database for one run of the cloud database tests. - import { step, run, newAdminPassword, runnerIpAddress, setPersistenceConnectionString, verifyDatabase, teardownStep } from './common.mts'; import * as azure from './azure.mts'; @@ -31,11 +29,6 @@ async function provision(name: string): Promise { '--end-ip-address', runnerIp, '--only-show-errors', '--output', 'none']); - // Business Critical, not General Purpose. The suites create, index and drop a schema per test, - // roughly 550 times, so the limit they hit is transaction log throughput rather than CPU. That - // is capped an order of magnitude lower on General Purpose, whose log is remote, and a run there - // measured 9 to 21 times slower than a local container and timed out. Business Critical keeps - // data and log on local SSD. Local backup redundancy because nothing here outlives the job. step(`Creating database ${databaseName}`); run('az', ['sql', 'db', 'create', '--name', databaseName, @@ -55,8 +48,6 @@ async function provision(name: string): Promise { } function teardown(name: string): void { - // The resource group is shared and long lived, so only the server goes. Its databases and - // firewall rules are children and go with it. teardownStep(`Deleting SQL server ${name}`, () => run('az', ['sql', 'server', 'delete', '--name', name, '--resource-group', azure.resourceGroup, '--yes', '--only-show-errors'])); } diff --git a/.github/actions/cloud-database/azure.mts b/.github/actions/cloud-database/azure.mts index 77f5542769..9d6c76178d 100644 --- a/.github/actions/cloud-database/azure.mts +++ b/.github/actions/cloud-database/azure.mts @@ -4,16 +4,12 @@ import { capture } from './common.mts'; const resourceGroup = 'GitHubActions-RG'; -// Created, Package and RunnerOS are required on every resource in the subscription. RunId is ours, -// and is what tells one run's resources from another's. function tags(runId: string): string[] { const created = new Date().toISOString().slice(0, 10); return ['--tags', `Created=${created}`, 'Package=ServiceControl', 'RunnerOS=Linux', `RunId=${runId}`]; } -// The subscription only grants write access to one long lived resource group, so resources are -// created inside it and deleted individually rather than by dropping a group per run. function location(): string { const value = capture('az', ['group', 'show', '--name', resourceGroup, '--query', 'location', '--output', 'tsv']); diff --git a/.github/actions/cloud-database/common.mts b/.github/actions/cloud-database/common.mts index 6b49587151..20faa4b60c 100644 --- a/.github/actions/cloud-database/common.mts +++ b/.github/actions/cloud-database/common.mts @@ -37,8 +37,6 @@ function captureJson(command: string, args: string[]): any { return JSON.parse(capture(command, args)); } -// Satisfies the complexity rules of all four services at once, and avoids the characters that would -// have to be escaped in a connection string or are rejected outright by RDS (/ " @ and space). function newAdminPassword(): string { const upper = 'ABCDEFGHJKLMNPQRSTUVWXYZ'; const lower = 'abcdefghijkmnpqrstuvwxyz'; @@ -60,15 +58,11 @@ function newAdminPassword(): string { const password = characters.join(''); - // Before it can reach a log line. Everything the password is later embedded in, the connection - // string included, is redacted along with it. console.log(`::add-mask::${password}`); return password; } -// The servers are reachable from the internet, so each one is firewalled to the single address this -// job connects from. async function runnerIpAddress(): Promise { const response = await fetch('https://api.ipify.org', { signal: AbortSignal.timeout(30000) }); @@ -85,9 +79,7 @@ function setPersistenceConnectionString(provider: Provider, connectionString: st step(`Exported ${name}`); } -// Waits until the database is really reachable. The provisioning CLIs return before that is -// necessarily true, and the alternative to checking here is the test run discovering it far less -// clearly twenty minutes later. +// Waits until the database is really reachable. function verifyDatabase(provider: Provider, connectionString: string): void { const script = provider === 'SqlServer' ? './verify-sqlserver.cs' : './verify-postgresql.cs'; diff --git a/.github/actions/cloud-database/index.mts b/.github/actions/cloud-database/index.mts index e171d02fbe..87c2f8d71e 100644 --- a/.github/actions/cloud-database/index.mts +++ b/.github/actions/cloud-database/index.mts @@ -1,11 +1,3 @@ -// Provisions a managed database on the way in, and deletes it on the way out. GitHub runs this same -// file twice: once as the step itself, and once as the job's post step, which is what makes the -// teardown impossible to forget. -// -// No dependencies on purpose. @actions/core would have to be either committed as node_modules or -// bundled by a build step, and everything used here is a documented workflow command or environment -// variable that the toolkit itself is a wrapper over. - import { appendFileSync } from 'node:fs'; import type { Target } from './common.mts'; @@ -36,14 +28,11 @@ async function main() { return; } - // Recorded before provisioning rather than after, so that a run which fails half way through - // creating its resources still tears down the ones it did create. appendFileSync(process.env.GITHUB_STATE, `provisioning=${target}\n`); await database.provision(name); } main().catch(error => { - // The CLI output has already been streamed, so only the summary is worth adding. console.log(`::error::${(error as Error).message}`); process.exit(1); }); diff --git a/.github/actions/cloud-database/rds-sqlserver.mts b/.github/actions/cloud-database/rds-sqlserver.mts index e8b5c0c8be..6b5c75e816 100644 --- a/.github/actions/cloud-database/rds-sqlserver.mts +++ b/.github/actions/cloud-database/rds-sqlserver.mts @@ -1,8 +1,3 @@ -// Provisions and tears down an RDS SQL Server instance for one run of the cloud database tests. -// -// This is the slowest of the four targets to provision, around 15 to 25 minutes, so the workflow -// starts it before waiting on the build. - import { step, run, capture, newAdminPassword, runnerIpAddress, setPersistenceConnectionString, verifyDatabase, teardownStep } from './common.mts'; import * as aws from './aws.mts'; @@ -10,13 +5,8 @@ const databaseName = 'servicecontrol'; const adminUser = 'sctestadmin'; const port = 1433; -// Web edition is the cheapest licence-included option. Full-Text Search availability is what decides -// whether it is usable; verify-sqlserver.cs fails the run with a clear message if this edition turns -// out not to have it, in which case move to sqlserver-se. const engine = 'sqlserver-web'; -// Both AWS targets run at once with the same run name, so each needs its own prefix or they collide -// on the security group and on RDS identifiers. function instanceName(name: string): string { return `${name}-mssql`; } @@ -31,9 +21,6 @@ async function provision(name: string): Promise { const { groupId, created } = aws.createSecurityGroup(instance); aws.allowRunner(groupId, port, runnerIp); - // db.m5.2xlarge on gp3, because a burstable class would run out of both CPU credits and gp2 - // burst balance part way through the run. No automated backups and no standby: the instance does not - // outlive the job, and both slow provisioning down. step(`Creating RDS SQL Server instance ${instance}`); run('aws', ['rds', 'create-db-instance', '--db-instance-identifier', instance, @@ -61,8 +48,6 @@ async function provision(name: string): Promise { // store. The connection is still encrypted; only the certificate chain goes unverified. const connectionString = `Server=tcp:${endpoint},${port};Initial Catalog=${databaseName};User ID=${adminUser};Password=${password};Encrypt=True;TrustServerCertificate=True;Connect Timeout=60`; - // RDS cannot create a user database as part of the instance, unlike every other target here, so - // this both creates it and waits for the server to accept connections. step(`Creating database ${databaseName} and verifying Full-Text Search`); verifyDatabase('SqlServer', connectionString); @@ -75,8 +60,6 @@ function teardown(name: string): void { teardownStep(`Deleting instance ${instance}`, () => run('aws', ['rds', 'delete-db-instance', '--db-instance-identifier', instance, '--skip-final-snapshot', '--delete-automated-backups', '--no-cli-pager'])); - // Will refuse while the instance still holds it, which is the normal case. removeStaleSecurityGroups - // on a later run is what actually clears it. teardownStep(`Deleting security group ${instance}`, () => aws.deleteSecurityGroup(instance)); } diff --git a/.github/actions/cloud-database/verify-postgresql.cs b/.github/actions/cloud-database/verify-postgresql.cs index f8359a6975..72d55a4f28 100644 --- a/.github/actions/cloud-database/verify-postgresql.cs +++ b/.github/actions/cloud-database/verify-postgresql.cs @@ -1,10 +1,6 @@ #:package Npgsql@10.0.3 // Waits until the server actually accepts connections on the test database. -// -// Only the connection is retried. The provisioning CLIs report a server as available before it is -// necessarily reachable, so a refused connection is worth waiting out. Anything the server actually -// answers is a real result, and retrying it would just turn a clear failure into a ten minute one. using Npgsql; diff --git a/.github/actions/cloud-database/verify-sqlserver.cs b/.github/actions/cloud-database/verify-sqlserver.cs index b1879b1ed6..28957078a3 100644 --- a/.github/actions/cloud-database/verify-sqlserver.cs +++ b/.github/actions/cloud-database/verify-sqlserver.cs @@ -2,11 +2,6 @@ // Waits until the server accepts connections, creates the test database if the provisioning CLI // could not, and fails the run if the server has no Full-Text Search. -// -// Only the connection is retried. The provisioning CLIs report a server as ready before it is -// necessarily reachable, and an Azure firewall rule takes a moment to propagate, so a refused -// connection is worth waiting out. Everything after that is a real answer from the server, and -// retrying it would just turn a clear failure into a ten minute one. using Microsoft.Data.SqlClient; From 273253ae522deccdb6bd5179681ccf0e70d3c96a Mon Sep 17 00:00:00 2001 From: John Simons Date: Fri, 11 Sep 2026 08:05:16 +1000 Subject: [PATCH 13/14] Fix password handling to prevent special characters from being parsed as CLI arguments Passwords containing characters like `-` or `+` could be interpreted as CLI flags when passed as a separate argument. Switching to the `--flag=value` form prevents the CLI from misinterpreting the password value. Also removes `-` and `+` from the generated password's symbol set to avoid a leading `-` after shuffling, and ensures the first character is always a letter to satisfy provider constraints that require passwords to begin with an alphabetic character. --- .github/actions/cloud-database/aurora-postgresql.mts | 2 +- .github/actions/cloud-database/azure-postgresql.mts | 2 +- .github/actions/cloud-database/azure-sqlserver.mts | 2 +- .github/actions/cloud-database/common.mts | 7 ++++--- .github/actions/cloud-database/rds-sqlserver.mts | 2 +- 5 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.github/actions/cloud-database/aurora-postgresql.mts b/.github/actions/cloud-database/aurora-postgresql.mts index 177edb9849..faa243609b 100644 --- a/.github/actions/cloud-database/aurora-postgresql.mts +++ b/.github/actions/cloud-database/aurora-postgresql.mts @@ -30,7 +30,7 @@ async function provision(name: string): Promise { '--db-cluster-identifier', base, '--engine', 'aurora-postgresql', '--master-username', adminUser, - '--master-user-password', password, + `--master-user-password=${password}`, '--database-name', databaseName, '--vpc-security-group-ids', groupId, '--db-subnet-group-name', aws.dbSubnetGroupName(), diff --git a/.github/actions/cloud-database/azure-postgresql.mts b/.github/actions/cloud-database/azure-postgresql.mts index 8869c2f983..ff1cc7efde 100644 --- a/.github/actions/cloud-database/azure-postgresql.mts +++ b/.github/actions/cloud-database/azure-postgresql.mts @@ -16,7 +16,7 @@ async function provision(name: string): Promise { '--resource-group', azure.resourceGroup, '--location', location, '--admin-user', adminUser, - '--admin-password', password, + `--admin-password=${password}`, '--tier', 'GeneralPurpose', '--sku-name', 'Standard_D8ds_v5', '--storage-size', '512', diff --git a/.github/actions/cloud-database/azure-sqlserver.mts b/.github/actions/cloud-database/azure-sqlserver.mts index be454cfd82..09a4e39a37 100644 --- a/.github/actions/cloud-database/azure-sqlserver.mts +++ b/.github/actions/cloud-database/azure-sqlserver.mts @@ -16,7 +16,7 @@ async function provision(name: string): Promise { '--resource-group', azure.resourceGroup, '--location', location, '--admin-user', adminUser, - '--admin-password', password, + `--admin-password=${password}`, ...tags, '--only-show-errors', '--output', 'none']); diff --git a/.github/actions/cloud-database/common.mts b/.github/actions/cloud-database/common.mts index 20faa4b60c..72d5fb9980 100644 --- a/.github/actions/cloud-database/common.mts +++ b/.github/actions/cloud-database/common.mts @@ -41,7 +41,7 @@ function newAdminPassword(): string { const upper = 'ABCDEFGHJKLMNPQRSTUVWXYZ'; const lower = 'abcdefghijkmnpqrstuvwxyz'; const digit = '23456789'; - const symbol = '!#$%*()-_+'; + const symbol = '!#$%*()_+'; const all = upper + lower + digit + symbol; const pick = (set: string) => set[randomInt(set.length)]; @@ -56,6 +56,9 @@ function newAdminPassword(): string { [characters[i], characters[j]] = [characters[j], characters[i]]; } + const letter = characters.findIndex(c => /[A-Za-z]/.test(c)); + [characters[0], characters[letter]] = [characters[letter], characters[0]]; + const password = characters.join(''); console.log(`::add-mask::${password}`); @@ -83,8 +86,6 @@ function setPersistenceConnectionString(provider: Provider, connectionString: st function verifyDatabase(provider: Provider, connectionString: string): void { const script = provider === 'SqlServer' ? './verify-sqlserver.cs' : './verify-postgresql.cs'; - // From this folder, because `dotnet run .cs` picks up any project file in the working - // directory and the repo root holds a tests.proj by this point. execFileSync('dotnet', ['run', script, '--', connectionString], { cwd: import.meta.dirname, stdio: 'inherit' }); } diff --git a/.github/actions/cloud-database/rds-sqlserver.mts b/.github/actions/cloud-database/rds-sqlserver.mts index 6b5c75e816..c9a7672b68 100644 --- a/.github/actions/cloud-database/rds-sqlserver.mts +++ b/.github/actions/cloud-database/rds-sqlserver.mts @@ -29,7 +29,7 @@ async function provision(name: string): Promise { '--allocated-storage', '100', '--storage-type', 'gp3', '--master-username', adminUser, - '--master-user-password', password, + `--master-user-password=${password}`, '--vpc-security-group-ids', groupId, '--db-subnet-group-name', aws.dbSubnetGroupName(), '--license-model', 'license-included', From a5a1da11a6d193d16a3bc67ab603c2382dd5fc22 Mon Sep 17 00:00:00 2001 From: John Simons Date: Fri, 11 Sep 2026 08:56:26 +1000 Subject: [PATCH 14/14] Run cloud database tests as part of the release workflow Converts cloud-database-tests.yml to a reusable workflow (workflow_call) and calls it from release.yml alongside the existing jobs, so cloud database tests run automatically on every release rather than requiring a manual trigger or branch/tag push. --- .github/workflows/cloud-database-tests.yml | 8 +------- .github/workflows/release.yml | 3 +++ 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/.github/workflows/cloud-database-tests.yml b/.github/workflows/cloud-database-tests.yml index 9446358dbc..11b6344555 100644 --- a/.github/workflows/cloud-database-tests.yml +++ b/.github/workflows/cloud-database-tests.yml @@ -1,12 +1,6 @@ name: Cloud database tests on: - pull_request: - push: - tags: - - '[0-9]+.[0-9]+.[0-9]+' - - '[0-9]+.[0-9]+.[0-9]+-*' - branches: - - john/cloud_ci_tests + workflow_call: workflow_dispatch: inputs: target: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ac8149db33..e9f2fea97d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -15,4 +15,7 @@ jobs: secrets: inherit db-container: uses: ./.github/workflows/build-db-container.yml + secrets: inherit + cloud-tests: + uses: ./.github/workflows/cloud-database-tests.yml secrets: inherit \ No newline at end of file