[FSTORE-1096] store password as secrets for JDBC connector (#1717) (#…

…1503) * Update hopsworks-IT/src/test/ruby/spec/storage_connector_spec.rb Co-authored-by: Ralf <[email protected]> * Update hopsworks-common/src/main/java/io/hops/hopsworks/common/featurestore/storageconnectors/jdbc/FeaturestoreJdbcConnectorController.java Co-authored-by: Ralf <[email protected]> * refactor create and update methods * refactor replace password methods to StorageConnectorUtil and add unit tests * minor refactoring * minor refactoring * minor cleanup * minor cleanup --------- Co-authored-by: Ralf <[email protected]> (cherry picked from commit 6d1507c)
logicalclocks · Mar 8, 2024 · 40bd0e3 · 40bd0e3
1 parent 40d54a8
commit 40bd0e3
Show file tree

Hide file tree

Showing 7 changed files with 379 additions and 58 deletions.
diff --git a/hopsworks-IT/src/test/ruby/spec/helpers/storage_connector_helper.rb b/hopsworks-IT/src/test/ruby/spec/helpers/storage_connector_helper.rb
@@ -82,7 +82,7 @@ def update_hopsfs_connector(project_id, featurestore_id, connector_name, dataset
     [json_result, connector_name]
   end
 
-  def create_jdbc_connector(project_id, featurestore_id, connectionString: "jdbc://test")
+  def create_jdbc_connector(project_id, featurestore_id, connectionString: "jdbc://test", arguments:[{name: "test1", value: "test2"}])
     type = "featurestoreJdbcConnectorDTO"
     storageConnectorType = "JDBC"
     create_jdbc_connector_endpoint = "#{ENV['HOPSWORKS_API']}/project/#{project_id}/featurestores/#{featurestore_id}/storageconnectors"
@@ -93,14 +93,14 @@ def create_jdbc_connector(project_id, featurestore_id, connectionString: "jdbc:/
         type: type,
         storageConnectorType: storageConnectorType,
         connectionString: connectionString,
-        arguments: [{name: "test1", value: "test2"}]
+        arguments: arguments
     }
     json_data = json_data.to_json
     json_result = post create_jdbc_connector_endpoint, json_data
     return json_result, jdbc_connector_name
   end
 
-  def update_jdbc_connector(project_id, featurestore_id, connector_name, connectionString: "jdbc://test")
+  def update_jdbc_connector(project_id, featurestore_id, connector_name, connectionString: "jdbc://test", arguments:[{name: "test1", value: "test2"}])
     type = "featurestoreJdbcConnectorDTO"
     storageConnectorType = "JDBC"
     update_jdbc_connector_endpoint = "#{ENV['HOPSWORKS_API']}/project/#{project_id}/featurestores/#{featurestore_id}/storageconnectors/#{connector_name}"
@@ -110,7 +110,7 @@ def update_jdbc_connector(project_id, featurestore_id, connector_name, connectio
         type: type,
         storageConnectorType: storageConnectorType,
         connectionString: connectionString,
-        arguments: [{name: "test1", value: "test2"}]
+        arguments: arguments
     }
     put update_jdbc_connector_endpoint, json_data.to_json
   end

diff --git a/hopsworks-IT/src/test/ruby/spec/storage_connector_spec.rb b/hopsworks-IT/src/test/ruby/spec/storage_connector_spec.rb
@@ -367,7 +367,8 @@
         it "should be able to add jdbc connector to the featurestore" do
           project = get_project
           featurestore_id = get_featurestore_id(project.id)
-          json_result, connector_name = create_jdbc_connector(project.id, featurestore_id, connectionString: "jdbc://test2")
+          json_result, connector_name = create_jdbc_connector(project.id, featurestore_id, connectionString:
+            "jdbc:mysql://localhost:3306/test?user=root&password=123pass")
           parsed_json = JSON.parse(json_result)
           expect_status_details(201)
           expect(parsed_json.key?("id")).to be true
@@ -379,7 +380,13 @@
           expect(parsed_json.key?("arguments")).to be true
           expect(parsed_json["name"] == connector_name).to be true
           expect(parsed_json["storageConnectorType"] == "JDBC").to be true
-          expect(parsed_json["connectionString"] == "jdbc://test2").to be true
+          expect(parsed_json["connectionString"] == "jdbc:mysql://localhost:3306/test?user=root&password=123pass").to be true
+          # expect secrets to be created
+          secret_name = "jdbc_"+connector_name+"_"+featurestore_id.to_s
+          result = get_private_secret(secret_name)
+          expect_status_details(200)
+          secrets_json = JSON.parse(result)
+          expect(secrets_json["items"][0]["secret"]).to eql("123pass")
         end
 
         it "should not be able to add jdbc connector without a connection string to the featurestore" do
@@ -394,6 +401,25 @@
           expect(parsed_json["errorCode"] == 270032).to be true
         end
 
+        it "should create secrets for password in arguments for jdbc connector" do
+          project = get_project
+          featurestore_id = get_featurestore_id(project.id)
+          json_result, connector_name = create_jdbc_connector(project.id, featurestore_id,
+                                                              connectionString: "jdbc:mysql://localhost:3306/test",
+                                                              arguments: [{name: "password", value: "123pass"},
+                                                                          {name: "user", value: "username"}])
+          # expect secrets created for password
+          parsed_json = JSON.parse(json_result)
+          expect_status_details(201)
+          expect(parsed_json.key?("arguments")).to be true
+          # expect secrets to be created
+          secret_name = "jdbc_"+connector_name+"_"+featurestore_id.to_s
+          result = get_private_secret(secret_name)
+          expect_status_details(200)
+          secrets_json = JSON.parse(result)
+          expect(secrets_json["items"][0]["secret"]).to eql("123pass")
+        end
+
         it "should be able to delete a hopsfs connector from the featurestore" do
           project = get_project
           featurestore_id = get_featurestore_id(project.id)
@@ -431,6 +457,9 @@
           delete_connector_endpoint = "#{ENV['HOPSWORKS_API']}/project/#{project.id}/featurestores/#{featurestore_id}/storageconnectors/#{connector_name}"
           delete delete_connector_endpoint
           expect_status_details(200)
+          get_private_secret("jdbc_"+connector_name+"_"+featurestore_id.to_s)
+          # secret should get deleted after update
+          expect_status_details(404)
         end
 
         it "should be able to update hopsfs connector in the featurestore" do
@@ -519,9 +548,13 @@
           project = get_project
           create_session(project[:username], "Pass123")
           featurestore_id = get_featurestore_id(project.id)
-          json_result1, connector_name = create_jdbc_connector(project.id, featurestore_id, connectionString: "jdbc://test2")
+          json_result1, connector_name = create_jdbc_connector(project.id, featurestore_id, connectionString:
+            "jdbc://test2",arguments: [{name: "password", value: "123pass"},
+                                       {name: "user", value: "username"}])
           expect_status_details(201)
-          json_result2 = update_jdbc_connector(project.id, featurestore_id, connector_name, connectionString: "jdbc://test3")
+          json_result2 = update_jdbc_connector(project.id, featurestore_id, connector_name,
+                                               connectionString: "jdbc://test3",
+                                               arguments: [{name: "password", value: "123pass_new"}])
           parsed_json2 = JSON.parse(json_result2)
           expect(parsed_json2.key?("id")).to be true
           expect(parsed_json2.key?("name")).to be true
@@ -533,6 +566,12 @@
           expect(parsed_json2["name"] == connector_name).to be true
           expect(parsed_json2["storageConnectorType"] == "JDBC").to be true
           expect(parsed_json2["connectionString"] == "jdbc://test3").to be true
+          # expect secrets to be updated
+          secret_name = "jdbc_"+connector_name+"_"+featurestore_id.to_s
+          result = get_private_secret(secret_name)
+          expect_status_details(200)
+          secrets_json = JSON.parse(result)
+          expect(secrets_json["items"][0]["secret"]).to eql("123pass_new")
         end
       end
 

diff --git a/...psworks/common/featurestore/storageconnectors/FeaturestoreStorageConnectorController.java b/...psworks/common/featurestore/storageconnectors/FeaturestoreStorageConnectorController.java
@@ -277,7 +277,7 @@ public FeaturestoreStorageConnectorDTO createStorageConnector(Users user, Projec
       case JDBC:
         featurestoreConnector.setConnectorType(FeaturestoreConnectorType.JDBC);
         featurestoreConnector.setJdbcConnector(jdbcConnectorController.createFeaturestoreJdbcConnector(
-            (FeaturestoreJdbcConnectorDTO) featurestoreStorageConnectorDTO));
+          user, featurestore, (FeaturestoreJdbcConnectorDTO) featurestoreStorageConnectorDTO));
         break;
       case REDSHIFT:
         featurestoreConnector.setConnectorType(FeaturestoreConnectorType.REDSHIFT);
