diff --git a/docs/getting-started/concepts/permission.md b/docs/getting-started/concepts/permission.md index 71e9efc629..ee25ef973e 100644 --- a/docs/getting-started/concepts/permission.md +++ b/docs/getting-started/concepts/permission.md @@ -42,6 +42,11 @@ The `Permission` class identifies a single permission configured on the feature - `actions`: The actions authorized by this permission. Defaults to `ALL_VALUES`, an alias defined in the `action` module. - `policy`: The policy to be applied to validate a client request. +The `decision strategy` parameter defines the strategy to be applied when multiple permissions match an execution request, it defaults to `UNANIMOUS`. and the possible values are: +- `UNANIMOUS`: All policies must evaluate to a positive decision for the final decision to be also positive. +- `AFFIRMATIVE`: At least one policy must evaluate to a positive decision +- `UNANIMOUS`: The number of positive decisions must be greater than the number of negative decisions. + Given the above definitions, the feature store can be configured with granular control over each resource, enabling partitioned access by teams to meet organizational requirements for service and data sharing, and protection of sensitive information. diff --git a/protos/feast/core/Registry.proto b/protos/feast/core/Registry.proto index b4f1ffb0a3..8f434ef76a 100644 --- a/protos/feast/core/Registry.proto +++ b/protos/feast/core/Registry.proto @@ -56,6 +56,15 @@ message Registry { } message ProjectMetadata { + enum DecisionStrategy{ + UNANIMOUS = 0; + AFFIRMATIVE = 1; + CONSENSUS = 2; + } + string project = 1; string project_uuid = 2; + DecisionStrategy decision_strategy = 3; } + + diff --git a/protos/feast/registry/RegistryServer.proto b/protos/feast/registry/RegistryServer.proto index 928354077b..d4927be5bf 100644 --- a/protos/feast/registry/RegistryServer.proto +++ b/protos/feast/registry/RegistryServer.proto @@ -74,7 +74,8 @@ service RegistryServer{ rpc Commit (google.protobuf.Empty) returns (google.protobuf.Empty) {} rpc Refresh (RefreshRequest) returns (google.protobuf.Empty) {} rpc Proto (google.protobuf.Empty) returns (feast.core.Registry) {} - + rpc SetDecisionStrategy (SetDecisionStrategyRequest) returns (google.protobuf.Empty) {} + rpc GetDecisionStrategy (GetDecisionStrategyRequest) returns (GetDecisionStrategyResponse) {} } message RefreshRequest { @@ -356,3 +357,17 @@ message DeletePermissionRequest { string project = 2; bool commit = 3; } + +message SetDecisionStrategyRequest { + feast.core.ProjectMetadata.DecisionStrategy decision_strategy = 1; + string project = 2; + bool commit = 3; +} + +message GetDecisionStrategyRequest { + string project = 1; +} + +message GetDecisionStrategyResponse { + feast.core.ProjectMetadata.DecisionStrategy decision_strategy = 1; +} diff --git a/sdk/python/feast/cli.py b/sdk/python/feast/cli.py index 672a5571e0..88d58ca53b 100644 --- a/sdk/python/feast/cli.py +++ b/sdk/python/feast/cli.py @@ -36,7 +36,6 @@ from feast.errors import FeastObjectNotFoundException, FeastProviderLoginError from feast.feature_view import FeatureView from feast.on_demand_feature_view import OnDemandFeatureView -from feast.permissions.permission import Permission from feast.permissions.policy import RoleBasedPolicy from feast.repo_config import load_repo_config from feast.repo_operations import ( @@ -990,7 +989,7 @@ def feast_permissions_list_command(ctx: click.Context, verbose: bool, tags: list ) ) print( - f"{Style.BRIGHT}Note:{Style.RESET_ALL}The configured decision strategy is {Permission.get_global_decision_strategy().value.upper()}" + f"{Style.BRIGHT}Note:{Style.RESET_ALL}The configured decision strategy is {store.registry.list_project_metadata(project=store.project)[0].decision_strategy.value.upper()}" ) else: cli_utils.print_permission_verbose_example() @@ -1111,7 +1110,7 @@ def feast_permissions_list_roles_command(ctx: click.Context, verbose: bool): store=store, ) cli_utils.handler_list_all_permissions_roles_verbose( - objects=objects, permissions=permissions, table=table + store=store, objects=objects, permissions=permissions, table=table ) print( tabulate( @@ -1127,7 +1126,7 @@ def feast_permissions_list_roles_command(ctx: click.Context, verbose: bool): ) print( - f"{Style.BRIGHT}Note:{Style.RESET_ALL}The configured decision strategy is {Permission.get_global_decision_strategy().value.upper()}" + f"{Style.BRIGHT}Note:{Style.RESET_ALL}The configured decision strategy is {store.registry.list_project_metadata(project=store.project)[0].decision_strategy.value.upper()}" ) diff --git a/sdk/python/feast/cli_utils.py b/sdk/python/feast/cli_utils.py index 5155eb814f..e5cf6e4340 100644 --- a/sdk/python/feast/cli_utils.py +++ b/sdk/python/feast/cli_utils.py @@ -13,7 +13,7 @@ ) from feast.feast_object import FeastObject from feast.permissions.action import ALL_ACTIONS -from feast.permissions.decision import DecisionEvaluator +from feast.permissions.decision import DecisionEvaluator, DecisionStrategy from feast.permissions.permission import Permission from feast.permissions.policy import Policy, RoleBasedPolicy from feast.permissions.user import User @@ -271,7 +271,10 @@ def handler_list_all_permissions_roles(permissions: list[Permission], table: lis def handler_list_all_permissions_roles_verbose( - objects: list[FeastObject], permissions: list[Permission], table: list[Any] + store: FeatureStore, + objects: list[FeastObject], + permissions: list[Permission], + table: list[Any], ): all_roles = fetch_all_permission_roles(permissions) @@ -288,7 +291,7 @@ def handler_list_all_permissions_roles_verbose( if matching_permissions: evaluator = DecisionEvaluator( - Permission.get_global_decision_strategy(), + get_global_decision_strategy(store), len(matching_permissions), ) for p in matching_permissions: @@ -316,3 +319,7 @@ def handler_list_all_permissions_roles_verbose( [a.value.upper() for a in permitted_actions], ] ) + + +def get_global_decision_strategy(store: FeatureStore) -> DecisionStrategy: + return store.get_decision_strategy(store.project) diff --git a/sdk/python/feast/errors.py b/sdk/python/feast/errors.py index ffafe31125..7be687c752 100644 --- a/sdk/python/feast/errors.py +++ b/sdk/python/feast/errors.py @@ -419,3 +419,8 @@ def __init__(self, query: str): class ZeroColumnQueryResult(Exception): def __init__(self, query: str): super().__init__(f"This query returned zero columns:\n{query}") + + +class DecisionStrategyNotFound(FeastObjectNotFoundException): + def __init__(self, project: str): + super().__init__(f"DecisionStrategy does not exist in project {project}") diff --git a/sdk/python/feast/feature_store.py b/sdk/python/feast/feature_store.py index 570948b6f5..2a2e06d8ba 100644 --- a/sdk/python/feast/feature_store.py +++ b/sdk/python/feast/feature_store.py @@ -76,6 +76,7 @@ from feast.infra.registry.sql import SqlRegistry from feast.on_demand_feature_view import OnDemandFeatureView from feast.online_response import OnlineResponse +from feast.permissions.decision import DecisionStrategy from feast.permissions.permission import Permission from feast.protos.feast.core.InfraObject_pb2 import Infra as InfraProto from feast.protos.feast.serving.ServingService_pb2 import ( @@ -2005,6 +2006,27 @@ def list_saved_datasets( self.project, allow_cache=allow_cache, tags=tags ) + def set_decision_strategy(self, decision_strategy: DecisionStrategy): + """ + Set the project decision strategy. + + Args: + decision_strategy: The decision strategy to set to. + """ + return self._registry.set_decision_strategy(self.project, decision_strategy) + + def get_decision_strategy(self, project: str) -> DecisionStrategy: + """ + Get the project decision strategy. + + Args: + project: The project to get the decision strategy for. + + Returns: + The decision strategy or None. + """ + return self._registry.get_decision_strategy(project) + def _print_materialization_log( start_date, end_date, num_feature_views: int, online_store: str diff --git a/sdk/python/feast/infra/registry/base_registry.py b/sdk/python/feast/infra/registry/base_registry.py index 5ba9cbedb9..2fa59970a9 100644 --- a/sdk/python/feast/infra/registry/base_registry.py +++ b/sdk/python/feast/infra/registry/base_registry.py @@ -28,6 +28,7 @@ from feast.feature_view import FeatureView from feast.infra.infra_object import Infra from feast.on_demand_feature_view import OnDemandFeatureView +from feast.permissions.decision import DecisionStrategy from feast.permissions.permission import Permission from feast.project_metadata import ProjectMetadata from feast.protos.feast.core.Entity_pb2 import Entity as EntityProto @@ -812,3 +813,26 @@ def deserialize_registry_values(serialized_proto, feast_obj_type) -> Any: if feast_obj_type == FeatureService: return FeatureServiceProto.FromString(serialized_proto) return None + + @abstractmethod + def set_decision_strategy(self, project: str, decision_strategy: DecisionStrategy): + """ + Set the project decision strategy. + + Args: + project: Feast project to set the decision strategy for + decision_strategy: The decision strategy to set for the project + """ + raise NotImplementedError + + def get_decision_strategy(self, project: str) -> DecisionStrategy: + """ + Get the project decision strategy. + + Args: + project: The project to get the decision strategy for. + + Returns: + The decision strategy or None. + """ + raise NotImplementedError diff --git a/sdk/python/feast/infra/registry/registry.py b/sdk/python/feast/infra/registry/registry.py index fa9d436400..16cefb5510 100644 --- a/sdk/python/feast/infra/registry/registry.py +++ b/sdk/python/feast/infra/registry/registry.py @@ -28,6 +28,7 @@ from feast.errors import ( ConflictingFeatureViewNames, DataSourceNotFoundException, + DecisionStrategyNotFound, EntityNotFoundException, FeatureServiceNotFoundException, FeatureViewNotFoundException, @@ -43,8 +44,10 @@ from feast.infra.registry.registry_store import NoopRegistryStore from feast.on_demand_feature_view import OnDemandFeatureView from feast.permissions.auth_model import AuthConfig, NoAuthConfig +from feast.permissions.decision import DecisionStrategy from feast.permissions.permission import Permission from feast.project_metadata import ProjectMetadata +from feast.protos.feast.core.Registry_pb2 import ProjectMetadata as ProjectMetadataProto from feast.protos.feast.core.Registry_pb2 import Registry as RegistryProto from feast.repo_config import RegistryConfig from feast.repo_contents import RepoContents @@ -966,3 +969,37 @@ def delete_permission(self, name: str, project: str, commit: bool = True): self.commit() return raise PermissionNotFoundException(name, project) + + def set_decision_strategy(self, project: str, decision_strategy: DecisionStrategy): + registry = self._prepare_registry_for_changes(project) + + project_metadata = None + for idx, existing_project_metadata in enumerate(registry.project_metadata): + if existing_project_metadata.project == project: + del registry.project_metadata[idx] + project_metadata = existing_project_metadata + break + + if project_metadata: + project_metadata.decision_strategy = ( + ProjectMetadataProto.DecisionStrategy.Value(decision_strategy.name) + ) + + registry.project_metadata.append(project_metadata) + + self.commit() + + def get_decision_strategy(self, project: str) -> DecisionStrategy: + registry_proto = self._get_registry_proto(project=project) + + for idx, existing_project_metadata in enumerate( + registry_proto.project_metadata + ): + if existing_project_metadata.project == project: + return DecisionStrategy[ + ProjectMetadataProto.DecisionStrategy.Name( + existing_project_metadata.decision_strategy + ) + ] + + raise DecisionStrategyNotFound(project=project) diff --git a/sdk/python/feast/infra/registry/remote.py b/sdk/python/feast/infra/registry/remote.py index 70c2af4c40..0bba7026ff 100644 --- a/sdk/python/feast/infra/registry/remote.py +++ b/sdk/python/feast/infra/registry/remote.py @@ -23,8 +23,10 @@ from feast.permissions.client.utils import ( create_auth_header, ) +from feast.permissions.decision import DecisionStrategy from feast.permissions.permission import Permission from feast.project_metadata import ProjectMetadata +from feast.protos.feast.core.Registry_pb2 import ProjectMetadata as ProjectMetadataProto from feast.protos.feast.core.Registry_pb2 import Registry as RegistryProto from feast.protos.feast.registry import RegistryServer_pb2, RegistryServer_pb2_grpc from feast.repo_config import RegistryConfig @@ -681,3 +683,29 @@ def refresh(self, project: Optional[str] = None): def teardown(self): pass + + def set_decision_strategy(self, project: str, decision_strategy: DecisionStrategy): + decision_strategy_proto = ProjectMetadataProto.DecisionStrategy.Value( + decision_strategy.name + ) + + request = RegistryServer_pb2.SetDecisionStrategyRequest( + decision_strategy=decision_strategy_proto, project=project + ) + + if self.auth_config.type is not AuthType.NONE.value: + metadata = create_auth_header(self.auth_config) + self.stub.SetDecisionStrategy(request=request, metadata=metadata) + else: + self.stub.SetDecisionStrategy(request) + + def get_decision_strategy(self, project: str) -> DecisionStrategy: + request = RegistryServer_pb2.GetDecisionStrategyRequest(project=project) + + if self.auth_config.type is not AuthType.NONE.value: + metadata = create_auth_header(self.auth_config) + response = self.stub.GetDecisionStrategy(request=request, metadata=metadata) + else: + response = self.stub.GetDecisionStrategy(request) + + return DecisionStrategy(response) diff --git a/sdk/python/feast/infra/registry/snowflake.py b/sdk/python/feast/infra/registry/snowflake.py index 6f660982f6..452392b902 100644 --- a/sdk/python/feast/infra/registry/snowflake.py +++ b/sdk/python/feast/infra/registry/snowflake.py @@ -15,6 +15,7 @@ from feast.entity import Entity from feast.errors import ( DataSourceObjectNotFoundException, + DecisionStrategyNotFound, EntityNotFoundException, FeatureServiceNotFoundException, FeatureViewNotFoundException, @@ -32,6 +33,7 @@ execute_snowflake_statement, ) from feast.on_demand_feature_view import OnDemandFeatureView +from feast.permissions.decision import DecisionStrategy from feast.permissions.permission import Permission from feast.project_metadata import ProjectMetadata from feast.protos.feast.core.DataSource_pb2 import DataSource as DataSourceProto @@ -64,6 +66,7 @@ class FeastMetadataKeys(Enum): LAST_UPDATED_TIMESTAMP = "last_updated_timestamp" PROJECT_UUID = "project_uuid" + PERMISSIONS_DECISION_STRATEGY = "permissions_decision_strategy" class SnowflakeRegistryConfig(RegistryConfig): @@ -920,7 +923,13 @@ def list_project_metadata( for row in df.iterrows(): if row[1]["METADATA_KEY"] == FeastMetadataKeys.PROJECT_UUID.value: project_metadata.project_uuid = row[1]["METADATA_VALUE"] - break + elif ( + row[1]["METADATA_KEY"] + == FeastMetadataKeys.PERMISSIONS_DECISION_STRATEGY.value + ): + project_metadata.decision_strategy = DecisionStrategy( + row[1]["METADATA_VALUE"] + ) # TODO(adchia): Add other project metadata in a structured way return [project_metadata] return [] @@ -1107,7 +1116,8 @@ def _maybe_init_project_metadata(self, project): query = f""" INSERT INTO {self.registry_path}."FEAST_METADATA" VALUES - ('{project}', '{FeastMetadataKeys.PROJECT_UUID.value}', '{new_project_uuid}', CURRENT_TIMESTAMP()) + ('{project}', '{FeastMetadataKeys.PROJECT_UUID.value}', '{new_project_uuid}', CURRENT_TIMESTAMP()), + ('{project}', '{FeastMetadataKeys.PERMISSIONS_DECISION_STRATEGY.value}', '{DecisionStrategy.UNANIMOUS.value}', CURRENT_TIMESTAMP()) """ execute_snowflake_statement(conn, query) @@ -1150,3 +1160,40 @@ def _set_last_updated_metadata(self, last_updated: datetime, project: str): def commit(self): pass + + def set_decision_strategy(self, project: str, decision_strategy: DecisionStrategy): + self._maybe_init_project_metadata(project) + + with GetSnowflakeConnection(self.registry_config) as conn: + query = f""" + UPDATE {self.registry_path}."FEAST_METADATA" + SET + project_id = '{project}', + metadata_key = '{FeastMetadataKeys.PERMISSIONS_DECISION_STRATEGY.value}', + metadata_value = '{decision_strategy.value}', + last_updated_timestamp = CURRENT_TIMESTAMP() + WHERE + project_id = '{project}' + AND metadata_key = '{FeastMetadataKeys.PERMISSIONS_DECISION_STRATEGY.value}', + """ + execute_snowflake_statement(conn, query) + + def get_decision_strategy(self, project: str) -> DecisionStrategy: + with GetSnowflakeConnection(self.registry_config) as conn: + query = f""" + SELECT + metadata_value + FROM + {self.registry_path}."FEAST_METADATA" + WHERE + project_id = '{project}' + AND metadata_key = '{FeastMetadataKeys.PERMISSIONS_DECISION_STRATEGY.value}' + LIMIT 1 + """ + df = execute_snowflake_statement(conn, query).fetch_pandas_all() + + if not df.empty: + for row in df.iterrows(): + return DecisionStrategy(row[1]["METADATA_VALUE"]) + + raise DecisionStrategyNotFound(project=project) diff --git a/sdk/python/feast/infra/registry/sql.py b/sdk/python/feast/infra/registry/sql.py index cd6d2e2bb3..4c43d0a5a9 100644 --- a/sdk/python/feast/infra/registry/sql.py +++ b/sdk/python/feast/infra/registry/sql.py @@ -27,6 +27,7 @@ from feast.entity import Entity from feast.errors import ( DataSourceObjectNotFoundException, + DecisionStrategyNotFound, EntityNotFoundException, FeatureServiceNotFoundException, FeatureViewNotFoundException, @@ -39,6 +40,7 @@ from feast.infra.infra_object import Infra from feast.infra.registry.caching_registry import CachingRegistry from feast.on_demand_feature_view import OnDemandFeatureView +from feast.permissions.decision import DecisionStrategy from feast.permissions.permission import Permission from feast.project_metadata import ProjectMetadata from feast.protos.feast.core.DataSource_pb2 import DataSource as DataSourceProto @@ -165,6 +167,7 @@ class FeastMetadataKeys(Enum): LAST_UPDATED_TIMESTAMP = "last_updated_timestamp" PROJECT_UUID = "project_uuid" + PERMISSIONS_DECISION_STRATEGY = "permissions_decision_strategy" feast_metadata = Table( @@ -501,7 +504,13 @@ def _list_project_metadata(self, project: str) -> List[ProjectMetadata]: == FeastMetadataKeys.PROJECT_UUID.value ): project_metadata.project_uuid = row._mapping["metadata_value"] - break + elif ( + row._mapping["metadata_key"] + == FeastMetadataKeys.PERMISSIONS_DECISION_STRATEGY.value + ): + project_metadata.decision_strategy = DecisionStrategy( + row._mapping["metadata_value"] + ) # TODO(adchia): Add other project metadata in a structured way return [project_metadata] return [] @@ -802,12 +811,20 @@ def _maybe_init_project_metadata(self, project): row = conn.execute(stmt).first() if not row: new_project_uuid = f"{uuid.uuid4()}" - values = { - "metadata_key": FeastMetadataKeys.PROJECT_UUID.value, - "metadata_value": new_project_uuid, - "last_updated_timestamp": update_time, - "project_id": project, - } + values = [ + { + "metadata_key": FeastMetadataKeys.PROJECT_UUID.value, + "metadata_value": new_project_uuid, + "last_updated_timestamp": update_time, + "project_id": project, + }, + { + "metadata_key": FeastMetadataKeys.PERMISSIONS_DECISION_STRATEGY.value, + "metadata_value": DecisionStrategy.UNANIMOUS.value, + "last_updated_timestamp": update_time, + "project_id": project, + }, + ] insert_stmt = insert(feast_metadata).values(values) conn.execute(insert_stmt) @@ -986,3 +1003,39 @@ def delete_permission(self, name: str, project: str, commit: bool = True): rows = conn.execute(stmt) if rows.rowcount < 1: raise PermissionNotFoundException(name, project) + + def set_decision_strategy(self, project: str, decision_strategy: DecisionStrategy): + self._maybe_init_project_metadata(project) + + with self.engine.begin() as conn: + values = { + "metadata_key": FeastMetadataKeys.PERMISSIONS_DECISION_STRATEGY.value, + "metadata_value": f"{decision_strategy.value}", + "last_updated_timestamp": int(_utc_now().timestamp()), + "project_id": project, + } + + update_stmt = ( + update(feast_metadata) + .where( + feast_metadata.c.metadata_key + == FeastMetadataKeys.PERMISSIONS_DECISION_STRATEGY.value, + feast_metadata.c.project_id == project, + ) + .values(values) + ) + + conn.execute(update_stmt) + + def get_decision_strategy(self, project: str) -> DecisionStrategy: + with self.engine.begin() as conn: + stmt = select(feast_metadata).where( + feast_metadata.c.metadata_key + == FeastMetadataKeys.PERMISSIONS_DECISION_STRATEGY.value, + feast_metadata.c.project_id == project, + ) + row = conn.execute(stmt).first() + if row: + return DecisionStrategy(row._mapping["metadata_value"]) + + raise DecisionStrategyNotFound(project=project) diff --git a/sdk/python/feast/permissions/decision.py b/sdk/python/feast/permissions/decision.py index 35099e10ed..d5217cb125 100644 --- a/sdk/python/feast/permissions/decision.py +++ b/sdk/python/feast/permissions/decision.py @@ -29,7 +29,7 @@ class DecisionEvaluator: Create the instance and specify the strategy and number of decisions: `evaluator = DecisionEvaluator(DecisionStrategy.UNANIMOUS, 3) - For each vote that you receivem, add a decision grant: `evaluator.add_grant(vote, message)` + For each vote that you receive, add a decision grant: `evaluator.add_grant(vote, message)` and check if the decision process ended: `if evaluator.is_decided():` Once decided, get the result and the failure explanations using: `grant, explanations = evaluator.grant()` @@ -86,7 +86,7 @@ def grant(self) -> tuple[bool, list[str]]: def add_grant(self, grant: bool, explanation: str): """ Add a single vote to the decision computation, with a possible denial reason. - If the evalluation process already ended, additional votes are discarded. + If the evaluation process already ended, additional votes are discarded. Args: grant: `True` is the decision is accepted, `False` otherwise. diff --git a/sdk/python/feast/permissions/enforcer.py b/sdk/python/feast/permissions/enforcer.py index 11db7bace0..5f1dd7f908 100644 --- a/sdk/python/feast/permissions/enforcer.py +++ b/sdk/python/feast/permissions/enforcer.py @@ -1,7 +1,7 @@ import logging from feast.feast_object import FeastObject -from feast.permissions.decision import DecisionEvaluator +from feast.permissions.decision import DecisionEvaluator, DecisionStrategy from feast.permissions.permission import ( AuthzedAction, Permission, @@ -16,6 +16,7 @@ def enforce_policy( user: User, resources: list[FeastObject], actions: list[AuthzedAction], + decision_strategy: DecisionStrategy, filter_only: bool = False, ) -> list[FeastObject]: """ @@ -29,6 +30,7 @@ def enforce_policy( user: The current user. resources: The resources for which we need to enforce authorized permission. actions: The requested actions to be authorized. + decision_strategy: The decision strategy to be applied when multiple permissions match an execution request. filter_only: If `True`, it removes unauthorized resources from the returned value, otherwise it raises a `PermissionError` the first unauthorized resource. Defaults to `False`. @@ -53,9 +55,7 @@ def enforce_policy( ] if matching_permissions: - evaluator = DecisionEvaluator( - Permission.get_global_decision_strategy(), len(matching_permissions) - ) + evaluator = DecisionEvaluator(decision_strategy, len(matching_permissions)) for p in matching_permissions: permission_grant, permission_explanation = p.policy.validate_user( user=user diff --git a/sdk/python/feast/permissions/permission.py b/sdk/python/feast/permissions/permission.py index ec7e86b6cf..cb2e1250f5 100644 --- a/sdk/python/feast/permissions/permission.py +++ b/sdk/python/feast/permissions/permission.py @@ -7,7 +7,6 @@ from feast.importer import import_class from feast.permissions.action import ALL_ACTIONS, AuthzedAction -from feast.permissions.decision import DecisionStrategy from feast.permissions.matcher import actions_match_config, resource_match_config from feast.permissions.policy import AllowAll, Policy from feast.protos.feast.core.Permission_pb2 import Permission as PermissionProto @@ -102,22 +101,6 @@ def __hash__(self): def __str__(self): return str(MessageToJson(self.to_proto())) - _global_decision_strategy: DecisionStrategy = DecisionStrategy.UNANIMOUS - - @staticmethod - def get_global_decision_strategy() -> DecisionStrategy: - """ - The global decision strategy to be applied when multiple permissions match an execution request. - """ - return Permission._global_decision_strategy - - @staticmethod - def set_global_decision_strategy(global_decision_strategy: DecisionStrategy): - """ - Define the global decision strategy to be applied when multiple permissions match an execution request. - """ - Permission._global_decision_strategy = global_decision_strategy - @property def name(self) -> str: return self._name diff --git a/sdk/python/feast/permissions/security_manager.py b/sdk/python/feast/permissions/security_manager.py index 178db6e6e9..fc59800f7a 100644 --- a/sdk/python/feast/permissions/security_manager.py +++ b/sdk/python/feast/permissions/security_manager.py @@ -5,6 +5,7 @@ from feast.feast_object import FeastObject from feast.infra.registry.base_registry import BaseRegistry from feast.permissions.action import AuthzedAction +from feast.permissions.decision import DecisionStrategy from feast.permissions.enforcer import enforce_policy from feast.permissions.permission import Permission from feast.permissions.user import User @@ -52,6 +53,14 @@ def permissions(self) -> list[Permission]: """ return self._registry.list_permissions(project=self._project) + @property + def decision_strategy(self) -> DecisionStrategy: + """ + Returns: + DecisionStrategy: the DecisionStrategy configured in the Feast registry. + """ + return self._registry.get_decision_strategy(project=self._project) + def assert_permissions( self, resources: list[FeastObject], @@ -80,6 +89,7 @@ def assert_permissions( user=self.current_user if self.current_user is not None else User("", []), resources=resources, actions=actions if isinstance(actions, list) else [actions], + decision_strategy=self.decision_strategy, filter_only=filter_only, ) diff --git a/sdk/python/feast/project_metadata.py b/sdk/python/feast/project_metadata.py index 64488a0362..ad4f622c82 100644 --- a/sdk/python/feast/project_metadata.py +++ b/sdk/python/feast/project_metadata.py @@ -17,6 +17,7 @@ from google.protobuf.json_format import MessageToJson from typeguard import typechecked +from feast.permissions.decision import DecisionStrategy from feast.protos.feast.core.Registry_pb2 import ProjectMetadata as ProjectMetadataProto @@ -32,12 +33,14 @@ class ProjectMetadata: project_name: str project_uuid: str + decision_strategy: DecisionStrategy def __init__( self, *args, project_name: Optional[str] = None, project_uuid: Optional[str] = None, + decision_strategy: Optional[DecisionStrategy] = None, ): """ Creates an Project metadata object. @@ -45,6 +48,7 @@ def __init__( Args: project_name: The registry-scoped unique name of the project. project_uuid: The UUID for this project + decision_strategy: The projects decision strategy to be applied when multiple permissions match an execution request. Raises: ValueError: Parameters are specified incorrectly. @@ -54,9 +58,10 @@ def __init__( self.project_name = project_name self.project_uuid = project_uuid or f"{uuid.uuid4()}" + self.decision_strategy = decision_strategy or DecisionStrategy.UNANIMOUS def __hash__(self) -> int: - return hash((self.project_name, self.project_uuid)) + return hash((self.project_name, self.project_uuid, self.decision_strategy)) def __eq__(self, other): if not isinstance(other, ProjectMetadata): @@ -67,6 +72,7 @@ def __eq__(self, other): if ( self.project_name != other.project_name or self.project_uuid != other.project_uuid + or self.decision_strategy != other.decision_strategy ): return False @@ -92,6 +98,11 @@ def from_proto(cls, project_metadata_proto: ProjectMetadataProto): entity = cls( project_name=project_metadata_proto.project, project_uuid=project_metadata_proto.project_uuid, + decision_strategy=DecisionStrategy[ + ProjectMetadataProto.DecisionStrategy.Name( + project_metadata_proto.decision_strategy + ) + ], ) return entity @@ -105,5 +116,9 @@ def to_proto(self) -> ProjectMetadataProto: """ return ProjectMetadataProto( - project=self.project_name, project_uuid=self.project_uuid + project=self.project_name, + project_uuid=self.project_uuid, + decision_strategy=ProjectMetadataProto.DecisionStrategy.Value( + self.decision_strategy.name + ), ) diff --git a/sdk/python/feast/registry_server.py b/sdk/python/feast/registry_server.py index 16c37725db..b836846d4d 100644 --- a/sdk/python/feast/registry_server.py +++ b/sdk/python/feast/registry_server.py @@ -15,6 +15,7 @@ from feast.infra.registry.base_registry import BaseRegistry from feast.on_demand_feature_view import OnDemandFeatureView from feast.permissions.action import CRUD, AuthzedAction +from feast.permissions.decision import DecisionStrategy from feast.permissions.permission import Permission from feast.permissions.security_manager import assert_permissions, permitted_resources from feast.permissions.server.grpc import grpc_interceptors @@ -24,6 +25,7 @@ init_security_manager, str_to_auth_manager_type, ) +from feast.protos.feast.core.Registry_pb2 import ProjectMetadata as ProjectMetadataProto from feast.protos.feast.registry import RegistryServer_pb2, RegistryServer_pb2_grpc from feast.saved_dataset import SavedDataset, ValidationReference from feast.stream_feature_view import StreamFeatureView @@ -580,6 +582,22 @@ def Refresh(self, request, context): def Proto(self, request, context): return self.proxied_registry.proto() + def SetDecisionStrategy( + self, request: RegistryServer_pb2.SetDecisionStrategyRequest, context + ): + self.proxied_registry.set_decision_strategy( + project=request.project, + decision_strategy=DecisionStrategy[ + ProjectMetadataProto.DecisionStrategy.Name(request.decision_strategy) + ], + ) + return Empty() + + def GetDecisionStrategy( + self, request: RegistryServer_pb2.GetDecisionStrategyRequest, context + ): + return self.proxied_registry.get_decision_strategy(project=request.project) + def start_server(store: FeatureStore, port: int, wait_for_termination: bool = True): auth_manager_type = str_to_auth_manager_type(store.config.auth_config.type) diff --git a/sdk/python/tests/integration/registration/test_universal_registry.py b/sdk/python/tests/integration/registration/test_universal_registry.py index fd824a6704..6935a5ed91 100644 --- a/sdk/python/tests/integration/registration/test_universal_registry.py +++ b/sdk/python/tests/integration/registration/test_universal_registry.py @@ -42,6 +42,7 @@ from feast.infra.registry.sql import SqlRegistry from feast.on_demand_feature_view import on_demand_feature_view from feast.permissions.action import AuthzedAction +from feast.permissions.decision import DecisionStrategy from feast.permissions.permission import Permission from feast.permissions.policy import RoleBasedPolicy from feast.protos.feast.registry import RegistryServer_pb2, RegistryServer_pb2_grpc @@ -347,10 +348,14 @@ def test_apply_entity_success(test_registry): assert len(project_metadata) == 1 project_uuid = project_metadata[0].project_uuid assert len(project_metadata[0].project_uuid) == 36 - assert_project_uuid(project, project_uuid, test_registry) + assert_project_uuid( + project, project_uuid, DecisionStrategy.UNANIMOUS, test_registry + ) entities = test_registry.list_entities(project, tags=entity.tags) - assert_project_uuid(project, project_uuid, test_registry) + assert_project_uuid( + project, project_uuid, DecisionStrategy.UNANIMOUS, test_registry + ) entity = entities[0] assert ( @@ -389,18 +394,23 @@ def test_apply_entity_success(test_registry): ) test_registry.delete_entity("driver_car_id", project) - assert_project_uuid(project, project_uuid, test_registry) + assert_project_uuid( + project, project_uuid, DecisionStrategy.UNANIMOUS, test_registry + ) entities = test_registry.list_entities(project) - assert_project_uuid(project, project_uuid, test_registry) + assert_project_uuid( + project, project_uuid, DecisionStrategy.UNANIMOUS, test_registry + ) assert len(entities) == 0 test_registry.teardown() -def assert_project_uuid(project, project_uuid, test_registry): +def assert_project_uuid(project, project_uuid, decision_strategy, test_registry): project_metadata = test_registry.list_project_metadata(project=project) assert len(project_metadata) == 1 assert project_metadata[0].project_uuid == project_uuid + assert project_metadata[0].decision_strategy == decision_strategy @pytest.mark.integration @@ -1283,7 +1293,7 @@ def test_commit(): project_metadata = test_registry.cached_registry_proto.project_metadata[0] project_uuid = project_metadata.project_uuid assert len(project_uuid) == 36 - validate_project_uuid(project_uuid, test_registry) + validate_project_metadata(project_uuid, test_registry) # Retrieving the entity should still succeed entities = test_registry.list_entities(project, allow_cache=True, tags=entity.tags) @@ -1295,7 +1305,7 @@ def test_commit(): and "team" in entity.tags and entity.tags["team"] == "matchmaking" ) - validate_project_uuid(project_uuid, test_registry) + validate_project_metadata(project_uuid, test_registry) entity = test_registry.get_entity("driver_car_id", project, allow_cache=True) assert ( @@ -1304,7 +1314,7 @@ def test_commit(): and "team" in entity.tags and entity.tags["team"] == "matchmaking" ) - validate_project_uuid(project_uuid, test_registry) + validate_project_metadata(project_uuid, test_registry) # Create new registry that points to the same store registry_with_same_store = Registry("project", registry_config, None) @@ -1312,7 +1322,7 @@ def test_commit(): # Retrieving the entity should fail since the store is empty entities = registry_with_same_store.list_entities(project) assert len(entities) == 0 - validate_project_uuid(project_uuid, registry_with_same_store) + validate_project_metadata(project_uuid, registry_with_same_store) # commit from the original registry test_registry.commit() @@ -1330,7 +1340,7 @@ def test_commit(): and "team" in entity.tags and entity.tags["team"] == "matchmaking" ) - validate_project_uuid(project_uuid, registry_with_same_store) + validate_project_metadata(project_uuid, registry_with_same_store) entity = test_registry.get_entity("driver_car_id", project) assert ( @@ -1347,10 +1357,12 @@ def test_commit(): test_registry._get_registry_proto(project=project) -def validate_project_uuid(project_uuid, test_registry): +def validate_project_metadata(project_uuid, test_registry): assert len(test_registry.cached_registry_proto.project_metadata) == 1 project_metadata = test_registry.cached_registry_proto.project_metadata[0] assert project_metadata.project_uuid == project_uuid + print(project_metadata.decision_strategy) + # assert project_metadata.decision_strategy == DecisionStrategy.UNANIMOUS @pytest.mark.integration @@ -1371,10 +1383,14 @@ def test_apply_permission_success(test_registry): assert len(project_metadata) == 1 project_uuid = project_metadata[0].project_uuid assert len(project_metadata[0].project_uuid) == 36 - assert_project_uuid(project, project_uuid, test_registry) + assert_project_uuid( + project, project_uuid, DecisionStrategy.UNANIMOUS, test_registry + ) permissions = test_registry.list_permissions(project) - assert_project_uuid(project, project_uuid, test_registry) + assert_project_uuid( + project, project_uuid, DecisionStrategy.UNANIMOUS, test_registry + ) permission = permissions[0] assert ( @@ -1421,7 +1437,9 @@ def test_apply_permission_success(test_registry): test_registry.apply_permission(updated_permission, project) permissions = test_registry.list_permissions(project) - assert_project_uuid(project, project_uuid, test_registry) + assert_project_uuid( + project, project_uuid, DecisionStrategy.UNANIMOUS, test_registry + ) assert len(permissions) == 1 updated_permission = test_registry.get_permission("read_permission", project) @@ -1460,7 +1478,9 @@ def test_apply_permission_success(test_registry): test_registry.apply_permission(updated_permission, project) permissions = test_registry.list_permissions(project) - assert_project_uuid(project, project_uuid, test_registry) + assert_project_uuid( + project, project_uuid, DecisionStrategy.UNANIMOUS, test_registry + ) assert len(permissions) == 1 updated_permission = test_registry.get_permission("read_permission", project) @@ -1482,9 +1502,38 @@ def test_apply_permission_success(test_registry): ) test_registry.delete_permission("read_permission", project) - assert_project_uuid(project, project_uuid, test_registry) + assert_project_uuid( + project, project_uuid, DecisionStrategy.UNANIMOUS, test_registry + ) permissions = test_registry.list_permissions(project) - assert_project_uuid(project, project_uuid, test_registry) + assert_project_uuid( + project, project_uuid, DecisionStrategy.UNANIMOUS, test_registry + ) assert len(permissions) == 0 test_registry.teardown() + + +@pytest.mark.integration +@pytest.mark.parametrize("test_registry", all_fixtures) +def test_set_decision_strategy_success(test_registry): + project = "project" + + entity = Entity( + name="driver_car_id", + description="Car driver id", + tags={"team": "matchmaking"}, + ) + + # Register Entity + test_registry.apply_entity(entity, project) + + decision_strategy = test_registry.get_decision_strategy(project=project) + assert decision_strategy == DecisionStrategy.UNANIMOUS + + test_registry.set_decision_strategy(project, DecisionStrategy.AFFIRMATIVE) + + decision_strategy = test_registry.get_decision_strategy(project=project) + assert decision_strategy == DecisionStrategy.AFFIRMATIVE + + test_registry.teardown() diff --git a/sdk/python/tests/unit/permissions/conftest.py b/sdk/python/tests/unit/permissions/conftest.py index c85ebb1596..d40d29e1af 100644 --- a/sdk/python/tests/unit/permissions/conftest.py +++ b/sdk/python/tests/unit/permissions/conftest.py @@ -4,6 +4,7 @@ from feast import FeatureView from feast.infra.registry.base_registry import BaseRegistry +from feast.permissions.decision import DecisionStrategy from feast.permissions.decorator import require_permissions from feast.permissions.permission import AuthzedAction, Permission from feast.permissions.policy import RoleBasedPolicy @@ -12,6 +13,7 @@ set_security_manager, ) from feast.permissions.user import User +from feast.project_metadata import ProjectMetadata class SecuredFeatureView(FeatureView): @@ -84,8 +86,11 @@ def security_manager() -> SecurityManager: ) ) + project_metadata = ProjectMetadata(project_name="test_project") registry = Mock(spec=BaseRegistry) registry.list_permissions = Mock(return_value=permissions) + registry.list_project_metadata = Mock(return_value=[project_metadata]) + registry.get_decision_strategy = Mock(return_value=DecisionStrategy.UNANIMOUS) sm = SecurityManager(project="any", registry=registry) set_security_manager(sm) return sm diff --git a/sdk/python/tests/unit/permissions/test_permission.py b/sdk/python/tests/unit/permissions/test_permission.py index 8bb473952e..dd0e0ae411 100644 --- a/sdk/python/tests/unit/permissions/test_permission.py +++ b/sdk/python/tests/unit/permissions/test_permission.py @@ -12,7 +12,6 @@ from feast.on_demand_feature_view import OnDemandFeatureView from feast.permissions.action import ALL_ACTIONS, AuthzedAction from feast.permissions.permission import ( - DecisionStrategy, Permission, ) from feast.permissions.policy import AllowAll, Policy @@ -20,18 +19,6 @@ from feast.stream_feature_view import StreamFeatureView -def test_global_decision_strategy(): - assertpy.assert_that(Permission.get_global_decision_strategy()).is_equal_to( - DecisionStrategy.UNANIMOUS - ) - - for value in DecisionStrategy.__members__.values(): - Permission.set_global_decision_strategy(value) - assertpy.assert_that(Permission.get_global_decision_strategy()).is_equal_to( - value - ) - - def test_defaults(): p = Permission(name="test") assertpy.assert_that(type(p.types)).is_equal_to(list) diff --git a/sdk/python/tests/unit/permissions/test_security_manager.py b/sdk/python/tests/unit/permissions/test_security_manager.py index 5dd0261c26..617d3346b8 100644 --- a/sdk/python/tests/unit/permissions/test_security_manager.py +++ b/sdk/python/tests/unit/permissions/test_security_manager.py @@ -2,16 +2,9 @@ import pytest from feast.permissions.action import QUERY, AuthzedAction -from feast.permissions.decision import DecisionStrategy -from feast.permissions.permission import Permission from feast.permissions.security_manager import assert_permissions, permitted_resources -@pytest.fixture(scope="module", autouse=True) -def setup_module(): - Permission.set_global_decision_strategy(DecisionStrategy.UNANIMOUS) - - @pytest.mark.parametrize( "username, requested_actions, allowed, allowed_single, raise_error_in_assert, raise_error_in_permit", [ diff --git a/setup.py b/setup.py index 89835779ec..9b95d5e833 100644 --- a/setup.py +++ b/setup.py @@ -22,10 +22,8 @@ from pathlib import Path from setuptools import find_packages, setup, Command -from setuptools.command.build_ext import build_ext as _build_ext from setuptools.command.build_py import build_py from setuptools.command.develop import develop -from setuptools.command.install import install NAME = "feast" DESCRIPTION = "Python SDK for Feast"