mirror of
https://github.com/apache/doris.git
synced 2026-11-04 07:31:17 +00:00
[Enhance](resource) Restrict modification of root-created AI resources (#67659)
- Persist whether an `AI resource` was created by root. - Only `root` can alter or drop root-created AI resources. - `ADMIN` users can still alter or drop AI resources created by `ADMIN`.
This commit is contained in:
1 parent
da298089d0
commit
ee4a91720f
9 files changed
+208
-9
No files matched your search
@@ -57,6 +57,8 @@ public class AIResource extends Resource {
|
||||
private static final Logger LOG = LogManager.getLogger(AIResource.class);
|
||||
@SerializedName(value = "properties")
|
||||
private Map<String, String> properties;
|
||||
@SerializedName(value = "createdByRoot")
|
||||
private boolean createdByRoot;
|
||||
|
||||
public AIResource() {
|
||||
super();
|
||||
@@ -67,6 +69,14 @@ public class AIResource extends Resource {
|
||||
properties = Maps.newHashMap();
|
||||
}
|
||||
|
||||
public boolean isCreatedByRoot() {
|
||||
return createdByRoot;
|
||||
}
|
||||
|
||||
void setCreatedByRoot(boolean createdByRoot) {
|
||||
this.createdByRoot = createdByRoot;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected void setProperties(ImmutableMap<String, String> newProperties) throws DdlException {
|
||||
Preconditions.checkState(newProperties != null);
|
||||
|
||||
@@ -17,6 +17,7 @@
|
||||
|
||||
package org.apache.doris.catalog;
|
||||
|
||||
import org.apache.doris.analysis.UserIdentity;
|
||||
import org.apache.doris.common.AnalysisException;
|
||||
import org.apache.doris.common.DdlException;
|
||||
import org.apache.doris.common.FeConstants;
|
||||
@@ -129,11 +130,18 @@ public abstract class Resource implements Writable, GsonPostProcessable {
|
||||
}
|
||||
|
||||
public static Resource fromCommand(CreateResourceCommand command) throws DdlException {
|
||||
return fromCommand(command, null);
|
||||
}
|
||||
|
||||
public static Resource fromCommand(CreateResourceCommand command, UserIdentity creator) throws DdlException {
|
||||
CreateResourceInfo info = command.getInfo();
|
||||
Resource resource = getResourceInstance(info.getResourceType(), info.getResourceName());
|
||||
resource.id = Env.getCurrentEnv().getNextId();
|
||||
resource.version = 0;
|
||||
resource.setProperties(info.getProperties());
|
||||
if (resource instanceof AIResource) {
|
||||
((AIResource) resource).setCreatedByRoot(creator != null && creator.isRootUser());
|
||||
}
|
||||
return resource;
|
||||
}
|
||||
|
||||
|
||||
@@ -17,6 +17,7 @@
|
||||
|
||||
package org.apache.doris.catalog;
|
||||
|
||||
import org.apache.doris.analysis.UserIdentity;
|
||||
import org.apache.doris.catalog.Resource.ResourceType;
|
||||
import org.apache.doris.common.AnalysisException;
|
||||
import org.apache.doris.common.DdlException;
|
||||
@@ -76,12 +77,16 @@ public class ResourceMgr implements Writable {
|
||||
}
|
||||
|
||||
public void createResource(CreateResourceCommand command) throws DdlException {
|
||||
createResource(command, null);
|
||||
}
|
||||
|
||||
public void createResource(CreateResourceCommand command, UserIdentity creator) throws DdlException {
|
||||
CreateResourceInfo info = command.getInfo();
|
||||
if (info.getResourceType() == ResourceType.UNKNOWN) {
|
||||
throw new DdlException(
|
||||
"Only support SPARK, ODBC_CATALOG, JDBC, S3_COOLDOWN, S3, HDFS(JFS/JUICEFS), and HMS resource.");
|
||||
}
|
||||
Resource resource = Resource.fromCommand(command);
|
||||
Resource resource = Resource.fromCommand(command, creator);
|
||||
if (createResource(resource, info.isIfNotExists())) {
|
||||
Env.getCurrentEnv().getEditLog().logCreateResource(resource);
|
||||
LOG.info("Create resource success. Resource: {}", resource.getName());
|
||||
|
||||
+5
-1
@@ -17,6 +17,7 @@
|
||||
|
||||
package org.apache.doris.nereids.trees.plans.commands;
|
||||
|
||||
import org.apache.doris.catalog.AIResource;
|
||||
import org.apache.doris.catalog.Env;
|
||||
import org.apache.doris.catalog.Resource;
|
||||
import org.apache.doris.common.AnalysisException;
|
||||
@@ -64,6 +65,10 @@ public class AlterResourceCommand extends AlterCommand implements NeedAuditEncry
|
||||
if (resource == null) {
|
||||
throw new AnalysisException("Unknown resource: " + resourceName);
|
||||
}
|
||||
if (resource instanceof AIResource && ((AIResource) resource).isCreatedByRoot()
|
||||
&& (ctx.getCurrentUserIdentity() == null || !ctx.getCurrentUserIdentity().isRootUser())) {
|
||||
throw new AnalysisException("Only root user can modify root-created AI resource: " + resourceName);
|
||||
}
|
||||
// check properties
|
||||
resource.checkProperties(properties);
|
||||
}
|
||||
@@ -86,4 +91,3 @@ public class AlterResourceCommand extends AlterCommand implements NeedAuditEncry
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
+1
-1
@@ -44,7 +44,7 @@ public class CreateResourceCommand extends Command implements ForwardWithSync, N
|
||||
@Override
|
||||
public void run(ConnectContext ctx, StmtExecutor executor) throws Exception {
|
||||
info.validate();
|
||||
Env.getCurrentEnv().getResourceMgr().createResource(this);
|
||||
Env.getCurrentEnv().getResourceMgr().createResource(this, ctx.getCurrentUserIdentity());
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
+8
@@ -19,7 +19,9 @@ package org.apache.doris.nereids.trees.plans.commands;
|
||||
|
||||
import org.apache.doris.analysis.ResourceTypeEnum;
|
||||
import org.apache.doris.analysis.StmtType;
|
||||
import org.apache.doris.catalog.AIResource;
|
||||
import org.apache.doris.catalog.Env;
|
||||
import org.apache.doris.catalog.Resource;
|
||||
import org.apache.doris.common.AnalysisException;
|
||||
import org.apache.doris.common.ErrorCode;
|
||||
import org.apache.doris.common.ErrorReport;
|
||||
@@ -67,6 +69,12 @@ public class DropResourceCommand extends DropCommand {
|
||||
}
|
||||
|
||||
FeNameFormat.checkResourceName(resourceName, ResourceTypeEnum.GENERAL);
|
||||
|
||||
Resource resource = Env.getCurrentEnv().getResourceMgr().getResource(resourceName);
|
||||
if (resource instanceof AIResource && ((AIResource) resource).isCreatedByRoot()
|
||||
&& (ctx.getCurrentUserIdentity() == null || !ctx.getCurrentUserIdentity().isRootUser())) {
|
||||
throw new AnalysisException("Only root user can modify root-created AI resource: " + resourceName);
|
||||
}
|
||||
}
|
||||
|
||||
@Override
|
||||
|
||||
@@ -17,10 +17,12 @@
|
||||
|
||||
package org.apache.doris.catalog;
|
||||
|
||||
import org.apache.doris.analysis.UserIdentity;
|
||||
import org.apache.doris.common.DdlException;
|
||||
import org.apache.doris.common.FeConstants;
|
||||
import org.apache.doris.common.FeMetaVersion;
|
||||
import org.apache.doris.common.UserException;
|
||||
import org.apache.doris.common.io.Text;
|
||||
import org.apache.doris.datasource.property.constants.AIProperties;
|
||||
import org.apache.doris.meta.MetaContext;
|
||||
import org.apache.doris.mysql.privilege.AccessControllerManager;
|
||||
@@ -28,9 +30,12 @@ import org.apache.doris.mysql.privilege.PrivPredicate;
|
||||
import org.apache.doris.nereids.trees.plans.commands.CreateResourceCommand;
|
||||
import org.apache.doris.nereids.trees.plans.commands.info.CreateResourceInfo;
|
||||
import org.apache.doris.persist.EditLog;
|
||||
import org.apache.doris.persist.gson.GsonUtils;
|
||||
import org.apache.doris.qe.ConnectContext;
|
||||
|
||||
import com.google.common.collect.ImmutableMap;
|
||||
import com.google.gson.JsonObject;
|
||||
import com.google.gson.JsonParser;
|
||||
import org.apache.logging.log4j.LogManager;
|
||||
import org.apache.logging.log4j.Logger;
|
||||
import org.junit.jupiter.api.Assertions;
|
||||
@@ -97,11 +102,21 @@ public class AIResourceTest {
|
||||
.thenReturn(true);
|
||||
|
||||
// resource with default settings
|
||||
CreateResourceCommand createResourceCommand = new CreateResourceCommand(
|
||||
CreateResourceCommand initialCreateResourceCommand = new CreateResourceCommand(
|
||||
new CreateResourceInfo(true, false, name, ImmutableMap.copyOf(aiProperties)));
|
||||
createResourceCommand.getInfo().validate();
|
||||
initialCreateResourceCommand.getInfo().validate();
|
||||
|
||||
AIResource legacyAIResource = (AIResource) Resource.fromCommand(initialCreateResourceCommand);
|
||||
Assertions.assertFalse(legacyAIResource.isCreatedByRoot());
|
||||
|
||||
AIResource rootCreatedAIResource =
|
||||
(AIResource) Resource.fromCommand(initialCreateResourceCommand, UserIdentity.ROOT);
|
||||
Assertions.assertTrue(rootCreatedAIResource.isCreatedByRoot());
|
||||
|
||||
AIResource aiResource =
|
||||
(AIResource) Resource.fromCommand(initialCreateResourceCommand, UserIdentity.ADMIN);
|
||||
Assertions.assertFalse(aiResource.isCreatedByRoot());
|
||||
|
||||
AIResource aiResource = (AIResource) Resource.fromCommand(createResourceCommand);
|
||||
Assertions.assertEquals(name, aiResource.getName());
|
||||
Assertions.assertEquals(type, aiResource.getType().name().toLowerCase());
|
||||
Assertions.assertEquals(endpoint, aiResource.getProperty(AIProperties.ENDPOINT));
|
||||
@@ -124,7 +139,7 @@ public class AIResourceTest {
|
||||
aiProperties.put(AIProperties.MAX_RETRIES, maxRetries);
|
||||
aiProperties.put(AIProperties.RETRY_DELAY_SECOND, retryDelaySecond);
|
||||
|
||||
createResourceCommand = new CreateResourceCommand(
|
||||
CreateResourceCommand createResourceCommand = new CreateResourceCommand(
|
||||
new CreateResourceInfo(true, false, name, ImmutableMap.copyOf(aiProperties)));
|
||||
createResourceCommand.getInfo().validate();
|
||||
|
||||
@@ -233,7 +248,9 @@ public class AIResourceTest {
|
||||
DataOutputStream aiDos = new DataOutputStream(Files.newOutputStream(path));
|
||||
|
||||
AIResource aiResource1 = new AIResource("ai_1");
|
||||
aiResource1.write(aiDos);
|
||||
JsonObject legacyResourceJson = JsonParser.parseString(GsonUtils.GSON.toJson(aiResource1)).getAsJsonObject();
|
||||
legacyResourceJson.remove("createdByRoot");
|
||||
Text.writeString(aiDos, legacyResourceJson.toString());
|
||||
|
||||
ImmutableMap<String, String> properties = ImmutableMap.of(
|
||||
"ai.endpoint", endpoint,
|
||||
@@ -243,6 +260,7 @@ public class AIResourceTest {
|
||||
"ai.validity_check", "false"
|
||||
);
|
||||
AIResource aiResource2 = new AIResource("ai_2");
|
||||
aiResource2.setCreatedByRoot(true);
|
||||
aiResource2.setProperties(properties);
|
||||
aiResource2.write(aiDos);
|
||||
|
||||
@@ -256,6 +274,8 @@ public class AIResourceTest {
|
||||
|
||||
Assertions.assertEquals("ai_1", rAiResource1.getName());
|
||||
Assertions.assertEquals("ai_2", rAiResource2.getName());
|
||||
Assertions.assertFalse(rAiResource1.isCreatedByRoot());
|
||||
Assertions.assertTrue(rAiResource2.isCreatedByRoot());
|
||||
|
||||
Assertions.assertEquals(rAiResource2.getProperty(AIProperties.ENDPOINT), endpoint);
|
||||
Assertions.assertEquals(rAiResource2.getProperty(AIProperties.PROVIDER_TYPE), providerType.toUpperCase());
|
||||
|
||||
+118
@@ -0,0 +1,118 @@
|
||||
// Licensed to the Apache Software Foundation (ASF) under one
|
||||
// or more contributor license agreements. See the NOTICE file
|
||||
// distributed with this work for additional information
|
||||
// regarding copyright ownership. The ASF licenses this file
|
||||
// to you under the Apache License, Version 2.0 (the
|
||||
// "License"); you may not use this file except in compliance
|
||||
// with the License. You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing,
|
||||
// software distributed under the License is distributed on an
|
||||
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
|
||||
// KIND, either express or implied. See the License for the
|
||||
// specific language governing permissions and limitations
|
||||
// under the License.
|
||||
|
||||
package org.apache.doris.nereids.trees.plans.commands;
|
||||
|
||||
import org.apache.doris.analysis.UserIdentity;
|
||||
import org.apache.doris.catalog.AIResource;
|
||||
import org.apache.doris.catalog.Env;
|
||||
import org.apache.doris.catalog.ResourceMgr;
|
||||
import org.apache.doris.common.AnalysisException;
|
||||
import org.apache.doris.mysql.privilege.AccessControllerManager;
|
||||
import org.apache.doris.mysql.privilege.PrivPredicate;
|
||||
import org.apache.doris.qe.ConnectContext;
|
||||
import org.apache.doris.qe.SessionVariable;
|
||||
|
||||
import org.junit.jupiter.api.AfterEach;
|
||||
import org.junit.jupiter.api.Assertions;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.mockito.MockedStatic;
|
||||
import org.mockito.Mockito;
|
||||
|
||||
import java.util.Collections;
|
||||
|
||||
public class AIResourceCommandTest {
|
||||
private static final String RESOURCE_NAME = "root_ai_resource";
|
||||
|
||||
private ConnectContext connectContext;
|
||||
private AIResource aiResource;
|
||||
private MockedStatic<Env> envMockedStatic;
|
||||
private MockedStatic<ConnectContext> connectContextMockedStatic;
|
||||
|
||||
@BeforeEach
|
||||
public void setUp() {
|
||||
Env env = Mockito.mock(Env.class);
|
||||
ResourceMgr resourceMgr = Mockito.mock(ResourceMgr.class);
|
||||
AccessControllerManager accessManager = Mockito.mock(AccessControllerManager.class);
|
||||
connectContext = Mockito.mock(ConnectContext.class);
|
||||
aiResource = Mockito.mock(AIResource.class);
|
||||
|
||||
envMockedStatic = Mockito.mockStatic(Env.class);
|
||||
connectContextMockedStatic = Mockito.mockStatic(ConnectContext.class);
|
||||
envMockedStatic.when(Env::getCurrentEnv).thenReturn(env);
|
||||
connectContextMockedStatic.when(ConnectContext::get).thenReturn(connectContext);
|
||||
|
||||
Mockito.when(env.getAccessManager()).thenReturn(accessManager);
|
||||
Mockito.when(env.getResourceMgr()).thenReturn(resourceMgr);
|
||||
Mockito.when(connectContext.getEnv()).thenReturn(env);
|
||||
Mockito.when(connectContext.getSessionVariable()).thenReturn(new SessionVariable());
|
||||
Mockito.when(accessManager.checkGlobalPriv(connectContext, PrivPredicate.ADMIN)).thenReturn(true);
|
||||
Mockito.when(resourceMgr.getResource(RESOURCE_NAME)).thenReturn(aiResource);
|
||||
}
|
||||
|
||||
@AfterEach
|
||||
public void tearDown() {
|
||||
connectContextMockedStatic.close();
|
||||
envMockedStatic.close();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testAdminCannotAlterOrDropRootCreatedAIResource() {
|
||||
Mockito.when(aiResource.isCreatedByRoot()).thenReturn(true);
|
||||
Mockito.when(connectContext.getCurrentUserIdentity()).thenReturn(UserIdentity.ADMIN);
|
||||
|
||||
AlterResourceCommand alterCommand = new AlterResourceCommand(
|
||||
RESOURCE_NAME, Collections.singletonMap("ai.temperature", "0.8"));
|
||||
AnalysisException alterException = Assertions.assertThrows(
|
||||
AnalysisException.class, () -> alterCommand.doRun(connectContext, null));
|
||||
Assertions.assertTrue(alterException.getMessage()
|
||||
.contains("Only root user can modify root-created AI resource"));
|
||||
|
||||
DropResourceCommand dropCommand = new DropResourceCommand(false, RESOURCE_NAME);
|
||||
AnalysisException dropException = Assertions.assertThrows(
|
||||
AnalysisException.class, () -> dropCommand.doRun(connectContext, null));
|
||||
Assertions.assertTrue(dropException.getMessage()
|
||||
.contains("Only root user can modify root-created AI resource"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testRootCanAlterAndDropRootCreatedAIResource() {
|
||||
Mockito.when(aiResource.isCreatedByRoot()).thenReturn(true);
|
||||
Mockito.when(connectContext.getCurrentUserIdentity()).thenReturn(UserIdentity.ROOT);
|
||||
|
||||
AlterResourceCommand alterCommand = new AlterResourceCommand(
|
||||
RESOURCE_NAME, Collections.singletonMap("ai.temperature", "0.8"));
|
||||
Assertions.assertDoesNotThrow(() -> alterCommand.doRun(connectContext, null));
|
||||
|
||||
DropResourceCommand dropCommand = new DropResourceCommand(false, RESOURCE_NAME);
|
||||
Assertions.assertDoesNotThrow(() -> dropCommand.doRun(connectContext, null));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testAdminCanAlterAndDropLegacyAIResource() {
|
||||
Mockito.when(aiResource.isCreatedByRoot()).thenReturn(false);
|
||||
Mockito.when(connectContext.getCurrentUserIdentity()).thenReturn(UserIdentity.ADMIN);
|
||||
|
||||
AlterResourceCommand alterCommand = new AlterResourceCommand(
|
||||
RESOURCE_NAME, Collections.singletonMap("ai.temperature", "0.8"));
|
||||
Assertions.assertDoesNotThrow(() -> alterCommand.doRun(connectContext, null));
|
||||
|
||||
DropResourceCommand dropCommand = new DropResourceCommand(false, RESOURCE_NAME);
|
||||
Assertions.assertDoesNotThrow(() -> dropCommand.doRun(connectContext, null));
|
||||
}
|
||||
}
|
||||
@@ -89,7 +89,33 @@ suite("test_ddl_ai_resource_auth","p0,auth_call") {
|
||||
assertTrue(res.size() == 0)
|
||||
}
|
||||
|
||||
sql """CREATE RESOURCE "${resourceName}"
|
||||
PROPERTIES(
|
||||
'type' = 'ai',
|
||||
'ai.provider_type' = 'deepseek',
|
||||
'ai.endpoint' = 'https://api.deepseek.com/chat/completions',
|
||||
'ai.model_name' = 'deepseek-chat',
|
||||
'ai.api_key' = 'sk-xxx',
|
||||
'ai.temperature' = '0.7',
|
||||
'ai.max_token' = '1024',
|
||||
'ai.max_retries' = '3',
|
||||
'ai.retry_delay_second' = '1',
|
||||
'ai.validity_check' = 'false'
|
||||
);"""
|
||||
connect(user, "${pwd}", context.config.jdbcUrl) {
|
||||
test {
|
||||
sql """ALTER RESOURCE '${resourceName}' PROPERTIES ('ai.temperature' = '0.8');"""
|
||||
exception "Only root user can modify root-created AI resource"
|
||||
}
|
||||
test {
|
||||
sql """DROP RESOURCE '${resourceName}'"""
|
||||
exception "Only root user can modify root-created AI resource"
|
||||
}
|
||||
}
|
||||
sql """ALTER RESOURCE '${resourceName}' PROPERTIES ('ai.temperature' = '0.8');"""
|
||||
sql """DROP RESOURCE '${resourceName}'"""
|
||||
|
||||
try_sql("""DROP RESOURCE '${resourceName}'""")
|
||||
sql """drop database if exists ${dbName}"""
|
||||
try_sql("DROP USER ${user}")
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user