Fixes #26774: expose custom properties in SpEL policy rule conditions by mohitjeswani01 · Pull Request #27218 · open-metadata/OpenMetadata

mohitjeswani01 · 2026-04-09T21:41:16Z

Describe your changes:

I worked on the Policy engine's SpEL evaluation context because entity
Custom Properties (e.g., "Data Sensitivity", "Department Owner") were
completely invisible to Policy Rule conditions — forcing teams to use
overly broad roles or Tags as a workaround instead of proper
attribute-based access control (ABAC).

Root Cause

ResourceContext never fetched the extension field (where custom
properties are stored) when resolving entities for policy evaluation.
Even if extension was available, no SpEL-callable method exposed it
to rule conditions.

What I Changed — 3 files, 1 commit:

ResourceContextInterface.java

Added getCustomProperties() default method returning
Map<String, Object> — safe default of empty map

ResourceContext.java

Implemented getCustomProperties() — fetches entity extension,
deserializes via JsonUtils, returns empty map on any failure
Extended resolveEntity() field list to include FIELD_EXTENSION
when the entity type supports it — so custom properties are
actually loaded from DB during policy evaluation

RuleEvaluator.java

Added getCustomProperties() bridge method — returns emptyMap()
(not null) when resourceContext is null, preventing NPE in SpEL
Added matchCustomProperty(String propertyName, String expectedValue)
— a convenient boolean helper for policy rule conditions

Usage in Policy Rule Conditions

After this change, policy rules can use:

// Direct map access
resource.customProperties.get('dataSensitivity') == 'PII'

// Convenient helper method (new)
matchCustomProperty('dataSensitivity', 'PII')
matchCustomProperty('department', 'Finance')

Why matchCustomProperty() matters

The issue explicitly requested a helper method to simplify syntax.
Without it, policy authors must use verbose map access syntax.
matchCustomProperty() handles all null/missing cases gracefully
and works correctly in both validation mode and runtime mode.

How I Tested

Added 6 unit tests to RuleEvaluatorTest.java covering:

getCustomProperties() returns empty map when no extension
getCustomProperties() returns correct map when extension is set
matchCustomProperty() returns true when property matches
matchCustomProperty() returns false when value doesn't match
matchCustomProperty() returns false when property is missing
matchCustomProperty() returns false when entity has no extension

Type of change:

New feature

Checklist:

I have read the CONTRIBUTING document.
My PR title is Fixes #26774: expose custom properties in SpEL policy rule conditions
I have commented on my code, particularly in hard-to-understand areas.
For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

The issue properly describes why the new feature is needed, what's the goal, and how we are building it.
I have added tests around the new logic.

github-actions · 2026-04-09T21:41:44Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

Copilot

Pull request overview

This PR extends the policy engine’s SpEL evaluation capabilities so policy conditions can evaluate entity Custom Properties (stored in the entity extension field), enabling ABAC-style rules based on those properties.

Changes:

Add getCustomProperties() to ResourceContextInterface and implement it in ResourceContext by deserializing the entity extension.
Update ResourceContext.resolveEntity() to include extension in the resolved field list when supported, so custom properties are available during policy evaluation.
Add getCustomProperties() and matchCustomProperty(...) helpers to RuleEvaluator, plus unit tests covering the new behavior.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/ResourceContextInterface.java	Adds a default `getCustomProperties()` contract returning an empty map.
openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/ResourceContext.java	Implements custom property extraction from `extension` and attempts to ensure `extension` is fetched during entity resolution.
openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/RuleEvaluator.java	Adds SpEL-callable accessors/helpers for custom properties.
openmetadata-service/src/test/java/org/openmetadata/service/security/policyevaluator/RuleEvaluatorTest.java	Adds unit tests for custom property access and matching behavior.

Copilot · 2026-04-09T21:47:29Z