@@ -365,7 +365,8 @@ public void updateStorageConnector(Users user, Project project, Featurestore fea
         break;
       case JDBC:
         featurestoreConnector.setJdbcConnector(jdbcConnectorController.updateFeaturestoreJdbcConnector(
-            (FeaturestoreJdbcConnectorDTO) featurestoreStorageConnectorDTO, featurestoreConnector.getJdbcConnector()));
+          user, featurestore, (FeaturestoreJdbcConnectorDTO) featurestoreStorageConnectorDTO,
+          featurestoreConnector.getJdbcConnector()));
         break;
       case REDSHIFT:
         featurestoreConnector.setRedshiftConnector(redshiftConnectorController.updateFeaturestoreRedshiftConnector(

diff --git a/...in/java/io/hops/hopsworks/common/featurestore/storageconnectors/StorageConnectorUtil.java b/...in/java/io/hops/hopsworks/common/featurestore/storageconnectors/StorageConnectorUtil.java
@@ -22,6 +22,7 @@
 import com.google.common.base.Strings;
 import io.hops.hopsworks.common.dao.user.UserFacade;
 import io.hops.hopsworks.common.dao.user.security.secrets.SecretPlaintext;
+import io.hops.hopsworks.common.featurestore.FeaturestoreConstants;
 import io.hops.hopsworks.common.featurestore.OptionDTO;
 import io.hops.hopsworks.common.hdfs.DistributedFileSystemOps;
 import io.hops.hopsworks.common.hdfs.DistributedFsService;
@@ -44,6 +45,7 @@
 import javax.ejb.EJB;
 import javax.ejb.Stateless;
 import java.io.IOException;
+import java.net.URI;
 import java.security.GeneralSecurityException;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -89,7 +91,7 @@ public StorageConnectorUtil(Settings settings, DistributedFsService dfs) {
    */
   public String createSecretName(Integer featurestoreId, String connectorName,
                                  FeaturestoreConnectorType connectorType) {
-    return connectorType.toString().toLowerCase() + "_" + connectorName.replaceAll(" ", "_").toLowerCase() + "_" +
+    return connectorType.toString().toLowerCase() + "_" + connectorName.replace(" ", "_").toLowerCase() + "_" +
       featurestoreId;
   }
 
@@ -279,4 +281,91 @@ public boolean isStorageConnectorTypeEnabled(FeaturestoreConnectorType storageCo
         throw new IllegalArgumentException("Unknown storage connector type: " + storageConnectorType);
     }
   }
+
+  /**
+   * Replace the password placeholder with the actual password
+   * @param connectionUrl
+   * @param connectorPassword
+   * @return Sting with the password replaced
+   */
+  public String replaceToPlainText(String connectionUrl, String connectorPassword) {
+    if (!Strings.isNullOrEmpty(connectionUrl) && !Strings.isNullOrEmpty(connectorPassword)) {
+      connectionUrl = connectionUrl.replace(FeaturestoreConstants.ONLINE_FEATURE_STORE_CONNECTOR_PASSWORD_TEMPLATE,
+        connectorPassword);
+    }
+    return connectionUrl;
+  }
+
+  /**
+   * Replace the password placeholder with the actual password
+   * @param arguments
+   * @param connectorPassword
+   * @return List of OptionDTO with the password replaced
+   */
+  public List<OptionDTO> replaceToPlainText(List<OptionDTO> arguments, String connectorPassword) {
+    if (!Strings.isNullOrEmpty(fromOptions(arguments))
+      && !Strings.isNullOrEmpty(connectorPassword)) {
+      arguments.forEach(argument -> {
+        if (argument.getValue().equals(FeaturestoreConstants.ONLINE_FEATURE_STORE_CONNECTOR_PASSWORD_TEMPLATE)) {
+          argument.setValue(connectorPassword);
+        }
+      });
+    }
+    return arguments;
+  }
+
+  /**
+   * Replace the password with the password placeholder
+   * @param connectionString
+   * @param password
+   * @return String with the password replaced
+   */
+  public String replaceToPasswordTemplate(String connectionString, String password) {
+    // if the connection string contains a password, replace it with the password placeholder
+    if (Strings.isNullOrEmpty(password)) {
+      return connectionString;
+    }
+    return connectionString.replace(password,
+      FeaturestoreConstants.ONLINE_FEATURE_STORE_CONNECTOR_PASSWORD_TEMPLATE);
+  }
+
+  /**
+   * Replace the password with the password placeholder
+   * @param arguments
+   * @return List of OptionDTO with the password replaced
+   */
+  public String replaceToPasswordTemplate(List<OptionDTO> arguments) {
+    // check if arguments not null
+    if (arguments == null) {
+      return null;
+    }
+    arguments.forEach(argument -> {
+      if (argument.getName().equals(FeaturestoreConstants.ONLINE_FEATURE_STORE_JDBC_PASSWORD_ARG)) {
+        argument.setValue(FeaturestoreConstants.ONLINE_FEATURE_STORE_CONNECTOR_PASSWORD_TEMPLATE);
+      }
+    });
+
+    return fromOptions(arguments);
+  }
+  /**
+   * Fetch the password from the jdbc url
+   * @param query
+   * @return String password
+   */
+  public String fetchPasswordFromJdbcUrl(String connectionUrl) {
+    // parse JDBC URL in connectionUrl and get property value for password
+    URI uri = URI.create(connectionUrl.substring(5));
+    String query = uri.getQuery();
+    String password = null;
+    if (!Strings.isNullOrEmpty(query)) {
+      String[] queryArgs = query.split("&");
+      for (String queryArg : queryArgs) {
+        String[] queryArgParts = queryArg.split("=");
+        if (queryArgParts[0].equals(FeaturestoreConstants.ONLINE_FEATURE_STORE_JDBC_PASSWORD_ARG)) {
+          password = queryArgParts[1];
+        }
+      }
+    }
+    return password;
+  }
 }