Skip to content

fix: Align v2.0 behavior and documentation#3321

Open
kfelternv wants to merge 6 commits into
NVIDIA:mainfrom
kfelternv:docs/v2.0-release-audit
Open

fix: Align v2.0 behavior and documentation#3321
kfelternv wants to merge 6 commits into
NVIDIA:mainfrom
kfelternv:docs/v2.0-release-audit

Conversation

@kfelternv

@kfelternv kfelternv commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Aligns the v2.0 REST behavior, OpenAPI contract, generated SDK, and operator documentation with the current implementation.

  • restricts Machine power control to Provider Admins instead of admitting Provider Viewers
  • corrects the BMC credential contract to its actual 200 response, documents validation errors, and regenerates the affected Go SDK surface
  • documents the required HostUpdateInProgress / PreventAllocations health report before DPU reprovisioning, the Set/Clear/Restart modes, attached-Instance acknowledgements, and current firmware behavior
  • updates the published nicocli action names to current and stats
  • documents the standalone read-only MCP server, Go 1.26.4 build requirement, current container targets, and other v2.0 behavior corrections

How we verified it

Current revision: 2a151c30919d561cb12626554fe863f692acac2e

Hands-on REST/MCP revision: 8766edc644fa5ce53d75eb83d30039e08289ba02. The current head adds only the generated SDK update after that runtime exercise.

DevSpace integration base: PR #3304 at 1a0d78b9454ad5fbf841f3886a0451efb4ad2e98. The tested composite revision was 3615be5437aa43a74ef2b98f07f47f37df9a8bc5.

Environment: Linux x86_64 NICo development VM, Docker 29.1.3, kind 0.27.0, kubectl 1.30.8, and DevSpace 6.3.21 using the kind-kind context.

The full integrated stack was built and deployed with:

devspace deploy -n nico-system --no-colors --max-concurrent-builds 3

DevSpace rolled out the REST API image localhost:5000/nico-rest-api:3615be54-1783656294 (sha256:934ecca2c644115eadeec3654cd708de31203f948852f1e884d826ab44ac375a) and MCP image localhost:5000/nico-mcp:3615be54-1783656294 (sha256:6bdbe7c15b547da210b9c0091bdc1e84db14c1525de8aab7c7a6b328ac77aee9). The post-deploy integration check reported the registered site online with seven current Core machines.

Hands-on results:

Machine power, old stack: Provider Viewer 404; Provider Admin 404
Machine power, patched stack: Provider Viewer 403; Provider Admin 404

Health report create: 200
DPU reprovision Set: 202
DPU reprovision Clear: 202
Health report cleanup: 204

BMC invalid request: 400
BMC credential write: 200
Password present in response: false
Vault write observed: true
Original local Vault value restored: true

nicocli tenant current: pass against the live REST API
nicocli tenant stats: pass against the live REST API
nicocli service-account current --help: pass from the deployed image

MCP tools/list: 200, 91 tools
MCP nico_get_all_site call with forwarded bearer token: 200

The authorization comparison used the same nonexistent Machine ID in both revisions. The old 404 proves the Provider Viewer passed authorization and reached Machine lookup; the patched 403 proves it is now rejected at the intended role gate. The Provider Admin still reached lookup and returned 404 in both revisions.

The DPU request was cleared, the temporary health report was deleted, the BMC credential was restored, and the temporary Provider Viewer role was removed after verification.

Supporting checks:

go test ./api/pkg/api/handler -run 'MachinePower|BMCCredential' -count=1
go test ./cli/pkg -run '^(TestOperationAction|TestNewApp_CurrentSingletonCommandSurface|TestBuildCommands_CurrentSingletonsAreRunnable|TestParseSpec_EmbeddedSpec|TestBuildCommands_EmbeddedSpec)$' -count=1
(cd sdk/standard && go test ./... -count=1)
make lint-openapi
npx -y fern-api@4.81.0 check
git diff --check upstream/main...HEAD

All listed checks passed. A broad go test ./cli/pkg also reaches the unrelated TestSiteBootstrapCommandWritesReplayableManifest failure; the same failure reproduces on upstream main at 2c3bf2f9b, while the focused current/stats command tests above pass.

