Skip to content

Enhance topic change audit records with request details - #1901

Open
xyphoze wants to merge 3 commits into
kafbat:mainfrom
xyphoze:fix/improve-topic-change-auditing
Open

Enhance topic change audit records with request details#1901
xyphoze wants to merge 3 commits into
kafbat:mainfrom
xyphoze:fix/improve-topic-change-auditing

Conversation

@xyphoze

@xyphoze xyphoze commented Jul 1, 2026

Copy link
Copy Markdown

What changes did you make?

This PR improves the audit context for topic modification operations.

Previously, some topic modification audit records did not include the submitted request details. That we could see who changed a topic and which operation was performed, but not what was actually changed. This was especially limiting for topic config updates, where the audit record did not show which config keys were updated or what values they were changed to.

This change ensures the relevant request details are included in the audit context, so the actual topic changes can be reviewed later when investigating behavior, checking operational history, or tracing configuration changes.

There is still room for improvement. For example, operationParams should ideally contain only the configs that were actually changed. However, that should be handled where the topic update params are created, rather than in the audit layer itself.

How Has This Been Tested?

  • Manually (please, describe, if necessary)
    • Deployed and tested in our internal environment. Audit records now include the additional operationParams.
      image
  • Unit checks
    • Added TopicsControllerTest to verify that topic modification requests include request details in the audit/access context.
  • Integration checks
  • Covered by existing automation

Checklist

  • I have performed a self-review of my own code
  • My changes generate no new warnings (e.g. Sonar is happy)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged

Check out Contributing and Code of Conduct

A picture of a cute animal (not mandatory but encouraged)
skyler-ewing-f3H5JBf2Xao-unsplash
Photo by Skyler Ewing on Unsplash.

Summary by CodeRabbit

  • Bug Fixes
    • Improved topic change requests so permissions and audit details are now captured more reliably for topic updates, partition increases, and replication factor changes.
    • Fixed topic configuration handling to better recover from fetch errors.
  • Tests
    • Added coverage for topic mutation flows to verify request details are passed through correctly and audited as expected.

Ensure topic update, partition increase, and replication factor changes include their request details
@xyphoze
xyphoze requested a review from a team as a code owner July 1, 2026 10:49
@kapybro kapybro Bot added status/triage/manual Manual triage in progress and removed status/triage/manual Manual triage in progress labels Jul 1, 2026
@kapybro

kapybro Bot commented Jul 1, 2026

Copy link
Copy Markdown

AI Summary

This issue addresses a gap in topic modification auditing where audit records lacked the submitted request details, making it difficult to track what changes were actually made—especially for topic config updates. The proposed solution enhances the audit context to include operationParams, ensuring visibility into the specific modifications, though further refinement is needed to filter only changed configs. Testing confirmed the fix via manual deployment and unit tests, with audit records now capturing the additional details.

@kapybro kapybro Bot changed the title BE: Improve topic change auditing Enhance topic change audit records with request details Jul 1, 2026
@kapybro kapybro Bot added area/audit area/topics scope/backend Related to backend changes type/enhancement En enhancement/improvement to an already existing feature labels Jul 1, 2026
@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

TopicsController's updateTopic, increaseTopicPartitions, and changeReplicationFactor handlers now build AccessContext after the request body Mono resolves, attaching the DTO as operationParams and setting operationName. TopicsService exposes updateTopic publicly, removes an unused overload, and adds error handling to invalidate admin clients on config fetch errors. New controller tests validate this behavior.

Changes

AccessContext Enrichment for Topic Mutations

Layer / File(s) Summary
Service API adjustments
api/src/main/java/io/kafbat/ui/service/TopicsService.java
updateTopic DTO-based overload made public and the Mono-based overload removed; getTopicConfigs adds doOnError to invalidate admin client; minor adjustment in getTopics mapping.
updateTopic and increaseTopicPartitions AccessContext deferral
api/src/main/java/io/kafbat/ui/controller/TopicsController.java
Both handlers now flatMap the request body Mono before building AccessContext, attaching operationParams (and operationName for partition increase), then validate access and audit.
changeReplicationFactor AccessContext deferral
api/src/main/java/io/kafbat/ui/controller/TopicsController.java
Handler flatMaps the request body Mono before building AccessContext with operationParams, then validates access and executes the change.
Controller test coverage
api/src/test/java/io/kafbat/ui/controller/TopicsControllerTest.java
New test class verifies updateTopic, increaseTopicPartitions, and changeReplicationFactor pass request DTOs through to AccessContext and audit context, including operation names, params, and requested permissions.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant TopicsController
  participant AccessControlService
  participant TopicsService
  participant AuditService

  Client->>TopicsController: Request body (TopicUpdate/PartitionsIncrease/ReplicationFactorChange)
  TopicsController->>TopicsController: flatMap body Mono, build AccessContext with operationParams
  TopicsController->>AccessControlService: validateAccess(context)
  AccessControlService-->>TopicsController: validated
  TopicsController->>TopicsService: execute mutation(cluster, topic, dto)
  TopicsService-->>TopicsController: response DTO
  TopicsController->>AuditService: audit(context)
  TopicsController-->>Client: response
Loading

Poem

A rabbit hops through code so neat,
Access checks now know the DTO's feet,
Params tucked in, audits align,
Tests confirm each context is fine,
Thump thump — the burrow's build compiles just right! 🐰✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding request details to topic change audit records.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi xyphoze! 👋

Welcome, and thank you for opening your first PR in the repo!

Please wait for triaging by our maintainers.

Please take a look at our contributing guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/audit area/topics scope/backend Related to backend changes type/enhancement En enhancement/improvement to an already existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant