test: add ShellSpec coverage for init-aks-cloud.sh - #8886
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes parts/linux/cloud-init/artifacts/init-aks-custom-cloud.sh safe to source (so ShellSpec can Include it) and adds ShellSpec coverage for key repo-depot and URL-checking helpers used in custom-cloud node boot.
Changes:
- Refactors
init-aks-custom-cloud.shto be source-safe by moving execution-only logic below an__SOURCED__guard and gatingset -xwhen sourced. - Makes several paths/env inputs overridable (e.g., apt sources/keyrings locations, wireserver endpoint) to enable hermetic tests.
- Adds/expands ShellSpec tests: keeps grep-based “wiring” checks and adds functional tests for
init_ubuntu_main_repo_depot,init_ubuntu_pmc_repo_depot, andcheck_url.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| spec/parts/linux/cloud-init/artifacts/init_aks_custom_cloud_spec.sh | Adds wiring + functional ShellSpec coverage for repo-depot helpers and URL checking. |
| parts/linux/cloud-init/artifacts/init-aks-custom-cloud.sh | Refactors for source-safety and testability (guarded execution, env-overridable paths/endpoints). |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d6aadc3-5e43-4ec8-999c-67e4dd763222
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d6aadc3-5e43-4ec8-999c-67e4dd763222
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
parts/linux/cloud-init/artifacts/init-aks-cloud.sh:17
validate-shellruns an additional ShellCheck pass with--shell=shfor every*.shnot in.pipelines/scripts/verify_shell.sh’sBASH_ONLY_LIST(verify_shell.sh:31-35, 99-103). This script isn’t currently in that allowlist, but the new__SOURCED__block adds more bash-only constructs (BASH_SOURCE[0],returnat top-level) on top of existing bashisms (e.g.,${var,,},[[ ... ]], arrays). Unless the lint configuration is updated, this is likely to break the POSIX ShellCheck gate.
# Dependency note: `jq` is guaranteed to be present on every AKS VHD (baked in
# by vhdbuilder/packer/install-dependencies.sh and shipped in the Azure Linux
# base image), so functions in this script use it without an explicit install
# step. Do not flag jq usage here as "used before install" — matches the
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
parts/linux/cloud-init/artifacts/init-aks-cloud.sh:477
init_ubuntu_pmc_repo_depotimplicitly depends on the globalubuntuRelvariable (it’s not set or validated inside the function). This makes the helper non-self-contained for ShellSpec and risks generating an invalid RepoDepot URL like.../ubuntu//prodif a caller ever invokes it beforeubuntuRelis set.
Consider making ubuntuRel an explicit parameter (e.g. init_ubuntu_pmc_repo_depot <endpoint> <ubuntuRel>) or deriving it inside the function (e.g. via lsb_release --release) with a clear fatal error if it can’t be determined.
function init_ubuntu_pmc_repo_depot {
local repodepot_endpoint="$1"
# Add Microsoft packages source to the azure specific sources.list.
echo "Adding the packages.microsoft.com Ubuntu-$ubuntuRel repo..."
local microsoftPackageSource="$repodepot_endpoint/microsoft/ubuntu/$ubuntuRel/prod"
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d6aadc3-5e43-4ec8-999c-67e4dd763222
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
spec/parts/linux/cloud-init/artifacts/init_aks_cloud_spec.sh:83
- This spec
Includesinit-aks-cloud.shwithout setting__SOURCED__. The common pattern in this repo’s ShellSpec tests (e.g.cse_main_spec.sh) is to set__SOURCED__=1before sourcing so the included script can reliably skip its top-level execution and so test-only overrides (likeOS_RELEASE_FILE) can be honored without depending on the runner’s real/etc/os-release.
Setting __SOURCED__=1 here also makes the test resilient if the script follows the established __SOURCED__ convention for sourced-vs-executed behavior.
Describe 'init-aks-cloud.sh functional tests'
# Include sources the file and loads function definitions only; top-level provisioning
# logic is skipped when sourced.
# without this Include would fall through into the top-level provisioning path
# (wireserver calls, cron install, exit) and cause side effects. Matches the
# sourcing convention documented in the script header and used in cse_main_spec.sh.
Include "./parts/linux/cloud-init/artifacts/init-aks-cloud.sh"
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d6aadc3-5e43-4ec8-999c-67e4dd763222
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
parts/linux/cloud-init/artifacts/init-aks-cloud.sh:475
init_ubuntu_pmc_repo_depotimplicitly depends on a globalubuntuRelvariable being set by the caller (top-level code sets it, but the function itself doesn’t). This makes the helper non-self-contained for sourcing/tests and could generate an invalid RepoDepot URL ifubuntuRelis unset.
Consider making the function derive ubuntuRel when missing (while still honoring a pre-set value) so the helper can be called safely in isolation.
local repodepot_endpoint="$1"
# Add Microsoft packages source to the azure specific sources.list.
echo "Adding the packages.microsoft.com Ubuntu-$ubuntuRel repo..."
local microsoftPackageSource="$repodepot_endpoint/microsoft/ubuntu/$ubuntuRel/prod"
…ud.sh Replaces the BASH_SOURCE[0] != \ is_script_sourced detection with the repo's standard \ pattern, consistent with localdns.sh, ubuntu-snapshot-update.sh, mariner-package-update.sh, and others. - Remove is_script_sourced variable and BASH_SOURCE detection - Replace is_script_sourced guards with [ -n "\" ] checks - Add \ guard at end of function definitions - Move set -x to after the guard (main execution block only) - Update spec: use __SOURCED__=1 . script in setup() instead of Include, matching the cse_main_spec.sh pattern Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d6aadc3-5e43-4ec8-999c-67e4dd763222
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d6aadc3-5e43-4ec8-999c-67e4dd763222
…n_repo_depot The __SOURCED__ check was gating the OS_RELEASE_FILE override, but __SOURCED__ is not exported so it isn't visible when ShellSpec's 'When call' runs the function in a subshell. OS_RELEASE_FILE is a test-only env var (unset in production), so honoring it unconditionally via \ is safe and matches the pattern of all other overrideable vars in the same function (APT_KEYRINGS_DIR, SSL_CERTS_DIR, etc.). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d6aadc3-5e43-4ec8-999c-67e4dd763222
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
parts/linux/cloud-init/artifacts/init-aks-cloud.sh:3
- Header comment has an extraneous trailing "-" and reads awkwardly ("tested in -"). This looks like a typo and can confuse readers trying to find the referenced spec file.
# functions defined until "${__SOURCED__:+return}" are sourced and tested in -
# spec/parts/linux/cloud-init/artifacts/init_aks_cloud_spec.sh.
No tested function calls wireserver, so the override path was unused. Simplify to a plain assignment — matches the rest of the codebase and removes unnecessary complexity. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d6aadc3-5e43-4ec8-999c-67e4dd763222
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d6aadc3-5e43-4ec8-999c-67e4dd763222
The added guards (skip if missing, skip if same inode, non-fatal failure) changed production behavior that was not called out in the PR description. Revert to the original simple cp - the test environment already provides a real fake ca-certificates.crt via SSL_CERTS_DIR override so no guards are needed for testability. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2d6aadc3-5e43-4ec8-999c-67e4dd763222
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
parts/linux/cloud-init/artifacts/init-aks-cloud.sh:302
init_ubuntu_main_repo_depotnow allowsOS_RELEASE_FILEto override the file that gets sourced (. "$os_release_file"). Since this script runs as root, sourcing a path influenced by environment enables arbitrary code execution if the environment can be manipulated. To keep testability without changing production behavior, only honorOS_RELEASE_FILEwhen the script is being sourced for ShellSpec (i.e.,__SOURCED__is set).
local sources_list="${APT_SOURCES_LIST:-/etc/apt/sources.list}"
local sources_list_d="${APT_SOURCES_LIST_D_DIR:-/etc/apt/sources.list.d}"
local os_release_file="${OS_RELEASE_FILE:-/etc/os-release}"
No functional behavior change in
init-aks-cloud.sh; this PR adds/adjusts ShellSpec coverage and testability wiring.This updates and extends test coverage after the script consolidation work from #8096.
Related: ADO#36479419
Changes
Script restructuring for testability
${__SOURCED__:+return 0}guard to allow safe sourcing in ShellSpec.determine_cert_endpoint_mode()coverage and script-level wiring assertions forca-refreshmode.APT_*,YUM_REPOS_DIR, etc.) while preserving runtime defaults.ShellSpec coverage added/expanded
ca-refreshflow and location-to-mode mapping.determine_cert_endpoint_modeinit_ubuntu_main_repo_depotinit_ubuntu_pmc_repo_depotcheck_urlinit_mariner_repo_depotinit_azurelinux_repo_depotussec*,usnat*) and rcv1p paths (e.g., fairfax/mooncake/bleu/empty).