From c1257c03bf33ae01471468897fb90efc58bd9043 Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Thu, 12 Dec 2024 13:35:40 -0700 Subject: [PATCH 1/2] Removing the tls_verify_client flag for offline server from feast application code. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/feast/cli.py | 12 +----------- sdk/python/feast/feature_store.py | 3 +-- sdk/python/feast/offline_server.py | 5 +---- .../feature_repos/universal/data_sources/file.py | 5 +---- 4 files changed, 4 insertions(+), 21 deletions(-) diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py index a02013b11f9..52c9b5d8198 100644 --- a/sdk/python/feast/cli.py +++ b/sdk/python/feast/cli.py @@ -1132,15 +1132,6 @@ def serve_registry_command( show_default=False, help="path to TLS certificate public key. You need to pass --key as well to start server in TLS mode", ) -@click.option( - "--verify_client", - "-v", - "tls_verify_client", - type=click.BOOL, - default="True", - show_default=True, - help="Verify the client or not for the TLS client certificate.", -) @click.pass_context def serve_offline_command( ctx: click.Context, @@ -1148,7 +1139,6 @@ def serve_offline_command( port: int, tls_key_path: str, tls_cert_path: str, - tls_verify_client: bool, ): """Start a remote server locally on a given host, port.""" if (tls_key_path and not tls_cert_path) or (not tls_key_path and tls_cert_path): @@ -1157,7 +1147,7 @@ def serve_offline_command( ) store = create_feature_store(ctx) - store.serve_offline(host, port, tls_key_path, tls_cert_path, tls_verify_client) + store.serve_offline(host, port, tls_key_path, tls_cert_path) @cli.command("validate") diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py index 79a0d752efb..ae5da8a517d 100644 --- a/sdk/python/feast/feature_store.py +++ b/sdk/python/feast/feature_store.py @@ -1964,13 +1964,12 @@ def serve_offline( port: int, tls_key_path: str = "", tls_cert_path: str = "", - tls_verify_client: bool = True, ) -> None: """Start offline server locally on a given port.""" from feast import offline_server offline_server.start_server( - self, host, port, tls_key_path, tls_cert_path, tls_verify_client + self, host, port, tls_key_path, tls_cert_path ) def serve_transformations(self, port: int) -> None: diff --git a/sdk/python/feast/offline_server.py b/sdk/python/feast/offline_server.py index 8774dea8aed..25fffa211d9 100644 --- a/sdk/python/feast/offline_server.py +++ b/sdk/python/feast/offline_server.py @@ -45,7 +45,6 @@ def __init__( location: str, host: str = "localhost", tls_certificates: List = [], - verify_client=False, **kwargs, ): super(OfflineServer, self).__init__( @@ -54,7 +53,7 @@ def __init__( str_to_auth_manager_type(store.config.auth_config.type) ), tls_certificates=tls_certificates, - verify_client=verify_client, + verify_client=False, # this is needed for when we don't need mTLS **kwargs, ) self._location = location @@ -568,7 +567,6 @@ def start_server( port: int, tls_key_path: str = "", tls_cert_path: str = "", - tls_verify_client: bool = True, ): _init_auth_manager(store) @@ -591,7 +589,6 @@ def start_server( location=location, host=host, tls_certificates=tls_certificates, - verify_client=tls_verify_client, ) try: logger.info(f"Offline store server serving at: {location}") diff --git a/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py b/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py index dc716f45e1e..120ec880f9e 100644 --- a/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py +++ b/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py @@ -451,10 +451,7 @@ def setup(self, registry: RegistryConfig): "--key", str(tls_key_path), "--cert", - str(self.tls_cert_path), - # This is needed for the self-signed certificate, disabled verify_client for integration tests. - "--verify_client", - str(False), + str(self.tls_cert_path) ] self.proc = subprocess.Popen( cmd, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL From 4e8dbb3a5aac1fe486e7001bf34b016dd6f1ff3f Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Thu, 12 Dec 2024 13:40:24 -0700 Subject: [PATCH 2/2] fixing lint errors. formatted the code. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/feast/feature_store.py | 4 +--- sdk/python/feast/offline_server.py | 2 +- .../integration/feature_repos/universal/data_sources/file.py | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py index ae5da8a517d..44975902018 100644 --- a/sdk/python/feast/feature_store.py +++ b/sdk/python/feast/feature_store.py @@ -1968,9 +1968,7 @@ def serve_offline( """Start offline server locally on a given port.""" from feast import offline_server - offline_server.start_server( - self, host, port, tls_key_path, tls_cert_path - ) + offline_server.start_server(self, host, port, tls_key_path, tls_cert_path) def serve_transformations(self, port: int) -> None: """Start the feature transformation server locally on a given port.""" diff --git a/sdk/python/feast/offline_server.py b/sdk/python/feast/offline_server.py index 25fffa211d9..1b714a45c7e 100644 --- a/sdk/python/feast/offline_server.py +++ b/sdk/python/feast/offline_server.py @@ -53,7 +53,7 @@ def __init__( str_to_auth_manager_type(store.config.auth_config.type) ), tls_certificates=tls_certificates, - verify_client=False, # this is needed for when we don't need mTLS + verify_client=False, # this is needed for when we don't need mTLS **kwargs, ) self._location = location diff --git a/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py b/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py index 120ec880f9e..fbfb418278e 100644 --- a/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py +++ b/sdk/python/tests/integration/feature_repos/universal/data_sources/file.py @@ -451,7 +451,7 @@ def setup(self, registry: RegistryConfig): "--key", str(tls_key_path), "--cert", - str(self.tls_cert_path) + str(self.tls_cert_path), ] self.proc = subprocess.Popen( cmd, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL