Skip to content

fix: Some fields in OpenAPI specs should be listed as nullable#440

Merged
pbreton merged 1 commit intomainfrom
fix/some-fields-should-be-nullable-in-openapi-specs
May 5, 2026
Merged

fix: Some fields in OpenAPI specs should be listed as nullable#440
pbreton merged 1 commit intomainfrom
fix/some-fields-should-be-nullable-in-openapi-specs

Conversation

@pbreton
Copy link
Copy Markdown
Contributor

@pbreton pbreton commented Apr 29, 2026

Description

Some fields in OpenAPI specs are not listed as nullable but can in fact be returned 'null' by the server: this PR fixes it.
Note: the only changes are in openapi/spec.yaml. Everything else is generated or induced (sdk/simple).

Type of Change

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

Services Affected

None

Breaking Changes

  • This PR contains breaking changes

In the generated Go SDK some fields previously pointers are now Nullable objects.

Testing

  • Manual testing performed

@pbreton pbreton requested a review from a team as a code owner April 29, 2026 20:16
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

Walkthrough

Pointer-based optional fields across the OpenAPI schema and generated Go SDK models were converted to nullable wrapper types, enabling explicit three-state semantics (unset, set-to-value, set-to-null) and updating getters, setters, presence checks, and JSON serialization accordingly.

Changes

