Fix #4106: Add validations to databaseServiceConnection while creating or updating service (#4107)

This commit is contained in:
Sriharsha Chintalapani 2022-04-13 10:07:24 -07:00 committed by GitHub
parent 31fbf1a500
commit 781b59e73b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 104 additions and 12 deletions

View File

@ -16,7 +16,6 @@ package org.openmetadata.catalog.exception;
import javax.ws.rs.core.Response;
public class AirflowException extends WebServiceException {
private static final String BY_NAME_MESSAGE = "Airflow Exception [%s] due to [%s].";
public AirflowException(String message) {

View File

@ -0,0 +1,27 @@
package org.openmetadata.catalog.exception;
import javax.ws.rs.core.Response;
public class InvalidServiceConnectionException extends WebServiceException {
private static final String BY_NAME_MESSAGE = "InvalidServiceConnectionException for service [%s] due to [%s].";
public InvalidServiceConnectionException(String message) {
super(Response.Status.BAD_REQUEST, message);
}
private InvalidServiceConnectionException(Response.Status status, String message) {
super(status, message);
}
public static InvalidServiceConnectionException byMessage(String name, String errorMessage, Response.Status status) {
return new InvalidServiceConnectionException(status, buildMessageByName(name, errorMessage));
}
public static InvalidServiceConnectionException byMessage(String name, String errorMessage) {
return new InvalidServiceConnectionException(Response.Status.BAD_REQUEST, buildMessageByName(name, errorMessage));
}
private static String buildMessageByName(String name, String errorMessage) {
return String.format(BY_NAME_MESSAGE, name, errorMessage);
}
}

View File

@ -22,8 +22,10 @@ import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
import org.openmetadata.catalog.Entity;
import org.openmetadata.catalog.api.services.CreateDatabaseService;
import org.openmetadata.catalog.api.services.DatabaseConnection;
import org.openmetadata.catalog.entity.services.DatabaseService;
import org.openmetadata.catalog.exception.InvalidServiceConnectionException;
import org.openmetadata.catalog.resources.services.database.DatabaseServiceResource;
import org.openmetadata.catalog.type.ChangeDescription;
import org.openmetadata.catalog.type.EntityReference;
@ -31,6 +33,7 @@ import org.openmetadata.catalog.type.Include;
import org.openmetadata.catalog.type.Relationship;
import org.openmetadata.catalog.util.EntityInterface;
import org.openmetadata.catalog.util.EntityUtil.Fields;
import org.openmetadata.catalog.util.JsonUtils;
public class DatabaseServiceRepository extends EntityRepository<DatabaseService> {
private static final String UPDATE_FIELDS = "owner";
@ -76,6 +79,8 @@ public class DatabaseServiceRepository extends EntityRepository<DatabaseService>
public void prepare(DatabaseService databaseService) throws IOException {
// Check if owner is valid and set the relationship
databaseService.setOwner(Entity.getEntityReference(databaseService.getOwner()));
// validate database service connection
validateDatabaseConnection(databaseService.getConnection(), databaseService.getServiceType());
}
@Override
@ -103,6 +108,21 @@ public class DatabaseServiceRepository extends EntityRepository<DatabaseService>
return new DatabaseServiceUpdater(original, updated, operation);
}
private void validateDatabaseConnection(
DatabaseConnection databaseConnection, CreateDatabaseService.DatabaseServiceType databaseServiceType) {
try {
Object connectionConfig = databaseConnection.getConfig();
String clazzName =
"org.openmetadata.catalog.services.connections.database." + databaseServiceType.value() + "Connection";
Class<?> clazz = Class.forName(clazzName);
JsonUtils.convertValue(connectionConfig, clazz);
} catch (Exception e) {
throw InvalidServiceConnectionException.byMessage(
databaseServiceType.value(),
String.format("Failed to construct connection instance of %s", databaseServiceType.value()));
}
}
public static class DatabaseServiceEntityInterface extends EntityInterface<DatabaseService> {
public DatabaseServiceEntityInterface(DatabaseService entity) {
super(Entity.DATABASE_SERVICE, entity);

View File

@ -47,6 +47,7 @@ import javax.ws.rs.core.SecurityContext;
import javax.ws.rs.core.UriInfo;
import org.openmetadata.catalog.Entity;
import org.openmetadata.catalog.api.services.CreateDatabaseService;
import org.openmetadata.catalog.api.services.DatabaseConnection;
import org.openmetadata.catalog.entity.services.DatabaseService;
import org.openmetadata.catalog.fernet.Fernet;
import org.openmetadata.catalog.jdbi3.CollectionDAO;
@ -355,4 +356,18 @@ public class DatabaseServiceResource extends EntityResource<DatabaseService, Dat
.withUpdatedBy(securityContext.getUserPrincipal().getName())
.withUpdatedAt(System.currentTimeMillis());
}
private void validateDatabaseConnection(
DatabaseConnection databaseConnection, CreateDatabaseService.DatabaseServiceType databaseServiceType) {
try {
Object connectionConfig = databaseConnection.getConfig();
String clazzName =
"org.openmetadata.catalog.services.connections.database." + databaseServiceType.value() + "Connection";
Class<?> clazz = Class.forName(clazzName);
JsonUtils.convertValue(connectionConfig, clazz);
} catch (Exception e) {
throw new RuntimeException(
String.format("Failed to construct connection instance of %s", databaseServiceType.value()));
}
}
}

View File

@ -29,6 +29,7 @@ import io.swagger.v3.oas.annotations.parameters.RequestBody;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import java.io.IOException;
import java.util.List;
import java.util.Locale;
import java.util.UUID;
import javax.json.JsonPatch;
import javax.validation.Valid;
@ -437,7 +438,7 @@ public class IngestionPipelineResource extends EntityResource<IngestionPipeline,
new Source()
.withServiceConnection(databaseService.getConnection())
.withServiceName(databaseService.getName())
.withType(databaseService.getServiceType().value());
.withType(databaseService.getServiceType().value().toLowerCase(Locale.ROOT));
break;
case Entity.DASHBOARD_SERVICE:
DashboardService dashboardService = Entity.getEntity(create.getService(), serviceFields, Include.ALL);
@ -445,7 +446,7 @@ public class IngestionPipelineResource extends EntityResource<IngestionPipeline,
new Source()
.withServiceName(dashboardService.getName())
.withServiceConnection(dashboardService.getConnection())
.withType(dashboardService.getServiceType().value());
.withType(dashboardService.getServiceType().value().toLowerCase(Locale.ROOT));
break;
case Entity.MESSAGING_SERVICE:
MessagingService messagingService = Entity.getEntity(create.getService(), serviceFields, Include.ALL);
@ -453,7 +454,7 @@ public class IngestionPipelineResource extends EntityResource<IngestionPipeline,
new Source()
.withServiceName(messagingService.getName())
.withServiceConnection(messagingService.getConnection())
.withType(messagingService.getServiceType().value());
.withType(messagingService.getServiceType().value().toLowerCase(Locale.ROOT));
break;
default:
throw new IllegalArgumentException(

View File

@ -9,8 +9,8 @@
"mySQLType": {
"description": "Service type.",
"type": "string",
"enum": ["MySQL"],
"default": "MySQL"
"enum": ["Mysql"],
"default": "Mysql"
},
"mySQLScheme": {
"description": "SQLAlchemy driver scheme options.",
@ -23,7 +23,7 @@
"type": {
"description": "Service Type",
"$ref": "#/definitions/mySQLType",
"default": "MySQL"
"default": "Mysql"
},
"scheme": {
"description": "SQLAlchemy driver scheme options.",

View File

@ -10,7 +10,7 @@
"type": "string",
"enum": [
"BigQuery",
"MySQL",
"Mysql",
"Redshift",
"Snowflake",
"Postgres",
@ -40,7 +40,7 @@
"name": "BigQuery"
},
{
"name": "MySQL"
"name": "Mysql"
},
{
"name": "Redshift"

View File

@ -342,7 +342,7 @@ public abstract class EntityResourceTest<T, K> extends CatalogApplicationTest {
createDatabaseService
.withName("mysqlDB")
.withServiceType(DatabaseServiceType.MySQL)
.withServiceType(DatabaseServiceType.Mysql)
.withConnection(TestUtils.MYSQL_DATABASE_CONNECTION);
databaseService = databaseServiceResourceTest.createEntity(createDatabaseService, ADMIN_AUTH_HEADERS);
MYSQL_REFERENCE = new DatabaseServiceEntityInterface(databaseService).getEntityReference();

View File

@ -78,7 +78,7 @@ public class DatabaseServiceResourceTest extends EntityResourceTest<DatabaseServ
}
@Test
void post_invalidDatabaseServiceNoJdbc_4xx(TestInfo test) {
void post_invalidDatabaseServiceNoConnection_4xx(TestInfo test) {
// No jdbc connection set
CreateDatabaseService create = createRequest(test).withConnection(null);
assertResponseContains(() -> createEntity(create, ADMIN_AUTH_HEADERS), BAD_REQUEST, "connection must not be null");
@ -115,6 +115,31 @@ public class DatabaseServiceResourceTest extends EntityResourceTest<DatabaseServ
assertEquals("description1", service.getDescription());
}
@Test
void post_put_invalidConnection_as_admin_4xx(TestInfo test) throws IOException {
RedshiftConnection redshiftConnection =
new RedshiftConnection().withHostPort("localhost:3300").withUsername("test");
DatabaseConnection dbConn = new DatabaseConnection().withConfig(redshiftConnection);
assertResponseContains(
() -> createEntity(createRequest(test).withDescription(null).withConnection(dbConn), ADMIN_AUTH_HEADERS),
BAD_REQUEST,
"InvalidServiceConnectionException for service [Snowflake] due to [Failed to construct connection instance of Snowflake]");
DatabaseService service = createAndCheckEntity(createRequest(test).withDescription(null), ADMIN_AUTH_HEADERS);
// Update database description and ingestion service that are null
CreateDatabaseService update = createRequest(test).withDescription("description1");
ChangeDescription change = getChangeDescription(service.getVersion());
change.getFieldsAdded().add(new FieldChange().withName("description").withNewValue("description1"));
updateAndCheckEntity(update, OK, ADMIN_AUTH_HEADERS, UpdateType.MINOR_UPDATE, change);
MysqlConnection mysqlConnection = new MysqlConnection().withHostPort("localhost:3300").withUsername("test");
DatabaseConnection databaseConnection = new DatabaseConnection().withConfig(mysqlConnection);
update.withConnection(databaseConnection);
assertResponseContains(
() -> updateEntity(update, OK, ADMIN_AUTH_HEADERS),
BAD_REQUEST,
"InvalidServiceConnectionException for service [Snowflake] due to [Failed to construct connection instance of Snowflake]");
}
@Test
void put_addIngestion_as_admin_2xx(TestInfo test) throws IOException {
DatabaseService service = createAndCheckEntity(createRequest(test).withDescription(null), ADMIN_AUTH_HEADERS);
@ -172,6 +197,11 @@ public class DatabaseServiceResourceTest extends EntityResourceTest<DatabaseServ
assertEquals(ingestionPipeline.getId(), expectedPipeline.getId());
assertEquals(ingestionPipeline.getName(), expectedPipeline.getName());
assertEquals(ingestionPipeline.getFullyQualifiedName(), expectedPipeline.getFullyQualifiedName());
DatabaseConnection expectedDatabaseConnection =
JsonUtils.convertValue(ingestionPipeline.getSource().getServiceConnection(), DatabaseConnection.class);
SnowflakeConnection expectedSnowflake =
JsonUtils.convertValue(expectedDatabaseConnection.getConfig(), SnowflakeConnection.class);
assertEquals(expectedSnowflake, snowflakeConnection);
}
@Override
@ -244,7 +274,7 @@ public class DatabaseServiceResourceTest extends EntityResourceTest<DatabaseServ
DatabaseServiceType databaseServiceType) {
// Validate Database Connection if available. We nullify when not admin or bot
if (expectedDatabaseConnection != null) {
if (databaseServiceType == DatabaseServiceType.MySQL) {
if (databaseServiceType == DatabaseServiceType.Mysql) {
MysqlConnection expectedMysqlConnection = (MysqlConnection) expectedDatabaseConnection.getConfig();
MysqlConnection actualMysqlConnection;
if (actualDatabaseConnection.getConfig() instanceof MysqlConnection) {