feat: add ACL CVM support - #9283
Conversation
Windows Unit Test Results 3 files 14 suites 53s ⏱️ Results for commit 3591a88. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
🟡 Changes recommended
The CVM build and deployment settings conflict with the specialized SIG contract, and VM user data exposes the bootstrap token through IMDS.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
vhdbuilder/packer/build-acl-cvm.sh:7
- 🔴 High Risk — Architecture: This passes the unchanged ACL template to Packer, but that template has none of the CVM builder settings (
security_type, secure boot, vTPM, guest-state encryption) and leaves the gallery destination generalized. Meanwhileproduce-packer-settings-functions.sh:523-535createscvmdefinitions as Specialized, and dependent PR #9340 maps ACL CVM toaclgen2CVMSpecialized. The build therefore cannot publish the advertised specialized confidential image consistently. Transform the ACL builder/destination with the same CVM settings as the existing CVM templates (while preserving ACL provisioners), or use a dedicated ACL CVM template.
packer build -timestamp-ui -var-file=vhdbuilder/packer/settings.json "$base_template"
- Files reviewed: 12/12 changed files
- Comments generated: 5
- Review effort level: Balanced
There was a problem hiding this comment.
🔵 Needs a closer look
The build, test, and scanning paths conflict with the specialized ACL CVM SIG contract and can fail publication or VM creation.
Review details
Suppressed comments (3)
vhdbuilder/packer/build-acl-cvm.sh:7
- 🔴 High Risk — Architecture: With
FEATURE_FLAGS=cvm,ensure_sig_vhd_existscreatesaclgen2CVMSpecializedas--os-state Specialized, but this script passes the ordinary ACL template unchanged; its gallery destination omitsspecialized, so Packer treats the output as generalized. Publication is therefore inconsistent with the destination definition and can fail before testing. Restore the ACL-specific transformation (includingshared_image_gallery_destination.specialized: trueand the required CVM builder/destination settings), or change the SIG definition and downstream mapping to generalized consistently.
base_template=${ACL_PACKER_TEMPLATE:-vhdbuilder/packer/vhd-image-builder-acl.json}
echo "Using pre-CPS ACL image settings derived from $base_template"
packer build -timestamp-ui -var-file=vhdbuilder/packer/settings.json "$base_template"
vhdbuilder/packer/test/run-test.sh:88
- 🔴 High Risk — Script Logic: ACL CVM currently resolves to the specialized SIG definition created by
produce-packer-settings-functions.sh, so omitting--specialized truemakesaz vm createuse generalized-image semantics and conflicts with that definition. This prevents the ACL CVM content-test VM from being created; keep the specialized flag for ACL as well.
if [ "${OS_SKU:-}" != "AzureContainerLinux" ]; then
TARGET_COMMAND_STRING+=" --specialized true"
fi
vhdbuilder/packer/vhd-scanning.sh:100
- 🔴 High Risk — Script Logic: The ACL CVM image definition is generated with specialized OS state, but this branch removes
--specialized trueonly for ACL. The scan VM is consequently created with generalized-image semantics and will fail deployment againstaclgen2CVMSpecialized; retain the flag for every CVM image.
if [ "${OS_SKU:-}" != "AzureContainerLinux" ]; then
VM_OPTIONS+=" --specialized true"
fi
- Files reviewed: 10/10 changed files
- Comments generated: 0 new
- Review effort level: Balanced
AgentBaker Linux gate detectiveRun: https://msazure.visualstudio.com/CloudNativeCompute/_build/results?buildId=179999642 TL;DR: Four MANA VM-series coverage scenarios failed after VMSS creation because the bastion tunnel/SSH handshake timed out with context deadline exceeded. Likely cause/signature: $sig - MANA E2E VMSS provisioning or bastion SSH infrastructure flake, not a VHD build failure. Evidence: failed task log Run AgentBaker E2E log 662; timeline failed stage/job/task; build metadata PR source |
Signed-off-by: Mayan Singh <mayankfz@gmail.com>
Derive the CVM build from the standard ACL Packer template and publish directly to a generalized CVM-supported gallery definition. Signed-off-by: Mayan Singh <mayankfz@gmail.com>
600680a to
70a84ca
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The generated ACL CVM image name conflicts with the definition referenced by dependent PR #9340.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 1
- Review effort level: Balanced
70a84ca to
4af09ca
Compare
There was a problem hiding this comment.
🔵 Needs a closer look
The generated aclgen2CVM definition does not match the aclgen2CVMSpecialized definition referenced by dependency PR #9340.
Review details
Suppressed comments (1)
vhdbuilder/packer/produce-packer-settings-functions.sh:55
- 🔴 High Risk — Architecture: This now publishes the ACL CVM definition as
aclgen2CVM, but the linked dependency PR #9340 currently mapsAKSACLCVMGen2toaclgen2CVMSpecialized. If both changes merge as-is, AgentBaker will request a definition that this release pipeline never publishes, so ACL CVM node provisioning cannot resolve its image. Update #9340 and its assertions to use the generalizedaclgen2CVMname, or keep the suffix here, before integrating the two PRs.
if [ "${OS_SKU,,}" != "azurecontainerlinux" ]; then
SIG_IMAGE_NAME+="Specialized"
- Files reviewed: 13/13 changed files
- Comments generated: 0 new
- Review effort level: Balanced
4af09ca to
3591a88
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The ACL test VM currently receives a literal custom-data filepath instead of the Ignition JSON contents.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 13/13 changed files
- Comments generated: 1
- Review effort level: Balanced
|
|
||
| TEST_VM_USER_DATA_ARGS=() | ||
| if [ "${OS_TYPE}" = "Linux" ] && [ "${OS_SKU:-}" = "AzureContainerLinux" ]; then | ||
| TEST_VM_USER_DATA_ARGS=(--user-data "./vhdbuilder/packer/acl-customdata.json") |
AgentBaker Linux gate detectiveRun: https://msazure.visualstudio.com/CloudNativeCompute/_build/results?buildId=180509996 Detective summaryTwo E2E scenarios failed in the same job, with E2E_FAILED_TESTS_RETRY_COUNT=0: LocalDNSHostsPlugin/Ubuntu2404 and Ubuntu2204Gen2_ImagePullIdentityBinding_NetworkIsolated. Timeline, failed test records, task log, build metadata, and PR metadata corroborate the failures. Likely cause / signature
ConfidenceMedium: both signatures point to shared environment/network path flakiness rather than this PR's ACL CVM changes. The PR does touch VHD build/E2E plumbing, but the failing scenarios are Ubuntu/localdns and image-pull identity network-isolated paths, not ACL CVM build validation. Recommended owner/actionNode Lifecycle/E2E owners: inspect westus3 shared E2E DNS/bastion health and retry history before blocking on PR code. PR author should only investigate if the same two signatures reproduce after the environment is healthy. Strongest alternativePR-change-caused E2E plumbing regression is the strongest alternative because this PR edits packer/E2E scripts; less likely because independent scenarios passed around these failures and the observed failures are DNS/bastion connectivity symptoms. Evidence links |
What this PR does / why we need it:
Add ACL CVM support
Which issue(s) this PR fixes:
Fixes https://dev.azure.com/mariner-org/ACL/_workitems/edit/23680
#9340, adds the ACL CVM distro, availability lists, SIG mapping, and API tests. The ACL CVM release build remains opt-in until that dependency lands.