fix(inkless): reject topic creation with config conflicting with diskless regex#642
Conversation
There was a problem hiding this comment.
Pull request overview
This PR tightens CREATE_TOPICS validation by rejecting requests that explicitly set diskless.enable=false for topic names that match the configured diskless-force include regexes, preventing ambiguous intent and surprising topic creation behavior.
Changes:
- Make
DisklessForceCreateTopicInterceptorthrowInvalidRequestExceptionwhendiskless.enable=falseconflicts with diskless-force include regex matching. - Catch
ApiExceptionfrom create-topic config interceptors inReplicationControlManager#createTopicsand convert it into a per-topic error response (instead of failing the whole request). - Update interceptor unit tests to assert the new rejection behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| metadata/src/main/java/org/apache/kafka/controller/DisklessForceCreateTopicInterceptor.java | Reject conflicting diskless.enable=false when forcing would apply. |
| metadata/src/main/java/org/apache/kafka/controller/ReplicationControlManager.java | Convert interceptor-thrown ApiException into per-topic ApiError during CreateTopics processing. |
| metadata/src/test/java/org/apache/kafka/controller/DisklessForceCreateTopicInterceptorTest.java | Update tests to expect InvalidRequestException for the conflicting configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…less regex If a create topic request includes diskless.enable=false, but the topic name matches one of the diskless.force.include.topic.regexes, it is unclear what the real intent is. Since this is mostly likely an user mistake, let's be stricter for now and reject the request to let the user know the topic would likely be created in a different way than they expected.
dd693e6 to
421ec52
Compare
|
Original requirement for #614 was So this change goes against that requirement. |
Yes, but I think it was written without being discussed. After some discussion, we opted for changing this behaviour. |
tvainika
left a comment
There was a problem hiding this comment.
Thanks for the clarification. LGTM.
…less regex (#642) If a create topic request includes diskless.enable=false, but the topic name matches one of the diskless.force.include.topic.regexes, it is unclear what the real intent is. Since this is mostly likely an user mistake, let's be stricter for now and reject the request to let the user know the topic would likely be created in a different way than they expected.
If a create topic request includes diskless.enable=false, but the topic name matches one of the diskless.force.include.topic.regexes, it is unclear what the real intent is.
Since this is mostly likely an user mistake, let's be stricter for now and reject the request to let the user know the topic would likely be created in a different way than they expected.