Skip to content
This repository was archived by the owner on Jun 2, 2026. It is now read-only.

docs: Remove invalid Allocation Constraint endpoints from OpenAPI schema - #584

Merged
thossain-nv merged 4 commits into
mainfrom
chore/fix-schema
May 29, 2026
Merged

docs: Remove invalid Allocation Constraint endpoints from OpenAPI schema#584
thossain-nv merged 4 commits into
mainfrom
chore/fix-schema

Conversation

@thossain-nv

@thossain-nv thossain-nv commented May 28, 2026

Copy link
Copy Markdown
Contributor

Description

OpenAPI schema is incorrectly listing the following endpoints, this PR removed them.

  • Create Allocation Constraint (User cannot explicitly create them, they are created as part of Allocation creation)
  • Get Allocation Constraint (User cannot explicitly retrieve them, they are retrieved as part of Allocation creation)
  • Delete Allocation Constraint (User cannot explicitly delete them, they are deleted as part of Allocation creation)

The PR also fixes missing 4xx responses for certain endpoints that were causing lint errors.

This PR also enables skipping no-ambigous-path lint error reported by redocly linter which is not taking into account the UUID format of strings to disambiguate paths.

Type of Change

  • Docs - Changes in documentation or OpenAPI schema (docs:)

Services Affected

None

Related Issues (Optional)

None

Breaking Changes

  • This PR contains breaking changes

Testing

  • No testing required (docs, internal refactor, etc.)

Additional Notes

None

@thossain-nv
thossain-nv requested a review from a team as a code owner May 28, 2026 21:36
@thossain-nv
thossain-nv requested a review from parmani-nv May 28, 2026 21:36
@coderabbitai

coderabbitai Bot commented May 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

OpenAPI adds UnauthorizedError and expands 400/403/401 responses, removes GET/DELETE for allocation-constraint; generated Go SDK removes corresponding request types, sets Accept: application/json, and decodes NICoAPIError for specific statuses; CI/Makefile lint now skips no-ambiguous-paths.

Changes

API Contract and SDK Generation

Layer / File(s) Summary
OpenAPI Response Component Definitions and Operation Removals
openapi/spec.yaml
New UnauthorizedError component defined referencing NICoAPIError. GET and DELETE operations removed from the allocation-constraint path; PATCH remains and is updated.
OpenAPI Error Response References Across Endpoints
openapi/spec.yaml
Multiple endpoints across allocation, site, IP block, machine, instance now reference 400 ValidationError and 403 ForbiddenError; user and metadata endpoints reference 401 UnauthorizedError.
Generated SDK Constraint Endpoint Removal
sdk/standard/api_allocation.go
Removed generated request types and AllocationAPIService wrappers/executors for allocation-constraint create/list/get operations.
Generated SDK Error Decoding and Accept Header Updates
sdk/standard/api_allocation.go, sdk/standard/api_instance.go, sdk/standard/api_instance_type.go, sdk/standard/api_ip_block.go, sdk/standard/api_machine.go, sdk/standard/api_site.go
Delete/Update Execute methods negotiate Accept: application/json and decode NICoAPIError for HTTP 400/403, populating GenericOpenAPIError with formatted messages and model; 400 branches typically return immediately.
Generated SDK 401 Unauthorized Handling
sdk/standard/api_metadata.go, sdk/standard/api_user.go
GetMetadataExecute and GetUserExecute now handle HTTP 401 by decoding NICoAPIError and attaching a formatted error message and model to GenericOpenAPIError.
CI / Makefile OpenAPI Lint Configuration
Makefile, .github/workflows/lint-and-test.yml
lint-openapi Make target and CI job updated to run npx @redocly/cli lint with --skip-rule=no-ambiguous-paths.
SDK client small fix and generated model formatting
sdk/standard/client.go, sdk/standard/model_*.go
addFile now defers file.Close(); many generated model struct fields were reformatted (whitespace/alignment only, no semantic change).

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~40 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the primary change: removal of invalid Allocation Constraint endpoints from the OpenAPI schema, which aligns with the main objective of this pull request.
Description check ✅ Passed The description is comprehensive and directly relates to the changeset, explaining the rationale for endpoint removal, missing 4xx response fixes, and the linting rule bypass for no-ambiguous-paths.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch chore/fix-schema

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

@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-05-28 21:37:42 UTC | Commit: 3078fa3

@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: 1

🤖 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 `@Makefile`:
- Line 674: The Makefile command currently uses "npx `@redocly/cli` lint
--skip-rule=no-ambiguous-paths ./openapi/spec.yaml", which causes local lint to
diverge from CI; either remove the "--skip-rule=no-ambiguous-paths" flag from
that Makefile target so local lint matches CI, or move the rule override into
the shared Redocly config (e.g., add the no-ambiguous-paths rule disable to the
repo-wide Redocly config consumed by both CI and Makefile) and then update the
Makefile to call "npx `@redocly/cli` lint ./openapi/spec.yaml" so both paths use
the same configuration.
🪄 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: e0807fd0-6a97-4268-b4b6-183abcc0dcc6

