[FSTORE-368] Remove code for secret store / parameter store from the hsfs APIs (logicalclocks#248)

* Remove code for secret store / parameter store from the hsfs APIs

* Fix locust_benchmark

* Remove unused constants from client.external

* Remove _get_secret from client.external

Author: aversey

locust_benchmark/common/hopsworks_client.py

@@ -26,7 +26,6 @@ def __init__(self, environment=None):
             host=self.hopsworks_config.get("host", "localhost"),
             port=self.hopsworks_config.get("port", 443),
             api_key_file=".api_key",
-            secrets_store="local",
             engine="python",
         )
         self.fs = self.connection.get_feature_store()

python/hopsworks/connection.py

@@ -245,8 +245,6 @@ def connect(self):
                     self._port,
                     self._project,
                     None,
-                    None,
-                    None,
                     self._hostname_verification,
                     self._trust_store_path,
                     self._cert_folder,

python/hopsworks_common/client/__init__.py

@@ -31,8 +31,6 @@ def init(
     port: Optional[int] = None,
     project: Optional[str] = None,
     engine: Optional[str] = None,
-    region_name: Optional[str] = None,
-    secrets_store=None,
     hostname_verification: Optional[bool] = None,
     trust_store_path: Optional[str] = None,
     cert_folder: Optional[str] = None,
@@ -49,16 +47,14 @@ def init(
                 port,
                 project,
                 engine,
-                region_name,
-                secrets_store,
                 hostname_verification,
                 trust_store_path,
                 cert_folder,
                 api_key_file,
                 api_key_value,
             )
     elif isinstance(_client, external.Client) and not _client._project_name:
-        _client._hsfs_post_init(project, engine, region_name)
+        _client._hsfs_post_init(project, engine)
 
 
 def get_instance() -> Union[hopsworks.Client, external.Client]:

python/hopsworks_common/client/base.py

@@ -54,8 +54,8 @@ def _get_verify(self, verify, trust_store_path):
 
         Credit to https://gist.github.com/gdamjan/55a8b9eec6cf7b771f92021d93b87b2c
 
-        :param verify: perform hostname verification, 'true' or 'false'
-        :type verify: str
+        :param verify: perform hostname verification
+        :type verify: bool
         :param trust_store_path: path of the truststore locally if it was uploaded manually to
             the external environment such as AWS Sagemaker
         :type trust_store_path: str
@@ -64,7 +64,7 @@ def _get_verify(self, verify, trust_store_path):
                  if verify is false, then return false
         :rtype: str or boolean
         """
-        if verify == "true":
+        if verify:
             if trust_store_path is not None:
                 return trust_store_path
             else:

python/hopsworks_common/client/external.py

@@ -17,11 +17,9 @@
 from __future__ import annotations
 
 import base64
-import json
 import logging
 import os
 
-import boto3
 import requests
 from hopsworks_common.client import auth, base, exceptions
 from hopsworks_common.client.exceptions import FeatureStoreException
@@ -37,19 +35,12 @@
 
 
 class Client(base.Client):
-    DEFAULT_REGION = "default"
-    SECRETS_MANAGER = "secretsmanager"
-    PARAMETER_STORE = "parameterstore"
-    LOCAL_STORE = "local"
-
     def __init__(
         self,
         host,
         port,
         project,
         engine,
-        region_name,
-        secrets_store,
         hostname_verification,
         trust_store_path,
         cert_folder,
@@ -65,17 +56,14 @@ def __init__(
         self._port = port
         self._base_url = "https://" + self._host + ":" + str(self._port)
         _logger.info("Base URL: %s", self._base_url)
-        self._region_name = region_name or self.DEFAULT_REGION
-        _logger.debug("Region name: %s", self._region_name)
 
         if api_key_value is not None:
             _logger.debug("Using provided API key value")
             api_key = api_key_value
         else:
-            _logger.debug("Querying secrets store for API key")
-            if secrets_store is None:
-                secrets_store = self.LOCAL_STORE
-            api_key = self._get_secret(secrets_store, "api-key", api_key_file)
+            _logger.debug(f"Reading api key from {api_key_file}")
+            with open(api_key_file) as f:
+                api_key = f.readline().strip()
 
         _logger.debug("Using api key to setup header authentification")
         self._auth = auth.ApiKeyAuth(api_key)
@@ -84,17 +72,16 @@ def __init__(
         self._session = requests.session()
         self._connected = True
 
-        self._verify = self._get_verify(self._host, trust_store_path)
+        self._verify = self._get_verify(hostname_verification, trust_store_path)
         _logger.debug("Verify: %s", self._verify)
 
         self._cert_key = None
         self._cert_folder_base = cert_folder
         self._cert_folder = None
 
-        self._hsfs_post_init(project, engine, region_name)
+        self._hsfs_post_init(project, engine)
 
-    def _hsfs_post_init(self, project, engine, region_name):
-        self._region_name = region_name or self._region_name or self.DEFAULT_REGION
+    def _hsfs_post_init(self, project, engine):
         self._project_name = project
         if project is not None:
             project_info = self._get_project_info(project)
@@ -295,82 +282,6 @@ def _get_client_key_path(self, project_name=None) -> str:
         _logger.debug(f"Getting client key path {path}")
         return path
 
-    def _get_secret(self, secrets_store, secret_key=None, api_key_file=None):
-        """Returns secret value from the AWS Secrets Manager or Parameter Store.
-
-        :param secrets_store: the underlying secrets storage to be used, e.g. `secretsmanager` or `parameterstore`
-        :type secrets_store: str
-        :param secret_key: key for the secret value, e.g. `api-key`, `cert-key`, `trust-store`, `key-store`, defaults to None
-        :type secret_key: str, optional
-        :param api_key_file: path to a file containing an api key, defaults to None
-        :type api_key_file: str optional
-        :raises hsfs.client.exceptions.ExternalClientError: `api_key_file` needs to be set for local mode
-        :raises hsfs.client.exceptions.UnknownSecretStorageError: Provided secrets storage not supported
-        :return: secret
-        :rtype: str
-        """
-        _logger.debug(f"Querying secrets store {secrets_store} for secret {secret_key}")
-        if secrets_store == self.SECRETS_MANAGER:
-            return self._query_secrets_manager(secret_key)
-        elif secrets_store == self.PARAMETER_STORE:
-            return self._query_parameter_store(secret_key)
-        elif secrets_store == self.LOCAL_STORE:
-            if not api_key_file:
-                raise exceptions.ExternalClientError(
-                    "api_key_file needs to be set for local mode"
-                )
-            _logger.debug(f"Reading api key from {api_key_file}")
-            with open(api_key_file) as f:
-                return f.readline().strip()
-        else:
-            raise exceptions.UnknownSecretStorageError(
-                "Secrets storage " + secrets_store + " is not supported."
-            )
-
-    def _query_secrets_manager(self, secret_key):
-        _logger.debug("Querying secrets manager for secret key: %s", secret_key)
-        secret_name = "hopsworks/role/" + self._assumed_role()
-        args = {"service_name": "secretsmanager"}
-        region_name = self._get_region()
-        if region_name:
-            args["region_name"] = region_name
-        client = boto3.client(**args)
-        get_secret_value_response = client.get_secret_value(SecretId=secret_name)
-        return json.loads(get_secret_value_response["SecretString"])[secret_key]
-
-    def _assumed_role(self):
-        _logger.debug("Getting assumed role")
-        client = boto3.client("sts")
-        response = client.get_caller_identity()
-        # arns for assumed roles in SageMaker follow the following schema
-        # arn:aws:sts::123456789012:assumed-role/my-role-name/my-role-session-name
-        local_identifier = response["Arn"].split(":")[-1].split("/")
-        if len(local_identifier) != 3 or local_identifier[0] != "assumed-role":
-            raise Exception(
-                "Failed to extract assumed role from arn: " + response["Arn"]
-            )
-        return local_identifier[1]
-
-    def _get_region(self):
-        if self._region_name != self.DEFAULT_REGION:
-            _logger.debug(f"Region name is not default, returning {self._region_name}")
-            return self._region_name
-        else:
-            _logger.debug("Region name is default, returning None")
-            return None
-
-    def _query_parameter_store(self, secret_key):
-        _logger.debug("Querying parameter store for secret key: %s", secret_key)
-        args = {"service_name": "ssm"}
-        region_name = self._get_region()
-        if region_name:
-            args["region_name"] = region_name
-        client = boto3.client(**args)
-        name = "/hopsworks/role/" + self._assumed_role() + "/type/" + secret_key
-        return client.get_parameter(Name=name, WithDecryption=True)["Parameter"][
-            "Value"
-        ]
-
     def _get_project_info(self, project_name):
         """Makes a REST call to hopsworks to get all metadata of a project for the provided project.
 

python/hopsworks_common/client/hopsworks.py

@@ -54,9 +54,9 @@ def __init__(self):
         self._cert_key = self._get_cert_pw()
         trust_store_path = self._get_trust_store_path()
         hostname_verification = (
-            os.environ[self.REQUESTS_VERIFY]
+            os.environ[self.REQUESTS_VERIFY] == "true"
             if self.REQUESTS_VERIFY in os.environ
-            else "true"
+            else True
         )
         self._project_id = os.environ[self.PROJECT_ID]
         self._project_name = self._project_name()

python/hsfs/connection.py

@@ -32,9 +32,7 @@
 from requests.exceptions import ConnectionError
 
 
-AWS_DEFAULT_REGION = "default"
 HOPSWORKS_PORT_DEFAULT = 443
-SECRETS_STORE_DEFAULT = "parameterstore"
 HOSTNAME_VERIFICATION_DEFAULT = True
 CERT_FOLDER_DEFAULT = "/tmp"
 
@@ -58,7 +56,7 @@ class Connection:
         ```
 
     !!! hint "Save API Key as File"
-        To get started quickly, without saving the Hopsworks API in a secret storage,
+        To get started quickly,
         you can simply create a file with the previously created Hopsworks API Key and
         place it on the environment from which you wish to connect to the Hopsworks
         Feature Store.
@@ -105,19 +103,14 @@ class Connection:
             allows you to override this behaviour. `"training"` engine is useful when only
             feature store metadata is needed, for example training dataset location and label
             information when Hopsworks training experiment is conducted.
-        region_name: The name of the AWS region in which the required secrets are
-            stored, defaults to `"default"`.
-        secrets_store: The secrets storage to be used, either `"secretsmanager"`,
-            `"parameterstore"` or `"local"`, defaults to `"parameterstore"`.
-        hostname_verification: Whether or not to verify Hopsworks’ certificate, defaults
+        hostname_verification: Whether or not to verify Hopsworks' certificate, defaults
             to `True`.
         trust_store_path: Path on the file system containing the Hopsworks certificates,
             defaults to `None`.
         cert_folder: The directory to store retrieved HopsFS certificates, defaults to
             `"/tmp"`. Only required when running without a Spark environment.
-        api_key_file: Path to a file containing the API Key, if provided,
-            `secrets_store` will be ignored, defaults to `None`.
-        api_key_value: API Key as string, if provided, `secrets_store` will be ignored`,
+        api_key_file: Path to a file containing the API Key, defaults to `None`.
+        api_key_value: API Key as string, if provided, `api_key_file` will be ignored,
             however, this should be used with care, especially if the used notebook or
             job script is accessible by multiple parties. Defaults to `None`.
 
@@ -132,8 +125,6 @@ def __init__(
         port: int = HOPSWORKS_PORT_DEFAULT,
         project: Optional[str] = None,
         engine: Optional[str] = None,
-        region_name: str = AWS_DEFAULT_REGION,
-        secrets_store: str = SECRETS_STORE_DEFAULT,
         hostname_verification: bool = HOSTNAME_VERIFICATION_DEFAULT,
         trust_store_path: Optional[str] = None,
         cert_folder: str = CERT_FOLDER_DEFAULT,
@@ -144,8 +135,6 @@ def __init__(
         self._port = port
         self._project = project
         self._engine = engine
-        self._region_name = region_name
-        self._secrets_store = secrets_store
         self._hostname_verification = hostname_verification
         self._trust_store_path = trust_store_path
         self._cert_folder = cert_folder
@@ -240,8 +229,6 @@ def connect(self) -> None:
                     self._port,
                     self._project,
                     self._engine,
-                    self._region_name,
-                    self._secrets_store,
                     self._hostname_verification,
                     self._trust_store_path,
                     self._cert_folder,
@@ -295,8 +282,6 @@ def connection(
         port: int = HOPSWORKS_PORT_DEFAULT,
         project: Optional[str] = None,
         engine: Optional[str] = None,
-        region_name: str = AWS_DEFAULT_REGION,
-        secrets_store: str = SECRETS_STORE_DEFAULT,
         hostname_verification: bool = HOSTNAME_VERIFICATION_DEFAULT,
         trust_store_path: Optional[str] = None,
         cert_folder: str = CERT_FOLDER_DEFAULT,
@@ -309,8 +294,6 @@ def connection(
             port,
             project,
             engine,
-            region_name,
-            secrets_store,
             hostname_verification,
             trust_store_path,
             cert_folder,
@@ -345,24 +328,6 @@ def project(self) -> Optional[str]:
     def project(self, project: Optional[str]) -> str:
         self._project = project
 
-    @property
-    def region_name(self) -> str:
-        return self._region_name
-
-    @region_name.setter
-    @not_connected
-    def region_name(self, region_name: str) -> None:
-        self._region_name = region_name
-
-    @property
-    def secrets_store(self) -> str:
-        return self._secrets_store
-
-    @secrets_store.setter
-    @not_connected
-    def secrets_store(self, secrets_store):
-        self._secrets_store = secrets_store
-
     @property
     def hostname_verification(self):
         return self._hostname_verification

python/hsml/connection.py

@@ -175,8 +175,6 @@ def connect(self):
                     self._port,
                     self._project,
                     None,
-                    None,
-                    None,
                     self._hostname_verification,
                     self._trust_store_path,
                     None,