+  @SuppressWarnings("unused")
+  public boolean matchCustomProperty(String propertyName, String expectedValue) {
+    if (expressionValidation) {
+      // During validation mode — just confirm syntax is valid, return false
+      return false;
+    }
+    if (resourceContext == null || propertyName == null || expectedValue == null) {
+      return false;


matchCustomProperty(...) is callable from SpEL as a function (has parentheses), but it is not annotated with @Function. ExpressionValidator only allows functions discovered via @Function, so policy conditions using matchCustomProperty('x','y') will be rejected as “Function 'matchCustomProperty' is not allowed”. Add a @Function annotation to this method (and ensure the annotation name matches the function name).

Fixed — added @function annotation to matchCustomProperty()
so ExpressionValidator recognizes it as an allowed function
in SpEL policy conditions.

Copilot · 2026-04-09T21:47:30Z

+  @SuppressWarnings("unused")
+  public Map<String, Object> getCustomProperties() {
+    if (resourceContext == null) {
+      return Collections.emptyMap();
+    }
+    return resourceContext.getCustomProperties();
+  }


getCustomProperties() is invoked as a function in the PR description/tests (getCustomProperties()), but it is not annotated with @Function. ExpressionValidator will reject expressions containing getCustomProperties() as an unapproved function call. Either add a @Function annotation for getCustomProperties or ensure docs/tests only use property access (resource.customProperties) without parentheses.

Fixed — added @function annotation to getCustomProperties()
as well for consistency and ExpressionValidator compliance.

Copilot · 2026-04-09T21:47:30Z

+        try {
+          Fields testFields = entityRepository.getFields(FIELD_EXTENSION);
+          if (testFields != null) {
+            fields = EntityUtil.addField(fields, FIELD_EXTENSION);
+          }
+        } catch (Exception ignored) {
+          // entity type does not support extension field — skip silently
+        }


The extension-field detection uses try/catch (Exception ignored) with an extra getFields(FIELD_EXTENSION) call just to decide whether to include the field. Since EntityRepository exposes getAllowedFields(), you can avoid exception-driven control flow by checking entityRepository.getAllowedFields().contains(FIELD_EXTENSION) (or Entity.entityHasField(...)). If you keep the current approach, narrow the catch to IllegalArgumentException so unexpected failures aren’t silently swallowed.

Copilot · 2026-04-09T21:47:30Z

+        try {
+          Fields testFields = entityRepository.getFields(FIELD_EXTENSION);
+          if (testFields != null) {
+            fields = EntityUtil.addField(fields, FIELD_EXTENSION);
+          }


Including FIELD_EXTENSION in the default resolveEntity() field list means every authorization/policy evaluation fetch for extension-supporting entities will also load extensions, even if no rule references custom properties. This can add extra DB work per request; consider loading extension lazily in getCustomProperties() instead of always requesting it up-front.

Copilot · 2026-04-09T21:47:30Z

+    // Set extension on the table entity
+    // Use JsonUtils to create a JsonNode with test properties
+    // set table.setExtension(node)
+    ObjectNode node = JsonUtils.getObjectMapper().createObjectNode();


There are leftover instructional comments in test_getCustomProperties_withExtension() (e.g., “Use JsonUtils to create…” / “set table.setExtension(node)”) that look like scaffolding rather than test intent. Please remove these to keep the test suite clean and consistent with the surrounding style.

Fixed — removed all leftover instructional comments from
test_getCustomProperties_withExtension().

Copilot · 2026-04-09T21:47:30Z

+    table.setExtension(null);
+    @SuppressWarnings("unchecked")
+    Map<String, Object> props = parseExpression("getCustomProperties()").getValue(evaluationContext, Map.class);
+    assertNotNull(props);
+    assertTrue(props.isEmpty());


The new parseExpression(...).getValue(...) assertions are written as long single lines (e.g., in test_getCustomProperties_noExtension() and test_getCustomProperties_withExtension()). This is likely to be reformatted/flagged by Spotless; consider wrapping the chained call across lines to match the surrounding formatting in this test file.

Fixed — broke the chained parseExpression().getValue() calls
across multiple lines to comply with Spotless formatting rules.

Copilot · 2026-04-09T21:47:31Z

+  @SuppressWarnings("unused")
+  public Map<String, Object> getCustomProperties() {
+    if (resourceContext == null) {
+      return Collections.emptyMap();
+    }
+    return resourceContext.getCustomProperties();


The PR description/examples use resource.customProperties.get('name'), but policy conditions are evaluated with RuleEvaluator as the SpEL root object (no resource variable), and ExpressionValidator will also reject .get(...) calls because it treats get( as an unapproved function. If you want direct map access, prefer SpEL index syntax (e.g., customProperties['dataSensitivity']) and/or update the documented examples accordingly (or expose a resource property explicitly).

Acknowledged — updated examples in Javadoc to use
matchCustomProperty('name', 'value') helper syntax
which is the correct SpEL callable pattern via @function.

github-actions · 2026-04-09T22:04:38Z

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

mohitjeswani01 · 2026-04-09T22:10:54Z

@harshach @pmbrull all review comments have been addressed:

✅ Gitar Bot — 2/2 resolved (Quality + Performance)
✅ Copilot — @function annotations added, test cleanup done,
line length fixed, caching implemented

Could you please add the safe to test label to trigger CI? thank you!🙏

github-actions · 2026-04-16T00:44:41Z

The Java checkstyle failed.

Please run mvn spotless:apply in the root of your repository and commit the changes to this PR.
You can also use pre-commit to automate the Java code formatting.

You can install the pre-commit hooks with make install_test precommit_install.

Fixes open-metadata#26774

…he customProperties, use isSupportsExtension()

Copilot

Pull request overview

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

Copilot · 2026-04-16T02:24:04Z

+  @Function
+  @SuppressWarnings("unused")
+  public boolean matchCustomProperty(String propertyName, String expectedValue) {
+    if (expressionValidation) {
+      // During validation mode — just confirm syntax is valid, return false
+      return false;
+    }


Same issue here: matchCustomProperty(...) is annotated with bare @Function, but the annotation requires name/input/description/examples. This will fail compilation and prevents ExpressionValidator from allowing matchCustomProperty(...) calls unless the annotation name is populated.

Copilot · 2026-04-16T02:24:04Z

  @Getter protected final boolean supportsCertification;
  @Getter protected final boolean supportsChildren;
  protected final boolean supportsFollower;
-  protected final boolean supportsExtension;
+  @Getter protected final boolean supportsExtension;
  protected final boolean supportsVotes;


PR description says this change is “3 files, 1 commit”, but this PR also changes EntityRepository.java (adds a getter for supportsExtension). Please update the PR description to match the actual set of modified files so reviewers know to consider this API change.

…ination

gitar-bot · 2026-04-16T02:38:16Z

Code Review ✅ Approved 4 resolved / 4 findings

Exposes custom properties in SpEL policy rule conditions by refactoring the extension deserialization and caching logic. Resolved issues include redundant deserialization, stale cache state, and improper type handling during property matching.

✅ 4 resolved

✅ Quality: Use @Getter for supportsExtension instead of try-catch workaround

📄 openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/ResourceContext.java:235-242
The try-catch block in resolveEntity() to check extension support (lines 235-242) catches all Exception types, which could silently swallow unrelated errors (e.g., database connectivity issues, OOM). Other field support checks use dedicated boolean getters like isSupportsOwners(), isSupportsTags(), etc. The supportsExtension field in EntityRepository already exists but lacks a @Getter annotation, unlike the other support flags.

This broad catch clause could mask real failures during policy evaluation, making debugging harder.

✅ Performance: getCustomProperties() re-deserializes extension on every call

📄 openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/ResourceContext.java:192-206
ResourceContext.getCustomProperties() calls JsonUtils.convertValue(entity.getExtension(), Map.class) every time it's invoked, without caching the result. If a policy has multiple custom property conditions (e.g., matchCustomProperty('sensitivity', 'PII') && matchCustomProperty('dept', 'Finance')), the extension is deserialized repeatedly for the same entity in the same evaluation.

While not critical for typical workloads, caching the deserialized map would be trivial and avoids redundant work, especially if custom property conditions become popular in policy rules.

✅ Bug: Stale cachedCustomProperties causes test failures across tests

📄 openmetadata-service/src/test/java/org/openmetadata/service/security/policyevaluator/RuleEvaluatorTest.java:1148-1154 📄 openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/ResourceContext.java:43 📄 openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/ResourceContext.java:194-208
The cachedCustomProperties field on ResourceContext is never cleared between tests. The test class creates a single resourceContext in @BeforeAll (line 224) and resetContext() in @AfterEach only recreates the RuleEvaluator/EvaluationContext — it does not recreate resourceContext.

Once any test calls getCustomProperties() (directly or via matchCustomProperty), the result is cached in cachedCustomProperties. Subsequent tests that change table.setExtension(...) will still get the stale cached value.

For example, if test_getCustomProperties_noExtension runs first (caches emptyMap), then test_matchCustomProperty_matches will fail because the cache returns an empty map despite the test setting extension to {dataSensitivity: PII}.

This is an order-dependent test failure — it may or may not manifest depending on JUnit 5's method ordering.

✅ Edge Case: matchCustomProperty uses toString() — may mismatch for non-string types

📄 openmetadata-service/src/main/java/org/openmetadata/service/security/policyevaluator/RuleEvaluator.java:456-460
Custom properties can be non-string types (integers, booleans, dates, arrays, objects). matchCustomProperty compares via expectedValue.equals(actual.toString()). For example, a boolean custom property stored as true (Java Boolean) would have toString() = "true", which works. But a list/array like ["a","b"] would produce an unhelpful toString() representation, and an integer 42 would need to be matched as matchCustomProperty('count', '42') — which is workable but not obvious.

Consider documenting that matchCustomProperty is designed for string/simple-valued custom properties, or handling richer types (e.g., list contains check).

Options

Display: compact → Showing less information.

Comment with these commands to change:

`Compact`
`gitar display:verbose`

_{Was this helpful? React with 👍 / 👎 | Gitar}

mohitjeswani01 · 2026-04-16T02:42:29Z

Hi @PubChimps 👋

I've just applied the spotless formatting fix — Java checkstyle should
now pass and also fixed the recent Bots comments

I noticed the related PR #27033 was closed and Issue #26774 was closed
as completed with my PR still open. My PR #27218 implements the full
feature:

Exposes custom properties in the SpEL evaluation context via
ResourceContext
Adds matchCustomProperty() helper method for simplified syntax

Could you let me know if this PR is still being considered ,
or if there are specific changes needed? I'm actively monitoring CI
and will address any feedback immediately.

Thank you! 🙏

Copilot AI review requested due to automatic review settings April 9, 2026 21:41

Copilot started reviewing on behalf of mohitjeswani01 April 9, 2026 21:42 View session

gitar-bot bot reviewed Apr 9, 2026

View reviewed changes

Comment thread ...service/src/main/java/org/openmetadata/service/security/policyevaluator/ResourceContext.java Outdated

gitar-bot bot reviewed Apr 9, 2026

View reviewed changes

Comment thread ...service/src/main/java/org/openmetadata/service/security/policyevaluator/ResourceContext.java

Copilot AI reviewed Apr 9, 2026

View reviewed changes

mohitjeswani01 mentioned this pull request Apr 9, 2026

Feature Request: Support for Custom Properties in Policy Rule Conditions #26774

Closed

PubChimps added the safe to test Add this label to run secure Github workflows on PRs label Apr 16, 2026

PubChimps mentioned this pull request Apr 16, 2026

feat(policy): expose custom properties in SpEL policy rule conditions (#26774) #27033

Closed

mohitjeswani01 added 3 commits April 16, 2026 07:42

feat(policy): expose custom properties in SpEL policy rule conditions

d1164f2

Fixes open-metadata#26774

fix(policy): address review comments - add @function annotations, cac…

c9b69aa

…he customProperties, use isSupportsExtension()

chore: run spotless:apply to fix checkstyle failures

2e365ad

mohitjeswani01 force-pushed the feat/26774-custom-properties-spel-policy-conditions branch from e51d312 to 2e365ad Compare April 16, 2026 02:19

Copilot AI review requested due to automatic review settings April 16, 2026 02:19

Copilot started reviewing on behalf of mohitjeswani01 April 16, 2026 02:20 View session

gitar-bot bot reviewed Apr 16, 2026

View reviewed changes

Comment thread ...service/src/main/java/org/openmetadata/service/security/policyevaluator/ResourceContext.java

gitar-bot bot reviewed Apr 16, 2026

View reviewed changes

Comment thread ...a-service/src/main/java/org/openmetadata/service/security/policyevaluator/RuleEvaluator.java

Copilot AI reviewed Apr 16, 2026

View reviewed changes

fix(security): address bot feedback on SpEL functions and test contam…

c6d5ee2

…ination

Conversation

mohitjeswani01 commented Apr 9, 2026

Describe your changes:

Root Cause

What I Changed — 3 files, 1 commit:

Usage in Policy Rule Conditions

Why matchCustomProperty() matters

How I Tested

Type of change:

Checklist:

Uh oh!

github-actions bot commented Apr 9, 2026

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

mohitjeswani01 Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

mohitjeswani01 Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

mohitjeswani01 Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

mohitjeswani01 Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

mohitjeswani01 Apr 9, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Apr 9, 2026

Uh oh!

mohitjeswani01 commented Apr 9, 2026

Uh oh!

github-actions bot commented Apr 16, 2026

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

gitar-bot bot commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mohitjeswani01 commented Apr 16, 2026

Uh oh!

Reviewers

gitar-bot bot commented Apr 16, 2026 •

edited

Loading