[Jay W (JayDoubleu)]

Airlock validation created a review VM without Nexus and then failed to remove it after its extension failed. Shared-service validation exhausted its test budget before certificate cleanup. Separately, scheduled cleanup stopped a recently used environment because it measured inactivity from the branch commit alone.

The coverage review also found no way to select one bundle through the branch workflow. Custom tests could start before smoke tests had passed.

Changes

Area	Change
Environment protection	Check recent workflow activity, hold the branch deployment lock from preparation, and defer cleanup when activity or lock ownership is uncertain.
Test prerequisites	Forward explicit Nexus EULA consent, prepare or validate Nexus and certificate dependencies, and use supported Nexus and admin VM test sizes. Keep Azure Identity TLS verification enabled.
Cleanup	Give cleanup its own deadline. Track accepted resources, recover failed provisioning, remove children before parents and preserve reused resources. Handle expected plain-text API errors without weakening status checks.
Selection and reports	Map all 25 bundles to tests and coverage gaps. Add exact bundle selection, require successful smoke tests, and run custom groups sequentially with separate budgets. Publish JUnit and JSON evidence.
Certificate reuse	Compare the public certificate fingerprint and hostname across uninstall and reinstall. Require a matching reuse log, verify gateway shutdown, and confirm API, gateway, public IP and storage removal.
Lifecycle checks	Add base-workspace and private SQL cases, including a SQL SKU change and retained-data check. Verify Airlock draft deletion through ARM and a rejected write with the old SAS. Add safe polling diagnostics.

Validation

The merged code, workflows and tests match the final tested combined revision fed265a4340ffd167fd490450e4be6ef7fb52ed3.
Final differences are limited to CHANGELOG.md, devops/version.txt and one documentation sentence.

The E2E helper CI job passed all 255 tests at final PR head 9d68738.

Live Azure validation

The following runs passed through the upstream branch deployment workflow, with the runtime changes from #5253 and certificate changes from #5254.

Validation	Test revision	Result
Fresh deployment	ff7f4c3	Deployment and all 23 smoke tests passed.
Certificate lifecycle	f35e4ad	Install, uninstall and reinstall retained the same SHA-256 fingerprint. Reuse logs, gateway shutdown and service, gateway, public IP and challenge storage removal passed.
Shared services	f35e4ad	Four shared-service cases and the certificate/Nexus case passed. API deletion and firewall restoration completed.
Targeted Airlock	1cec703	Both cases passed: independent draft-deletion checks and import review VM provisioning, approval and cleanup.
Airlock API and storage	1cec703	All 12 cases passed. Workspace deletion and test teardown completed.
Extended workspace and Guacamole	1cec703	All three cases passed, including Windows VM provisioning and cleanup.
AAD workspace	1cec703	The Guacamole-in-AAD-workspace case and workspace cleanup passed.
Base workspace	1cec703	Creation, identity-scoped API access, disable and deletion passed.
Private SQL and Windows VM	1cec703	Private queries, S1-to-S2 configuration change, retained data and cleanup passed.
Linux VM	fed265a	Provisioning, disable and cleanup passed. The TRE API reported all five test-created resources deleted.
Repeat deployment	fed265a	Deployment into the existing environment and all 23 smoke tests passed. Custom E2E groups were intentionally skipped.

The revision column identifies the checked-out test code. Certificate and shared-service runs reused core/runtime deployment ff7f4c3. Linux tests at fed265a reused runtime 1cec703.
Repeat deployment then exercised the final combined revision. Cleanup claims reflect each test's checks, not an inventory of every Azure resource.

Earlier local checks and Azure evidence

Local checks at f23259d86eef: 238 E2E helper tests passed, including 33 certificate tests. Applicable pre-commit checks and the strict documentation build passed. Azure calls were mocked; certificate parsing used real local certificates.

Earlier checks at e461a1e4d6a1: 273 workflow/script tests and 249 deployment-script tests passed.

