fix: Added support for multiple name patterns to Permissions (feast-dev#4633)

* added support for multiple name patterns to Permissions

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

* fixed lint issues

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>

---------

Signed-off-by: Daniele Martinoli <dmartino@redhat.com>
Signed-off-by: Rob Howley <howley.robert@gmail.com>

Author: dmartinol

docs/getting-started/concepts/permission.md

@@ -36,7 +36,7 @@ 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. 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
+- `name_patterns`: A list of regex patterns to match resource names. If any regex matches, the `Permission` policy is applied. Defaults to `[]`, meaning 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.
@@ -95,7 +95,7 @@ The following permission grants authorization to read the offline store of all t
 Permission(
     name="reader",
     types=[FeatureView],
-    name_pattern=".*risky.*",
+    name_patterns=".*risky.*", # Accepts both `str` or `list[str]` types
     policy=RoleBasedPolicy(roles=["trusted"]),
     actions=[AuthzedAction.READ_OFFLINE],
 )

docs/reference/feast-cli-commands.md

@@ -172,9 +172,10 @@ Options:
 
 ```text
 +-----------------------+-------------+-----------------------+-----------+----------------+-------------------------+
-| NAME                  | TYPES       | NAME_PATTERN          | ACTIONS   | ROLES          | REQUIRED_TAGS           |
+| NAME                  | TYPES       | NAME_PATTERNS         | ACTIONS   | ROLES          | REQUIRED_TAGS           |
 +=======================+=============+=======================+===========+================+================+========+
 | reader_permission1234 | FeatureView | transformed_conv_rate | DESCRIBE  | reader         | -                       |
+|                       |             | driver_hourly_stats   | DESCRIBE  | reader         | -                       |
 +-----------------------+-------------+-----------------------+-----------+----------------+-------------------------+
 | writer_permission1234 | FeatureView | transformed_conv_rate | CREATE    | writer         | -                       |
 +-----------------------+-------------+-----------------------+-----------+----------------+-------------------------+

protos/feast/core/Permission.proto

@@ -50,7 +50,7 @@ message PermissionSpec {
 
   repeated Type types = 3;
 
-  string name_pattern = 4;
+  repeated string name_patterns = 4;
 
   map<string, string> required_tags = 5;
 

sdk/python/feast/cli.py

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

sdk/python/feast/cli_utils.py

@@ -196,7 +196,7 @@ 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.name_pattern,
+            _to_multi_line(p.name_patterns),
             _to_multi_line([a.value.upper() for a in p.actions]),
             _to_multi_line(sorted(roles)),
             _dict_to_multi_line(p.required_tags),

sdk/python/feast/permissions/matcher.py

@@ -44,7 +44,7 @@ def _get_type(resource: "FeastObject") -> Any:
 def resource_match_config(
     resource: "FeastObject",
     expected_types: list["FeastObject"],
-    name_pattern: Optional[str] = None,
+    name_patterns: list[str],
     required_tags: Optional[dict[str, str]] = None,
 ) -> bool:
     """
@@ -53,7 +53,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. Type match also includes all the sub-classes.
-        name_pattern: The optional name pattern filter configured in the permission.
+        name_patterns: The possibly empty list of name pattern filters configured in the permission.
         required_tags: The optional dictionary of required tags configured in the permission.
 
     Returns:
@@ -75,21 +75,8 @@ def resource_match_config(
         )
         return False
 
-    if name_pattern is not None:
-        if hasattr(resource, "name"):
-            if isinstance(resource.name, str):
-                match = bool(re.fullmatch(name_pattern, resource.name))
-                if not match:
-                    logger.info(
-                        f"Resource name {resource.name} does not match pattern {name_pattern}"
-                    )
-                    return False
-            else:
-                logger.warning(
-                    f"Resource {resource} has no `name` attribute of unexpected type {type(resource.name)}"
-                )
-        else:
-            logger.warning(f"Resource {resource} has no `name` attribute")
+    if not _resource_name_matches_name_patterns(resource, name_patterns):
+        return False
 
     if required_tags:
         if hasattr(resource, "required_tags"):
@@ -112,6 +99,39 @@ def resource_match_config(
     return True
 
 
+def _resource_name_matches_name_patterns(
+    resource: "FeastObject",
+    name_patterns: list[str],
+) -> bool:
+    if not hasattr(resource, "name"):
+        logger.warning(f"Resource {resource} has no `name` attribute")
+        return True
+
+    if not name_patterns:
+        return True
+
+    if resource.name is None:
+        return True
+
+    if not isinstance(resource.name, str):
+        logger.warning(
+            f"Resource {resource} has `name` attribute of unexpected type {type(resource.name)}"
+        )
+        return True
+
+    for name_pattern in name_patterns:
+        match = bool(re.fullmatch(name_pattern, resource.name))
+        if not match:
+            logger.info(
+                f"Resource name {resource.name} does not match pattern {name_pattern}"
+            )
+        else:
+            logger.info(f"Resource name {resource.name} matched pattern {name_pattern}")
+            return True
+
+    return False
+
+
 def actions_match_config(
     requested_actions: list[AuthzedAction],
     allowed_actions: list[AuthzedAction],

sdk/python/feast/permissions/permission.py

@@ -33,7 +33,7 @@ class Permission(ABC):
         name: The permission name (can be duplicated, used for logging troubleshooting).
         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)
-        name_pattern: A regex to match the resource name. Defaults to None, meaning that no name filtering is applied
+        name_patterns: A possibly empty list of regex patterns to match the resource name. Defaults to empty list, e.g. 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`.
         policy: The policy to be applied to validate a client request.
@@ -43,7 +43,7 @@ class Permission(ABC):
 
     _name: str
     _types: list["FeastObject"]
-    _name_pattern: Optional[str]
+    _name_patterns: list[str]
     _actions: list[AuthzedAction]
     _policy: Policy
     _tags: Dict[str, str]
@@ -54,8 +54,8 @@ class Permission(ABC):
     def __init__(
         self,
         name: str,
-        types: Optional[Union[list["FeastObject"], "FeastObject"]] = None,
-        name_pattern: Optional[str] = None,
+        types: Optional[Union[list["FeastObject"], "FeastObject"]] = [],
+        name_patterns: Optional[Union[str, list[str]]] = [],
         actions: Union[list[AuthzedAction], AuthzedAction] = ALL_ACTIONS,
         policy: Policy = AllowAll,
         tags: Optional[dict[str, str]] = None,
@@ -74,7 +74,7 @@ def __init__(
             raise ValueError("The list 'policy' must be non-empty.")
         self._name = name
         self._types = types if isinstance(types, list) else [types]
-        self._name_pattern = _normalize_name_pattern(name_pattern)
+        self._name_patterns = _normalize_name_patterns(name_patterns)
         self._actions = actions if isinstance(actions, list) else [actions]
         self._policy = policy
         self._tags = _normalize_tags(tags)
@@ -88,7 +88,7 @@ def __eq__(self, other):
 
         if (
             self.name != other.name
-            or self.name_pattern != other.name_pattern
+            or self.name_patterns != other.name_patterns
             or self.tags != other.tags
             or self.policy != other.policy
             or self.actions != other.actions
@@ -116,8 +116,8 @@ def types(self) -> list["FeastObject"]:
         return self._types
 
     @property
-    def name_pattern(self) -> Optional[str]:
-        return self._name_pattern
+    def name_patterns(self) -> list[str]:
+        return self._name_patterns
 
     @property
     def actions(self) -> list[AuthzedAction]:
@@ -143,7 +143,7 @@ def match_resource(self, resource: "FeastObject") -> bool:
         return resource_match_config(
             resource=resource,
             expected_types=self.types,
-            name_pattern=self.name_pattern,
+            name_patterns=self.name_patterns,
             required_tags=self.required_tags,
         )
 
@@ -175,6 +175,9 @@ def from_proto(permission_proto: PermissionProto) -> Any:
             )
             for t in permission_proto.spec.types
         ]
+        name_patterns = [
+            name_pattern for name_pattern in permission_proto.spec.name_patterns
+        ]
         actions = [
             AuthzedAction[PermissionSpecProto.AuthzedAction.Name(action)]
             for action in permission_proto.spec.actions
@@ -183,7 +186,7 @@ def from_proto(permission_proto: PermissionProto) -> Any:
         permission = Permission(
             permission_proto.spec.name,
             types,
-            permission_proto.spec.name_pattern or None,
+            name_patterns,
             actions,
             Policy.from_proto(permission_proto.spec.policy),
             dict(permission_proto.spec.tags) or None,
@@ -220,7 +223,7 @@ def to_proto(self) -> PermissionProto:
         permission_spec = PermissionSpecProto(
             name=self.name,
             types=types,
-            name_pattern=self.name_pattern if self.name_pattern is not None else "",
+            name_patterns=self.name_patterns,
             actions=actions,
             policy=self.policy.to_proto(),
             tags=self.tags,
@@ -236,10 +239,17 @@ def to_proto(self) -> PermissionProto:
         return PermissionProto(spec=permission_spec, meta=meta)
 
 
-def _normalize_name_pattern(name_pattern: Optional[str]):
-    if name_pattern is not None:
-        return name_pattern.strip()
-    return None
+def _normalize_name_patterns(
+    name_patterns: Optional[Union[str, list[str]]],
+) -> list[str]:
+    if name_patterns is None:
+        return []
+    if isinstance(name_patterns, str):
+        return _normalize_name_patterns([name_patterns])
+    normalized_name_patterns = []
+    for name_pattern in name_patterns:
+        normalized_name_patterns.append(name_pattern.strip())
+    return normalized_name_patterns
 
 
 def _normalize_tags(tags: Optional[dict[str, str]]):

sdk/python/feast/protos/feast/core/Permission_pb2.py

@@ -134,7 +134,7 @@ class PermissionSpec(google.protobuf.message.Message):
     NAME_FIELD_NUMBER: builtins.int
     PROJECT_FIELD_NUMBER: builtins.int
     TYPES_FIELD_NUMBER: builtins.int
-    NAME_PATTERN_FIELD_NUMBER: builtins.int
+    NAME_PATTERNS_FIELD_NUMBER: builtins.int
     REQUIRED_TAGS_FIELD_NUMBER: builtins.int
     ACTIONS_FIELD_NUMBER: builtins.int
     POLICY_FIELD_NUMBER: builtins.int
@@ -145,7 +145,8 @@ class PermissionSpec(google.protobuf.message.Message):
     """Name of Feast project."""
     @property
     def types(self) -> google.protobuf.internal.containers.RepeatedScalarFieldContainer[global___PermissionSpec.Type.ValueType]: ...
-    name_pattern: builtins.str
+    @property
+    def name_patterns(self) -> google.protobuf.internal.containers.RepeatedScalarFieldContainer[builtins.str]: ...
     @property
     def required_tags(self) -> google.protobuf.internal.containers.ScalarMap[builtins.str, builtins.str]: ...
     @property
@@ -163,14 +164,14 @@ class PermissionSpec(google.protobuf.message.Message):
         name: builtins.str = ...,
         project: builtins.str = ...,
         types: collections.abc.Iterable[global___PermissionSpec.Type.ValueType] | None = ...,
-        name_pattern: builtins.str = ...,
+        name_patterns: collections.abc.Iterable[builtins.str] | None = ...,
         required_tags: collections.abc.Mapping[builtins.str, builtins.str] | None = ...,
         actions: collections.abc.Iterable[global___PermissionSpec.AuthzedAction.ValueType] | None = ...,
         policy: feast.core.Policy_pb2.Policy | None = ...,
         tags: collections.abc.Mapping[builtins.str, builtins.str] | None = ...,
     ) -> None: ...
     def HasField(self, field_name: typing_extensions.Literal["policy", b"policy"]) -> builtins.bool: ...
-    def ClearField(self, field_name: typing_extensions.Literal["actions", b"actions", "name", b"name", "name_pattern", b"name_pattern", "policy", b"policy", "project", b"project", "required_tags", b"required_tags", "tags", b"tags", "types", b"types"]) -> None: ...
+    def ClearField(self, field_name: typing_extensions.Literal["actions", b"actions", "name", b"name", "name_patterns", b"name_patterns", "policy", b"policy", "project", b"project", "required_tags", b"required_tags", "tags", b"tags", "types", b"types"]) -> None: ...
 
 global___PermissionSpec = PermissionSpec