How to reproduce the verification

Prerequisites:

  • the NICo development VM with Docker, kind, kubectl, jq, curl, and DevSpace
  • access to the local kind-kind development cluster
  • enough Docker storage for the integrated stack build

Start from a clean checkout and assemble the exact product and DevSpace revisions:

git clone https://github.com/NVIDIA/infra-controller.git
cd infra-controller
git fetch origin pull/3321/head:pr-3321 pull/3304/head:pr-3304
git checkout --detach 2a151c30919d561cb12626554fe863f692acac2e
git merge --no-commit --no-ff 1a0d78b9454ad5fbf841f3886a0451efb4ad2e98

kubectl config use-context kind-kind
devspace deploy -n nico-system --no-colors --max-concurrent-builds 3

Expected checkpoint: the deploy succeeds, all application pods become Ready, the registered site is online, and the post-deploy check reports at least one current Core Machine.

Forward the REST API and Keycloak in separate terminals:

kubectl --context kind-kind -n nico-rest port-forward service/nico-rest-api 18388:8388
kubectl --context kind-kind -n nico-rest port-forward service/keycloak 18082:8082

Acquire the local Provider Admin token:

TOKEN="$(curl -fsS -X POST \
  http://localhost:18082/realms/nico-dev/protocol/openid-connect/token \
  -H 'Content-Type: application/x-www-form-urlencoded' \
  -d client_id=nico-api \
  -d client_secret=nico-local-secret \
  -d grant_type=password \
  -d username=admin@example.com \
  -d password=adminpassword | jq -er '.access_token')"

Verify that Provider Viewer is rejected before Machine lookup while Provider Admin still reaches it:

KCADM=(kubectl --context kind-kind -n nico-rest exec deployment/keycloak -- \
  /opt/keycloak/bin/kcadm.sh)
"${KCADM[@]}" config credentials --server http://localhost:8080 \
  --realm master --user admin --password admin
"${KCADM[@]}" add-roles -r nico-dev --uusername testuser \
  --rolename 'test-org:PROVIDER_VIEWER'
remove_viewer() {
  "${KCADM[@]}" remove-roles -r nico-dev --uusername testuser \
    --rolename 'test-org:PROVIDER_VIEWER' >/dev/null
}
trap remove_viewer EXIT

VIEWER_TOKEN="$(curl -fsS -X POST \
  http://localhost:18082/realms/nico-dev/protocol/openid-connect/token \
  -H 'Content-Type: application/x-www-form-urlencoded' \
  -d client_id=nico-api \
  -d client_secret=nico-local-secret \
  -d grant_type=password \
  -d username=testuser@example.com \
  -d password=testpassword | jq -er '.access_token')"
URL=http://localhost:18388/v2/org/test-org/nico/machine/00000000-0000-0000-0000-000000000000/power

curl -sS -o /dev/null -w 'viewer=%{http_code}\n' -X PATCH "${URL}" \
  -H "Authorization: Bearer ${VIEWER_TOKEN}" \
  -H 'Content-Type: application/json' -d '{"action":"On"}'
curl -sS -o /dev/null -w 'admin=%{http_code}\n' -X PATCH "${URL}" \
  -H "Authorization: Bearer ${TOKEN}" \
  -H 'Content-Type: application/json' -d '{"action":"On"}'

remove_viewer
trap - EXIT
unset VIEWER_TOKEN

Expected result: viewer=403 and admin=404.

Verify the BMC credential response contract while preserving the seeded local credential:

SITE_ID="$(kubectl --context kind-kind -n nico-rest get configmap \
  nico-rest-site-agent-config -o jsonpath='{.data.CLUSTER_ID}')"
