Add delete secrets options for managed SM (#11972)

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/ServiceEntityRepository.java

@@ -100,6 +100,14 @@ public abstract class ServiceEntityRepository<
     return service;
   }
 
+  /** Remove the secrets from the secret manager */
+  @Override
+  protected void postDelete(T service) {
+    SecretsManagerFactory.getSecretsManager()
+        .deleteSecretsFromServiceConnectionConfig(
+            service.getConnection().getConfig(), service.getServiceType().value(), service.getName(), serviceType);
+  }
+
   @Override
   public ServiceUpdater getUpdater(T original, T updated, Operation operation) {
     return new ServiceUpdater(original, updated, operation);

openmetadata-service/src/main/java/org/openmetadata/service/jdbi3/WorkflowRepository.java

@@ -59,6 +59,12 @@ public class WorkflowRepository extends EntityRepository<Workflow> {
     entity.withOwner(owner).withOpenMetadataServerConnection(openmetadataConnection);
   }
 
+  /** Remove the secrets from the secret manager */
+  @Override
+  protected void postDelete(Workflow workflow) {
+    SecretsManagerFactory.getSecretsManager().deleteSecretsFromWorkflow(workflow);
+  }
+
   @Override
   public void storeRelationships(Workflow entity) {
     storeOwner(entity, entity.getOwner());

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

@@ -19,6 +19,7 @@ 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;
@@ -72,6 +73,12 @@ public class AWSSSMSecretsManager extends AWSBasedSecretsManager {
     return ssmClient.getParameter(parameterRequest).parameter().value();
   }
 
+  @Override
+  protected void deleteSecretInternal(String secretName) {
+    DeleteParameterRequest deleteParameterRequest = DeleteParameterRequest.builder().name(secretName).build();
+    this.ssmClient.deleteParameter(deleteParameterRequest);
+  }
+
   public static AWSSSMSecretsManager getInstance(SecretsManagerConfiguration config, String clusterPrefix) {
     if (INSTANCE == null) INSTANCE = new AWSSSMSecretsManager(config, clusterPrefix);
     return INSTANCE;

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

@@ -22,6 +22,7 @@ import software.amazon.awssdk.auth.credentials.AwsCredentialsProvider;
 import software.amazon.awssdk.regions.Region;
 import software.amazon.awssdk.services.secretsmanager.SecretsManagerClient;
 import software.amazon.awssdk.services.secretsmanager.model.CreateSecretRequest;
+import software.amazon.awssdk.services.secretsmanager.model.DeleteSecretRequest;
 import software.amazon.awssdk.services.secretsmanager.model.GetSecretValueRequest;
 import software.amazon.awssdk.services.secretsmanager.model.UpdateSecretRequest;
 
@@ -74,6 +75,12 @@ public class AWSSecretsManager extends AWSBasedSecretsManager {
     return this.secretsClient.getSecretValue(getSecretValueRequest).secretString();
   }
 
+  @Override
+  protected void deleteSecretInternal(String secretName) {
+    DeleteSecretRequest deleteSecretRequest = DeleteSecretRequest.builder().secretId(secretName).build();
+    this.secretsClient.deleteSecret(deleteSecretRequest);
+  }
+
   public static AWSSecretsManager getInstance(SecretsManagerConfiguration config, String clusterPrefix) {
     if (INSTANCE == null) INSTANCE = new AWSSecretsManager(config, clusterPrefix);
     return INSTANCE;

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

@@ -45,6 +45,11 @@ public class InMemorySecretsManager extends ExternalSecretsManager {
     storeSecret(secretName, secretValue);
   }
 
+  @Override
+  protected void deleteSecretInternal(String secretName) {
+    secretsMap.remove(secretName);
+  }
+
   @Override
   String getSecret(String secretName) {
     String value = secretsMap.getOrDefault(secretName, null);

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

@@ -32,4 +32,8 @@ public class NoopSecretsManager extends SecretsManager {
   protected String storeValue(String fieldName, String value, String secretId, boolean store) {
     return value;
   }
+
+  // Nothing to delete on the Noop SM. We only delete on External SM
+  @Override
+  protected void deleteSecretInternal(String secretName) {}
 }

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

@@ -301,4 +301,57 @@ public abstract class SecretsManager {
   void setFernet(Fernet fernet) {
     this.fernet = fernet;
   }
+
+  protected abstract void deleteSecretInternal(String secretName);
+
+  public void deleteSecretsFromServiceConnectionConfig(
+      Object connectionConfig, String connectionType, String connectionName, ServiceType serviceType) {
+
+    try {
+      Class<?> clazz = ReflectionUtil.createConnectionConfigClass(connectionType, serviceType);
+      Object newConnectionConfig = ClassConverterFactory.getConverter(clazz).convert(connectionConfig);
+      deleteSecrets(newConnectionConfig, buildSecretId(true, serviceType.value(), connectionName));
+
+    } catch (Exception e) {
+      String message = SecretsUtil.buildExceptionMessageConnection(e.getMessage(), connectionType, true);
+      if (message != null) {
+        throw new InvalidServiceConnectionException(message);
+      }
+      throw InvalidServiceConnectionException.byMessage(
+          connectionType, String.format("Failed to delete secrets from connection instance of %s", connectionType));
+    }
+  }
+
+  public void deleteSecretsFromWorkflow(Workflow workflow) {
+    Workflow workflowConverted = (Workflow) ClassConverterFactory.getConverter(Workflow.class).convert(workflow);
+    // we don't store OM conn sensitive data
+    workflowConverted.setOpenMetadataServerConnection(null);
+    try {
+      deleteSecrets(workflowConverted, buildSecretId(true, "workflow", workflow.getName()));
+    } catch (Exception e) {
+      throw new CustomExceptionMessage(
+          Response.Status.BAD_REQUEST,
+          String.format("Failed to delete secrets from workflow instance [%s]", workflow.getName()));
+    }
+  }
+
+  private void deleteSecrets(Object toDeleteSecretsFrom, String secretId) {
+    if (!DO_NOT_ENCRYPT_CLASSES.contains(toDeleteSecretsFrom.getClass())) {
+      Arrays.stream(toDeleteSecretsFrom.getClass().getMethods())
+          .filter(ReflectionUtil::isGetMethodOfObject)
+          .forEach(
+              method -> {
+                Object obj = ReflectionUtil.getObjectFromMethod(method, toDeleteSecretsFrom);
+                String fieldName = method.getName().replaceFirst("get", "");
+                // check if it has annotation:
+                // We are replicating the logic that we use for storing the fields we need to encrypt
+                // at encryptPasswordFields
+                if (obj != null && obj.getClass().getPackageName().startsWith("org.openmetadata")) {
+                  deleteSecrets(obj, buildSecretId(false, secretId, fieldName.toLowerCase(Locale.ROOT)));
+                } else if (obj != null && method.getAnnotation(PasswordField.class) != null) {
+                  deleteSecretInternal(buildSecretId(false, secretId, fieldName.toLowerCase(Locale.ROOT)));
+                }
+              });
+    }
+  }
 }

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

@@ -34,6 +34,17 @@ public class SecretsManagerFactory {
       case NOOP:
       case AWS_SSM:
       case AWS:
+        /*
+        We handle AWS and AWS_SSM as a NoopSecretsManager since we don't
+        need to WRITE any secrets. We will be just reading them out of the
+        AWS instance on the INGESTION side, but the server does not need
+        to do anything here.
+
+        If for example we want to set the AWS SSM (non-managed) we configure
+        the server as `secretsManager: aws-ssm` and set the Airflow env vars
+        to connect to AWS SSM as specified in the docs:
+        https://docs.open-metadata.org/v1.0.0/deployment/secrets-manager/supported-implementations/aws-ssm-parameter-store
+        */
         secretsManager = NoopSecretsManager.getInstance(clusterName, secretsManagerProvider);
         break;
       case MANAGED_AWS:

openmetadata-service/src/main/java/org/openmetadata/service/util/IngestionPipelineBuilder.java

@@ -27,7 +27,7 @@ public final class IngestionPipelineBuilder {
   }
 
   /** Build `IngestionPipeline` object with concrete class for the config which by definition it is a `Object`. */
-  public static IngestionPipeline addDefinedConfig(IngestionPipeline ingestionPipeline) {
+  public static void addDefinedConfig(IngestionPipeline ingestionPipeline) {
     if (DBT.equals(ingestionPipeline.getPipelineType()) && ingestionPipeline.getSourceConfig() != null) {
       ingestionPipeline
           .getSourceConfig()
@@ -41,6 +41,5 @@ public final class IngestionPipelineBuilder {
               ClassConverterFactory.getConverter(OpenMetadataConnection.class)
                   .convert(ingestionPipeline.getOpenMetadataServerConnection()));
     }
-    return ingestionPipeline;
   }
 }

openmetadata-service/src/test/java/org/openmetadata/service/secrets/SecretsManagerLifecycleTest.java

@@ -0,0 +1,125 @@
+package org.openmetadata.service.secrets;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.lenient;
+import static org.openmetadata.schema.api.services.CreateDatabaseService.DatabaseServiceType.Mysql;
+
+import java.util.Map;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mockito;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.openmetadata.schema.api.services.DatabaseConnection;
+import org.openmetadata.schema.entity.automations.TestServiceConnectionRequest;
+import org.openmetadata.schema.entity.automations.Workflow;
+import org.openmetadata.schema.entity.automations.WorkflowType;
+import org.openmetadata.schema.entity.services.ServiceType;
+import org.openmetadata.schema.services.connections.database.MysqlConnection;
+import org.openmetadata.service.exception.SecretsManagerException;
+import org.openmetadata.service.fernet.Fernet;
+
+@ExtendWith(MockitoExtension.class)
+public class SecretsManagerLifecycleTest {
+
+  private static final String ENCRYPTED_VALUE = "fernet:abcdef";
+  private static final String DECRYPTED_VALUE = "123456";
+
+  // We'll test the secret creation and deletion using the In Memory SM
+  private static InMemorySecretsManager secretsManager;
+
+  @BeforeAll
+  static void setUp() {
+    secretsManager = InMemorySecretsManager.getInstance("openmetadata");
+    Fernet fernet = Mockito.mock(Fernet.class);
+    lenient().when(fernet.decrypt(anyString())).thenReturn(DECRYPTED_VALUE);
+    lenient().when(fernet.decryptIfApplies(anyString())).thenReturn(DECRYPTED_VALUE);
+    lenient().when(fernet.encrypt(anyString())).thenReturn(ENCRYPTED_VALUE);
+    secretsManager.setFernet(fernet);
+  }
+
+  @Test
+  void testDatabaseServiceConnectionConfigLifecycle() {
+    String password = "openmetadata-test";
+    String secretName = "/openmetadata/database/test/password";
+    String connectionName = "test";
+    Map<String, String> mysqlConnection = Map.of("password", password);
+
+    // Ensure encrypted service connection config encrypts the password
+    MysqlConnection actualConnection =
+        (MysqlConnection)
+            secretsManager.encryptServiceConnectionConfig(
+                mysqlConnection, Mysql.value(), connectionName, ServiceType.DATABASE);
+    assertNotEquals(password, actualConnection.getPassword());
+
+    // Decrypt the encrypted password and validate
+    actualConnection =
+        (MysqlConnection)
+            secretsManager.decryptServiceConnectionConfig(actualConnection, Mysql.value(), ServiceType.DATABASE);
+    assertEquals(DECRYPTED_VALUE, actualConnection.getPassword());
+
+    // SM will have the key stored
+    String secretValue = secretsManager.getSecret(secretName);
+    assertEquals(secretValue, DECRYPTED_VALUE);
+
+    // Now we delete the service
+    secretsManager.deleteSecretsFromServiceConnectionConfig(
+        mysqlConnection, "Mysql", connectionName, ServiceType.DATABASE);
+
+    // We won't be able to get the key again
+    SecretsManagerException exception =
+        assertThrows(SecretsManagerException.class, () -> secretsManager.getSecret(secretName));
+
+    assertEquals(exception.getMessage(), String.format("Key [%s] not found in in-memory secrets manager", secretName));
+  }
+
+  @Test
+  void testWorkflowLifecycle() {
+    String password = "openmetadata_password";
+    String secretName = "/openmetadata/workflow/test-connection/request/connection/config/password";
+
+    Workflow workflow =
+        new Workflow()
+            .withName("test-connection")
+            .withWorkflowType(WorkflowType.TEST_CONNECTION)
+            .withRequest(
+                new TestServiceConnectionRequest()
+                    .withServiceType(ServiceType.DATABASE)
+                    .withConnectionType("Mysql")
+                    .withConnection(
+                        new DatabaseConnection()
+                            .withConfig(
+                                new MysqlConnection()
+                                    .withHostPort("mysql:3306")
+                                    .withUsername("openmetadata_user")
+                                    .withPassword(password))));
+
+    Workflow encrypted = secretsManager.encryptWorkflow(workflow);
+    TestServiceConnectionRequest encryptedRequest = (TestServiceConnectionRequest) encrypted.getRequest();
+    DatabaseConnection encryptedConnection = (DatabaseConnection) encryptedRequest.getConnection();
+    MysqlConnection encryptedConfig = (MysqlConnection) encryptedConnection.getConfig();
+    assertNotEquals(password, encryptedConfig.getPassword());
+
+    Workflow decrypted = secretsManager.decryptWorkflow(encrypted);
+    TestServiceConnectionRequest decryptedRequest = (TestServiceConnectionRequest) decrypted.getRequest();
+    DatabaseConnection decryptedConnection = (DatabaseConnection) decryptedRequest.getConnection();
+    MysqlConnection decryptedConfig = (MysqlConnection) decryptedConnection.getConfig();
+    assertEquals(DECRYPTED_VALUE, decryptedConfig.getPassword());
+
+    // SM will have the key stored
+    String secretValue = secretsManager.getSecret(secretName);
+    assertEquals(secretValue, DECRYPTED_VALUE);
+
+    // Now we delete the service
+    secretsManager.deleteSecretsFromWorkflow(workflow);
+
+    // We won't be able to get the key again
+    SecretsManagerException exception =
+        assertThrows(SecretsManagerException.class, () -> secretsManager.getSecret(secretName));
+
+    assertEquals(exception.getMessage(), String.format("Key [%s] not found in in-memory secrets manager", secretName));
+  }
+}