Earlier Azure evidence	Test revision	Result
Shared services	4f431ff8	Four independent cases and the certificate/Nexus case passed, including cleanup.
Airlock API/storage and targeted Airlock	c486c015	Twelve API/storage cases and two targeted cases passed in separate runs. Review VM and workspace cleanup were verified.
Private SQL lifecycle	f957b6bc	Private queries, the S1-to-S2 change, retained data and cleanup passed.
Base workspace	89f29848	Creation, identity access, disable, deletion and API removal passed against deployment f957b6bc.
Extended test ordering	ae9314e5	Three extended cases passed after all 23 smoke tests completed.

These earlier runs retain their original revisions and coverage limits.

The certificate-only case requires bundle version 0.9.0 or later from #5254. It checks public certificate metadata through ARM and reads aggregate Log Analytics evidence with the existing deployment identity. It does not read private keys or change vault access. Actual renewal and direct inspection of vault soft-deletion remain unproven.

The successful reuse proof used e2eBundle=tre-shared-service-certs on the combined upstream branch. Ordinary /test and /test-shared-services commands do not select this case.

These runs exercised the branch workflow. PR-comment commands use workflows from main, so their updated routing still needs a separate post-merge check. Live cleanup coordination remains in #5231. The SQL polling HTTP 500 still fails without retry; its cause remains open in #5202. Sealed-content integrity remains separate in #5181.

Fixes #5125.

Fixes #5131.

Fixes #5132.

Fixes #5173.

Fixes #5174.

Fixes #5175.

Fixes #5182.

Fixes #5183.

Fixes #5186.

Fixes #5201.

Fixes #5225.

Fixes #5237.

Combines the validation changes from #5256, #5257 and #5258 with this PR. Identity TLS verification and CI cleanup protection are included here. Earlier PR descriptions retain the original reviews and test evidence. Scope: #5119.

[github-actions]

Unit Test Results

0 tests   0 ✅  0s ⏱️
0 suites  0 💤
0 files    0 ❌

Results for commit 9d68738.

♻️ This comment has been updated with latest results.

[github-actions]

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/38585311329

(in response to this comment from Jay W (@JayDoubleu))

[Jay W (JayDoubleu)]

Fixed the certificate lifecycle test's template response handling in 7d63e09.

• Original failure: run 38755824404, job 116495438575 stopped with KeyError: 'template' before provisioning. The API returns the template fields directly, but the test expected a template wrapper.
• Fix: Read the flat response and correct the unit-test fixture to match the API. All 33 focused certificate helper tests and applicable pre-commit checks passed.
• Live confirmation: run 38758184044, job 116503447099, using the same fix on the combined branch at 2a4ef068, passed the template lookup and started certificate provisioning (202 Accepted). This confirms that the response-handling failure is fixed.

The job is still running. Certificate issuance, reuse after reinstall and final cleanup are not yet confirmed.

[Copilot]

🔵 Needs a closer look

Deployment locking and destructive cleanup still need current-head Azure coordination evidence and human review.

0 open findings

Files not reviewed (1)

• .github/scripts/package-lock.json: Generated file

Previously missed (2)

In code that hasn't changed since last review

Scope certificate evidence to the intended installation message span

e2e_tests/​resources/​certificate.py:71

Filtering only by OperationId can mix certificate installations. The resource processor keeps a parent span open around its message loop (resource_processor/vmss_porter/runner.py:471–475), so successive message spans share a trace. Issuance during the first installation can therefore make a correct reinstall fail issued == 0; an unrelated resource's reuse log can also contribute to the reuse count. Scope the evidence to the intended install operation and its message span, using both OperationId and ParentId. Add a regression with two installations sharing one trace.

Prevent Windows VM test from leaking newly created VMs

e2e_tests/​run_bundle.py:202

Selecting the Windows VM case with both TEST_WORKSPACE_ID and TEST_WORKSPACE_SERVICE_ID configured can pass while leaving a newly created VM running. The case discards the VM ID, and both fixture finalizers preserve the reused parents, so cascading deletion never runs. Reject this configuration before executing the selection, or explicitly track and delete the new VM while preserving its parents.

🧠 Review effort: Balanced

[Jay W (JayDoubleu)]

Two test fixes are now included:

Failure	Fix	Current evidence
Certificate validation raised KeyError: publicCertData after installation in job 116503447099.	Commit 4f99399 reads the referenced public certificate through the resource processor when the gateway omits it. It also correlates reuse logs with the installation message.	Job 116550564377 recorded Certificate install: hostname=... SHA-256=... at 17:11:08 UTC. The first installation passed the metadata, hostname, validity and fingerprint checks.
The smoke health request exceeded HTTPX's five-second timeout in job 116542255620. The server returned HTTP 200 after 15.399 seconds.	Commit 2ce0cdf sets a 30-second request timeout and preserves all service-health assertions.	Smoke job 116549286947 passed all 23 tests. A local delayed-response check verified the longer timeout and rejection of unhealthy responses.

The combined validation branch contains both fixes at f35e4ad4fd86d242db3d835cb4ad27b2827d0840. These are combined-branch results. The selected certificate lifecycle case has now passed, with no failures or skips. Reinstallation retained the same certificate fingerprint. Correlated logs confirmed reuse without entering the issuance path. Both cleanup phases removed the API resource, gateway, public IP and challenge storage. Both temporary probe commands were removed. The case took 56 minutes 7 seconds. Renewal, upgrades and Nexus/Airlock validation remain outside this result.

Local validation: all 255 E2E helper tests, applicable pre-commit checks and the strict documentation build passed for the certificate evidence change. The health timeout change passed its focused behavioural and lint checks.

[Copilot]

🔵 Needs a closer look

Destructive cleanup coordination and workflow-lock delegation still require live validation and final human review.

0 open findings

Files not reviewed (1)

• .github/scripts/package-lock.json: Generated file

Previously missed (1)

In code that hasn't changed since last review

Review-VM lifecycle test does not verify submitted file integrity

docs/​tre-developers/​end-to-end-tests.md:41

The review-VM lifecycle test provisions the VM, approves the request and checks deletion, but does not read or compare submitted file contents. This sentence overstates the integrity coverage, which remains separate in #5181. Describe submitted-content integrity as unverified rather than covered by that test.

🧠 Review effort: Balanced

[Copilot]

🔵 Needs a closer look

Cross-workflow locking and destructive cleanup still need current-candidate coordination evidence and final human review.

0 open findings

Files not reviewed (1)

• .github/scripts/package-lock.json: Generated file

Previously missed (2)

In code that hasn't changed since last review

Mark submitted-byte integrity validation as outstanding

docs/​tre-developers/​end-to-end-tests.md:41

The review-VM case uploads a sample before submission, then checks VM provisioning, approval and automatic deletion; it never reads back or compares the submitted bytes (e2e_tests/test_airlock.py:343–425). This sentence therefore overstates content-integrity coverage. Describe that validation as outstanding, consistent with #5181.

Use a stable certificate name to enable certificate reuse

e2e_tests/​resources/​nexus.py:87

When no certificate service remains after cleanup, this generates a new certificate name for the same nexus hostname. Certificate reuse in #5254 looks up the supplied cert_name, so subsequent Airlock or Linux VM runs cannot recover the previous run's retained certificate and may hit issuance limits again. Use a stable name for this environment and hostname, while preserving existing-resource and dependency checks.

🧠 Review effort: Balanced

[Jay W (JayDoubleu)]

/test-force-approve

Waiving another deployment for 9d68738. All code, workflows and tests match the final tested combined branch (fed265a). Differences are limited to the changelog, DevOps version and a documentation correction.

The following validation passed through the upstream branch deployment workflow:

Validation	Passed evidence
Certificates and shared services	Same fingerprint after reinstall, Nexus and service cleanup
Airlock	2 targeted cases, 12 API and storage cases
Extended validation	3 workspace and Guacamole cases, AAD validation
Base workspace	Creation, identity access and deletion
Private SQL	Queries, SKU change, retained data and cleanup
Linux VM	Provisioning, disable and cleanup
Repeat deployment	Deployment into the existing environment and all 23 smoke tests

[github-actions]

🤖 pr-bot 🤖

✅ Marking tests as complete (for commit 9d68738)

(in response to this comment from Jay W (@JayDoubleu))