Merge pull request feast-dev#57 from tmihalac/fix-decision-strategy-n…

…ot-saved

Fix decision strategy not saved

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.
 

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;
 }
+
+

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;
+}

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()}"
         )
 
 

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)

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}")

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

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

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)

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)

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)