Azure / Azure/AgentBaker

test: add script-level ShellSpec to lock RCV1P mode-selection wiring (follow-up to #8886)

Open
#8,946 0 comments 0 reactions 0 assignees View on GitHub
testing
Dominant language
Go
Stars
159
Forks
277
Avg merge
1d 16h
Merged PRs (30d)
164

Description

## Background

Follow-up to PR #8886, which added ShellSpec coverage for `parts/linux/cloud-init/artifacts/init-aks-custom-cloud.sh`.

The extracted `determine_cert_endpoint_mode()` function is the direct behavioral surface of RCV1P's cloud selection. It is now unit-tested for 8 inputs (`ussec`, `usnat`, `USSecWest`, `USNatEast`, `usgovvirginia`, `chinaeast2`, `francesouth`, empty).

However, there is **no test that verifies the `refresh_location` env-var/arg2 handoff into it actually happens at the script top-level** — that is currently covered only by a grep/static assertion. If a future refactor accidentally passes `$1` instead of `$refresh_location` to `determine_cert_endpoint_mode`, **all unit tests still pass** because:
1. ShellSpec sources the script with `__SOURCED__` set, so the `${__SOURCED__:+return 0}` guard returns before the top-level "main" (where the handoff lives) ever loads.
2. Shell functions enforce no arity/signature, so passing the wrong-but-valid argument fails silently.

## Proposed work

Add a **script-level** ShellSpec example (the "Option B" discussed in #8886) that runs the whole script and asserts the runtime wiring, e.g.:

```
LOCATION=usseceast bash init-aks-custom-cloud.sh ca-refresh
```

and asserts the emitted `AKS.CSE.rcv1p.certEndpointMode` event says `mode=legacy`.

This locks the **entire RCV1P mode-selection wiring** end-to-end (arg/env → `refresh_location` → `determine_cert_endpoint_mode` → emitted mode), not just the isolated function. A dedicated `ussec → legacy` script-level case would catch the "USSec node silently gets rcv1p" regression that unit tests structurally cannot.

## Why it wasn't done in #8886

Confirmed in-PR discussion (rchincha + @Devinwong): valid concern, but out of scope for #8886 (a "no logic change / add coverage" PR). It needs more than a one-line test.

## Implementation notes (from #8886 investigation)

Running the whole script under ShellSpec needs a few test seams so it doesn't exit early or hit the network in the CI container (Debian, root):

- Make `EVENTS_LOGGING_DIR` overridable so the emitted event file can be captured/inspected by the test.
- Make the top-level `/etc/os-release` path overridable (the Debian ShellSpec container fails OS detection otherwise and exits before reaching the mode-selection logic).
- Stub `curl` on `PATH` (e.g. returning `\n200`) so the cert-retrieval path doesn't make real wireserver calls.
- Tolerate `/root/AzureCACertificates` writes (harmless in the root container) or make that path overridable.

Then assert on the captured `AKS.CSE.rcv1p.certEndpointMode` event content (`mode=legacy` for `usseceast`, `mode=rcv1p` for a commercial region, etc.).

## Acceptance criteria

- [ ] A script-level ShellSpec example runs `init-aks-custom-cloud.sh` end-to-end and asserts the emitted `certEndpointMode` event for at least a `ussec*`/`usnat*` (→ `legacy`) case and a commercial (→ `rcv1p`) case.
- [ ] The test fails if the `refresh_location` handoff into `determine_cert_endpoint_mode` is broken (verified by temporarily reintroducing the `$1` bug).
- [ ] Passes in CI (`make shellspec-ci`) and `make validate-shell` (shellcheck) stays clean.
- [ ] No production logic changes to `init-aks-custom-cloud.sh` beyond adding overridable test seams.

_Filed as a follow-up so it can be picked up in a separate session/PR once #8886 merges._

Contributor guide

Open the contributing guide

Research direction

Start with parts/linux/cloud-init/artifacts/init-aks-custom-cloud.sh and the ShellSpec coverage added by PR #8886. Run make shellspec-ci, then add an end-to-end case that captures the emitted certEndpointMode event after invoking the script with a refresh location, covering both a ussec/usnat legacy result and a commercial rcv1p result. Keep production behavior unchanged apart from overridable test seams and verify make validate-shell remains clean.

Written by the indexing model from the issue text.

Assessment

Tech stack
bash
Domain
testing-qa
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.