Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Loading the CA trusted store certificate into Feast to verify the public certificate. #4852

Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
f11ce84
Initial Draft version to load the CA trusted store code.
lokeshrangineni Dec 14, 2024
cc40246
Initial Draft version to load the CA trusted store code.
lokeshrangineni Dec 14, 2024
eda6900
Fixing the lint error.
lokeshrangineni Dec 14, 2024
abf4c7e
Trying to fix the online store test cases.
lokeshrangineni Dec 17, 2024
d497a9c
Merge branch 'master' into feature/adding-ca-store-support
lokeshrangineni Dec 17, 2024
767f241
Formatted the python to fix lint errors.
lokeshrangineni Dec 17, 2024
436f0db
Fixing the unit test cases.
lokeshrangineni Dec 17, 2024
1d64ebb
Fixing the unit test cases.
lokeshrangineni Dec 17, 2024
9540e7e
removing unnecessary cli args.
lokeshrangineni Dec 17, 2024
fff05f4
Now configuring the SSL ca store configurations on the feast client s…
lokeshrangineni Dec 17, 2024
36859bf
Renamed the remote registry is_tls_mode variable to is_tls.
lokeshrangineni Dec 17, 2024
9b6f8e5
Adding the existing trust store certificates to the newly created tru…
lokeshrangineni Dec 17, 2024
4a5de3b
Clearing the existing trust store configuration to see if it fixes th…
lokeshrangineni Dec 17, 2024
a6d3420
Clearing the existing trust store configuration to see if it fixes th…
lokeshrangineni Dec 17, 2024
706e9b4
Clearing the existing trust store configuration to see if it fixes th…
lokeshrangineni Dec 17, 2024
4970151
combining the default system ca store with the custom one to fix the …
lokeshrangineni Dec 18, 2024
f9aea9a
Final clean up and adding documentation.
lokeshrangineni Dec 18, 2024
6084fb9
Incorporating the code review comments from Francisco.
lokeshrangineni Dec 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/how-to-guides/starting-feast-servers-tls-mode.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
lokeshrangineni marked this conversation as resolved.
Show resolved Hide resolved
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`
lokeshrangineni marked this conversation as resolved.
Show resolved Hide resolved
1 change: 0 additions & 1 deletion sdk/python/feast/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -982,7 +982,6 @@ def serve_command(
raise click.BadParameter(
"Please pass --cert and --key args to start the feature server in TLS mode."
)

store = create_feature_store(ctx)

store.serve(
Expand Down
13 changes: 11 additions & 2 deletions sdk/python/feast/feature_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -1949,13 +1952,19 @@ 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 = "",
) -> 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,
)

def serve_offline(
Expand Down
2 changes: 1 addition & 1 deletion sdk/python/feast/infra/offline_stores/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."
)
Expand Down
36 changes: 27 additions & 9 deletions sdk/python/feast/infra/registry/remote.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import os
from datetime import datetime
from pathlib import Path
from typing import List, Optional, Union
Expand Down Expand Up @@ -59,6 +60,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: bool = False
""" bool: if you are planning to connect the registry server which started in TLS(SSL) mode then this should be true.
lokeshrangineni marked this conversation as resolved.
Show resolved Hide resolved
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.
lokeshrangineni marked this conversation as resolved.
Show resolved Hide resolved
"""


class RemoteRegistry(BaseRegistry):
def __init__(
Expand All @@ -70,20 +76,32 @@ 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 = 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:
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
)
return grpc.secure_channel(registry_config.path, tls_credentials)
else:
# Create an insecure gRPC channel
return grpc.insecure_channel(registry_config.path)

def close(self):
if self.channel:
self.channel.close()
Expand Down
22 changes: 22 additions & 0 deletions sdk/python/feast/ssl_ca_trust_store_setup.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import logging
import os

logger = logging.getLogger(__name__)
logger.setLevel(logging.INFO)


def configure_ca_trust_store_env_variables():
"""
configures the environment variable so that other libraries or servers refer the TLS ca file path.
lokeshrangineni marked this conversation as resolved.
Show resolved Hide resolved
:param ca_file_path:
:return:
"""
if (
"FEAST_CA_CERT_FILE_PATH" in os.environ
and os.environ["FEAST_CA_CERT_FILE_PATH"]
):
logger.info(
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"]
31 changes: 27 additions & 4 deletions sdk/python/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,12 @@
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 (
combine_trust_stores,
create_ca_trust_store,
generate_self_signed_cert,
)

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -514,17 +518,36 @@ 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]
output_combined_truststore_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:
# 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=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
return is_tls_mode, tls_key_path, tls_cert_path, output_combined_truststore_path
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@
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
from tests.utils.ssl_certifcates_util import generate_self_signed_cert

logger = logging.getLogger(__name__)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@


@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,
Expand Down Expand Up @@ -56,13 +59,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(
Expand Down Expand Up @@ -172,14 +175,15 @@ 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(
repo_path=str(store.repo_path),
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:
Expand All @@ -203,20 +207,33 @@ 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,
)
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(
repo_path=str(repo_path),
registry_path=server_registry_path,
feature_server_url=feature_server_url,
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)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ 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:
print(f"Starting Registry in TLS mode at {server_port}")
server = start_server(
Expand Down Expand Up @@ -74,6 +74,9 @@ 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,
Expand Down
25 changes: 19 additions & 6 deletions sdk/python/tests/utils/auth_permissions_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -127,18 +128,30 @@ 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=True,
)
else:
registry_config = RemoteRegistryConfig(
registry_type="remote",
path=f"localhost:{server_port}",
is_tls=True,
cert=tls_cert_path,
)
else:
registry_config = RemoteRegistryConfig(
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,
Expand Down
Loading
Loading