From b829a2cbf32336dd1c3b80b3e335f8cb67ec5db7 Mon Sep 17 00:00:00 2001 From: Nahuel Date: Wed, 7 Sep 2022 09:18:59 +0200 Subject: [PATCH] Fix: Improvements on secret manager implementation (#7282) * Change local secret manager by noop * Update openmetadata-secure-test.yaml --- ...tsManager.java => NoopSecretsManager.java} | 14 +- .../secrets/SecretsManagerConfiguration.java | 2 +- .../secrets/SecretsManagerFactory.java | 4 +- .../metadata/secretsManagerProvider.json | 2 +- .../secrets/AWSSSMSecretsManagerTest.java | 2 +- .../secrets/AWSSecretsManagerTest.java | 2 +- .../secrets/ExternalSecretsManagerTest.java | 2 +- ...rTest.java => NoopSecretsManagerTest.java} | 8 +- .../secrets/SecretsManagerFactoryTest.java | 8 +- .../resources/openmetadata-secure-test.yaml | 2 +- conf/openmetadata.yaml | 2 +- .../secrets/aws_based_secrets_manager.py | 109 +------------ .../utils/secrets/external_secrets_manager.py | 147 ++++++++++++++++++ ...ets_manager.py => noop_secrets_manager.py} | 4 +- .../utils/secrets/secrets_manager_factory.py | 6 +- .../ometa/test_ometa_secrets_manager.py | 6 +- ...anager.py => test_noop_secrets_manager.py} | 32 ++-- .../secrets/test_secrets_manager_factory.py | 4 +- .../metadata/secretsManagerProvider.json | 2 +- 19 files changed, 201 insertions(+), 157 deletions(-) rename catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/{LocalSecretsManager.java => NoopSecretsManager.java} (91%) rename catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/{LocalSecretsManagerTest.java => NoopSecretsManagerTest.java} (97%) create mode 100644 ingestion/src/metadata/utils/secrets/external_secrets_manager.py rename ingestion/src/metadata/utils/secrets/{local_secrets_manager.py => noop_secrets_manager.py} (97%) rename ingestion/tests/unit/metadata/utils/secrets/{test_local_secrets_manager.py => test_noop_secrets_manager.py} (77%) diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/LocalSecretsManager.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/NoopSecretsManager.java similarity index 91% rename from catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/LocalSecretsManager.java rename to catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/NoopSecretsManager.java index f843740e1b2..12a5f168779 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/LocalSecretsManager.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/NoopSecretsManager.java @@ -13,7 +13,7 @@ package org.openmetadata.catalog.secrets; -import static org.openmetadata.catalog.services.connections.metadata.SecretsManagerProvider.LOCAL; +import static org.openmetadata.catalog.services.connections.metadata.SecretsManagerProvider.NOOP; import com.google.common.annotations.VisibleForTesting; import java.lang.reflect.InvocationTargetException; @@ -27,14 +27,14 @@ import org.openmetadata.catalog.fernet.Fernet; import org.openmetadata.catalog.services.connections.metadata.OpenMetadataServerConnection; import org.openmetadata.catalog.util.JsonUtils; -public class LocalSecretsManager extends SecretsManager { +public class NoopSecretsManager extends SecretsManager { - private static LocalSecretsManager INSTANCE; + private static NoopSecretsManager INSTANCE; private Fernet fernet; - private LocalSecretsManager(String clusterPrefix) { - super(LOCAL, clusterPrefix); + private NoopSecretsManager(String clusterPrefix) { + super(NOOP, clusterPrefix); this.fernet = Fernet.getInstance(); } @@ -113,8 +113,8 @@ public class LocalSecretsManager extends SecretsManager { } } - public static LocalSecretsManager getInstance(String clusterPrefix) { - if (INSTANCE == null) INSTANCE = new LocalSecretsManager(clusterPrefix); + public static NoopSecretsManager getInstance(String clusterPrefix) { + if (INSTANCE == null) INSTANCE = new NoopSecretsManager(clusterPrefix); return INSTANCE; } diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/SecretsManagerConfiguration.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/SecretsManagerConfiguration.java index 09383a27bea..773d59c899c 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/SecretsManagerConfiguration.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/SecretsManagerConfiguration.java @@ -22,7 +22,7 @@ import org.openmetadata.catalog.services.connections.metadata.SecretsManagerProv @Setter public class SecretsManagerConfiguration { - public static final SecretsManagerProvider DEFAULT_SECRET_MANAGER = SecretsManagerProvider.LOCAL; + public static final SecretsManagerProvider DEFAULT_SECRET_MANAGER = SecretsManagerProvider.NOOP; private SecretsManagerProvider secretsManager; diff --git a/catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/SecretsManagerFactory.java b/catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/SecretsManagerFactory.java index 8ee634b9774..e945478a00b 100644 --- a/catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/SecretsManagerFactory.java +++ b/catalog-rest-service/src/main/java/org/openmetadata/catalog/secrets/SecretsManagerFactory.java @@ -23,8 +23,8 @@ public class SecretsManagerFactory { ? config.getSecretsManager() : SecretsManagerConfiguration.DEFAULT_SECRET_MANAGER; switch (secretManager) { - case LOCAL: - return LocalSecretsManager.getInstance(clusterName); + case NOOP: + return NoopSecretsManager.getInstance(clusterName); case AWS: return AWSSecretsManager.getInstance(config, clusterName); case AWS_SSM: diff --git a/catalog-rest-service/src/main/resources/json/schema/entity/services/connections/metadata/secretsManagerProvider.json b/catalog-rest-service/src/main/resources/json/schema/entity/services/connections/metadata/secretsManagerProvider.json index 12a68c6cf4c..0dc9a90fdbe 100644 --- a/catalog-rest-service/src/main/resources/json/schema/entity/services/connections/metadata/secretsManagerProvider.json +++ b/catalog-rest-service/src/main/resources/json/schema/entity/services/connections/metadata/secretsManagerProvider.json @@ -5,6 +5,6 @@ "description": "OpenMetadata Secrets Manager Provider. Make sure to configure the same secrets manager providers as the ones configured on the OpenMetadata server.", "type": "string", "javaType": "org.openmetadata.catalog.services.connections.metadata.SecretsManagerProvider", - "enum": ["local", "aws", "aws-ssm"], + "enum": ["noop", "aws", "aws-ssm"], "additionalProperties": false } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/AWSSSMSecretsManagerTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/AWSSSMSecretsManagerTest.java index acfcca2a101..52f87c9dcab 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/AWSSSMSecretsManagerTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/AWSSSMSecretsManagerTest.java @@ -85,7 +85,7 @@ public class AWSSSMSecretsManagerTest extends ExternalSecretsManagerTest { verifySecretIdGetCalls(expectedSecretId, 1); assertEquals(expectedSecretId, createSecretCaptor.getValue().name()); assertEquals( - "{\"type\":\"Mysql\",\"scheme\":\"mysql+pymysql\",\"supportsMetadataExtraction\":true,\"supportsProfiler\":true}", + "{\"type\":\"Mysql\",\"scheme\":\"mysql+pymysql\",\"supportsMetadataExtraction\":true,\"supportsProfiler\":true,\"supportsQueryComment\":true}", createSecretCaptor.getValue().value()); assertNull(serviceConnection); } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/AWSSecretsManagerTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/AWSSecretsManagerTest.java index bc794e8649d..78794f919c1 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/AWSSecretsManagerTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/AWSSecretsManagerTest.java @@ -67,7 +67,7 @@ public class AWSSecretsManagerTest extends ExternalSecretsManagerTest { verifySecretIdGetCalls(expectedSecretId, 1); assertEquals(expectedSecretId, createSecretCaptor.getValue().name()); assertEquals( - "{\"type\":\"Mysql\",\"scheme\":\"mysql+pymysql\",\"supportsMetadataExtraction\":true,\"supportsProfiler\":true}", + "{\"type\":\"Mysql\",\"scheme\":\"mysql+pymysql\",\"supportsMetadataExtraction\":true,\"supportsProfiler\":true,\"supportsQueryComment\":true}", createSecretCaptor.getValue().secretString()); assertNull(serviceConnection); } diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/ExternalSecretsManagerTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/ExternalSecretsManagerTest.java index e8c32bab58d..d1aa8ad5fce 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/ExternalSecretsManagerTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/ExternalSecretsManagerTest.java @@ -66,7 +66,7 @@ public abstract class ExternalSecretsManagerTest { static final String TEST_CONNECTION_SECRET_ID_PREFIX = "test-connection-temp"; static final boolean DECRYPT = false; static final String EXPECTED_CONNECTION_JSON = - "{\"type\":\"Mysql\",\"scheme\":\"mysql+pymysql\",\"password\":\"openmetadata-test\",\"supportsMetadataExtraction\":true,\"supportsProfiler\":true}"; + "{\"type\":\"Mysql\",\"scheme\":\"mysql+pymysql\",\"password\":\"openmetadata-test\",\"supportsMetadataExtraction\":true,\"supportsProfiler\":true,\"supportsQueryComment\":true}"; static final String EXPECTED_SECRET_ID = "/openmetadata/service/database/mysql/test"; AWSBasedSecretsManager secretsManager; diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/LocalSecretsManagerTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/NoopSecretsManagerTest.java similarity index 97% rename from catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/LocalSecretsManagerTest.java rename to catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/NoopSecretsManagerTest.java index 55904d8c89b..3f258ad8218 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/LocalSecretsManagerTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/NoopSecretsManagerTest.java @@ -62,17 +62,17 @@ import org.openmetadata.catalog.services.connections.mlModel.SklearnConnection; import org.openmetadata.catalog.type.EntityReference; @ExtendWith(MockitoExtension.class) -public class LocalSecretsManagerTest { +public class NoopSecretsManagerTest { private static final boolean ENCRYPT = true; private static final boolean DECRYPT = false; private static final String ENCRYPTED_VALUE = "fernet:abcdef"; private static final String DECRYPTED_VALUE = "123456"; - private static LocalSecretsManager secretsManager; + private static NoopSecretsManager secretsManager; @BeforeAll static void setUp() { - secretsManager = LocalSecretsManager.getInstance("openmetadata"); + secretsManager = NoopSecretsManager.getInstance("openmetadata"); Fernet fernet = Mockito.mock(Fernet.class); lenient().when(fernet.decrypt(anyString())).thenReturn(DECRYPTED_VALUE); lenient().when(fernet.encrypt(anyString())).thenReturn(ENCRYPTED_VALUE); @@ -146,7 +146,7 @@ public class LocalSecretsManagerTest { @Test void testReturnsExpectedSecretManagerProvider() { - assertEquals(SecretsManagerProvider.LOCAL, secretsManager.getSecretsManagerProvider()); + assertEquals(SecretsManagerProvider.NOOP, secretsManager.getSecretsManagerProvider()); } @ParameterizedTest diff --git a/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/SecretsManagerFactoryTest.java b/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/SecretsManagerFactoryTest.java index 7ff3e7cbb92..4d63faa1f6d 100644 --- a/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/SecretsManagerFactoryTest.java +++ b/catalog-rest-service/src/test/java/org/openmetadata/catalog/secrets/SecretsManagerFactoryTest.java @@ -33,18 +33,18 @@ public class SecretsManagerFactoryTest { @Test void testDefaultIsCreatedIfNullConfig() { - assertTrue(SecretsManagerFactory.createSecretsManager(config, CLUSTER_NAME) instanceof LocalSecretsManager); + assertTrue(SecretsManagerFactory.createSecretsManager(config, CLUSTER_NAME) instanceof NoopSecretsManager); } @Test void testDefaultIsCreatedIfMissingSecretManager() { - assertTrue(SecretsManagerFactory.createSecretsManager(config, CLUSTER_NAME) instanceof LocalSecretsManager); + assertTrue(SecretsManagerFactory.createSecretsManager(config, CLUSTER_NAME) instanceof NoopSecretsManager); } @Test void testIsCreatedIfLocalSecretsManager() { - config.setSecretsManager(SecretsManagerProvider.LOCAL); - assertTrue(SecretsManagerFactory.createSecretsManager(config, CLUSTER_NAME) instanceof LocalSecretsManager); + config.setSecretsManager(SecretsManagerProvider.NOOP); + assertTrue(SecretsManagerFactory.createSecretsManager(config, CLUSTER_NAME) instanceof NoopSecretsManager); } @Test diff --git a/catalog-rest-service/src/test/resources/openmetadata-secure-test.yaml b/catalog-rest-service/src/test/resources/openmetadata-secure-test.yaml index b698272be8a..32040d7c4f4 100644 --- a/catalog-rest-service/src/test/resources/openmetadata-secure-test.yaml +++ b/catalog-rest-service/src/test/resources/openmetadata-secure-test.yaml @@ -114,7 +114,7 @@ migrationConfiguration: # port: 0 secretsManagerConfiguration: - secretsManager: local + secretsManager: noop health: delayedShutdownHandlerEnabled: true diff --git a/conf/openmetadata.yaml b/conf/openmetadata.yaml index fd25caa3692..e34df0c2677 100644 --- a/conf/openmetadata.yaml +++ b/conf/openmetadata.yaml @@ -210,7 +210,7 @@ fernetConfiguration: fernetKey: ${FERNET_KEY:-jJ/9sz0g0OHxsfxOoSfdFdmk3ysNmPRnH3TUAbz3IHA=} secretsManagerConfiguration: - secretsManager: ${SECRET_MANAGER:-local} # Possible values are "local", "aws", "aws-ssm" + secretsManager: ${SECRET_MANAGER:-noop} # Possible values are "noop", "aws", "aws-ssm" # it will use the default auth provider for the secrets' manager service if parameters are not set parameters: region: ${OM_SM_REGION:-""} diff --git a/ingestion/src/metadata/utils/secrets/aws_based_secrets_manager.py b/ingestion/src/metadata/utils/secrets/aws_based_secrets_manager.py index 8e5af2200e0..f5a87df0915 100644 --- a/ingestion/src/metadata/utils/secrets/aws_based_secrets_manager.py +++ b/ingestion/src/metadata/utils/secrets/aws_based_secrets_manager.py @@ -12,40 +12,23 @@ """ Abstract class for AWS based secrets manager implementations """ -import json from abc import ABC, abstractmethod from typing import Optional from metadata.clients.aws_client import AWSClient -from metadata.generated.schema.entity.services.connections.metadata.openMetadataConnection import ( - AuthProvider, - OpenMetadataConnection, -) from metadata.generated.schema.entity.services.connections.metadata.secretsManagerProvider import ( SecretsManagerProvider, ) -from metadata.generated.schema.entity.services.connections.serviceConnection import ( - ServiceConnection, -) -from metadata.generated.schema.metadataIngestion.workflow import SourceConfig from metadata.generated.schema.security.credentials.awsCredentials import AWSCredentials from metadata.utils.logger import utils_logger -from metadata.utils.secrets.secrets_manager import ( - AUTH_PROVIDER_MAPPING, - AUTH_PROVIDER_SECRET_PREFIX, - DBT_SOURCE_CONFIG_SECRET_PREFIX, - TEST_CONNECTION_TEMP_SECRET_PREFIX, - SecretsManager, - ServiceConnectionType, - ServiceWithConnectionType, -) +from metadata.utils.secrets.external_secrets_manager import ExternalSecretsManager logger = utils_logger() NULL_VALUE = "null" -class AWSBasedSecretsManager(SecretsManager, ABC): +class AWSBasedSecretsManager(ExternalSecretsManager, ABC): def __init__( self, credentials: Optional[AWSCredentials], @@ -53,94 +36,8 @@ class AWSBasedSecretsManager(SecretsManager, ABC): provider: SecretsManagerProvider, cluster_prefix: str, ): - super().__init__(cluster_prefix) + super().__init__(cluster_prefix, provider) self.client = AWSClient(credentials).get_client(client) - self.provider = provider.name - - def retrieve_service_connection( - self, - service: ServiceWithConnectionType, - service_type: str, - ) -> ServiceConnection: - """ - Retrieve the service connection from the AWS client store from a given service connection object. - :param service: Service connection object e.g. DatabaseConnection - :param service_type: Service type e.g. databaseService - """ - logger.debug( - f"Retrieving service connection from {self.provider} secrets' manager for {service_type} - {service.name}" - ) - service_connection_type = service.serviceType.value - service_name = service.name.__root__ - secret_id = self.build_secret_id( - "service", service_type, service_connection_type, service_name - ) - connection_class = self.get_connection_class( - service_type, service_connection_type - ) - service_conn_class = self.get_service_connection_class(service_type) - service_connection = service_conn_class( - config=connection_class.parse_obj( - json.loads(self.get_string_value(secret_id)) - ) - ) - return ServiceConnection(__root__=service_connection) - - def add_auth_provider_security_config(self, config: OpenMetadataConnection) -> None: - """ - Add the auth provider security config from the AWS client store to a given OpenMetadata connection object. - :param config: OpenMetadataConnection object - """ - logger.debug( - f"Adding auth provider security config using {self.provider} secrets' manager" - ) - if config.authProvider != AuthProvider.no_auth: - secret_id = self.build_secret_id( - AUTH_PROVIDER_SECRET_PREFIX, config.authProvider.value.lower() - ) - auth_config_json = self.get_string_value(secret_id) - try: - config.securityConfig = AUTH_PROVIDER_MAPPING.get( - config.authProvider - ).parse_obj(json.loads(auth_config_json)) - except KeyError as err: - msg = f"No client implemented for auth provider [{config.authProvider}]: {err}" - raise NotImplementedError(msg) - - def retrieve_dbt_source_config( - self, source_config: SourceConfig, pipeline_name: str - ) -> object: - """ - Retrieve the DBT source config from the AWS client store from a source config object. - :param source_config: SourceConfig object - :param pipeline_name: the pipeline's name - :return: - """ - logger.debug( - f"Retrieving source_config from {self.provider} secrets' manager for {pipeline_name}" - ) - secret_id = self.build_secret_id(DBT_SOURCE_CONFIG_SECRET_PREFIX, pipeline_name) - source_config_json = self.get_string_value(secret_id) - return json.loads(source_config_json) if source_config_json else None - - def retrieve_temp_service_test_connection( - self, - connection: ServiceConnectionType, - service_type: str, - ) -> ServiceConnectionType: - """ - Retrieve the service connection from the AWS client stored in a temporary secret depending on the service - type. - :param connection: Connection of the service - :param service_type: Service type e.g. Database - """ - secret_id = self.build_secret_id( - TEST_CONNECTION_TEMP_SECRET_PREFIX, service_type - ) - service_conn_class = self.get_service_connection_class(service_type) - stored_connection = service_conn_class() - source_config_json = self.get_string_value(secret_id) - return stored_connection.parse_raw(source_config_json) @abstractmethod def get_string_value(self, name: str) -> str: diff --git a/ingestion/src/metadata/utils/secrets/external_secrets_manager.py b/ingestion/src/metadata/utils/secrets/external_secrets_manager.py new file mode 100644 index 00000000000..e7c4b67d10a --- /dev/null +++ b/ingestion/src/metadata/utils/secrets/external_secrets_manager.py @@ -0,0 +1,147 @@ +# Copyright 2022 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. + +""" +Abstract class for third party secrets' manager implementations +""" +import json +from abc import ABC, abstractmethod + +from metadata.clients.aws_client import AWSClient +from metadata.generated.schema.entity.services.connections.metadata.openMetadataConnection import ( + AuthProvider, + OpenMetadataConnection, +) +from metadata.generated.schema.entity.services.connections.metadata.secretsManagerProvider import ( + SecretsManagerProvider, +) +from metadata.generated.schema.entity.services.connections.serviceConnection import ( + ServiceConnection, +) +from metadata.generated.schema.metadataIngestion.workflow import SourceConfig +from metadata.generated.schema.security.credentials.awsCredentials import AWSCredentials +from metadata.utils.logger import utils_logger +from metadata.utils.secrets.secrets_manager import ( + AUTH_PROVIDER_MAPPING, + AUTH_PROVIDER_SECRET_PREFIX, + DBT_SOURCE_CONFIG_SECRET_PREFIX, + TEST_CONNECTION_TEMP_SECRET_PREFIX, + SecretsManager, + ServiceConnectionType, + ServiceWithConnectionType, +) + +logger = utils_logger() + +NULL_VALUE = "null" + + +class ExternalSecretsManager(SecretsManager, ABC): + def __init__( + self, + cluster_prefix: str, + provider: SecretsManagerProvider, + ): + super().__init__(cluster_prefix) + self.provider = provider.name + + def retrieve_service_connection( + self, + service: ServiceWithConnectionType, + service_type: str, + ) -> ServiceConnection: + """ + Retrieve the service connection from the AWS client store from a given service connection object. + :param service: Service connection object e.g. DatabaseConnection + :param service_type: Service type e.g. databaseService + """ + logger.debug( + f"Retrieving service connection from {self.provider} secrets' manager for {service_type} - {service.name}" + ) + service_connection_type = service.serviceType.value + service_name = service.name.__root__ + secret_id = self.build_secret_id( + "service", service_type, service_connection_type, service_name + ) + connection_class = self.get_connection_class( + service_type, service_connection_type + ) + service_conn_class = self.get_service_connection_class(service_type) + service_connection = service_conn_class( + config=connection_class.parse_obj( + json.loads(self.get_string_value(secret_id)) + ) + ) + return ServiceConnection(__root__=service_connection) + + def add_auth_provider_security_config(self, config: OpenMetadataConnection) -> None: + """ + Add the auth provider security config from the AWS client store to a given OpenMetadata connection object. + :param config: OpenMetadataConnection object + """ + logger.debug( + f"Adding auth provider security config using {self.provider} secrets' manager" + ) + if config.authProvider != AuthProvider.no_auth: + secret_id = self.build_secret_id( + AUTH_PROVIDER_SECRET_PREFIX, config.authProvider.value.lower() + ) + auth_config_json = self.get_string_value(secret_id) + try: + config.securityConfig = AUTH_PROVIDER_MAPPING.get( + config.authProvider + ).parse_obj(json.loads(auth_config_json)) + except KeyError as err: + msg = f"No client implemented for auth provider [{config.authProvider}]: {err}" + raise NotImplementedError(msg) + + def retrieve_dbt_source_config( + self, source_config: SourceConfig, pipeline_name: str + ) -> object: + """ + Retrieve the DBT source config from the AWS client store from a source config object. + :param source_config: SourceConfig object + :param pipeline_name: the pipeline's name + :return: + """ + logger.debug( + f"Retrieving source_config from {self.provider} secrets' manager for {pipeline_name}" + ) + secret_id = self.build_secret_id(DBT_SOURCE_CONFIG_SECRET_PREFIX, pipeline_name) + source_config_json = self.get_string_value(secret_id) + return json.loads(source_config_json) if source_config_json else None + + def retrieve_temp_service_test_connection( + self, + connection: ServiceConnectionType, + service_type: str, + ) -> ServiceConnectionType: + """ + Retrieve the service connection from the AWS client stored in a temporary secret depending on the service + type. + :param connection: Connection of the service + :param service_type: Service type e.g. Database + """ + secret_id = self.build_secret_id( + TEST_CONNECTION_TEMP_SECRET_PREFIX, service_type + ) + service_conn_class = self.get_service_connection_class(service_type) + stored_connection = service_conn_class() + source_config_json = self.get_string_value(secret_id) + return stored_connection.parse_raw(source_config_json) + + @abstractmethod + def get_string_value(self, name: str) -> str: + """ + :param name: The secret name to retrieve + :return: The value of the secret + """ + pass diff --git a/ingestion/src/metadata/utils/secrets/local_secrets_manager.py b/ingestion/src/metadata/utils/secrets/noop_secrets_manager.py similarity index 97% rename from ingestion/src/metadata/utils/secrets/local_secrets_manager.py rename to ingestion/src/metadata/utils/secrets/noop_secrets_manager.py index 6048fac0099..ab90f8f148c 100644 --- a/ingestion/src/metadata/utils/secrets/local_secrets_manager.py +++ b/ingestion/src/metadata/utils/secrets/noop_secrets_manager.py @@ -30,12 +30,12 @@ from metadata.utils.secrets.secrets_manager import ( ) -class LocalSecretsManager(SecretsManager): +class NoopSecretsManager(SecretsManager): """ LocalSecretsManager is used when there is not a secrets' manager configured. """ - provider: str = SecretsManagerProvider.local.name + provider: str = SecretsManagerProvider.noop.name def add_auth_provider_security_config( self, open_metadata_connection: OpenMetadataConnection diff --git a/ingestion/src/metadata/utils/secrets/secrets_manager_factory.py b/ingestion/src/metadata/utils/secrets/secrets_manager_factory.py index 15ea977538a..978803687a4 100644 --- a/ingestion/src/metadata/utils/secrets/secrets_manager_factory.py +++ b/ingestion/src/metadata/utils/secrets/secrets_manager_factory.py @@ -23,7 +23,7 @@ from metadata.generated.schema.entity.services.connections.metadata.secretsManag from metadata.generated.schema.security.credentials.awsCredentials import AWSCredentials from metadata.utils.secrets.aws_secrets_manager import AWSSecretsManager from metadata.utils.secrets.aws_ssm_secrets_manager import AWSSSMSecretsManager -from metadata.utils.secrets.local_secrets_manager import LocalSecretsManager +from metadata.utils.secrets.noop_secrets_manager import NoopSecretsManager from metadata.utils.secrets.secrets_manager import SecretsManager @@ -58,9 +58,9 @@ def get_secrets_manager( """ if ( secrets_manager_provider is None - or secrets_manager_provider == SecretsManagerProvider.local + or secrets_manager_provider == SecretsManagerProvider.noop ): - return LocalSecretsManager(cluster_name) + return NoopSecretsManager(cluster_name) elif secrets_manager_provider == SecretsManagerProvider.aws: return AWSSecretsManager(credentials, cluster_name) elif secrets_manager_provider == SecretsManagerProvider.aws_ssm: diff --git a/ingestion/tests/integration/ometa/test_ometa_secrets_manager.py b/ingestion/tests/integration/ometa/test_ometa_secrets_manager.py index 8a7bfda6fd7..9cfdd77864c 100644 --- a/ingestion/tests/integration/ometa/test_ometa_secrets_manager.py +++ b/ingestion/tests/integration/ometa/test_ometa_secrets_manager.py @@ -30,7 +30,7 @@ from metadata.ingestion.ometa.auth_provider import ( ) from metadata.ingestion.ometa.ometa_api import OpenMetadata from metadata.utils.secrets.aws_secrets_manager import AWSSecretsManager -from metadata.utils.secrets.local_secrets_manager import LocalSecretsManager +from metadata.utils.secrets.noop_secrets_manager import NoopSecretsManager class OMetaSecretManagerTest(TestCase): @@ -55,7 +55,7 @@ class OMetaSecretManagerTest(TestCase): def test_ometa_with_local_secret_manager(self): self._init_local_secret_manager() - assert type(self.metadata.secrets_manager_client) is LocalSecretsManager + assert type(self.metadata.secrets_manager_client) is NoopSecretsManager assert type(self.metadata._auth_provider) is NoOpAuthenticationProvider def test_ometa_with_local_secret_manager_with_google_auth(self): @@ -64,7 +64,7 @@ class OMetaSecretManagerTest(TestCase): secretKey="/fake/path" ) self._init_local_secret_manager() - assert type(self.metadata.secrets_manager_client) is LocalSecretsManager + assert type(self.metadata.secrets_manager_client) is NoopSecretsManager assert type(self.metadata._auth_provider) is GoogleAuthenticationProvider def test_ometa_with_aws_secret_manager(self): diff --git a/ingestion/tests/unit/metadata/utils/secrets/test_local_secrets_manager.py b/ingestion/tests/unit/metadata/utils/secrets/test_noop_secrets_manager.py similarity index 77% rename from ingestion/tests/unit/metadata/utils/secrets/test_local_secrets_manager.py rename to ingestion/tests/unit/metadata/utils/secrets/test_noop_secrets_manager.py index dbdc01796c0..ef5880330b4 100644 --- a/ingestion/tests/unit/metadata/utils/secrets/test_local_secrets_manager.py +++ b/ingestion/tests/unit/metadata/utils/secrets/test_noop_secrets_manager.py @@ -33,14 +33,14 @@ from .test_secrets_manager import TestSecretsManager class TestLocalSecretsManager(TestSecretsManager.External): - def test_local_manager_add_service_config_connection(self): - local_manager = get_secrets_manager_from_om_connection( - self.build_open_metadata_connection(SecretsManagerProvider.local), None + def test_noop_manager_add_service_config_connection(self): + noop_manager = get_secrets_manager_from_om_connection( + self.build_open_metadata_connection(SecretsManagerProvider.noop), None ) expected_service_connection = self.service_connection actual_service_connection: ServiceConnection = ( - local_manager.retrieve_service_connection(self.service, self.service_type) + noop_manager.retrieve_service_connection(self.service, self.service_type) ) self.assertEqual(actual_service_connection, expected_service_connection) @@ -48,41 +48,41 @@ class TestLocalSecretsManager(TestSecretsManager.External): expected_service_connection.__root__.config ) - def test_local_manager_add_auth_provider_security_config(self): - local_manager = get_secrets_manager_from_om_connection( - self.build_open_metadata_connection(SecretsManagerProvider.local), None + def test_noop_manager_add_auth_provider_security_config(self): + noop_manager = get_secrets_manager_from_om_connection( + self.build_open_metadata_connection(SecretsManagerProvider.noop), None ) actual_om_connection = deepcopy(self.om_connection) actual_om_connection.securityConfig = self.auth_provider_config - local_manager.add_auth_provider_security_config(actual_om_connection) + noop_manager.add_auth_provider_security_config(actual_om_connection) self.assertEqual(self.auth_provider_config, actual_om_connection.securityConfig) assert id(self.auth_provider_config) == id(actual_om_connection.securityConfig) - def test_local_manager_retrieve_dbt_source_config(self): - local_manager = get_secrets_manager_from_om_connection( - self.build_open_metadata_connection(SecretsManagerProvider.local), None + def test_noop_manager_retrieve_dbt_source_config(self): + noop_manager = get_secrets_manager_from_om_connection( + self.build_open_metadata_connection(SecretsManagerProvider.noop), None ) source_config = SourceConfig() source_config.config = DatabaseServiceMetadataPipeline( dbtConfigSource=self.dbt_source_config ) - actual_dbt_source_config = local_manager.retrieve_dbt_source_config( + actual_dbt_source_config = noop_manager.retrieve_dbt_source_config( source_config, "test-pipeline" ) self.assertEqual(self.dbt_source_config.dict(), actual_dbt_source_config) - def test_local_manager_retrieve_temp_service_test_connection(self): - local_manager = get_secrets_manager_from_om_connection( - self.build_open_metadata_connection(SecretsManagerProvider.local), None + def test_noop_manager_retrieve_temp_service_test_connection(self): + noop_manager = get_secrets_manager_from_om_connection( + self.build_open_metadata_connection(SecretsManagerProvider.noop), None ) expected_service_connection = self.service.connection actual_service_connection: DatabaseConnection = ( - local_manager.retrieve_temp_service_test_connection( + noop_manager.retrieve_temp_service_test_connection( self.service.connection, "Database" ) ) diff --git a/ingestion/tests/unit/metadata/utils/secrets/test_secrets_manager_factory.py b/ingestion/tests/unit/metadata/utils/secrets/test_secrets_manager_factory.py index 3ee553783e3..321afe833b2 100644 --- a/ingestion/tests/unit/metadata/utils/secrets/test_secrets_manager_factory.py +++ b/ingestion/tests/unit/metadata/utils/secrets/test_secrets_manager_factory.py @@ -39,7 +39,7 @@ class TestSecretsManagerFactory(TestCase): with self.assertRaises(NotImplementedError) as not_implemented_error: om_connection: OpenMetadataConnection = ( TestSecretsManager.External.build_open_metadata_connection( - SecretsManagerProvider.local + SecretsManagerProvider.noop ) ) om_connection.secretsManagerProvider = "aws" @@ -51,7 +51,7 @@ class TestSecretsManagerFactory(TestCase): def test_get_none_secret_manager(self): om_connection: OpenMetadataConnection = ( TestSecretsManager.External.build_open_metadata_connection( - SecretsManagerProvider.local + SecretsManagerProvider.noop ) ) om_connection.secretsManagerProvider = None diff --git a/openmetadata-core/src/main/resources/json/schema/entity/services/connections/metadata/secretsManagerProvider.json b/openmetadata-core/src/main/resources/json/schema/entity/services/connections/metadata/secretsManagerProvider.json index 12a68c6cf4c..0dc9a90fdbe 100644 --- a/openmetadata-core/src/main/resources/json/schema/entity/services/connections/metadata/secretsManagerProvider.json +++ b/openmetadata-core/src/main/resources/json/schema/entity/services/connections/metadata/secretsManagerProvider.json @@ -5,6 +5,6 @@ "description": "OpenMetadata Secrets Manager Provider. Make sure to configure the same secrets manager providers as the ones configured on the OpenMetadata server.", "type": "string", "javaType": "org.openmetadata.catalog.services.connections.metadata.SecretsManagerProvider", - "enum": ["local", "aws", "aws-ssm"], + "enum": ["noop", "aws", "aws-ssm"], "additionalProperties": false }