Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Added support for multiple name patterns to Permissions #4633

Merged
merged 2 commits into from
Oct 16, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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]]] = [],

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, here's the init with list and 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
Loading