📥 Commits

Reviewing files that changed from the base of the PR and between c39c263 and 3078fa3.

📒 Files selected for processing (11)
  • Makefile
  • docs/index.html
  • openapi/spec.yaml
  • sdk/standard/api_allocation.go
  • sdk/standard/api_instance.go
  • sdk/standard/api_instance_type.go
  • sdk/standard/api_ip_block.go
  • sdk/standard/api_machine.go
  • sdk/standard/api_metadata.go
  • sdk/standard/api_site.go
  • sdk/standard/api_user.go

Comment thread Makefile

@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: 1

🤖 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 @.github/workflows/lint-and-test.yml:
- Around line 114-115: The workflow currently skips the Redocly rule
no-ambiguous-paths in the "Run OpenAPI lint" step; either add a brief inline
comment in that step explaining why the rule is intentionally skipped (e.g.,
documented design decision that /type/{instanceTypeId} and /{instanceId}/... are
distinct resources and cannot be disambiguated by linting), or fix the OpenAPI
spec to remove ambiguity by renaming or restructuring the conflicting paths
(adjust /v2/org/{org}/nico/instance/type/{instanceTypeId} and the
instance-specific paths /v2/org/{org}/nico/instance/{instanceId}/status-history,
/interface, /infiniband-interface, /nvlink-interface so path segments are
uniquely identifiable), and update the workflow to stop skipping
no-ambiguous-paths once resolved.
🪄 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: 4c28a47a-d4f8-415e-a707-185b64e3480e

📥 Commits

Reviewing files that changed from the base of the PR and between 3078fa3 and 9e82783.

📒 Files selected for processing (1)
  • .github/workflows/lint-and-test.yml

Comment on lines 114 to +115
- name: Run OpenAPI lint
run: redocly lint ./openapi/spec.yaml --format=github-actions
run: redocly lint ./openapi/spec.yaml --format=github-actions --skip-rule=no-ambiguous-paths

@coderabbitai coderabbitai Bot May 28, 2026

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.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Identify paths flagged by the no-ambiguous-paths rule

# Run redocly lint without the skip flag to see what it flags
npx `@redocly/cli` lint ./openapi/spec.yaml 2>&1 | grep -i "ambiguous\|no-ambiguous-paths" || echo "No ambiguous paths warnings found"

Repository: NVIDIA/infra-controller-rest

Length of output: 1003


Document why we skip no-ambiguous-paths (or fix the OpenAPI paths)

redocly lint flags real no-ambiguous-paths issues for:

  • /v2/org/{org}/nico/instance/type/{instanceTypeId} vs /v2/org/{org}/nico/instance/{instanceId}/status-history
  • /v2/org/{org}/nico/instance/type/{instanceTypeId} vs /v2/org/{org}/nico/instance/{instanceId}/interface
  • /v2/org/{org}/nico/instance/type/{instanceTypeId} vs /v2/org/{org}/nico/instance/{instanceId}/infiniband-interface
  • /v2/org/{org}/nico/instance/type/{instanceTypeId} vs /v2/org/{org}/nico/instance/{instanceId}/nvlink-interface

Skipping the rule at .github/workflows/lint-and-test.yml (lines 114–115) needs an explicit rationale, or the spec should be refactored to remove the ambiguity.

Suggested inline documentation
      - name: Run OpenAPI lint
-        run: redocly lint ./openapi/spec.yaml --format=github-actions --skip-rule=no-ambiguous-paths
+        # Skip no-ambiguous-paths: [brief explanation of why this rule is not applicable or produces false positives]
+        run: redocly lint ./openapi/spec.yaml --format=github-actions --skip-rule=no-ambiguous-paths
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Run OpenAPI lint
run: redocly lint ./openapi/spec.yaml --format=github-actions
run: redocly lint ./openapi/spec.yaml --format=github-actions --skip-rule=no-ambiguous-paths
- name: Run OpenAPI lint
# Skip no-ambiguous-paths: [brief explanation of why this rule is not applicable or produces false positives]
run: redocly lint ./openapi/spec.yaml --format=github-actions --skip-rule=no-ambiguous-paths
🤖 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 @.github/workflows/lint-and-test.yml around lines 114 - 115, The workflow
currently skips the Redocly rule no-ambiguous-paths in the "Run OpenAPI lint"
step; either add a brief inline comment in that step explaining why the rule is
intentionally skipped (e.g., documented design decision that
/type/{instanceTypeId} and /{instanceId}/... are distinct resources and cannot
be disambiguated by linting), or fix the OpenAPI spec to remove ambiguity by
renaming or restructuring the conflicting paths (adjust
/v2/org/{org}/nico/instance/type/{instanceTypeId} and the instance-specific
paths /v2/org/{org}/nico/instance/{instanceId}/status-history, /interface,
/infiniband-interface, /nvlink-interface so path segments are uniquely
identifiable), and update the workflow to stop skipping no-ambiguous-paths once
resolved.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The ambigous path lint error is raised by redocly linter because it doesn't take into account the UUID format for the param.

