Defer CI cleanup after branch validation

.github/linters/actionlint.yml

@@ -4,6 +4,9 @@
 # Remove these exact parser exceptions when actionlint supports queue.
 # ci_environment_workflow.test.js validates all reference group settings.
 paths:
+  .github/workflows/deploy_tre_branch.yml:
+    ignore:
+      - '^unexpected key "queue" for "concurrency" section\. expected one of "cancel-in-progress", "group"$'
   .github/workflows/clean_validation_envs.yml:
     ignore:
       - '^unexpected key "queue" for "concurrency" section\. expected one of "cancel-in-progress", "group"$'

.github/scripts/ci_environment_workflow.test.js

@@ -3,30 +3,53 @@ const os = require('os');
 const path = require('path');
 const { execFileSync } = require('child_process');
 const yaml = require('js-yaml');
+const vm = require('vm');
 
 const root = path.resolve(__dirname, '../..');
 const workflow = name => yaml.load(fs.readFileSync(path.join(root, '.github/workflows', name), 'utf8'));
 
 describe('reference concurrency queues', () => {
   test.each([
     ['deploy_tre.yml', 'run-deploy-tre-main', '${{ github.ref }}'],
-    ['deploy_tre_branch.yml', 'run-deploy-tre-not-main', '${{ github.ref }}'],
     ['pr_comment_bot.yml', 'run_test', '${{ needs.pr_comment.outputs.ciGitRef }}']
   ])('%s routes deployment into the shared queue without an outer queue', (file, job, ref) => {
     const caller = workflow(file);
     expect(caller.concurrency).toBeUndefined();
     expect(caller.jobs[job].concurrency).toBeUndefined();
     expect(caller.jobs[job].uses).toBe('./.github/workflows/deploy_tre_reusable.yml');
     expect(caller.jobs[job].with.ciGitRef).toBe(ref);
+    expect(caller.jobs[job].with.callerOwnsDeploymentLock).toBeUndefined();
     // Preparation may run independently, but must not add another waiting group.
     const needs = caller.jobs[job].needs || [];
     for (const dependency of Array.isArray(needs) ? needs : [needs]) {
       expect(caller.jobs[dependency].concurrency).toBeUndefined();
     }
   });
 
+  test('branch validation owns the reference queue before preparation', () => {
+    const caller = workflow('deploy_tre_branch.yml');
+    expect(caller.concurrency).toEqual({ group: 'deploy-${{ github.ref }}', 'cancel-in-progress': false, queue: 'max' });
+    const job = caller.jobs['run-deploy-tre-not-main'];
+    expect(job.uses).toBe('./.github/workflows/deploy_tre_reusable.yml');
+    expect(job.with.ciGitRef).toBe('${{ github.ref }}');
+    expect(job.with.callerOwnsDeploymentLock).toBe(true);
+    expect(Object.values(caller.jobs).every(value => value.concurrency === undefined)).toBe(true);
+  });
+
+  test('delegation avoids a nested reference lock and preserves the default queue', () => {
+    const reusable = workflow('deploy_tre_reusable.yml');
+    expect(reusable.on.workflow_call.inputs.callerOwnsDeploymentLock).toMatchObject({
+      type: 'boolean', required: false, default: false
+    });
+    expect(reusable.concurrency).toEqual({
+      group: "${{ inputs.callerOwnsDeploymentLock && format('deploy-delegated-{0}-{1}', github.run_id, github.run_attempt) " +
+        "|| format('deploy-{0}', inputs.ciGitRef) }}",
+      'cancel-in-progress': false,
+      queue: 'max'
+    });
+  });
+
   test.each([
-    ['deploy_tre_reusable.yml', null, 'deploy-${{ inputs.ciGitRef }}'],
     ['pr_comment_bot.yml', 'destroy_pr_env', 'deploy-${{ needs.pr_comment.outputs.ciGitRef }}'],
     ['pr_comment_bot.yml', 'destroy_branch_env', 'deploy-${{ needs.pr_comment.outputs.branchCiGitRef }}'],
     ['clean_validation_envs.yml', 'clean', 'deploy-${{ matrix.ref }}'],
@@ -53,6 +76,62 @@ describe('reference concurrency queues', () => {
   });
 });
 
+function e2eCondition(job, lockResult, skipDeployment) {
+  const needs = Object.fromEntries(job.needs.map(name => [name, {
+    result: name === 'verify_deployment_lock' ? lockResult
+      : name.startsWith('e2e_tests_') ? 'success' : (skipDeployment ? 'skipped' : 'success')
+  }]));
+  const inputs = { e2eBundle: '', e2eTestsCustomSelector: 'airlock', skipDeployment };
+  const results = Object.values(needs).map(value => value.result);
+  const expression = job.if.trim().slice(3, -2).replaceAll('needs.*.result', 'results');
+  return vm.runInNewContext(expression, {
+    needs, inputs, results, always: () => true, contains: (values, value) => values.includes(value)
+  }, { timeout: 100 });
+}
+
+describe('delegated deployment guard', () => {
+  test('the guard reads workflow source and has no Azure credentials', () => {
+    const job = workflow('deploy_tre_reusable.yml').jobs.verify_deployment_lock;
+    expect(job.permissions).toEqual({ actions: 'read', contents: 'read' });
+    expect(job.steps[0].if).toBe('inputs.callerOwnsDeploymentLock');
+    expect(job.steps[0].with).toEqual({ 'persist-credentials': false, ref: '${{ github.sha }}' });
+    const check = job.steps.find(step => step.run?.includes('verify_deployment_lock.py'));
+    expect(check.if).toBe('inputs.callerOwnsDeploymentLock');
+    expect(check.env).toEqual({
+      CI_DEPLOYMENT_LOCK_DELEGATED: '${{ inputs.callerOwnsDeploymentLock }}',
+      CI_GIT_REF: '${{ inputs.ciGitRef }}',
+      GITHUB_TOKEN: '${{ github.token }}'
+    });
+    const fallback = job.steps.find(step => step.if === '${{ !inputs.callerOwnsDeploymentLock }}');
+    expect(fallback.run).toBe('echo "The reusable workflow holds the deployment reference lock."');
+  });
+
+  test('every Azure job depends on the guard', () => {
+    const jobs = workflow('deploy_tre_reusable.yml').jobs;
+    expect(Object.keys(jobs).filter(name => !jobs[name].needs)).toEqual(['verify_deployment_lock']);
+    expect(jobs.deploy_management.needs).toEqual(['verify_deployment_lock']);
+    for (const name of ['e2e_tests_smoke', 'e2e_tests_custom']) {
+      expect(jobs[name].needs).toContain('verify_deployment_lock');
+    }
+  });
+
+  test.each(['success', 'failure', 'cancelled', 'skipped'])('E2E requires guard success when its result is %s', result => {
+    const jobs = workflow('deploy_tre_reusable.yml').jobs;
+    for (const name of ['e2e_tests_smoke', 'e2e_tests_custom']) {
+      for (const skipDeployment of [true, false]) {
+        expect(e2eCondition(jobs[name], result, skipDeployment)).toBe(result === 'success');
+      }
+    }
+  });
+
+  test('guard changes select the DevOps tests', () => {
+    const filter = workflow('build_validation_develop.yml').jobs;
+    const steps = Object.values(filter).flatMap(job => job.steps || []);
+    const paths = yaml.load(steps.find(step => step.id === 'filter').with.filters);
+    expect(paths.ci_cleanup).toContain('devops/scripts/verify_deployment_lock.py');
+  });
+});
+
 function prepare(job, location, ref, output) {
   const step = job.steps.find(value => value.id === 'environment-id' || value.id === 'run-id');
   execFileSync('bash', ['-c', step.run], {

.github/scripts/package.json

@@ -1,6 +1,6 @@
 {
   "name": "build-scripts",
-  "version": "1.0.3",
+  "version": "1.0.4",
   "description": "Scripts used in the GitHub workflows",
   "main": "build.js",
   "license": "MIT",

.github/workflows/build_validation_develop.yml

@@ -74,6 +74,8 @@ jobs:
               - 'devops/scripts/recover_bootstrap_lease.py'
               - 'devops/scripts/ci_environment_id.py'
               - 'devops/scripts/ci_cleanup_scope.py'
+              - 'devops/scripts/ci_cleanup_activity.py'
+              - 'devops/scripts/verify_deployment_lock.py'
               - 'devops/scripts/destroy_ci_ref_env.py'
               - 'devops/scripts/destroy_env_no_terraform.sh'
               - 'devops/scripts/validate_ci_cleanup.py'

.github/workflows/deploy_tre_branch.yml

@@ -35,8 +35,13 @@ on: # yamllint disable-line rule:truthy
         default: false
         required: false
 
-# The reusable deployment workflow holds the shared reference queue.
-# Separate caller queues can discard deployments before they reach that group.
+# Register branch validation before Preparation can start. The reusable workflow
+# delegates this reference lock to the caller. Older workflow reruns keep their
+# original locking behaviour; dispatch this workflow to use the early lock.
+concurrency:
+  group: deploy-${{ github.ref }}
+  cancel-in-progress: false
+  queue: max
 
 permissions:
   contents: read
@@ -90,6 +95,7 @@ jobs:
       actions: read # Required by the reusable management job
     with:
       ciGitRef: ${{ github.ref }}
+      callerOwnsDeploymentLock: true
       ciEnvironmentId: ${{ needs.prepare-not-main.outputs.refid }}
       prHeadSha: ${{ github.sha }}
       e2eTestsCustomSelector: ${{ github.event.inputs.e2eTestsCustomSelector }}

.github/workflows/deploy_tre_reusable.yml

@@ -18,6 +18,11 @@ on: # yamllint disable-line rule:truthy
         description: The git ref to use in annotations to associate a deployment with the code that triggered it
         type: string
         required: true
+      callerOwnsDeploymentLock:
+        description: The branch caller holds the deployment reference lock for its entire workflow
+        type: boolean
+        required: false
+        default: false
       ciEnvironmentId:
         description: Isolated CI environment ID derived from the Git reference, Azure cloud and location
         type: string
@@ -128,19 +133,47 @@ on: # yamllint disable-line rule:truthy
         description: ""
         required: false
 
-# This will prevent multiple runs of this entire workflow.
-# We should NOT cancel in progress runs as that can destabilize the environment.
+# Other callers retain the reference lock here. A branch caller already holds
+# it, so use a run-specific inner group and verify the outer owner before work.
 concurrency:
-  group: deploy-${{ inputs.ciGitRef }}
+  group: >-
+    ${{ inputs.callerOwnsDeploymentLock && format('deploy-delegated-{0}-{1}', github.run_id, github.run_attempt)
+    || format('deploy-{0}', inputs.ciGitRef) }}
   cancel-in-progress: false
   queue: max
 
 env:
   CI_ENVIRONMENT_ID: ${{ inputs.ciEnvironmentId }}
 
 jobs:
+  verify_deployment_lock:
+    name: Verify deployment lock
+    runs-on: ubuntu-latest
+    timeout-minutes: 5
+    permissions:
+      actions: read
+      contents: read
+    steps:
+      - name: Checkout workflow source
+        if: inputs.callerOwnsDeploymentLock
+        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
+        with:
+          persist-credentials: false
+          ref: ${{ github.sha }}
+      - name: Verify the delegated reference lock
+        if: inputs.callerOwnsDeploymentLock
+        env:
+          CI_DEPLOYMENT_LOCK_DELEGATED: ${{ inputs.callerOwnsDeploymentLock }}
+          CI_GIT_REF: ${{ inputs.ciGitRef }}
+          GITHUB_TOKEN: ${{ github.token }}
+        run: python3 devops/scripts/verify_deployment_lock.py
+      - name: Use the reusable workflow reference lock
+        if: ${{ !inputs.callerOwnsDeploymentLock }}
+        run: echo "The reusable workflow holds the deployment reference lock."
+
   deploy_management:
     name: Deploy Management
+    needs: [verify_deployment_lock]
     if: ${{ !inputs.skipDeployment }}
     runs-on: ubuntu-latest
     permissions:
@@ -861,12 +894,13 @@ jobs:
     # always() lets this job still evaluate its guard and run against the existing PR environment.
     if: >-
       ${{ always() &&
+      needs.verify_deployment_lock.result == 'success' &&
       (inputs.skipDeployment || !contains(needs.*.result, 'skipped')) &&
       !contains(needs.*.result, 'failure') &&
       !contains(needs.*.result, 'cancelled') }}
     runs-on: ubuntu-latest
     environment: ${{ inputs.environmentName }}
-    needs: [deploy_shared_services, register_bundles, deploy_ui]
+    needs: [verify_deployment_lock, deploy_shared_services, register_bundles, deploy_ui]
     timeout-minutes: 10
     steps:
       - name: Checkout
@@ -911,13 +945,14 @@ jobs:
     # Same as smoke tests: skipDeployment intentionally bypasses deploy jobs but still runs E2E tests.
     if: >-
       ${{ always() && inputs.e2eTestsCustomSelector != '' &&
+      needs.verify_deployment_lock.result == 'success' &&
       (inputs.skipDeployment || !contains(needs.*.result, 'skipped')) &&
       !contains(needs.*.result, 'failure') &&
       !contains(needs.*.result, 'cancelled') }}
     runs-on: ubuntu-latest
     environment: ${{ inputs.environmentName }}
     needs:
-      [deploy_shared_services, register_bundles, register_user_resource_bundles]
+      [verify_deployment_lock, deploy_shared_services, register_bundles, register_user_resource_bundles]
     timeout-minutes: 300
     steps:
       - name: Checkout

CHANGELOG.md

@@ -27,6 +27,7 @@ See [Legacy Airlock & migration](docs/azure-tre-overview/airlock.md#legacy-airlo
 * Add weekly and manual builds for Dockerfiles and Porter bundles, with selected pull request builds. Retry a failed target once and report final results. ([#5088](https://github.com/microsoft/AzureTRE/issues/5088))
 
 BUG FIXES:
+* Defer CI environment cleanup for queued operations and recent native-branch validation activity. Lock new branch validation runs before preparation and verify delegated deployment ownership. Preserve scoped ownership checks and orphan deletion. (`devops` 0.6.14, `build-scripts` 1.0.4) ([#5231](https://github.com/microsoft/AzureTRE/issues/5231))
 * Fix denial-of-service dependency alerts by updating `source-map-js` to 1.2.2 in the UI and removing transitive `sprintf-js` from workflow tests through an `argparse` 2 override for `js-yaml` 3. (`ui` 0.10.3, `build-scripts` 1.0.3) ([#5118](https://github.com/microsoft/AzureTRE/pull/5118))
 * Align the `create_aad_groups` schema and Porter defaults to `true` across all three workspace bundles, preventing an unrelated edit from unintentionally removing workspace role groups and app-role assignments.
   Add regression checks for each bundle. (`tre-workspace-base` 2.12.2, `tre-workspace-unrestricted` 0.15.1, `tre-workspace-airlock-import-review` 0.18.1) ([#5050](https://github.com/microsoft/AzureTRE/issues/5050))