From 8fef1944d9b6acb6b47a80d0fab27a724cf1bc5c Mon Sep 17 00:00:00 2001 From: cburroughs Date: Tue, 3 Sep 2024 03:01:41 -0400 Subject: [PATCH 1/2] build: Explicit protobuf build version; consistent build/setup deps (#4472) build: explicit protobuf build version; consistent build/setup deps Right now if one downloads `feast-0.40.1-py2.py3-none-any.whl` from PyPi it contains: ``` $ grep 'Protobuf Python Version' feast/protos/feast/registry/RegistryServer_pb2.py ``` Which is outside ``` $ grep 'protobuf<' feast-0.40.1.dist-info/METADATA Requires-Dist: protobuf<5.0.0,>=4.24.0 ``` Leading to runtime errors (#4437). This was mitigated by #4438. This change tightens this up further by: * Deleting the Makefile command that was trying to do this unsuccessfully. * Aligns the setup/build requirements * Sets the version of protobuf to match the *minimum* of the range. There is no guarantee that protos generated by `4.X` will work with `4.(X-1)`. Signed-off-by: Chris Burroughs --- .github/workflows/build_wheels.yml | 1 - Makefile | 3 --- environment-setup.md | 3 +-- pyproject.toml | 11 ++++++----- setup.py | 7 ++++--- 5 files changed, 11 insertions(+), 14 deletions(-) diff --git a/.github/workflows/build_wheels.yml b/.github/workflows/build_wheels.yml index f04015a9892..df8534d078e 100644 --- a/.github/workflows/build_wheels.yml +++ b/.github/workflows/build_wheels.yml @@ -97,7 +97,6 @@ jobs: # There's a `git restore` in here because `make install-go-ci-dependencies` is actually messing up go.mod & go.sum. run: | pip install -U pip setuptools wheel twine - make install-protoc-dependencies make build-ui git status git restore go.mod go.sum diff --git a/Makefile b/Makefile index 6ebab4e3be2..78a0b6d3285 100644 --- a/Makefile +++ b/Makefile @@ -395,9 +395,6 @@ test-trino-plugin-locally: kill-trino-locally: cd ${ROOT_DIR}; docker stop trino -install-protoc-dependencies: - pip install --ignore-installed protobuf==4.24.0 "grpcio-tools>=1.56.2,<2" mypy-protobuf==3.1.0 - # Docker build-docker: build-feature-server-python-aws-docker build-feature-transformation-server-docker build-feature-server-java-docker diff --git a/environment-setup.md b/environment-setup.md index 5dde9dfd942..581dc35f778 100644 --- a/environment-setup.md +++ b/environment-setup.md @@ -13,11 +13,10 @@ pip install cryptography -U conda install protobuf conda install pymssql pip install -e ".[dev]" -make install-protoc-dependencies PYTHON=3.9 make install-python-ci-dependencies PYTHON=3.9 ``` 4. start the docker daemon 5. run unit tests: ```bash make test-python-unit -``` \ No newline at end of file +``` diff --git a/pyproject.toml b/pyproject.toml index af448615029..15921e633cd 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,13 +1,14 @@ [build-system] requires = [ + "grpcio-tools>=1.56.2,<2", + "grpcio>=1.56.2,<2", + "mypy-protobuf==3.1", + "protobuf==4.24.0", + "pybindgen==0.22.0", "setuptools>=60", - "wheel", "setuptools_scm>=6.2", - "grpcio", - "grpcio-tools>=1.47.0", - "mypy-protobuf==3.1", - "protobuf>=4.24.0,<5.0.0", "sphinx!=4.0.0", + "wheel", ] build-backend = "setuptools.build_meta" diff --git a/setup.py b/setup.py index a9f9cafacc5..6da5e8226af 100644 --- a/setup.py +++ b/setup.py @@ -403,11 +403,12 @@ def run(self): entry_points={"console_scripts": ["feast=feast.cli:cli"]}, use_scm_version=use_scm_version, setup_requires=[ - "setuptools_scm", - "grpcio>=1.56.2,<2", "grpcio-tools>=1.56.2,<2", - "mypy-protobuf>=3.1", + "grpcio>=1.56.2,<2", + "mypy-protobuf==3.1", + "protobuf==4.24.0", "pybindgen==0.22.0", + "setuptools_scm>=6.2", ], cmdclass={ "build_python_protos": BuildPythonProtosCommand, From 3f3a4e852c3f508e38560e248c1ba68d64c4e799 Mon Sep 17 00:00:00 2001 From: lokeshrangineni <19699092+lokeshrangineni@users.noreply.github.com> Date: Wed, 4 Sep 2024 12:05:08 -0400 Subject: [PATCH 2/2] =?UTF-8?q?refactor:=20Making=20username=20and=20passw?= =?UTF-8?q?ord=20fields=20in=20OidcAuthModel=20as=20mandatory=20onl?= =?UTF-8?q?=E2=80=A6=20(#4460)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * squashed the last 23 commits and make username, password, client_Secret fields are required for oidc client configuration Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com> * squashed the last 23 commits and make username, password, client_Secret fields are required for oidc client configuration Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com> * Fixing the failing tests. Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com> * Fixing the Integration test failures. Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com> * Fixing the Integration test failures. Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com> * Removing the unnecessary configuration not needed after recent change. Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com> * Added client_secret also to calculate the oidc_client type calculation. Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com> --------- Signed-off-by: Lokesh Rangineni <19699092+lokeshrangineni@users.noreply.github.com> --- .../components/authz_manager.md | 20 ++++++-- sdk/python/feast/permissions/auth_model.py | 8 ++-- .../client/auth_client_manager_factory.py | 4 +- .../oidc_authentication_client_manager.py | 4 +- sdk/python/feast/permissions/server/utils.py | 5 +- sdk/python/feast/repo_config.py | 26 +++++++---- sdk/python/tests/conftest.py | 1 - .../feature_repos/repo_configuration.py | 11 ++--- .../universal/data_sources/file.py | 4 -- .../infra/scaffolding/test_repo_config.py | 46 ++++++++++++++----- .../tests/unit/permissions/auth/conftest.py | 5 +- .../unit/permissions/test_oidc_auth_client.py | 7 +-- 12 files changed, 90 insertions(+), 51 deletions(-) diff --git a/docs/getting-started/components/authz_manager.md b/docs/getting-started/components/authz_manager.md index 876dd84f2e3..0d011fbf2b3 100644 --- a/docs/getting-started/components/authz_manager.md +++ b/docs/getting-started/components/authz_manager.md @@ -61,24 +61,36 @@ For example, the access token for a client `app` of a user with `reader` role sh } ``` -An example of OIDC authorization configuration is the following: +An example of feast OIDC authorization configuration on the server side is the following: ```yaml project: my-project auth: type: oidc client_id: _CLIENT_ID__ - client_secret: _CLIENT_SECRET__ - realm: _REALM__ auth_discovery_url: _OIDC_SERVER_URL_/realms/master/.well-known/openid-configuration ... ``` -In case of client configuration, the following settings must be added to specify the current user: +In case of client configuration, the following settings username, password and client_secret must be added to specify the current user: ```yaml auth: + type: oidc ... username: _USERNAME_ password: _PASSWORD_ + client_secret: _CLIENT_SECRET__ +``` + +Below is an example of feast full OIDC client auth configuration: +```yaml +project: my-project +auth: + type: oidc + client_id: test_client_id + client_secret: test_client_secret + username: test_user_name + password: test_password + auth_discovery_url: http://localhost:8080/realms/master/.well-known/openid-configuration ``` ### Kubernetes RBAC Authorization diff --git a/sdk/python/feast/permissions/auth_model.py b/sdk/python/feast/permissions/auth_model.py index 28eeb951a78..a3a3b32a4b2 100644 --- a/sdk/python/feast/permissions/auth_model.py +++ b/sdk/python/feast/permissions/auth_model.py @@ -1,4 +1,4 @@ -from typing import Literal, Optional +from typing import Literal from feast.repo_config import FeastConfigBaseModel @@ -10,10 +10,12 @@ class AuthConfig(FeastConfigBaseModel): class OidcAuthConfig(AuthConfig): auth_discovery_url: str client_id: str - client_secret: Optional[str] = None + + +class OidcClientAuthConfig(OidcAuthConfig): username: str password: str - realm: str = "master" + client_secret: str class NoAuthConfig(AuthConfig): diff --git a/sdk/python/feast/permissions/client/auth_client_manager_factory.py b/sdk/python/feast/permissions/client/auth_client_manager_factory.py index 4e49802047e..3dff5fb45de 100644 --- a/sdk/python/feast/permissions/client/auth_client_manager_factory.py +++ b/sdk/python/feast/permissions/client/auth_client_manager_factory.py @@ -2,7 +2,7 @@ from feast.permissions.auth_model import ( AuthConfig, KubernetesAuthConfig, - OidcAuthConfig, + OidcClientAuthConfig, ) from feast.permissions.client.auth_client_manager import AuthenticationClientManager from feast.permissions.client.kubernetes_auth_client_manager import ( @@ -15,7 +15,7 @@ def get_auth_client_manager(auth_config: AuthConfig) -> AuthenticationClientManager: if auth_config.type == AuthType.OIDC.value: - assert isinstance(auth_config, OidcAuthConfig) + assert isinstance(auth_config, OidcClientAuthConfig) return OidcAuthClientManager(auth_config) elif auth_config.type == AuthType.KUBERNETES.value: assert isinstance(auth_config, KubernetesAuthConfig) diff --git a/sdk/python/feast/permissions/client/oidc_authentication_client_manager.py b/sdk/python/feast/permissions/client/oidc_authentication_client_manager.py index 0f99cea86f0..3ba1c1b6a77 100644 --- a/sdk/python/feast/permissions/client/oidc_authentication_client_manager.py +++ b/sdk/python/feast/permissions/client/oidc_authentication_client_manager.py @@ -4,7 +4,7 @@ import jwt import requests -from feast.permissions.auth_model import OidcAuthConfig +from feast.permissions.auth_model import OidcClientAuthConfig from feast.permissions.client.auth_client_manager import AuthenticationClientManager from feast.permissions.oidc_service import OIDCDiscoveryService @@ -12,7 +12,7 @@ class OidcAuthClientManager(AuthenticationClientManager): - def __init__(self, auth_config: OidcAuthConfig): + def __init__(self, auth_config: OidcClientAuthConfig): self.auth_config = auth_config def get_token(self): diff --git a/sdk/python/feast/permissions/server/utils.py b/sdk/python/feast/permissions/server/utils.py index 34a2c0024a8..ac70f187ce3 100644 --- a/sdk/python/feast/permissions/server/utils.py +++ b/sdk/python/feast/permissions/server/utils.py @@ -15,7 +15,10 @@ from feast.permissions.auth.oidc_token_parser import OidcTokenParser from feast.permissions.auth.token_extractor import TokenExtractor from feast.permissions.auth.token_parser import TokenParser -from feast.permissions.auth_model import AuthConfig, OidcAuthConfig +from feast.permissions.auth_model import ( + AuthConfig, + OidcAuthConfig, +) from feast.permissions.security_manager import ( SecurityManager, no_security_manager, diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index 199ef314123..52372f2987f 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -87,10 +87,13 @@ "local": "feast.infra.feature_servers.local_process.config.LocalFeatureServerConfig", } +ALLOWED_AUTH_TYPES = ["no_auth", "kubernetes", "oidc"] + AUTH_CONFIGS_CLASS_FOR_TYPE = { "no_auth": "feast.permissions.auth_model.NoAuthConfig", "kubernetes": "feast.permissions.auth_model.KubernetesAuthConfig", "oidc": "feast.permissions.auth_model.OidcAuthConfig", + "oidc_client": "feast.permissions.auth_model.OidcClientAuthConfig", } @@ -291,11 +294,17 @@ def offline_store(self): def auth_config(self): if not self._auth: if isinstance(self.auth, Dict): - self._auth = get_auth_config_from_type(self.auth.get("type"))( - **self.auth + is_oidc_client = ( + self.auth.get("type") == AuthType.OIDC.value + and "username" in self.auth + and "password" in self.auth + and "client_secret" in self.auth ) + self._auth = get_auth_config_from_type( + "oidc_client" if is_oidc_client else self.auth.get("type") + )(**self.auth) elif isinstance(self.auth, str): - self._auth = get_auth_config_from_type(self.auth.get("type"))() + self._auth = get_auth_config_from_type(self.auth)() elif self.auth: self._auth = self.auth @@ -336,22 +345,21 @@ def _validate_auth_config(cls, values: Any) -> Any: from feast.permissions.auth_model import AuthConfig if "auth" in values: - allowed_auth_types = AUTH_CONFIGS_CLASS_FOR_TYPE.keys() if isinstance(values["auth"], Dict): if values["auth"].get("type") is None: raise ValueError( - f"auth configuration is missing authentication type. Possible values={allowed_auth_types}" + f"auth configuration is missing authentication type. Possible values={ALLOWED_AUTH_TYPES}" ) - elif values["auth"]["type"] not in allowed_auth_types: + elif values["auth"]["type"] not in ALLOWED_AUTH_TYPES: raise ValueError( f'auth configuration has invalid authentication type={values["auth"]["type"]}. Possible ' - f'values={allowed_auth_types}' + f'values={ALLOWED_AUTH_TYPES}' ) elif isinstance(values["auth"], AuthConfig): - if values["auth"].type not in allowed_auth_types: + if values["auth"].type not in ALLOWED_AUTH_TYPES: raise ValueError( f'auth configuration has invalid authentication type={values["auth"].type}. Possible ' - f'values={allowed_auth_types}' + f'values={ALLOWED_AUTH_TYPES}' ) return values diff --git a/sdk/python/tests/conftest.py b/sdk/python/tests/conftest.py index b5b3e2d9e57..5e70da074cb 100644 --- a/sdk/python/tests/conftest.py +++ b/sdk/python/tests/conftest.py @@ -466,7 +466,6 @@ def is_integration_test(all_markers_from_module): client_secret: feast-integration-client-secret username: reader_writer password: password - realm: master auth_discovery_url: KEYCLOAK_URL_PLACE_HOLDER/realms/master/.well-known/openid-configuration """), ], diff --git a/sdk/python/tests/integration/feature_repos/repo_configuration.py b/sdk/python/tests/integration/feature_repos/repo_configuration.py index 660a937f5af..73f99fb7c28 100644 --- a/sdk/python/tests/integration/feature_repos/repo_configuration.py +++ b/sdk/python/tests/integration/feature_repos/repo_configuration.py @@ -28,7 +28,7 @@ ) from feast.infra.feature_servers.local_process.config import LocalFeatureServerConfig from feast.permissions.action import AuthzedAction -from feast.permissions.auth_model import OidcAuthConfig +from feast.permissions.auth_model import OidcClientAuthConfig from feast.permissions.permission import Permission from feast.permissions.policy import RoleBasedPolicy from feast.repo_config import RegistryConfig, RepoConfig @@ -447,15 +447,14 @@ class OfflineServerPermissionsEnvironment(Environment): def setup(self): self.data_source_creator.setup(self.registry) keycloak_url = self.data_source_creator.get_keycloak_url() - auth_config = OidcAuthConfig( + auth_config = OidcClientAuthConfig( client_id="feast-integration-client", - client_secret="feast-integration-client-secret", - username="reader_writer", - password="password", - realm="master", type="oidc", auth_discovery_url=f"{keycloak_url}/realms/master/.well-known" f"/openid-configuration", + client_secret="feast-integration-client-secret", + username="reader_writer", + password="password", ) self.config = RepoConfig( registry=self.registry, 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 10d348c0564..d8b75aca248 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 @@ -445,10 +445,6 @@ def __init__(self, project_name: str, *args, **kwargs): auth: type: oidc client_id: feast-integration-client - client_secret: feast-integration-client-secret - username: reader_writer - password: password - realm: master auth_discovery_url: {keycloak_url}/realms/master/.well-known/openid-configuration """ self.auth_config = auth_config_template.format(keycloak_url=self.keycloak_url) diff --git a/sdk/python/tests/unit/infra/scaffolding/test_repo_config.py b/sdk/python/tests/unit/infra/scaffolding/test_repo_config.py index 5331d350e2e..9dcf7e4caf6 100644 --- a/sdk/python/tests/unit/infra/scaffolding/test_repo_config.py +++ b/sdk/python/tests/unit/infra/scaffolding/test_repo_config.py @@ -9,6 +9,7 @@ KubernetesAuthConfig, NoAuthConfig, OidcAuthConfig, + OidcClientAuthConfig, ) from feast.repo_config import FeastConfigError, load_repo_config @@ -213,7 +214,6 @@ def test_auth_config(): client_secret: test_client_secret username: test_user_name password: test_password - realm: master auth_discovery_url: http://localhost:8080/realms/master/.well-known/openid-configuration registry: "registry.db" provider: local @@ -235,7 +235,6 @@ def test_auth_config(): client_secret: test_client_secret username: test_user_name password: test_password - realm: master auth_discovery_url: http://localhost:8080/realms/master/.well-known/openid-configuration registry: "registry.db" provider: local @@ -247,7 +246,32 @@ def test_auth_config(): expect_error="invalid authentication type=not_valid_auth_type", ) - oidc_repo_config = _test_config( + oidc_server_repo_config = _test_config( + dedent( + """ + project: foo + auth: + type: oidc + client_id: test_client_id + auth_discovery_url: http://localhost:8080/realms/master/.well-known/openid-configuration + registry: "registry.db" + provider: local + online_store: + path: foo + entity_key_serialization_version: 2 + """ + ), + expect_error=None, + ) + assert oidc_server_repo_config.auth["type"] == AuthType.OIDC.value + assert isinstance(oidc_server_repo_config.auth_config, OidcAuthConfig) + assert oidc_server_repo_config.auth_config.client_id == "test_client_id" + assert ( + oidc_server_repo_config.auth_config.auth_discovery_url + == "http://localhost:8080/realms/master/.well-known/openid-configuration" + ) + + oidc_client_repo_config = _test_config( dedent( """ project: foo @@ -257,7 +281,6 @@ def test_auth_config(): client_secret: test_client_secret username: test_user_name password: test_password - realm: master auth_discovery_url: http://localhost:8080/realms/master/.well-known/openid-configuration registry: "registry.db" provider: local @@ -268,15 +291,14 @@ def test_auth_config(): ), expect_error=None, ) - assert oidc_repo_config.auth["type"] == AuthType.OIDC.value - assert isinstance(oidc_repo_config.auth_config, OidcAuthConfig) - assert oidc_repo_config.auth_config.client_id == "test_client_id" - assert oidc_repo_config.auth_config.client_secret == "test_client_secret" - assert oidc_repo_config.auth_config.username == "test_user_name" - assert oidc_repo_config.auth_config.password == "test_password" - assert oidc_repo_config.auth_config.realm == "master" + assert oidc_client_repo_config.auth["type"] == AuthType.OIDC.value + assert isinstance(oidc_client_repo_config.auth_config, OidcClientAuthConfig) + assert oidc_client_repo_config.auth_config.client_id == "test_client_id" + assert oidc_client_repo_config.auth_config.client_secret == "test_client_secret" + assert oidc_client_repo_config.auth_config.username == "test_user_name" + assert oidc_client_repo_config.auth_config.password == "test_password" assert ( - oidc_repo_config.auth_config.auth_discovery_url + oidc_client_repo_config.auth_config.auth_discovery_url == "http://localhost:8080/realms/master/.well-known/openid-configuration" ) diff --git a/sdk/python/tests/unit/permissions/auth/conftest.py b/sdk/python/tests/unit/permissions/auth/conftest.py index 0d6acd7fb22..ea6e2e43117 100644 --- a/sdk/python/tests/unit/permissions/auth/conftest.py +++ b/sdk/python/tests/unit/permissions/auth/conftest.py @@ -75,10 +75,7 @@ def oidc_config() -> OidcAuthConfig: return OidcAuthConfig( auth_discovery_url="https://localhost:8080/realms/master/.well-known/openid-configuration", client_id=_CLIENT_ID, - client_secret="", - username="", - password="", - realm="", + type="oidc", ) diff --git a/sdk/python/tests/unit/permissions/test_oidc_auth_client.py b/sdk/python/tests/unit/permissions/test_oidc_auth_client.py index 22ed5b6f87d..68aec70fc79 100644 --- a/sdk/python/tests/unit/permissions/test_oidc_auth_client.py +++ b/sdk/python/tests/unit/permissions/test_oidc_auth_client.py @@ -5,7 +5,7 @@ from feast.permissions.auth_model import ( KubernetesAuthConfig, NoAuthConfig, - OidcAuthConfig, + OidcClientAuthConfig, ) from feast.permissions.client.http_auth_requests_wrapper import ( AuthenticatedRequestsSession, @@ -21,13 +21,14 @@ MOCKED_TOKEN_VALUE: str = "dummy_token" -def _get_dummy_oidc_auth_type() -> OidcAuthConfig: - oidc_config = OidcAuthConfig( +def _get_dummy_oidc_auth_type() -> OidcClientAuthConfig: + oidc_config = OidcClientAuthConfig( auth_discovery_url="http://localhost:8080/realms/master/.well-known/openid-configuration", type="oidc", username="admin_test", password="password_test", client_id="dummy_client_id", + client_secret="client_secret", ) return oidc_config