Skip to content

patch(v2.1): use lowercase phone_home post key - #4692

Merged
nv-dmendoza merged 2 commits into
NVIDIA:release/v2.1from
pbreton:codex/backport-4670-v2.1
Aug 7, 2026
Merged

patch(v2.1): use lowercase phone_home post key#4692
nv-dmendoza merged 2 commits into
NVIDIA:release/v2.1from
pbreton:codex/backport-4670-v2.1

Conversation

@pbreton

@pbreton pbreton commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Backport of #4670 to release/v2.1.

What changed

  • Use the lowercase post property in the cloud-init phone_home configuration.
  • Add schema-backed coverage for the generated nested autoinstall.user-data phone-home configuration.

Why

Ubuntu validates nested cloud-config strictly and rejects the uppercase POST property, leaving affected installations stuck in Provisioning. Cloud-init mandates the lowercase post property.

Validation

  • go test ./api/pkg/api/model/util

Ref: 6569456

SnobbishFish40 and others added 2 commits August 6, 2026 16:02
@copy-pr-bot

copy-pr-bot Bot commented Aug 6, 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 Aug 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Summary by CodeRabbit

  • Bug Fixes

    • Updated cloud-init phone-home configuration to use the correct lowercase post field.
    • Added validation to ensure generated phone-home settings conform to the cloud-init schema.
  • Documentation

    • Added a schema describing supported phone-home options, including URL, posted fields, and retry settings.

Walkthrough

The PR adds a cloud-init phone-home JSON Schema, changes the phone-home key to lowercase, adds JSON Schema validation for generated autoinstall data, and declares the schema validation dependency.

Changes

Phone-home schema validation

Layer / File(s) Summary
Cloud-init phone-home schema contract
rest-api/api/pkg/api/model/util/testdata/cloud-init-phone-home.schema.json, rest-api/go.mod
The schema validates the required URI, supported post values, retry count, and disallowed properties. The JSON Schema library is added as a direct dependency.
Phone-home insertion validation
rest-api/api/pkg/api/model/util/util.go, rest-api/api/pkg/api/model/util/util_test.go
SitePhoneHomePost now uses "post". The test validates inserted autoinstall phone-home data against the schema.

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

Possibly related PRs

Suggested labels: rest-api

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the lowercase key change, schema coverage, motivation, and validation.
Title check ✅ Passed The title clearly and concisely identifies the primary change: using the lowercase phone_home post key.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

@pbreton
pbreton marked this pull request as ready for review August 6, 2026 23:09
@pbreton
pbreton requested a review from a team August 6, 2026 23:09
@pbreton pbreton self-assigned this Aug 6, 2026
@pbreton
pbreton requested a review from nv-dmendoza August 6, 2026 23:10
@pbreton

pbreton commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Full review finished.

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

🧹 Nitpick comments (1)
rest-api/api/pkg/api/model/util/util_test.go (1)

15-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Move the schema check into TestInsertPhoneHomeIntoUserData.

This test validates InsertPhoneHomeIntoUserData, which already has a table-driven top-level test in this file. Add the schema-validation case as a named t.Run subtest there.

As per coding guidelines and path instructions, use “one top-level Test... function and named table-driven t.Run subtests” for tests around the production function under test.

🤖 Prompt for 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.

In `@rest-api/api/pkg/api/model/util/util_test.go` around lines 15 - 34, Move the
schema-validation logic from TestInsertedPhoneHomeMatchesCloudInitSchema into
the existing table-driven TestInsertPhoneHomeIntoUserData test. Add it as a
named t.Run subtest covering the production function’s output, and remove the
separate top-level schema test while preserving its assertions and schema setup.

Sources: Coding guidelines, Path instructions

🤖 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.

Nitpick comments:
In `@rest-api/api/pkg/api/model/util/util_test.go`:
- Around line 15-34: Move the schema-validation logic from
TestInsertedPhoneHomeMatchesCloudInitSchema into the existing table-driven
TestInsertPhoneHomeIntoUserData test. Add it as a named t.Run subtest covering
the production function’s output, and remove the separate top-level schema test
while preserving its assertions and schema setup.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c5d9abe1-84bc-4e39-a68b-8701910c8bf9

📥 Commits

Reviewing files that changed from the base of the PR and between 85c95bf and e107ef5.

📒 Files selected for processing (4)
  • rest-api/api/pkg/api/model/util/testdata/cloud-init-phone-home.schema.json
  • rest-api/api/pkg/api/model/util/util.go
  • rest-api/api/pkg/api/model/util/util_test.go
  • rest-api/go.mod

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown

🔐 TruffleHog Secret Scan

🚨 Potential secrets detected!

TruffleHog found potential secrets in your code changes. This could include:

  • 🔑 API keys and tokens
  • 🔐 Passwords and credentials
  • 🎫 OAuth tokens
  • 🗝️ Private keys

📋 View Detailed Findings

👉 Click here to view the full TruffleHog scan results

The job logs contain:

  • Exact file paths and line numbers
  • Secret types detected
  • Verification status (verified/unverified)

Next Steps

  1. Review Details: Check the scan logs for specific findings
  2. Verify Findings: Determine if detected items are actual secrets
  3. Remove Secrets: If real, remove them from your code immediately
  4. Rotate Credentials: Revoke and regenerate any leaked credentials
  5. Prevent Future Leaks: Add sensitive files to .gitignore

Security Best Practices

⚠️ Never commit secrets to your repository! Use:

  • Environment variables for configuration
  • Secret management tools (Vault, AWS Secrets Manager, etc.)
  • GitHub Secrets for CI/CD workflows
  • .env files (added to .gitignore)

📖 Learn more about TruffleHog

🕐 Last updated: 2026-08-06 23:25:46 UTC | Commit: e107ef5

@thossain-nv thossain-nv added the rest-api Add this label when an issue or PR concerns NICo REST API label Aug 6, 2026 — with ChatGPT Codex Connector

@nv-dmendoza nv-dmendoza left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, trufflescan failing because of ci issue and is fine to ignore

@nv-dmendoza
nv-dmendoza merged commit f594b17 into NVIDIA:release/v2.1 Aug 7, 2026
283 of 293 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rest-api Add this label when an issue or PR concerns NICo REST API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants