-
Notifications
You must be signed in to change notification settings - Fork 146
OCM-20588 | Add patch operation for version gates to allow partial updates #1106
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
OCM-20588 | Add patch operation for version gates to allow partial updates #1106
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: miguelhbrito The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
WalkthroughThe changes introduce an Update workflow for the VersionGate resource, adding a PATCH operation to the OpenAPI specification, corresponding client-side request/response types with builder pattern support, JSON marshaling helpers, and bumping transitive dependencies to v0.0.439. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant UpdateReq as VersionGateUpdateRequest
participant Transport as HTTP Client
participant UpdateResp as VersionGateUpdateResponse
Client->>UpdateReq: Update()
Client->>UpdateReq: Parameter(name, value)
Client->>UpdateReq: Header(name, value)
Client->>UpdateReq: Body(versionGate)
Client->>UpdateReq: Send()
UpdateReq->>UpdateReq: Construct PATCH request
UpdateReq->>Transport: Execute PATCH
Transport-->>UpdateReq: Response
UpdateReq->>UpdateReq: Unmarshal response body
UpdateReq->>UpdateResp: Create response
UpdateResp-->>Client: Return result or error
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Comment |
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)
clustersmgmt/v1/version_gate_resource_request_json.go (1)
38-45: Update helpers correctly mirror existing Get semantics
writeVersionGateUpdateRequestandreadVersionGateUpdateResponsecleanly reuseMarshalVersionGate/UnmarshalVersionGateand are consistent with the existingreadVersionGateGetResponsepattern. This keeps all wire-format logic centralized on the VersionGate marshaller while making Update symmetric with Get.Only usage caveat: callers should ensure
VersionGateUpdateRequest.Bodyis set to a non-nil*VersionGatebeforeSendContext, so the marshaller can construct the intended PATCH payload. The implementation here looks correct.clustersmgmt/v1/version_gate_client.go (1)
437-578: PATCH request/response flow is consistent and soundThe new
VersionGateUpdateRequest/VersionGateUpdateResponsetypes andSend/SendContextimplementation closely follow the existing Get/Delete conventions:
- Builder methods (
Parameter,Header,Impersonate,Body) match other request types and reusehelpersutilities.SendContext:
- Copies query/headers to avoid mutation.
- Serializes the body via
writeVersionGateUpdateRequestinto abytes.Buffer.- Issues a
PATCHrequest with that buffer as the body.- Uses the same
bufio.Peekcheck anderrors.UnmarshalErrorStatuspath for non-2xx responses as Get/Delete.- Delegates successful body parsing to
readVersionGateUpdateResponse, which mirrors the Get path.This yields predictable status/error handling and returns the updated
VersionGateinBody()/GetBody()with the usual nil-safety checks.One behavioural note for callers: since the payload is entirely derived from
VersionGateUpdateRequest.body, ensureBody()is called with a properly-populated*VersionGate(or explicitly rely on whatever semanticsMarshalVersionGate(nil, ...)provides) beforeSendContext.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
⛔ Files ignored due to path filters (3)
examples/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.summetamodel_generator/go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
clustersmgmt/v1/version_gate_client.go(3 hunks)clustersmgmt/v1/version_gate_resource_request_json.go(1 hunks)examples/go.mod(1 hunks)go.mod(1 hunks)metamodel_generator/go.mod(1 hunks)openapi/clusters_mgmt/v1/openapi.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
clustersmgmt/v1/version_gate_resource_request_json.go (1)
clustersmgmt/v1/version_gate_client.go (2)
VersionGateUpdateRequest(438-444)VersionGateUpdateResponse(531-536)
clustersmgmt/v1/version_gate_client.go (3)
helpers/helpers.go (5)
AddValue(33-45)AddHeader(61-66)AddImpersonationHeader(79-81)CopyQuery(48-57)CopyHeader(69-75)request.go (1)
Request(32-39)errors/errors.go (2)
UnmarshalErrorStatus(371-379)Error(55-65)
⏰ 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). (5)
- GitHub Check: Lint
- GitHub Check: Generate
- GitHub Check: Test (1.21, windows-latest)
- GitHub Check: Test (1.21, ubuntu-latest)
- GitHub Check: Test (1.21, macos-latest)
🔇 Additional comments (6)
go.mod (1)
30-31: Based on my verification:
- Availability: v0.0.439 of the ocm-api-model module is available in the public Go module registry and can be fetched via standard Go tooling.
- Security: No published CVEs or security advisories specifically target ocm-api-model v0.0.439.
Both packages are available at v0.0.439 with no known security vulnerabilities.
The dependency updates to v0.0.439 are verified and safe to proceed with.
metamodel_generator/go.mod (1)
5-5: Based on my verification through public sources, I have confirmed that:
- v0.0.439 exists: The version is published in Go module registry package indexes
- No security vulnerabilities found: Web searches across public CVE databases and security advisory sources returned no specific security issues for this version
The version bump appears to be a routine patch-level update with no identified security concerns.
Dependency version verified as available with no known security vulnerabilities.
The metamodel_generator dependency is updated to v0.0.439, which is published in the Go module registry and has no known security vulnerabilities identified in public advisory databases.
examples/go.mod (1)
46-47: Version v0.0.439 cannot be verified in public Go module registries.The web search results indicate that v0.0.439 for both
github.com/openshift-online/ocm-api-model/clientapiandgithub.com/openshift-online/ocm-api-model/modelis not publicly available on pkg.go.dev, GitHub releases, or Libraries.io. While no CVEs are associated with the ocm-api-model module itself, the inability to locate these specific versions in standard registries raises a question about their legitimacy.Verify that v0.0.439 is a valid release or confirm whether these versions are available through a private registry or internal repository.
openapi/clusters_mgmt/v1/openapi.json (1)
13757-13800: Verification of schema references and JSON structure could not be completed due to repository access limitations.The provided PATCH operation snippet follows OpenAPI 3.0 standards correctly with proper parameter definition, request body structure, and response handling. However, the original review's verification requests could not be executed:
- Cannot confirm
#/components/schemas/VersionGateand#/components/schemas/Errorschema definitions exist in the file- Cannot validate overall JSON structure closure and path nesting integrity
The code snippet itself appears structurally sound, but confirmation of the referenced schemas and complete JSON validity requires direct file inspection.
clustersmgmt/v1/version_gate_client.go (2)
22-33: Newbytesimport is appropriate and usedThe added
bytesimport is required for thebytes.Bufferused when building the PATCH request body inVersionGateUpdateRequest.SendContext, so the import is correct and not superfluous.
73-81:Update()constructor aligns with existing client patterns
VersionGateClient.Update()mirrorsDelete()andGet()by wiringtransportandpathinto a freshVersionGateUpdateRequest. This keeps the public API consistent and makes the new Update flow discoverable alongside the other methods.
Update model to 0.0.439