#14340 & #13849 - Clean secret ID and improve encrypt/decrypt excepti…

…on management (#14356)

* Fix supported characters in SM

* Update SM

* Fixes

* Fixes

* Improve class conversion exceptions

* Comments

* Rename noop to db secrets manager providee

* Update sm

* Fix

* db SM

* db SM

* Fix test

* UI

* Update openmetadata-ui/src/main/resources/ui/src/mocks/IngestionListTable.mock.ts

* update default

bootstrap/sql/migrations/native/1.3.0/mysql/schemaChanges.sql

@@ -31,3 +31,9 @@ set json = JSON_INSERT(
   false
 )
 where name = 'DataInsightsApplication';
+
+-- Rename NOOP Secret Manager to DB
+update metadata_service_entity
+set json = JSON_REPLACE(json, '$.connection.config.secretsManagerProvider', 'db')
+where name = 'OpenMetadata'
+  and JSON_EXTRACT(json, '$.connection.config.secretsManagerProvider') = 'noop';

bootstrap/sql/migrations/native/1.3.0/postgres/schemaChanges.sql

@@ -38,3 +38,14 @@ SET json = jsonb_set(
 	to_jsonb(false)
 )
 where name = 'DataInsightsApplication';
+
+-- Rename NOOP Secret Manager to DB
+update metadata_service_entity
+set json = jsonb_set(
+  json #- '{connection,config,secretsManagerProvider}',
+  '{connection,config,secretsManagerProvider}',
+  '"db"',
+  true
+)
+where name = 'OpenMetadata'
+  and json #>> '{connection,config,secretsManagerProvider}' = 'noop';

common/src/main/java/org/openmetadata/common/utils/CommonUtil.java

@@ -50,7 +50,7 @@
 @Slf4j
 public final class CommonUtil {
 
-  private static final List<String> jarNameFilter = List.of("openmetadata", "collate");
+  private static final List<String> JAR_NAME_FILTER = List.of("openmetadata", "collate");
 
   private CommonUtil() {}
 
@@ -60,7 +60,7 @@ public static List<String> getResources(Pattern pattern) throws IOException {
     String classPath = System.getProperty("java.class.path", ".");
     List<String> classPathElements =
         Arrays.stream(classPath.split(File.pathSeparator))
-            .filter(jarName -> jarNameFilter.stream().anyMatch(jarName.toLowerCase()::contains))
+            .filter(jarName -> JAR_NAME_FILTER.stream().anyMatch(jarName.toLowerCase()::contains))
             .toList();
 
     for (String element : classPathElements) {
@@ -71,6 +71,13 @@ public static List<String> getResources(Pattern pattern) throws IOException {
     return resources;
   }
 
+  /** Check if any given object falls under OM, or Collate packages */
+  public static Boolean isOpenMetadataObject(Object obj) {
+    return obj != null
+        && JAR_NAME_FILTER.stream()
+            .anyMatch(Arrays.stream(obj.getClass().getPackageName().split("\\.")).toList()::contains);
+  }
+
   private static Collection<String> getResourcesFromJarFile(File file, Pattern pattern) {
     ArrayList<String> retval = new ArrayList<>();
     try (ZipFile zf = new ZipFile(file)) {

conf/openmetadata.yaml

@@ -278,7 +278,9 @@ fernetConfiguration:
   fernetKey: ${FERNET_KEY:-jJ/9sz0g0OHxsfxOoSfdFdmk3ysNmPRnH3TUAbz3IHA=}
 
 secretsManagerConfiguration:
-  secretsManager: ${SECRET_MANAGER:-noop} # Possible values are "noop", "aws", "aws-ssm"
+  secretsManager: ${SECRET_MANAGER:-db} # Possible values are "db", "managed-aws", "managed-aws-ssm"
+  prefix: ${SECRET_MANAGER_PREFIX:-""} # Define the secret key ID as /<prefix>/<clusterName>/<key>
+  tags: ${SECRET_MANAGER_TAGS:-[]} # Add tags to the created resource, e.g., in AWS. Format is `[key1:value1,key2:value2,...]`
 # it will use the default auth provider for the secrets' manager service if parameters are not set
   parameters:
     region: ${OM_SM_REGION:-""}

docker/development/docker-compose-postgres.yml

@@ -149,7 +149,7 @@ services:
       FERNET_KEY: ${FERNET_KEY:-jJ/9sz0g0OHxsfxOoSfdFdmk3ysNmPRnH3TUAbz3IHA=}
 
       #secretsManagerConfiguration
-      SECRET_MANAGER: ${SECRET_MANAGER:-noop}
+      SECRET_MANAGER: ${SECRET_MANAGER:-db}
       #parameters:
       OM_SM_REGION: ${OM_SM_REGION:-""}
       OM_SM_ACCESS_KEY_ID: ${OM_SM_ACCESS_KEY_ID:-""}
@@ -295,7 +295,7 @@ services:
       FERNET_KEY: ${FERNET_KEY:-jJ/9sz0g0OHxsfxOoSfdFdmk3ysNmPRnH3TUAbz3IHA=}
 
       #secretsManagerConfiguration
-      SECRET_MANAGER: ${SECRET_MANAGER:-noop}
+      SECRET_MANAGER: ${SECRET_MANAGER:-db}
       #parameters:
       OM_SM_REGION: ${OM_SM_REGION:-""}
       OM_SM_ACCESS_KEY_ID: ${OM_SM_ACCESS_KEY_ID:-""}

docker/development/docker-compose.yml

@@ -148,7 +148,7 @@ services:
       FERNET_KEY: ${FERNET_KEY:-jJ/9sz0g0OHxsfxOoSfdFdmk3ysNmPRnH3TUAbz3IHA=}
 
       #secretsManagerConfiguration
-      SECRET_MANAGER: ${SECRET_MANAGER:-noop}
+      SECRET_MANAGER: ${SECRET_MANAGER:-db}
       #parameters:
       OM_SM_REGION: ${OM_SM_REGION:-""}
       OM_SM_ACCESS_KEY_ID: ${OM_SM_ACCESS_KEY_ID:-""}
@@ -296,7 +296,7 @@ services:
       FERNET_KEY: ${FERNET_KEY:-jJ/9sz0g0OHxsfxOoSfdFdmk3ysNmPRnH3TUAbz3IHA=}
 
       #secretsManagerConfiguration
-      SECRET_MANAGER: ${SECRET_MANAGER:-noop}
+      SECRET_MANAGER: ${SECRET_MANAGER:-db}
       #parameters:
       OM_SM_REGION: ${OM_SM_REGION:-""}
       OM_SM_ACCESS_KEY_ID: ${OM_SM_ACCESS_KEY_ID:-""}

docker/docker-compose-openmetadata/docker-compose-openmetadata.yml

@@ -98,7 +98,7 @@ services:
       FERNET_KEY: ${FERNET_KEY:-jJ/9sz0g0OHxsfxOoSfdFdmk3ysNmPRnH3TUAbz3IHA=}
 
       #secretsManagerConfiguration
-      SECRET_MANAGER: ${SECRET_MANAGER:-noop}
+      SECRET_MANAGER: ${SECRET_MANAGER:-db}
       #parameters:
       OM_SM_REGION: ${OM_SM_REGION:-""}
       OM_SM_ACCESS_KEY_ID: ${OM_SM_ACCESS_KEY_ID:-""}
@@ -240,7 +240,7 @@ services:
       FERNET_KEY: ${FERNET_KEY:-jJ/9sz0g0OHxsfxOoSfdFdmk3ysNmPRnH3TUAbz3IHA=}
 
       #secretsManagerConfiguration
-      SECRET_MANAGER: ${SECRET_MANAGER:-noop}
+      SECRET_MANAGER: ${SECRET_MANAGER:-db}
       #parameters:
       OM_SM_REGION: ${OM_SM_REGION:-""}
       OM_SM_ACCESS_KEY_ID: ${OM_SM_ACCESS_KEY_ID:-""}

docker/docker-compose-quickstart/docker-compose-postgres.yml

@@ -142,7 +142,7 @@ services:
       FERNET_KEY: ${FERNET_KEY:-jJ/9sz0g0OHxsfxOoSfdFdmk3ysNmPRnH3TUAbz3IHA=}
 
       #secretsManagerConfiguration
-      SECRET_MANAGER: ${SECRET_MANAGER:-noop}
+      SECRET_MANAGER: ${SECRET_MANAGER:-db}
       #parameters:
       OM_SM_REGION: ${OM_SM_REGION:-""}
       OM_SM_ACCESS_KEY_ID: ${OM_SM_ACCESS_KEY_ID:-""}
@@ -285,7 +285,7 @@ services:
       FERNET_KEY: ${FERNET_KEY:-jJ/9sz0g0OHxsfxOoSfdFdmk3ysNmPRnH3TUAbz3IHA=}
 
       #secretsManagerConfiguration
-      SECRET_MANAGER: ${SECRET_MANAGER:-noop}
+      SECRET_MANAGER: ${SECRET_MANAGER:-db}
       #parameters:
       OM_SM_REGION: ${OM_SM_REGION:-""}
       OM_SM_ACCESS_KEY_ID: ${OM_SM_ACCESS_KEY_ID:-""}

docker/docker-compose-quickstart/docker-compose.yml

@@ -140,7 +140,7 @@ services:
       FERNET_KEY: ${FERNET_KEY:-jJ/9sz0g0OHxsfxOoSfdFdmk3ysNmPRnH3TUAbz3IHA=}
 
       #secretsManagerConfiguration
-      SECRET_MANAGER: ${SECRET_MANAGER:-noop}
+      SECRET_MANAGER: ${SECRET_MANAGER:-db}
       #parameters:
       OM_SM_REGION: ${OM_SM_REGION:-""}
       OM_SM_ACCESS_KEY_ID: ${OM_SM_ACCESS_KEY_ID:-""}
@@ -283,7 +283,7 @@ services:
       FERNET_KEY: ${FERNET_KEY:-jJ/9sz0g0OHxsfxOoSfdFdmk3ysNmPRnH3TUAbz3IHA=}
 
       #secretsManagerConfiguration
-      SECRET_MANAGER: ${SECRET_MANAGER:-noop}
+      SECRET_MANAGER: ${SECRET_MANAGER:-db}
       #parameters:
       OM_SM_REGION: ${OM_SM_REGION:-""}
       OM_SM_ACCESS_KEY_ID: ${OM_SM_ACCESS_KEY_ID:-""}

ingestion/src/metadata/ingestion/models/custom_pydantic.py

@@ -25,6 +25,8 @@
 
 logger = logging.getLogger("metadata")
 
+SECRET = "secret:"
+
 
 class CustomSecretStr(SecretStr):
     """
@@ -91,10 +93,10 @@ def get_secret_value(self, skip_secret_manager: bool = False) -> str:
 
         if (
             not skip_secret_manager
-            and self._secret_value.startswith("secret:")
+            and self._secret_value.startswith(SECRET)
             and SecretsManagerFactory().get_secrets_manager()
         ):
-            secret_id = self._secret_value.replace("secret:", "")
+            secret_id = self._secret_value.replace(SECRET, "")
             try:
                 return (
                     SecretsManagerFactory()

ingestion/src/metadata/utils/secrets/noop_secrets_manager.py

@@ -18,12 +18,13 @@
 from metadata.utils.secrets.secrets_manager import SecretsManager
 
 
-class NoopSecretsManager(SecretsManager):
+class DBSecretsManager(SecretsManager):
     """
-    LocalSecretsManager is used when there is not a secrets' manager configured.
+    Will return the string as received. The client does not need to pick
+    it up from any external resource.
     """
 
-    provider: str = SecretsManagerProvider.noop.name
+    provider: str = SecretsManagerProvider.db.name
 
     def get_string_value(self, secret_id: str) -> str:
         return secret_id

ingestion/src/metadata/utils/secrets/secrets_manager_factory.py

@@ -23,7 +23,7 @@
 from metadata.utils.secrets.aws_secrets_manager import AWSSecretsManager
 from metadata.utils.secrets.aws_ssm_secrets_manager import AWSSSMSecretsManager
 from metadata.utils.secrets.client.loader import secrets_manager_client_loader
-from metadata.utils.secrets.noop_secrets_manager import NoopSecretsManager
+from metadata.utils.secrets.noop_secrets_manager import DBSecretsManager
 from metadata.utils.secrets.secrets_manager import SecretsManager
 from metadata.utils.singleton import Singleton
 
@@ -85,9 +85,9 @@ def _get_secrets_manager(
         """
         if (
             secrets_manager_provider is None
-            or secrets_manager_provider == SecretsManagerProvider.noop
+            or secrets_manager_provider == SecretsManagerProvider.db
         ):
-            return NoopSecretsManager()
+            return DBSecretsManager()
         if secrets_manager_provider in (
             SecretsManagerProvider.aws,
             SecretsManagerProvider.managed_aws,

ingestion/tests/integration/ometa/test_ometa_secrets_manager.py

@@ -30,7 +30,7 @@
 )
 from metadata.ingestion.ometa.ometa_api import OpenMetadata
 from metadata.utils.secrets.aws_secrets_manager import AWSSecretsManager
-from metadata.utils.secrets.noop_secrets_manager import NoopSecretsManager
+from metadata.utils.secrets.noop_secrets_manager import DBSecretsManager
 from metadata.utils.singleton import Singleton
 
 
@@ -55,7 +55,7 @@ def setUp(cls) -> None:
 
     def test_ometa_with_local_secret_manager(self):
         self._init_local_secret_manager()
-        assert type(self.metadata.secrets_manager_client) is NoopSecretsManager
+        assert type(self.metadata.secrets_manager_client) is DBSecretsManager
         assert type(self.metadata._auth_provider) is NoOpAuthenticationProvider
 
     def test_ometa_with_local_secret_manager_with_google_auth(self):
@@ -64,7 +64,7 @@ def test_ometa_with_local_secret_manager_with_google_auth(self):
             secretKey="/fake/path"
         )
         self._init_local_secret_manager()
-        assert type(self.metadata.secrets_manager_client) is NoopSecretsManager
+        assert type(self.metadata.secrets_manager_client) is DBSecretsManager
         assert type(self.metadata._auth_provider) is GoogleAuthenticationProvider
 
     @mock.patch.dict(os.environ, {"AWS_DEFAULT_REGION": "us-east-2"}, clear=True)

ingestion/tests/unit/metadata/utils/secrets/test_secrets_manager_factory.py

@@ -24,7 +24,7 @@
 from metadata.generated.schema.security.secrets.secretsManagerProvider import (
     SecretsManagerProvider,
 )
-from metadata.utils.secrets.noop_secrets_manager import NoopSecretsManager
+from metadata.utils.secrets.noop_secrets_manager import DBSecretsManager
 from metadata.utils.secrets.secrets_manager_factory import (
     SecretsManagerConfigException,
     SecretsManagerFactory,
@@ -40,7 +40,7 @@ def setUp(cls) -> None:
     def test_get_not_implemented_secret_manager(self):
         with self.assertRaises(NotImplementedError) as not_implemented_error:
             om_connection: OpenMetadataConnection = self.build_open_metadata_connection(
-                SecretsManagerProvider.noop,
+                SecretsManagerProvider.db,
                 SecretsManagerClientLoader.noop,
             )
             om_connection.secretsManagerProvider = "aws"
@@ -54,7 +54,7 @@ def test_get_not_implemented_secret_manager(self):
     def test_invalid_config_secret_manager(self):
 
         om_connection: OpenMetadataConnection = self.build_open_metadata_connection(
-            SecretsManagerProvider.noop,
+            SecretsManagerProvider.db,
             SecretsManagerClientLoader.noop,
         )
         om_connection.secretsManagerLoader = "random"
@@ -66,7 +66,7 @@ def test_invalid_config_secret_manager(self):
 
     def test_get_none_secret_manager(self):
         om_connection: OpenMetadataConnection = self.build_open_metadata_connection(
-            SecretsManagerProvider.noop,
+            SecretsManagerProvider.db,
             SecretsManagerClientLoader.noop,
         )
         om_connection.secretsManagerProvider = None
@@ -77,7 +77,7 @@ def test_get_none_secret_manager(self):
         )
         assert secrets_manager_factory.get_secrets_manager() is not None
         assert isinstance(
-            secrets_manager_factory.get_secrets_manager(), NoopSecretsManager
+            secrets_manager_factory.get_secrets_manager(), DBSecretsManager
         )
 
     @patch("metadata.clients.aws_client.boto3")

openmetadata-service/src/main/java/org/openmetadata/service/exception/SecretsManagerException.java

@@ -24,6 +24,10 @@ public SecretsManagerException(String message) {
     super(Response.Status.INTERNAL_SERVER_ERROR, message);
   }
 
+  public SecretsManagerException(Response.Status status, String message) {
+    super(status.getStatusCode(), message);
+  }
+
   public static SecretsManagerException byMessage(String secretManager, String connectionType, String errorMessage) {
     return new SecretsManagerException(buildMessageByName(secretManager, connectionType, errorMessage));
   }

openmetadata-service/src/main/java/org/openmetadata/service/secrets/AWSBasedSecretsManager.java

@@ -14,7 +14,6 @@
 package org.openmetadata.service.secrets;
 
 import org.apache.logging.log4j.util.Strings;
-import org.openmetadata.schema.security.secrets.SecretsManagerConfiguration;
 import org.openmetadata.schema.security.secrets.SecretsManagerProvider;
 import software.amazon.awssdk.auth.credentials.AwsBasicCredentials;
 import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
@@ -27,17 +26,17 @@ public abstract class AWSBasedSecretsManager extends ExternalSecretsManager {
   public static final String SECRET_ACCESS_KEY = "secretAccessKey";
   public static final String REGION = "region";
 
-  protected AWSBasedSecretsManager(
-      SecretsManagerProvider awsProvider, SecretsManagerConfiguration config, String clusterPrefix) {
-    super(awsProvider, clusterPrefix, 100);
+  protected AWSBasedSecretsManager(SecretsManagerProvider awsProvider, SecretsConfig secretsConfig) {
+    super(awsProvider, secretsConfig, 100);
     // initialize the secret client depending on the SecretsManagerConfiguration passed
-    if (config != null
-        && config.getParameters() != null
-        && !Strings.isBlank((String) config.getParameters().getAdditionalProperties().getOrDefault(REGION, ""))) {
-      String region = (String) config.getParameters().getAdditionalProperties().getOrDefault(REGION, "");
-      String accessKeyId = (String) config.getParameters().getAdditionalProperties().getOrDefault(ACCESS_KEY_ID, "");
+    if (secretsConfig != null
+        && secretsConfig.parameters() != null
+        && !Strings.isBlank((String) secretsConfig.parameters().getAdditionalProperties().getOrDefault(REGION, ""))) {
+      String region = (String) secretsConfig.parameters().getAdditionalProperties().getOrDefault(REGION, "");
+      String accessKeyId =
+          (String) secretsConfig.parameters().getAdditionalProperties().getOrDefault(ACCESS_KEY_ID, "");
       String secretAccessKey =
-          (String) config.getParameters().getAdditionalProperties().getOrDefault(SECRET_ACCESS_KEY, "");
+          (String) secretsConfig.parameters().getAdditionalProperties().getOrDefault(SECRET_ACCESS_KEY, "");
       AwsCredentialsProvider credentialsProvider;
       if (Strings.isBlank(accessKeyId) && Strings.isBlank(secretAccessKey)) {
         credentialsProvider = DefaultCredentialsProvider.create();

openmetadata-service/src/main/java/org/openmetadata/service/secrets/AWSSSMSecretsManager.java

@@ -15,21 +15,21 @@
 import static org.openmetadata.schema.security.secrets.SecretsManagerProvider.MANAGED_AWS_SSM;
 
 import com.google.common.annotations.VisibleForTesting;
-import org.openmetadata.schema.security.secrets.SecretsManagerConfiguration;
 import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
 import software.amazon.awssdk.regions.Region;
 import software.amazon.awssdk.services.ssm.SsmClient;
 import software.amazon.awssdk.services.ssm.model.DeleteParameterRequest;
 import software.amazon.awssdk.services.ssm.model.GetParameterRequest;
 import software.amazon.awssdk.services.ssm.model.ParameterType;
 import software.amazon.awssdk.services.ssm.model.PutParameterRequest;
+import software.amazon.awssdk.services.ssm.model.Tag;
 
 public class AWSSSMSecretsManager extends AWSBasedSecretsManager {
   private static AWSSSMSecretsManager instance = null;
   private SsmClient ssmClient;
 
-  private AWSSSMSecretsManager(SecretsManagerConfiguration config, String clusterPrefix) {
-    super(MANAGED_AWS_SSM, config, clusterPrefix);
+  private AWSSSMSecretsManager(SecretsConfig secretsConfig) {
+    super(MANAGED_AWS_SSM, secretsConfig);
   }
 
   @Override
@@ -61,6 +61,10 @@ private void putSecretParameter(String parameterName, String parameterValue, boo
             .value(parameterValue)
             .overwrite(overwrite)
             .type(ParameterType.SECURE_STRING)
+            .tags(
+                SecretsManager.getTags(getSecretsConfig()).entrySet().stream()
+                    .map(entry -> Tag.builder().key(entry.getKey()).value(entry.getValue()).build())
+                    .toList())
             .build();
     this.ssmClient.putParameter(putParameterRequest);
   }
@@ -77,8 +81,8 @@ protected void deleteSecretInternal(String secretName) {
     this.ssmClient.deleteParameter(deleteParameterRequest);
   }
 
-  public static AWSSSMSecretsManager getInstance(SecretsManagerConfiguration config, String clusterPrefix) {
-    if (instance == null) instance = new AWSSSMSecretsManager(config, clusterPrefix);
+  public static AWSSSMSecretsManager getInstance(SecretsConfig secretsConfig) {
+    if (instance == null) instance = new AWSSSMSecretsManager(secretsConfig);
     return instance;
   }