-
Notifications
You must be signed in to change notification settings - Fork 69
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix cozystack-api to show correct List types in openapi #542
Conversation
WalkthroughThe pull request involves two main changes: updating the Docker image reference for the CozyStack API from a specific version to the latest tag, and modifying the OpenAPI schema generation logic in the server start command. The image update changes the SHA256 digest, indicating a new underlying image, while the server code enhancement improves the flexibility of OpenAPI resource definitions by dynamically generating schemas for different Group-Version-Kind (GVK) pairs. Changes
Sequence DiagramsequenceDiagram
participant Server as API Server
participant OpenAPI as OpenAPI Schema Generator
Server->>OpenAPI: Initiate Schema Generation
OpenAPI->>OpenAPI: Iterate through registered GVKs
OpenAPI->>OpenAPI: Create Deep Copy of Application Schema
OpenAPI->>OpenAPI: Update Schema Extensions
OpenAPI->>OpenAPI: Store New Resource Definitions
OpenAPI->>OpenAPI: Remove Original Definitions
OpenAPI-->>Server: Return Enhanced OpenAPI Schema
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Signed-off-by: Andrei Kvapil <[email protected]>
3d1078c
to
e06f56f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/cmd/server/start.go (2)
Line range hint
221-239
: Confirm no side effects on existing schema extensions.Deep copying and then assigning the “x-kubernetes-group-version-kind” array is correct. However, if
newDef.Extensions
contained other keys by default, they remain intact. Confirm that preserving arbitrary extensions is actually desired in this context. If you need a fresh set each time, consider re-initializing the map or removing undesired keys.
243-251
: Leverage a helper function for DRY approach.Here you replicate the pattern used for creating new definitions to also handle the List definitions. For clarity and maintainability, consider extracting a helper function or method that takes in the base definition (e.g.,
Application
) and auto-generates both the resource and List definitions together. This will reduce duplication and possible future errors if one path is updated but the other is missed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/system/cozystack-api/values.yaml
(1 hunks)pkg/cmd/server/start.go
(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/system/cozystack-api/values.yaml
🔇 Additional comments (4)
pkg/cmd/server/start.go (4)
204-214
: Consider adding fallback logic or warnings for missing definitions.
Here, the code checks for the presence of Application
and ApplicationList
definitions and fails if they are not found. This is good for clarity, but might lead to errors when new objects are added in the future. You may want to log a warning instead or provide more context in the returned error message for easier debugging.
215-220
: Validate the availability of registered GVKs.
When iterating over RegisteredGVKs
, consider logging or handling the case where the list is empty or partially filled to ensure that the schema generation doesn't silently skip resources.
262-272
: Exhaustively verify the “items” property path for correctness.
The logic properly checks for itemsProp
and updates the reference to point to the newly generated resource definition. In some OpenAPI versions, array elements or nested references may appear under different property paths (e.g. allOf
or oneOf
). Consider verifying you aren’t missing corner cases in more complex object schemas.
277-277
: Ensure no references to Application
and ApplicationList
remain.
Removing these definitions streamlines the schema, but verify that no other references point to the old definitions. Consider searching through your codebase for the literal strings of the removed definitions to ensure there are no broken references downstream.
Summary by CodeRabbit
New Features
cozystackAPI
to the latest version.Bug Fixes