VAULT=(kubectl --context kind-kind -n vault exec statefulset/vault -- \
  env VAULT_ADDR=http://127.0.0.1:8200 VAULT_TOKEN=root vault)
VAULT_WRITE=(kubectl --context kind-kind -n vault exec -i statefulset/vault -- \
  env VAULT_ADDR=http://127.0.0.1:8200 VAULT_TOKEN=root vault)
VAULT_PATH=secrets/machines/bmc/site/root
ORIGINAL="$("${VAULT[@]}" kv get -format=json "${VAULT_PATH}" | jq -c '.data.data')"
restore_bmc() {
  printf '%s' "${ORIGINAL}" | "${VAULT_WRITE[@]}" kv put "${VAULT_PATH}" - >/dev/null
}
trap restore_bmc EXIT

read -rsp 'Temporary local BMC password: ' BMC_PASSWORD
printf '\n'

curl -sS -o /dev/null -w 'invalid=%{http_code}\n' -X PUT \
  http://localhost:18388/v2/org/test-org/nico/credential/bmc \
  -H "Authorization: Bearer ${TOKEN}" -H 'Content-Type: application/json' \
  -d "{\"siteId\":\"${SITE_ID}\",\"kind\":\"Invalid\",\"password\":\"${BMC_PASSWORD}\"}"

curl -sS -o bmc-response.json -w 'valid=%{http_code}\n' -X PUT \
  http://localhost:18388/v2/org/test-org/nico/credential/bmc \
  -H "Authorization: Bearer ${TOKEN}" -H 'Content-Type: application/json' \
  -d "{\"siteId\":\"${SITE_ID}\",\"kind\":\"SiteWideRoot\",\"password\":\"${BMC_PASSWORD}\"}"

jq -e --arg site "${SITE_ID}" \
  '.siteId == $site and .kind == "SiteWideRoot" and (has("password") | not)' \
  bmc-response.json
"${VAULT[@]}" kv get -format=json "${VAULT_PATH}" | \
  jq -e --arg password "${BMC_PASSWORD}" \
  '[.. | strings] | index($password) != null'

restore_bmc
trap - EXIT
rm -f bmc-response.json
unset ORIGINAL BMC_PASSWORD

Expected result: invalid=400, valid=200, the response omits password, Vault contains the temporary value during the check, and cleanup restores the original local value.

Verify the documented health precondition and DPU Set/Clear path with a current unassigned simulated host:

MACHINE_ID="$(curl -fsS \
  "http://localhost:18388/v2/org/test-org/nico/machine?siteId=${SITE_ID}&isMissingOnSite=false&pageSize=100" \
  -H "Authorization: Bearer ${TOKEN}" | jq -er '.[0].id')"

curl -fsS -X PUT \
  "http://localhost:18388/v2/org/test-org/nico/machine/${MACHINE_ID}/health-report" \
  -H "Authorization: Bearer ${TOKEN}" -H 'Content-Type: application/json' \
  -d '{"source":"maintenance.dpu-reprovision","mode":"Merge","alerts":[{"id":"HostUpdateInProgress","message":"DPU reprovisioning in progress","classifications":["PreventAllocations"]}]}'

curl -fsS -X PATCH \
  "http://localhost:18388/v2/org/test-org/nico/machine/${MACHINE_ID}/dpu/reprovision" \
  -H "Authorization: Bearer ${TOKEN}" -H 'Content-Type: application/json' \
  -d '{"mode":"Set"}'

curl -fsS -X PATCH \
  "http://localhost:18388/v2/org/test-org/nico/machine/${MACHINE_ID}/dpu/reprovision" \
  -H "Authorization: Bearer ${TOKEN}" -H 'Content-Type: application/json' \
  -d '{"mode":"Clear"}'

curl -fsS -X DELETE \
  "http://localhost:18388/v2/org/test-org/nico/machine/${MACHINE_ID}/health-report/maintenance.dpu-reprovision" \
  -H "Authorization: Bearer ${TOKEN}" -o /dev/null

Expected statuses are 200 for the health report, 202 for Set, 202 for Clear, and 204 for cleanup.

Verify the deployed nicocli action names against the live API:

IMAGE="$(kubectl --context kind-kind -n nico-rest get deployment nico-rest-api \
  -o jsonpath='{.spec.template.spec.containers[0].image}')"
export NICO_BASE_URL=http://localhost:18388 NICO_ORG=test-org NICO_TOKEN="${TOKEN}"

docker run --rm --network host -e NICO_BASE_URL -e NICO_ORG -e NICO_TOKEN \
  --entrypoint /app/nicocli "${IMAGE}" tenant current --output json
docker run --rm --network host -e NICO_BASE_URL -e NICO_ORG -e NICO_TOKEN \
  --entrypoint /app/nicocli "${IMAGE}" tenant stats --output json
docker run --rm --network host --entrypoint /app/nicocli "${IMAGE}" \
  service-account current --help

Expected result: both live tenant commands return JSON successfully, and the service-account current action is present.

Finally, forward the MCP service and exercise an authenticated GET tool:

kubectl --context kind-kind -n nico-rest port-forward service/nico-mcp 18080:8080
curl -fsS http://localhost:18080/mcp \
  -H 'Content-Type: application/json' \
  -H 'Accept: application/json, text/event-stream' \
  -d '{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{}}' | jq '.result.tools | length'

curl -fsS http://localhost:18080/mcp \
  -H 'Content-Type: application/json' \
  -H 'Accept: application/json, text/event-stream' \
  -H "Authorization: Bearer ${TOKEN}" \
  -d '{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"nico_get_all_site","arguments":{"org":"test-org"}}}' | jq

Expected result: the tool list is non-empty and the site call returns a successful MCP result without an error.

Signed-off-by: Kyle Felter <kfelter@nvidia.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 409d1dcd-42c5-4415-945c-6a057ede16de

📥 Commits

Reviewing files that changed from the base of the PR and between f9c9078 and 37cda21.

⛔ Files ignored due to path filters (1)
  • rest-api/sdk/standard/model_dpu_reprovision_request.go is excluded by !rest-api/sdk/standard/model_*.go
📒 Files selected for processing (2)
  • rest-api/AGENTS.md
  • rest-api/openapi/spec.yaml
✅ Files skipped from review due to trivial changes (1)
  • rest-api/AGENTS.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • rest-api/openapi/spec.yaml

Summary by CodeRabbit

  • Documentation
    • Rewrote health override behavior docs with explicit REST GET/PUT/DELETE endpoints and updated Merge/Replace terminology.
    • Updated tenant/service-account and nicocli command guidance, plus clarified container build outputs.
    • Expanded DPU reprovisioning, MCP server, and host lifecycle documentation (including conditional UEFI lockdown).
  • Bug Fixes
    • Blocked provider-viewer access to machine power control; added automated coverage.
  • API Updates
    • Refreshed OpenAPI/Swagger descriptions and examples for BMC, machine power, DPU reprovisioning, and health report payloads.

Walkthrough

The changes update CLI and MCP documentation, clarify health and lifecycle REST workflows, revise OpenAPI examples and descriptions, and restrict machine power control for provider viewers with test coverage.

Changes

API and Documentation Alignment

Layer / File(s) Summary
CLI command and MCP documentation
docs/configuration/*, docs/manuals/*
CLI examples use tenant current, tenant stats, and service-account current; the NICo reference adds action-resolution details, Go requirements, MCP server guidance, and updated image-build documentation.
Lifecycle and health API contracts
docs/architecture/health_aggregation.md, docs/dpu-management/*, docs/provisioning/*, rest-api/openapi/spec.yaml
Health override operations, host lifecycle lockdown behavior, DPU reprovisioning prerequisites, modes, acknowledgment requirements, and health-report examples are documented.
Machine API authorization and descriptions
rest-api/api/pkg/api/handler/*, rest-api/openapi/*, rest-api/AGENTS.md
Machine power control rejects provider viewers, and BMC, power-control, lockdown, Redfish, enum-formatting, and breaking-change handling are updated.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the main change: aligning v2.0 behavior and documentation.
Description check ✅ Passed The description is directly related to the changeset and clearly matches the documented REST, SDK, CLI, and docs updates.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

kfelternv added 2 commits July 9, 2026 22:53
Signed-off-by: Kyle Felter <kfelter@nvidia.com>
Signed-off-by: Kyle Felter <kfelter@nvidia.com>
@kfelternv kfelternv marked this pull request as ready for review July 10, 2026 04:27
@kfelternv kfelternv requested a review from a team as a code owner July 10, 2026 04:27
@github-actions

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/architecture/health_aggregation.md`:
- Around line 293-305: Update the operator-facing prose around the health-report
API to consistently use “NICo-managed host” and “built-in” instead of “NICo
managed host” and “builtin NICo tools,” including the Merge and Replace mode
descriptions.

In `@docs/configuration/tenant_management.md`:
- Line 105: Label the fenced code block in tenant_management.md with a language
identifier, such as text, by changing the opening fence to ```text while
preserving the block contents and closing fence.

In `@docs/manuals/building_nico_containers.md`:
- Line 49: Update the command comment describing machine-validation and
boot-artifact images to use the exact architecture name “x86_64” instead of
“x86”, resulting in “x86_64/aarch64”.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 72a719ed-4eeb-4a8a-8754-8b8a592194b8

📥 Commits

Reviewing files that changed from the base of the PR and between 2c3bf2f and 2a151c3.

⛔ Files ignored due to path filters (7)
  • rest-api/sdk/standard/api_bmc_credential.go is excluded by !rest-api/sdk/standard/api_*.go
  • rest-api/sdk/standard/api_bmc_reset.go is excluded by !rest-api/sdk/standard/api_*.go
  • rest-api/sdk/standard/api_dpu_reprovision.go is excluded by !rest-api/sdk/standard/api_*.go
  • rest-api/sdk/standard/api_machine.go is excluded by !rest-api/sdk/standard/api_*.go
  • rest-api/sdk/standard/model_bmc_reset_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_dpu_reprovision_request.go is excluded by !rest-api/sdk/standard/model_*.go
  • rest-api/sdk/standard/model_machine_power_control_request.go is excluded by !rest-api/sdk/standard/model_*.go
📒 Files selected for processing (11)
  • docs/architecture/health_aggregation.md
  • docs/configuration/org-permissions.md
  • docs/configuration/tenant_management.md
  • docs/dpu-management/dpu-lifecycle-management.md
  • docs/manuals/building_nico_containers.md
  • docs/manuals/nicocli-reference.md
  • docs/provisioning/ingesting-hosts-rest-api.md
  • rest-api/api/pkg/api/handler/bmccredential.go
  • rest-api/api/pkg/api/handler/machinepower.go
  • rest-api/api/pkg/api/handler/machinepower_test.go
  • rest-api/openapi/spec.yaml

Comment thread docs/architecture/health_aggregation.md Outdated
Comment thread docs/configuration/tenant_management.md Outdated
Comment thread docs/manuals/building_nico_containers.md Outdated
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
nico-flow 15 1 3 3 0 8
nico-nsm 7 0 1 5 0 1
nico-psm 15 1 3 3 0 8
nico-rest-api 17 1 4 4 0 8
nico-rest-cert-manager 14 1 3 3 0 7
nico-rest-db 15 1 3 3 0 8
nico-rest-site-agent 15 1 3 3 0 8
nico-rest-site-manager 14 1 3 3 0 7
nico-rest-workflow 15 1 3 3 0 8
TOTAL 127 8 26 30 0 63

Per-CVE detail lives in the per-service grype-* artifacts (JSON + SARIF). Severity counts only — no CVE IDs published here.

Signed-off-by: Kyle Felter <kfelter@nvidia.com>
@github-actions

Copy link
Copy Markdown

🔐 TruffleHog Secret Scan

No secrets or credentials found!

Your code has been scanned for 700+ types of secrets and credentials. All clear! 🎉

🔗 View scan details

🕐 Last updated: 2026-07-10 04:32:52 UTC | Commit: 2a151c3

Signed-off-by: Kyle Felter <kfelter@nvidia.com>

@thossain-nv thossain-nv 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.

Thanks for the fixes @kfelternv

Comment thread rest-api/openapi/spec.yaml Outdated
Signed-off-by: Kyle Felter <kfelter@nvidia.com>
@kfelternv kfelternv enabled auto-merge (squash) July 10, 2026 19:00
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