diff --git a/hopsworks-IT/src/test/ruby/spec/gcs_storage_connector_spec.rb b/hopsworks-IT/src/test/ruby/spec/gcs_storage_connector_spec.rb index 7a9c072708..b04acb015e 100644 --- a/hopsworks-IT/src/test/ruby/spec/gcs_storage_connector_spec.rb +++ b/hopsworks-IT/src/test/ruby/spec/gcs_storage_connector_spec.rb @@ -122,10 +122,9 @@ expect(parsed_result_update['bucket']).to eql(bucket) end - it "should update only encryption secret of gcs connector with same name" do + it "should update encryption secret of gcs connector" do project = get_project featurestore_id = get_featurestore_id(project.id) - key_path = "/Projects/#{@project['projectname']}/Resources/sampleKey.json" bucket = 'testbucket' encryption_key = "yFXZv3V+KT++v8/L6HcRqjVPAotNO3beIzcetgTBoSM=" @@ -135,7 +134,11 @@ encryption_key: encryption_key, encryption_key_hash: encryption_hash) expect_status_details(201) - + # check secret created for connector + secret_name = "gcs_"+connector_name+"_"+featurestore_id.to_s + get_private_secret(secret_name) + expect_status_details(200) + # updates description = "test gcs updated" encryption_key = "new key" encryption_hash = "new hash" @@ -154,6 +157,15 @@ update_result = update_gcs_connector_json(project.id, featurestore_id, connector_name, additional_data ) parsed_result_update = JSON.parse(update_result) expect_status_details(200) + # check updated secret created for connector + result = get_private_secret(secret_name) + expect_status_details(200) + updated_secret = JSON.parse(result) + updated_secret_value = JSON.parse(updated_secret["items"][0]["secret"]) + # assert secrets data + expect(updated_secret_value["encryptionKey"]).to eql(encryption_key) + expect(updated_secret_value["encryptionKeyHash"]). to eql(encryption_hash) + # assert connectors data expect(parsed_result_update.key?("id")).to be true expect(parsed_result_update["name"]).to eql(connector_name) expect(parsed_result_update["storageConnectorType"]).to eql("GCS") @@ -223,7 +235,7 @@ expect(parsed_result["name"]).to eql(connector_name) end - it "should update connector with encryption to without encryption" do + it "should update connector with encryption to without encryption and delete secret" do project = get_project featurestore_id = get_featurestore_id(project.id) key_path = "/Projects/#{@project['projectname']}/Resources/sampleKey.json" @@ -234,6 +246,11 @@ _, connector_name = create_gcs_connector(project.id, featurestore_id, key_path, bucket, algorithm:algorithm, encryption_key:encryption_key, encryption_key_hash:encryption_hash ) expect_status_details(201) + # check secret created for connector + secret_name = "gcs_"+connector_name+"_"+featurestore_id.to_s + get_private_secret(secret_name) + expect_status_details(200) + # updates description = "test gcs updated" key_path = "/Projects/#{@project['projectname']}/Resources/newSampleKey.json" additional_data = { @@ -247,6 +264,9 @@ update_result = update_gcs_connector_json(project.id, featurestore_id, connector_name, additional_data ) parsed_result_update = JSON.parse(update_result) expect_status_details(200) + get_private_secret(secret_name) + # secret should get deleted after update + expect_status_details(404) expect(parsed_result_update.key?("id")).to be true expect(parsed_result_update["name"]).to eql(connector_name) expect(parsed_result_update["storageConnectorType"]).to eql("GCS") @@ -266,10 +286,8 @@ bucket = 'testbucket' _, connector_name = create_gcs_connector(project.id, featurestore_id, key_path, bucket) expect_status_details(201) - delete_connector(project.id, featurestore_id, connector_name) expect_status_details(200) expect(test_file(key_path)).to be false end - end diff --git a/hopsworks-common/src/main/java/io/hops/hopsworks/common/featurestore/storageconnectors/gcs/FeatureStoreGcsConnectorController.java b/hopsworks-common/src/main/java/io/hops/hopsworks/common/featurestore/storageconnectors/gcs/FeatureStoreGcsConnectorController.java index cb49e7d8da..dcb5d82706 100644 --- a/hopsworks-common/src/main/java/io/hops/hopsworks/common/featurestore/storageconnectors/gcs/FeatureStoreGcsConnectorController.java +++ b/hopsworks-common/src/main/java/io/hops/hopsworks/common/featurestore/storageconnectors/gcs/FeatureStoreGcsConnectorController.java @@ -17,6 +17,7 @@ package io.hops.hopsworks.common.featurestore.storageconnectors.gcs; import com.google.common.base.Strings; +import io.hops.hopsworks.common.dao.user.security.secrets.SecretsFacade; import io.hops.hopsworks.common.featurestore.storageconnectors.StorageConnectorUtil; import io.hops.hopsworks.exceptions.FeaturestoreException; import io.hops.hopsworks.exceptions.ProjectException; @@ -26,6 +27,7 @@ import io.hops.hopsworks.persistence.entity.featurestore.storageconnector.gcs.FeatureStoreGcsConnector; import io.hops.hopsworks.persistence.entity.project.Project; import io.hops.hopsworks.persistence.entity.user.Users; +import io.hops.hopsworks.persistence.entity.user.security.secrets.Secret; import io.hops.hopsworks.restutils.RESTCodes; import javax.ejb.EJB; @@ -40,11 +42,11 @@ @Stateless @TransactionAttribute(TransactionAttributeType.NEVER) public class FeatureStoreGcsConnectorController { - private static final Logger LOGGER = Logger.getLogger(FeatureStoreGcsConnectorController.class.getName()); - @EJB private StorageConnectorUtil storageConnectorUtil; + @EJB + private SecretsFacade secretsFacade; public FeatureStoreGcsConnectorController() {} @@ -71,20 +73,11 @@ public FeatureStoreGcsConnector createConnector(Project project, Users user, Fea FeatureStoreGcsConnectorDTO gcsConnectorDTO) throws FeaturestoreException, ProjectException, UserException { validateInput(project, user, gcsConnectorDTO); - FeatureStoreGcsConnector gcsConnector = new FeatureStoreGcsConnector(); gcsConnector.setKeyPath(gcsConnectorDTO.getKeyPath()); gcsConnector.setAlgorithm(gcsConnectorDTO.getAlgorithm()); gcsConnector.setBucket(gcsConnectorDTO.getBucket()); - - if (gcsConnectorDTO.getAlgorithm() != null) { - String secretName = storageConnectorUtil.createSecretName(featureStore.getId(), gcsConnectorDTO.getName(), - gcsConnectorDTO.getStorageConnectorType()); - EncryptionSecrets secretsClass = new EncryptionSecrets(gcsConnectorDTO.getEncryptionKey(), - gcsConnectorDTO.getEncryptionKeyHash()); - gcsConnector.setEncryptionSecret(storageConnectorUtil.createProjectSecret(user, secretName, featureStore, - secretsClass)); - } + updateSecret(user, gcsConnectorDTO, featureStore, gcsConnector); return gcsConnector; } @@ -99,25 +92,41 @@ public FeatureStoreGcsConnector updateConnector(Project project, Users user, Fea gcsConnector.setKeyPath(gcsConnectorDTO.getKeyPath()); gcsConnector.setAlgorithm(gcsConnectorDTO.getAlgorithm()); gcsConnector.setBucket(gcsConnectorDTO.getBucket()); + updateSecret(user, gcsConnectorDTO, featureStore, gcsConnector); + return gcsConnector; + } + private void updateSecret(Users user, FeatureStoreGcsConnectorDTO gcsConnectorDTO, + Featurestore featureStore, FeatureStoreGcsConnector gcsConnector) throws UserException, + ProjectException { + Secret secret = gcsConnector.getEncryptionSecret(); if (gcsConnectorDTO.getAlgorithm() != null) { - String secretName = storageConnectorUtil.createSecretName(featureStore.getId(), gcsConnectorDTO.getName(), - gcsConnectorDTO.getStorageConnectorType()); + String secretName; + if (secret != null) { // update existing secret + secretName = secret.getId().getName(); + } else { // create new secret + secretName = storageConnectorUtil.createSecretName(featureStore.getId(), gcsConnectorDTO.getName(), + gcsConnectorDTO.getStorageConnectorType()); + } EncryptionSecrets secretsClass = new EncryptionSecrets(gcsConnectorDTO.getEncryptionKey(), - gcsConnectorDTO.getEncryptionKeyHash()); + gcsConnectorDTO.getEncryptionKeyHash()); + // update secret gcsConnector.setEncryptionSecret( - storageConnectorUtil.updateProjectSecret(user, gcsConnector.getEncryptionSecret(), - secretName, featureStore, secretsClass)); - } else { + storageConnectorUtil.updateProjectSecret(user, gcsConnector.getEncryptionSecret(), + secretName , featureStore, secretsClass) + ); + } else { // set secrets to null gcsConnector.setEncryptionSecret(null); } - return gcsConnector; + if (gcsConnector.getEncryptionSecret()==null && secret!=null) { + // delete old secret + secretsFacade.deleteSecret(secret.getId()); + } } - + public void validateInput(Project project, Users user, FeatureStoreGcsConnectorDTO gcsConnectorDTO) throws FeaturestoreException { storageConnectorUtil.validatePath(project, user, gcsConnectorDTO.getKeyPath(), "Key file path"); - if (Strings.isNullOrEmpty(gcsConnectorDTO.getKeyPath())) { throw new FeaturestoreException(RESTCodes.FeaturestoreErrorCode.GCS_FIELD_MISSING, Level.FINE, "Key File Path cannot be empty"); @@ -126,7 +135,6 @@ public void validateInput(Project project, Users user, FeatureStoreGcsConnectorD throw new FeaturestoreException(RESTCodes.FeaturestoreErrorCode.GCS_FIELD_MISSING, Level.FINE, "Bucket cannot be empty"); } - // either none of the three should be set or all if (!Strings.isNullOrEmpty(gcsConnectorDTO.getEncryptionKey()) || !Strings.isNullOrEmpty(gcsConnectorDTO.getEncryptionKeyHash())