Skip to content

fix(glossary): reject field edits to system-defined relation types via generic settings PUT - #31865

Open
sonika-shah wants to merge 2 commits into
mainfrom
fix/glossary-relation-systemdefined-immutable
Open

fix(glossary): reject field edits to system-defined relation types via generic settings PUT#31865
sonika-shah wants to merge 2 commits into
mainfrom
fix/glossary-relation-systemdefined-immutable

Conversation

@sonika-shah

@sonika-shah sonika-shah commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Describe your changes

Fixes #31864.

System-defined glossary relation types (relatedTo, partOf, hasPart, …; isSystemDefined: true) are a seeded, immutable classification. The UI (edit/delete disabled) and the dedicated PUT /v1/system/settings/glossaryTermRelationSettings/relationTypes/{name} endpoint (which force-sets isSystemDefined=false on create and rejects updates) enforce this — but the generic PUT /v1/system/settings path did not fully. This closes the remaining gaps on that path by extending GlossaryTermRelationSettingsUtil.validateSystemDefinedRelationTypesPreserved (already invoked via validateGlossaryTermRelationSettingsUpdate).

Behavior after this change

Scenario Before After
Edit a field of a system-defined type (e.g. partOf.isTransitive, color, category) allowed ❌ blocked
Rename a system-defined type blocked blocked ✅
Delete / downgrade (isSystemDefined t→f) a system-defined type blocked blocked ✅
Create a new type with isSystemDefined: true allowed ❌ blocked
Promote a custom type to isSystemDefined: true allowed ❌ blocked
Add / edit / delete a custom type allowed allowed ✅ (unchanged)
Re-save system-defined types unchanged (+ add a custom) allowed allowed ✅

Two guards are added:

  1. No fabrication/promotion — any incoming type flagged isSystemDefined: true whose name isn't already a seeded system-defined name is rejected ("Cannot create or promote system-defined relation types: <names>").
  2. Immutability — an existing system-defined type whose fields differ from the stored copy is rejected ("Cannot modify system-defined relation types: <names>"); compared on normalized copies so derived sourceMax/targetMax don't false-positive.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How was this change tested?

TDD (RED→GREEN) unit tests in GlossaryTermRelationSettingsUtilTest:

  • rejects field modification of a system-defined type
  • rejects a new isSystemDefined:true type
  • rejects promoting a custom type to system-defined
  • allows custom-type modification, and allows re-saving system-defined types unchanged

Tests run: 9, Failures: 0, Errors: 0 (mvn -pl openmetadata-service -Dtest=GlossaryTermRelationSettingsUtilTest test).

Checklist

  • I have performed a self-review of my code
  • I have added tests that prove my fix is effective

Greptile Summary

The PR strengthens glossary relation settings validation so generic settings updates cannot modify, create, or promote system-defined relation types.

  • Compares normalized system-defined relation types against their persisted definitions.
  • Preserves editability for custom relation types.
  • Adds focused tests for immutable system types and editable custom types.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
openmetadata-service/src/main/java/org/openmetadata/service/util/GlossaryTermRelationSettingsUtil.java Adds validation for creation, promotion, and field modification of system-defined glossary relation types.
openmetadata-service/src/test/java/org/openmetadata/service/util/GlossaryTermRelationSettingsUtilTest.java Adds unit coverage for rejected system-type changes and permitted custom-type changes.

Reviews (2): Last reviewed commit: "fix(glossary): block creating or promoti..." | Re-trigger Greptile

…a generic settings PUT

System-defined glossary relation types are meant to be immutable. The UI
and the dedicated relationTypes/{name} endpoint enforce this, but the
generic PUT /v1/system/settings path only blocked removing them and
duplicate names - it allowed editing their fields (e.g. isTransitive).

Extend validateSystemDefinedRelationTypesPreserved to also reject
modifications to system-defined types (normalized field comparison),
leaving custom relation types fully editable.
@sonika-shah
sonika-shah requested a review from a team as a code owner August 21, 2026 07:06
Copilot AI lite review requested due to automatic review settings August 21, 2026 07:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

@github-actions

github-actions Bot commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 8c3cc377edcf58a683121223fa6d4c12b339e463 in Playwright run 32481245328, attempt 1.

✅ 553 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 47m 31s

⏱️ Max setup 4m 5s · max shard execution 15m 35s · max shard-job elapsed before upload 18m 51s · reporting 3s

🌐 215.60 requests/attempt · 2.83 app boots/UI scenario · 17.86% common-shard skew

Optimization targets still in progress:

  • Common shard skew was 17.86% (convergence target: at most 15%).
  • Browser traffic was 215.6 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.83 per UI scenario (1623 boots / 574 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 103 0 0 0 0 0
✅ Shard chromium-02 103 0 0 0 0 0
✅ Shard chromium-03 91 0 0 0 0 0
✅ Shard chromium-04 102 0 0 0 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 16 0 0 0 0 0
✅ Shard global-state-01 34 0 0 0 0 0
✅ Shard ingestion-01 1 0 0 0 0 0
✅ Shard reindex-01 2 0 0 0 0 0
✅ Shard search-01 11 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

…pes via settings PUT

Also reject settings updates that flag a new or existing custom relation
type as isSystemDefined=true. system-defined is a seeded, immutable
classification (the dedicated relationTypes endpoint force-sets it to
false); only already-seeded names may carry the flag.
Copilot AI review requested due to automatic review settings August 21, 2026 12:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gitar-bot

gitar-bot Bot commented Aug 21, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Extends glossary term relation settings validation to block field modifications on system-defined types via the generic PUT endpoint. No issues found.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend safe to test Add this label to run secure Github workflows on PRs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System-defined glossary relation types are editable via generic /v1/system/settings PUT (bypasses immutability)

2 participants