[#12176] feat(core): add policy-on-tag core support by roryqi · Pull Request #12718 · apache/gravitino

roryqi · 2026-08-28T11:59:12Z

What changes were proposed in this pull request?

Add Core operations for listing, adding, and removing policy-to-tag associations.
Add JSON serialization and deserialization for ALL_VALUES and TAG_VALUE policy association selectors.
Add effective tag resolution with nearest-assignment override semantics.
Add a reusable object policy resolver that evaluates effective tags and selectors, filters disabled policies, detects conflicting matches, and deduplicates policies.
Keep the existing metadata-object policy APIs based on direct policy associations. Integration with tag-derived policies is deferred to follow-up API work.

Why are the changes needed?

These changes provide the Core runtime foundation for the policy-on-tag governance model while preserving the behavior of existing metadata-object policy APIs until the related public APIs and integration are ready.

Fix: #12176

Does this PR introduce any user-facing change?

No. The new capabilities are internal Core operations and resolvers. Existing metadata-object policy APIs continue to return directly associated policies only.

How was this patch tested?

./gradlew :common:test --tests org.apache.gravitino.json.TestPolicyAssociationSelectorSerde
./gradlew :core:test --tests org.apache.gravitino.policy.TestObjectPolicyResolver --tests org.apache.gravitino.policy.TestPolicyManager --tests org.apache.gravitino.tag.TestEffectiveTagResolver --tests org.apache.gravitino.tag.TestTagManager
GitHub Actions PR checks passed.

Copilot

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR introduces policy-on-tag behavior by resolving enabled policies from effective tag assignments on metadata objects, while keeping direct object-policy associations and deduplicating by policy ID.

Changes:

Add effective tag resolution and tag-derived policy resolution (ALL_VALUES / TAG_VALUE selectors).
Add TagManager operations for listing/adding/removing policy-to-tag associations.
Move selector JSON serde into common for reuse across modules and add targeted tests.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
core/src/main/java/org/apache/gravitino/tag/TagManager.java	Adds core APIs to manage policy↔tag associations via relation operations.
core/src/main/java/org/apache/gravitino/tag/TagDispatcher.java	Exposes new dispatcher defaults for policy associations on tags.
core/src/main/java/org/apache/gravitino/tag/EffectiveTagResolver.java	Computes effective tags for an object with nearest-assignment override semantics.
core/src/main/java/org/apache/gravitino/policy/ObjectPolicyResolver.java	Resolves enabled policies for an object from effective tags + selectors.
core/src/main/java/org/apache/gravitino/policy/PolicyManager.java	Merges direct and tag-derived policies; adds API to list tag associations for a policy.
core/src/main/java/org/apache/gravitino/policy/PolicyDispatcher.java	Exposes new dispatcher defaults for tag associations on policies.
common/src/main/java/org/apache/gravitino/json/PolicyAssociationSelectorSerde.java	Centralizes selector JSON serialization/deserialization.
core/src/test/java/org/apache/gravitino/tag/TestTagManager.java	Adds tests for policy↔tag association APIs.
core/src/test/java/org/apache/gravitino/tag/TestEffectiveTagResolver.java	Adds unit tests for effective tag resolution ordering/override behavior.
core/src/test/java/org/apache/gravitino/policy/TestObjectPolicyResolver.java	Adds unit tests for selector evaluation, conflicts, and filtering.
core/src/test/java/org/apache/gravitino/policy/TestPolicyManager.java	Updates integration tests to include tag-derived policies in results.
common/src/test/java/org/apache/gravitino/json/TestPolicyAssociationSelectorSerde.java	Adds serde round-trip and rejection tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

roryqi · 2026-09-01T07:58:46Z

 import java.util.Map;
 import javax.annotation.Nullable;
 import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.RelationalEntity;
 import org.apache.gravitino.exceptions.NoSuchTagException;
 import org.apache.gravitino.exceptions.TagAlreadyExistsException;
+import org.apache.gravitino.policy.PolicyAssociationSelector;


Thanks. java.util.Arrays is already imported at line 21 in the current head, and the build passes, so no change is needed here.

roryqi · 2026-09-01T07:58:46Z

+  default String[] listPoliciesForTag(String metalake, String name) {
+    return Arrays.stream(listPolicyAssociationsForTag(metalake, name))
+        .map(association -> association.targetEntity().name())
+        .toArray(String[]::new);
+  }


Thanks. This duplicates the other import comment. java.util.Arrays is already imported in the current head, so no change is needed here.

roryqi · 2026-09-01T07:58:46Z

+  public RelationalEntity<?>[] listTagAssociationsForPolicy(String metalake, String policyName) {
+    NameIdentifier policyIdentifier = NameIdentifierUtil.ofPolicy(metalake, policyName);
    checkMetalake(NameIdentifier.of(metalake), entityStore);
-
    return TreeLockUtils.doWithTreeLock(
-        entityIdent,
+        policyIdentifier,
        LockType.READ,
        () -> {
+          getPolicyWithoutLock(metalake, policyName);
          try {
            return entityStore
                .relationOperations()
-                .listEntitiesByRelation(
-                    SupportsRelationOperations.Type.POLICY_METADATA_OBJECT_REL,
-                    entityIdent,
-                    entityType,
-                    true /* allFields */)
-                .stream()
-                .map(entity -> (PolicyEntity) entity)
-                .toArray(PolicyEntity[]::new);
-          } catch (NoSuchEntityException e) {
-            throw new NoSuchMetadataObjectException(
-                e,
-                "Failed to list policies for metadata object %s due to not found",
-                metadataObject);
+                .batchListEntitiesByRelation(
+                    SupportsRelationOperations.Type.POLICY_TAG_REL,
+                    Collections.singletonList(policyIdentifier),
+                    Entity.EntityType.POLICY)
+                .toArray(new RelationalEntity<?>[0]);


Thanks. batchListEntitiesByRelation supports either endpoint for POLICY_TAG_REL. With a POLICY anchor, PolicyTagRelService.listRelations calls listByPolicyNames and then tagTargets, so the returned target entities are tags. The current implementation is correct and no change is needed here.

roryqi · 2026-09-01T07:58:46Z

+      PolicyEntity policy = (PolicyEntity) relation.targetEntity();
+      PolicyAssociationSelector selector =
+          PolicyAssociationSelectorSerde.deserialize(relation.relationValue().orElseThrow());
+      TagAssignment assignment = tag.assignment().orElseGet(TagAssignment::noValue);
+      boolean matches = matches(selector, assignment);


Fixed in e533d2d. A missing relation payload is now treated as AllValuesSelector, preserving the tag-presence compatibility behavior, and a regression test was added.

roryqi · 2026-09-01T07:58:46Z

+      String type = node.get(TYPE).asText();
+      if (AllValuesSelector.TYPE.equals(type)) {
+        return AllValuesSelector.get();
+      }
+      if (TagValueSelector.TYPE.equals(type)) {
+        return TagValueSelector.of(node.get(VALUE).asText());
+      }
+      throw JsonMappingException.from(
+          parser, "Unsupported policy association selector type: " + type);
+    }


Fixed in e533d2d. The deserializer now validates the required textual type and value fields and reports a clear JsonMappingException through the public deserialize helper. Tests cover missing and null fields.

github-actions · 2026-08-31T12:58:05Z

Code Coverage Report

Overall Project	68.92% `+0.08%`	🟢
Files changed	79.8%	🟢

Module	Coverage
aliyun	19.74%	🔴
api	51.72%	🟢
authorization-common	85.96%	🟢
authorization-ranger	4.38%	🔴
aws	53.54%	🟢
azure	32.1%	🔴
catalog-common	19.1%	🔴
catalog-fileset	80.3%	🟢
catalog-glue	69.24%	🟢
catalog-hive	82.96%	🟢
catalog-jdbc-common	45.69%	🟢
catalog-jdbc-doris	82.69%	🟢
catalog-jdbc-mysql	79.33%	🟢
catalog-jdbc-postgresql	83.39%	🟢
catalog-jdbc-starrocks	79.16%	🟢
catalog-kafka	76.99%	🟢
catalog-lakehouse-generic	60.55%	🟢
catalog-lakehouse-hudi	79.1%	🟢
catalog-lakehouse-iceberg	85.93%	🟢
catalog-lakehouse-paimon	84.26%	🟢
catalog-model	77.99%	🟢
cli	44.48%	🟢
client-java	77.49%	🟢
common	56.15% `+0.19%`	🟢
core	83.8% `-0.11%`	🟢
filesystem-hadoop3	76.45%	🟢
flink	0.0%	🔴
flink-common	52.1%	🟢
flink-runtime	0.0%	🔴
gcp	32.2%	🔴
hadoop-auth	68.0%	🟢
hadoop-common	17.84%	🔴
hive-metastore-common	53.4%	🟢
iceberg-aliyun-bundle	0.0%	🔴
iceberg-common	64.75%	🟢
iceberg-rest-server	75.96%	🟢
idp-basic	85.98%	🟢
integration-test-common	0.0%	🔴
jobs	62.92%	🟢
lance-common	32.63%	🔴
lance-rest-server	65.46%	🟢
lineage	53.02%	🟢
optimizer	83.24%	🟢
optimizer-api	21.95%	🔴
server	88.33%	🟢
server-common	80.5%	🟢
spark	28.57%	🔴
spark-common	48.92%	🟢
tencent	81.78%	🟢
trino-connector	51.26%	🟢

Files

Module	File	Coverage
common	PolicyAssociationSelectorSerde.java	100.0%	🟢
core	EffectiveTagResolver.java	87.5%	🟢
	ObjectPolicyResolver.java	86.57%	🟢
	TagManager.java	85.57%	🟢
	PolicyManager.java	73.58%	🟢
	PolicyDispatcher.java	42.86%	🔴
	TagDispatcher.java	0.0%	🔴

mchades

Non-duplicate findings from a current-head review.

mchades · 2026-09-01T06:30:21Z

+    Map<Long, PolicyEntity> policiesById = new LinkedHashMap<>();
+    Arrays.stream(listDirectPoliciesForMetadataObject(entityIdent, entityType, metadataObject))
+        .forEach(policy -> policiesById.putIfAbsent(policy.id(), policy));
+    Arrays.stream(objectPolicyResolver.resolve(metalake, metadataObject))


ObjectPolicyResolver.resolve already walks the requested object and all parents through EffectiveTagResolver, while MetadataObjectPolicyOperations still calls this method once for the object and again for each parent. A child override such as data_domain=risk over a parent data_domain=finance is therefore evaluated correctly here, but the later parent call adds a TAG_VALUE("finance") policy back. Policies selected from inherited tags are also returned in the first call and marked inherited=false by the REST layer. Please resolve tag-derived policies exactly once for the requested object, keep any direct-policy compatibility traversal separate, and add an end-to-end override regression test.

Thanks. We agree this must be addressed together with the planned object-policy semantic change in the follow-up REST PR. That change will separate direct-policy compatibility traversal from effective tag-derived resolution and define inherited flags there. We are intentionally not changing the existing REST traversal in this core runtime PR.

This PR already changes the existing user-facing list/get behavior by invoking ObjectPolicyResolver from PolicyManager. Those requests currently flow through MetadataObjectPolicyOperations, so merging this PR before the follow-up exposes the duplicate parent traversal immediately. Please either update the REST traversal and add override/inherited regression coverage here, or defer the PolicyManager resolver integration to the follow-up PR.

Fixed in 9123233. The ObjectPolicyResolver integration is deferred to the follow-up REST PR. PolicyManager now returns direct policies only, preserving the existing REST parent traversal and avoiding duplicate effective-tag resolution and incorrect inherited flags. The resolver and its unit coverage remain in this PR.

mchades · 2026-09-01T06:30:21Z

+   * @param name The name of the tag.
+   * @return The policy-to-tag associations.
+   */
+  default RelationalEntity<?>[] listPolicyAssociationsForTag(String metalake, String name) {


GravitinoEnv exposes TagHookDispatcher(TagEventDispatcher(TagManager)), but neither wrapper overrides these new default methods, so calls through the production dispatcher stop at this UnsupportedOperationException instead of reaching TagManager. PolicyHookDispatcher/PolicyEventDispatcher have the same gap for listTagAssociationsForPolicy. Please delegate the new operations through both wrapper layers and test the composed runtime chain.

Thanks. The REST entry points and the corresponding Tag/Policy Event and Hook dispatcher delegation will be added together in the follow-up REST integration PR. This core runtime PR intentionally does not expose these operations through the composed production chain yet.

mchades · 2026-09-01T06:30:22Z

+                    RelationEdgeTarget.of(
+                        policyIdentifier,
+                        Entity.EntityType.POLICY,
+                        PolicyAssociationSelectorSerde.serialize(selector))


This method persists the selector without validating it against the tag's TagValueConstraint. It accepts TAG_VALUE("engineering") for an ALLOWED_VALUES("finance", "risk") tag, or any TAG_VALUE for a NO_VALUE tag, creating an association that can never match. Please load the tag under the lock, validate the selector against its constraint, and add negative tests for unsupported values and constraint types.

Thanks. Policy-tag association create, update, and delete operations enter through REST, so selector validation against TagValueConstraint will be implemented in the follow-up REST PR. TagValueConstraint is immutable after tag creation, and this keeps semantic request validation in the REST layer instead of duplicating it in core.

mchades · 2026-09-01T06:30:22Z

+          try {
+            return entityStore
+                .relationOperations()
+                .batchListEntitiesByRelation(


batchListEntitiesByRelation returns an empty list when no matching relation exists and does not prove that the anchor tag exists. Because this method never loads the tag, a nonexistent tag is indistinguishable from an existing tag with no policies, so the planned GET endpoint cannot honor its 404 contract. Please check the tag under the read lock before querying and add a missing-tag test.

Fixed in e533d2d. listPolicyAssociationsForTag now loads the anchor tag under the read lock before querying relations, and the regression test verifies that a missing tag raises NoSuchTagException.

mchades

Please update the PR title and description before merging, as they no longer match the current implementation.

roryqi · 2026-09-02T09:09:32Z

Please update the PR title and description before merging, as they no longer match the current implementation.

Updated.

[apache#12176] feat(core): resolve policies from tag associations

d753bce

roryqi force-pushed the policy-on-tag-core-runtime branch from e2a5489 to d753bce Compare August 28, 2026 13:19

roryqi added 2 commits August 31, 2026 09:04

[apache#12176] fix(core): preserve direct policy associations

d9b1845

[apache#12176] refactor(core): simplify policy tag associations

18ca560

roryqi requested a review from mchades August 31, 2026 09:29

mchades requested a lite review from Copilot August 31, 2026 09:33

Copilot AI reviewed Aug 31, 2026

View reviewed changes

Copilot started reviewing on behalf of mchades August 31, 2026 10:29 View session

mchades reviewed Sep 1, 2026

View reviewed changes

roryqi self-assigned this Sep 1, 2026

[apache#12176] fix(core): address policy tag review comments

e533d2d

roryqi requested a review from mchades September 1, 2026 08:00

[apache#12176] fix(core): defer tag policy integration

9123233

mchades approved these changes Sep 2, 2026

View reviewed changes

roryqi changed the title ~~[#12176] feat(core): resolve policies from tag associations~~ [#12176] feat(core): add policy-on-tag core support Sep 2, 2026

roryqi merged commit 65b613d into apache:main Sep 2, 2026
41 of 42 checks passed

roryqi mentioned this pull request Sep 2, 2026

[#12176] feat(core): add policy-tag relation events #12855

Open

Uh oh!

Conversation

roryqi commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage Report

Uh oh!

mchades left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mchades left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

roryqi commented Sep 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

roryqi commented Aug 28, 2026 •

edited

Loading

github-actions Bot commented Aug 31, 2026 •

edited

Loading