/v2/org/{org}/nico/instance/type/{instanceTypeId} differs from /v2/org/{org}/nico/instance/{instanceId}/interface because type is a plain string in the former whereas {instanceId} is a string of UUID format.

Having said that, we do have plans to migrate /v2/org/{org}/nico/instance/type/* to /v2/org/{org}/nico/instance-type/* which can resolve this concern.

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.

@thossain-nv, understood — that is a well-known limitation of structural path-ambiguity analysis in OpenAPI linters. Redocly evaluates path segments purely lexically and has no visibility into format: uuid constraints on path parameters, which means the literal type segment in .../instance/type/{instanceTypeId} is indistinguishable from a UUID-format {instanceId} segment at lint time, producing a false positive.

The planned migration to /v2/org/{org}/nico/instance-type/* is the correct long-term fix, as it eliminates the structural overlap entirely.

In the interim, I would strongly recommend capturing this rationale as an inline comment so future maintainers do not inadvertently remove the skip or miscategorize it as technical debt:

      - name: Run OpenAPI lint
        # Skip no-ambiguous-paths: Redocly does not evaluate parameter format
        # constraints (e.g., format: uuid), so it incorrectly flags
        # /instance/type/{instanceTypeId} (literal 'type' segment) as
        # ambiguous with /instance/{instanceId}/... (UUID-format parameter).
        # This will be resolved when /instance/type/* is migrated to
        # /instance-type/* (tracked in <issue-link>).
        run: redocly lint ./openapi/spec.yaml --format=github-actions --skip-rule=no-ambiguous-paths

Would you like me to open a follow-up issue to track the /instance/type/*/instance-type/* path migration?


✏️ Learnings added
Learnt from: thossain-nv
Repo: NVIDIA/infra-controller-rest PR: 584
File: .github/workflows/lint-and-test.yml:114-115
Timestamp: 2026-05-28T23:29:05.792Z
Learning: In the NVIDIA/infra-controller-rest repository, the Redocly OpenAPI linter rule `no-ambiguous-paths` is intentionally skipped in `.github/workflows/lint-and-test.yml` because Redocly does not account for parameter format constraints (e.g., `format: uuid`). The false positive arises from the structural overlap between `/v2/org/{org}/nico/instance/type/{instanceTypeId}` (literal `type` segment) and `/v2/org/{org}/nico/instance/{instanceId}/...` (UUID-format `{instanceId}` parameter). There is a planned migration of `/v2/org/{org}/nico/instance/type/*` to `/v2/org/{org}/nico/instance-type/*` which will structurally resolve the ambiguity.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

@github-actions

Copy link
Copy Markdown

Test Results

9 643 tests  ±0   9 643 ✅ ±0   7m 9s ⏱️ -29s
  159 suites ±0       0 💤 ±0 
   14 files   ±0       0 ❌ ±0 

Results for commit 9e82783. ± Comparison against base commit c39c263.

@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown

Test Results

9 643 tests  ±0   9 643 ✅ ±0   7m 25s ⏱️ -6s
  159 suites ±0       0 💤 ±0 
   14 files   ±0       0 ❌ ±0 

Results for commit 049168c. ± Comparison against base commit b7b0cc4.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
nico-flow 66 4 34 18 2 8
nico-nsm 82 2 28 43 9 0
nico-psm 67 4 35 18 2 8
nico-rest-api 100 6 53 30 3 8
nico-rest-cert-manager 65 4 34 18 1 8
nico-rest-db 66 4 34 18 2 8
nico-rest-site-agent 65 4 34 18 1 8
nico-rest-site-manager 65 4 34 18 1 8
nico-rest-workflow 67 4 35 18 2 8
TOTAL 643 36 321 199 23 64

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

@thossain-nv
thossain-nv force-pushed the chore/fix-schema branch 2 times, most recently from b83b716 to c16ec90 Compare May 29, 2026 18:44
Signed-off-by: Tareque Hossain <thossain@nvidia.com>
Signed-off-by: Tareque Hossain <thossain@nvidia.com>
Signed-off-by: Tareque Hossain <thossain@nvidia.com>
Signed-off-by: Tareque Hossain <thossain@nvidia.com>
@thossain-nv
thossain-nv enabled auto-merge (squash) May 29, 2026 19:08
@thossain-nv
thossain-nv merged commit 59a3ae8 into main May 29, 2026
98 checks passed
@thossain-nv
thossain-nv deleted the chore/fix-schema branch May 29, 2026 19:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants