Merge pull request feast-dev#74 from dmartinol/remove_with_subclasses

Removed with_subclasses option (it's the default and unique behavior)

docs/getting-started/concepts/permission.md

@@ -35,13 +35,23 @@ The permission model is based on the following components:
 
 The `Permission` class identifies a single permission configured on the feature store and is identified by these attributes:
 - `name`: The permission name.
-- `types`: The list of protected resource  types. Defaults to all managed types, e.g. the `ALL_RESOURCE_TYPES` alias
-- `with_subclasses`: Specify if sub-classes are included in the resource match or not. Defaults to `True`.
+- `types`: The list of protected resource  types. Defaults to all managed types, e.g. the `ALL_RESOURCE_TYPES` alias. All sub-classes are included in the resource match.
 - `name_pattern`: A regex to match the resource name. Defaults to `None`, meaning that no name filtering is applied
 - `required_tags`: Dictionary of key-value pairs that must match the resource tags. Defaults to `None`, meaning that no tags filtering is applied.
 - `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.
 
+To simplify configuration, several constants are defined to streamline the permissions setup:
+- In module `feast.feast_object`:
+  - `ALL_RESOURCE_TYPES` is the list of all the `FeastObject` types.
+  - `ALL_FEATURE_VIEW_TYPES` is the list of all the feature view types, including those not inheriting from `FeatureView` type like 
+  `OnDemandFeatureView`.
+- In module `feast.permissions.action`:
+  - `ALL_ACTIONS` is the list of all managed actions.
+  - `QUERY` includes all the query actions for online and offline store.
+  - `WRITE` includes all the write actions for online and offline store.
+  - `CRUD` includes all the state management actions to create, read, update or delete a Feast resource.
+
 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.
 
@@ -62,7 +72,7 @@ Permission(
     actions=[AuthzedAction.READ, QUERY],
 )
 ```
-Please note that all sub-classes of `FeatureView` are also included since the default for the `with_subclasses` parameter is `True`.
+Please note that all sub-classes of `FeatureView` are also included.
 
 This example grants permission to write on all the data sources with `risk_level` tag set to `high` only to users with role `admin` or `data_team`:
 ```py
@@ -85,7 +95,6 @@ The following permission grants authorization to query the offline store of all
 Permission(
     name="reader",
     types=[FeatureView],
-    with_subclasses=False, # exclude sub-classes
     name_pattern=".*risky.*",
     policy=RoleBasedPolicy(roles=["trusted"]),
     actions=[AuthzedAction.QUERY_OFFLINE],

docs/reference/feast-cli-commands.md

@@ -171,10 +171,10 @@ Options:
 ```
 
 ```text
-NAME                   TYPES            WITH_SUBCLASS    NAME_PATTERN           ACTIONS                         ROLES
-reader_permission1234  FeatureView      True             transformed_conv_rate  READ                            reader
+NAME                   TYPES            NAME_PATTERN           ACTIONS                         ROLES
+reader_permission1234  FeatureView      transformed_conv_rate  READ                            reader
                        FeaduteService
-writer_permission1234  FeatureView      True             transformed_conv_rate  CREATE                          writer
+writer_permission1234  FeatureView      transformed_conv_rate  CREATE                          writer
 ```
 
 `verbose` option describes the resources matching each configured permission: 

protos/feast/core/Permission.proto

@@ -41,8 +41,6 @@ message Permission {
 
   repeated Type types = 3;
 
-  bool with_subclasses = 4;
-
   string name_pattern = 5;
 
   map<string, string> required_tags = 6;

sdk/python/feast/cli.py

@@ -989,7 +989,6 @@ def feast_permissions_list_command(ctx: click.Context, verbose: bool, tags: list
                 headers=[
                     "NAME",
                     "TYPES",
-                    "WITH_SUBCLASS",
                     "NAME_PATTERN",
                     "ACTIONS",
                     "ROLES",

sdk/python/feast/cli_utils.py

@@ -196,7 +196,6 @@ def handle_not_verbose_permissions_command(
         [
             p.name,
             _to_multi_line([t.__name__ for t in p.types]),  # type: ignore[union-attr, attr-defined]
-            p.with_subclasses,
             p.name_pattern,
             _to_multi_line([a.value.upper() for a in p.actions]),
             _to_multi_line(sorted(roles)),

sdk/python/feast/feast_object.py

@@ -48,3 +48,9 @@
 ]
 
 ALL_RESOURCE_TYPES = list(get_args(FeastObject))
+ALL_FEATURE_VIEW_TYPES = [
+    FeatureView,
+    OnDemandFeatureView,
+    BatchFeatureView,
+    StreamFeatureView,
+]

sdk/python/feast/permissions/matcher.py

@@ -41,14 +41,9 @@ def _get_type(resource: "FeastObject") -> Any:
         return getattr(resource, "_spec_class", None)
 
 
-def _is_abstract_type(type: Any) -> bool:
-    return bool(getattr(type, "__abstractmethods__", False))
-
-
 def resource_match_config(
     resource: "FeastObject",
     expected_types: list["FeastObject"],
-    with_subclasses: bool = True,
     name_pattern: Optional[str] = None,
     required_tags: Optional[dict[str, str]] = None,
 ) -> bool:
@@ -57,8 +52,7 @@ def resource_match_config(
 
     Args:
         resource: A FeastObject instance to match agains the permission.
-        expected_types: The list of object types configured in the permission.
-        with_subclasses: `True` if the type match includes sub-classes, `False` if the type match is exact.
+        expected_types: The list of object types configured in the permission. Type match also includes all the sub-classes.
         name_pattern: The optional name pattern filter configured in the permission.
         required_tags: The optional dicstionary of required tags configured in the permission.
 
@@ -74,37 +68,12 @@ def resource_match_config(
         logger.warning(f"Given resource is not of a managed type but {_type}")
         return False
 
-    is_abstract = _is_abstract_type(_type)
-    if is_abstract and not with_subclasses:
-        logger.debug(
-            f"Overriding default configuration for abstract type {_type}: with_subclasses=True"
+    # mypy check ignored because of https://github.com/python/mypy/issues/11673, or it raises "Argument 2 to "isinstance" has incompatible type "tuple[Featu ..."
+    if not isinstance(resource, tuple(expected_types)):  # type: ignore
+        logger.info(
+            f"Resource does not match any of the expected type {expected_types}"
         )
-        with_subclasses = True
-
-    if with_subclasses:
-        # mypy check ignored because of https://github.com/python/mypy/issues/11673, or it raises "Argument 2 to "isinstance" has incompatible type "tuple[Featu ..."
-        if not isinstance(resource, tuple(expected_types)):  # type: ignore
-            logger.info(
-                f"Resource does not match any of the expected type {expected_types} (with_subclasses={with_subclasses})"
-            )
-            return False
-    else:
-        is_mock = isinstance(resource, Mock)
-        exact_type_match = False
-        for t in expected_types:
-            if not is_mock:
-                if type(resource) is t:
-                    exact_type_match = True
-                    break
-            else:
-                if getattr(resource, "_spec_class", None) is t:
-                    exact_type_match = True
-                    break
-        if not exact_type_match:
-            logger.info(
-                f"Resource does not match any of the expected type {expected_types} (with_subclasses={with_subclasses})"
-            )
-            return False
+        return False
 
     if name_pattern is not None:
         if hasattr(resource, "name"):

sdk/python/feast/permissions/permission.py

@@ -28,10 +28,8 @@ class Permission(ABC):
 
     Attributes:
         name: The permission name (can be duplicated, used for logging troubleshooting).
-        types: The list of protected resource  types as defined by the `FeastObject` type.
+        types: The list of protected resource types as defined by the `FeastObject` type. The match includes all the sub-classes of the given types.
         Defaults to all managed types (e.g. the `ALL_RESOURCE_TYPES` constant)
-        with_subclasses: If `True`, it includes sub-classes of the given types in the match, otherwise only exact type match is applied.
-        Defaults to `True`.
         name_pattern: A regex to match the resource name. Defaults to None, meaning that no name filtering is applied
         be present in a resource tags with the given value. Defaults to None, meaning that no tags filtering is applied.
         actions: The actions authorized by this permission. Defaults to `ALL_ACTIONS`.
@@ -41,7 +39,6 @@ class Permission(ABC):
 
     _name: str
     _types: list["FeastObject"]
-    _with_subclasses: bool
     _name_pattern: Optional[str]
     _actions: list[AuthzedAction]
     _policy: Policy
@@ -51,7 +48,6 @@ def __init__(
         self,
         name: str,
         types: Optional[Union[list["FeastObject"], "FeastObject"]] = None,
-        with_subclasses: bool = True,
         name_pattern: Optional[str] = None,
         actions: Union[list[AuthzedAction], AuthzedAction] = ALL_ACTIONS,
         policy: Policy = AllowAll,
@@ -70,7 +66,6 @@ def __init__(
             raise ValueError("The list 'policy' must be non-empty.")
         self._name = name
         self._types = types if isinstance(types, list) else [types]
-        self._with_subclasses = with_subclasses
         self._name_pattern = _normalize_name_pattern(name_pattern)
         self._actions = actions if isinstance(actions, list) else [actions]
         self._policy = policy
@@ -82,7 +77,6 @@ def __eq__(self, other):
 
         if (
             self.name != other.name
-            or self.with_subclasses != other.with_subclasses
             or self.name_pattern != other.name_pattern
             or self.tags != other.tags
             or self.policy != other.policy
@@ -109,10 +103,6 @@ def name(self) -> str:
     def types(self) -> list["FeastObject"]:
         return self._types
 
-    @property
-    def with_subclasses(self) -> bool:
-        return self._with_subclasses
-
     @property
     def name_pattern(self) -> Optional[str]:
         return self._name_pattern
@@ -137,7 +127,6 @@ def match_resource(self, resource: "FeastObject") -> bool:
         return resource_match_config(
             resource=resource,
             expected_types=self.types,
-            with_subclasses=self.with_subclasses,
             name_pattern=self.name_pattern,
             required_tags=self.tags,
         )
@@ -178,7 +167,6 @@ def from_proto(permission_proto: PermissionProto) -> Any:
         permission = Permission(
             permission_proto.name,
             types,
-            permission_proto.with_subclasses,
             permission_proto.name_pattern or None,
             actions,
             Policy.from_proto(permission_proto.policy),
@@ -205,7 +193,6 @@ def to_proto(self) -> PermissionProto:
         permission_proto = PermissionProto(
             name=self.name,
             types=types,
-            with_subclasses=self.with_subclasses,
             name_pattern=self.name_pattern if self.name_pattern is not None else "",
             actions=actions,
             policy=self.policy.to_proto(),

sdk/python/tests/integration/feature_repos/repo_configuration.py

@@ -488,35 +488,30 @@ def setup(self):
             Permission(
                 name="offline_fv_perm",
                 types=FeatureView,
-                with_subclasses=False,
                 policy=RoleBasedPolicy(roles=["writer"]),
                 actions=[AuthzedAction.QUERY_OFFLINE, AuthzedAction.WRITE_OFFLINE],
             ),
             Permission(
                 name="offline_odfv_perm",
                 types=OnDemandFeatureView,
-                with_subclasses=False,
                 policy=RoleBasedPolicy(roles=["writer"]),
                 actions=[AuthzedAction.QUERY_OFFLINE, AuthzedAction.WRITE_OFFLINE],
             ),
             Permission(
                 name="offline_sfv_perm",
                 types=StreamFeatureView,
-                with_subclasses=False,
                 policy=RoleBasedPolicy(roles=["writer"]),
                 actions=[AuthzedAction.QUERY_OFFLINE, AuthzedAction.WRITE_OFFLINE],
             ),
             Permission(
                 name="offline_fs_perm",
                 types=FeatureService,
-                with_subclasses=False,
                 policy=RoleBasedPolicy(roles=["writer"]),
                 actions=[AuthzedAction.QUERY_OFFLINE, AuthzedAction.WRITE_OFFLINE],
             ),
             Permission(
                 name="offline_datasource_perm",
                 types=DataSource,
-                with_subclasses=True,
                 policy=RoleBasedPolicy(roles=["writer"]),
                 actions=[AuthzedAction.QUERY_OFFLINE, AuthzedAction.WRITE_OFFLINE],
             ),

sdk/python/tests/integration/online_store/test_remote_online_store.py

@@ -25,21 +25,18 @@ def test_remote_online_store_read(auth_config):
             Permission(
                 name="online_list_fv_perm",
                 types=FeatureView,
-                with_subclasses=False,
                 policy=RoleBasedPolicy(roles=["reader"]),
                 actions=[AuthzedAction.QUERY_ONLINE],
             ),
             Permission(
                 name="online_list_odfv_perm",
                 types=OnDemandFeatureView,
-                with_subclasses=False,
                 policy=RoleBasedPolicy(roles=["reader"]),
                 actions=[AuthzedAction.QUERY_ONLINE],
             ),
             Permission(
                 name="online_list_sfv_perm",
                 types=StreamFeatureView,
-                with_subclasses=False,
                 policy=RoleBasedPolicy(roles=["reader"]),
                 actions=[AuthzedAction.QUERY_ONLINE],
             ),

sdk/python/tests/integration/registration/test_universal_registry.py

@@ -1387,7 +1387,6 @@ def test_apply_permission_success(test_registry):
         and isinstance(permission.policy, RoleBasedPolicy)
         and len(permission.policy.roles) == 1
         and permission.policy.roles[0] == "reader"
-        and permission.with_subclasses
         and permission.name_pattern is None
         and permission.tags is None
     )
@@ -1402,7 +1401,6 @@ def test_apply_permission_success(test_registry):
         and isinstance(permission.policy, RoleBasedPolicy)
         and len(permission.policy.roles) == 1
         and permission.policy.roles[0] == "reader"
-        and permission.with_subclasses
         and permission.name_pattern is None
         and permission.tags is None
     )
@@ -1436,7 +1434,6 @@ def test_apply_permission_success(test_registry):
         and len(updated_permission.policy.roles) == 2
         and "reader" in updated_permission.policy.roles
         and "writer" in updated_permission.policy.roles
-        and updated_permission.with_subclasses
         and updated_permission.name_pattern is None
         and updated_permission.tags is None
     )
@@ -1453,7 +1450,6 @@ def test_apply_permission_success(test_registry):
         actions=[AuthzedAction.READ, AuthzedAction.WRITE_ONLINE],
         policy=RoleBasedPolicy(roles=["reader", "writer"]),
         types=FeatureView,
-        with_subclasses=False,
         name_pattern="aaa",
         tags={"team": "matchmaking"},
     )
@@ -1475,7 +1471,6 @@ def test_apply_permission_success(test_registry):
         and len(updated_permission.policy.roles) == 2
         and "reader" in updated_permission.policy.roles
         and "writer" in updated_permission.policy.roles
-        and not updated_permission.with_subclasses
         and updated_permission.name_pattern == "aaa"
         and "team" in updated_permission.tags
         and updated_permission.tags["team"] == "matchmaking"

sdk/python/tests/unit/diff/test_registry_diff.py

@@ -180,14 +180,12 @@ def test_diff_registry_objects_permissions():
     pre_changed = Permission(
         name="reader",
         types=ALL_RESOURCE_TYPES,
-        with_subclasses=True,
         policy=RoleBasedPolicy(roles=["reader"]),
         actions=[AuthzedAction.READ],
     )
     post_changed = Permission(
         name="reader",
         types=ALL_RESOURCE_TYPES,
-        with_subclasses=True,
         policy=RoleBasedPolicy(roles=["reader"]),
         actions=[AuthzedAction.CREATE],
     )

sdk/python/tests/unit/local_feast_tests/test_local_feature_store.py

@@ -352,7 +352,6 @@ def test_apply_permissions(test_feature_store):
     permission = Permission(
         name="reader",
         types=ALL_RESOURCE_TYPES,
-        with_subclasses=True,
         policy=RoleBasedPolicy(roles=["reader"]),
         actions=[AuthzedAction.READ],
     )

sdk/python/tests/unit/permissions/auth/server/test_utils.py

@@ -17,39 +17,34 @@
 read_entities_perm = Permission(
     name="read_entities_perm",
     types=Entity,
-    with_subclasses=False,
     policy=RoleBasedPolicy(roles=["reader"]),
     actions=[AuthzedAction.READ],
 )
 
 read_fv_perm = Permission(
     name="read_fv_perm",
     types=FeatureView,
-    with_subclasses=False,
     policy=RoleBasedPolicy(roles=["reader"]),
     actions=[AuthzedAction.READ],
 )
 
 read_odfv_perm = Permission(
     name="read_odfv_perm",
     types=OnDemandFeatureView,
-    with_subclasses=False,
     policy=RoleBasedPolicy(roles=["reader"]),
     actions=[AuthzedAction.READ],
 )
 
 read_sfv_perm = Permission(
     name="read_sfv_perm",
     types=StreamFeatureView,
-    with_subclasses=False,
     policy=RoleBasedPolicy(roles=["reader"]),
     actions=[AuthzedAction.READ],
 )
 
 invalid_list_entities_perm = Permission(
     name="invalid_list_entity_perm",
     types=Entity,
-    with_subclasses=False,
     policy=RoleBasedPolicy(roles=["dancer"]),
     actions=[AuthzedAction.READ],
 )