Allow users to specify a commit summary when mutating xDS resources - #1332
Merged
Conversation
Motivation: There was no way to specify a commit for xds resource update. Modifications: - Add an optional `summary` field to all xDS Create/Update/Delete request proto messages (cluster, listener, route, endpoint, group, k8s). The field is a query parameter in the HTTP-JSON transcoding because the request body is already mapped to the resource message. Result: - You can now enter a free-form commit summary when creating, updating, or deleting an xDS resource.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughxDS request schemas, backend services, frontend API mutations, and editors now support optional commit summaries for create, update, and delete operations, with default summaries retained when no value is provided. ChangesxDS commit summary flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Editor
participant FrontendAPI
participant XdsService
participant ResourceManager
Editor->>FrontendAPI: mutation with optional summary
FrontendAPI->>XdsService: request with summary query or field
XdsService->>XdsService: select provided or default summary
XdsService->>ResourceManager: create, update, or delete with summary
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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 (1)
xds/src/main/java/com/linecorp/centraldogma/xds/cluster/v1/XdsClusterService.java (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated summary-fallback logic into a shared helper.
The
isNullOrEmpty(request.getSummary()) ? "<Verb> <type>: " + name : request.getSummary()pattern is duplicated identically 3× in this file and repeats acrossXdsEndpointService,XdsKubernetesService,XdsListenerService, andXdsRouteService(15 occurrences total). A single shared helper would remove this duplication and also lets you fix the edge case where a whitespace-only summary (e.g. a single space) is currently treated as "non-empty" and used verbatim instead of falling back to the default.♻️ Proposed helper (e.g. add to a shared xDS service utility)
+ static String summaryOrDefault(String summary, String defaultSummary) { + return isNullOrEmpty(summary) || summary.trim().isEmpty() ? defaultSummary : summary; + }Then in each call site:
- final String createSummary = isNullOrEmpty(request.getSummary()) ? - "Create cluster: " + clusterName : request.getSummary(); + final String createSummary = summaryOrDefault(request.getSummary(), "Create cluster: " + clusterName);Also applies to: 82-107
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@xds/src/main/java/com/linecorp/centraldogma/xds/cluster/v1/XdsClusterService.java` at line 18, Extract the repeated summary construction used by XdsClusterService and the related xDS service classes into one shared helper. Have the helper treat null, empty, and whitespace-only summaries as missing, then generate the existing verb/type/name fallback; otherwise preserve the supplied summary. Replace all 15 duplicated call sites with the helper.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@xds/src/main/proto/centraldogma/xds/group/v1/xds_group.proto`:
- Around line 50-51: Propagate the optional summary through the group mutation
flow before exposing the proto fields: update XdsGroupService.createGroup and
deleteGroup to consume request.getSummary(), and extend the frontend group
mutations to send the summary through to those requests so custom operation
summaries are applied instead of ignored.
---
Nitpick comments:
In
`@xds/src/main/java/com/linecorp/centraldogma/xds/cluster/v1/XdsClusterService.java`:
- Line 18: Extract the repeated summary construction used by XdsClusterService
and the related xDS service classes into one shared helper. Have the helper
treat null, empty, and whitespace-only summaries as missing, then generate the
existing verb/type/name fallback; otherwise preserve the supplied summary.
Replace all 15 duplicated call sites with the helper.
🪄 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: Pro
Run ID: d1b1438c-6482-498a-9741-e4463840d0d6
📒 Files selected for processing (15)
webapp/src/dogma/common/components/DeleteConfirmationModal.tsxwebapp/src/dogma/features/xds/K8sAggregatorEditor.tsxwebapp/src/dogma/features/xds/ResourceEditor.tsxwebapp/src/dogma/features/xds/xdsApiSlice.tsxds/src/main/java/com/linecorp/centraldogma/xds/cluster/v1/XdsClusterService.javaxds/src/main/java/com/linecorp/centraldogma/xds/endpoint/v1/XdsEndpointService.javaxds/src/main/java/com/linecorp/centraldogma/xds/k8s/v1/XdsKubernetesService.javaxds/src/main/java/com/linecorp/centraldogma/xds/listener/v1/XdsListenerService.javaxds/src/main/java/com/linecorp/centraldogma/xds/route/v1/XdsRouteService.javaxds/src/main/proto/centraldogma/xds/cluster/v1/xds_cluster.protoxds/src/main/proto/centraldogma/xds/endpoint/v1/xds_endpoint.protoxds/src/main/proto/centraldogma/xds/group/v1/xds_group.protoxds/src/main/proto/centraldogma/xds/k8s/v1/xds_kubernetes.protoxds/src/main/proto/centraldogma/xds/listener/v1/xds_listener.protoxds/src/main/proto/centraldogma/xds/route/v1/xds_route.proto
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1332 +/- ##
============================================
- Coverage 69.85% 68.93% -0.92%
- Complexity 5612 5649 +37
============================================
Files 532 540 +8
Lines 23646 24146 +500
Branches 2700 2772 +72
============================================
+ Hits 16517 16646 +129
- Misses 5644 5969 +325
- Partials 1485 1531 +46 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
minwoox
marked this pull request as ready for review
July 13, 2026 02:42
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation:
There was no way to specify a commit for xds resource update.
Modifications:
summaryfield to all xDS Create/Update/Delete request proto messages (cluster, listener, route, endpoint, group, k8s). The field is a query parameter in the HTTP-JSON transcoding because the request body is already mapped to the resource message.Result: