This is an automated email from the ASF dual-hosted git repository.
lizhimins pushed a commit to branch rocketmq-studio
in repository https://gitbox.apache.org/repos/asf/rocketmq-dashboard.git
The following commit(s) were added to refs/heads/rocketmq-studio by this push:
new d5d0d502c fix(ai): address an ACL rule by principal when it has no
numeric id (#6536)
d5d0d502c is described below
commit d5d0d502c09ce0ec00e0a5585be34cca85fb96d3
Author: Apulupie <[email protected]>
AuthorDate: Thu Oct 15 18:14:50 2026 +0800
fix(ai): address an ACL rule by principal when it has no numeric id (#6536)
rmq.acl.update demanded a numeric id, so on an instance whose ACL rules
have no numeric database id
(Tencent Cloud roles; rmq.acl.list omits the id for them since #4574) the
tool always failed with
"ACL id must be numeric", for both the preview and the execute call - even
though AclService.getRule
already resolves a role name (id.equals(rule.getPrincipal())) and
AclService.updateRule's Tencent
branch never asks for an id (TencentAclService.updateRule maps the rule's
principal onto ModifyRole).
rmq.acl.get and rmq.acl.delete pass the identifier through and work; #4574
left this update path as a
follow-up.
The handler now resolves the addressed rule through AclService.getRule -
the service that owns the
"numeric id or principal" decision - and updates by that rule's id, in
preview and execute alike.
A Tencent rule therefore travels with a null id and is updated by
principal, an Apache rule keeps the
numeric id it has today, and a bogus identifier is reported by the service
(EntityIds.parseId answers
400 "id must be a numeric value", a missing rule 404). The acl.yaml id
field of get/update/delete now
documents both accepted forms so the model knows a principal is a valid
identifier, and catalog_gen.go
is regenerated. ToolError.ACL_ID_INVALID lost its only reference and is
removed.
Verified with `mvn -B -ntp test
-Dtest='org.apache.rocketmq.studio.ops.ai.**.*Test'`: 951 tests green
(including AclUpdateToolHandlerTest, AclMutationPlanTest, ToolCatalogTest
and
ToolOutputSchemaContractTest), 0 checkstyle violations, and `make
catalog-verify` green.
---
rmqctl/internal/catalog/catalog_gen.go | 8 +--
.../studio/ops/ai/tool/core/ToolError.java | 4 --
.../ai/tool/handler/acl/AclUpdateToolHandler.java | 21 ++----
.../src/main/resources/tool-catalog/tools/acl.yaml | 9 +++
.../tool/handler/acl/AclUpdateToolHandlerTest.java | 74 ++++++++++++++++++++++
5 files changed, 93 insertions(+), 23 deletions(-)
diff --git a/rmqctl/internal/catalog/catalog_gen.go
b/rmqctl/internal/catalog/catalog_gen.go
index 73a23cb2d..fedfdff2a 100644
--- a/rmqctl/internal/catalog/catalog_gen.go
+++ b/rmqctl/internal/catalog/catalog_gen.go
@@ -21,7 +21,7 @@ package catalog
var defaultDocument = Document{
Version: "2.0.0",
MinimumClientVersion: "2.0.0",
- Digest:
"d067890205e30d380d26cf26ba456adba96294f2343fcd6b49e589c42e9f3694",
+ Digest:
"66ac58f23fbec620f3a0127e901a89f4bcad6c7ee08dd54cbd8f304730ed2199",
Tools: []Tool{
{
Name: "rmq.acl.list",
@@ -58,7 +58,7 @@ var defaultDocument = Document{
InputSchema: InputSchema{
Fields: []Field{
{Name: "instanceId", Flag:
"instance-id", Description: "Studio Instance identifier.", Kind: StringField,
Required: true, MinLength: 1},
- {Name: "id", Flag: "id", Kind:
StringField, Required: true, MinLength: 1},
+ {Name: "id", Flag: "id", Description:
"Numeric ACL rule id, or the rule's principal on a provider whose rules have no
numeric id (Tencent Cloud roles); rmq.acl.list reports no id for those rules.",
Kind: StringField, Required: true, MinLength: 1},
},
},
ViewHint: "object",
@@ -96,7 +96,7 @@ var defaultDocument = Document{
InputSchema: InputSchema{
Fields: []Field{
{Name: "instanceId", Flag:
"instance-id", Description: "Studio Instance identifier.", Kind: StringField,
Required: true, MinLength: 1},
- {Name: "id", Flag: "id", Kind:
StringField, Required: true, MinLength: 1},
+ {Name: "id", Flag: "id", Description:
"Numeric ACL rule id, or the rule's principal on a provider whose rules have no
numeric id (Tencent Cloud roles); rmq.acl.list reports no id for those rules.",
Kind: StringField, Required: true, MinLength: 1},
{Name: "principal", Flag: "principal",
Kind: StringField},
{Name: "resource", Flag: "resource",
Kind: StringField},
{Name: "resourceType", Flag:
"resource-type", Kind: StringField},
@@ -120,7 +120,7 @@ var defaultDocument = Document{
InputSchema: InputSchema{
Fields: []Field{
{Name: "instanceId", Flag:
"instance-id", Description: "Studio Instance identifier.", Kind: StringField,
Required: true, MinLength: 1},
- {Name: "id", Flag: "id", Kind:
StringField, Required: true, MinLength: 1},
+ {Name: "id", Flag: "id", Description:
"Numeric ACL rule id, or the rule's principal on a provider whose rules have no
numeric id (Tencent Cloud roles); rmq.acl.list reports no id for those rules.",
Kind: StringField, Required: true, MinLength: 1},
{Name: "dry_run", Flag: "dry-run",
Description: "When true, preview the mutation without executing it; omit to
apply with a confirm_token.", Kind: BooleanField},
{Name: "confirm_token", Flag:
"confirm-token", Kind: StringField, MinLength: 1},
{Name: "break_glass", Flag:
"break-glass", Kind: BooleanField},
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/ops/ai/tool/core/ToolError.java
b/server/src/main/java/org/apache/rocketmq/studio/ops/ai/tool/core/ToolError.java
index 5220136ac..38ce5a4c7 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/ops/ai/tool/core/ToolError.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/ops/ai/tool/core/ToolError.java
@@ -92,10 +92,6 @@ public enum ToolError {
"Tool execution failed unexpectedly.",
"Retry once; if the failure persists, contact an administrator."),
- ACL_ID_INVALID(HttpStatus.BAD_REQUEST, "INVALID_ARGUMENT",
- "ACL id must be numeric: %s",
- "Use the numeric ACL id returned by rmq.acl.list."),
-
ROLE_NOT_FOUND(HttpStatus.NOT_FOUND, "NOT_FOUND",
"Role not found: %s",
"List the roles for the selected Instance and choose an existing
role."),
diff --git
a/server/src/main/java/org/apache/rocketmq/studio/ops/ai/tool/handler/acl/AclUpdateToolHandler.java
b/server/src/main/java/org/apache/rocketmq/studio/ops/ai/tool/handler/acl/AclUpdateToolHandler.java
index 5f859357b..a5a62039d 100644
---
a/server/src/main/java/org/apache/rocketmq/studio/ops/ai/tool/handler/acl/AclUpdateToolHandler.java
+++
b/server/src/main/java/org/apache/rocketmq/studio/ops/ai/tool/handler/acl/AclUpdateToolHandler.java
@@ -18,7 +18,6 @@ package org.apache.rocketmq.studio.ops.ai.tool.handler.acl;
import org.apache.rocketmq.studio.instance.acl.AclRuleVO;
import org.apache.rocketmq.studio.instance.acl.AclService;
-import org.apache.rocketmq.studio.ops.ai.tool.core.ToolError;
import org.apache.rocketmq.studio.ops.ai.tool.core.ToolExecutionContext;
import org.apache.rocketmq.studio.ops.ai.tool.handler.MutationToolHandler;
import org.apache.rocketmq.studio.ops.ai.tool.contract.acl.AclMutationInput;
@@ -51,8 +50,9 @@ public class AclUpdateToolHandler extends
MutationToolHandler<AclMutationInput,
@Override
public ToolPlan preview(AclMutationInput input, ToolExecutionContext
context) {
String instanceId = context.instanceId();
- AclMutationInput before =
AclMutationInput.from(aclService.getRule(input.id(), instanceId));
- AclMutationInput after =
AclMutationInput.from(input.toRule(parseId(input.id())));
+ AclRuleVO existing = aclService.getRule(input.id(), instanceId);
+ AclMutationInput before = AclMutationInput.from(existing);
+ AclMutationInput after =
AclMutationInput.from(input.toRule(existing.getId()));
return PLAN_DESCRIPTION.builder(input.id(), context.instanceId())
.before(before)
.after(after)
@@ -63,17 +63,8 @@ public class AclUpdateToolHandler extends
MutationToolHandler<AclMutationInput,
@Override
public AclRuleVO execute(AclMutationInput input, ToolExecutionContext
context) {
- return aclService.updateRule(input.toRule(parseId(input.id())),
context.instanceId());
- }
-
- private static Long parseId(String id) {
- if (id == null || id.isBlank()) {
- throw ToolError.ACL_ID_INVALID.exception(id);
- }
- try {
- return Long.parseLong(id);
- } catch (NumberFormatException e) {
- throw ToolError.ACL_ID_INVALID.exception(id);
- }
+ String instanceId = context.instanceId();
+ AclRuleVO existing = aclService.getRule(input.id(), instanceId);
+ return aclService.updateRule(input.toRule(existing.getId()),
instanceId);
}
}
diff --git a/server/src/main/resources/tool-catalog/tools/acl.yaml
b/server/src/main/resources/tool-catalog/tools/acl.yaml
index d03f9e30d..9f649af3e 100644
--- a/server/src/main/resources/tool-catalog/tools/acl.yaml
+++ b/server/src/main/resources/tool-catalog/tools/acl.yaml
@@ -108,6 +108,9 @@ tools:
minLength: 1
id:
type: string
+ description: >-
+ Numeric ACL rule id, or the rule's principal on a provider whose
rules have no numeric
+ id (Tencent Cloud roles); rmq.acl.list reports no id for those
rules.
minLength: 1
outputSchema:
type: object
@@ -255,6 +258,9 @@ tools:
minLength: 1
id:
type: string
+ description: >-
+ Numeric ACL rule id, or the rule's principal on a provider whose
rules have no numeric
+ id (Tencent Cloud roles); rmq.acl.list reports no id for those
rules.
minLength: 1
principal:
type: string
@@ -344,6 +350,9 @@ tools:
minLength: 1
id:
type: string
+ description: >-
+ Numeric ACL rule id, or the rule's principal on a provider whose
rules have no numeric
+ id (Tencent Cloud roles); rmq.acl.list reports no id for those
rules.
minLength: 1
dry_run:
type: boolean
diff --git
a/server/src/test/java/org/apache/rocketmq/studio/ops/ai/tool/handler/acl/AclUpdateToolHandlerTest.java
b/server/src/test/java/org/apache/rocketmq/studio/ops/ai/tool/handler/acl/AclUpdateToolHandlerTest.java
index 5c62738bc..621f1d4b2 100644
---
a/server/src/test/java/org/apache/rocketmq/studio/ops/ai/tool/handler/acl/AclUpdateToolHandlerTest.java
+++
b/server/src/test/java/org/apache/rocketmq/studio/ops/ai/tool/handler/acl/AclUpdateToolHandlerTest.java
@@ -17,8 +17,10 @@
package org.apache.rocketmq.studio.ops.ai.tool.handler.acl;
import org.apache.rocketmq.studio.instance.acl.AclRuleVO;
+import org.apache.rocketmq.studio.common.exception.BusinessException;
import org.apache.rocketmq.studio.instance.acl.AclService;
import org.apache.rocketmq.studio.ops.ai.tool.contract.acl.AclMutationInput;
+import org.apache.rocketmq.studio.ops.ai.tool.contract.plan.ToolPlan;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.ArgumentCaptor;
@@ -27,6 +29,7 @@ import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.verify;
@@ -51,6 +54,11 @@ class AclUpdateToolHandlerTest {
.resource("TopicB")
.decision("DENY")
.build();
+ when(aclService.getRule("1",
"cluster-1")).thenReturn(AclRuleVO.builder()
+ .id(1L)
+ .principal("user-1")
+ .resource("TopicA")
+ .build());
when(aclService.updateRule(any(AclRuleVO.class), eq("cluster-1")))
.thenReturn(updated);
@@ -67,4 +75,70 @@ class AclUpdateToolHandlerTest {
assertThat(captor.getValue().getId()).isEqualTo(1L);
}
+ /**
+ * Providers whose ACL rules have no numeric database id are addressed by
their principal:
+ * AclService.getRule resolves a role name and AclService.updateRule never
asks Tencent for an
+ * id (TencentAclService.updateRule maps the rule's principal onto
ModifyRole). The handler must
+ * therefore resolve the addressed rule instead of demanding a number, or
an ACL rule of such an
+ * instance can never be updated through the tool while get, list and
delete all work.
+ */
+ @Test
+ void applyShouldUpdateARuleAddressedByPrincipalTest() {
+ AclRuleVO existing = AclRuleVO.builder()
+ .principal("GID_role")
+ .resource("TopicA")
+ .decision("GRANT")
+ .build();
+ AclRuleVO updated = AclRuleVO.builder()
+ .principal("GID_role")
+ .resource("TopicB")
+ .decision("DENY")
+ .build();
+ when(aclService.getRule("GID_role", "cluster-1")).thenReturn(existing);
+ when(aclService.updateRule(any(AclRuleVO.class), eq("cluster-1")))
+ .thenReturn(updated);
+
+ Object result = handler.execute(new AclMutationInput(
+ "cluster-1", "GID_role", "GID_role", "TopicB", null, null,
+ null, "DENY", null), context("cluster-1"));
+
+ assertThat(result).isSameAs(updated);
+ ArgumentCaptor<AclRuleVO> captor =
ArgumentCaptor.forClass(AclRuleVO.class);
+ verify(aclService).updateRule(captor.capture(), eq("cluster-1"));
+ assertThat(captor.getValue().getId()).isNull();
+ assertThat(captor.getValue().getPrincipal()).isEqualTo("GID_role");
+ assertThat(captor.getValue().getResource()).isEqualTo("TopicB");
+ assertThat(captor.getValue().getDecision()).isEqualTo("DENY");
+ }
+
+ @Test
+ void planShouldPreviewARuleAddressedByPrincipalTest() {
+ AclRuleVO existing = AclRuleVO.builder()
+ .principal("GID_role")
+ .resource("TopicA")
+ .decision("GRANT")
+ .build();
+ when(aclService.getRule("GID_role", "cluster-1")).thenReturn(existing);
+
+ ToolPlan plan = handler.preview(new AclMutationInput(
+ "cluster-1", "GID_role", "GID_role", "TopicB", null, null,
+ null, "DENY", null), context("cluster-1"));
+
+ assertThat(plan.before()).containsEntry("principal", "GID_role");
+ assertThat(plan.after())
+ .containsEntry("principal", "GID_role")
+ .containsEntry("resource", "TopicB");
+ }
+
+ @Test
+ void applyShouldReportAnUnknownRuleFromTheServiceTest() {
+ when(aclService.getRule("999", "cluster-1"))
+ .thenThrow(new BusinessException(404, "ACL rule not found:
999"));
+
+ assertThatThrownBy(() -> handler.execute(new AclMutationInput(
+ "cluster-1", "999", "user-1", "TopicB", null, null,
+ null, "DENY", null), context("cluster-1")))
+ .isInstanceOf(BusinessException.class)
+ .hasMessageContaining("ACL rule not found");
+ }
}
