This is an automated email from the ASF dual-hosted git repository.
caolu pushed a commit to branch kylin5
in repository https://gitbox.apache.org/repos/asf/kylin.git
The following commit(s) were added to refs/heads/kylin5 by this push:
new 721b7a5e02 KYLIN-6036 Skip tables without permissions when loading
tables
721b7a5e02 is described below
commit 721b7a5e02696e9ce6398b9537579586937539b8
Author: jlf <[email protected]>
AuthorDate: Fri Jul 12 13:49:46 2024 +0800
KYLIN-6036 Skip tables without permissions when loading tables
* validate host pattern
---
Co-authored-by: longfei.jiang <[email protected]>
Co-authored-by: sheng.huang <[email protected]>
---
.../org/apache/kylin/common/KylinConfigBase.java | 4 +-
.../org/apache/kylin/common/util/AddressUtil.java | 6 +
.../org/apache/kylin/common/util/StringHelper.java | 4 +
.../apache/kylin/common/KylinConfigBaseTest.java | 17 +++
.../apache/kylin/common/util/AddressUtilTest.java | 21 ++++
.../apache/kylin/common/util/StringHelperTest.java | 31 +++++
src/datasource-service/pom.xml | 10 ++
.../apache/kylin/rest/service/TableExtService.java | 3 +-
.../apache/kylin/rest/service/TableService.java | 39 +++---
.../kylin/rest/service/TableExtServiceTest.java | 16 +--
.../kylin/rest/service/TableServiceTest.java | 133 +++++++++++++++++++++
.../kylin/rest/controller/OpsController.java | 6 +
.../kylin/rest/service/SparderUIService.java | 3 +
.../kylin/rest/service/SparderUIServiceTest.java | 61 +++++++---
14 files changed, 314 insertions(+), 40 deletions(-)
diff --git
a/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
b/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
index 709b959634..0ca67fdceb 100644
--- a/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
+++ b/src/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
@@ -726,7 +726,9 @@ public abstract class KylinConfigBase implements
Serializable {
// Caution: config 'kylin.server.address' is essential in yarn cluster
mode.
// The value may be the address of loadbalancer
// format: ip:port
- return getOptional("kylin.server.address", getDefaultServerAddress());
+ String serverAddress = getOptional("kylin.server.address",
getDefaultServerAddress());
+ AddressUtil.validateHost(serverAddress);
+ return serverAddress;
}
private String getDefaultServerAddress() {
diff --git
a/src/core-common/src/main/java/org/apache/kylin/common/util/AddressUtil.java
b/src/core-common/src/main/java/org/apache/kylin/common/util/AddressUtil.java
index 85a60f82b7..24eda85407 100644
---
a/src/core-common/src/main/java/org/apache/kylin/common/util/AddressUtil.java
+++
b/src/core-common/src/main/java/org/apache/kylin/common/util/AddressUtil.java
@@ -121,4 +121,10 @@ public class AddressUtil {
public static void clearLocalIpAddressCache() {
localIpAddressCache = null;
}
+
+ public static void validateHost(String host) {
+ if (StringUtils.isNotBlank(host) && !StringHelper.validateHost(host)) {
+ throw new IllegalArgumentException("Url contains disallowed chars,
host: " + host);
+ }
+ }
}
diff --git
a/src/core-common/src/main/java/org/apache/kylin/common/util/StringHelper.java
b/src/core-common/src/main/java/org/apache/kylin/common/util/StringHelper.java
index 9eb1f7c296..791b6f9ece 100644
---
a/src/core-common/src/main/java/org/apache/kylin/common/util/StringHelper.java
+++
b/src/core-common/src/main/java/org/apache/kylin/common/util/StringHelper.java
@@ -149,6 +149,10 @@ public class StringHelper {
return
Pattern.compile("^(http(s)?://)?[a-zA-Z0-9._-]+(:[0-9]+)?(/[a-zA-Z0-9._-]+)*/?$").matcher(s).matches();
}
+ public static boolean validateHost(String s) {
+ return
Pattern.compile("^(http(s)?://)?[a-zA-Z0-9._-]+(:[0-9]+)?").matcher(s).matches();
+ }
+
public static boolean validateDbName(String s) {
return Pattern.compile("^[0-9a-zA-Z_-]+$").matcher(s).matches();
}
diff --git
a/src/core-common/src/test/java/org/apache/kylin/common/KylinConfigBaseTest.java
b/src/core-common/src/test/java/org/apache/kylin/common/KylinConfigBaseTest.java
index 0d2f359f19..7017a3fa06 100644
---
a/src/core-common/src/test/java/org/apache/kylin/common/KylinConfigBaseTest.java
+++
b/src/core-common/src/test/java/org/apache/kylin/common/KylinConfigBaseTest.java
@@ -1562,6 +1562,23 @@ class KylinConfigBaseTest {
val withoutExpected = "," + celebornJar.getAbsolutePath() + "," +
mysqlJar.getAbsolutePath();
Assertions.assertEquals(withoutExpected, withoutGluten);
}
+
+ void testGetServerAddress() {
+ KylinConfig config = KylinConfig.getInstanceFromEnv();
+ config.setProperty("kylin.server.address", "127.0.0.1");
+ Assertions.assertEquals("127.0.0.1", config.getServerAddress());
+ config.setProperty("kylin.server.address", "8080");
+ Assertions.assertEquals("8080", config.getServerAddress());
+
+ try {
+ config.setProperty("kylin.server.address", "8080>");
+ config.getServerAddress();
+ Assertions.fail();
+ } catch (Exception e) {
+ Assertions.assertInstanceOf(IllegalArgumentException.class, e);
+ Assertions.assertTrue(e.getMessage().contains("Url contains
disallowed chars, host: "));
+ }
+ }
}
class EnvironmentUpdateUtils {
diff --git
a/src/core-common/src/test/java/org/apache/kylin/common/util/AddressUtilTest.java
b/src/core-common/src/test/java/org/apache/kylin/common/util/AddressUtilTest.java
index 0d49fd6c25..fd0dd5f2e3 100644
---
a/src/core-common/src/test/java/org/apache/kylin/common/util/AddressUtilTest.java
+++
b/src/core-common/src/test/java/org/apache/kylin/common/util/AddressUtilTest.java
@@ -80,6 +80,27 @@ class AddressUtilTest {
}
}
+ @Test
+ void testCheckHost() {
+ AddressUtil.validateHost("127.0.0.1:7070");
+ AddressUtil.validateHost("127.0.0.1");
+ AddressUtil.validateHost("707");
+ AddressUtil.validateHost("");
+ testCheckHostFail("127.0.0.1:");
+ testCheckHostFail(":7070");
+ testCheckHostFail("127.0.0.1:7070>");
+ }
+
+ void testCheckHostFail(String host) {
+ try {
+ AddressUtil.validateHost(host);
+ Assertions.fail();
+ } catch (Exception e) {
+ Assertions.assertInstanceOf(IllegalArgumentException.class, e);
+ Assertions.assertTrue(e.getMessage().contains("Url contains
disallowed chars, host: "));
+ }
+ }
+
@Test
void testIsSameHost() {
Assertions.assertTrue(AddressUtil.isSameHost(hostInfoFetcher.getHostname()));
diff --git
a/src/core-common/src/test/java/org/apache/kylin/common/util/StringHelperTest.java
b/src/core-common/src/test/java/org/apache/kylin/common/util/StringHelperTest.java
index 07c11e28d2..72cbda2471 100644
---
a/src/core-common/src/test/java/org/apache/kylin/common/util/StringHelperTest.java
+++
b/src/core-common/src/test/java/org/apache/kylin/common/util/StringHelperTest.java
@@ -143,6 +143,37 @@ class StringHelperTest {
Assertions.assertFalse(StringHelper.validateUrl(""));
}
+ @Test
+ void testValidateHost() {
+ Assertions.assertTrue(StringHelper.validateHost("127.0.0.1"));
+ Assertions.assertTrue(StringHelper.validateHost("kylin.apache.org"));
+ Assertions.assertTrue(StringHelper.validateHost("kylin"));
+ Assertions.assertTrue(StringHelper.validateHost("http://127.0.0.1"));
+
Assertions.assertTrue(StringHelper.validateHost("http://127.0.0.1:80"));
+
Assertions.assertTrue(StringHelper.validateHost("https://kylin.apache.org"));
+ Assertions.assertTrue(StringHelper.validateHost("http://kylin"));
+ Assertions.assertTrue(StringHelper.validateHost("http://ky-lin"));
+ Assertions.assertTrue(StringHelper.validateHost("http://ky.lin"));
+ Assertions.assertTrue(StringHelper.validateHost("http://ky_lin"));
+ Assertions.assertTrue(StringHelper.validateHost("http://ky_lin:80"));
+ }
+
+ @Test
+ void testValidIllegalHost() {
+
Assertions.assertFalse(StringHelper.validateHost("http://127.0.0.1/a_p.i"));
+
Assertions.assertFalse(StringHelper.validateHost("http://127.0.0.1:80/"));
+ Assertions.assertFalse(StringHelper.validateHost("http://kylin/"));
+ Assertions.assertFalse(StringHelper.validateHost("http://kylin/$(rm
-rf /)"));
+ Assertions.assertFalse(StringHelper.validateHost("http://kylin/`rm
-rf`"));
+ Assertions.assertFalse(StringHelper.validateHost("http://kylin/'&ls"));
+ Assertions.assertFalse(StringHelper.validateHost("http://kylin/;ls"));
+ Assertions.assertFalse(StringHelper.validateHost("http://kylin/>ls"));
+ Assertions.assertFalse(StringHelper.validateHost(""));
+ Assertions.assertFalse(StringHelper.validateHost("http://ky-lin:80/"));
+ Assertions.assertFalse(StringHelper.validateHost("http://ky.lin/"));
+ Assertions.assertFalse(StringHelper.validateHost("http://ky_lin/"));
+ }
+
@Test
void testValidateDB() {
Assertions.assertTrue(StringHelper.validateDbName("db_TEST-01"));
diff --git a/src/datasource-service/pom.xml b/src/datasource-service/pom.xml
index cc4a08dabd..1ee600064c 100644
--- a/src/datasource-service/pom.xml
+++ b/src/datasource-service/pom.xml
@@ -91,6 +91,16 @@
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.powermock</groupId>
+ <artifactId>powermock-module-junit4</artifactId>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.powermock</groupId>
+ <artifactId>powermock-api-mockito2</artifactId>
+ <scope>test</scope>
+ </dependency>
<dependency>
<groupId>org.awaitility</groupId>
<artifactId>awaitility</artifactId>
diff --git
a/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableExtService.java
b/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableExtService.java
index 019382d96a..2154b51908 100644
---
a/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableExtService.java
+++
b/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableExtService.java
@@ -428,7 +428,8 @@ public class TableExtService extends BasicService {
KylinConfig config = KylinConfig.getInstanceFromEnv();
return ugi.doAs((PrivilegedExceptionAction<List<Pair<TableDesc,
TableExtDesc>>>) () -> {
ProjectInstance projectInstance =
getManager(NProjectManager.class).getProject(project);
- List<Pair<TableDesc, TableExtDesc>> extractTableMetas =
tableService.extractTableMeta(tables, project);
+ List<Pair<TableDesc, TableExtDesc>> extractTableMetas =
tableService.extractTableMeta(tables, project,
+ tableResponse);
if (config.getTableAccessFilterEnable() &&
projectInstance.isProjectKerberosEnabled()) {
return extractTableMetas.stream().map(pair -> {
TableDesc tableDesc = pair.getFirst();
diff --git
a/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableService.java
b/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableService.java
index 130e9bbb3e..feddd5487c 100644
---
a/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableService.java
+++
b/src/datasource-service/src/main/java/org/apache/kylin/rest/service/TableService.java
@@ -43,6 +43,7 @@ import java.util.AbstractMap;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.HashSet;
@@ -156,6 +157,7 @@ import org.apache.kylin.rest.request.S3TableExtInfo;
import org.apache.kylin.rest.request.TableDescRequest;
import org.apache.kylin.rest.response.AutoMergeConfigResponse;
import org.apache.kylin.rest.response.EnvelopeResponse;
+import org.apache.kylin.rest.response.LoadTableResponse;
import org.apache.kylin.rest.response.NHiveTableNameResponse;
import org.apache.kylin.rest.response.NInitTablesResponse;
import org.apache.kylin.rest.response.OpenPreReloadTableResponse;
@@ -345,6 +347,11 @@ public class TableService extends BasicService {
}
public List<Pair<TableDesc, TableExtDesc>> extractTableMeta(String[]
tables, String project) {
+ return extractTableMeta(tables, project, null);
+ }
+
+ public List<Pair<TableDesc, TableExtDesc>> extractTableMeta(String[]
tables, String project,
+ LoadTableResponse tableResponse) {
// de-dup
SetMultimap<String, String> databaseTables =
LinkedHashMultimap.create();
for (String fullTableName : tables) {
@@ -356,7 +363,10 @@ public class TableService extends BasicService {
// load all tables first Pair<TableDesc, TableExtDesc>
ProjectInstance projectInstance =
getManager(NProjectManager.class).getProject(project);
ISourceMetadataExplorer explr =
SourceFactory.getSource(projectInstance).getSourceMetadataExplorer();
- List<Pair<Map.Entry<String, String>, Object>> results =
databaseTables.entries().parallelStream().map(entry -> {
+
+ List<Pair<TableDesc, TableExtDesc>> successResults =
Collections.synchronizedList(new ArrayList<>());
+ List<String> failedTableNames = Collections.synchronizedList(new
ArrayList<>());
+ databaseTables.entries().parallelStream().forEach(entry -> {
try {
Pair<TableDesc, TableExtDesc> pair =
explr.loadTableMetadata(entry.getKey(), entry.getValue(), project);
TableDesc tableDesc = pair.getFirst();
@@ -366,24 +376,23 @@ public class TableService extends BasicService {
entry.getKey().toUpperCase(Locale.ROOT) + "." +
entry.getValue().toUpperCase(Locale.ROOT)));
TableExtDesc extDesc = pair.getSecond();
Preconditions.checkState(tableDesc.getIdentity().equals(extDesc.getIdentity()));
- return new Pair<Map.Entry<String, String>, Object>(entry,
pair);
+ successResults.add(pair);
} catch (Exception e) {
- return new Pair<Map.Entry<String, String>, Object>(entry, e);
+ logger.error("Failed to extract meta data of table:{}.{}",
entry.getKey(), entry.getValue(), e);
+ failedTableNames.add(entry.getKey() + "." + entry.getValue());
}
- }).collect(Collectors.toList());
- List<Pair<Map.Entry<String, String>, Object>> errorList =
results.stream()
- .filter(pair -> pair.getSecond() instanceof
Throwable).collect(Collectors.toList());
- if (!errorList.isEmpty()) {
- errorList.forEach(e -> logger.error(e.getFirst().getKey() + "." +
e.getFirst().getValue(),
- (Throwable) (e.getSecond())));
- String errorTables = StringUtils
- .join(errorList.stream().map(error ->
error.getFirst().getKey() + "." + error.getFirst().getValue())
- .collect(Collectors.toList()), ",");
+ });
+ if (!failedTableNames.isEmpty()) {
+ String errorTables = StringUtils.join(failedTableNames, "\n");
String errorMessage = String.format(Locale.ROOT,
MsgPicker.getMsg().getHiveTableNotFound(), errorTables);
- throw new KylinException(TABLE_NOT_EXIST, errorMessage);
+ if (Objects.isNull(tableResponse)) {
+ throw new KylinException(TABLE_NOT_EXIST, errorMessage);
+ } else {
+ tableResponse.getFailed().addAll(failedTableNames);
+ logger.error("Skip load these failed tables:\n{}",
errorTables);
+ }
}
- return results.stream().map(pair -> (Pair<TableDesc, TableExtDesc>)
pair.getSecond())
- .collect(Collectors.toList());
+ return successResults;
}
public List<String> getSourceDbNames(String project) throws Exception {
diff --git
a/src/datasource-service/src/test/java/org/apache/kylin/rest/service/TableExtServiceTest.java
b/src/datasource-service/src/test/java/org/apache/kylin/rest/service/TableExtServiceTest.java
index 2195d90ca6..8574410b0e 100644
---
a/src/datasource-service/src/test/java/org/apache/kylin/rest/service/TableExtServiceTest.java
+++
b/src/datasource-service/src/test/java/org/apache/kylin/rest/service/TableExtServiceTest.java
@@ -95,7 +95,7 @@ public class TableExtServiceTest extends
NLocalFileMetadataTestCase {
String[] tables = { "DEFAULT.TEST_KYLIN_FACT", "DEFAULT.TEST_ACCOUNT"
};
String[] tableNames = { "TEST_KYLIN_FACT", "TEST_ACCOUNT" };
List<Pair<TableDesc, TableExtDesc>> result = mockTablePair(2,
"DEFAULT");
-
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any());
+
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any(), Mockito.any());
Mockito.doNothing().when(tableExtService).loadTable(result.get(0).getFirst(),
result.get(0).getSecond(),
"default");
Mockito.doNothing().when(tableExtService).loadTable(result.get(1).getFirst(),
result.get(1).getSecond(),
@@ -123,7 +123,7 @@ public class TableExtServiceTest extends
NLocalFileMetadataTestCase {
crossAccountTableReq.add(s3TableExtInfo1);
crossAccountTableReq.add(s3TableExtInfo2);
List<Pair<TableDesc, TableExtDesc>> result = mockTablePair(2,
"DEFAULT", "TABLE");
-
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any());
+
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any(), Mockito.any());
Mockito.doNothing().when(tableExtService).loadTable(result.get(0).getFirst(),
result.get(0).getSecond(),
"default");
Mockito.doNothing().when(tableExtService).loadTable(result.get(1).getFirst(),
result.get(1).getSecond(),
@@ -193,7 +193,7 @@ public class TableExtServiceTest extends
NLocalFileMetadataTestCase {
List<Pair<TableDesc, TableExtDesc>> result = mockTablePair(3, "EDW");
Mockito.doNothing().when(tableExtService).loadTable(result.get(1).getFirst(),
result.get(1).getSecond(),
"default");
-
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any());
+
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any(), Mockito.any());
loadTableResponse.setLoaded(Sets.newHashSet(tableIdentities));
Mockito.doReturn(Lists.newArrayList(tableNames)).when(tableService).getSourceTableNames(Mockito.any(),
@@ -220,7 +220,7 @@ public class TableExtServiceTest extends
NLocalFileMetadataTestCase {
NTableMetadataManager tableManager =
NTableMetadataManager.getInstance(getTestConfig(), "default");
tableManager.removeSourceTable("EDW.TEST_CAL_DT");
Mockito.doReturn(Lists.newArrayList("EDW")).when(tableService).getSourceDbNames("default");
-
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any());
+
Mockito.doReturn(result).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any(), Mockito.any());
LoadTableResponse response = tableExtService.loadDbTables(new String[]
{ "EDW" }, "default", true);
Assert.assertEquals(0, response.getLoaded().size());
}
@@ -551,7 +551,7 @@ public class TableExtServiceTest extends
NLocalFileMetadataTestCase {
@Test
public void testLoadTablesWithShortCircuit() throws Exception {
List<Pair<TableDesc, TableExtDesc>> lt1000 = mockTablePair(8, "TB");
-
Mockito.doReturn(lt1000).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any());
+
Mockito.doReturn(lt1000).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any(), Mockito.any());
TableLoadRequest request = new TableLoadRequest();
request.setDatabases(new String[] { "DEFAULT" });
request.setProject("default");
@@ -559,7 +559,7 @@ public class TableExtServiceTest extends
NLocalFileMetadataTestCase {
Assert.assertEquals(8, lt1000response.getFailed().size());
List<Pair<TableDesc, TableExtDesc>> gt1000 = mockTablePair(1001, "TB");
-
Mockito.doReturn(gt1000).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any());
+
Mockito.doReturn(gt1000).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any(), Mockito.any());
Assert.assertThrows(KylinException.class, () ->
tableExtService.loadTablesWithShortCircuit(request));
request.setTables(mockInputDBOrTable());
@@ -571,13 +571,13 @@ public class TableExtServiceTest extends
NLocalFileMetadataTestCase {
request.setDatabases(null);
gt1000.forEach(
t ->
Mockito.doNothing().when(tableExtService).loadTable(t.getFirst(),
t.getSecond(), "default"));
-
Mockito.doReturn(gt1000).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any());
+
Mockito.doReturn(gt1000).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any(), Mockito.any());
Assert.assertThrows(KylinException.class, () ->
tableExtService.loadTablesWithShortCircuit(request));
request.setDatabases(null);
request.setTables(new String[] { "TEST_KYLIN_FACT" });
List<Pair<TableDesc, TableExtDesc>> table8 = mockTablePair(8, "TB");
-
Mockito.doReturn(table8).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any());
+
Mockito.doReturn(table8).when(tableService).extractTableMeta(Mockito.any(),
Mockito.any(), Mockito.any());
LoadTableResponse response1 =
tableExtService.loadTablesWithShortCircuit(request);
Assert.assertEquals(8, response1.getFailed().size());
diff --git
a/src/datasource-service/src/test/java/org/apache/kylin/rest/service/TableServiceTest.java
b/src/datasource-service/src/test/java/org/apache/kylin/rest/service/TableServiceTest.java
new file mode 100644
index 0000000000..29193b9c6b
--- /dev/null
+++
b/src/datasource-service/src/test/java/org/apache/kylin/rest/service/TableServiceTest.java
@@ -0,0 +1,133 @@
+/*
+ * 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.kylin.rest.service;
+
+import java.io.IOException;
+import java.security.AccessControlException;
+import java.util.List;
+
+import org.apache.kylin.common.exception.KylinException;
+import org.apache.kylin.common.util.NLocalFileMetadataTestCase;
+import org.apache.kylin.common.util.Pair;
+import org.apache.kylin.engine.spark.mockup.CsvSource;
+import org.apache.kylin.metadata.model.TableDesc;
+import org.apache.kylin.metadata.model.TableExtDesc;
+import org.apache.kylin.rest.constant.Constant;
+import org.apache.kylin.rest.response.LoadTableResponse;
+import org.apache.kylin.rest.util.AclEvaluate;
+import org.apache.kylin.rest.util.AclUtil;
+import org.apache.kylin.source.ISourceMetadataExplorer;
+import org.apache.kylin.source.SourceFactory;
+import org.junit.After;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.InjectMocks;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.powermock.api.mockito.PowerMockito;
+import org.powermock.core.classloader.annotations.PowerMockIgnore;
+import org.powermock.core.classloader.annotations.PrepareForTest;
+import org.powermock.modules.junit4.PowerMockRunner;
+import org.springframework.security.authentication.TestingAuthenticationToken;
+import org.springframework.security.core.context.SecurityContextHolder;
+import org.springframework.test.util.ReflectionTestUtils;
+
+@RunWith(PowerMockRunner.class)
+@PrepareForTest({ SourceFactory.class, ISourceMetadataExplorer.class })
+@PowerMockIgnore({ "com.sun.security.*", "org.w3c.*", "javax.xml.*",
"org.xml.*", "org.w3c.dom.*", "org.apache.cxf.*",
+ "javax.management.*", "javax.script.*",
"org.apache.hadoop.security.*", "org.apache.hadoop.*",
+ "javax.security.*", "java.security.*", "javax.crypto.*",
"javax.net.ssl.*" })
+public class TableServiceTest extends NLocalFileMetadataTestCase {
+
+ @Mock
+ private final TableService tableService = Mockito.spy(TableService.class);
+
+ @Mock
+ private final AclEvaluate aclEvaluate = Mockito.spy(AclEvaluate.class);
+
+ @InjectMocks
+ private final TableExtService tableExtService = Mockito.spy(new
TableExtService());
+
+ @Before
+ public void setup() throws IOException {
+ overwriteSystemProp("HADOOP_USER_NAME", "root");
+ createTestMetadata();
+ SecurityContextHolder.getContext()
+ .setAuthentication(new TestingAuthenticationToken("ADMIN",
"ADMIN", Constant.ROLE_ADMIN));
+ ReflectionTestUtils.setField(aclEvaluate, "aclUtil",
Mockito.spy(AclUtil.class));
+ ReflectionTestUtils.setField(tableService, "aclEvaluate", aclEvaluate);
+ ReflectionTestUtils.setField(tableExtService, "aclEvaluate",
aclEvaluate);
+ ReflectionTestUtils.setField(tableExtService, "tableService",
tableService);
+ }
+
+ @After
+ public void tearDown() {
+ cleanupTestMetadata();
+ }
+
+ @Test
+ public void testLoadTablesSuccessfully() {
+ String[] tables = { "DEFAULT.TEST_KYLIN_FACT", "DEFAULT.TEST_ACCOUNT"
};
+ LoadTableResponse tableResponse = new LoadTableResponse();
+ List<Pair<TableDesc, TableExtDesc>> canLoadTables =
tableService.extractTableMeta(tables, "default",
+ tableResponse);
+ Assert.assertEquals(0, tableResponse.getFailed().size());
+ Assert.assertEquals(2, canLoadTables.size());
+ }
+
+ @Test
+ public void testLoadTablesError() throws Exception {
+ String[] tables = { "DEFAULT.TEST_KYLIN_FACT", "DEFAULT.TEST_ACCOUNT"
};
+ LoadTableResponse tableResponse = new LoadTableResponse();
+ CsvSource csvSource = PowerMockito.mock(CsvSource.class);
+ PowerMockito.whenNew(CsvSource.class);
+ ISourceMetadataExplorer mockExplorer =
Mockito.mock(ISourceMetadataExplorer.class);
+ PowerMockito.mockStatic(SourceFactory.class);
+ PowerMockito.doReturn(csvSource).when(SourceFactory.class,
"getSource", Mockito.any());
+
Mockito.when((csvSource).getSourceMetadataExplorer()).thenReturn(mockExplorer);
+ Mockito.when(mockExplorer.loadTableMetadata(Mockito.anyString(),
Mockito.anyString(), Mockito.anyString()))
+ .thenThrow(new AccessControlException("Mock can not fetch
table meta"));
+ List<Pair<TableDesc, TableExtDesc>> canLoadTables =
tableService.extractTableMeta(tables, "default",
+ tableResponse);
+ Assert.assertEquals(2, tableResponse.getFailed().size());
+ Assert.assertEquals(0, canLoadTables.size());
+
+ Assert.assertThrows(KylinException.class, () -> {
+ tableService.extractTableMeta(tables, "default");
+ });
+ }
+
+ @Test
+ public void testLoadTablesErrorAndThrowException() throws Exception {
+ String[] tables = { "DEFAULT.TEST_KYLIN_FACT", "DEFAULT.TEST_ACCOUNT"
};
+ CsvSource csvSource = PowerMockito.mock(CsvSource.class);
+ PowerMockito.whenNew(CsvSource.class);
+ ISourceMetadataExplorer mockExplorer =
Mockito.mock(ISourceMetadataExplorer.class);
+ PowerMockito.mockStatic(SourceFactory.class);
+ PowerMockito.doReturn(csvSource).when(SourceFactory.class,
"getSource", Mockito.any());
+
Mockito.when((csvSource).getSourceMetadataExplorer()).thenReturn(mockExplorer);
+ Mockito.when(mockExplorer.loadTableMetadata(Mockito.anyString(),
Mockito.anyString(), Mockito.anyString()))
+ .thenThrow(new AccessControlException("Mock can not fetch
table meta"));
+ Assert.assertThrows(KylinException.class, () -> {
+ tableService.extractTableMeta(tables, "default");
+ });
+ }
+}
diff --git
a/src/ops-server/src/main/java/org/apache/kylin/rest/controller/OpsController.java
b/src/ops-server/src/main/java/org/apache/kylin/rest/controller/OpsController.java
index 3d2fddf9aa..07a9aa6c3d 100644
---
a/src/ops-server/src/main/java/org/apache/kylin/rest/controller/OpsController.java
+++
b/src/ops-server/src/main/java/org/apache/kylin/rest/controller/OpsController.java
@@ -36,6 +36,7 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.kylin.common.KylinConfig;
import org.apache.kylin.common.exception.KylinException;
import org.apache.kylin.common.persistence.transaction.UnitOfWork;
+import org.apache.kylin.common.util.AddressUtil;
import org.apache.kylin.common.util.Pair;
import org.apache.kylin.guava30.shaded.common.annotations.VisibleForTesting;
import org.apache.kylin.guava30.shaded.common.collect.Maps;
@@ -134,6 +135,7 @@ public class OpsController extends NBasicController {
@RequestBody DiagPackageRequest diagPackageRequest, @RequestHeader
HttpHeaders headers,
final HttpServletRequest request) throws Exception {
host = decodeHost(host);
+ AddressUtil.validateHost(host);
if (StringUtils.isNotBlank(diagPackageRequest.getJobId())) {
diagPackageRequest.setStart("");
diagPackageRequest.setEnd("");
@@ -162,6 +164,7 @@ public class OpsController extends NBasicController {
@RequestBody QueryDiagPackageRequest queryDiagPackageRequest,
@RequestHeader HttpHeaders headers,
final HttpServletRequest request) throws Exception {
host = decodeHost(host);
+ AddressUtil.validateHost(host);
if (StringUtils.isEmpty(host) ||
KylinConfig.getInstanceFromEnv().getMicroServiceMode() != null) {
String uuid =
systemService.dumpLocalQueryDiagPackage(queryDiagPackageRequest.getQueryId(),
queryDiagPackageRequest.getProject(), headers);
@@ -188,6 +191,7 @@ public class OpsController extends NBasicController {
@RequestParam(value = "project", required = false) String project,
final HttpServletRequest request)
throws Exception {
host = decodeHost(host);
+ AddressUtil.validateHost(host);
if (StringUtils.isEmpty(host) ||
KylinConfig.getInstanceFromEnv().getMicroServiceMode() != null) {
return systemService.getExtractorStatus(id, project);
} else {
@@ -206,6 +210,7 @@ public class OpsController extends NBasicController {
@RequestParam(value = "id") String id, @RequestParam(value =
"project", required = false) String project,
final HttpServletRequest request, final HttpServletResponse
response) throws IOException {
host = decodeHost(host);
+ AddressUtil.validateHost(host);
if (StringUtils.isEmpty(host) ||
KylinConfig.getInstanceFromEnv().getMicroServiceMode() != null) {
setDownloadResponse(systemService.getDiagPackagePath(id, project),
MediaType.APPLICATION_OCTET_STREAM_VALUE,
response);
@@ -224,6 +229,7 @@ public class OpsController extends NBasicController {
public EnvelopeResponse<String> remoteStopPackage(@RequestParam(value =
"host", required = false) String host,
@RequestParam(value = "id") String id, final HttpServletRequest
request) throws Exception {
host = decodeHost(host);
+ AddressUtil.validateHost(host);
if (StringUtils.isEmpty(host) ||
KylinConfig.getInstanceFromEnv().getMicroServiceMode() != null) {
systemService.stopDiagTask(id);
return new EnvelopeResponse<>(CODE_SUCCESS, "", "");
diff --git
a/src/query-service/src/main/java/org/apache/kylin/rest/service/SparderUIService.java
b/src/query-service/src/main/java/org/apache/kylin/rest/service/SparderUIService.java
index 7c3c675447..bb7f22d696 100644
---
a/src/query-service/src/main/java/org/apache/kylin/rest/service/SparderUIService.java
+++
b/src/query-service/src/main/java/org/apache/kylin/rest/service/SparderUIService.java
@@ -37,6 +37,7 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.kylin.common.KylinConfig;
import org.apache.kylin.common.exception.ErrorCode;
import org.apache.kylin.common.msg.MsgPicker;
+import org.apache.kylin.common.util.AddressUtil;
import org.apache.kylin.rest.util.AclEvaluate;
import org.apache.kylin.rest.util.SparderUIUtil;
import org.springframework.beans.factory.annotation.Autowired;
@@ -83,6 +84,7 @@ public class SparderUIService extends BasicService {
val server = getServer(servletRequest);
if (StringUtils.isNotBlank(server) &&
!TRUE.equalsIgnoreCase(servletRequest.getHeader(ROUTED))
&& routeService.needRoute()) {
+ AddressUtil.validateHost(server);
log.info("proxy sparder UI to server : [{}]", server);
val queryString = servletRequest.getQueryString();
proxyToServer(server, queryString, restTemplate, servletRequest,
servletResponse);
@@ -111,6 +113,7 @@ public class SparderUIService extends BasicService {
}
if (StringUtils.isNotBlank(realServer) &&
!TRUE.equalsIgnoreCase(servletRequest.getHeader(ROUTED))
&& routeService.needRoute()) {
+ AddressUtil.validateHost(realServer);
log.info("proxy sparder UI to server : [{}] queryId : [{}] Id :
[{}]", realServer, queryId, id);
val queryString = "id=" + id;
proxyToServer(realServer, queryString, restTemplate,
servletRequest, servletResponse);
diff --git
a/src/query-service/src/test/java/org/apache/kylin/rest/service/SparderUIServiceTest.java
b/src/query-service/src/test/java/org/apache/kylin/rest/service/SparderUIServiceTest.java
index 39e67bf6cb..ea7409d02d 100644
---
a/src/query-service/src/test/java/org/apache/kylin/rest/service/SparderUIServiceTest.java
+++
b/src/query-service/src/test/java/org/apache/kylin/rest/service/SparderUIServiceTest.java
@@ -20,7 +20,9 @@ package org.apache.kylin.rest.service;
import static org.apache.commons.net.util.Base64.encodeBase64;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
import java.nio.charset.Charset;
import java.util.stream.Collectors;
@@ -101,6 +103,38 @@ public class SparderUIServiceTest {
}
+ @Test
+ public void proxyFail() {
+ {
+ try {
+ Mockito.when(routeService.needRoute()).thenReturn(true);
+ val req = new MockHttpServletRequest();
+ req.addHeader("routed", false);
+ setCookie(true, req, "127.0.0.1:7979");
+ val res = new MockHttpServletResponse();
+ sparderUIService.proxy("123", "321", "127.0.0.1:7979>", req,
res);
+ fail();
+ } catch (Exception e) {
+ assertInstanceOf(IllegalArgumentException.class, e);
+ assertTrue(e.getMessage().contains("Url contains disallowed
chars, host: "));
+ }
+ }
+ {
+ try {
+ Mockito.when(routeService.needRoute()).thenReturn(true);
+ val req = new MockHttpServletRequest();
+ setCookie(true, req, "127.0.0.1:7979>");
+ req.addHeader("routed", false);
+ val res = new MockHttpServletResponse();
+ sparderUIService.proxy(req, res);
+ fail();
+ } catch (Exception e) {
+ assertInstanceOf(IllegalArgumentException.class, e);
+ assertTrue(e.getMessage().contains("Url contains disallowed
chars, host: "));
+ }
+ }
+ }
+
@Test
public void proxy() throws Exception {
proxyInner();
@@ -132,14 +166,7 @@ public class SparderUIServiceTest {
((Logger) LogManager.getRootLogger()).addAppender(appender);
Mockito.when(routeService.needRoute()).thenReturn(needRoute);
val req = new MockHttpServletRequest();
- if (needCookie) {
- val server = "127.0.0.1:7070";
- val serverBytes = server.getBytes(Charset.defaultCharset());
- val serverCookie = new String(encodeBase64(serverBytes),
Charset.defaultCharset());
- Cookie cookie = new Cookie("server", serverCookie);
- cookie.setPath(SparderUIUtil.KYLIN_UI_BASE);
- req.setCookies(cookie);
- }
+ setCookie(needCookie, req, "127.0.0.1:7070");
req.addHeader("routed", isRouted);
val res = new MockHttpServletResponse();
sparderUIService.proxy(req, res);
@@ -169,13 +196,7 @@ public class SparderUIServiceTest {
((Logger) LogManager.getRootLogger()).addAppender(appender);
Mockito.when(routeService.needRoute()).thenReturn(needRoute);
val req = new MockHttpServletRequest();
- if (needCookie) {
- val serverBytes =
"127.0.0.1:7979".getBytes(Charset.defaultCharset());
- val serverCookie = new String(encodeBase64(serverBytes),
Charset.defaultCharset());
- Cookie cookie = new Cookie("server", serverCookie);
- cookie.setPath(SparderUIUtil.KYLIN_UI_BASE);
- req.setCookies(cookie);
- }
+ setCookie(needCookie, req, "127.0.0.1:7979");
req.addHeader("routed", isRouted);
val res = new MockHttpServletResponse();
sparderUIService.proxy("123", "321", server, req, res);
@@ -197,4 +218,14 @@ public class SparderUIServiceTest {
((Logger) LogManager.getRootLogger()).removeAppender(appender);
}
}
+
+ private void setCookie(boolean needCookie, MockHttpServletRequest req,
String server) {
+ if (needCookie) {
+ val serverBytes = server.getBytes(Charset.defaultCharset());
+ val serverCookie = new String(encodeBase64(serverBytes),
Charset.defaultCharset());
+ Cookie cookie = new Cookie("server", serverCookie);
+ cookie.setPath(SparderUIUtil.KYLIN_UI_BASE);
+ req.setCookies(cookie);
+ }
+ }
}