From f11ce849edade251b4f3c8615cbf60c4429b00e0 Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Sat, 14 Dec 2024 03:19:26 -0500 Subject: [PATCH 01/17] Initial Draft version to load the CA trusted store code. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/feast/cli.py | 44 +++++++++++---- sdk/python/feast/infra/registry/remote.py | 55 ++++++++++++++++--- sdk/python/feast/ssl_ca_setup.py | 20 +++++++ sdk/python/tests/conftest.py | 16 ++++-- .../universal/data_sources/file.py | 1 - .../online_store/test_remote_online_store.py | 36 ++++++++---- .../auth/server/test_auth_registry_server.py | 10 +++- .../tests/utils/auth_permissions_util.py | 21 +++++-- ...ifcate_util.py => ssl_certifcates_util.py} | 40 ++++++++++++++ 9 files changed, 201 insertions(+), 42 deletions(-) create mode 100644 sdk/python/feast/ssl_ca_setup.py rename sdk/python/tests/utils/{generate_self_signed_certifcate_util.py => ssl_certifcates_util.py} (59%) diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py index 15b592119ca..694793fa1bc 100644 --- a/sdk/python/feast/cli.py +++ b/sdk/python/feast/cli.py @@ -49,6 +49,7 @@ teardown, ) from feast.saved_dataset import SavedDataset, ValidationReference +from feast.ssl_ca_setup import configure_ssl_ca from feast.utils import maybe_local_tz _logger = logging.getLogger(__name__) @@ -865,7 +866,6 @@ def materialize_incremental_command(ctx: click.Context, end_ts: str, views: List "cassandra", "hazelcast", "ikv", - "couchbase", ], case_sensitive=False, ), @@ -956,6 +956,15 @@ def init_command(project_directory, minimal: bool, template: str): 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( + "--ca", + "-ca", + "tls_ca_file_path", + type=click.STRING, + default="", + show_default=False, + help="path to ca trust store file. This will override the global path set as part of the environment variable FEAST_CA_CERT_FILE_PATH", +) @click.option( "--metrics", "-m", @@ -975,6 +984,7 @@ def serve_command( keep_alive_timeout: int, tls_key_path: str, tls_cert_path: str, + tls_ca_file_path: str, registry_ttl_sec: int = 5, ): """Start a feature server locally on a given port.""" @@ -983,6 +993,8 @@ def serve_command( "Please pass --cert and --key args to start the feature server in TLS mode." ) + configure_ssl_ca(ca_file_path=tls_ca_file_path) + store = create_feature_store(ctx) store.serve( @@ -1082,14 +1094,25 @@ def serve_transformations_command(ctx: click.Context, port: int): 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( + "--ca", + "-ca", + "tls_ca_file_path", + type=click.STRING, + default="", + show_default=False, + help="path to ca trust store file. This will override the global path set as part of the environment variable FEAST_CA_CERT_FILE_PATH", +) @click.pass_context def serve_registry_command( ctx: click.Context, port: int, tls_key_path: str, tls_cert_path: str, + tls_ca_file_path: str, ): """Start a registry server locally on a given port.""" + configure_ssl_ca(ca_file_path=tls_ca_file_path) if (tls_key_path and not tls_cert_path) or (not tls_key_path and tls_cert_path): raise click.BadParameter( "Please pass --cert and --key args to start the registry server in TLS mode." @@ -1134,13 +1157,13 @@ def serve_registry_command( 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.", + "--ca", + "-ca", + "tls_ca_file_path", + type=click.STRING, + default="", + show_default=False, + help="path to ca trust store file. This will override the global path set as part of the environment variable FEAST_CA_CERT_FILE_PATH", ) @click.pass_context def serve_offline_command( @@ -1149,16 +1172,17 @@ def serve_offline_command( port: int, tls_key_path: str, tls_cert_path: str, - tls_verify_client: bool, + tls_ca_file_path: str, ): """Start a remote server locally on a given host, port.""" + configure_ssl_ca(ca_file_path=tls_ca_file_path) if (tls_key_path and not tls_cert_path) or (not tls_key_path and tls_cert_path): raise click.BadParameter( "Please pass --cert and --key args to start the offline server in TLS mode." ) 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/infra/registry/remote.py b/sdk/python/feast/infra/registry/remote.py index 6cc80d5dad1..94ada74e60f 100644 --- a/sdk/python/feast/infra/registry/remote.py +++ b/sdk/python/feast/infra/registry/remote.py @@ -1,3 +1,5 @@ +import os +import ssl from datetime import datetime from pathlib import Path from typing import List, Optional, Union @@ -6,6 +8,7 @@ from google.protobuf.empty_pb2 import Empty from google.protobuf.timestamp_pb2 import Timestamp from pydantic import StrictStr +from sqlglot.expressions import false from feast.base_feature_view import BaseFeatureView from feast.data_source import DataSource @@ -59,6 +62,11 @@ class RemoteRegistryConfig(RegistryConfig): """ str: Path to the public certificate when the registry server starts in TLS(SSL) mode. This may be needed if the registry server started with a self-signed certificate, typically this file ends with `*.crt`, `*.cer`, or `*.pem`. If registry_type is 'remote', then this configuration is needed to connect to remote registry server in TLS mode. If the remote registry started in non-tls mode then this configuration is not needed.""" + is_tls_mode: bool = False + """ str: Path to the public certificate when the registry server starts in TLS(SSL) mode. This may be needed if the registry server started with a self-signed certificate, typically this file ends with `*.crt`, `*.cer`, or `*.pem`. + If registry_type is 'remote', then this configuration is needed to connect to remote registry server in TLS mode. If the remote registry started in non-tls mode then this configuration is not needed.""" + + class RemoteRegistry(BaseRegistry): def __init__( @@ -70,20 +78,49 @@ def __init__( ): self.auth_config = auth_config assert isinstance(registry_config, RemoteRegistryConfig) - if registry_config.cert: - with open(registry_config.cert, "rb") as cert_file: - trusted_certs = cert_file.read() - tls_credentials = grpc.ssl_channel_credentials( - root_certificates=trusted_certs - ) - self.channel = grpc.secure_channel(registry_config.path, tls_credentials) - else: - self.channel = grpc.insecure_channel(registry_config.path) + # self.channel = create_tls_channel(registry_config) + + self._create_grpc_channel(registry_config) auth_header_interceptor = GrpcClientAuthHeaderInterceptor(auth_config) self.channel = grpc.intercept_channel(self.channel, auth_header_interceptor) self.stub = RegistryServer_pb2_grpc.RegistryServerStub(self.channel) + def _create_grpc_channel(self, registry_config): + assert isinstance(registry_config, RemoteRegistryConfig) + if registry_config.cert or registry_config.is_tls_mode: + cafile = os.getenv("SSL_CERT_FILE") or os.getenv("REQUESTS_CA_BUNDLE") + if not cafile and not registry_config.cert: + raise EnvironmentError( + "SSL_CERT_FILE or REQUESTS_CA_BUNDLE environment variable must be set to use secure TLS or set the cert parameter in feature_Store.yaml file under remote registry configuration." + ) + + with open(registry_config.cert if registry_config.cert else cafile, "rb") as cert_file: + trusted_certs = cert_file.read() + tls_credentials = grpc.ssl_channel_credentials( + root_certificates=trusted_certs + ) + self.channel = grpc.secure_channel(registry_config.path, tls_credentials) + # elif registry_config.is_tls_mode: + # # Use the trust store path from the environment variable + # cafile = os.getenv("SSL_CERT_FILE") or os.getenv("REQUESTS_CA_BUNDLE") + # if cafile: + # # Load the CA certificates from the trust store + # with open(cafile, "rb") as cert_file: + # root_certificates = cert_file.read() + # + # # Create TLS credentials using the root certificates + # tls_credentials = grpc.ssl_channel_credentials(root_certificates=root_certificates) + # # Create a secure gRPC channel + # self.channel = grpc.secure_channel(registry_config.path, tls_credentials) + # else: + # raise EnvironmentError( + # "SSL_CERT_FILE or REQUESTS_CA_BUNDLE environment variable must be set to use secure TLS or set the cert parameter in feature_Store.yaml file under remote registry configuration." + # ) + else: + # Create an insecure gRPC channel + self.channel = grpc.insecure_channel(registry_config.path) + def close(self): if self.channel: self.channel.close() diff --git a/sdk/python/feast/ssl_ca_setup.py b/sdk/python/feast/ssl_ca_setup.py new file mode 100644 index 00000000000..e79d95a86b6 --- /dev/null +++ b/sdk/python/feast/ssl_ca_setup.py @@ -0,0 +1,20 @@ +import logging +import os + +logger = logging.getLogger(__name__) +logger.setLevel(logging.INFO) + +def configure_ssl_ca(ca_file_path:str= ""): + """ + configures the environment variable so that other libraries or servers refer the TLS ca file path. + :param ca_file_path: + :return: + """ + if ca_file_path: + os.environ["SSL_CERT_FILE"] = ca_file_path + os.environ["REQUESTS_CA_BUNDLE"] = ca_file_path + elif 'FEAST_CA_CERT_FILE_PATH' in os.environ and os.environ['FEAST_CA_CERT_FILE_PATH']: + logger.info(f"CA Cert file path found in environment variable FEAST_CA_CERT_FILE_PATH={os.environ['FEAST_CA_CERT_FILE_PATH']}") + os.environ["SSL_CERT_FILE"] = os.environ['FEAST_CA_CERT_FILE_PATH'] + os.environ["REQUESTS_CA_BUNDLE"] = os.environ['FEAST_CA_CERT_FILE_PATH'] + diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index 24c8f40f742..967f6bb1731 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -29,6 +29,7 @@ from feast.data_source import DataSource from feast.feature_store import FeatureStore # noqa: E402 +from feast.ssl_ca_setup import configure_ssl_ca from feast.utils import _utc_now from feast.wait import wait_retry_backoff # noqa: E402 from tests.data.data_creator import ( @@ -57,8 +58,8 @@ location, ) from tests.utils.auth_permissions_util import default_store -from tests.utils.generate_self_signed_certifcate_util import generate_self_signed_cert from tests.utils.http_server import check_port_open, free_port # noqa: E402 +from tests.utils.ssl_certifcates_util import create_ca_trust_store, generate_self_signed_cert logger = logging.getLogger(__name__) @@ -514,17 +515,24 @@ def auth_config(request, is_integration_test): return auth_configuration -@pytest.fixture(params=[True, False], scope="module") +@pytest.fixture(scope="module") def tls_mode(request): - is_tls_mode = request.param + is_tls_mode = request.param[0] + ca_trust_store_path = "" if is_tls_mode: certificates_path = tempfile.mkdtemp() tls_key_path = os.path.join(certificates_path, "key.pem") tls_cert_path = os.path.join(certificates_path, "cert.pem") + generate_self_signed_cert(cert_path=tls_cert_path, key_path=tls_key_path) + is_ca_trust_store_set = request.param[1] + if is_ca_trust_store_set: + ca_trust_store_path = os.path.join(certificates_path, "ca_trust_store.pem") + create_ca_trust_store(public_key_path=tls_cert_path, private_key_path=tls_key_path, output_trust_store_path=ca_trust_store_path) + configure_ssl_ca(ca_file_path=ca_trust_store_path) else: tls_key_path = "" tls_cert_path = "" - return is_tls_mode, tls_key_path, tls_cert_path + return is_tls_mode, tls_key_path, tls_cert_path, ca_trust_store_path 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..b4b5c7c4819 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 @@ -34,7 +34,6 @@ DataSourceCreator, ) from tests.utils.auth_permissions_util import include_auth_config -from tests.utils.generate_self_signed_certifcate_util import generate_self_signed_cert from tests.utils.http_server import check_port_open, free_port # noqa: E402 logger = logging.getLogger(__name__) diff --git a/sdk/python/tests/integration/online_store/test_remote_online_store.py b/sdk/python/tests/integration/online_store/test_remote_online_store.py index 10f1180d8e6..1e932498326 100644 --- a/sdk/python/tests/integration/online_store/test_remote_online_store.py +++ b/sdk/python/tests/integration/online_store/test_remote_online_store.py @@ -22,6 +22,11 @@ @pytest.mark.integration +@pytest.mark.parametrize("tls_mode", [ + ("True", "True"), + ("True", "False"), + ("False", "") +], indirect=True) def test_remote_online_store_read(auth_config, tls_mode): with ( tempfile.TemporaryDirectory() as remote_server_tmp_dir, @@ -56,13 +61,13 @@ def test_remote_online_store_read(auth_config, tls_mode): ) ) assert None not in (server_store, server_url, registry_path) - _, _, tls_cert_path = tls_mode + client_store = _create_remote_client_feature_store( temp_dir=remote_client_tmp_dir, server_registry_path=str(registry_path), feature_server_url=server_url, auth_config=auth_config, - tls_cert_path=tls_cert_path, + tls_mode=tls_mode, ) assert client_store is not None _assert_non_existing_entity_feature_views_entity( @@ -172,7 +177,7 @@ def _create_server_store_spin_feature_server( ): store = default_store(str(temp_dir), auth_config, permissions_list) feast_server_port = free_port() - is_tls_mode, tls_key_path, tls_cert_path = tls_mode + is_tls_mode, tls_key_path, tls_cert_path, _ = tls_mode server_url = next( start_feature_server( @@ -203,20 +208,29 @@ def _create_remote_client_feature_store( server_registry_path: str, feature_server_url: str, auth_config: str, - tls_cert_path: str = "", + tls_mode ) -> FeatureStore: project_name = "REMOTE_ONLINE_CLIENT_PROJECT" runner = CliRunner() result = runner.run(["init", project_name], cwd=temp_dir) assert result.returncode == 0 repo_path = os.path.join(temp_dir, project_name, "feature_repo") - _overwrite_remote_client_feature_store_yaml( - repo_path=str(repo_path), - registry_path=server_registry_path, - feature_server_url=feature_server_url, - auth_config=auth_config, - tls_cert_path=tls_cert_path, - ) + _, _, tls_cert_path, ca_trust_store_path = tls_mode + if ca_trust_store_path: + _overwrite_remote_client_feature_store_yaml( + repo_path=str(repo_path), + registry_path=server_registry_path, + feature_server_url=feature_server_url, + auth_config=auth_config + ) + else: + _overwrite_remote_client_feature_store_yaml( + repo_path=str(repo_path), + registry_path=server_registry_path, + feature_server_url=feature_server_url, + auth_config=auth_config, + tls_cert_path=tls_cert_path, + ) return FeatureStore(repo_path=repo_path) diff --git a/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py b/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py index 25c5fe3eb8c..f42781d605e 100644 --- a/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py +++ b/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py @@ -13,6 +13,7 @@ ) from feast.permissions.permission import Permission from feast.registry_server import start_server +from feast.ssl_ca_setup import configure_ssl_ca from feast.wait import wait_retry_backoff # noqa: E402 from tests.unit.permissions.auth.server import mock_utils from tests.unit.permissions.auth.server.test_utils import ( @@ -44,8 +45,10 @@ def start_registry_server( assertpy.assert_that(server_port).is_not_equal_to(0) - is_tls_mode, tls_key_path, tls_cert_path = tls_mode + is_tls_mode, tls_key_path, tls_cert_path, tls_ca_file_path= tls_mode if is_tls_mode: + #configure_ssl_ca(ca_file_path=tls_ca_file_path) + # Setting the ca_trust_store_path environment variables. print(f"Starting Registry in TLS mode at {server_port}") server = start_server( store=feature_store, @@ -74,6 +77,11 @@ def start_registry_server( server.stop(grace=None) # Teardown server +@pytest.mark.parametrize("tls_mode", [ + ("True", "True"), + ("True", "False"), + ("False", "") +], indirect=True) def test_registry_apis( auth_config, tls_mode, diff --git a/sdk/python/tests/utils/auth_permissions_util.py b/sdk/python/tests/utils/auth_permissions_util.py index 6f0a3c8eeac..38fb596e72d 100644 --- a/sdk/python/tests/utils/auth_permissions_util.py +++ b/sdk/python/tests/utils/auth_permissions_util.py @@ -10,6 +10,7 @@ ) from feast.infra.registry.remote import RemoteRegistryConfig from feast.permissions.permission import Permission +from feast.ssl_ca_setup import configure_ssl_ca from feast.wait import wait_retry_backoff from tests.utils.cli_repo_creator import CliRunner from tests.utils.http_server import check_port_open @@ -127,13 +128,21 @@ def start_feature_server( def get_remote_registry_store(server_port, feature_store, tls_mode): - is_tls_mode, _, tls_cert_path = tls_mode + is_tls_mode, _, tls_cert_path, ca_trust_store_path = tls_mode if is_tls_mode: - registry_config = RemoteRegistryConfig( - registry_type="remote", - path=f"localhost:{server_port}", - cert=tls_cert_path, - ) + if ca_trust_store_path: + registry_config = RemoteRegistryConfig( + registry_type="remote", + path=f"localhost:{server_port}", + is_tls_mode=True, + ) + else: + registry_config = RemoteRegistryConfig( + registry_type="remote", + path=f"localhost:{server_port}", + is_tls_mode=True, + cert=tls_cert_path, + ) else: registry_config = RemoteRegistryConfig( registry_type="remote", path=f"localhost:{server_port}" diff --git a/sdk/python/tests/utils/generate_self_signed_certifcate_util.py b/sdk/python/tests/utils/ssl_certifcates_util.py similarity index 59% rename from sdk/python/tests/utils/generate_self_signed_certifcate_util.py rename to sdk/python/tests/utils/ssl_certifcates_util.py index 559ee18cde7..7d17e0b30f6 100644 --- a/sdk/python/tests/utils/generate_self_signed_certifcate_util.py +++ b/sdk/python/tests/utils/ssl_certifcates_util.py @@ -8,6 +8,11 @@ from cryptography.hazmat.primitives.asymmetric import rsa from cryptography.x509.oid import NameOID +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric import rsa +from cryptography.x509 import load_pem_x509_certificate +from cryptography.hazmat.backends import default_backend + logger = logging.getLogger(__name__) @@ -77,3 +82,38 @@ def generate_self_signed_cert( logger.info( f"Self-signed certificate and private key have been generated at {cert_path} and {key_path}." ) + + +def create_ca_trust_store(public_key_path:str, private_key_path:str, output_trust_store_path:str): + """ + Create a CA trust store file and add the public certificate to it. + + :param public_key_path: Path to the public certificate (e.g., PEM file). + :param private_key_path: Path to the private key (optional, to verify signing authority). + :param output_trust_store_path: Path to save the trust store. + """ + try: + # Read and load the public certificate + with open(public_key_path, "rb") as pub_file: + public_cert_data = pub_file.read() + public_cert = load_pem_x509_certificate(public_cert_data, backend=default_backend()) + + # Verify the private key matches (optional, adds validation) + if private_key_path: + with open(private_key_path, "rb") as priv_file: + private_key_data = priv_file.read() + private_key = serialization.load_pem_private_key( + private_key_data, password=None, backend=default_backend() + ) + # Check the public/private key match + if private_key.public_key().public_numbers() != public_cert.public_key().public_numbers(): + raise ValueError("Public certificate does not match the private key.") + + # Create or append to the trust store + with open(output_trust_store_path, "ab") as trust_store_file: + trust_store_file.write(public_cert.public_bytes(serialization.Encoding.PEM)) + + print(f"Trust store created/updated successfully at: {output_trust_store_path}") + + except Exception as e: + print(f"Error creating CA trust store: {e}") From cc40246e8b60f5992cd1537fa05098553d057ad5 Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Sat, 14 Dec 2024 03:19:26 -0500 Subject: [PATCH 02/17] Initial Draft version to load the CA trusted store code. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/feast/infra/registry/remote.py | 23 ++------------- sdk/python/feast/ssl_ca_setup.py | 29 +++++++++++-------- sdk/python/tests/conftest.py | 11 +++++-- .../universal/data_sources/file.py | 1 + .../online_store/test_remote_online_store.py | 12 ++++---- .../auth/server/test_auth_registry_server.py | 13 ++++----- .../tests/utils/auth_permissions_util.py | 3 +- .../tests/utils/ssl_certifcates_util.py | 23 +++++++++------ 8 files changed, 55 insertions(+), 60 deletions(-) diff --git a/sdk/python/feast/infra/registry/remote.py b/sdk/python/feast/infra/registry/remote.py index 94ada74e60f..e0bd627ec82 100644 --- a/sdk/python/feast/infra/registry/remote.py +++ b/sdk/python/feast/infra/registry/remote.py @@ -1,5 +1,4 @@ import os -import ssl from datetime import datetime from pathlib import Path from typing import List, Optional, Union @@ -8,7 +7,6 @@ from google.protobuf.empty_pb2 import Empty from google.protobuf.timestamp_pb2 import Timestamp from pydantic import StrictStr -from sqlglot.expressions import false from feast.base_feature_view import BaseFeatureView from feast.data_source import DataSource @@ -67,7 +65,6 @@ class RemoteRegistryConfig(RegistryConfig): If registry_type is 'remote', then this configuration is needed to connect to remote registry server in TLS mode. If the remote registry started in non-tls mode then this configuration is not needed.""" - class RemoteRegistry(BaseRegistry): def __init__( self, @@ -95,28 +92,14 @@ def _create_grpc_channel(self, registry_config): "SSL_CERT_FILE or REQUESTS_CA_BUNDLE environment variable must be set to use secure TLS or set the cert parameter in feature_Store.yaml file under remote registry configuration." ) - with open(registry_config.cert if registry_config.cert else cafile, "rb") as cert_file: + with open( + registry_config.cert if registry_config.cert else cafile, "rb" + ) as cert_file: trusted_certs = cert_file.read() tls_credentials = grpc.ssl_channel_credentials( root_certificates=trusted_certs ) self.channel = grpc.secure_channel(registry_config.path, tls_credentials) - # elif registry_config.is_tls_mode: - # # Use the trust store path from the environment variable - # cafile = os.getenv("SSL_CERT_FILE") or os.getenv("REQUESTS_CA_BUNDLE") - # if cafile: - # # Load the CA certificates from the trust store - # with open(cafile, "rb") as cert_file: - # root_certificates = cert_file.read() - # - # # Create TLS credentials using the root certificates - # tls_credentials = grpc.ssl_channel_credentials(root_certificates=root_certificates) - # # Create a secure gRPC channel - # self.channel = grpc.secure_channel(registry_config.path, tls_credentials) - # else: - # raise EnvironmentError( - # "SSL_CERT_FILE or REQUESTS_CA_BUNDLE environment variable must be set to use secure TLS or set the cert parameter in feature_Store.yaml file under remote registry configuration." - # ) else: # Create an insecure gRPC channel self.channel = grpc.insecure_channel(registry_config.path) diff --git a/sdk/python/feast/ssl_ca_setup.py b/sdk/python/feast/ssl_ca_setup.py index e79d95a86b6..1a060b25d02 100644 --- a/sdk/python/feast/ssl_ca_setup.py +++ b/sdk/python/feast/ssl_ca_setup.py @@ -4,17 +4,22 @@ logger = logging.getLogger(__name__) logger.setLevel(logging.INFO) -def configure_ssl_ca(ca_file_path:str= ""): + +def configure_ssl_ca(ca_file_path: str = ""): + """ + configures the environment variable so that other libraries or servers refer the TLS ca file path. + :param ca_file_path: + :return: """ - configures the environment variable so that other libraries or servers refer the TLS ca file path. - :param ca_file_path: - :return: - """ if ca_file_path: - os.environ["SSL_CERT_FILE"] = ca_file_path - os.environ["REQUESTS_CA_BUNDLE"] = ca_file_path - elif 'FEAST_CA_CERT_FILE_PATH' in os.environ and os.environ['FEAST_CA_CERT_FILE_PATH']: - logger.info(f"CA Cert file path found in environment variable FEAST_CA_CERT_FILE_PATH={os.environ['FEAST_CA_CERT_FILE_PATH']}") - os.environ["SSL_CERT_FILE"] = os.environ['FEAST_CA_CERT_FILE_PATH'] - os.environ["REQUESTS_CA_BUNDLE"] = os.environ['FEAST_CA_CERT_FILE_PATH'] - + os.environ["SSL_CERT_FILE"] = ca_file_path + os.environ["REQUESTS_CA_BUNDLE"] = ca_file_path + elif ( + "FEAST_CA_CERT_FILE_PATH" in os.environ + and os.environ["FEAST_CA_CERT_FILE_PATH"] + ): + logger.info( + f"CA Cert file path found in environment variable FEAST_CA_CERT_FILE_PATH={os.environ['FEAST_CA_CERT_FILE_PATH']}" + ) + os.environ["SSL_CERT_FILE"] = os.environ["FEAST_CA_CERT_FILE_PATH"] + os.environ["REQUESTS_CA_BUNDLE"] = os.environ["FEAST_CA_CERT_FILE_PATH"] diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index 967f6bb1731..9de2d36e03e 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -59,7 +59,10 @@ ) from tests.utils.auth_permissions_util import default_store from tests.utils.http_server import check_port_open, free_port # noqa: E402 -from tests.utils.ssl_certifcates_util import create_ca_trust_store, generate_self_signed_cert +from tests.utils.ssl_certifcates_util import ( + create_ca_trust_store, + generate_self_signed_cert, +) logger = logging.getLogger(__name__) @@ -529,7 +532,11 @@ def tls_mode(request): is_ca_trust_store_set = request.param[1] if is_ca_trust_store_set: ca_trust_store_path = os.path.join(certificates_path, "ca_trust_store.pem") - create_ca_trust_store(public_key_path=tls_cert_path, private_key_path=tls_key_path, output_trust_store_path=ca_trust_store_path) + create_ca_trust_store( + public_key_path=tls_cert_path, + private_key_path=tls_key_path, + output_trust_store_path=ca_trust_store_path, + ) configure_ssl_ca(ca_file_path=ca_trust_store_path) else: tls_key_path = "" 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 b4b5c7c4819..83f7a9c2a56 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 @@ -35,6 +35,7 @@ ) from tests.utils.auth_permissions_util import include_auth_config from tests.utils.http_server import check_port_open, free_port # noqa: E402 +from tests.utils.ssl_certifcates_util import generate_self_signed_cert logger = logging.getLogger(__name__) diff --git a/sdk/python/tests/integration/online_store/test_remote_online_store.py b/sdk/python/tests/integration/online_store/test_remote_online_store.py index 1e932498326..50a33d024d3 100644 --- a/sdk/python/tests/integration/online_store/test_remote_online_store.py +++ b/sdk/python/tests/integration/online_store/test_remote_online_store.py @@ -22,11 +22,9 @@ @pytest.mark.integration -@pytest.mark.parametrize("tls_mode", [ - ("True", "True"), - ("True", "False"), - ("False", "") -], indirect=True) +@pytest.mark.parametrize( + "tls_mode", [("True", "True"), ("True", "False"), ("False", "")], indirect=True +) def test_remote_online_store_read(auth_config, tls_mode): with ( tempfile.TemporaryDirectory() as remote_server_tmp_dir, @@ -208,7 +206,7 @@ def _create_remote_client_feature_store( server_registry_path: str, feature_server_url: str, auth_config: str, - tls_mode + tls_mode, ) -> FeatureStore: project_name = "REMOTE_ONLINE_CLIENT_PROJECT" runner = CliRunner() @@ -221,7 +219,7 @@ def _create_remote_client_feature_store( repo_path=str(repo_path), registry_path=server_registry_path, feature_server_url=feature_server_url, - auth_config=auth_config + auth_config=auth_config, ) else: _overwrite_remote_client_feature_store_yaml( diff --git a/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py b/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py index f42781d605e..f67da3b4d30 100644 --- a/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py +++ b/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py @@ -13,7 +13,6 @@ ) from feast.permissions.permission import Permission from feast.registry_server import start_server -from feast.ssl_ca_setup import configure_ssl_ca from feast.wait import wait_retry_backoff # noqa: E402 from tests.unit.permissions.auth.server import mock_utils from tests.unit.permissions.auth.server.test_utils import ( @@ -45,9 +44,9 @@ def start_registry_server( assertpy.assert_that(server_port).is_not_equal_to(0) - is_tls_mode, tls_key_path, tls_cert_path, tls_ca_file_path= tls_mode + is_tls_mode, tls_key_path, tls_cert_path, tls_ca_file_path = tls_mode if is_tls_mode: - #configure_ssl_ca(ca_file_path=tls_ca_file_path) + # configure_ssl_ca(ca_file_path=tls_ca_file_path) # Setting the ca_trust_store_path environment variables. print(f"Starting Registry in TLS mode at {server_port}") server = start_server( @@ -77,11 +76,9 @@ def start_registry_server( server.stop(grace=None) # Teardown server -@pytest.mark.parametrize("tls_mode", [ - ("True", "True"), - ("True", "False"), - ("False", "") -], indirect=True) +@pytest.mark.parametrize( + "tls_mode", [("True", "True"), ("True", "False"), ("False", "")], indirect=True +) def test_registry_apis( auth_config, tls_mode, diff --git a/sdk/python/tests/utils/auth_permissions_util.py b/sdk/python/tests/utils/auth_permissions_util.py index 38fb596e72d..de88741a2a6 100644 --- a/sdk/python/tests/utils/auth_permissions_util.py +++ b/sdk/python/tests/utils/auth_permissions_util.py @@ -10,7 +10,6 @@ ) from feast.infra.registry.remote import RemoteRegistryConfig from feast.permissions.permission import Permission -from feast.ssl_ca_setup import configure_ssl_ca from feast.wait import wait_retry_backoff from tests.utils.cli_repo_creator import CliRunner from tests.utils.http_server import check_port_open @@ -128,7 +127,7 @@ def start_feature_server( def get_remote_registry_store(server_port, feature_store, tls_mode): - is_tls_mode, _, tls_cert_path, ca_trust_store_path = tls_mode + is_tls_mode, _, tls_cert_path, ca_trust_store_path = tls_mode if is_tls_mode: if ca_trust_store_path: registry_config = RemoteRegistryConfig( diff --git a/sdk/python/tests/utils/ssl_certifcates_util.py b/sdk/python/tests/utils/ssl_certifcates_util.py index 7d17e0b30f6..b06c23b924a 100644 --- a/sdk/python/tests/utils/ssl_certifcates_util.py +++ b/sdk/python/tests/utils/ssl_certifcates_util.py @@ -6,12 +6,8 @@ from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import hashes, serialization from cryptography.hazmat.primitives.asymmetric import rsa -from cryptography.x509.oid import NameOID - -from cryptography.hazmat.primitives import serialization -from cryptography.hazmat.primitives.asymmetric import rsa from cryptography.x509 import load_pem_x509_certificate -from cryptography.hazmat.backends import default_backend +from cryptography.x509.oid import NameOID logger = logging.getLogger(__name__) @@ -84,7 +80,9 @@ def generate_self_signed_cert( ) -def create_ca_trust_store(public_key_path:str, private_key_path:str, output_trust_store_path:str): +def create_ca_trust_store( + public_key_path: str, private_key_path: str, output_trust_store_path: str +): """ Create a CA trust store file and add the public certificate to it. @@ -96,7 +94,9 @@ def create_ca_trust_store(public_key_path:str, private_key_path:str, output_trus # Read and load the public certificate with open(public_key_path, "rb") as pub_file: public_cert_data = pub_file.read() - public_cert = load_pem_x509_certificate(public_cert_data, backend=default_backend()) + public_cert = load_pem_x509_certificate( + public_cert_data, backend=default_backend() + ) # Verify the private key matches (optional, adds validation) if private_key_path: @@ -106,8 +106,13 @@ def create_ca_trust_store(public_key_path:str, private_key_path:str, output_trus private_key_data, password=None, backend=default_backend() ) # Check the public/private key match - if private_key.public_key().public_numbers() != public_cert.public_key().public_numbers(): - raise ValueError("Public certificate does not match the private key.") + if ( + private_key.public_key().public_numbers() + != public_cert.public_key().public_numbers() + ): + raise ValueError( + "Public certificate does not match the private key." + ) # Create or append to the trust store with open(output_trust_store_path, "ab") as trust_store_file: From eda69003a6e534fb64dba966c138141fbc2c49a0 Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Sat, 14 Dec 2024 03:30:31 -0500 Subject: [PATCH 03/17] Fixing the lint error. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/feast/infra/registry/remote.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/python/feast/infra/registry/remote.py b/sdk/python/feast/infra/registry/remote.py index e0bd627ec82..a35bbc01c34 100644 --- a/sdk/python/feast/infra/registry/remote.py +++ b/sdk/python/feast/infra/registry/remote.py @@ -77,7 +77,7 @@ def __init__( assert isinstance(registry_config, RemoteRegistryConfig) # self.channel = create_tls_channel(registry_config) - self._create_grpc_channel(registry_config) + self.channel = self._create_grpc_channel(registry_config) auth_header_interceptor = GrpcClientAuthHeaderInterceptor(auth_config) self.channel = grpc.intercept_channel(self.channel, auth_header_interceptor) @@ -99,10 +99,10 @@ def _create_grpc_channel(self, registry_config): tls_credentials = grpc.ssl_channel_credentials( root_certificates=trusted_certs ) - self.channel = grpc.secure_channel(registry_config.path, tls_credentials) + return grpc.secure_channel(registry_config.path, tls_credentials) else: # Create an insecure gRPC channel - self.channel = grpc.insecure_channel(registry_config.path) + return grpc.insecure_channel(registry_config.path) def close(self): if self.channel: From abf4c7ebe2930d12c67d35a02604429540f46932 Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Mon, 16 Dec 2024 22:13:28 -0500 Subject: [PATCH 04/17] Trying to fix the online store test cases. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/tests/conftest.py | 1 - .../integration/online_store/test_remote_online_store.py | 9 +++++---- sdk/python/tests/utils/auth_permissions_util.py | 5 +++++ sdk/python/tests/utils/ssl_certifcates_util.py | 4 ++-- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index 9de2d36e03e..4f913a7ca79 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -537,7 +537,6 @@ def tls_mode(request): private_key_path=tls_key_path, output_trust_store_path=ca_trust_store_path, ) - configure_ssl_ca(ca_file_path=ca_trust_store_path) else: tls_key_path = "" tls_cert_path = "" diff --git a/sdk/python/tests/integration/online_store/test_remote_online_store.py b/sdk/python/tests/integration/online_store/test_remote_online_store.py index 50a33d024d3..45f3d87fab8 100644 --- a/sdk/python/tests/integration/online_store/test_remote_online_store.py +++ b/sdk/python/tests/integration/online_store/test_remote_online_store.py @@ -175,7 +175,7 @@ def _create_server_store_spin_feature_server( ): store = default_store(str(temp_dir), auth_config, permissions_list) feast_server_port = free_port() - is_tls_mode, tls_key_path, tls_cert_path, _ = tls_mode + is_tls_mode, tls_key_path, tls_cert_path, ca_trust_store_path = tls_mode server_url = next( start_feature_server( @@ -183,6 +183,7 @@ def _create_server_store_spin_feature_server( server_port=feast_server_port, tls_key_path=tls_key_path, tls_cert_path=tls_cert_path, + ca_trust_store_path=ca_trust_store_path ) ) if is_tls_mode: @@ -213,13 +214,14 @@ def _create_remote_client_feature_store( result = runner.run(["init", project_name], cwd=temp_dir) assert result.returncode == 0 repo_path = os.path.join(temp_dir, project_name, "feature_repo") - _, _, tls_cert_path, ca_trust_store_path = tls_mode - if ca_trust_store_path: + is_tls_mode, _, tls_cert_path, ca_trust_store_path = tls_mode + if is_tls_mode and not ca_trust_store_path: _overwrite_remote_client_feature_store_yaml( repo_path=str(repo_path), registry_path=server_registry_path, feature_server_url=feature_server_url, auth_config=auth_config, + tls_cert_path=tls_cert_path, ) else: _overwrite_remote_client_feature_store_yaml( @@ -227,7 +229,6 @@ def _create_remote_client_feature_store( registry_path=server_registry_path, feature_server_url=feature_server_url, auth_config=auth_config, - tls_cert_path=tls_cert_path, ) return FeatureStore(repo_path=repo_path) diff --git a/sdk/python/tests/utils/auth_permissions_util.py b/sdk/python/tests/utils/auth_permissions_util.py index de88741a2a6..2efb27d4e8b 100644 --- a/sdk/python/tests/utils/auth_permissions_util.py +++ b/sdk/python/tests/utils/auth_permissions_util.py @@ -60,6 +60,7 @@ def start_feature_server( metrics: bool = False, tls_key_path: str = "", tls_cert_path: str = "", + ca_trust_store_path: str = "" ): host = "0.0.0.0" cmd = [ @@ -78,6 +79,10 @@ def start_feature_server( cmd.append("--cert") cmd.append(tls_cert_path) + if ca_trust_store_path: + cmd.append("--tls_ca_file_path") + cmd.append(ca_trust_store_path) + feast_server_process = subprocess.Popen( cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE ) diff --git a/sdk/python/tests/utils/ssl_certifcates_util.py b/sdk/python/tests/utils/ssl_certifcates_util.py index b06c23b924a..e24f147cd6d 100644 --- a/sdk/python/tests/utils/ssl_certifcates_util.py +++ b/sdk/python/tests/utils/ssl_certifcates_util.py @@ -118,7 +118,7 @@ def create_ca_trust_store( with open(output_trust_store_path, "ab") as trust_store_file: trust_store_file.write(public_cert.public_bytes(serialization.Encoding.PEM)) - print(f"Trust store created/updated successfully at: {output_trust_store_path}") + logger.info(f"Trust store created/updated successfully at: {output_trust_store_path}") except Exception as e: - print(f"Error creating CA trust store: {e}") + logger.error(f"Error creating CA trust store: {e}") From 767f241f98116eda7e2c8dd4341280d0e2e099ab Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Mon, 16 Dec 2024 22:46:28 -0500 Subject: [PATCH 05/17] Formatted the python to fix lint errors. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/feast/infra/registry/remote.py | 2 +- sdk/python/tests/conftest.py | 1 - .../integration/online_store/test_remote_online_store.py | 2 +- sdk/python/tests/utils/auth_permissions_util.py | 2 +- sdk/python/tests/utils/ssl_certifcates_util.py | 4 +++- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/sdk/python/feast/infra/registry/remote.py b/sdk/python/feast/infra/registry/remote.py index a35bbc01c34..4333c877b28 100644 --- a/sdk/python/feast/infra/registry/remote.py +++ b/sdk/python/feast/infra/registry/remote.py @@ -77,7 +77,7 @@ def __init__( assert isinstance(registry_config, RemoteRegistryConfig) # self.channel = create_tls_channel(registry_config) - self.channel = self._create_grpc_channel(registry_config) + self.channel = self._create_grpc_channel(registry_config) auth_header_interceptor = GrpcClientAuthHeaderInterceptor(auth_config) self.channel = grpc.intercept_channel(self.channel, auth_header_interceptor) diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index 4f913a7ca79..a12d10a7dcd 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -29,7 +29,6 @@ from feast.data_source import DataSource from feast.feature_store import FeatureStore # noqa: E402 -from feast.ssl_ca_setup import configure_ssl_ca from feast.utils import _utc_now from feast.wait import wait_retry_backoff # noqa: E402 from tests.data.data_creator import ( diff --git a/sdk/python/tests/integration/online_store/test_remote_online_store.py b/sdk/python/tests/integration/online_store/test_remote_online_store.py index 45f3d87fab8..ae8ca4bfb31 100644 --- a/sdk/python/tests/integration/online_store/test_remote_online_store.py +++ b/sdk/python/tests/integration/online_store/test_remote_online_store.py @@ -183,7 +183,7 @@ def _create_server_store_spin_feature_server( server_port=feast_server_port, tls_key_path=tls_key_path, tls_cert_path=tls_cert_path, - ca_trust_store_path=ca_trust_store_path + ca_trust_store_path=ca_trust_store_path, ) ) if is_tls_mode: diff --git a/sdk/python/tests/utils/auth_permissions_util.py b/sdk/python/tests/utils/auth_permissions_util.py index 2efb27d4e8b..0d150bf55f0 100644 --- a/sdk/python/tests/utils/auth_permissions_util.py +++ b/sdk/python/tests/utils/auth_permissions_util.py @@ -60,7 +60,7 @@ def start_feature_server( metrics: bool = False, tls_key_path: str = "", tls_cert_path: str = "", - ca_trust_store_path: str = "" + ca_trust_store_path: str = "", ): host = "0.0.0.0" cmd = [ diff --git a/sdk/python/tests/utils/ssl_certifcates_util.py b/sdk/python/tests/utils/ssl_certifcates_util.py index e24f147cd6d..eb0430e98be 100644 --- a/sdk/python/tests/utils/ssl_certifcates_util.py +++ b/sdk/python/tests/utils/ssl_certifcates_util.py @@ -118,7 +118,9 @@ def create_ca_trust_store( with open(output_trust_store_path, "ab") as trust_store_file: trust_store_file.write(public_cert.public_bytes(serialization.Encoding.PEM)) - logger.info(f"Trust store created/updated successfully at: {output_trust_store_path}") + logger.info( + f"Trust store created/updated successfully at: {output_trust_store_path}" + ) except Exception as e: logger.error(f"Error creating CA trust store: {e}") From 436f0db62dbb73a9c1ddcbb055911a58fa780a20 Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Mon, 16 Dec 2024 22:59:37 -0500 Subject: [PATCH 06/17] Fixing the unit test cases. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/feast/cli.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py index 08f585cc0af..cff91e2b0f6 100644 --- a/sdk/python/feast/cli.py +++ b/sdk/python/feast/cli.py @@ -1156,6 +1156,15 @@ 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, @@ -1163,6 +1172,7 @@ 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): @@ -1171,7 +1181,7 @@ def serve_offline_command( ) store = create_feature_store(ctx) - store.serve_offline(host, port, tls_key_path, tls_cert_path) + store.serve_offline(host, port, tls_key_path, tls_cert_path, tls_verify_client) @cli.command("validate") From 1d64ebbe23120c9eddae518871a243ef653ee42a Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Mon, 16 Dec 2024 23:11:11 -0500 Subject: [PATCH 07/17] Fixing the unit test cases. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/feast/cli.py | 3 +-- sdk/python/feast/feature_store.py | 12 ++++++++++-- sdk/python/feast/registry_server.py | 3 +++ .../auth/server/test_auth_registry_server.py | 1 + 4 files changed, 15 insertions(+), 4 deletions(-) diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py index cff91e2b0f6..30facbf78f7 100644 --- a/sdk/python/feast/cli.py +++ b/sdk/python/feast/cli.py @@ -1112,14 +1112,13 @@ def serve_registry_command( tls_ca_file_path: str, ): """Start a registry server locally on a given port.""" - configure_ssl_ca(ca_file_path=tls_ca_file_path) if (tls_key_path and not tls_cert_path) or (not tls_key_path and tls_cert_path): raise click.BadParameter( "Please pass --cert and --key args to start the registry server in TLS mode." ) store = create_feature_store(ctx) - store.serve_registry(port, tls_key_path, tls_cert_path) + store.serve_registry(port, tls_key_path, tls_cert_path, tls_ca_file_path) @cli.command("serve_offline") diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py index 44975902018..3b73b7b7096 100644 --- a/sdk/python/feast/feature_store.py +++ b/sdk/python/feast/feature_store.py @@ -1949,13 +1949,21 @@ def serve_ui( ) def serve_registry( - self, port: int, tls_key_path: str = "", tls_cert_path: str = "" + self, + port: int, + tls_key_path: str = "", + tls_cert_path: str = "", + tls_ca_file_path: str = "", ) -> None: """Start registry server locally on a given port.""" from feast import registry_server registry_server.start_server( - self, port=port, tls_key_path=tls_key_path, tls_cert_path=tls_cert_path + self, + port=port, + tls_key_path=tls_key_path, + tls_cert_path=tls_cert_path, + tls_ca_file_path=tls_ca_file_path, ) def serve_offline( diff --git a/sdk/python/feast/registry_server.py b/sdk/python/feast/registry_server.py index c9abf62ccd7..f9df6513efc 100644 --- a/sdk/python/feast/registry_server.py +++ b/sdk/python/feast/registry_server.py @@ -37,6 +37,7 @@ from feast.project import Project from feast.protos.feast.registry import RegistryServer_pb2, RegistryServer_pb2_grpc from feast.saved_dataset import SavedDataset, ValidationReference +from feast.ssl_ca_setup import configure_ssl_ca from feast.stream_feature_view import StreamFeatureView logger = logging.getLogger(__name__) @@ -763,7 +764,9 @@ def start_server( wait_for_termination: bool = True, tls_key_path: str = "", tls_cert_path: str = "", + tls_ca_file_path: str = "", ): + configure_ssl_ca(ca_file_path=tls_ca_file_path) auth_manager_type = str_to_auth_manager_type(store.config.auth_config.type) init_security_manager(auth_type=auth_manager_type, fs=store) init_auth_manager( diff --git a/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py b/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py index f67da3b4d30..977fffc4a71 100644 --- a/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py +++ b/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py @@ -55,6 +55,7 @@ def start_registry_server( wait_for_termination=False, tls_key_path=tls_key_path, tls_cert_path=tls_cert_path, + tls_ca_file_path=tls_ca_file_path, ) else: print(f"Starting Registry in Non-TLS mode at {server_port}") From 9540e7e8928e49a6eb57b7230993b773c9e1129f Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Mon, 16 Dec 2024 23:19:53 -0500 Subject: [PATCH 08/17] removing unnecessary cli args. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/feast/cli.py | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py index 30facbf78f7..6deaa1ab51d 100644 --- a/sdk/python/feast/cli.py +++ b/sdk/python/feast/cli.py @@ -1155,15 +1155,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, @@ -1171,7 +1162,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): @@ -1180,7 +1170,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") From fff05f421e51b4db0e5a60076acdc69679f3f1fd Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Tue, 17 Dec 2024 12:22:36 -0500 Subject: [PATCH 09/17] Now configuring the SSL ca store configurations on the feast client side rather than on the server side. And also fixing the integration tests. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/feast/cli.py | 26 +------------------ sdk/python/feast/feature_store.py | 5 ++-- sdk/python/feast/registry_server.py | 3 --- ...a_setup.py => ssl_ca_trust_store_setup.py} | 9 +++---- .../online_store/test_remote_online_store.py | 4 +++ .../auth/server/test_auth_registry_server.py | 1 - .../tests/utils/auth_permissions_util.py | 8 +++--- 7 files changed, 15 insertions(+), 41 deletions(-) rename sdk/python/feast/{ssl_ca_setup.py => ssl_ca_trust_store_setup.py} (63%) diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py index 6deaa1ab51d..56c5752ed80 100644 --- a/sdk/python/feast/cli.py +++ b/sdk/python/feast/cli.py @@ -49,7 +49,6 @@ teardown, ) from feast.saved_dataset import SavedDataset, ValidationReference -from feast.ssl_ca_setup import configure_ssl_ca from feast.utils import maybe_local_tz _logger = logging.getLogger(__name__) @@ -956,15 +955,6 @@ def init_command(project_directory, minimal: bool, template: str): 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( - "--ca", - "-ca", - "tls_ca_file_path", - type=click.STRING, - default="", - show_default=False, - help="path to ca trust store file. This will override the global path set as part of the environment variable FEAST_CA_CERT_FILE_PATH", -) @click.option( "--metrics", "-m", @@ -984,7 +974,6 @@ def serve_command( keep_alive_timeout: int, tls_key_path: str, tls_cert_path: str, - tls_ca_file_path: str, registry_ttl_sec: int = 5, ): """Start a feature server locally on a given port.""" @@ -992,9 +981,6 @@ def serve_command( raise click.BadParameter( "Please pass --cert and --key args to start the feature server in TLS mode." ) - - configure_ssl_ca(ca_file_path=tls_ca_file_path) - store = create_feature_store(ctx) store.serve( @@ -1094,22 +1080,12 @@ def serve_transformations_command(ctx: click.Context, port: int): 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( - "--ca", - "-ca", - "tls_ca_file_path", - type=click.STRING, - default="", - show_default=False, - help="path to ca trust store file. This will override the global path set as part of the environment variable FEAST_CA_CERT_FILE_PATH", -) @click.pass_context def serve_registry_command( ctx: click.Context, port: int, tls_key_path: str, tls_cert_path: str, - tls_ca_file_path: str, ): """Start a registry server locally on a given port.""" if (tls_key_path and not tls_cert_path) or (not tls_key_path and tls_cert_path): @@ -1118,7 +1094,7 @@ def serve_registry_command( ) store = create_feature_store(ctx) - store.serve_registry(port, tls_key_path, tls_cert_path, tls_ca_file_path) + store.serve_registry(port, tls_key_path, tls_cert_path) @cli.command("serve_offline") diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py index 3b73b7b7096..edbd060e106 100644 --- a/sdk/python/feast/feature_store.py +++ b/sdk/python/feast/feature_store.py @@ -86,6 +86,7 @@ from feast.repo_config import RepoConfig, load_repo_config from feast.repo_contents import RepoContents from feast.saved_dataset import SavedDataset, SavedDatasetStorage, ValidationReference +from feast.ssl_ca_trust_store_setup import configure_ca_trust_store_env_variables from feast.stream_feature_view import StreamFeatureView from feast.utils import _utc_now @@ -129,6 +130,8 @@ def __init__( if fs_yaml_file is not None and config is not None: raise ValueError("You cannot specify both fs_yaml_file and config.") + configure_ca_trust_store_env_variables() + if repo_path: self.repo_path = Path(repo_path) else: @@ -1953,7 +1956,6 @@ def serve_registry( port: int, tls_key_path: str = "", tls_cert_path: str = "", - tls_ca_file_path: str = "", ) -> None: """Start registry server locally on a given port.""" from feast import registry_server @@ -1963,7 +1965,6 @@ def serve_registry( port=port, tls_key_path=tls_key_path, tls_cert_path=tls_cert_path, - tls_ca_file_path=tls_ca_file_path, ) def serve_offline( diff --git a/sdk/python/feast/registry_server.py b/sdk/python/feast/registry_server.py index f9df6513efc..c9abf62ccd7 100644 --- a/sdk/python/feast/registry_server.py +++ b/sdk/python/feast/registry_server.py @@ -37,7 +37,6 @@ from feast.project import Project from feast.protos.feast.registry import RegistryServer_pb2, RegistryServer_pb2_grpc from feast.saved_dataset import SavedDataset, ValidationReference -from feast.ssl_ca_setup import configure_ssl_ca from feast.stream_feature_view import StreamFeatureView logger = logging.getLogger(__name__) @@ -764,9 +763,7 @@ def start_server( wait_for_termination: bool = True, tls_key_path: str = "", tls_cert_path: str = "", - tls_ca_file_path: str = "", ): - configure_ssl_ca(ca_file_path=tls_ca_file_path) auth_manager_type = str_to_auth_manager_type(store.config.auth_config.type) init_security_manager(auth_type=auth_manager_type, fs=store) init_auth_manager( diff --git a/sdk/python/feast/ssl_ca_setup.py b/sdk/python/feast/ssl_ca_trust_store_setup.py similarity index 63% rename from sdk/python/feast/ssl_ca_setup.py rename to sdk/python/feast/ssl_ca_trust_store_setup.py index 1a060b25d02..396ff36703c 100644 --- a/sdk/python/feast/ssl_ca_setup.py +++ b/sdk/python/feast/ssl_ca_trust_store_setup.py @@ -5,21 +5,18 @@ logger.setLevel(logging.INFO) -def configure_ssl_ca(ca_file_path: str = ""): +def configure_ca_trust_store_env_variables(): """ configures the environment variable so that other libraries or servers refer the TLS ca file path. :param ca_file_path: :return: """ - if ca_file_path: - os.environ["SSL_CERT_FILE"] = ca_file_path - os.environ["REQUESTS_CA_BUNDLE"] = ca_file_path - elif ( + if ( "FEAST_CA_CERT_FILE_PATH" in os.environ and os.environ["FEAST_CA_CERT_FILE_PATH"] ): logger.info( - f"CA Cert file path found in environment variable FEAST_CA_CERT_FILE_PATH={os.environ['FEAST_CA_CERT_FILE_PATH']}" + f"Feast CA Cert file path found in environment variable FEAST_CA_CERT_FILE_PATH={os.environ['FEAST_CA_CERT_FILE_PATH']}. Going to refer this path." ) os.environ["SSL_CERT_FILE"] = os.environ["FEAST_CA_CERT_FILE_PATH"] os.environ["REQUESTS_CA_BUNDLE"] = os.environ["FEAST_CA_CERT_FILE_PATH"] diff --git a/sdk/python/tests/integration/online_store/test_remote_online_store.py b/sdk/python/tests/integration/online_store/test_remote_online_store.py index ae8ca4bfb31..285253dfaaf 100644 --- a/sdk/python/tests/integration/online_store/test_remote_online_store.py +++ b/sdk/python/tests/integration/online_store/test_remote_online_store.py @@ -231,6 +231,10 @@ def _create_remote_client_feature_store( auth_config=auth_config, ) + if is_tls_mode and ca_trust_store_path: + # configure trust store path only when is_tls_mode and ca_trust_store_path exists. + os.environ["FEAST_CA_CERT_FILE_PATH"] = ca_trust_store_path + return FeatureStore(repo_path=repo_path) diff --git a/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py b/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py index 977fffc4a71..f67da3b4d30 100644 --- a/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py +++ b/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py @@ -55,7 +55,6 @@ def start_registry_server( wait_for_termination=False, tls_key_path=tls_key_path, tls_cert_path=tls_cert_path, - tls_ca_file_path=tls_ca_file_path, ) else: print(f"Starting Registry in Non-TLS mode at {server_port}") diff --git a/sdk/python/tests/utils/auth_permissions_util.py b/sdk/python/tests/utils/auth_permissions_util.py index 0d150bf55f0..a223cef176d 100644 --- a/sdk/python/tests/utils/auth_permissions_util.py +++ b/sdk/python/tests/utils/auth_permissions_util.py @@ -79,10 +79,6 @@ def start_feature_server( cmd.append("--cert") cmd.append(tls_cert_path) - if ca_trust_store_path: - cmd.append("--tls_ca_file_path") - cmd.append(ca_trust_store_path) - feast_server_process = subprocess.Popen( cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE ) @@ -152,6 +148,10 @@ def get_remote_registry_store(server_port, feature_store, tls_mode): registry_type="remote", path=f"localhost:{server_port}" ) + if is_tls_mode and ca_trust_store_path: + # configure trust store path only when is_tls_mode and ca_trust_store_path exists. + os.environ["FEAST_CA_CERT_FILE_PATH"] = ca_trust_store_path + store = FeatureStore( config=RepoConfig( project=PROJECT_NAME, From 36859bf2b9244ee04e36a33c6109a4bc9ca22a31 Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Tue, 17 Dec 2024 13:41:50 -0500 Subject: [PATCH 10/17] Renamed the remote registry is_tls_mode variable to is_tls. Changed the offline store TLS setting decision from cert to scheme. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/feast/infra/offline_stores/remote.py | 2 +- sdk/python/feast/infra/registry/remote.py | 5 ++--- sdk/python/tests/utils/auth_permissions_util.py | 4 ++-- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/sdk/python/feast/infra/offline_stores/remote.py b/sdk/python/feast/infra/offline_stores/remote.py index 6f26e06c6ba..d11fb4673db 100644 --- a/sdk/python/feast/infra/offline_stores/remote.py +++ b/sdk/python/feast/infra/offline_stores/remote.py @@ -74,7 +74,7 @@ def build_arrow_flight_client( scheme: str, host: str, port, auth_config: AuthConfig, cert: str = "" ): arrow_scheme = "grpc+tcp" - if cert: + if scheme == "https": logger.info( "Scheme is https so going to connect offline server in SSL(TLS) mode." ) diff --git a/sdk/python/feast/infra/registry/remote.py b/sdk/python/feast/infra/registry/remote.py index 4333c877b28..f9177aeb243 100644 --- a/sdk/python/feast/infra/registry/remote.py +++ b/sdk/python/feast/infra/registry/remote.py @@ -60,7 +60,7 @@ class RemoteRegistryConfig(RegistryConfig): """ str: Path to the public certificate when the registry server starts in TLS(SSL) mode. This may be needed if the registry server started with a self-signed certificate, typically this file ends with `*.crt`, `*.cer`, or `*.pem`. If registry_type is 'remote', then this configuration is needed to connect to remote registry server in TLS mode. If the remote registry started in non-tls mode then this configuration is not needed.""" - is_tls_mode: bool = False + is_tls: bool = False """ str: Path to the public certificate when the registry server starts in TLS(SSL) mode. This may be needed if the registry server started with a self-signed certificate, typically this file ends with `*.crt`, `*.cer`, or `*.pem`. If registry_type is 'remote', then this configuration is needed to connect to remote registry server in TLS mode. If the remote registry started in non-tls mode then this configuration is not needed.""" @@ -85,13 +85,12 @@ def __init__( def _create_grpc_channel(self, registry_config): assert isinstance(registry_config, RemoteRegistryConfig) - if registry_config.cert or registry_config.is_tls_mode: + if registry_config.cert or registry_config.is_tls: cafile = os.getenv("SSL_CERT_FILE") or os.getenv("REQUESTS_CA_BUNDLE") if not cafile and not registry_config.cert: raise EnvironmentError( "SSL_CERT_FILE or REQUESTS_CA_BUNDLE environment variable must be set to use secure TLS or set the cert parameter in feature_Store.yaml file under remote registry configuration." ) - with open( registry_config.cert if registry_config.cert else cafile, "rb" ) as cert_file: diff --git a/sdk/python/tests/utils/auth_permissions_util.py b/sdk/python/tests/utils/auth_permissions_util.py index a223cef176d..8a1e7b7c4d7 100644 --- a/sdk/python/tests/utils/auth_permissions_util.py +++ b/sdk/python/tests/utils/auth_permissions_util.py @@ -134,13 +134,13 @@ def get_remote_registry_store(server_port, feature_store, tls_mode): registry_config = RemoteRegistryConfig( registry_type="remote", path=f"localhost:{server_port}", - is_tls_mode=True, + is_tls=True, ) else: registry_config = RemoteRegistryConfig( registry_type="remote", path=f"localhost:{server_port}", - is_tls_mode=True, + is_tls=True, cert=tls_cert_path, ) else: From 9b6f8e5d04a0bcff7f141c4b2a1019f6a5daa3fb Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Tue, 17 Dec 2024 14:34:53 -0500 Subject: [PATCH 11/17] Adding the existing trust store certificates to the newly created trust store. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- .../tests/utils/ssl_certifcates_util.py | 29 ++++++++++++++++--- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/sdk/python/tests/utils/ssl_certifcates_util.py b/sdk/python/tests/utils/ssl_certifcates_util.py index eb0430e98be..92adcb58825 100644 --- a/sdk/python/tests/utils/ssl_certifcates_util.py +++ b/sdk/python/tests/utils/ssl_certifcates_util.py @@ -1,5 +1,7 @@ import ipaddress import logging +import os +import shutil from datetime import datetime, timedelta from cryptography import x509 @@ -84,14 +86,33 @@ def create_ca_trust_store( public_key_path: str, private_key_path: str, output_trust_store_path: str ): """ - Create a CA trust store file and add the public certificate to it. + Create a new CA trust store as a copy of the existing one (if available), + and add the provided public certificate to it. :param public_key_path: Path to the public certificate (e.g., PEM file). :param private_key_path: Path to the private key (optional, to verify signing authority). - :param output_trust_store_path: Path to save the trust store. + :param output_trust_store_path: Path to save the new trust store. """ try: - # Read and load the public certificate + # Step 1: Identify the existing trust store (if available via environment variables) + existing_trust_store = os.environ.get("SSL_CERT_FILE") or os.environ.get( + "REQUESTS_CA_BUNDLE" + ) + + # Step 2: Copy the existing trust store to the new location (if it exists) + # Step 2: Copy the existing trust store to the new location (if it exists) + if existing_trust_store and os.path.exists(existing_trust_store): + shutil.copy(existing_trust_store, output_trust_store_path) + logger.info( + f"Copied existing trust store from {existing_trust_store} to {output_trust_store_path}" + ) + else: + # Log the creation of a new trust store (without opening a file unnecessarily) + logger.info( + f"No existing trust store found. Creating a new trust store at {output_trust_store_path}" + ) + + # Step 3: Load and validate the public certificate with open(public_key_path, "rb") as pub_file: public_cert_data = pub_file.read() public_cert = load_pem_x509_certificate( @@ -114,7 +135,7 @@ def create_ca_trust_store( "Public certificate does not match the private key." ) - # Create or append to the trust store + # Step 4: Add the public certificate to the new trust store with open(output_trust_store_path, "ab") as trust_store_file: trust_store_file.write(public_cert.public_bytes(serialization.Encoding.PEM)) From 4a5de3bc7635910aabdc669f1d91da2eb8e839c9 Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Tue, 17 Dec 2024 17:00:39 -0500 Subject: [PATCH 12/17] Clearing the existing trust store configuration to see if it fixes the PR integration failures. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/tests/conftest.py | 5 +++- .../tests/utils/ssl_certifcates_util.py | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index a12d10a7dcd..14cba36d62e 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -18,6 +18,7 @@ import tempfile from datetime import timedelta from multiprocessing import Process +from pathlib import Path from sys import platform from textwrap import dedent from typing import Any, Dict, List, Tuple, no_type_check @@ -60,7 +61,7 @@ from tests.utils.http_server import check_port_open, free_port # noqa: E402 from tests.utils.ssl_certifcates_util import ( create_ca_trust_store, - generate_self_signed_cert, + generate_self_signed_cert, save_certificates_to_persistent_dir, clear_previous_cert_env_vars, ) logger = logging.getLogger(__name__) @@ -520,6 +521,8 @@ def auth_config(request, is_integration_test): @pytest.fixture(scope="module") def tls_mode(request): is_tls_mode = request.param[0] + # remove any existing environment variables if there are any + clear_previous_cert_env_vars() ca_trust_store_path = "" if is_tls_mode: diff --git a/sdk/python/tests/utils/ssl_certifcates_util.py b/sdk/python/tests/utils/ssl_certifcates_util.py index 92adcb58825..1ff5ae634d7 100644 --- a/sdk/python/tests/utils/ssl_certifcates_util.py +++ b/sdk/python/tests/utils/ssl_certifcates_util.py @@ -82,6 +82,30 @@ def generate_self_signed_cert( ) +def clear_previous_cert_env_vars(): + """ + Clear SSL_CERT_FILE and REQUESTS_CA_BUNDLE environment variables if they match FEAST_CA_CERT_FILE_PATH. + """ + # Fetch FEAST_CA_CERT_FILE_PATH value + feast_ca_cert_file_path = os.environ.get("FEAST_CA_CERT_FILE_PATH") + + if not feast_ca_cert_file_path: + print("FEAST_CA_CERT_FILE_PATH is not set. Skipping cleanup.") + return + + print(f"FEAST_CA_CERT_FILE_PATH: {feast_ca_cert_file_path}") + env_vars_to_check = ["SSL_CERT_FILE", "REQUESTS_CA_BUNDLE"] + + # Compare and clear the environment variables + for var in env_vars_to_check: + env_value = os.environ.get(var) + if env_value and env_value == feast_ca_cert_file_path: + del os.environ[var] + print(f"Cleared environment variable: {var}") + else: + print(f"Skipped clearing {var}. Current value: {env_value}") + + def create_ca_trust_store( public_key_path: str, private_key_path: str, output_trust_store_path: str ): From a6d3420ea575b4dbd986c527f0508c77407e338d Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Tue, 17 Dec 2024 17:00:39 -0500 Subject: [PATCH 13/17] Clearing the existing trust store configuration to see if it fixes the PR integration failures. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/tests/conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index 14cba36d62e..133e433f4e2 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -60,6 +60,7 @@ from tests.utils.auth_permissions_util import default_store from tests.utils.http_server import check_port_open, free_port # noqa: E402 from tests.utils.ssl_certifcates_util import ( + clear_previous_cert_env_vars, create_ca_trust_store, generate_self_signed_cert, save_certificates_to_persistent_dir, clear_previous_cert_env_vars, ) From 706e9b44193344222ad8a11d78966afd50ad9263 Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Tue, 17 Dec 2024 17:00:39 -0500 Subject: [PATCH 14/17] Clearing the existing trust store configuration to see if it fixes the PR integration failures. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/tests/conftest.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index 133e433f4e2..422d10fd8bd 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -18,7 +18,6 @@ import tempfile from datetime import timedelta from multiprocessing import Process -from pathlib import Path from sys import platform from textwrap import dedent from typing import Any, Dict, List, Tuple, no_type_check @@ -62,7 +61,7 @@ from tests.utils.ssl_certifcates_util import ( clear_previous_cert_env_vars, create_ca_trust_store, - generate_self_signed_cert, save_certificates_to_persistent_dir, clear_previous_cert_env_vars, + generate_self_signed_cert, ) logger = logging.getLogger(__name__) From 4970151eed6ae6a9d401c501ce1ea17e3279f2cf Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Tue, 17 Dec 2024 19:13:02 -0500 Subject: [PATCH 15/17] combining the default system ca store with the custom one to fix the integration tests. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- sdk/python/tests/conftest.py | 21 ++++++++++----- .../tests/utils/ssl_certifcates_util.py | 27 +++++++++++++++++++ 2 files changed, 42 insertions(+), 6 deletions(-) diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index 422d10fd8bd..443926404f5 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -59,7 +59,7 @@ from tests.utils.auth_permissions_util import default_store from tests.utils.http_server import check_port_open, free_port # noqa: E402 from tests.utils.ssl_certifcates_util import ( - clear_previous_cert_env_vars, + combine_trust_stores, create_ca_trust_store, generate_self_signed_cert, ) @@ -522,8 +522,8 @@ def auth_config(request, is_integration_test): def tls_mode(request): is_tls_mode = request.param[0] # remove any existing environment variables if there are any - clear_previous_cert_env_vars() - ca_trust_store_path = "" + # clear_previous_cert_env_vars() + output_combined_truststore_path = "" if is_tls_mode: certificates_path = tempfile.mkdtemp() @@ -533,14 +533,23 @@ def tls_mode(request): generate_self_signed_cert(cert_path=tls_cert_path, key_path=tls_key_path) is_ca_trust_store_set = request.param[1] if is_ca_trust_store_set: - ca_trust_store_path = os.path.join(certificates_path, "ca_trust_store.pem") + # Paths + feast_ca_trust_store_path = os.path.join( + certificates_path, "feast_trust_store.pem" + ) create_ca_trust_store( public_key_path=tls_cert_path, private_key_path=tls_key_path, - output_trust_store_path=ca_trust_store_path, + output_trust_store_path=feast_ca_trust_store_path, + ) + + # Combine trust stores + output_combined_path = os.path.join( + certificates_path, "combined_trust_store.pem" ) + combine_trust_stores(feast_ca_trust_store_path, output_combined_path) else: tls_key_path = "" tls_cert_path = "" - return is_tls_mode, tls_key_path, tls_cert_path, ca_trust_store_path + return is_tls_mode, tls_key_path, tls_cert_path, output_combined_truststore_path diff --git a/sdk/python/tests/utils/ssl_certifcates_util.py b/sdk/python/tests/utils/ssl_certifcates_util.py index 1ff5ae634d7..e8dc0a483e9 100644 --- a/sdk/python/tests/utils/ssl_certifcates_util.py +++ b/sdk/python/tests/utils/ssl_certifcates_util.py @@ -4,6 +4,7 @@ import shutil from datetime import datetime, timedelta +import certifi from cryptography import x509 from cryptography.hazmat.backends import default_backend from cryptography.hazmat.primitives import hashes, serialization @@ -169,3 +170,29 @@ def create_ca_trust_store( except Exception as e: logger.error(f"Error creating CA trust store: {e}") + + +def combine_trust_stores(custom_cert_path: str, output_combined_path: str): + """ + Combine the default certifi CA bundle with a custom certificate file. + + :param custom_cert_path: Path to the custom certificate PEM file. + :param output_combined_path: Path where the combined CA bundle will be saved. + """ + try: + # Get the default certifi CA bundle + certifi_ca_bundle = certifi.where() + + with open(output_combined_path, "wb") as combined_file: + # Write the default CA bundle + with open(certifi_ca_bundle, "rb") as default_file: + combined_file.write(default_file.read()) + + # Append the custom certificates + with open(custom_cert_path, "rb") as custom_file: + combined_file.write(custom_file.read()) + + print(f"Combined trust store created at: {output_combined_path}") + + except Exception as e: + print(f"Error combining trust stores: {e}") From f9aea9a1660eb3d41efa9c3d102e8de34cc1518d Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Tue, 17 Dec 2024 20:09:56 -0500 Subject: [PATCH 16/17] Final clean up and adding documentation. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- .../starting-feast-servers-tls-mode.md | 5 ++++ sdk/python/feast/cli.py | 1 + sdk/python/feast/infra/registry/remote.py | 7 ++--- sdk/python/tests/conftest.py | 2 -- .../auth/server/test_auth_registry_server.py | 2 -- .../tests/utils/ssl_certifcates_util.py | 30 ++----------------- 6 files changed, 12 insertions(+), 35 deletions(-) diff --git a/docs/how-to-guides/starting-feast-servers-tls-mode.md b/docs/how-to-guides/starting-feast-servers-tls-mode.md index e1ddbc08be5..f5075fdab37 100644 --- a/docs/how-to-guides/starting-feast-servers-tls-mode.md +++ b/docs/how-to-guides/starting-feast-servers-tls-mode.md @@ -189,3 +189,8 @@ INFO: Waiting for application startup. INFO: Application startup complete. INFO: Uvicorn running on https://0.0.0.0:8888 (Press CTRL+C to quit) ``` + + +## Adding public key to CA trust store and configuring the feast to use the trust store. +You can pass the public key for SSL verification using `cert` parameter, however, it is sometimes a hassle to maintain individual certificate and pass the public certificate individually. +The alternate recommendation is to add the public certificate to CA trust store and set the path as environment variable `FEAST_CA_CERT_FILE_PATH`. Feast will refer the trust store path set as environment variable as `FEAST_CA_CERT_FILE_PATH` \ No newline at end of file diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py index 56c5752ed80..165677a843a 100644 --- a/sdk/python/feast/cli.py +++ b/sdk/python/feast/cli.py @@ -865,6 +865,7 @@ def materialize_incremental_command(ctx: click.Context, end_ts: str, views: List "cassandra", "hazelcast", "ikv", + "couchbase", ], case_sensitive=False, ), diff --git a/sdk/python/feast/infra/registry/remote.py b/sdk/python/feast/infra/registry/remote.py index f9177aeb243..c654a008aa0 100644 --- a/sdk/python/feast/infra/registry/remote.py +++ b/sdk/python/feast/infra/registry/remote.py @@ -61,8 +61,9 @@ class RemoteRegistryConfig(RegistryConfig): If registry_type is 'remote', then this configuration is needed to connect to remote registry server in TLS mode. If the remote registry started in non-tls mode then this configuration is not needed.""" is_tls: bool = False - """ str: Path to the public certificate when the registry server starts in TLS(SSL) mode. This may be needed if the registry server started with a self-signed certificate, typically this file ends with `*.crt`, `*.cer`, or `*.pem`. - If registry_type is 'remote', then this configuration is needed to connect to remote registry server in TLS mode. If the remote registry started in non-tls mode then this configuration is not needed.""" + """ bool: if you are planning to connect the registry server which started in TLS(SSL) mode then this should be true. + If you are planning to add the public certificate as part of the trust store instead of passing it as a `cert` parameters then setting this field to `true` is a mandatory. + """ class RemoteRegistry(BaseRegistry): @@ -75,8 +76,6 @@ def __init__( ): self.auth_config = auth_config assert isinstance(registry_config, RemoteRegistryConfig) - # self.channel = create_tls_channel(registry_config) - self.channel = self._create_grpc_channel(registry_config) auth_header_interceptor = GrpcClientAuthHeaderInterceptor(auth_config) diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index 443926404f5..6e5f1e14870 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -521,8 +521,6 @@ def auth_config(request, is_integration_test): @pytest.fixture(scope="module") def tls_mode(request): is_tls_mode = request.param[0] - # remove any existing environment variables if there are any - # clear_previous_cert_env_vars() output_combined_truststore_path = "" if is_tls_mode: diff --git a/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py b/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py index f67da3b4d30..0395f995410 100644 --- a/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py +++ b/sdk/python/tests/unit/permissions/auth/server/test_auth_registry_server.py @@ -46,8 +46,6 @@ def start_registry_server( is_tls_mode, tls_key_path, tls_cert_path, tls_ca_file_path = tls_mode if is_tls_mode: - # configure_ssl_ca(ca_file_path=tls_ca_file_path) - # Setting the ca_trust_store_path environment variables. print(f"Starting Registry in TLS mode at {server_port}") server = start_server( store=feature_store, diff --git a/sdk/python/tests/utils/ssl_certifcates_util.py b/sdk/python/tests/utils/ssl_certifcates_util.py index e8dc0a483e9..53a56e04f3d 100644 --- a/sdk/python/tests/utils/ssl_certifcates_util.py +++ b/sdk/python/tests/utils/ssl_certifcates_util.py @@ -83,30 +83,6 @@ def generate_self_signed_cert( ) -def clear_previous_cert_env_vars(): - """ - Clear SSL_CERT_FILE and REQUESTS_CA_BUNDLE environment variables if they match FEAST_CA_CERT_FILE_PATH. - """ - # Fetch FEAST_CA_CERT_FILE_PATH value - feast_ca_cert_file_path = os.environ.get("FEAST_CA_CERT_FILE_PATH") - - if not feast_ca_cert_file_path: - print("FEAST_CA_CERT_FILE_PATH is not set. Skipping cleanup.") - return - - print(f"FEAST_CA_CERT_FILE_PATH: {feast_ca_cert_file_path}") - env_vars_to_check = ["SSL_CERT_FILE", "REQUESTS_CA_BUNDLE"] - - # Compare and clear the environment variables - for var in env_vars_to_check: - env_value = os.environ.get(var) - if env_value and env_value == feast_ca_cert_file_path: - del os.environ[var] - print(f"Cleared environment variable: {var}") - else: - print(f"Skipped clearing {var}. Current value: {env_value}") - - def create_ca_trust_store( public_key_path: str, private_key_path: str, output_trust_store_path: str ): @@ -124,7 +100,6 @@ def create_ca_trust_store( "REQUESTS_CA_BUNDLE" ) - # Step 2: Copy the existing trust store to the new location (if it exists) # Step 2: Copy the existing trust store to the new location (if it exists) if existing_trust_store and os.path.exists(existing_trust_store): shutil.copy(existing_trust_store, output_trust_store_path) @@ -192,7 +167,8 @@ def combine_trust_stores(custom_cert_path: str, output_combined_path: str): with open(custom_cert_path, "rb") as custom_file: combined_file.write(custom_file.read()) - print(f"Combined trust store created at: {output_combined_path}") + logger.info(f"Combined trust store created at: {output_combined_path}") except Exception as e: - print(f"Error combining trust stores: {e}") + logger.error(f"Error combining trust stores: {e}") + raise e From 6084fb9165614bbaeba63e23b116425c794d97bf Mon Sep 17 00:00:00 2001 From: lrangine <19699092+lokeshrangineni@users.noreply.github.com> Date: Wed, 18 Dec 2024 09:58:06 -0500 Subject: [PATCH 17/17] Incorporating the code review comments from Francisco. Signed-off-by: lrangine <19699092+lokeshrangineni@users.noreply.github.com> --- docs/how-to-guides/starting-feast-servers-tls-mode.md | 4 ++-- sdk/python/feast/infra/registry/remote.py | 5 +++-- sdk/python/feast/ssl_ca_trust_store_setup.py | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/how-to-guides/starting-feast-servers-tls-mode.md b/docs/how-to-guides/starting-feast-servers-tls-mode.md index f5075fdab37..a868e17cf96 100644 --- a/docs/how-to-guides/starting-feast-servers-tls-mode.md +++ b/docs/how-to-guides/starting-feast-servers-tls-mode.md @@ -192,5 +192,5 @@ INFO: Uvicorn running on https://0.0.0.0:8888 (Press CTRL+C to quit) ## Adding public key to CA trust store and configuring the feast to use the trust store. -You can pass the public key for SSL verification using `cert` parameter, however, it is sometimes a hassle to maintain individual certificate and pass the public certificate individually. -The alternate recommendation is to add the public certificate to CA trust store and set the path as environment variable `FEAST_CA_CERT_FILE_PATH`. Feast will refer the trust store path set as environment variable as `FEAST_CA_CERT_FILE_PATH` \ No newline at end of file +You can pass the public key for SSL verification using the `cert` parameter, however, it is sometimes difficult to maintain individual certificates and pass them individually. +The alternative recommendation is to add the public certificate to CA trust store and set the path as an environment variable (e.g., `FEAST_CA_CERT_FILE_PATH`). Feast will use the trust store path in the `FEAST_CA_CERT_FILE_PATH` environment variable. \ No newline at end of file diff --git a/sdk/python/feast/infra/registry/remote.py b/sdk/python/feast/infra/registry/remote.py index c654a008aa0..590c0454b73 100644 --- a/sdk/python/feast/infra/registry/remote.py +++ b/sdk/python/feast/infra/registry/remote.py @@ -61,8 +61,9 @@ class RemoteRegistryConfig(RegistryConfig): If registry_type is 'remote', then this configuration is needed to connect to remote registry server in TLS mode. If the remote registry started in non-tls mode then this configuration is not needed.""" is_tls: bool = False - """ bool: if you are planning to connect the registry server which started in TLS(SSL) mode then this should be true. - If you are planning to add the public certificate as part of the trust store instead of passing it as a `cert` parameters then setting this field to `true` is a mandatory. + """ bool: Set to `True` if you plan to connect to a registry server running in TLS (SSL) mode. + If you intend to add the public certificate to the trust store instead of passing it via the `cert` parameter, this field must be set to `True`. + If you are planning to add the public certificate as part of the trust store instead of passing it as a `cert` parameters then setting this field to `true` is mandatory. """ diff --git a/sdk/python/feast/ssl_ca_trust_store_setup.py b/sdk/python/feast/ssl_ca_trust_store_setup.py index 396ff36703c..72e84132187 100644 --- a/sdk/python/feast/ssl_ca_trust_store_setup.py +++ b/sdk/python/feast/ssl_ca_trust_store_setup.py @@ -7,7 +7,7 @@ def configure_ca_trust_store_env_variables(): """ - configures the environment variable so that other libraries or servers refer the TLS ca file path. + configures the environment variable so that other libraries or servers refer to the TLS ca file path. :param ca_file_path: :return: """