Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for an enterprise audit mode where audit records can be saved inside the audit database itself (instead of being sent to taoskeeper), including plumbing for propagating audit target vnode info, creating the audit supertable on audit DB creation, and adding CI coverage + docs.
Changes:
- Introduces
auditSaveInSelfserver config and status propagation of audit targetSEpSet+vgId. - Creates the audit supertable (
operations) when an audit database is created (with vgroups constrained to 1) and adds vnode-side handling for persisting audit records. - Adds a new component test for self-audit and updates docs/error codes accordingly.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test/ci/cases.task | Adds CI entry for the new self-audit component test. |
| test/cases/80-Components/01-Taosd/test_com_taosd_self_audit.py | New test validating audit records are written into audit.operations when auditSaveInSelf=1. |
| test/cases/80-Components/01-Taosd/test_com_taosd_audit.py | Adjusts audit DB creation SQL (vgroups=1) and logging. |
| source/util/src/tjson.c | Adds NULL guard in tjsonGetStringValue2; introduces tjsonGetStringPointer. |
| include/util/tjson.h | Declares tjsonGetStringPointer with lifetime documentation. |
| source/util/src/terror.c / include/util/taoserror.h / docs/*error-code.md | Adds and documents new error code for audit DB vgroup restriction. |
| source/common/src/tglobal.c / include/common/tglobal.h / docs/*taosd.md | Adds and documents auditSaveInSelf config option. |
| source/common/src/msg/tmsg.c / include/common/tmsg.h | Extends status req/rsp encoding/decoding with audit epSet + vgId; adds SVAuditRecordReq serde helpers. |
| include/common/tmsgdef.h / source/dnode/mgmt/mgmt_vnode/src/vmHandle.c | Adds vnode msg type TDMT_VND_AUDIT_RECORD and routes it to vnode write queue. |
| source/dnode/vnode/src/vnd/vnodeSvr.c | Implements vnode-side decoding and persistence path for audit records into audit DB tables. |
| source/dnode/mnode/impl/src/mndDb.c | Enforces audit DB vgroups == 1; triggers creation of audit supertable when self-save enabled. |
| source/dnode/mnode/impl/src/mndStb.c / source/dnode/mnode/impl/inc/mndStb.h | Adds audit supertable schema builder + minimal rsp handler wiring. |
| source/dnode/mnode/impl/src/mndDnode.c / source/dnode/mgmt/mgmt_dnode/src/dmHandle.c | Adds audit epSet/vgId propagation via status messages; masks audit token in logs. |
| source/dnode/mgmt/mgmt_mnode/src/mmWorker.c / mmHandle.c / mmInt.h / include/common/tmsgcb.h | Adds a dedicated AUDIT_QUEUE and worker routing for audit record responses. |
| source/dnode/mnode/impl/src/mndTrans.c / mndIndex.c / mndUser.c | Minor logging level adjustment / includes / small formatting. |
| include/util/tdef.h / include/libs/audit/audit.h | Adds constants (AUDIT_CLIENT_ADD_LEN, AUDIT_STABLE_NAME) and adjusts audit record struct fields. |
| docs/zh/en/08-operation/16-security.md | Documents self-save mode and the vgroup restriction. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a crucial enhancement to the auditing system by enabling audit information to be stored locally within the database instance itself, bypassing the need for an external Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new feature to save audit logs locally within a dedicated audit database, instead of sending them to taoskeeper. The changes include a new configuration parameter auditSaveInSelf, a new constraint that audit databases can only have one vgroup, and the necessary logic in mnode and vnode to handle the creation and writing of audit records. The implementation is comprehensive, with updated documentation and new tests. My review focuses on improving documentation clarity, code hygiene, and addressing a potential security risk in logging.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (3)
test/cases/80-Components/01-Taosd/test_com_taosd_self_audit.py:1
eval()onremoteIPis unsafe even in tests because it executes arbitrary code if the string is compromised or malformed. Prefer parsing as JSON (e.g.,json.loads) or using a safer literal parser (e.g.,ast.literal_eval) and validating expected keys/types.
test/cases/80-Components/01-Taosd/test_com_taosd_self_audit.py:1- This test uses
print()for diagnostic output, while similar tests usetdLog.*(and one was updated in this PR). Use the same logging mechanism here to keep CI logs consistent and to respect any log filtering/formatting the framework provides.
source/util/src/tjson.c:1 - Parameter validation is incomplete/fragile here:
maxLen - 1can underflow whenmaxLen <= 0, andpValis validated only after the length check. It would be safer to validatepVal != NULLandmaxLen > 0before usingmaxLenin arithmetic, and returnTSDB_CODE_INVALID_PARAfor invalid arguments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 6 comments.
Comments suppressed due to low confidence (8)
test/cases/80-Components/01-Taosd/test_com_taosd_self_audit.py:1
- The test operates on
db.*objects but this diff doesn’t showdbbeing created (orUSE db) anywhere in the test. This makes the test dependent on external state and can fail in clean environments. Create thedbdatabase (and/orUSE db) explicitly in this test before creatingdb.stb/db.tb, or switch the statements to a database that the test itself creates.
test/cases/80-Components/01-Taosd/test_com_taosd_self_audit.py:1 - The test operates on
db.*objects but this diff doesn’t showdbbeing created (orUSE db) anywhere in the test. This makes the test dependent on external state and can fail in clean environments. Create thedbdatabase (and/orUSE db) explicitly in this test before creatingdb.stb/db.tb, or switch the statements to a database that the test itself creates.
test/cases/80-Components/01-Taosd/test_com_taosd_self_audit.py:1 - The test operates on
db.*objects but this diff doesn’t showdbbeing created (orUSE db) anywhere in the test. This makes the test dependent on external state and can fail in clean environments. Create thedbdatabase (and/orUSE db) explicitly in this test before creatingdb.stb/db.tb, or switch the statements to a database that the test itself creates.
test/cases/80-Components/01-Taosd/test_com_taosd_self_audit.py:1 - The test operates on
db.*objects but this diff doesn’t showdbbeing created (orUSE db) anywhere in the test. This makes the test dependent on external state and can fail in clean environments. Create thedbdatabase (and/orUSE db) explicitly in this test before creatingdb.stb/db.tb, or switch the statements to a database that the test itself creates.
test/cases/80-Components/01-Taosd/test_com_taosd_self_audit.py:1 - This unconditional
printwill pollute CI output and bypass the test framework’s logging controls. PrefertdLog.info(...)(or remove the debug output) so logs are consistent and can be filtered/disabled.
test/cases/80-Components/01-Taosd/test_com_taosd_self_audit.py:1 - Correct the typo “excute” to “execute”.
source/util/src/tjson.c:1 pValis validated only after the length check, which can returnTSDB_CODE_OUT_OF_MEMORYeven when the real problem ispVal == NULL. ValidatepValbefore performing size/length checks so the function returns the correct error code deterministically.
source/util/src/terror.c:1- The message reads a bit awkwardly (“not allowed to keep multiple vgroups”). Consider rephrasing to something clearer for users, e.g. “Audit database must have exactly 1 vgroup” or “Audit database cannot have multiple vgroups”.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.