Skip to content

Commit

Permalink
Merge pull request feast-dev#30 from tmihalac/store-permission-in-reg…
Browse files Browse the repository at this point in the history
…istry

Store and Manage permissions in the Registry
  • Loading branch information
tmihalac authored Jul 3, 2024
2 parents 41b152b + 8c70a2c commit 66644d3
Show file tree
Hide file tree
Showing 11 changed files with 51 additions and 32 deletions.
1 change: 1 addition & 0 deletions protos/feast/registry/RegistryServer.proto
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ message GetPermissionRequest {
message ListPermissionsRequest {
string project = 1;
bool allow_cache = 2;
map<string,string> tags = 3;
}

message ListPermissionsResponse {
Expand Down
8 changes: 6 additions & 2 deletions sdk/python/feast/feature_store.py
Original file line number Diff line number Diff line change
Expand Up @@ -2093,7 +2093,9 @@ def get_validation_reference(
ref._dataset = self.get_saved_dataset(ref.dataset_name)
return ref

def list_permissions(self, allow_cache: bool = False) -> List[Permission]:
def list_permissions(
self, allow_cache: bool = False, tags: Optional[dict[str, str]] = None
) -> List[Permission]:
"""
Retrieves the list of permissions from the registry.
Expand All @@ -2103,7 +2105,9 @@ def list_permissions(self, allow_cache: bool = False) -> List[Permission]:
Returns:
A list of data sources.
"""
return self._registry.list_permissions(self.project, allow_cache=allow_cache)
return self._registry.list_permissions(
self.project, allow_cache=allow_cache, tags=tags
)


def _print_materialization_log(
Expand Down
8 changes: 8 additions & 0 deletions sdk/python/feast/infra/registry/base_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,7 @@ def list_permissions(
self,
project: str,
allow_cache: bool = False,
tags: Optional[dict[str, str]] = None,
) -> List[Permission]:
"""
Retrieve a list of permissions from the registry
Expand Down Expand Up @@ -779,6 +780,13 @@ def to_dict(self, project: str) -> Dict[str, List[Any]]:
registry_dict["infra"].append(
self._message_to_sorted_dict(infra_object.to_proto())
)
for permission in sorted(
self.list_permissions(project=project), key=lambda ds: ds.name
):
registry_dict["permissions"].append(
self._message_to_sorted_dict(permission.to_proto())
)

return registry_dict

@staticmethod
Expand Down
9 changes: 6 additions & 3 deletions sdk/python/feast/infra/registry/caching_registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,20 +324,23 @@ def get_permission(
return self._get_permission(name, project)

@abstractmethod
def _list_permissions(self, project: str) -> List[Permission]:
def _list_permissions(
self, project: str, tags: Optional[dict[str, str]]
) -> List[Permission]:
pass

def list_permissions(
self,
project: str,
allow_cache: bool = False,
tags: Optional[dict[str, str]] = None,
) -> List[Permission]:
if allow_cache:
self._refresh_cached_registry_if_necessary()
return proto_registry_utils.list_permissions(
self.cached_registry_proto, project
self.cached_registry_proto, project, tags
)
return self._list_permissions(project)
return self._list_permissions(project, tags)

def refresh(self, project: Optional[str] = None):
if project:
Expand Down
10 changes: 7 additions & 3 deletions sdk/python/feast/infra/registry/proto_registry_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -289,11 +289,15 @@ def list_project_metadata(
]


@registry_proto_cache
def list_permissions(registry_proto: RegistryProto, project: str) -> List[Permission]:
@registry_proto_cache_with_tags
def list_permissions(
registry_proto: RegistryProto, project: str, tags: Optional[dict[str, str]]
) -> List[Permission]:
permissions = []
for permission_proto in registry_proto.permissions:
if permission_proto.project == project:
if permission_proto.project == project and utils.has_all_tags(
permission_proto.tags, tags
):
permissions.append(Permission.from_proto(permission_proto))
return permissions

Expand Down
7 changes: 5 additions & 2 deletions sdk/python/feast/infra/registry/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -916,12 +916,15 @@ def get_permission(
return proto_registry_utils.get_permission(registry_proto, name, project)

def list_permissions(
self, project: str, allow_cache: bool = False
self,
project: str,
allow_cache: bool = False,
tags: Optional[dict[str, str]] = None,
) -> List[Permission]:
registry_proto = self._get_registry_proto(
project=project, allow_cache=allow_cache
)
return proto_registry_utils.list_permissions(registry_proto, project)
return proto_registry_utils.list_permissions(registry_proto, project, tags)

def apply_permission(
self, permission: Permission, project: str, commit: bool = True
Expand Down
3 changes: 2 additions & 1 deletion sdk/python/feast/infra/registry/remote.py
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,10 @@ def list_permissions(
self,
project: str,
allow_cache: bool = False,
tags: Optional[dict[str, str]] = None,
) -> List[Permission]:
request = RegistryServer_pb2.ListPermissionsRequest(
project=project, allow_cache=allow_cache
project=project, allow_cache=allow_cache, tags=tags
)

response = self.stub.ListPermissions(request)
Expand Down
2 changes: 2 additions & 0 deletions sdk/python/feast/infra/registry/snowflake.py
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,7 @@ def list_permissions(
self,
project: str,
allow_cache: bool = False,
tags: Optional[dict[str, str]] = None,
) -> List[Permission]:
if allow_cache:
self._refresh_cached_registry_if_necessary()
Expand All @@ -850,6 +851,7 @@ def list_permissions(
PermissionProto,
Permission,
"PERMISSION_PROTO",
tags,
)

def apply_materialization(
Expand Down
5 changes: 4 additions & 1 deletion sdk/python/feast/infra/registry/sql.py
Original file line number Diff line number Diff line change
Expand Up @@ -949,13 +949,16 @@ def _get_permission(self, name: str, project: str) -> Permission:
not_found_exception=PermissionNotFoundException,
)

def _list_permissions(self, project: str) -> List[Permission]:
def _list_permissions(
self, project: str, tags: Optional[dict[str, str]]
) -> List[Permission]:
return self._list_objects(
permissions,
project,
PermissionProto,
Permission,
"permission_proto",
tags=tags,
)

def apply_permission(
Expand Down
26 changes: 8 additions & 18 deletions sdk/python/feast/permissions/permission.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,16 @@ class Permission(ABC):
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
required_tags: Dictionary of key-value pairs that must match the resource tags. All these required_tags must
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.
tags: Dictionary of key-value pairs that must match the resource tags. All these tags must
"""

_name: str
_types: list["FeastObject"]
_with_subclasses: bool
_name_pattern: Optional[str]
_required_tags: Optional[dict[str, str]]
_actions: list[AuthzedAction]
_policy: Policy
_tags: Dict[str, str]
Expand All @@ -53,10 +52,9 @@ def __init__(
types: Optional[Union[list["FeastObject"], "FeastObject"]] = None,
with_subclasses: bool = True,
name_pattern: Optional[str] = None,
required_tags: Optional[dict[str, str]] = None,
actions: Union[list[AuthzedAction], AuthzedAction] = ALL_ACTIONS,
policy: Policy = AllowAll,
tags: Optional[Dict[str, str]] = None,
tags: Optional[dict[str, str]] = None,
):
from feast.feast_object import ALL_RESOURCE_TYPES

Expand All @@ -73,10 +71,9 @@ def __init__(
self._types = types if isinstance(types, list) else [types]
self._with_subclasses = with_subclasses
self._name_pattern = _normalize_name_pattern(name_pattern)
self._required_tags = _normalize_required_tags(required_tags)
self._actions = actions if isinstance(actions, list) else [actions]
self._policy = policy
self._tags = tags or {}
self._tags = _normalize_tags(tags)

def __eq__(self, other):
if not isinstance(other, Permission):
Expand All @@ -86,7 +83,7 @@ def __eq__(self, other):
self.name != other.name
or self.with_subclasses != other.with_subclasses
or self.name_pattern != other.name_pattern
or self.required_tags != other.required_tags
or self.tags != other.tags
or self.policy != other.policy
or self.actions != other.actions
):
Expand Down Expand Up @@ -129,10 +126,6 @@ def with_subclasses(self) -> bool:
def name_pattern(self) -> Optional[str]:
return self._name_pattern

@property
def required_tags(self) -> Optional[dict[str, str]]:
return self._required_tags

@property
def actions(self) -> list[AuthzedAction]:
return self._actions
Expand All @@ -155,7 +148,7 @@ def match_resource(self, resource: "FeastObject") -> bool:
expected_types=self.types,
with_subclasses=self.with_subclasses,
name_pattern=self.name_pattern,
required_tags=self.required_tags,
required_tags=self.tags,
)

def match_actions(self, requested_actions: list[AuthzedAction]) -> bool:
Expand Down Expand Up @@ -196,7 +189,6 @@ def from_proto(permission_proto: PermissionProto) -> Any:
types,
permission_proto.with_subclasses,
permission_proto.name_pattern or None,
dict(permission_proto.required_tags),
actions,
Policy.from_proto(permission_proto.policy),
dict(permission_proto.tags) or None,
Expand Down Expand Up @@ -224,7 +216,6 @@ def to_proto(self) -> PermissionProto:
types=types,
with_subclasses=self.with_subclasses,
name_pattern=self.name_pattern if self.name_pattern is not None else None,
required_tags=self.required_tags,
actions=actions,
policy=self.policy.to_proto(),
tags=self._tags if self._tags is not None else None,
Expand All @@ -239,11 +230,10 @@ def _normalize_name_pattern(name_pattern: Optional[str]):
return None


def _normalize_required_tags(required_tags: Optional[dict[str, str]]):
if required_tags:
def _normalize_tags(tags: Optional[dict[str, str]]):
if tags:
return {
k.strip(): v.strip() if isinstance(v, str) else v
for k, v in required_tags.items()
k.strip(): v.strip() if isinstance(v, str) else v for k, v in tags.items()
}
return None

Expand Down
4 changes: 2 additions & 2 deletions sdk/python/tests/unit/permissions/test_permission.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ def test_defaults():
assertpy.assert_that(p.types).is_equal_to(ALL_RESOURCE_TYPES)
assertpy.assert_that(p.with_subclasses).is_true()
assertpy.assert_that(p.name_pattern).is_none()
assertpy.assert_that(p.required_tags).is_none()
assertpy.assert_that(p.tags).is_none()
assertpy.assert_that(type(p.actions)).is_equal_to(list)
assertpy.assert_that(p.actions).is_equal_to(ALL_ACTIONS)
assertpy.assert_that(type(p.actions)).is_equal_to(list)
Expand Down Expand Up @@ -246,7 +246,7 @@ def test_resource_match_with_name_filter(pattern, name, match):
)
def test_resource_match_with_tags(required_tags, tags, result):
# Missing tags
p = Permission(name="test", required_tags=required_tags)
p = Permission(name="test", tags=required_tags)
for t in ALL_RESOURCE_TYPES:
resource = Mock(spec=t)
resource.name = "test"
Expand Down

0 comments on commit 66644d3

Please sign in to comment.