Skip to content

Commit

Permalink
fix: Added support for multiple name patterns to Permissions (feast-d…
Browse files Browse the repository at this point in the history
…ev#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>
  • Loading branch information
dmartinol authored Oct 16, 2024
1 parent 95fe8c2 commit f05e928
Show file tree
Hide file tree
Showing 12 changed files with 143 additions and 65 deletions.
4 changes: 2 additions & 2 deletions docs/getting-started/concepts/permission.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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],
)
Expand Down
3 changes: 2 additions & 1 deletion docs/reference/feast-cli-commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 | - |
+-----------------------+-------------+-----------------------+-----------+----------------+-------------------------+
Expand Down
2 changes: 1 addition & 1 deletion protos/feast/core/Permission.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion sdk/python/feast/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion sdk/python/feast/cli_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
54 changes: 37 additions & 17 deletions sdk/python/feast/permissions/matcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
"""
Expand All @@ -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:
Expand All @@ -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"):
Expand All @@ -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],
Expand Down
40 changes: 25 additions & 15 deletions sdk/python/feast/permissions/permission.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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]
Expand All @@ -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,
Expand All @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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]:
Expand All @@ -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,
)

Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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]]):
Expand Down
24 changes: 12 additions & 12 deletions sdk/python/feast/protos/feast/core/Permission_pb2.py

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 5 additions & 4 deletions sdk/python/feast/protos/feast/core/Permission_pb2.pyi
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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

Expand Down
Loading

0 comments on commit f05e928

Please sign in to comment.