Cohort / File(s) Summary
OpenAPI Schema
openapi/spec.yaml
Replaced many object $ref properties with oneOf: [<ref>, {type: 'null'}] (and a nullable date-time) to permit explicit null values across stats, summaries, and nested resources.
Core nullable conversions
sdk/standard/model_allocation_constraint.go, sdk/standard/model_audit_entry.go, sdk/standard/model_component_diff.go, sdk/standard/model_expected_machine.go, sdk/standard/model_expected_power_shelf.go, sdk/standard/model_expected_switch.go
Converted pointer fields to generated Nullable* wrappers; updated Get/GetOk/Has/Set semantics; added SetNil/Unset helpers; adjusted ToMap conditional serialization.
DPU extension & observability
sdk/standard/model_dpu_extension_service.go, sdk/standard/model_dpu_extension_service_deployment.go, sdk/standard/model_dpu_extension_service_observability_config.go, sdk/standard/model_dpu_extension_service_version_info.go
Migrated nested observability/version/deployment fields to nullable wrappers; preserved explicit-null vs unset semantics in accessors and JSON mapping.
Site & site-stats models
sdk/standard/model_site.go, sdk/standard/model_site_summary.go, sdk/standard/model_site_machine_stats.go, sdk/standard/model_site_machine_stats_by_status_and_health.go
Replaced multiple site-related pointer fields and nested breakdowns with Nullable* types; added nil/unset controls and updated serialization to include fields only when wrappers are set.
Infrastructure & tenant stats
sdk/standard/model_infrastructure_provider_stats.go, sdk/standard/model_tenant_stats.go
Converted various count/breakdown fields (Machine, IpBlock, TenantAccount, Instance, Vpc, Subnet) to nullable wrappers with corresponding accessor and ToMap changes.
Instance / InstanceType / stats
sdk/standard/model_instance.go, sdk/standard/model_instance_type.go, sdk/standard/model_instance_type_stats.go, sdk/standard/model_machine_instance_type_stats.go, sdk/standard/model_machine_instance_type_summary.go
Converted allocation/usage/status breakdowns and propagation details to nullable wrappers; adjusted labels handling and serialization gating.
Machine, metadata & tray/rack
sdk/standard/model_machine.go, sdk/standard/model_machine_metadata.go, sdk/standard/model_tray.go, sdk/standard/model_rack.go
Migrated Health, Metadata, DMI/BMC info, Tray position, and Rack location to nullable wrappers; added explicit-nil and unset methods; updated getters and ToMap logic.
IP block, SKU & components
sdk/standard/model_ip_block.go, sdk/standard/model_sku.go, sdk/standard/model_sku_components.go
Changed UsageStats, Components, Chassis to Nullable* wrappers; updated presence detection and JSON output to use wrapper Get()/IsSet().
Diverse resource models
sdk/standard/model_vpc.go, sdk/standard/model_tenant_account.go, sdk/standard/model_sku_components.go
Converted various resource-related optional fields to nullable wrappers and added helper methods for explicit-null/unset handling.
Widefile/consistency edits
multiple sdk/standard/*.go files (30+)
Consistent pattern applied across ~30+ generated model files: pointer → Nullable wrapper conversions, updated accessors (Get/GetOk/Has/Set), new SetNil/Unset helpers, and ToMap conditional serialization changes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: marking fields in OpenAPI specs as nullable. It is concise, specific, and directly reflects the primary objective of the PR.
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.
Description check ✅ Passed The PR description clearly articulates that OpenAPI specs are being updated to mark fields as nullable when servers can return null values, with induced SDK changes.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/some-fields-should-be-nullable-in-openapi-specs

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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-04-29 20:17:16 UTC | Commit: 93c3b41

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 29, 2026

Test Results

9 051 tests  ±0   9 051 ✅ ±0   7m 27s ⏱️ +24s
  151 suites ±0       0 💤 ±0 
   14 files   ±0       0 ❌ ±0 

Results for commit da03ff2. ± Comparison against base commit 20e7305.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
sdk/standard/model_expected_power_shelf.go (1)

645-650: ⚠️ Potential issue | 🟡 Minor

Fix GetLabelsOk documentation/behavior mismatch.

At Line 645, the comment says explicit nil returns (nil, true), but the function returns false when Labels is nil. This is misleading for SDK consumers.

Proposed minimal alignment fix
-// NOTE: If the value is an explicit nil, `nil, true` will be returned
+// NOTE: For this map field, nil is treated as not set.
 func (o *ExpectedPowerShelf) GetLabelsOk() (map[string]string, bool) {
 	if o == nil || IsNil(o.Labels) {
-		return map[string]string{}, false
+		return nil, false
 	}
 	return o.Labels, true
 }

As per coding guidelines: **/*.go: review Go code against clean code and expressiveness.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@sdk/standard/model_expected_power_shelf.go` around lines 645 - 650, The
comment for GetLabelsOk is incorrect: it claims an explicit nil returns (nil,
true) but the implementation checks o == nil || IsNil(o.Labels) and returns an
empty map and false. Update the comment to reflect the actual behavior of
GetLabelsOk (i.e., when o is nil or Labels is nil it returns an empty map and
false), or alternatively change the function to return (map[string]string(nil),
true) when Labels is explicitly nil; adjust either the doc or the IsNil branch
in GetLabelsOk accordingly and ensure references to IsNil(o.Labels) and the
return values are consistent.
sdk/standard/model_expected_switch.go (1)

589-606: ⚠️ Potential issue | 🟠 Major

Labels field lacks proper nullable support despite misleading documentation.

The GetLabelsOk() method's comment at line 600 incorrectly documents its behavior. It states "If the value is an explicit nil, nil, true will be returned," but the actual implementation at lines 602–603 returns (map[string]string{}, false) when the field is nil. This violates the stated contract and creates API ambiguity.

Additionally, Labels is inconsistently modeled compared to other optional fields in the struct. Fields like RackId, Name, and Manufacturer use Nullable wrappers (lines 32–48), which properly distinguish between unset and explicitly null values. The Labels field, however, uses a plain map[string]string (line 50), preventing the distinction. The ToMap() method at lines 735–737 uses if o.Labels != nil, which cannot emit "labels": null in JSON serialization.

Since this is generated code, the root cause lies in the OpenAPI schema or code generator configuration. Ensure the generator produces a NullableMap wrapper for the Labels field to align with the struct's nullable semantics and the documented API contract.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@sdk/standard/model_expected_switch.go` around lines 589 - 606, The Labels
field on ExpectedSwitch is not using a nullable wrapper so GetLabelsOk and JSON
serialization cannot distinguish unset vs explicit null; update the
ExpectedSwitch struct to use the generated nullable map wrapper (e.g.,
NullableMap[string]string or the project's NullableMapStringString type) for the
Labels field, then update GetLabels, GetLabelsOk, Set/Unset helpers and the
ToMap() logic to use that nullable wrapper's IsSet/IsNil/Get/Set semantics
(adjust references in methods GetLabels, GetLabelsOk, and ToMap) so that
GetLabelsOk returns (nil, true) when the wrapper is explicitly null and ToMap
emits "labels": null when appropriate.
sdk/standard/model_machine.go (1)

701-717: ⚠️ Potential issue | 🟠 Major

Labels field does not support explicit-null semantics despite the comments promising it.

The OpenAPI schema marks labels as nullable (oneOf with type: 'null', identical to health and metadata), yet the generated Go code does not handle this correctly. The comment at line 712 states "If the value is an explicit nil, nil, true will be returned," but GetLabelsOk() returns map[string]string{}, false instead. Additionally, ToMap() omits the field when nil rather than explicitly serializing null.

Since labels uses a plain map[string]string—not a nullable wrapper type like Health and Metadata—it cannot distinguish explicit null from unset. Either refactor labels to use the same nullable wrapper pattern as the other nullable fields in this PR, or revert the comments to remove the false guarantees of explicit-null handling.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@sdk/standard/model_machine.go` around lines 701 - 717, The comments promise
explicit-null semantics for the Labels field but the code uses plain
map[string]string so GetLabelsOk, GetLabels and ToMap cannot represent an
explicit null; fix by making Labels follow the same nullable-wrapper pattern
used for Health and Metadata (e.g., replace the Machine.Labels plain map type
with the same Nullable wrapper type used for Health/Metadata, add or reuse a
NullableMapStringString type with IsSet()/Get()/Set()/Unset()/IsNil()
semantics), then update GetLabels, GetLabelsOk, SetLabels/SetLabelsNil (or
equivalent) to use the wrapper and change Machine.ToMap to serialize explicit
null when the wrapper is set to nil; alternatively, if you prefer not to support
explicit-null, update the comments on GetLabelsOk to remove the claim about
explicit-nil handling and ensure ToMap omits the field consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@openapi/spec.yaml`:
- Line 12357: The description string in openapi/spec.yaml contains a double
space ("registration token  expires"); update the description value to fix the
spacing and grammar so it reads "Date/time when registration token expires.
Value only exposed to Provider" by removing the extra space between "token" and
"expires" in the description field.
- Around line 17908-17910: The Labels field is declared nullable in the OpenAPI
spec but the SDK uses a plain map[string]string, losing explicit null vs unset
semantics; change the Labels property in affected models (e.g., the model
containing Labels and ExpectedSwitch) to use a NullableLabels wrapper type
(create NullableLabels if missing) and implement the same API pattern used by
other nullable wrappers: methods IsSet(), Get(), Set(map[string]string),
Unset(), plus proper JSON marshal/unmarshal behavior; update all
serialization/deserialization checks (replace direct nil checks like o.Labels !=
nil) to use the NullableLabels.IsSet()/Get() pattern so explicit null, unset,
and set states match the spec.

In `@sdk/standard/model_infrastructure_provider_stats.go`:
- Around line 22-24: The change replaced exported pointer-typed fields on the
InfrastructureProviderStats struct (Machine, IpBlock, TenantAccount) with new
Nullable... wrapper types, which is a breaking API change; restore backward
compatibility by keeping the original exported fields (Machine
*MachineCountByStatus, IpBlock *IpBlockCountByStatus, TenantAccount
*TenantAccountCountByStatus) while introducing the nullable wrappers as either
new fields (e.g., MachineNullable, IpBlockNullable, TenantAccountNullable) or by
adding accessor methods that map between the
NullableMachineCountByStatus/NullableIpBlockCountByStatus/NullableTenantAccountCountByStatus
types and the original pointer types; ensure JSON tags and
marshalling/unmarshalling logic preserve the same serialized shape and
deprecation comments are added to the old fields or accessors to guide future
migration.

---

Outside diff comments:
In `@sdk/standard/model_expected_power_shelf.go`:
- Around line 645-650: The comment for GetLabelsOk is incorrect: it claims an
explicit nil returns (nil, true) but the implementation checks o == nil ||
IsNil(o.Labels) and returns an empty map and false. Update the comment to
reflect the actual behavior of GetLabelsOk (i.e., when o is nil or Labels is nil
it returns an empty map and false), or alternatively change the function to
return (map[string]string(nil), true) when Labels is explicitly nil; adjust
either the doc or the IsNil branch in GetLabelsOk accordingly and ensure
references to IsNil(o.Labels) and the return values are consistent.

In `@sdk/standard/model_expected_switch.go`:
- Around line 589-606: The Labels field on ExpectedSwitch is not using a
nullable wrapper so GetLabelsOk and JSON serialization cannot distinguish unset
vs explicit null; update the ExpectedSwitch struct to use the generated nullable
map wrapper (e.g., NullableMap[string]string or the project's
NullableMapStringString type) for the Labels field, then update GetLabels,
GetLabelsOk, Set/Unset helpers and the ToMap() logic to use that nullable
wrapper's IsSet/IsNil/Get/Set semantics (adjust references in methods GetLabels,
GetLabelsOk, and ToMap) so that GetLabelsOk returns (nil, true) when the wrapper
is explicitly null and ToMap emits "labels": null when appropriate.

In `@sdk/standard/model_machine.go`:
- Around line 701-717: The comments promise explicit-null semantics for the
Labels field but the code uses plain map[string]string so GetLabelsOk, GetLabels
and ToMap cannot represent an explicit null; fix by making Labels follow the
same nullable-wrapper pattern used for Health and Metadata (e.g., replace the
Machine.Labels plain map type with the same Nullable wrapper type used for
Health/Metadata, add or reuse a NullableMapStringString type with
IsSet()/Get()/Set()/Unset()/IsNil() semantics), then update GetLabels,
GetLabelsOk, SetLabels/SetLabelsNil (or equivalent) to use the wrapper and
change Machine.ToMap to serialize explicit null when the wrapper is set to nil;
alternatively, if you prefer not to support explicit-null, update the comments
on GetLabelsOk to remove the claim about explicit-nil handling and ensure ToMap
omits the field consistently.
🪄 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: bd493ebd-172a-40b1-ba62-66746652c58e

📥 Commits

Reviewing files that changed from the base of the PR and between 33ddf04 and 93c3b41.

📒 Files selected for processing (32)
  • docs/index.html
  • openapi/spec.yaml
  • sdk/standard/model_allocation_constraint.go
  • sdk/standard/model_audit_entry.go
  • sdk/standard/model_component_diff.go
  • sdk/standard/model_dpu_extension_service.go
  • sdk/standard/model_dpu_extension_service_deployment.go
  • sdk/standard/model_dpu_extension_service_observability_config.go
  • sdk/standard/model_dpu_extension_service_version_info.go
  • sdk/standard/model_expected_machine.go
  • sdk/standard/model_expected_power_shelf.go
  • sdk/standard/model_expected_switch.go
  • sdk/standard/model_infrastructure_provider_stats.go
  • sdk/standard/model_instance.go
  • sdk/standard/model_instance_type.go
  • sdk/standard/model_instance_type_stats.go
  • sdk/standard/model_ip_block.go
  • sdk/standard/model_machine.go
  • sdk/standard/model_machine_instance_type_stats.go
  • sdk/standard/model_machine_instance_type_summary.go
  • sdk/standard/model_machine_metadata.go
  • sdk/standard/model_rack.go
  • sdk/standard/model_site.go
  • sdk/standard/model_site_machine_stats.go
  • sdk/standard/model_site_machine_stats_by_status_and_health.go
  • sdk/standard/model_site_summary.go
  • sdk/standard/model_sku.go
  • sdk/standard/model_sku_components.go
  • sdk/standard/model_tenant_account.go
  • sdk/standard/model_tenant_stats.go
  • sdk/standard/model_tray.go
  • sdk/standard/model_vpc.go

Comment thread openapi/spec.yaml Outdated
Comment thread openapi/spec.yaml Outdated
Comment thread sdk/standard/model_infrastructure_provider_stats.go Outdated
@pbreton pbreton force-pushed the fix/some-fields-should-be-nullable-in-openapi-specs branch from 93c3b41 to 7ebba12 Compare April 29, 2026 22:51
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
sdk/standard/model_expected_power_shelf.go (1)

634-650: ⚠️ Potential issue | 🟠 Major

The generated labels field cannot preserve the explicit-null state declared in the OpenAPI spec.

The OpenAPI spec correctly declares labels as nullable (oneOf: [#/components/schemas/Labels, null]), but the generated SDK implements Labels as a plain map[string]string, which cannot distinguish between an omitted field and an explicit null value. Consequently:

  • GetLabelsOk() returns (map[string]string{}, false) whenever o.Labels is nil, conflating unset and null.
  • ToMap() omits the labels property entirely when o.Labels == nil, rather than serializing null.

This discrepancy between spec and SDK is a code generation artifact that should be addressed in the OpenAPI generator configuration (e.g., by using a nullable wrapper type for labels instead of a plain map). The spec itself is correct and should not be modified.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@sdk/standard/model_expected_power_shelf.go` around lines 634 - 650, The
Labels field on ExpectedPowerShelf is generated as plain map[string]string so it
cannot represent explicit null from the OpenAPI oneOf (Labels|null); update the
model to use a nullable wrapper (e.g., a nullable/optional type or
*map[string]string with a boolean "isSet" or a generated NullableLabels type)
and change the accessor/serializer functions (GetLabels, GetLabelsOk, and ToMap)
to use that wrapper: ensure GetLabelsOk returns (nil, true) for explicit null,
returns (map, true) for set value, and returns (nil, false) for unset, and
ensure ToMap serializes labels as null when the wrapper indicates explicit null
and omits the key when unset.
sdk/standard/model_expected_switch.go (1)

589-606: ⚠️ Potential issue | 🟠 Major

Labels field type cannot represent the three-state nullable semantics specified in the OpenAPI schema.

Across the SDK, all Labels fields are declared as plain map[string]string, not as a nullable wrapper type. Go's JSON unmarshaling collapses both omitted fields and explicit null to nil, making it impossible to distinguish them at runtime. The GetLabelsOk() accessor returns false when Labels is nil, which prevents representing the explicit-null case. Furthermore, the JSON serialization layer cannot emit an explicit null for a plain map field.

This type mismatch affects all model types with Labels fields (ExpectedSwitch, ExpectedPowerShelf, ExpectedMachine, Machine, Instance, InfiniBandPartition, NetworkSecurityGroup, Vpc, and others). The fix requires updating the OpenAPI Generator configuration to emit nullable wrapper types (e.g., NullableMap[string]string) and corresponding state-management accessors (SetLabelsNil, UnsetLabels), not changes to the OpenAPI specification.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@sdk/standard/model_expected_switch.go` around lines 589 - 606, The Labels
fields use plain map[string]string which cannot represent omitted vs
explicit-null; update the generated model types (e.g., ExpectedSwitch) so Labels
is a nullable wrapper type (e.g., NullableMap[string]string) instead of
map[string]string, update accessors like GetLabels, GetLabelsOk and IsNil checks
to use the wrapper API (return nil/true for explicit-null via wrapper, and
provide Unset/SetNil semantics), and add the state-management methods
SetLabelsNil and UnsetLabels (and corresponding changes in ExpectedPowerShelf,
ExpectedMachine, Machine, Instance, InfiniBandPartition, NetworkSecurityGroup,
Vpc, etc.); implement JSON marshal/unmarshal to emit explicit null when the
wrapper is in nil state.
🧹 Nitpick comments (1)
sdk/standard/model_sku.go (1)

195-236: 🏗️ Heavy lift

Add regression coverage for unset vs explicit-null JSON behavior.

This change is part of the new public three-state nullable contract, but the PR only mentions manual testing. A small table-driven SDK test covering unset, explicit null, and populated components would materially reduce the risk of future generator regressions here and in similar models.

Also applies to: 324-326

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@sdk/standard/model_sku.go` around lines 195 - 236, Add a table-driven unit
test that exercises the three-state nullable behavior for the Components field
on Sku: cover (1) unset (field omitted), (2) explicit JSON null, and (3)
populated SkuComponents; assert JSON round-trip and behavior of GetComponents,
GetComponentsOk, HasComponents, SetComponentsNil, and UnsetComponents to ensure
they return/flag nil vs set correctly (e.g., GetComponentsOk returns (nil,true)
for explicit null and (nil,false) for unset). Target test helpers around the Sku
methods: GetComponents, GetComponentsOk, HasComponents, SetComponentsNil,
UnsetComponents and verify marshalling/unmarshalling produces the expected JSON
and method results for each case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@openapi/spec.yaml`:
- Around line 19804-19814: The prometheus and logging branches under the
observability schema are incorrectly marked nullable (they include "type:
'null'"), allowing payloads like { prometheus: null } to satisfy the oneOf;
remove the nullable alternatives from the prometheus and logging branches so the
schema only references the concrete components
(DpuExtensionServiceObservabilityPrometheus and
DpuExtensionServiceObservabilityLogging) and cannot be satisfied by null, and if
nullability is needed for responses make the parent observability field nullable
where referenced or create separate request/response schemas instead of adding
"type: 'null'" to the prometheus/logging branches.

---

Outside diff comments:
In `@sdk/standard/model_expected_power_shelf.go`:
- Around line 634-650: The Labels field on ExpectedPowerShelf is generated as
plain map[string]string so it cannot represent explicit null from the OpenAPI
oneOf (Labels|null); update the model to use a nullable wrapper (e.g., a
nullable/optional type or *map[string]string with a boolean "isSet" or a
generated NullableLabels type) and change the accessor/serializer functions
(GetLabels, GetLabelsOk, and ToMap) to use that wrapper: ensure GetLabelsOk
returns (nil, true) for explicit null, returns (map, true) for set value, and
returns (nil, false) for unset, and ensure ToMap serializes labels as null when
the wrapper indicates explicit null and omits the key when unset.

In `@sdk/standard/model_expected_switch.go`:
- Around line 589-606: The Labels fields use plain map[string]string which
cannot represent omitted vs explicit-null; update the generated model types
(e.g., ExpectedSwitch) so Labels is a nullable wrapper type (e.g.,
NullableMap[string]string) instead of map[string]string, update accessors like
GetLabels, GetLabelsOk and IsNil checks to use the wrapper API (return nil/true
for explicit-null via wrapper, and provide Unset/SetNil semantics), and add the
state-management methods SetLabelsNil and UnsetLabels (and corresponding changes
in ExpectedPowerShelf, ExpectedMachine, Machine, Instance, InfiniBandPartition,
NetworkSecurityGroup, Vpc, etc.); implement JSON marshal/unmarshal to emit
explicit null when the wrapper is in nil state.

---

Nitpick comments:
In `@sdk/standard/model_sku.go`:
- Around line 195-236: Add a table-driven unit test that exercises the
three-state nullable behavior for the Components field on Sku: cover (1) unset
(field omitted), (2) explicit JSON null, and (3) populated SkuComponents; assert
JSON round-trip and behavior of GetComponents, GetComponentsOk, HasComponents,
SetComponentsNil, and UnsetComponents to ensure they return/flag nil vs set
correctly (e.g., GetComponentsOk returns (nil,true) for explicit null and
(nil,false) for unset). Target test helpers around the Sku methods:
GetComponents, GetComponentsOk, HasComponents, SetComponentsNil, UnsetComponents
and verify marshalling/unmarshalling produces the expected JSON and method
results for each case.
🪄 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: aafe4239-2e66-4c5d-b825-97b3d8706c10

📥 Commits

Reviewing files that changed from the base of the PR and between 93c3b41 and 7ebba12.

📒 Files selected for processing (32)
  • docs/index.html
  • openapi/spec.yaml
  • sdk/standard/model_allocation_constraint.go
  • sdk/standard/model_audit_entry.go
  • sdk/standard/model_component_diff.go
  • sdk/standard/model_dpu_extension_service.go
  • sdk/standard/model_dpu_extension_service_deployment.go
  • sdk/standard/model_dpu_extension_service_observability_config.go
  • sdk/standard/model_dpu_extension_service_version_info.go
  • sdk/standard/model_expected_machine.go
  • sdk/standard/model_expected_power_shelf.go
  • sdk/standard/model_expected_switch.go
  • sdk/standard/model_infrastructure_provider_stats.go
  • sdk/standard/model_instance.go
  • sdk/standard/model_instance_type.go
  • sdk/standard/model_instance_type_stats.go
  • sdk/standard/model_ip_block.go
  • sdk/standard/model_machine.go
  • sdk/standard/model_machine_instance_type_stats.go
  • sdk/standard/model_machine_instance_type_summary.go
  • sdk/standard/model_machine_metadata.go
  • sdk/standard/model_rack.go
  • sdk/standard/model_site.go
  • sdk/standard/model_site_machine_stats.go
  • sdk/standard/model_site_machine_stats_by_status_and_health.go
  • sdk/standard/model_site_summary.go
  • sdk/standard/model_sku.go
  • sdk/standard/model_sku_components.go
  • sdk/standard/model_tenant_account.go
  • sdk/standard/model_tenant_stats.go
  • sdk/standard/model_tray.go
  • sdk/standard/model_vpc.go
🚧 Files skipped from review as they are similar to previous changes (6)
  • sdk/standard/model_tray.go
  • sdk/standard/model_dpu_extension_service_version_info.go
  • sdk/standard/model_allocation_constraint.go
  • sdk/standard/model_machine.go
  • sdk/standard/model_tenant_stats.go
  • sdk/standard/model_expected_machine.go

Comment thread openapi/spec.yaml
Comment on lines 19804 to 19814
prometheus:
$ref: '#/components/schemas/DpuExtensionServiceObservabilityPrometheus'
oneOf:
- $ref: '#/components/schemas/DpuExtensionServiceObservabilityPrometheus'
- type: 'null'
logging:
$ref: '#/components/schemas/DpuExtensionServiceObservabilityLogging'
oneOf:
- $ref: '#/components/schemas/DpuExtensionServiceObservabilityLogging'
- type: 'null'
oneOf:
- required:
- prometheus
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 | 🏗️ Heavy lift

Keep the observability backend branches non-nullable.

This schema already uses a top-level oneOf to require exactly one concrete backend. Making prometheus / logging nullable means payloads like { prometheus: null } now satisfy that contract, so the spec starts accepting an empty observability config.

If only responses need to emit null, keep the parent observability field nullable where it is referenced, or split request/response schemas instead of weakening the branch definitions here.

Suggested schema adjustment
        prometheus:
-          oneOf:
-            - $ref: '#/components/schemas/DpuExtensionServiceObservabilityPrometheus'
-            - type: 'null'
+          $ref: '#/components/schemas/DpuExtensionServiceObservabilityPrometheus'
        logging:
-          oneOf:
-            - $ref: '#/components/schemas/DpuExtensionServiceObservabilityLogging'
-            - type: 'null'
+          $ref: '#/components/schemas/DpuExtensionServiceObservabilityLogging'

As per coding guidelines, openapi/spec.yaml should be reviewed for consistency and correctness.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@openapi/spec.yaml` around lines 19804 - 19814, The prometheus and logging
branches under the observability schema are incorrectly marked nullable (they
include "type: 'null'"), allowing payloads like { prometheus: null } to satisfy
the oneOf; remove the nullable alternatives from the prometheus and logging
branches so the schema only references the concrete components
(DpuExtensionServiceObservabilityPrometheus and
DpuExtensionServiceObservabilityLogging) and cannot be satisfied by null, and if
nullability is needed for responses make the parent observability field nullable
where referenced or create separate request/response schemas instead of adding
"type: 'null'" to the prometheus/logging branches.

@pbreton pbreton marked this pull request as draft April 29, 2026 23:00
@copy-pr-bot
Copy link
Copy Markdown

copy-pr-bot Bot commented Apr 29, 2026

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.

@pbreton pbreton force-pushed the fix/some-fields-should-be-nullable-in-openapi-specs branch from 7ebba12 to 99b4fe6 Compare May 5, 2026 17:19
@pbreton pbreton marked this pull request as ready for review May 5, 2026 17:20
@pbreton pbreton force-pushed the fix/some-fields-should-be-nullable-in-openapi-specs branch from 99b4fe6 to 8477d3c Compare May 5, 2026 18:32
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 5, 2026

🔍 Container Scan Summary

Service Total Critical High Medium Low Other
nico-nsm 64 2 20 33 9 0
nico-psm 56 4 29 13 2 8
nico-rest-api 57 4 30 13 2 8
nico-rest-cert-manager 54 4 28 13 1 8
nico-rest-db 55 4 28 13 2 8
nico-rest-site-agent 54 4 28 13 1 8
nico-rest-site-manager 54 4 28 13 1 8
nico-rest-workflow 56 4 29 13 2 8
nico-rla 55 4 28 13 2 8
TOTAL 505 34 248 137 22 64

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

@pbreton pbreton force-pushed the fix/some-fields-should-be-nullable-in-openapi-specs branch 2 times, most recently from 6848430 to 56b4e64 Compare May 5, 2026 21:32
Signed-off-by: Patrice Breton <pbreton@nvidia.com>
@pbreton pbreton force-pushed the fix/some-fields-should-be-nullable-in-openapi-specs branch from 56b4e64 to da03ff2 Compare May 5, 2026 21:50
Copy link
Copy Markdown
Contributor

@thossain-nv thossain-nv left a comment

Choose a reason for hiding this comment

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

Looks good, thank you for this @pbreton

@pbreton pbreton merged commit fbd95ed into main May 5, 2026
104 checks passed
@pbreton pbreton deleted the fix/some-fields-should-be-nullable-in-openapi-specs branch May 5, 2026 22:06
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