Skip to content

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

Open
sonika-shah wants to merge 1 commit into
2.0from
backport/31865-glossary-systemdefined-2.0
Open

fix(glossary): reject field edits to system-defined relation types via generic settings PUT (#31865) [2.0 backport]#31944
sonika-shah wants to merge 1 commit into
2.0from
backport/31865-glossary-systemdefined-2.0

Conversation

@sonika-shah

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

Copy link
Copy Markdown
Collaborator

Backport of #31865 to the 2.0 branch.

Enforces the immutability contract for seeded (system-defined) glossary relation types on the generic PUT /v1/system/settings path: they cannot be edited, renamed, removed, or downgraded, and no new type may be created or promoted to system-defined. Custom relation types remain fully editable.

Cherry-picked cleanly from the squashed merge commit c38c8c0. Verified on 2.0: GlossaryTermRelationSettingsUtilTestTests run: 9, Failures: 0.

Original PR: #31865 · Issue: #31864

Greptile Summary

The PR strengthens glossary relation settings validation so generic settings updates cannot remove, downgrade, promote, create, or modify system-defined relation types while preserving edits to custom types.

  • Splits immutable-type validation into removal, unsanctioned-definition, and field-modification checks.
  • Compares normalized copies to avoid cardinality representation differences.
  • Adds unit and integration coverage for field edits and fabricated system-defined types.

Confidence Score: 4/5

The PR appears safe to merge functionally, with only non-blocking Java maintainability issues in the newly added implementation and tests.

The new validation closes the generic-settings mutation path and its tests cover the intended rejection behavior; the accepted comments concern repository conventions rather than a demonstrated runtime defect.

Files Needing Attention: openmetadata-service/src/main/java/org/openmetadata/service/util/GlossaryTermRelationSettingsUtil.java, openmetadata-service/src/test/java/org/openmetadata/service/util/GlossaryTermRelationSettingsUtilTest.java, openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermRelationSettingsIT.java

Important Files Changed

Filename Overview
openmetadata-service/src/main/java/org/openmetadata/service/util/GlossaryTermRelationSettingsUtil.java Adds comprehensive immutable-system-type validation; behavior appears sound, with repository-style issues in method sizing and final usage.
openmetadata-service/src/test/java/org/openmetadata/service/util/GlossaryTermRelationSettingsUtilTest.java Adds focused unit coverage for modification, promotion, creation, and permitted custom edits, but repeats identifiers and omits final declarations.
openmetadata-integration-tests/src/test/java/org/openmetadata/it/tests/GlossaryTermRelationSettingsIT.java Adds API-level rejection and persistence checks, with non-blocking maintainability violations in literals, method size, and final usage.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Generic settings PUT] --> B[Deserialize relation settings]
  B --> C[Normalize cardinality]
  C --> D[Validate unique names]
  D --> E[Load current settings]
  E --> F{System-defined contract preserved?}
  F -->|Removed or downgraded| G[Reject update]
  F -->|Created or promoted| G
  F -->|Fields modified| G
  F -->|Unchanged| H[Persist settings]
  H --> I[Custom relation-type edits remain allowed]
Loading

Reviews (1): Last reviewed commit: "fix(glossary): reject field edits to sys..." | Re-trigger Greptile

Greptile also left 3 inline comments on this PR.

Context used:

  • Context used - CLAUDE.md (source)

…a generic settings PUT (#31865)

* fix(glossary): reject field edits to system-defined relation types via 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.

* fix(glossary): block creating or promoting system-defined relation types 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.

* refactor(glossary): extract system-defined validation into focused helpers

Split validateSystemDefinedRelationTypesPreserved into single-responsibility
helpers (removed / created-or-promoted / modified), build the name index once
instead of iterating the payload three times, and add a javadoc. No behavior
change - same checks, same messages.

* test(glossary): IT coverage for system-defined immutability on settings PUT

Add integration tests hitting the real PUT /v1/system/settings that a
field edit to a system-defined relation type, and creating a new
system-defined type, are both rejected. Complements the existing
delete-protection IT and the unit tests.
@sonika-shah
sonika-shah requested a review from a team as a code owner August 24, 2026 09:55
@github-actions

Copy link
Copy Markdown
Contributor

❌ PR checklist incomplete

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

  • No GitHub issue is linked. Link an issue in the Development section of the PR (or add Fixes #12345 to the description). For a same-org cross-repo issue, add Fixes open-metadata/<repo>#123 to the description.

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 added backend safe to test Add this label to run secure Github workflows on PRs labels Aug 24, 2026
@gitar-bot

gitar-bot Bot commented Aug 24, 2026

Copy link
Copy Markdown
Code Review ✅ Approved

Backport enforcing the immutability contract for system-defined glossary relation types on the generic settings PUT path. 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

@github-actions

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 3a0d5066da7922aff364263e682a7234b7b7a2aa in Playwright run 32714073493, attempt 1.

✅ 551 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) 48m 22s

⏱️ Max setup 2m 58s · max shard execution 17m 34s · max shard-job elapsed before upload 21m 20s · reporting 4s

🌐 210.41 requests/attempt · 2.83 app boots/UI scenario · 17.65% common-shard skew

Optimization targets still in progress:

  • Common shard skew was 17.65% (convergence target: at most 15%).
  • Browser traffic was 210.41 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.83 per UI scenario (1619 boots / 572 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 133 0 0 0 0 0
✅ Shard chromium-02 140 0 0 0 0 0
✅ Shard chromium-03 126 0 0 0 0 0
✅ Shard data-asset-rules-01 61 0 0 0 0 0
✅ Shard domain-isolation-01 14 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

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.

1 participant