Skip to content

Commit

Permalink
[FSTORE-989] fix update issues for encryption fields (#1622)
Browse files Browse the repository at this point in the history 
Co-authored-by: Ralf <[email protected]>
  • Loading branch information
@dhananjay-mk @bubriks @SirOibaf
2 people authored and SirOibaf committed Jan 5, 2024
1 parent 620399a commit 8112cab
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 28 deletions.
30 changes: 24 additions & 6 deletions hopsworks-IT/src/test/ruby/spec/gcs_storage_connector_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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="
Expand All @@ -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"
Expand All @@ -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")
Expand Down Expand Up @@ -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"
Expand All @@ -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 = {
Expand All @@ -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")
Expand All @@ -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
52 changes: 30 additions & 22 deletions ...psworks/common/featurestore/storageconnectors/gcs/FeatureStoreGcsConnectorController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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() {}

Expand All @@ -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;
}

Expand All @@ -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");
Expand All @@ -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())
Expand Down

0 comments on commit 8112cab

Please sign in to comment.