<feature>[sdk]: support log server#4108
Open
ZStack-Robot wants to merge 3 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Walkthrough本 PR 增加 Log Server 的持久化表与注释、全局配置；扩展 SDK 数据模型（枚举、Inventory）；并新增增删改查四个 SDK 动作、类型映射及生成器子路径设置。 ChangesLog Server 功能实现
🎯 3 (Moderate) | ⏱️ ~20 分钟
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Collaborator
|
Comment from yaohua.wu: Review: MR !10004 — ZSV-12254Background
P0 — Critical无（重型实现都在 premium 侧）。 P1 — Warning
P2 — Suggestion
跨仓库一致性
Verdict: APPROVED with notes本 MR 自身代码改动小、风险低（基本是 SDK 生成代码 + schema + config）。主要风险集中在 !14087，请同时关注那边的 P0 阻塞项。建议把上面的 Warning #1 schema 改造（state 列、密码脱敏）与 !14087 一并修订，避免 5.1.0 上线后出现升级痛点。 🤖 Robot Reviewer |
![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)
sdk/src/main/java/org/zstack/sdk/QueryLogServerResult.java (1)
6-11: ⚡ Quick win优化 SDK 返回类型：inventories 避免原始 List（需先确认生成/反序列化约定）
当前inventories为原始java.util.List；但在本仓库 SDK 的大量*Result类中也使用同样的 rawList inventories（如AddVRouterNetworksToOspfAreaResult、GetZoneResult），疑似为生成器/序列化约定。建议先确认约定后再考虑在QueryLogServerResult中改为List<具体清单类型>并同步 setter/getter。🤖 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 `@sdk/src/main/java/org/zstack/sdk/QueryLogServerResult.java` around lines 6 - 11, The field inventories in QueryLogServerResult is declared as a raw java.util.List (with setInventories and getInventories) which risks ClassCastExceptions and weak typing; confirm whether the SDK code generator/serialization convention allows introducing generics, and if so change the field to a typed List<YourInventoryType> and update setInventories(java.util.List<YourInventoryType>) and getInventories() to return List<YourInventoryType>, or otherwise document/annotate this class to indicate why raw List is required to keep consistency with other Result classes.
🤖 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 `@conf/db/zsv/V5.1.0__schema.sql`:
- Line 51: Replace the illegal zero datetime default for the `createDate` column
by changing its DEFAULT from the literal '0000-00-00 00:00:00' to DEFAULT
CURRENT_TIMESTAMP; update the table/schema definition that contains the
`createDate` column so the column declaration reads with DEFAULT
CURRENT_TIMESTAMP to avoid strict-mode failures.
---
Nitpick comments:
In `@sdk/src/main/java/org/zstack/sdk/QueryLogServerResult.java`:
- Around line 6-11: The field inventories in QueryLogServerResult is declared as
a raw java.util.List (with setInventories and getInventories) which risks
ClassCastExceptions and weak typing; confirm whether the SDK code
generator/serialization convention allows introducing generics, and if so change
the field to a typed List<YourInventoryType> and update
setInventories(java.util.List<YourInventoryType>) and getInventories() to return
List<YourInventoryType>, or otherwise document/annotate this class to indicate
why raw List is required to keep consistency with other Result classes.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 3efb8186-136b-4427-996a-7d7657fe8946
⛔ Files ignored due to path filters (1)
conf/persistence.xmlis excluded by!**/*.xml
📒 Files selected for processing (18)
conf/db/zsv/V5.1.0__schema.sqlcore/src/main/java/org/zstack/core/log/LogGlobalConfig.javarest/src/main/resources/scripts/SdkDataStructureGenerator.groovysdk/src/main/java/org/zstack/sdk/AddLogConfigurationAction.javasdk/src/main/java/org/zstack/sdk/AddLogServerAction.javasdk/src/main/java/org/zstack/sdk/AddLogServerResult.javasdk/src/main/java/org/zstack/sdk/DeleteLogServerAction.javasdk/src/main/java/org/zstack/sdk/DeleteLogServerResult.javasdk/src/main/java/org/zstack/sdk/LogCategory.javasdk/src/main/java/org/zstack/sdk/LogLevel.javasdk/src/main/java/org/zstack/sdk/LogServerInventory.javasdk/src/main/java/org/zstack/sdk/LogType.javasdk/src/main/java/org/zstack/sdk/QueryLogServerAction.javasdk/src/main/java/org/zstack/sdk/QueryLogServerResult.javasdk/src/main/java/org/zstack/sdk/SourceClassMap.javasdk/src/main/java/org/zstack/sdk/UpdateLogServerAction.javasdk/src/main/java/org/zstack/sdk/UpdateLogServerResult.javautils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.java
Collaborator
Author
|
Comment from taiyue.chen: 转移这个功能之前已经实现了，只要不在cascade删除那边引入account这个edge，即可避免在account删除时，一起被删掉。然后，这些无主资源会被自动流转给admin用户。 |
MatheMatrix
force-pushed
the
sync/zstackio/ZSV-12254@@3
branch
4 times, most recently
from
19e0aae to
028c6b9
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@conf/db/zsv/V5.1.0__schema.sql`:
- Line 50: The schema currently defines the `configuration` column as text NOT
NULL which stores full server configs in plaintext; change this by identifying
sensitive keys within `configuration` and either: (a) extract those keys into
dedicated columns (e.g., `api_key`, `password`, `token`) and store them
encrypted at rest (use your app's encryption service or DB encryption functions)
with appropriate NOT NULL/NULL constraints, or (b) keep a single `configuration`
column but ensure the application encrypts it before INSERT/UPDATE and decrypts
only when needed, plus add DB-level comments/constraints to enforce that
`configuration` is stored encrypted and ensure SELECTs used by Inventory/log
APIs strip or mask sensitive fields. Ensure the migration updates the table to
add new encrypted columns and a migration path to move and remove plaintext data
from the `configuration` text field.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 07aef937-c23f-4846-93db-f9cb8b362bd5
⛔ Files ignored due to path filters (1)
conf/persistence.xmlis excluded by!**/*.xml
📒 Files selected for processing (18)
conf/db/zsv/V5.1.0__schema.sqlcore/src/main/java/org/zstack/core/log/LogGlobalConfig.javarest/src/main/resources/scripts/SdkDataStructureGenerator.groovysdk/src/main/java/org/zstack/sdk/AddLogConfigurationAction.javasdk/src/main/java/org/zstack/sdk/AddLogServerAction.javasdk/src/main/java/org/zstack/sdk/AddLogServerResult.javasdk/src/main/java/org/zstack/sdk/DeleteLogServerAction.javasdk/src/main/java/org/zstack/sdk/DeleteLogServerResult.javasdk/src/main/java/org/zstack/sdk/LogCategory.javasdk/src/main/java/org/zstack/sdk/LogLevel.javasdk/src/main/java/org/zstack/sdk/LogServerInventory.javasdk/src/main/java/org/zstack/sdk/LogServerState.javasdk/src/main/java/org/zstack/sdk/LogType.javasdk/src/main/java/org/zstack/sdk/QueryLogServerAction.javasdk/src/main/java/org/zstack/sdk/QueryLogServerResult.javasdk/src/main/java/org/zstack/sdk/SourceClassMap.javasdk/src/main/java/org/zstack/sdk/UpdateLogServerAction.javasdk/src/main/java/org/zstack/sdk/UpdateLogServerResult.java
✅ Files skipped from review due to trivial changes (2)
- sdk/src/main/java/org/zstack/sdk/LogServerState.java
- sdk/src/main/java/org/zstack/sdk/UpdateLogServerResult.java
🚧 Files skipped from review as they are similar to previous changes (12)
- sdk/src/main/java/org/zstack/sdk/LogLevel.java
- core/src/main/java/org/zstack/core/log/LogGlobalConfig.java
- sdk/src/main/java/org/zstack/sdk/LogType.java
- sdk/src/main/java/org/zstack/sdk/LogServerInventory.java
- sdk/src/main/java/org/zstack/sdk/AddLogConfigurationAction.java
- sdk/src/main/java/org/zstack/sdk/QueryLogServerResult.java
- sdk/src/main/java/org/zstack/sdk/DeleteLogServerResult.java
- sdk/src/main/java/org/zstack/sdk/QueryLogServerAction.java
- sdk/src/main/java/org/zstack/sdk/AddLogServerAction.java
- rest/src/main/resources/scripts/SdkDataStructureGenerator.groovy
- sdk/src/main/java/org/zstack/sdk/UpdateLogServerAction.java
- sdk/src/main/java/org/zstack/sdk/DeleteLogServerAction.java
| `type` varchar(255) NOT NULL, | ||
| `level` varchar(255) NULL, | ||
| `state` varchar(255) NOT NULL DEFAULT 'Enabled', | ||
| `configuration` text NOT NULL, |
There was a problem hiding this comment.
避免将敏感配置明文存储在 configuration 字段。
configuration 使用 text NOT NULL 会把日志服务器配置整体落库；若包含如密码/令牌等敏感项，将形成明文泄露面（备份、审计导出、SQL 日志）。建议将密钥类字段拆分为单独加密存储（或至少入库前加密、出参脱敏），并避免在 Inventory/日志中直接回传原文。
🤖 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 `@conf/db/zsv/V5.1.0__schema.sql` at line 50, The schema currently defines the
`configuration` column as text NOT NULL which stores full server configs in
plaintext; change this by identifying sensitive keys within `configuration` and
either: (a) extract those keys into dedicated columns (e.g., `api_key`,
`password`, `token`) and store them encrypted at rest (use your app's encryption
service or DB encryption functions) with appropriate NOT NULL/NULL constraints,
or (b) keep a single `configuration` column but ensure the application encrypts
it before INSERT/UPDATE and decrypts only when needed, plus add DB-level
comments/constraints to enforce that `configuration` is stored encrypted and
ensure SELECTs used by Inventory/log APIs strip or mask sensitive fields. Ensure
the migration updates the table to add new encrypted columns and a migration
path to move and remove plaintext data from the `configuration` text field.
028c6b9 to
7929805
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@sdk/src/main/java/org/zstack/sdk/LogType.java`:
- Around line 4-5: The enum constants Log4j2 and FluentBit in the LogType enum
violate the all-caps underscore convention; rename them to LOG4J2 and FLUENT_BIT
in the org.zstack.sdk.LogType enum (update any references to LogType.Log4j2 and
LogType.FluentBit accordingly) and, if these enum names are used for
serialization or mapped by name to the server, coordinate/verify compatibility
with the server-side enum or add a custom serializer/annotation to preserve
external names before changing; ensure tests and usages are updated to the new
identifiers.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e809a433-0915-48d2-be48-e5e85c6a7153
⛔ Files ignored due to path filters (1)
conf/persistence.xmlis excluded by!**/*.xml
📒 Files selected for processing (18)
conf/db/zsv/V5.1.0__schema.sqlcore/src/main/java/org/zstack/core/log/LogGlobalConfig.javarest/src/main/resources/scripts/SdkDataStructureGenerator.groovysdk/src/main/java/org/zstack/sdk/AddLogConfigurationAction.javasdk/src/main/java/org/zstack/sdk/AddLogServerAction.javasdk/src/main/java/org/zstack/sdk/AddLogServerResult.javasdk/src/main/java/org/zstack/sdk/DeleteLogServerAction.javasdk/src/main/java/org/zstack/sdk/DeleteLogServerResult.javasdk/src/main/java/org/zstack/sdk/LogCategory.javasdk/src/main/java/org/zstack/sdk/LogLevel.javasdk/src/main/java/org/zstack/sdk/LogServerInventory.javasdk/src/main/java/org/zstack/sdk/LogServerState.javasdk/src/main/java/org/zstack/sdk/LogType.javasdk/src/main/java/org/zstack/sdk/QueryLogServerAction.javasdk/src/main/java/org/zstack/sdk/QueryLogServerResult.javasdk/src/main/java/org/zstack/sdk/SourceClassMap.javasdk/src/main/java/org/zstack/sdk/UpdateLogServerAction.javasdk/src/main/java/org/zstack/sdk/UpdateLogServerResult.java
✅ Files skipped from review due to trivial changes (1)
- sdk/src/main/java/org/zstack/sdk/LogServerInventory.java
🚧 Files skipped from review as they are similar to previous changes (16)
- sdk/src/main/java/org/zstack/sdk/LogLevel.java
- rest/src/main/resources/scripts/SdkDataStructureGenerator.groovy
- sdk/src/main/java/org/zstack/sdk/DeleteLogServerResult.java
- sdk/src/main/java/org/zstack/sdk/AddLogConfigurationAction.java
- sdk/src/main/java/org/zstack/sdk/LogCategory.java
- sdk/src/main/java/org/zstack/sdk/SourceClassMap.java
- sdk/src/main/java/org/zstack/sdk/QueryLogServerResult.java
- core/src/main/java/org/zstack/core/log/LogGlobalConfig.java
- sdk/src/main/java/org/zstack/sdk/AddLogServerResult.java
- conf/db/zsv/V5.1.0__schema.sql
- sdk/src/main/java/org/zstack/sdk/AddLogServerAction.java
- sdk/src/main/java/org/zstack/sdk/UpdateLogServerResult.java
- sdk/src/main/java/org/zstack/sdk/DeleteLogServerAction.java
- sdk/src/main/java/org/zstack/sdk/UpdateLogServerAction.java
- sdk/src/main/java/org/zstack/sdk/LogServerState.java
- sdk/src/main/java/org/zstack/sdk/QueryLogServerAction.java
Comment on lines
+4
to
+5
| Log4j2, | ||
| FluentBit, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
枚举常量命名不符合规范，建议与服务端枚举一起统一。
Line 4 和 Line 5 的 Log4j2、FluentBit 不符合常量全大写下划线命名规则。若这些值参与 SDK/服务端按名称映射，请同步评估服务端枚举与序列化兼容性后再统一重命名。
♻️ 建议修改
public enum LogType {
- Log4j2,
- FluentBit,
+ LOG4J2,
+ FLUENT_BIT,
}As per coding guidelines “常量命名：全部大写，使用下划线分隔单词。”
🤖 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 `@sdk/src/main/java/org/zstack/sdk/LogType.java` around lines 4 - 5, The enum
constants Log4j2 and FluentBit in the LogType enum violate the all-caps
underscore convention; rename them to LOG4J2 and FLUENT_BIT in the
org.zstack.sdk.LogType enum (update any references to LogType.Log4j2 and
LogType.FluentBit accordingly) and, if these enum names are used for
serialization or mapped by name to the server, coordinate/verify compatibility
with the server-side enum or add a custom serializer/annotation to preserve
external names before changing; ensure tests and usages are updated to the new
identifiers.
DBImpact GlobalConfigImpact Resolves: ZSV-12254 Change-Id: I77716165756971736379716977626b6f6f65656e
MatheMatrix
force-pushed
the
sync/zstackio/ZSV-12254@@3
branch
from
7929805 to
f0891e6
Compare
| @@ -0,0 +1,122 @@ | |||
| package org.zstack.sdk; | |||
Collaborator
There was a problem hiding this comment.
Comment from wenhao.zhang:
创一个子目录放进去，别直接放 org.zstack.sdk 下面
看看其它的 PackageInfo 怎么做的
Resolves: ZSV-12254 Change-Id: I6d6a6b77796564796d6d6e6d6f6a6c6d73616d76
MatheMatrix
force-pushed
the
sync/zstackio/ZSV-12254@@3
branch
2 times, most recently
from
7bb7845 to
1596591
Compare
Resolves: ZSV-12254 Change-Id: I676a756578776c7272767165657a6a6761676a64
MatheMatrix
force-pushed
the
sync/zstackio/ZSV-12254@@3
branch
from
1596591 to
73cfa2c
Compare
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.
DBImpact
GlobalConfigImpact
Resolves: ZSV-12254
Change-Id: I77716165756971736379716977626b6f6f65656e
sync from gitlab !10004