Skip to content

Commit

Permalink
fix(ui): Fixes handling of resources filters in UI (#9087)
Browse files Browse the repository at this point in the history
  • Loading branch information
skrydal authored Oct 24, 2023
1 parent d13553f commit 378d84a
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ export default function PolicyDetailsModal({ policy, visible, onClose, privilege
const isMetadataPolicy = policy?.type === PolicyType.Metadata;

const resources = convertLegacyResourceFilter(policy?.resources);
const resourceTypes = getFieldValues(resources?.filter, 'RESOURCE_TYPE') || [];
const resourceEntities = getFieldValues(resources?.filter, 'RESOURCE_URN') || [];
const resourceTypes = getFieldValues(resources?.filter, 'TYPE') || [];
const resourceEntities = getFieldValues(resources?.filter, 'URN') || [];
const domains = getFieldValues(resources?.filter, 'DOMAIN') || [];

const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ export default function PolicyPrivilegeForm({
} = useAppConfig();

const resources: ResourceFilter = convertLegacyResourceFilter(maybeResources) || EMPTY_POLICY.resources;
const resourceTypes = getFieldValues(resources.filter, 'RESOURCE_TYPE') || [];
const resourceEntities = getFieldValues(resources.filter, 'RESOURCE_URN') || [];
const resourceTypes = getFieldValues(resources.filter, 'TYPE') || [];
const resourceEntities = getFieldValues(resources.filter, 'URN') || [];

const getDisplayName = (entity) => {
if (!entity) {
Expand Down Expand Up @@ -145,10 +145,7 @@ export default function PolicyPrivilegeForm({
};
setResources({
...resources,
filter: setFieldValues(filter, 'RESOURCE_TYPE', [
...resourceTypes,
createCriterionValue(selectedResourceType),
]),
filter: setFieldValues(filter, 'TYPE', [...resourceTypes, createCriterionValue(selectedResourceType)]),
});
};

Expand All @@ -160,7 +157,7 @@ export default function PolicyPrivilegeForm({
...resources,
filter: setFieldValues(
filter,
'RESOURCE_TYPE',
'TYPE',
resourceTypes?.filter((criterionValue) => criterionValue.value !== deselectedResourceType),
),
});
Expand All @@ -173,7 +170,7 @@ export default function PolicyPrivilegeForm({
};
setResources({
...resources,
filter: setFieldValues(filter, 'RESOURCE_URN', [
filter: setFieldValues(filter, 'URN', [
...resourceEntities,
createCriterionValueWithEntity(
resource,
Expand All @@ -192,7 +189,7 @@ export default function PolicyPrivilegeForm({
...resources,
filter: setFieldValues(
filter,
'RESOURCE_URN',
'URN',
resourceEntities?.filter((criterionValue) => criterionValue.value !== resource),
),
});
Expand Down
4 changes: 2 additions & 2 deletions datahub-web-react/src/app/permissions/policy/policyUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,10 @@ export const convertLegacyResourceFilter = (resourceFilter: Maybe<ResourceFilter
}
const criteria = new Array<PolicyMatchCriterion>();
if (resourceFilter.type) {
criteria.push(createCriterion('RESOURCE_TYPE', [createCriterionValue(resourceFilter.type)]));
criteria.push(createCriterion('TYPE', [createCriterionValue(resourceFilter.type)]));
}
if (resourceFilter.resources && resourceFilter.resources.length > 0) {
criteria.push(createCriterion('RESOURCE_URN', resourceFilter.resources.map(createCriterionValue)));
criteria.push(createCriterion('URN', resourceFilter.resources.map(createCriterionValue)));
}
return {
filter: {
Expand Down
8 changes: 4 additions & 4 deletions docs/authorization/policies.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ We currently support the following:
#### Resources

Resource filter defines the set of resources that the policy applies to is defined using a list of criteria. Each
criterion defines a field type (like resource_type, resource_urn, domain), a list of field values to compare, and a
criterion defines a field type (like type, urn, domain), a list of field values to compare, and a
condition (like EQUALS). It essentially checks whether the field of a certain resource matches any of the input values.
Note, that if there are no criteria or resource is not set, policy is applied to ALL resources.

Expand All @@ -149,7 +149,7 @@ For example, the following resource filter will apply the policy to datasets, ch
"filter": {
"criteria": [
{
"field": "RESOURCE_TYPE",
"field": "TYPE",
"condition": "EQUALS",
"values": [
"dataset",
Expand All @@ -175,8 +175,8 @@ Supported fields are as follows

| Field Type | Description | Example |
|---------------|------------------------|-------------------------|
| resource_type | Type of the resource | dataset, chart, dataJob |
| resource_urn | Urn of the resource | urn:li:dataset:... |
| type | Type of the resource | dataset, chart, dataJob |
| urn | Urn of the resource | urn:li:dataset:... |
| domain | Domain of the resource | urn:li:domain:domainX |

## Managing Policies
Expand Down
8 changes: 4 additions & 4 deletions metadata-ingestion/tests/unit/serde/test_serde.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ def test_missing_optional_simple() -> None:
"criteria": [
{
"condition": "EQUALS",
"field": "RESOURCE_TYPE",
"field": "TYPE",
"values": ["notebook", "dataset", "dashboard"],
}
]
Expand All @@ -252,7 +252,7 @@ def test_missing_optional_simple() -> None:
"criteria": [
{
"condition": "EQUALS",
"field": "RESOURCE_TYPE",
"field": "TYPE",
"values": ["notebook", "dataset", "dashboard"],
}
]
Expand All @@ -267,13 +267,13 @@ def test_missing_optional_simple() -> None:
def test_missing_optional_in_union() -> None:
# This one doesn't contain any optional fields and should work fine.
revised_json = json.loads(
'{"lastUpdatedTimestamp":1662356745807,"actors":{"groups":[],"resourceOwners":false,"allUsers":true,"allGroups":false,"users":[]},"privileges":["EDIT_ENTITY_ASSERTIONS","EDIT_DATASET_COL_GLOSSARY_TERMS","EDIT_DATASET_COL_TAGS","EDIT_DATASET_COL_DESCRIPTION"],"displayName":"customtest","resources":{"filter":{"criteria":[{"field":"RESOURCE_TYPE","condition":"EQUALS","values":["notebook","dataset","dashboard"]}]},"allResources":false},"description":"","state":"ACTIVE","type":"METADATA"}'
'{"lastUpdatedTimestamp":1662356745807,"actors":{"groups":[],"resourceOwners":false,"allUsers":true,"allGroups":false,"users":[]},"privileges":["EDIT_ENTITY_ASSERTIONS","EDIT_DATASET_COL_GLOSSARY_TERMS","EDIT_DATASET_COL_TAGS","EDIT_DATASET_COL_DESCRIPTION"],"displayName":"customtest","resources":{"filter":{"criteria":[{"field":"TYPE","condition":"EQUALS","values":["notebook","dataset","dashboard"]}]},"allResources":false},"description":"","state":"ACTIVE","type":"METADATA"}'
)
revised = models.DataHubPolicyInfoClass.from_obj(revised_json)

# This one is missing the optional filters.allResources field.
original_json = json.loads(
'{"privileges":["EDIT_ENTITY_ASSERTIONS","EDIT_DATASET_COL_GLOSSARY_TERMS","EDIT_DATASET_COL_TAGS","EDIT_DATASET_COL_DESCRIPTION"],"actors":{"resourceOwners":false,"groups":[],"allGroups":false,"allUsers":true,"users":[]},"lastUpdatedTimestamp":1662356745807,"displayName":"customtest","description":"","resources":{"filter":{"criteria":[{"field":"RESOURCE_TYPE","condition":"EQUALS","values":["notebook","dataset","dashboard"]}]}},"state":"ACTIVE","type":"METADATA"}'
'{"privileges":["EDIT_ENTITY_ASSERTIONS","EDIT_DATASET_COL_GLOSSARY_TERMS","EDIT_DATASET_COL_TAGS","EDIT_DATASET_COL_DESCRIPTION"],"actors":{"resourceOwners":false,"groups":[],"allGroups":false,"allUsers":true,"users":[]},"lastUpdatedTimestamp":1662356745807,"displayName":"customtest","description":"","resources":{"filter":{"criteria":[{"field":"TYPE","condition":"EQUALS","values":["notebook","dataset","dashboard"]}]}},"state":"ACTIVE","type":"METADATA"}'
)
original = models.DataHubPolicyInfoClass.from_obj(original_json)

Expand Down

0 comments on commit 378d84a

Please sign in to comment.