Implement Kubectl Patch operations#1698
Conversation
Co-authored-by: brendandburns <5751682+brendandburns@users.noreply.github.com>
Co-authored-by: brendandburns <5751682+brendandburns@users.noreply.github.com>
|
@copilot extend the tests to do an explicit Get for the resource and validate that it has been correctly patched. |
Co-authored-by: brendandburns <5751682+brendandburns@users.noreply.github.com>
Extended all patch tests to include explicit Get operations after patching. Each test now:
Changes in commit 173c4a5. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: brendandburns, Copilot The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR implements Kubectl Patch operations for the Kubernetes C# client, adding support for patching both cluster-scoped and namespaced resources. The implementation follows the established patterns in the codebase by wrapping the existing GenericClient patch methods.
Key Changes:
- Added asynchronous patch methods supporting cluster-scoped and namespaced resources with all three Kubernetes patch types (JsonPatch, MergePatch, StrategicMergePatch)
- Added synchronous wrapper methods following the existing Kubectl pattern
- Comprehensive test coverage with explicit validation of persisted changes
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/KubernetesClient.Kubectl/Beta/AsyncKubectl.Patch.cs | Implements async patch methods (PatchAsync, PatchNamespacedAsync) that wrap GenericClient patch operations |
| src/KubernetesClient.Kubectl/Beta/Kubectl.Patch.cs | Provides synchronous wrappers (Patch, PatchNamespaced) using GetAwaiter().GetResult() pattern |
| tests/Kubectl.Tests/KubectlTests.Patch.cs | Adds comprehensive tests for all three patch types on both namespaced (ConfigMap) and cluster-scoped (Namespace) resources |
Based on my comprehensive review of this pull request, I found the implementation to be well-structured and consistent with existing patterns in the codebase. The code correctly follows the established conventions for:
- API Design: The parameter ordering matches the underlying GenericClient API (namespace before name for namespaced operations)
- Async/Sync Patterns: Uses the standard GetAwaiter().GetResult() pattern for synchronous wrappers
- Resource Management: Properly creates GenericClient instances with
disposeClient: false - Testing: Comprehensive test coverage with proper cleanup patterns and explicit validation
The implementation is clean, follows best practices, and doesn't introduce any security vulnerabilities or bugs. All methods are properly documented, and the tests thoroughly validate the functionality with all three patch types on both cluster-scoped and namespaced resources.
No issues found - the implementation is ready for merge.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Implement Kubectl Patch
Security Summary
No new security vulnerabilities introduced. The implementation:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.