[HOPSWORKS-3109] remove aws_instance_role (logicalclocks#934) (logica…

…lclocks#1097)

(cherry picked from commit 2236d65)

hopsworks-IT/src/test/ruby/spec/helpers/storage_connector_helper.rb

@@ -131,8 +131,11 @@ def create_s3_connector(project_id, featurestore_id, encryption_algorithm: nil,
     end
 
     unless access_key.nil?
-      json_data["secretKey"] = access_key
-      json_data["accessKey"] = secret_key
+      json_data["accessKey"] = access_key
+    end
+
+    unless secret_key.nil?
+      json_data["secretKey"] = secret_key
     end
 
     json_result = post create_s3_connector_endpoint, json_data.to_json

hopsworks-IT/src/test/ruby/spec/redshift_storage_connector_spec.rb

@@ -104,9 +104,9 @@
                                   iamRole: "arn:aws:iam::123456789012:role/test-role-p1")
         expect_status_details(400)
       end
-      it "should fail to create with no iam and password" do
+      it "should create with no iam and password" do
         create_redshift_connector(@p1[:id], @featurestore1["featurestoreId"], databasePassword: nil, iamRole: nil)
-        expect_status_details(400)
+        expect_status_details(201)
       end
     end
   end
@@ -348,18 +348,18 @@
         expect_status_details(200)
         check_redshift_connector_update(json_body, connector)
       end
-      it "should fail to update with no password and iam role" do
+      it "should update with no password and iam role" do
         get_storage_connector(@p2[:id], @featurestore2["featurestoreId"], "redshift_connector_iam")
         connector = json_body
         copyConnector = connector.clone
         copyConnector[:databasePassword] = nil
         copyConnector[:iamRole] = nil
         update_redshift_connector(@p2[:id], @featurestore2["featurestoreId"], copyConnector[:name], copyConnector)
-        expect_status_details(400)
+        expect_status_details(200)
 
-        get_storage_connector(@p2[:id], @featurestore2["featurestoreId"], connector[:name])
+        get_storage_connector(@p2[:id], @featurestore2["featurestoreId"], copyConnector[:name])
         expect_status_details(200)
-        check_redshift_connector_update(json_body, connector)
+        check_redshift_connector_update(json_body, copyConnector)
       end
     end
   end
@@ -467,4 +467,4 @@
       end
     end
   end
-end
+end

hopsworks-IT/src/test/ruby/spec/storage_connector_spec.rb

@@ -23,7 +23,6 @@
       end
 
       after :each do
-        setVar("aws_instance_role", "false")
         create_session(@project[:username], "Pass123")
       end
 
@@ -80,20 +79,7 @@
         expect(parsed_json["bucket"] == "testbucket").to be true
       end
 
-      it "should not be able to add s3 connector without providing the access and secret key if the IAM Role is set to false" do
-        project = get_project
-        featurestore_id = get_featurestore_id(project.id)
-        json_result, _ = create_s3_connector(project.id, featurestore_id, bucket: "testbucket")
-        parsed_json = JSON.parse(json_result)
-        expect_status(400)
-        expect(parsed_json.key?("errorCode")).to be true
-        expect(parsed_json.key?("errorMsg")).to be true
-        expect(parsed_json.key?("usrMsg")).to be true
-        expect(parsed_json["errorCode"] == 270035 || parsed_json["errorCode"] == 270036).to be true
-      end
-
-      it "should be able to add s3 connector without providing the access and secret key if the IAM Role is set to true" do
-        setVar("aws_instance_role", "true")
+      it "should be able to add s3 connector without providing the access and secret key or iamRole" do
         project = get_project
         create_session(project[:username], "Pass123")
         featurestore_id = get_featurestore_id(project.id)
@@ -111,7 +97,6 @@
         expect(parsed_json["name"] == connector_name).to be true
         expect(parsed_json["storageConnectorType"] == "S3").to be true
         expect(parsed_json["bucket"] == "testbucket").to be true
-        setVar("aws_instance_role", "false")
         create_session(project[:username], "Pass123")
       end
 
@@ -248,7 +233,7 @@
         expect(parsed_json["errorCode"] == 270104).to be true
       end
 
-      it "should be not able to add s3 connector to the featurestore without access key" do
+      it "should not be able to add s3 connector to the featurestore with secret key and without access key" do
         project = get_project
         featurestore_id = get_featurestore_id(project.id)
         json_result, _ = create_s3_connector(project.id, featurestore_id, secret_key: "test", bucket: "testbucket")
@@ -259,6 +244,17 @@
         expect(parsed_json.key?("usrMsg")).to be true
       end
 
+      it "should not be able to add s3 connector to the featurestore with access key and without secret key" do
+        project = get_project
+        featurestore_id = get_featurestore_id(project.id)
+        json_result, _ = create_s3_connector(project.id, featurestore_id, access_key: "test", bucket: "testbucket")
+        parsed_json = JSON.parse(json_result)
+        expect_status(400)
+        expect(parsed_json.key?("errorCode")).to be true
+        expect(parsed_json.key?("errorMsg")).to be true
+        expect(parsed_json.key?("usrMsg")).to be true
+      end
+
       it "should not be able to add s3 connector to the featurestore without specifying a bucket" do
         project = get_project
         featurestore_id = get_featurestore_id(project.id)

hopsworks-api/src/main/java/io/hops/hopsworks/api/featurestore/FeaturestoreService.java

@@ -172,7 +172,6 @@ public Response getFeaturestore(
   public Response getFeaturestoreSettings(@Context SecurityContext sc) {
     FeaturestoreClientSettingsDTO featurestoreClientSettingsDTO = new FeaturestoreClientSettingsDTO();
     featurestoreClientSettingsDTO.setOnlineFeaturestoreEnabled(settings.isOnlineFeaturestore());
-    featurestoreClientSettingsDTO.setS3IAMRole(settings.isIAMRoleConfigured());
     GenericEntity<FeaturestoreClientSettingsDTO> featurestoreClientSettingsDTOGeneric =
       new GenericEntity<FeaturestoreClientSettingsDTO>(featurestoreClientSettingsDTO) {
     };

hopsworks-common/src/main/java/io/hops/hopsworks/common/featurestore/settings/FeaturestoreClientSettingsDTO.java

@@ -53,7 +53,6 @@ public class FeaturestoreClientSettingsDTO {
     FeaturestoreConstants.S3_STORAGE_CONNECTOR_SECRETKEY_MAX_LENGTH;
   private int s3StorageServerEncryptionKeyMaxLength =
     FeaturestoreConstants.S3_STORAGE_SERVER_ENCRYPTION_KEY_MAX_LENGTH;
-  private boolean s3IAMRole = false;
   private List<String> trainingDatasetDataFormats = FeaturestoreConstants.TRAINING_DATASET_DATA_FORMATS;
   private String jdbcConnectorType = FeaturestoreConstants.JDBC_CONNECTOR_TYPE;
   private String redshiftConnectorType = FeaturestoreConstants.REDSHIFT_CONNECTOR_TYPE;
@@ -153,15 +152,6 @@ public void setJdbcStorageConnectorArgumentsMaxLength(int jdbcStorageConnectorAr
     this.jdbcStorageConnectorArgumentsMaxLength = jdbcStorageConnectorArgumentsMaxLength;
   }
 
-  @XmlElement
-  public boolean isS3IAMRole() {
-    return s3IAMRole;
-  }
-
-  public void setS3IAMRole(boolean s3IAMRole) {
-    this.s3IAMRole = s3IAMRole;
-  }
-
   @XmlElement
   public int getS3StorageConnectorBucketMaxLength() {
     return s3StorageConnectorBucketMaxLength;

hopsworks-common/src/main/java/io/hops/hopsworks/common/featurestore/storageconnectors/redshift/FeaturestoreRedshiftConnectorController.java

@@ -46,7 +46,7 @@
 public class FeaturestoreRedshiftConnectorController {
 
   private static final Logger LOGGER = Logger.getLogger(FeaturestoreRedshiftConnectorController.class.getName());
-
+  
   @EJB
   private SecretsController secretsController;
   @EJB
@@ -66,7 +66,7 @@ public FeaturestoreRedshiftConnectorDTO getRedshiftConnectorDTO(FeaturestoreConn
       storageConnectorUtil.toOptions(featurestoreConnector.getRedshiftConnector().getArguments()));
     return featurestoreRedshiftConnectorDTO;
   }
-
+  
   /**
    *
    * @param featurestore
@@ -82,7 +82,7 @@ public FeatureStoreRedshiftConnector createFeaturestoreRedshiftConnector(Users u
     setPassword(user, featurestoreRedshiftConnectorDTO, featurestore, featurestoreRedshiftConnector);
     return featurestoreRedshiftConnector;
   }
-
+  
   private void setConnector(FeatureStoreRedshiftConnector featurestoreRedshiftConnector,
       FeaturestoreRedshiftConnectorDTO featurestoreRedshiftConnectorDTO) {
     featurestoreRedshiftConnector.setClusterIdentifier(
@@ -107,7 +107,7 @@ private void setConnector(FeatureStoreRedshiftConnector featurestoreRedshiftConn
       storageConnectorUtil.getValueOrNull(
         storageConnectorUtil.fromOptions(featurestoreRedshiftConnectorDTO.getArguments())));
   }
-
+  
   private void setPassword(Users user, FeaturestoreRedshiftConnectorDTO featurestoreRedshiftConnectorDTO,
     Featurestore featurestore, FeatureStoreRedshiftConnector featurestoreRedshiftConnector)
     throws UserException, ProjectException {
@@ -120,7 +120,7 @@ private void setPassword(Users user, FeaturestoreRedshiftConnectorDTO featuresto
       featurestoreRedshiftConnector.setSecret(secret);
     }
   }
-
+  
   private void verifyCreateDTO(FeaturestoreRedshiftConnectorDTO featurestoreRedshiftConnectorDTO)
       throws FeaturestoreException {
     if (featurestoreRedshiftConnectorDTO == null) {
@@ -170,24 +170,9 @@ private void verifyPassword(FeaturestoreRedshiftConnectorDTO featurestoreRedshif
     verifyPassword(featurestoreRedshiftConnectorDTO.getIamRole(),
         featurestoreRedshiftConnectorDTO.getDatabasePassword());
   }
-
+  
   private void verifyPassword(String iamRole, String password) throws FeaturestoreException {
-    boolean needPassword = !settings.isIAMRoleConfigured() && Strings.isNullOrEmpty(iamRole);
-    if (needPassword && Strings.isNullOrEmpty(password)) {
-      throw new FeaturestoreException(
-        RESTCodes.FeaturestoreErrorCode.ILLEGAL_STORAGE_CONNECTOR_ARG, Level.FINE, "Database password not set.");
-    } else if (!needPassword && !Strings.isNullOrEmpty(password)) {
-      throw new FeaturestoreException(
-        RESTCodes.FeaturestoreErrorCode.ILLEGAL_STORAGE_CONNECTOR_ARG, Level.FINE, "Database password is not allowed.");
-    }
-  }
-
-  private void verifyPassword(String iamRole, Secret secret) throws FeaturestoreException {
-    boolean needPassword = !settings.isIAMRoleConfigured() && Strings.isNullOrEmpty(iamRole);
-    if (needPassword && secret == null) {
-      throw new FeaturestoreException(
-        RESTCodes.FeaturestoreErrorCode.ILLEGAL_STORAGE_CONNECTOR_ARG, Level.FINE, "Database password not set.");
-    } else if (!needPassword && secret != null) {
+    if (!Strings.isNullOrEmpty(iamRole) && !Strings.isNullOrEmpty(password)) {
       throw new FeaturestoreException(
         RESTCodes.FeaturestoreErrorCode.ILLEGAL_STORAGE_CONNECTOR_ARG, Level.FINE, "Database password is not allowed.");
     }
@@ -212,15 +197,14 @@ public FeatureStoreRedshiftConnector updateFeaturestoreRedshiftConnector(Users u
 
     featureStoreRedshiftConnector.setArguments(
       storageConnectorUtil.fromOptions(featurestoreRedshiftConnectorDTO.getArguments()));
-
-    verifyPassword(featureStoreRedshiftConnector.getIamRole(), featureStoreRedshiftConnector.getSecret());
+
     if (featureStoreRedshiftConnector.getSecret() == null && secret != null) {
       secretsFacade.deleteSecret(secret.getId());
     }
 
     return featureStoreRedshiftConnector;
   }
-
+  
   private Secret updatePassword(Users user, FeaturestoreRedshiftConnectorDTO featurestoreRedshiftConnectorDTO,
     Featurestore featurestore, FeatureStoreRedshiftConnector featureStoreRedshiftConnector)
     throws UserException, ProjectException {

hopsworks-common/src/main/java/io/hops/hopsworks/common/featurestore/storageconnectors/s3/FeaturestoreS3ConnectorController.java

@@ -116,14 +116,8 @@ public FeaturestoreS3Connector updateFeaturestoreS3Connector(Users user, Feature
       featurestoreS3Connector.getIamRole(), featurestoreS3ConnectorDTO.getIamRole())) {
       featurestoreS3Connector.setIamRole(featurestoreS3ConnectorDTO.getIamRole());
     }
-
-    Secret secret = null;
-    FeaturestoreS3ConnectorAccessAndSecretKey keys = storageConnectorUtil.getSecret(
-      featurestoreS3Connector.getSecret(), FeaturestoreS3ConnectorAccessAndSecretKey.class);
-    if (storageConnectorUtil.shouldUpdate(keys.getAccessKey(), featurestoreS3ConnectorDTO.getAccessKey()) ||
-      storageConnectorUtil.shouldUpdate(keys.getSecretKey(), featurestoreS3ConnectorDTO.getSecretKey())) {
-      secret = updateSecret(user, featurestoreS3ConnectorDTO, featurestore, featurestoreS3Connector);
-    }
+
+    Secret secret = updateSecret(user, featurestoreS3ConnectorDTO, featurestore, featurestoreS3Connector);
 
     String currentEncryptionAlgorithm = featurestoreS3Connector.getServerEncryptionAlgorithm() != null ?
             featurestoreS3Connector.getServerEncryptionAlgorithm().getAlgorithm() : null;
@@ -160,16 +154,12 @@ public FeaturestoreS3Connector updateFeaturestoreS3Connector(Users user, Feature
   }
 
   private void verifyKeyAndIAMRole(String iamRole, Secret secret) throws FeaturestoreException {
-    boolean needPassword = !settings.isIAMRoleConfigured() && Strings.isNullOrEmpty(iamRole);
-    if (needPassword && secret == null) {
-      throw new FeaturestoreException(
-        RESTCodes.FeaturestoreErrorCode.ILLEGAL_STORAGE_CONNECTOR_ARG, Level.FINE, "S3 Access Keys are not set.");
-    } else if (!needPassword && secret != null) {
+    if (!Strings.isNullOrEmpty(iamRole) && secret != null) {
       throw new FeaturestoreException(
         RESTCodes.FeaturestoreErrorCode.ILLEGAL_STORAGE_CONNECTOR_ARG, Level.FINE, "S3 Access Keys are not allowed");
     }
   }
-
+  
   private Secret updateSecret(Users user, FeaturestoreS3ConnectorDTO featurestoreS3ConnectorDTO,
     Featurestore featurestore, FeaturestoreS3Connector featurestoreS3Connector) throws UserException,
     FeaturestoreException, ProjectException {
@@ -197,9 +187,9 @@ private Secret updateSecret(Users user, FeaturestoreS3ConnectorDTO featurestoreS
       //Secret can't be removed here b/c of ON DELETE RESTRICT
     }
     return secret;
-
+    
   }
-
+  
   private boolean keysNotNullOrEmpty(FeaturestoreS3ConnectorDTO featurestoreS3ConnectorDTO) {
     return !Strings.isNullOrEmpty(featurestoreS3ConnectorDTO.getSecretKey()) &&
       !Strings.isNullOrEmpty(featurestoreS3ConnectorDTO.getAccessKey());
@@ -287,10 +277,12 @@ private void verifyUserInput(FeaturestoreS3ConnectorDTO featurestoreS3ConnectorD
       throw new IllegalArgumentException("Null input data");
     }
     verifyS3ConnectorBucket(featurestoreS3ConnectorDTO.getBucket());
-    
-    if (settings.isIAMRoleConfigured() || !Strings.isNullOrEmpty(featurestoreS3ConnectorDTO.getIamRole())) {
+
+    if (!Strings.isNullOrEmpty(featurestoreS3ConnectorDTO.getIamRole())) {
       verifySecretAndAccessKeysForIamRole(featurestoreS3ConnectorDTO);
-    } else {
+    }
+    if (!Strings.isNullOrEmpty(featurestoreS3ConnectorDTO.getAccessKey()) ||
+      !Strings.isNullOrEmpty(featurestoreS3ConnectorDTO.getSecretKey())) {
       verifyS3ConnectorAccessKey(featurestoreS3ConnectorDTO.getAccessKey());
       verifyS3ConnectorSecretKey(featurestoreS3ConnectorDTO.getSecretKey());
     }
@@ -316,16 +308,11 @@ private void verifyUserInput(FeaturestoreS3ConnectorDTO featurestoreS3ConnectorD
    */
   private void verifySecretAndAccessKeysForIamRole(FeaturestoreS3ConnectorDTO featurestoreS3ConnectorDTO)
     throws FeaturestoreException{
-    if ((settings.isIAMRoleConfigured() || !Strings.isNullOrEmpty(featurestoreS3ConnectorDTO.getIamRole())) &&
-      (!Strings.isNullOrEmpty(featurestoreS3ConnectorDTO.getAccessKey()) ||
-      !Strings.isNullOrEmpty(featurestoreS3ConnectorDTO.getSecretKey()))) {
+    if (!Strings.isNullOrEmpty(featurestoreS3ConnectorDTO.getAccessKey()) ||
+      !Strings.isNullOrEmpty(featurestoreS3ConnectorDTO.getSecretKey())) {
       throw new FeaturestoreException(RESTCodes.FeaturestoreErrorCode.S3_KEYS_FORBIDDEN, Level.FINE,
         "S3 Access Keys are not allowed");
     }
-    if (!settings.isIAMRoleConfigured() && Strings.isNullOrEmpty(featurestoreS3ConnectorDTO.getIamRole())) {
-      throw new FeaturestoreException(RESTCodes.FeaturestoreErrorCode.ILLEGAL_STORAGE_CONNECTOR_ARG, Level.FINE,
-        "S3 IAM role not set.");
-    }
   }
 
   private FeaturestoreS3ConnectorEncryptionAlgorithm getEncryptionAlgorithm(String s) throws FeaturestoreException {