Skip to content

ci: enable Copilot cloud agent on 1ES - #791

Open
Brian Telfer (Britel) wants to merge 16 commits into
mainfrom
user/britel/cloud-agent-runner-smoke
Open

ci: enable Copilot cloud agent on 1ES#791
Brian Telfer (Britel) wants to merge 16 commits into
mainfrom
user/britel/cloud-agent-runner-smoke

Conversation

@Britel

@Britel Brian Telfer (Britel) commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

This gives GitHub Copilot coding agents a reproducible Trident environment where they can make changes, build the project, and run unit tests before opening a PR.

Summary

  • Configure GitHub Copilot coding agent to use Trident's ephemeral 1ES runner scale set
  • Install Trident's documented native dependencies, Go 1.25, Rust 1.93, protobuf 33.2, and pinned Go protobuf generators before the agent starts

Security boundaries

  • Privileged setup never runs for pull_request events
  • GitHub Actions permissions are limited to contents: read

Validation

  • Copilot setup: https://github.com/microsoft/trident/actions/runs/34640839559
    • passed on commit 97f3fc7; the setup workflow and script are unchanged in the current head
    • installed rustc 1.93.0, cargo 1.93.0, go1.25.5, libprotoc 33.2, protoc-gen-go v1.36.11, and protoc-gen-go-grpc 1.6.2
    • verified /mnt/storage and linked the repository's target/ path to it
  • Pre-removal infrastructure validation: https://github.com/microsoft/trident/actions/runs/34640841874
    • the runner user created and destroyed a transient KVM domain through qemu:///system
    • make build completed and produced the expected target/release artifacts
    • make test passed with 1,057 tests, 0 failures, and 3 ignored tests
    • the temporary smoke workflow and script used for this proof were removed from the PR in d73e08a

Scope

This is the minimum cloud-agent development path. More will follow.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Configure the proven 1ES runner for Copilot setup, install Trident's documented toolchain, and validate the unit suite on the same ephemeral pool.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
@Britel
Brian Telfer (Britel) requested a review from a team as a code owner September 11, 2026 18:03
Copilot AI lite review requested due to automatic review settings September 11, 2026 18:03
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A critical workflow security issue and two moderate validation issues remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Enables Copilot cloud agents on ephemeral 1ES runners with required toolchains, Cargo data-disk storage, and KVM/libvirt validation.

Changes:

  • Installs native dependencies, Go, Rust, and protobuf.
  • Configures Cargo output on the runner data disk.
  • Adds Copilot setup, runner smoke tests, and unit-test validation workflows.
File summaries
File Description Final review finding
scripts/cloud-agent/setup.sh Provisions dependencies and configures Cargo storage. Moderate (1 vote): Verify the expected data-disk mount before creating the Cargo target directory.
scripts/cloud-agent/runner-smoke.sh Validates KVM, QEMU, and libvirt. Moderate (1 vote): Handle nonzero virt-host-validate status instead of reporting PASS.
.github/workflows/copilot-setup-steps.yml Runs Copilot setup on 1ES runners. Critical (3 votes): Do not execute privileged setup from untrusted pull requests.
.github/workflows/cloud-agent-runner-smoke.yml Runs manual runner and unit-test validation. No findings.
.gitattributes Defines line-ending rules. No findings.
Review details

Suppressed comments (2)

scripts/cloud-agent/runner-smoke.sh:67

  • The host validator's exit status is only recorded, so a nonzero virt-host-validate result is ignored and the script still reaches result.txt with PASS. That allows the manual validation workflow to succeed while libvirt reports a host failure. Either fail on a nonzero status or explicitly treat this command as informational and exclude it from the pass claim.
set +e
sudo virt-host-validate qemu 2>&1 | tee "$log_dir/virt-host-validate.txt"
virt_validate_status=${PIPESTATUS[0]}
set -e
echo "$virt_validate_status" > "$log_dir/virt-host-validate.exit-code"

scripts/cloud-agent/setup.sh:46

  • Creating this directory with mkdir -p does not verify that /mnt/trident-cloud-agent is backed by the runner's data-disk mount. If the mount is missing or the pool configuration regresses, Cargo silently falls back to the nearly-full OS disk while setup still succeeds, defeating the purpose of CARGO_TARGET_DIR. Check the expected mount/source and fail closed before creating the target directory.
sudo install -d -o "$(id -u)" -g "$(id -g)" "$CARGO_TARGET_DIR"
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/copilot-setup-steps.yml Outdated
Prevent fork pull requests from executing privileged setup on the self-hosted runner, require the configured data-disk mount, and fail when virt-host-validate reports an error.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Copilot AI review requested due to automatic review settings September 11, 2026 18:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Two moderate issues remain unresolved in setup and nested-virtualization validation.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

scripts/cloud-agent/runner-smoke.sh:56

  • This block only records the module parameter and does not validate it: a value such as N/0, or the missing-parameter branch, still lets the smoke test continue and report PASS. Because this workflow is intended to validate nested KVM, parse the value and fail unless nested virtualization is enabled (and fail when the parameter cannot be found).

scripts/cloud-agent/setup.sh:31

  • Installing protoc alone leaves the Go code-generation path incomplete: make bin/netlaunch runs go generate pkg/tridentgrpc/grpc.go, whose generator invokes both --go_out and --go-grpc_out, so it requires protoc-gen-go and protoc-gen-go-grpc. Those plugins are listed in the repository's documented dependencies and installed by the existing build pipeline; without them, a Copilot task that regenerates the Go stubs fails. Install both plugins here and add their Go bin directory to the current and subsequent-step PATH.
sudo unzip -o "$RUNNER_TEMP/$protoc_archive" -d /usr/local
rm -f "$RUNNER_TEMP/$protoc_archive"
  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Install the pinned Go protobuf generators and require nested virtualization to be enabled before the runner smoke succeeds.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Copilot AI review requested due to automatic review settings September 11, 2026 18:52

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical rustup bootstrap and moderate runner-permission findings remain unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

scripts/cloud-agent/runner-smoke.sh:48

  • These checks use sudo, and the same is true of the QEMU probe and every virsh call below, so this smoke test can pass even when the GitHub runner account cannot access /dev/kvm or qemu:///system. That is the account Copilot and Trident's libvirt tooling use: virtdeploy opens libvirt without sudo and the docs require the user to be in the libvirt group. Run the validation as the runner user (using sudo only for installation/service setup), or explicitly provision and assert those permissions.
sudo test -r /dev/kvm
sudo test -w /dev/kvm
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread scripts/cloud-agent/setup.sh Outdated
Require the trusted runner image to provide rustup, grant the job user precise KVM and libvirt ACLs, and run the virtualization probe as that user.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Copilot AI review requested due to automatic review settings September 11, 2026 19:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The smoke-test diagnostic upload can mask the original setup failure when no artifact directory exists.

Review details

Suppressed comments (1)

.github/workflows/cloud-agent-runner-smoke.yml:57

  • If checkout or the prerequisite-install step fails, the smoke-test step never creates $RUNNER_TEMP/trident-cloud-agent-runner-smoke, but this upload still runs because !cancelled() is true for ordinary failures. With if-no-files-found: error, the diagnostic step then adds a second failure and hides the original setup error; use a non-error missing-files policy or create a marker directory before the earlier steps.
          if-no-files-found: error
  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Copilot AI review requested due to automatic review settings September 11, 2026 19:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Cargo’s configured target directory conflicts with the Makefile’s hard-coded artifact paths.

Get a fresh assessment by requesting another Copilot review.

Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread scripts/cloud-agent/setup.sh Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Copilot AI review requested due to automatic review settings September 11, 2026 19:25

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Resolve the workflow trust boundary and Cargo/Makefile artifact-path mismatch.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

scripts/cloud-agent/setup.sh:67

  • Setting a global Cargo build.target-dir here redirects outputs to /mnt/storage/..., but the Makefile still declares and consumes artifacts under target/ (for example, target/release/trident in Makefile:111-169 and Makefile:635-637). As a result, commands such as make validate run Cargo into the scratch directory and then execute a missing or stale target/release/trident; update the Makefile artifact paths or provide a target-dir-compatible mapping before applying this configuration globally.
target-dir = "$CARGO_TARGET_DIR"
  • Files reviewed: 5/5 changed files
  • Comments generated: 1
  • Review effort level: Lite

Comment thread .github/workflows/copilot-setup-steps.yml
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Copilot AI review requested due to automatic review settings September 11, 2026 19:34

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Resolve the privileged workflow trigger and Cargo/Makefile artifact path issues.

Review details

Suppressed comments (2)

.github/workflows/copilot-setup-steps.yml:10

  • This push trigger has no branch restriction, so a collaborator can push a feature branch that changes setup.sh and automatically execute it with sudo on the self-hosted 1ES runner. That breaks the stated trusted-default-branch boundary; restrict this trigger to the default branch (or remove the automatic trigger) before running privileged setup from pushed code.
  push:
    paths:
      - .github/workflows/copilot-setup-steps.yml
      - scripts/cloud-agent/setup.sh

scripts/cloud-agent/setup.sh:67

  • This global Cargo build.target-dir redirects builds to /mnt/storage/..., but the Makefile still declares and consumes artifacts under the repository's target/ tree (for example, make build and make validate use target/release/trident at Makefile:111-169 and 635-637). Those commands will build in the scratch directory and then look for a missing or stale binary, so update the Makefile artifact paths or provide a compatible mapping before applying this config globally.
target-dir = "$CARGO_TARGET_DIR"
  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Copilot AI review requested due to automatic review settings September 11, 2026 19:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The changes affect CI infrastructure, self-hosted runner setup, and virtualization validation, warranting final human review.

Review details
  • Files reviewed: 5/5 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 937dfad0-bd6e-454f-9046-bd773f4a3347
Copilot AI review requested due to automatic review settings September 11, 2026 20:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Runner infrastructure, privileged setup, and network-bound configuration require final human review.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants