Add support for create api when there is no specifying id #11020
Conversation
yujin-emma
requested review from
AMoo-Miki,
BionIT,
Flyingliuhub,
LDrago27,
Maosaic,
SuZhou-Joe,
ZilongX,
abbyhu2000,
ananzh,
angle943,
ashwin-pc,
bandinib-amzn,
curq,
d-buckner,
huyaboo,
joshuali925,
joshuarrrr,
kavilla,
manasvinibs,
mengweieric,
ps48,
raintygao,
ruanyl,
sejli,
virajsanghvi,
wanglam,
xinruiba,
zengyan-amazon,
zhongnansu and
zhyuanqi
as code owners
yujin-emma
requested review from
TackAdam,
paulstn,
ruchidh and
sumukhswamy
as code owners
❌ Changelog Entry Missing HyphenChangelog entries must begin with a hyphen (-). |
WalkthroughAdded top-level OpenAPI components (path parameters and schemas), replaced inline request/response definitions with component references, introduced POST /api/saved_objects (create without ID), updated POST /api/saved_objects/{type}/{id} summary and requestBody, expanded error responses, and removed a legacy inline components block. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
❌ Changelog Entry Missing HyphenChangelog entries must begin with a hyphen (-). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/openapi/saved_objects/saved_objects.yml (3)
178-179: Standardize path parameter ordering across endpoints.The GET endpoint at line 178-179 orders parameters as
[id, type], but the POST (lines 198-199), PUT (lines 316-317), and DELETE (lines 362-363) endpoints use[type, id]. Standardize the parameter order across all endpoints for consistency.parameters: - - $ref: '#/components/parameters/id' - - $ref: '#/components/parameters/type' + - $ref: '#/components/parameters/type' + - $ref: '#/components/parameters/id'
146-146: Remove trailing whitespace.Lines 146 and 285 have trailing spaces after
type: 'visualization'. Clean up formatting for consistency.Also applies to: 285-285
82-102: Consider addingmaxItemsconstraints to array properties.The static analysis tool flagged that
references(lines 82-94) andworkspaces(lines 98-102) arrays lackmaxItemsconstraints. While not strictly required, consider defining reasonable limits to set API expectations.references: description: List of objects that describe other saved objects the created object references. type: array + maxItems: 100 items: type: object properties: id: type: string name: type: string type: type: string workspaces: type: array + maxItems: 50 items: type: string description: Workspaces that this saved object exists in.This is optional and depends on your API's design constraints.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/openapi/saved_objects/saved_objects.yml(5 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
docs/openapi/saved_objects/saved_objects.yml
[medium] 82-94: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: linkchecker
- GitHub Check: lighthouse
🔇 Additional comments (2)
docs/openapi/saved_objects/saved_objects.yml (2)
54-171: New create-without-ID endpoint looks good.The new POST endpoint for creating saved objects without specifying an ID is well-structured with proper request/response schemas, examples, and error handling (200, 400, 409, 500 responses). The parameter reference to the reusable
typecomponent is correct.
193-310: Create-with-ID endpoint properly updated.The endpoint now uses standardized parameter references and includes comprehensive error responses (409 for conflicts, 500 for server errors). The addition of
409and500responses improves API documentation completeness.
yujin-emma
force-pushed
the
no-id-support
branch
from
bccf8aa to
2484c2a
Compare
yujin-emma
force-pushed
the
no-id-support
branch
from
f9f98fb to
efc27f2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/openapi/saved_objects/saved_objects.yml (1)
180-181: Fix parameter order inconsistency in GET endpoint.The GET endpoint lists parameters in order
[id, type], while the path template is/api/saved_objects/{type}/{id}and all other endpoints (POST, PUT, DELETE) use[type, id]order. Reorder for consistency.parameters: - - $ref: '#/components/parameters/id' - $ref: '#/components/parameters/type' + - $ref: '#/components/parameters/id'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/openapi/saved_objects/saved_objects.yml(6 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
docs/openapi/saved_objects/saved_objects.yml
[medium] 47-59: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lighthouse
🔇 Additional comments (3)
docs/openapi/saved_objects/saved_objects.yml (3)
87-173: LGTM!The new POST /api/saved_objects/{type} endpoint is well-designed. It correctly leverages the reusable
SavedObjectCreateRequestschema, includes comprehensive examples, and provides harmonized responses (200, 400, 409, 500). The endpoint properly addresses the PR objective of supporting object creation without an explicit ID.
195-281: LGTM!The POST /api/saved_objects/{type}/{id} endpoint has been properly updated. The summary clarifies it creates objects "with a specific ID," parameters use absolute component references, and the request body correctly references
SavedObjectCreateRequest. The addition of 409 and 500 response handlers improves error coverage. The past refactoring objective to extract duplicate schemas has been successfully applied here.
282-358: LGTM!PUT and DELETE endpoints have been correctly updated to use absolute parameter references (
$ref: '#/components/parameters/type'and$ref: '#/components/parameters/id'). The parameter order is consistent with the path template and aligns with other endpoints.
| references: | ||
| description: List of objects that describe other saved objects the created object references. | ||
| type: array | ||
| items: | ||
| type: object | ||
| properties: | ||
| id: | ||
| type: string | ||
| name: | ||
| type: string | ||
| type: | ||
| type: string |
There was a problem hiding this comment.
Add maxItems constraint to the references array.
Checkov flags that arrays should have a maximum number of items defined. The references array lacks a maxItems constraint, which could allow unbounded arrays in requests.
Consider adding an upper bound to prevent abuse:
references:
description: List of objects that describe other saved objects the created object references.
type: array
+ maxItems: 100
items:
type: objectThis ensures the API is more resilient against potentially malicious or accidental requests with excessive references.
📝 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.
| references: | |
| description: List of objects that describe other saved objects the created object references. | |
| type: array | |
| items: | |
| type: object | |
| properties: | |
| id: | |
| type: string | |
| name: | |
| type: string | |
| type: | |
| type: string | |
| references: | |
| description: List of objects that describe other saved objects the created object references. | |
| type: array | |
| maxItems: 100 | |
| items: | |
| type: object | |
| properties: | |
| id: | |
| type: string | |
| name: | |
| type: string | |
| type: | |
| type: string |
🤖 Prompt for AI Agents
In docs/openapi/saved_objects/saved_objects.yml around lines 46 to 57, the
`references` array schema is missing a `maxItems` constraint; add a sensible
upper bound (for example `maxItems: 100` or another project-appropriate limit)
directly under the `references:` definition so the YAML defines both `type:
array`, `items: ...`, and `maxItems: <limit>` to prevent unbounded arrays in
requests and update any related validation/tests accordingly.
Signed-off-by: Yu Jin <112784385+yujin-emma@users.noreply.github.com>
SuZhou-Joe
force-pushed
the
no-id-support
branch
from
8a958b1 to
426e89b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
changelogs/fragments/11020.yml(1 hunks)docs/openapi/saved_objects/saved_objects.yml(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- changelogs/fragments/11020.yml
🧰 Additional context used
🪛 Checkov (3.2.334)
docs/openapi/saved_objects/saved_objects.yml
[medium] 47-59: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lighthouse
- GitHub Check: bundle-analyzer
- GitHub Check: linkchecker
🔇 Additional comments (3)
docs/openapi/saved_objects/saved_objects.yml (3)
12-85: ✓ Good refactoring: extracted reusable components and consolidated parameter definitions.The extraction of
SavedObjectCreateRequestand400_bad_requestschemas into top-level components eliminates duplication and improves maintainability. Parameter definitions (type,id) are now properly centralized.
87-173: ✓ Consistent and well-structured endpoints with proper schema references.Both POST endpoints correctly use absolute
$refpaths for parameters and the sharedSavedObjectCreateRequestschema. The consolidation improves maintainability and prevents schema drift.Also applies to: 195-281
287-288: ✓ Consistent use of absolute parameter references across PUT and DELETE methods.All parameter references now use the shared component definitions. Good consistency across the API specification.
Also applies to: 333-334
| references: | ||
| description: List of objects that describe other saved objects the created object references. | ||
| type: array | ||
| items: | ||
| type: object | ||
| properties: | ||
| id: | ||
| type: string | ||
| name: | ||
| type: string | ||
| type: | ||
| type: string | ||
| initialNamespaces: | ||
| description: Namespaces that this saved object exists in. This attribute is only used for multi-namespace saved object types. | ||
| type: array | ||
| items: | ||
| type: string | ||
| workspaces: | ||
| type: array | ||
| items: | ||
| type: string | ||
| description: Workspaces that this saved object exists in. |
There was a problem hiding this comment.
Add maxItems constraints to unbounded arrays in SavedObjectCreateRequest.
The references, initialNamespaces, and workspaces arrays lack maxItems constraints. This allows unbounded arrays in requests, which can impact resilience and security. This concern was previously flagged but appears not to have been addressed.
Apply this diff to add sensible upper bounds:
references:
description: List of objects that describe other saved objects the created object references.
type: array
+ maxItems: 100
items:
type: object
properties:
id:
type: string
name:
type: string
type:
type: string
initialNamespaces:
description: Namespaces that this saved object exists in. This attribute is only used for multi-namespace saved object types.
type: array
+ maxItems: 50
items:
type: string
workspaces:
type: array
+ maxItems: 50
items:
type: string
description: Workspaces that this saved object exists in.Adjust the limits (100, 50) based on project requirements.
🧰 Tools
🪛 Checkov (3.2.334)
[medium] 47-59: Ensure that arrays have a maximum number of items
(CKV_OPENAPI_21)
🤖 Prompt for AI Agents
In docs/openapi/saved_objects/saved_objects.yml around lines 46 to 67, the
arrays references, initialNamespaces, and workspaces are unbounded; add maxItems
to each to limit request size (suggest maxItems: 100 for references and
maxItems: 50 for initialNamespaces and workspaces or adjust to project needs),
keep existing types/descriptions intact, and update any schema documentation or
tests that assert array sizes accordingly.
huyaboo
left a comment
huyaboo
left a comment
There was a problem hiding this comment.
I saw in the screenshot the version is still tagged as 3.1.0. We should change that to 3.4.0 (the grey tag not the OAS tag)
5 participants
Description
Added reusable components (type/id parameters, 400 bad request schema); introduced creation endpoint without explicit ID and updated create-with-ID flow; expanded and harmonized response schemas (200, 400, 409, 500) across endpoints; corrected parameter references from local to absolute paths (#/components/...); consolidated document structure by removing legacy components block; applied formatting and typing refinements throughout.
Add support for create api when there is no specifying id
Issues Resolved
Add support for create api when there is no specifying id
Screenshot
Testing the changes
Changelog
Check List
yarn test:jestyarn test:jest_integrationSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.