Skip to content

feat(appConfig): add tenant-level defaultAppMode override via yaml - #31099

Closed
chirag-madlani wants to merge 5 commits into
mainfrom
feat/app-mode-tenant-override
Closed

feat(appConfig): add tenant-level defaultAppMode override via yaml#31099
chirag-madlani wants to merge 5 commits into
mainfrom
feat/app-mode-tenant-override

Conversation

@chirag-madlani

Copy link
Copy Markdown
Collaborator

Summary

Add a yaml-configured tenant-level force for the app mode. Operators can pin every user in a deployment to a specific mode (ai or classic) via openmetadata.yaml — overriding the per-user preference. When unset, the current per-user behavior wins.

Complements #31030 (per-user preference persistence). Independent of it — this PR branches off main and can land in either order.

Fallback chain at boot

  1. appConfiguration.defaultAppMode (tenant force from yaml)
  2. user.preferences.appMode (per-user preference, from feat(user): persist per-user UI preferences on the User entity (starting with appMode) #31030)
  3. DEFAULT_APP_MODE constant

What changed

Backend (yaml → Java → REST):

  • New JSON schema openmetadata-spec/.../api/configuration/appConfiguration.json with a nullable defaultAppMode field (enum: "ai" | "classic" | null).
  • OpenMetadataApplicationConfig.appConfiguration field, defaulted to new AppConfiguration() so getDefaultAppMode() is null-safe when yaml is absent.
  • New endpoint GET /v1/system/config/appConfig in ConfigResource — mirrors the /authorizer pattern (reads from openMetadataApplicationConfig, not SettingsCache). Standard JwtFilter auth.
  • conf/openmetadata.yaml gains:
    appConfiguration:
      defaultAppMode: ${APP_DEFAULT_MODE:-null}

UI:

  • getAppConfig() in rest/miscAPI.ts.
  • New useAppModeConfig Zustand store with { isForced, forcedMode, setForced }.
  • hydrateAppModeConfig(config) — called from both bootstrap paths in AuthProvider.tsx (returning-session AND fresh-login), via Promise.all([getLoggedInUser(), getAppConfig()]) to avoid a waterfall. When defaultAppMode is non-null, pins runtime via writeAppMode and sets isForced=true.
  • Hard force: both writeAppMode(...) (in useAppMode.ts) and setPreference({ appMode }) (in useCurrentUserStore.ts) short-circuit when isForced is true. The setPreference guard is per-key — other preference keys still write through normally.
  • Correctness fix discovered during implementation: added a CONFIG_MODE_TO_RUNTIME translation ('classic'DEFAULT_APP_MODE which is 'default'). Without this, forcing "classic" would have set an unrecognized runtime string, and useIsAiMode() would incorrectly return true for a classic-forced tenant. Mirrors the existing APP_MODE_ENUM_TO_RUNTIME pattern in useResolvedAppMode.ts.

Testing

  • Java: ConfigResourceTest (new, 3/3 pass) — Mockito unit test in the same style as IndexResourceTest.java. No infra needed; runs via plain mvn test. Covers: default returns null; configured value is echoed; null-safe when getAppConfiguration() returns null.
  • Jest: 54/54 pass across useAppModeConfig (new suite), useCurrentUserStore (+2 guard tests), useAppMode (+2 guard tests). Pre-existing AuthProvider.test.tsx still 12/12 (no regression).
  • Lint + tsc: clean on touched files.
  • mvn spotless:apply clean on openmetadata-service / openmetadata-spec.

Design decisions locked

  • Yaml-only — no DB storage, no SettingsCache, no admin UI page, no runtime mutation. Ops change the value by editing yaml + restart, same as authorizerConfiguration.
  • Boot-only enforcement — no mid-session flip when yaml changes.
  • Hard force — user can't fight the admin's choice. AppModeSwitcher (Collate-side follow-up) will hide when forced.
  • Per-key guard — only the appMode key is guarded on setPreference; other user prefs pass through.

Not in scope

  • Admin UI settings page — future PR if ever needed. For now ops set via env var / yaml.
  • Live enforcement / SSE push of yaml changes — would need a new subsystem.
  • AppModeSwitcher visibility guard — lives in Collate, tracked separately.

Test plan for reviewers

  • CI passes (Java unit tests + Jest suite).
  • Deploy with APP_DEFAULT_MODE=ai → every user lands in AI mode at login; user toggle attempts are no-ops.
  • Deploy with APP_DEFAULT_MODE=classic → same in reverse (verifies the runtime translation).
  • Deploy without setting the env → behavior is identical to today (user preference wins, or DEFAULT_APP_MODE if no preference).
  • GET /api/v1/system/config/appConfig returns { defaultAppMode: null } on a default deployment; returns the configured value when set.

🤖 Generated with Claude Code

@chirag-madlani
chirag-madlani requested a review from a team as a code owner August 6, 2026 10:30
Copilot AI review requested due to automatic review settings August 6, 2026 10:30
@chirag-madlani
chirag-madlani requested a review from a team as a code owner August 6, 2026 10:30

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

github-actions Bot commented Aug 6, 2026

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 safe to test Add this label to run secure Github workflows on PRs UI UI specific issues labels Aug 6, 2026
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 22 warning(s)

Errors block the build. Warnings do not yet — they are rules whose backlog is still
being worked down, listed so this PR does not add to it. See docs/ui-code-quality-gate.md.

0 error(s), 22 warning(s) across 3 changed file(s).

Count Rule
9 react-hooks/exhaustive-deps
8 @typescript-eslint/no-explicit-any
3 sonarjs/no-nested-functions
1 sonarjs/cyclomatic-complexity
1 sonarjs/expression-complexity
All findings
Location Rule Message
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:193:9 react-hooks/exhaustive-deps The 'onLoginHandler' function makes the dependencies of useMemo Hook (at line 838) change on every render. Move it inside the useMemo callback. Alternatively, w
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:265:6 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'navigate', 'setApplicationLoading', 'setCurrentUser', and 'setIsAuthenticated'. Either include them or remove
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:323:9 react-hooks/exhaustive-deps The 'resetUserDetails' function makes the dependencies of useMemo Hook (at line 838) change on every render. To fix this, wrap the definition of 'resetUserDetai
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:408:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'startTokenExpiryTimer'. Either include it or remove the dependency array.
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:451:6 react-hooks/exhaustive-deps React Hook useEffect has a missing dependency: 'startTokenExpiryTimer'. Either include it or remove the dependency array.
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:461:9 react-hooks/exhaustive-deps The 'handleFailedLogin' function makes the dependencies of useMemo Hook (at line 838) change on every render. Move it inside the useMemo callback. Alternatively
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:522:5 react-hooks/exhaustive-deps React Hook useCallback has missing dependencies: 'authConfig?.provider', 'handledVerifiedUser', 'navigate', 'resetUserDetails', and 'startTokenExpiryTimer'. Eit
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:567:9 react-hooks/exhaustive-deps The 'initializeAxiosInterceptors' function makes the dependencies of useMemo Hook (at line 838) change on every render. To fix this, wrap the definition of 'ini
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:643:65 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:653:39 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:668:41 sonarjs/no-nested-functions Refactor this code to not nest functions more than 4 levels deep.
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:738:30 sonarjs/cyclomatic-complexity {"message":"Function has a complexity of 17 which is greater than 10 authorized.","cost":7,"secondaryLocations":[{"line":738,"column":29,"endLine":738,"endColum
🟡 src/components/Auth/AuthProviders/AuthProvider.tsx:827:6 react-hooks/exhaustive-deps React Hook useEffect has missing dependencies: 'cleanup', 'fetchAuthConfig', 'initializeAxiosInterceptors', and 'startTokenExpiryTimer'. Either include them or
🟡 src/hooks/currentUserStore/useCurrentUserStore.test.ts:47:12 @typescript-eslint/no-explicit-any Unexpected any. Specify a different type.
🟡 src/hooks/currentUserStore/useCurrentUserStore.test.ts:90:14 @typescript-eslint/no-explicit-any Unexpected any. Specify a different type.
🟡 src/hooks/currentUserStore/useCurrentUserStore.test.ts:116:14 @typescript-eslint/no-explicit-any Unexpected any. Specify a different type.
🟡 src/hooks/currentUserStore/useCurrentUserStore.test.ts:129:16 @typescript-eslint/no-explicit-any Unexpected any. Specify a different type.
🟡 src/hooks/currentUserStore/useCurrentUserStore.test.ts:153:14 @typescript-eslint/no-explicit-any Unexpected any. Specify a different type.
🟡 src/hooks/currentUserStore/useCurrentUserStore.test.ts:197:14 @typescript-eslint/no-explicit-any Unexpected any. Specify a different type.
🟡 src/hooks/currentUserStore/useCurrentUserStore.test.ts:225:16 @typescript-eslint/no-explicit-any Unexpected any. Specify a different type.
🟡 src/hooks/currentUserStore/useCurrentUserStore.test.ts:248:16 @typescript-eslint/no-explicit-any Unexpected any. Specify a different type.
🟡 src/hooks/useAppMode.ts:137:7 sonarjs/expression-complexity Reduce the number of conditional operators (5) used in the expression (maximum allowed 3).

Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

✅ TypeScript Types Auto-Updated

The generated TypeScript types have been automatically updated based on JSON schema changes in this PR.

Copilot AI review requested due to automatic review settings August 6, 2026 10:37

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

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 66%
66.2% (78115/117990) 50.2% (47190/94000) 51.39% (14200/27630)

…ation works

The store is module-level and survives an SPA logout→login on the same tab.
If a prior session left `isForced=true`, `writeAppMode`'s guard would no-op
the initial pin in `hydrateAppModeConfig` and the new session would be stuck
on the previous runtime mode — for example, user A gets 'ai' pinned, logs
out, user B logs in on the same tab, and B's initial pin gets silently
dropped even though the tenant force is unchanged.

Fix: `setForced(null)` first, then `writeAppMode(runtimeMode)`, then
`setForced(wireMode)`. The initial pin always lands regardless of prior
state; every subsequent user-initiated write is still blocked.

Added regression test.

Reported by Gitar review on PR #31099.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 7, 2026 05:54

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

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

✅ Playwright Results — workflow succeeded

Validated commit 3f65de7cea0e7e952a61df06fa6ddcb6f3ae1218 in Playwright run 31156446924, attempt 1.

✅ 1004 passed · ❌ 0 failed · 🟡 1 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) 57m 23s

⏱️ Max setup 3m 19s · max shard execution 20m 9s · max shard-job elapsed before upload 23m 34s · reporting 7s

🌐 208.46 requests/attempt · 2.47 app boots/UI scenario · 12.08% common-shard skew

Optimization targets still in progress:

  • Browser traffic was 208.46 requests per attempt (convergence target: fewer than 200).
  • Application boot ratio was 2.47 per UI scenario (2850 boots / 1152 scenarios; convergence target: at most 1).
Shard Passed Failed Flaky Skipped Lifecycle failed Lifecycle flaky
✅ Shard chromium-01 149 0 0 0 0 0
✅ Shard chromium-02 167 0 0 0 0 0
✅ Shard chromium-03 168 0 0 0 0 0
🟡 Shard chromium-04 157 0 1 0 0 0
✅ Shard chromium-05 195 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 import-export-01 17 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 10 0 0 0 0 0
✅ Shard search-rbac-01 29 0 0 0 0 0
🟡 1 flaky test(s) (passed on retry)
  • Features/Permissions/EntityPermissions.spec.tsDirectory allow common operations permissions (shard chromium-04, 1 retry)

📦 Download artifacts

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

…ched test

The generated `DefaultAppMode` enum exports members as `AI` (all caps) and
`Classic`. The hand-written `CONFIG_MODE_TO_RUNTIME` map used
`DefaultAppMode.Ai` (title case), which is `undefined` at runtime —
`[undefined]: AI_APP_MODE` becomes the key `"undefined"`, so a wire value
of `"ai"` looked up as `CONFIG_MODE_TO_RUNTIME["ai"]` returned `undefined`
and `writeAppMode` was never called. Only the `Classic` case worked. Fix
the reference to `DefaultAppMode.AI`.

Also re-format `useCurrentUserStore.test.ts` — CI's prettier check flagged
it after the last commit even though the file passed prettier locally at
authoring time (line-length drift from an unrelated re-flow).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings August 7, 2026 07:07

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.

@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

@sonarqubecloud

sonarqubecloud Bot commented Aug 7, 2026

Copy link
Copy Markdown

Comment thread conf/openmetadata.yaml
dataset: ${RDF_DATASET:-"openmetadata"}
inferenceEnabled: ${RDF_INFERENCE_ENABLED:-false}

# App Configuration

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

let's remove it from here , we can keep this only in the database

@mohityadav766 mohityadav766 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Needs some work. Use SettingsCache to load the setting into the database if not existing, else don't .
We don't need to store the settings in opemetadata.yaml.

@chirag-madlani

Copy link
Copy Markdown
Collaborator Author

Closing in favor of the v2 architecture per reviewer feedback (Pablo + Mohit). appConfiguration moves from yaml-only hard force to a DB-backed SettingsCache setting (yaml seeded on first boot only), admin-mutable via API, and becomes a soft default — user and admin can always override.

New combined PR forthcoming — will link when opened. Branch feat/app-mode-tenant-override stays on the remote for reference.

@gitar-bot

gitar-bot Bot commented Aug 7, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 1 resolved / 1 findings

Adds YAML-configured tenant-level app mode override supporting bootstrap hydration and user write guards, addressing the stale isForced blocks force re-pin on same-tab user switch finding. No issues found.

✅ 1 resolved
Bug: Stale isForced blocks force re-pin on same-tab user switch

📄 openmetadata-ui/src/main/resources/ui/src/hooks/currentUserStore/useAppModeConfig.ts:31-35 📄 openmetadata-ui/src/main/resources/ui/src/hooks/currentUserStore/useAppModeConfig.ts:61-68 📄 openmetadata-ui/src/main/resources/ui/src/hooks/useAppMode.ts:272-274 📄 openmetadata-ui/src/main/resources/ui/src/components/Auth/AuthProviders/AuthProvider.tsx:256
useAppModeConfig.isForced is never reset on logout (onLogoutHandler calls clearAppMode() but not setForced(null)), and the store is module-level so it survives an SPA logout→login without a page reload. hydrateAppModeConfig relies on the invariant that its initial writeAppMode(runtimeMode) pin runs while the store still looks unforced (writeAppMode no-ops when isForced is true). When user A logs out and user B logs in in the same tab, isForced is still true, so hydrateAppModeConfig's writeAppMode is blocked and the runtime mode is never re-pinned — user B is left in the reset DEFAULT_APP_MODE ('default'/classic) even when the tenant forces 'ai', and useIsAiMode() returns the wrong value. Fix by clearing the force on logout (reset useAppModeConfig in clearAppMode/logout) and/or having hydrateAppModeConfig call setForced(null) before the initial writeAppMode.

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

safe to test Add this label to run secure Github workflows on PRs UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants