From a4af11fba4534f8018b06f54f6dcee410caddb46 Mon Sep 17 00:00:00 2001 From: Nahuel Date: Thu, 27 Apr 2023 17:45:25 +0200 Subject: [PATCH] Fix: Improve error messages (#11244) --- .../service/secrets/SecretsManager.java | 4 ++ .../service/secrets/SecretsUtil.java | 64 +++++++++++++++++++ .../secrets/masker/PasswordEntityMasker.java | 9 +++ .../secrets/ExternalSecretsManagerTest.java | 30 +++++++++ .../masker/PasswordEntityMaskerTest.java | 38 +++++++++++ .../en-US/Database/workflows/metadata.md | 2 +- .../ui/src/locale/languages/en-us.json | 2 +- .../ui/src/locale/languages/es-es.json | 2 +- .../ui/src/locale/languages/fr-fr.json | 2 +- .../ui/src/locale/languages/ja-jp.json | 2 +- .../ui/src/locale/languages/pt-br.json | 2 +- .../ui/src/locale/languages/zh-cn.json | 2 +- 12 files changed, 152 insertions(+), 7 deletions(-) create mode 100644 openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsUtil.java diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManager.java b/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManager.java index b89c5fd1fb9..969a01b41dd 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManager.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsManager.java @@ -62,6 +62,10 @@ public abstract class SecretsManager { return encryptOrDecryptPasswordFields( newConnectionConfig, buildSecretId(true, serviceType.value(), connectionName), encrypt, true); } catch (Exception e) { + String message = SecretsUtil.buildExceptionMessageConnection(e.getMessage(), connectionType, encrypt); + if (message != null) { + throw new InvalidServiceConnectionException(message); + } throw InvalidServiceConnectionException.byMessage( connectionType, String.format("Failed to encrypt connection instance of %s", connectionType)); } diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsUtil.java b/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsUtil.java new file mode 100644 index 00000000000..17920ca0258 --- /dev/null +++ b/openmetadata-service/src/main/java/org/openmetadata/service/secrets/SecretsUtil.java @@ -0,0 +1,64 @@ +/* + * Copyright 2021 Collate + * Licensed 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.openmetadata.service.secrets; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class SecretsUtil { + + /** + * Returns an error message when it is related to an Unrecognized field + * + * @param message the message to be formatted if the Unrecognized field is between quotes + * @param defaultMessage default message to be formatted if the Unrecognized field is not between quotes + * @param exceptionMessage the exception message + * @param type the type of error + * @return null if the message does not contain 'Unrecognized field' in the exception message + */ + public static String buildExceptionMessageUnrecognizedField( + String message, String defaultMessage, String exceptionMessage, String type) { + if (exceptionMessage != null && exceptionMessage.contains("Unrecognized field")) { + Pattern pattern = Pattern.compile("Unrecognized field \"(.*?)\""); + Matcher matcher = pattern.matcher(exceptionMessage); + if (matcher.find()) { + String fieldValue = matcher.group(1); + return String.format(message, type, fieldValue); + } + return String.format(defaultMessage, type); + } + return null; + } + + public static String buildExceptionMessageConnection( + String exceptionMessage, String type, String firstAction, String secondAction, boolean isFirstAction) { + return buildExceptionMessageUnrecognizedField( + "Failed to " + + (isFirstAction ? firstAction : secondAction) + + " '%s' connection stored in DB due to an unrecognized field: '%s'", + "Failed to " + + (isFirstAction ? firstAction : secondAction) + + " '%s' connection stored in DB due to malformed connection object.", + exceptionMessage, + type); + } + + public static String buildExceptionMessageConnection(String exceptionMessage, String type, boolean encrypt) { + return buildExceptionMessageConnection(exceptionMessage, type, "encrypt", "decrypt", encrypt); + } + + public static String buildExceptionMessageConnectionMask(String exceptionMessage, String type, boolean mask) { + return buildExceptionMessageConnection(exceptionMessage, type, "mask", "unmask", mask); + } +} diff --git a/openmetadata-service/src/main/java/org/openmetadata/service/secrets/masker/PasswordEntityMasker.java b/openmetadata-service/src/main/java/org/openmetadata/service/secrets/masker/PasswordEntityMasker.java index 8343b1c905f..06b9cacdf5e 100644 --- a/openmetadata-service/src/main/java/org/openmetadata/service/secrets/masker/PasswordEntityMasker.java +++ b/openmetadata-service/src/main/java/org/openmetadata/service/secrets/masker/PasswordEntityMasker.java @@ -24,6 +24,7 @@ import org.openmetadata.schema.entity.services.ingestionPipelines.IngestionPipel import org.openmetadata.schema.entity.teams.AuthenticationMechanism; import org.openmetadata.service.exception.EntityMaskException; import org.openmetadata.service.fernet.Fernet; +import org.openmetadata.service.secrets.SecretsUtil; import org.openmetadata.service.secrets.converter.ClassConverterFactory; import org.openmetadata.service.util.AuthenticationMechanismBuilder; import org.openmetadata.service.util.IngestionPipelineBuilder; @@ -54,6 +55,10 @@ public class PasswordEntityMasker extends EntityMasker { maskPasswordFields(convertedConnectionConfig); return convertedConnectionConfig; } catch (Exception e) { + String message = SecretsUtil.buildExceptionMessageConnectionMask(e.getMessage(), connectionType, true); + if (message != null) { + throw new EntityMaskException(message); + } throw new EntityMaskException(String.format("Failed to mask connection instance of %s", connectionType)); } } @@ -109,6 +114,10 @@ public class PasswordEntityMasker extends EntityMasker { unmaskPasswordFields(toUnmaskConfig, NEW_KEY, passwordsMap); return toUnmaskConfig; } catch (Exception e) { + String message = SecretsUtil.buildExceptionMessageConnectionMask(e.getMessage(), connectionType, false); + if (message != null) { + throw new EntityMaskException(message); + } throw new EntityMaskException(String.format("Failed to unmask connection instance of %s", connectionType)); } } diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/secrets/ExternalSecretsManagerTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/secrets/ExternalSecretsManagerTest.java index a9f3bf384e1..2e34a9c93de 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/secrets/ExternalSecretsManagerTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/secrets/ExternalSecretsManagerTest.java @@ -15,6 +15,7 @@ package org.openmetadata.service.secrets; import static org.junit.jupiter.api.Assertions.assertEquals; import java.util.Map; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; @@ -41,6 +42,7 @@ import org.openmetadata.schema.security.secrets.SecretsManagerProvider; import org.openmetadata.schema.services.connections.database.MysqlConnection; import org.openmetadata.schema.services.connections.metadata.OpenMetadataConnection; import org.openmetadata.service.Entity; +import org.openmetadata.service.exception.InvalidServiceConnectionException; import org.openmetadata.service.fernet.Fernet; import org.openmetadata.service.util.JsonUtils; @@ -105,6 +107,34 @@ public abstract class ExternalSecretsManagerTest { testEncryptWorkflowObject(ENCRYPT); } + @Test + void testExceptionConnection() { + CreateDatabaseService.DatabaseServiceType databaseServiceType = CreateDatabaseService.DatabaseServiceType.Mysql; + String connectionName = "test"; + Map mysqlConnection = Map.of("password", "openmetadata-test", "username1", "openmetadata-test"); + + InvalidServiceConnectionException thrown = + Assertions.assertThrows( + InvalidServiceConnectionException.class, + () -> + secretsManager.encryptOrDecryptServiceConnectionConfig( + mysqlConnection, databaseServiceType.value(), connectionName, ServiceType.DATABASE, true)); + + Assertions.assertEquals( + "Failed to encrypt 'Mysql' connection stored in DB due to an unrecognized field: 'username1'", + thrown.getMessage()); + thrown = + Assertions.assertThrows( + InvalidServiceConnectionException.class, + () -> + secretsManager.encryptOrDecryptServiceConnectionConfig( + mysqlConnection, databaseServiceType.value(), connectionName, ServiceType.DATABASE, false)); + + Assertions.assertEquals( + "Failed to decrypt 'Mysql' connection stored in DB due to an unrecognized field: 'username1'", + thrown.getMessage()); + } + @Test void testReturnsExpectedSecretManagerProvider() { assertEquals(expectedSecretManagerProvider(), secretsManager.getSecretsManagerProvider()); diff --git a/openmetadata-service/src/test/java/org/openmetadata/service/secrets/masker/PasswordEntityMaskerTest.java b/openmetadata-service/src/test/java/org/openmetadata/service/secrets/masker/PasswordEntityMaskerTest.java index 5c4f9df71b6..4ce2c5ce5d3 100644 --- a/openmetadata-service/src/test/java/org/openmetadata/service/secrets/masker/PasswordEntityMaskerTest.java +++ b/openmetadata-service/src/test/java/org/openmetadata/service/secrets/masker/PasswordEntityMaskerTest.java @@ -1,5 +1,12 @@ package org.openmetadata.service.secrets.masker; +import java.util.Map; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.openmetadata.schema.entity.services.ServiceType; +import org.openmetadata.schema.services.connections.database.MysqlConnection; +import org.openmetadata.service.exception.EntityMaskException; + public class PasswordEntityMaskerTest extends TestEntityMasker { public PasswordEntityMaskerTest() { CONFIG.setMaskPasswordsAPI(true); @@ -9,4 +16,35 @@ public class PasswordEntityMaskerTest extends TestEntityMasker { protected String getMaskedPassword() { return PasswordEntityMasker.PASSWORD_MASK; } + + @Test + void testExceptionConnection() { + Map mysqlConnectionObject = + Map.of("password", "openmetadata-test", "username1", "openmetadata-test"); + + EntityMaskException thrown = + Assertions.assertThrows( + EntityMaskException.class, + () -> { + EntityMaskerFactory.createEntityMasker(CONFIG) + .maskServiceConnectionConfig(mysqlConnectionObject, "Mysql", ServiceType.DATABASE); + }); + + Assertions.assertEquals( + "Failed to mask 'Mysql' connection stored in DB due to an unrecognized field: 'username1'", + thrown.getMessage()); + + thrown = + Assertions.assertThrows( + EntityMaskException.class, + () -> { + EntityMaskerFactory.createEntityMasker(CONFIG) + .unmaskServiceConnectionConfig( + mysqlConnectionObject, new MysqlConnection(), "Mysql", ServiceType.DATABASE); + }); + + Assertions.assertEquals( + "Failed to unmask 'Mysql' connection stored in DB due to an unrecognized field: 'username1'", + thrown.getMessage()); + } } diff --git a/openmetadata-ui/src/main/resources/ui/public/locales/en-US/Database/workflows/metadata.md b/openmetadata-ui/src/main/resources/ui/public/locales/en-US/Database/workflows/metadata.md index a32f592e9fa..a880208a3a5 100644 --- a/openmetadata-ui/src/main/resources/ui/public/locales/en-US/Database/workflows/metadata.md +++ b/openmetadata-ui/src/main/resources/ui/public/locales/en-US/Database/workflows/metadata.md @@ -1,6 +1,6 @@ # Metadata -DatabaseService Metadata Pipeline Configuration. +Database Service Metadata Pipeline Configuration. ## Configuration diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json index 4a302fe1935..dee96be0c16 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/en-us.json @@ -1382,4 +1382,4 @@ "update-entity-success": "{{entity}} updated successfully.", "you-have-not-action-anything-yet": "You have not {{action}} anything yet." } -} +} \ No newline at end of file diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/es-es.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/es-es.json index 18af2563b06..f0495c48a2d 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/es-es.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/es-es.json @@ -1382,4 +1382,4 @@ "update-entity-success": "{{entity}} actualizado exitosamente.", "you-have-not-action-anything-yet": "Todavía no has {{action}} nada." } -} +} \ No newline at end of file diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/fr-fr.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/fr-fr.json index 3cf7e317334..d3d2fba9a33 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/fr-fr.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/fr-fr.json @@ -1382,4 +1382,4 @@ "update-entity-success": "{{entity}} mis à jour avec succès.", "you-have-not-action-anything-yet": "Vous n'avez encore rien {{action}}." } -} +} \ No newline at end of file diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/ja-jp.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/ja-jp.json index 761a2243a3d..a7a5d0eb0d2 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/ja-jp.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/ja-jp.json @@ -1382,4 +1382,4 @@ "update-entity-success": "{{entity}}は正常に更新されました。", "you-have-not-action-anything-yet": "あなたが{{action}}のデータはありません。" } -} +} \ No newline at end of file diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/pt-br.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/pt-br.json index d47251604bc..f1d697ca4a0 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/pt-br.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/pt-br.json @@ -1382,4 +1382,4 @@ "update-entity-success": "{{entity}} atualizado com sucesso.", "you-have-not-action-anything-yet": "Você ainda não {{action}} nada." } -} +} \ No newline at end of file diff --git a/openmetadata-ui/src/main/resources/ui/src/locale/languages/zh-cn.json b/openmetadata-ui/src/main/resources/ui/src/locale/languages/zh-cn.json index 38ea7e310a4..9b70b1c041f 100644 --- a/openmetadata-ui/src/main/resources/ui/src/locale/languages/zh-cn.json +++ b/openmetadata-ui/src/main/resources/ui/src/locale/languages/zh-cn.json @@ -1382,4 +1382,4 @@ "update-entity-success": "{{entity}}已成功更新", "you-have-not-action-anything-yet": "您还没有{{action}}任何内容" } -} +} \ No newline at end of file