Repository navigation
feat: [1/3] add native cloud certificate capabilities - #9902
Closed
Abigail Liang (abigailliang-aks-sig-node) wants to merge 1 commit into
Closed
Abigail Liang (abigailliang-aks-sig-node) wants to merge 1 commit into
Abigail Liang (abigailliang-aks-sig-node) wants to merge 1 commit into
Conversation
Split certificate retrieval, trust-store installation, shared runtime, and the standalone refresh command from #9891. Preserve certificate failure semantics while keeping existing provisioning on the shell path. Include corresponding unit coverage and direct refresh tests for all supported OS families. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: be499bac-1283-498e-b5b1-59d8c91047a4
Abigail Liang (abigailliang-aks-sig-node)
requested review from
a team
as code owners
October 26, 2026 21:50
Copilot started reviewing on behalf of
Abigail Liang (abigailliang-aks-sig-node)
October 26, 2026 21:51
Contributor
Windows Unit Test Results 3 files 18 suites 55s ⏱️ Results for commit c502637. |
Contributor
|
Contributor
|
Contributor
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A child process retaining stdout/stderr can keep the refresh blocked beyond both timeout budgets; process-group cancellation, a finite WaitDelay, and a subprocess regression test are needed.
1 open issue
What changed in this PR
Adds native cloud certificate capabilities to ANC as the first part of the cloud-initialization series. Provisioning continues to use init-aks-cloud.sh; repository configuration and refresh scheduling remain out of scope.
Changes:
- Adds legacy and RCV1P certificate retrieval, caching, and platform-specific trust-store installation.
- Exposes
refresh-cloud-certificates --location <region>with timeouts and guest-agent events. - Adds runtime, certificate, and CLI coverage across supported Linux platforms.
| File | Description |
|---|---|
| aks-node-controller/README.md | Documents refresh usage, behavior, and scope. |
| aks-node-controller/embeddednodecustomdata.go | Shares the Mariner OS identifier constant. |
| aks-node-controller/cloud_test_helpers_test.go | Adds certificate fixtures and mock endpoints. |
| aks-node-controller/cloud_runtime.go | Adds shared platform, HTTP, command, file, and event helpers. |
| aks-node-controller/cloud_runtime_test.go | Tests file writes and platform detection. |
| aks-node-controller/cloud_refresh.go | Implements standalone refresh and location normalization. |
| aks-node-controller/cloud_refresh_test.go | Tests refresh arguments and CLI error handling. |
| aks-node-controller/cloud_certificates.go | Implements certificate retrieval, caching, and trust-store updates. |
| aks-node-controller/cloud_certificates_test.go | Tests platform behavior, retries, cancellation, and certificate handling. |
| aks-node-controller/app.go | Registers the refresh command and test override. |
Review effort: Balanced
Comment on lines
+145
to
+149
| cmd := exec.CommandContext(ctx, name, args...) | ||
| cmd.Env = append(os.Environ(), "LC_ALL=C") | ||
| cmd.Stdin = bytes.NewReader(input) | ||
| var stdout, stderr bytes.Buffer | ||
| cmd.Stdout, cmd.Stderr = &stdout, &stderr |
Abigail Liang (abigailliang-aks-sig-node)
changed the title
This was referenced Oct 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it:
Part [1/3] of the native ANC cloud-initialization series split from #9891.
This PR adds certificate capabilities only; it does not switch provisioning away
from
init-aks-cloud.sh.into focused files without changing the original implementation's behavior.
refresh, bounded HTTP/command execution, cancellation, and guest-agent events.
refresh-cloud-certificates --location <region>command.It does not configure RepoDepot or install cron/systemd schedules.
per-certificate download skipping, and best-effort legacy trust-store updates.
Mariner, Azure Linux, Flatcar, and Azure Container Linux.
Existing provisioning, NBC precedence, the native shell template, generated CSE
fixtures, CustomData delivery, and VHD mappings are unchanged.
Migration and PR split (series overview)
The diagrams below describe the complete series, not activation in this PR.
PRs 1 and 2 add capabilities; the native provisioning-path switch happens only
in PR 3.
Migration: initialization moves to Go; the existing provisioning scripts remain.
flowchart TB subgraph Before["Before migration - also after PRs 1 and 2"] B1["ANC: App.Provision"] --> B2["cse_cmd.sh"] B2 --> B3["init-aks-cloud.sh, if present<br/>Certificates / RepoDepot / refresh scheduling"] B3 --> B4["provision_start.sh<br/>Existing provisioning scripts"] end subgraph After["After PR 3 - native provisioning path"] A1["ANC: App.Provision"] --> A2["Go: initializeAKSCloud<br/>1. Initialize certificates<br/>2. Configure refresh scheduling, if eligible<br/>3. Initialize RepoDepot"] A2 --> A3["cse_cmd.sh<br/>Shell initialization call removed"] A3 --> A4["provision_start.sh<br/>Existing provisioning scripts"] end B3 -.->|"Initialization responsibilities reimplemented in Go"| A2 classDef migrated fill:#CFE4FA,stroke:#0078D4,color:#000000 classDef unchanged fill:#DFF6DD,stroke:#107C10,color:#000000 class B3,A2 migrated class B4,A4 unchangedAfter PR 3, only
--provision-configwithout--nbc-cmduses the new path.Legacy CSE/NBC remain unchanged; NBC takes precedence when both flags are supplied.
The original shell artifact remains available for those compatibility paths.
PR split: dependency and merge order, not runtime execution order.
flowchart LR MAIN["main"] P1["[1/3] #9902<br/>Shared runtime and certificate capabilities<br/>Standalone certificate-refresh command<br/>No provisioning-path switch"] P2["[2/3] #9903<br/>RepoDepot configuration<br/>Package-cache refresh and retries<br/>Still no provisioning-path switch"] P3["[3/3] #9904<br/>Wire Go initialization into App.Provision<br/>Add cron/systemd refresh scheduling<br/>Remove the native shell-init call<br/>Switch the native provisioning path"] MAIN -->|"base for"| P1 P1 -->|"base for"| P2 P2 -->|"base for"| P3 classDef capability fill:#CFE4FA,stroke:#0078D4,color:#000000 classDef activation fill:#E8DAEF,stroke:#5C2D91,color:#000000 class P1,P2 capability class P3 activationPR 3 keeps activation atomic: adding the Go call without removing the shell
call would initialize twice; removing the shell call without adding the Go call
would skip initialization.
Series and merge order
main.call and shell-template removal together.
The aggregate #9891 remains a reference; review and merge the split series in order.
Validation
From
aks-node-controller, passed the ANC/parser race suites, CI-pinnedgolangci-lint v2.14.0 against the layer's base, and Linux AMD64/ARM64 builds.
Confirmed no changes to the provisioning template or native status-reporting
implementation. Azure/VHD end-to-end provisioning has not been run.
Which issue(s) this PR fixes:
No linked issue.