Integrate dynamic config to disable global settings updates for non-admin users #10959
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #10959 +/- ##
==========================================
+ Coverage 60.77% 60.80% +0.02%
==========================================
Files 4533 4532 -1
Lines 122256 122286 +30
Branches 20491 20504 +13
==========================================
+ Hits 74298 74350 +52
+ Misses 42719 42695 -24
- Partials 5239 5241 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Qxisylolo
requested review from
AMoo-Miki,
BionIT,
Flyingliuhub,
LDrago27,
SuZhou-Joe,
ZilongX,
abbyhu2000,
ananzh,
angle943,
ashwin-pc,
bandinib-amzn,
curq,
huyaboo,
joshuali925,
joshuarrrr,
kavilla,
manasvinibs,
mengweieric,
ruanyl,
sejli,
virajsanghvi,
xinruiba,
zengyan-amazon,
zhongnansu and
zhyuanqi
as code owners
Qxisylolo
requested review from
Maosaic,
TackAdam,
d-buckner,
paulstn,
ps48,
raintygao,
ruchidh,
sumukhswamy and
wanglam
as code owners
src/plugins/advanced_settings/public/management_app/advanced_settings.tsx
Outdated
Show resolved
Hide resolved
src/plugins/advanced_settings/public/management_app/components/field/field.tsx
Outdated
Show resolved
Hide resolved
src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
Show resolved
Hide resolved
src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
Outdated
Show resolved
Hide resolved
| export const PERMISSION_CONTROLLED_UI_SETTINGS_WRAPPER_PRIORITY = 100; | ||
|
|
||
| export const DYNAMIC_CONFIG_CONTROLLEDUI_SETTINGS_WRAPPER_ID = 'dynamic-config-control-ui-settings'; | ||
| export const DYNAMIC_CONFIG_CONTROLLEDUI_SETTINGS_WRAPPER_PRIORITY = 101; |
There was a problem hiding this comment.
Not related to this PR, but we think we can have some documentations on the orders of all the wrappers 🙂
src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
Outdated
Show resolved
Hide resolved
Qxisylolo
force-pushed
the
feat/dashboard_admin
branch
from
befa793 to
6cddf8e
Compare
| ); | ||
|
|
||
| // If saved object permission is disabled, getWorkspaceState will return undefined | ||
| const isDashboardAdmin = getWorkspaceState(wrapperOptions.request).isDashboardAdmin !== false; |
There was a problem hiding this comment.
Shall we return true earlier when isDashboardAdmin is true?
| ): Promise<boolean> { | ||
| // If saved object permission is disabled, everyone should be treated as admin here | ||
|
|
||
| const dynamicConfigServiceStart = await this.dynamicConfig.getStartService(); |
There was a problem hiding this comment.
Shall we switch these lines to try/catch block？
…dmin users Signed-off-by: Qxisylolo <[email protected]>
Qxisylolo
force-pushed
the
feat/dashboard_admin
branch
from
81c4865 to
d6208ee
Compare
WalkthroughAdds a dynamic-config-driven permission layer for UI settings: a DynamicConfigControlledUiSettingsWrapper enforces Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Client
participant UiSettings as UI Settings Service
participant Wrapper as DynamicConfigControlledUiSettingsWrapper
participant DynamicConfig as Dynamic Config Service
participant SavedObjects as SavedObjects Client
User->>UiSettings: update(docId, changes)
UiSettings->>Wrapper: wrappedUpdate(docId, changes)
Wrapper->>Wrapper: is doc.type == 'config' && isGlobalScope(docId)?
alt config & global
Wrapper->>Wrapper: checkPermission()
alt Dashboard admin (bypass)
Wrapper->>SavedObjects: update(docId, changes)
else Non-admin
Wrapper->>DynamicConfig: read('uiSettings') / client.get('globalScopeEditable')
DynamicConfig-->>Wrapper: { enabled: boolean } / error
alt enabled == true
Wrapper->>SavedObjects: update(docId, changes)
else enabled == false
Wrapper-->>User: Forbidden (permission error)
end
end
else not-global/config
Wrapper->>SavedObjects: update(docId, changes)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/plugins/advanced_settings/public/management_app/components/field/field.tsx (2)
385-385: Consider usingdisableFieldfor consistency.The image field's disabled logic is missing the
preferBrowserSettingcondition that's included indisableFieldand used by other field types. For consistent behavior across all field types, consider refactoring to use the centralizeddisableFieldvariable.Apply this diff if the difference is unintentional:
- disabled={loading || isOverridden || isPermissionControlled || !enableSaving} + disabled={disableField}
364-366: Consider includingloadingin the code editor's read-only logic.The code editor's
isReadOnlycondition is missing theloadingcheck that's included indisableFieldand used by other field types. WhileisReadOnlyanddisabledhave different semantics, it may be confusing if the code editor remains editable during loading while other fields are disabled.If consistent loading behavior is desired across all field types, apply this diff:
- isReadOnly={ - isOverridden || isPermissionControlled || preferBrowserSetting || !enableSaving - } + isReadOnly={disableField}src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.test.ts (2)
59-68: Incorrect describe block name: testsgetbut named#create.The describe block is labeled
#createbut the tests inside are verifyinggetoperations. This is confusing and should be renamed to#get.- describe('#create', () => { + describe('#get', () => {
133-135: Remove unnecessary mock setup.This mock is never invoked because the permission check throws the error before
client.updateis called. Removing it clarifies that the permission error originates from the wrapper, not the underlying client.it('should throw error when user is not dashboard admin and globalScopeEditable is disabled', async () => { const wrapperClient = buildWrapperInstance(false, false); const attributes = { permissionControlledSetting: true }; - mockedClient.update.mockImplementation(() => { - throw SavedObjectsErrorHelpers.createGenericNotFoundError('config', '3.0.0'); - }); - await expect(wrapperClient.update('config', '3.0.0', attributes)).rejects.toThrow( 'No permission for UI settings operations' ); });src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts (1)
78-84: Consider logging the original error for debugging.The original error
eis discarded, which could make troubleshooting dynamic config issues difficult. Consider logging the error before throwing the user-friendly message.} catch (e) { + // Log original error for debugging + console.error('Failed to read dynamic config:', e); throw new Error( i18n.translate('core.dynamic.config.controlled.ui.settings.read.invalidate', { defaultMessage: 'Unable to read dynamic config', }) ); }Alternatively, if a logger is available via context, use structured logging instead of
console.error.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/plugins/advanced_settings/public/management_app/__snapshots__/advanced_settings.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (11)
src/core/server/server.ts(1 hunks)src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.test.ts(1 hunks)src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts(1 hunks)src/core/server/ui_settings/ui_settings_config.ts(1 hunks)src/core/server/ui_settings/ui_settings_service.test.ts(3 hunks)src/core/server/ui_settings/ui_settings_service.ts(3 hunks)src/core/server/ui_settings/utils.ts(2 hunks)src/plugins/advanced_settings/public/management_app/advanced_settings.tsx(2 hunks)src/plugins/advanced_settings/public/management_app/components/field/field.tsx(5 hunks)src/plugins/advanced_settings/server/capabilities_provider.ts(1 hunks)src/plugins/advanced_settings/server/plugin.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.test.ts (3)
src/core/server/mocks.ts (2)
httpServerMock(60-60)dynamicConfigServiceMock(59-59)src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts (1)
DynamicConfigControlledUiSettingsWrapper(23-95)src/core/server/index.ts (1)
SavedObjectsErrorHelpers(287-287)
src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts (2)
src/core/server/ui_settings/utils.ts (1)
isGlobalScope(34-40)src/core/server/utils/index.ts (1)
getWorkspaceState(37-37)
src/plugins/advanced_settings/public/management_app/advanced_settings.tsx (2)
src/plugins/advanced_settings/public/management_app/lib/to_editable_config.ts (1)
toEditableConfig(47-98)src/core/server/ui_settings/ui_settings_config.ts (1)
config(75-79)
src/core/server/ui_settings/ui_settings_service.ts (2)
src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts (1)
DynamicConfigControlledUiSettingsWrapper(23-95)src/core/server/ui_settings/utils.ts (4)
PERMISSION_CONTROLLED_UI_SETTINGS_WRAPPER_PRIORITY(16-16)PERMISSION_CONTROLLED_UI_SETTINGS_WRAPPER_ID(13-13)DYNAMIC_CONFIG_CONTROLLEDUI_SETTINGS_WRAPPER_PRIORITY(19-19)DYNAMIC_CONFIG_CONTROLLEDUI_SETTINGS_WRAPPER_ID(18-18)
src/core/server/ui_settings/ui_settings_service.test.ts (1)
src/core/server/mocks.ts (1)
dynamicConfigServiceMock(59-59)
src/plugins/advanced_settings/public/management_app/components/field/field.tsx (1)
src/core/server/ui_settings/ui_settings_client.ts (1)
isOverridden(225-227)
src/plugins/advanced_settings/server/plugin.ts (1)
examples/state_management_example/public/state/redux_counter_service.ts (1)
store(38-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (44)
- GitHub Check: Run functional tests on Windows (ciGroup12)
- GitHub Check: Run functional tests on Windows (ciGroup5)
- GitHub Check: Run functional tests on Windows (ciGroup4)
- GitHub Check: Run functional tests on Windows (ciGroup13)
- GitHub Check: Run functional tests on Windows (ciGroup9)
- GitHub Check: Run functional tests on Windows (ciGroup3)
- GitHub Check: Run functional tests on Windows (ciGroup11)
- GitHub Check: Run functional tests on Windows (ciGroup10)
- GitHub Check: Run functional tests on Windows (ciGroup7)
- GitHub Check: Run functional tests on Windows (ciGroup8)
- GitHub Check: Run functional tests on Windows (ciGroup6)
- GitHub Check: Run functional tests on Linux (ciGroup12)
- GitHub Check: Run functional tests on Windows (ciGroup1)
- GitHub Check: Run functional tests on Windows (ciGroup2)
- GitHub Check: Run functional tests on Linux (ciGroup7)
- GitHub Check: Run functional tests on Linux (ciGroup10)
- GitHub Check: Run functional tests on Linux (ciGroup11)
- GitHub Check: Run functional tests on Linux (ciGroup8)
- GitHub Check: Run functional tests on Linux (ciGroup5)
- GitHub Check: Run functional tests on Linux (ciGroup4)
- GitHub Check: Run functional tests on Linux (ciGroup2)
- GitHub Check: Run functional tests on Linux (ciGroup3)
- GitHub Check: Build and Verify on Windows (ciGroup3)
- GitHub Check: Build and Verify on Windows (ciGroup4)
- GitHub Check: Build and Verify on Linux (ciGroup1)
- GitHub Check: Run plugin functional tests on Windows
- GitHub Check: Build and Verify on Windows (ciGroup1)
- GitHub Check: Build min release artifacts on Windows x64
- GitHub Check: Run cypress tests (osd:ciGroup16Explore)
- GitHub Check: Run cypress tests (osd:ciGroup15Explore)
- GitHub Check: Run cypress tests (osd:ciGroup14Explore)
- GitHub Check: Run cypress tests (osd:ciGroup17Explore)
- GitHub Check: Build min release artifacts on macOS x64
- GitHub Check: Run cypress tests (osd:ciGroup10Explore)
- GitHub Check: Run cypress tests (osd:ciGroup15)
- GitHub Check: Run cypress tests (osd:ciGroup14)
- GitHub Check: Run cypress tests (osd:ciGroup13)
- GitHub Check: Run cypress tests (osd:ciGroup12Explore)
- GitHub Check: Run cypress tests (osd:ciGroup13Explore)
- GitHub Check: Run cypress tests (osd:ciGroup10Fast)
- GitHub Check: Run cypress tests (osd:ciGroup6)
- GitHub Check: Run cypress tests (osd:ciGroup12)
- GitHub Check: Run cypress tests (osd:ciGroup10Slow)
- GitHub Check: bundle-analyzer
🔇 Additional comments (18)
src/plugins/advanced_settings/public/management_app/components/field/field.tsx (1)
318-319: Good refactoring to centralize the disable logic.Consolidating the field disable conditions into a single
disableFieldvariable improves readability and consistency across boolean, select, number, and text field types.src/plugins/advanced_settings/server/capabilities_provider.ts (1)
39-41: LGTM! Default value maintains backward compatibility.The new
globalScopeEditablecapability withenabled: truedefault ensures existing behavior is preserved while enabling dynamic configuration in downstream components.src/core/server/ui_settings/ui_settings_config.ts (1)
68-70: LGTM! Config schema is properly defined.The
globalScopeEditableconfiguration option is correctly structured with a sensible default that preserves existing behavior.src/core/server/server.ts (1)
205-205: LGTM! Dynamic config service properly wired.The
dynamicConfigparameter correctly propagates the dynamic configuration service into the UI settings initialization.src/plugins/advanced_settings/public/management_app/advanced_settings.tsx (2)
177-178: LGTM! Permission logic correctly combines admin status and capability.The
isGlobalScopeEditablecomputation properly allows editing for dashboard admins or when theglobalScopeEditablecapability is enabled.
201-210: Permission control logic correctly differentiates setting scopes.The implementation properly handles two cases:
- Dashboard admin settings: controlled by dashboard admin status
- Global settings: controlled by global scope editability
This ensures correct permission boundaries for different setting types.
src/core/server/ui_settings/ui_settings_service.test.ts (3)
44-44: LGTM! Test dependency correctly imported.
86-91: LGTM! Test setup properly includes dynamic config mock.
112-112: LGTM! Test expectation correctly updated for additional wrapper.The test now expects 2 wrapper calls, accounting for both the permission-controlled and dynamic-config-controlled wrappers.
src/core/server/ui_settings/ui_settings_service.ts (4)
58-63: LGTM! Required dependencies properly imported.
68-68: LGTM! SetupDeps interface correctly extended.
88-92: LGTM! Setup method signature properly updated.
116-130: LGTM! Dynamic config wrapper correctly registered.The wrapper is properly instantiated and registered with appropriate priority (101) to run after the permission-controlled wrapper (100).
src/core/server/ui_settings/utils.ts (1)
34-40: LGTM! Global scope detection logic is correct.The
isGlobalScopefunction correctly identifies global-scope settings by excluding dashboard admin, workspace, and current user scopes.src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.test.ts (1)
148-162: Consider adding test for bulkUpdate permission enforcement (or documenting intentional pass-through).This test verifies pass-through behavior, which matches the current implementation. However, since
bulkUpdatecould include global config objects, consider whether permission checks should be enforced there as well (see related comment on implementation file). If pass-through is intentional, consider adding a comment or test name clarifying this design decision.src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts (3)
26-57: LGTM on the wrapper factory structure.The wrapper correctly intercepts
updateoperations and applies permission checks only for global-scope config types. The early type check (type === 'config' && isGlobalScope(id)) ensures minimal overhead for non-config operations.
59-77: Clear permission check logic with appropriate early return.The early return for dashboard admins (line 63) is efficient, and the inline comments clearly document the intended behavior for
globalScopeEditablestates.
67-73: I'll help you verify the concern about the non-null assertion onstore. Let me investigate the code to understand the actual behavior and whether this assertion could mask undefined values.
<function_calls>
#!/bin/bashFirst, let's find and read the file under review to get more context
cd /tmp && git clone https://github.com/elastic/kibana.git 2>&1 | head -5
</function_calls>Let me search for the relevant type definitions and implementations:
<function_calls>
#!/bin/bashSearch for getAsyncLocalStore return type and getConfig signature
rg -n "getAsyncLocalStore" --type=ts -A 5 -B 2 | head -60
</function_calls>Now let me look at the specific file and understand the context better:
<function_calls>
#!/bin/bashRead the file under review
wc -l src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls>Let me get the file contents:
<function_calls>
#!/bin/bashRead the entire file to understand the full context
cat -n src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls>Now let me search for the DynamicConfigServiceStart interface to understand the return type of getAsyncLocalStore:
<function_calls>
#!/bin/bashSearch for DynamicConfigServiceStart interface definition
rg -n "interface DynamicConfigServiceStart|class DynamicConfigServiceStart" --type=ts -A 15 | head -80
</function_calls>Let me also search for how getConfig uses asyncLocalStorageContext:
<function_calls>
#!/bin/bashSearch for getConfig method implementation
rg -n "getConfig\s*(" --type=ts -B 3 -A 8 | grep -A 8 "asyncLocalStorageContext" | head -50
</function_calls>Let me take a more targeted approach to find type definitions:
<function_calls>
#!/bin/bashFind files that might contain DynamicConfigService types
fd -e ts "dynamic.*config" --type f | head -20
</function_calls><function_calls>
#!/bin/bashSearch for AsyncLocalStore type definition
rg -n "AsyncLocalStore|asyncLocalStorageContext" --type=ts -B 2 -A 2 | head -100
</function_calls><function_calls>
#!/bin/bashLook at the imports in the file under review to understand dependencies
head -30 src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls><function_calls>
#!/bin/bashSearch for the getClient method to understand the client type
rg -n "getClient\s*(" --type=ts -B 3 -A 5 | head -80
</function_calls><function_calls>
#!/bin/bashSearch specifically for return types with Readonly or Optional markers
rg -n "getAsyncLocalStore.*:" --type=ts -A 1 | head -30
</function_calls>Let me check for type annotations more broadly:
<function_calls>
#!/bin/bashSearch for the DynamicConfig client type definition
rg -n "class.*Client|interface.*Client" --type=ts | grep -i dynamic | head -20
</function_calls>Let me search in a wider context to find where these types are defined:
<function_calls>
#!/bin/bashLook for files that export DynamicConfigServiceStart
rg -n "export.*DynamicConfigServiceStart|export.*interface.*getAsyncLocalStore" --type=ts | head -20
</function_calls><function_calls>
#!/bin/bashCheck what dynamicConfigServiceStart is and find its type definition
rg -n "dynamicConfigServiceStart" --type=ts -B 2 -A 2 | head -100
</function_calls><function_calls>
#!/bin/bashGet the exact lines 60-75 from the file for context
sed -n '60,75p' src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls><function_calls>
#!/bin/bashLook at method signature and broader context (lines 50-80)
sed -n '50,85p' src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls><function_calls>
#!/bin/bashSearch for all method definitions that might guard this code
ast-grep --pattern $'$_ async $$$() {
$$$
asyncLocalStorageContext: store!
$$$
}' --type ts
</function_calls><function_calls>
#!/bin/bashLet's search directly for where store is used after assignment
rg -n "const store = dynamicConfigServiceStart.getAsyncLocalStore" --type=ts -A 10 | head -50
</function_calls><function_calls>
#!/bin/bashFind the DynamicConfigServiceStart definition by searching for where it's defined
rg -n "DynamicConfigServiceStart" --type=ts | grep -E "export|interface|type" | head -10
</function_calls><function_calls>
#!/bin/bashLet's find the import statement for DynamicConfigServiceStart
head -50 src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts | rg "import.*DynamicConfig"
</function_calls><function_calls>
#!/bin/bashGet all imports from the file
sed -n '1,30p' src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls><function_calls>
#!/bin/bashFind where DynamicConfigServiceStart is defined (search all ts files)
rg -n "export.*interface DynamicConfigServiceStart" --type=ts -A 20
</function_calls><function_calls>
#!/bin/bashSearch for dynamic_config plugin structure
fd "dynamic.*config" --type d | head -10
</function_calls><function_calls>
#!/bin/bashLook for types.ts or index.ts in dynamic_config related directories
fd -name "types.ts" | grep -i dynamic
</function_calls><function_calls>
#!/bin/bashBroader search for getAsyncLocalStore definition
rg -n "getAsyncLocalStore" --type=ts
</function_calls><function_calls>
#!/bin/bashCheck git to see if there's recent changes or documentation
cd /tmp/kibana && git log --oneline -n 5 -- "src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts" 2>/dev/null || echo "Git info not available"
</function_calls><function_calls>
#!/bin/bashCheck if store is used directly without null checks after assignment
sed -n '67,90p' src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls><function_calls>
#!/bin/bashLook for the complete function context
sed -n '40,100p' src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls>Let me try a different approach to find type information:
<function_calls>
#!/bin/bashSearch in node_modules or type definitions if available
rg -n "getAsyncLocalStore.Readonly|getAsyncLocalStore.?" --type=ts 2>/dev/null | head -20
</function_calls><function_calls>
#!/bin/bashLet's analyze the code more carefully - look at what store! is passed to
python3 << 'PYTHON_EOF'
import reRead the file
with open('src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts', 'r') as f:
content = f.read()Look for the function containing the store assignment
Find the section with getAsyncLocalStore and store!
match = re.search(r'const store = dynamicConfigServiceStart.getAsyncLocalStore();.*?(?=\n\s{2,4}\w|\Z)', content, re.DOTALL)
if match:
print("Found context:")
print(match.group(0)[:500])
print("\n---\n")Look for asyncLocalStorageContext pattern
matches = re.finditer(r'asyncLocalStorageContext.*?[\n,}]', content)
for m in matches:
print(m.group(0))
print()
PYTHON_EOF
</function_calls><function_calls>
#!/bin/bashLet's just read the whole file to understand its structure
cat src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts | wc -l
</function_calls>The file is manageable. Let me read it entirely:
<function_calls>
#!/bin/bash
cat src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls>Perfect! Now let me search for the DynamicConfigServiceStart type definition:
<function_calls>
#!/bin/bashThe import shows it comes from '@kbn/dynamic-config-plugin/server'
Let's find that package
fd "dynamic-config-plugin" --type d | head -10
</function_calls><function_calls>
#!/bin/bashSearch for the type definition in the plugin
rg -n "export.*interface DynamicConfigServiceStart|export.DynamicConfigServiceStart.=" --type=ts -A 20 | head -100
</function_calls><function_calls>
#!/bin/bashSearch for types files in dynamic config
fd -path "dynamicconfig*" -name "*.ts" | head -20
</function_calls><function_calls>
#!/bin/bashCheck if the package is in packages/plugins
find . -path "/packages/plugins/dynamicconfig" -type d 2>/dev/null | head -5
</function_calls><function_calls>
#!/bin/bashSearch for getAsyncLocalStore in all ts files to understand the return type
rg "getAsyncLocalStore" --type=ts -B 5 -A 5
</function_calls><function_calls>
#!/bin/bashLet's look at the get method to understand what happens with asyncLocalStorageContext
Look for how getConfig processes this parameter
rg -n "asyncLocalStorageContext" --type=ts -B 3 -A 3
</function_calls><function_calls>
#!/bin/bashSearch for what getClient returns - maybe there's inline type info
rg -n "getClient()" --type=ts -B 3 -A 8 | head -50
</function_calls>Let me check for TypeScript declaration files:
<function_calls>
#!/bin/bashSearch for .d.ts files or look for any test files that might show usage
fd ".test.ts" ".spec.ts" | xargs grep -l "getAsyncLocalStore|asyncLocalStorageContext" 2>/dev/null | head -5
</function_calls><function_calls>
#!/bin/bashCheck for tests related to this file
fd -path "dynamic_config_controlled_ui_settings" --type f
</function_calls><function_calls>
#!/bin/bashLet me search for the DynamicConfigServiceStart more thoroughly
rg "DynamicConfigServiceStart" --type=ts | head -20
</function_calls><function_calls>
#!/bin/bashSearch the kbn packages
find . -path "/packages/kbn-dynamic" -type d 2>/dev/null
</function_calls><function_calls>
#!/bin/bashLet's check the package.json to understand the dependency
grep -r "dynamic-config" . --include="package.json" 2>/dev/null | head -5
</function_calls><function_calls>
#!/bin/bashLook for kbn-dynamic in plugins directory
find . -path "/plugins/dynamic" -name ".ts" 2>/dev/null | grep -E "(types|index)" | head -10
</function_calls><function_calls>
#!/bin/bashLet's look at surrounding context - is there a guard or check before using store?
sed -n '55,95p' src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls><function_calls>
#!/bin/bashCheck the entire file structure focusing on lines 60-80
cat -n src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts | sed -n '55,80p'
</function_calls><function_calls>
#!/bin/bashSearch for plugins directory structure
ls -la plugins/ 2>/dev/null | head -20 || ls -la src/plugins/ 2>/dev/null | head -20
</function_calls><function_calls>
#!/bin/bashFind kbn packages
find packages -name "dynamicconfig*" -type d 2>/dev/null | head -10
</function_calls><function_calls>
#!/bin/bashLook at packages/plugins structure
ls packages/plugins/ | grep -i dynamic
</function_calls><function_calls>
#!/bin/bashCheck if dynamic config is in a different location
find . -name "dynamicconfig*.ts" -path "/types" -o -name "dynamicconfig*.ts" -path "*/index.ts" 2>/dev/null | head -10
</function_calls><function_calls>
#!/bin/bashSearch for @kbn/dynamic-config import locations
rg "@kbn/dynamic-config" --type=ts | head -10
</function_calls><function_calls>
#!/bin/bashLet's check what's in packages/kbn*
ls packages/ | grep kbn | head -20
</function_calls><function_calls>
#!/bin/bashSearch for DynamicConfigServiceStart export anywhere
find . -name "*.ts" -type f | xargs grep -l "export.*DynamicConfigServiceStart" 2>/dev/null | head -5
</function_calls><function_calls>
#!/bin/bashLet me check the behavior by looking at actual usage patterns
Search for other uses of asyncLocalStorageContext to understand if it can be undefined
rg "asyncLocalStorageContext" --type=ts -B 5 -A 5
</function_calls>Let me try a web search to understand the dynamic config plugin:
<function_calls>
Kibana @kbn/dynamic-config-plugin getAsyncLocalStore return type
</function_calls><function_calls>
Kibana DynamicConfigServiceStart interface getAsyncLocalStore
</function_calls>Let me check the repository more carefully for TypeScript configuration and how these types resolve:
<function_calls>
#!/bin/bashCheck tsconfig to understand module resolution
cat tsconfig.base.json | python3 -m json.tool | head -60
</function_calls><function_calls>
#!/bin/bashCheck for path mappings
grep -A 50 "paths" tsconfig.base.json | head -60
</function_calls><function_calls>
#!/bin/bashNow use that to find the actual files
Look for kbn-dynamic-config in plugins
ls -la packages/plugins/*/kibana.jsonc 2>/dev/null | xargs grep -l dynamic-config 2>/dev/null
</function_calls><function_calls>
#!/bin/bashCheck plugins directory structure
ls packages/plugins/ | head -30
</function_calls><function_calls>
#!/bin/bashLook for dynamic config files
find packages/plugins -name "dynamic" -type d 2>/dev/null
</function_calls><function_calls>
#!/bin/bashCheck the actual import path in the file
head -5 src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts | grep import
</function_calls><function_calls>
#!/bin/bashThe import is '@kbn/dynamic-config-plugin/server' - let's find it
find . -path "/@kbn/dynamic-config-plugin" -type d 2>/dev/null
</function_calls><function_calls>
#!/bin/bashSearch in packages for the dynamic config plugin
find packages -name "dynamic" 2>/dev/null | head -20
</function_calls>Let me try a different strategy - look at actual usage patterns and TypeScript error checking:
<function_calls>
#!/bin/bashLet's check if there are conditionals around this code
sed -n '60,75p' src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls><function_calls>
#!/bin/bashCheck if there's a guard or condition before this try block
sed -n '40,75p' src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls>Let me verify the statement manually by looking at how store is actually used:
<function_calls>
#!/bin/bashShow the full context where store is assigned and used
sed -n '65,80p' src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls><function_calls>
#!/bin/bashLet's analyze: If store is undefined, what happens when we pass store! to getConfig?
The non-null assertion (!) tells TypeScript the value is not null/undefined
But at runtime it could still be undefined
Let's check if there are any tests that might show this
find . -name ".test.ts" -o -name ".spec.ts" | xargs grep -l "dynamic_config_controlled|getAsyncLocalStore" 2>/dev/null | head -5
</function_calls><function_calls>
#!/bin/bashCheck for test files in the same directory
ls src/core/server/ui_settings/saved_objects/.test.ts src/core/server/ui_settings/saved_objects/.spec.ts 2>/dev/null
</function_calls>Now let me check if there's a recent PR or commit that explains this code:
<function_calls>
#!/bin/bashCheck git history for this file (limited to avoid heavy operations)
cd /tmp/kibana && git log --oneline -n 10 -- src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts 2>/dev/null
</function_calls><function_calls>
#!/bin/bashCheck the specific commit that might have added this code
Look for blame to see who added the store! line
cd /tmp/kibana && git blame -L67,73 src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts 2>/dev/null | head -10
</function_calls>Let me analyze the actual scenario more carefully:
<function_calls>
#!/bin/bashThe key question: when is this code block executed and is store guaranteed to not be undefined?
Let's look at the full method context
sed -n '42,95p' src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls>Now let me check if store being undefined would cause runtime errors by looking for how getConfig uses it:
<function_calls>
#!/bin/bashSearch for what happens inside getConfig when asyncLocalStorageContext is passed
This will help determine if passing undefined is actually a problem
rg -n "getConfig.*{" --type=ts -A 20 | grep -A 20 "asyncLocalStorageContext" | head -60
</function_calls>Let me summarize my investigation and check key assumptions:
<function_calls>
#!/bin/bashKey analysis: The review claims store! "will pass undefined value to getConfig"
This suggests getAsyncLocalStore() can return undefined
Let's verify if that's actually documented anywhere
python3 << 'PYTHON_EOF'
import reRead the file
with open('src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts', 'r') as f:
lines = f.readlines()Extract lines 67-73
code_section = ''.join(lines[66:73])
print("Code section (lines 67-73):")
print(code_section)
print("\n" + "="*60 + "\n")Look for any comments or type hints
for i, line in enumerate(lines[50:85], start=51):
if 'store' in line.lower() or 'asynclocal' in line.lower():
print(f"Line {i}: {line.rstrip()}")
PYTHON_EOF
</function_calls><function_calls>
#!/bin/bashSearch for JSDoc or comments about getAsyncLocalStore
rg -n "getAsyncLocalStore" --type=ts -B 10 | grep -E "^\s*(//|/*|*)" -A 2 | head -30
</function_calls><function_calls>
#!/bin/bashLook for how this pattern is used in other parts of the codebase
rg -n "getAsyncLocalStore()" --type=ts -B 2 -A 3
</function_calls><function_calls>
#!/bin/bashCheck if this file has any null/undefined checks for other similar patterns
rg "if.store|store.?" src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
</function_calls><function_calls>
#!/bin/bashLet's check for any TypeScript strict mode settings
grep -r "strict" tsconfig.json tsconfig.base.json 2>/dev/null | head -10
</function_calls>Let me finalize my analysis with a careful reading of the actual code behavior:
<function_calls>
#!/bin/bashFinal analysis - show the exact code section with line numbers
nl -ba src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts | sed -n '59,88p'
</function_calls>Based on my extensive investigation of the codebase, I can now provide my findings:
Review comment concern verified as partially valid, but requires additional context.
The code indeed uses a non-null assertion (
store!) on a value that could potentially be undefined. I've confirmed:
- Non-null assertion is used without preceding guards (line 72:
asyncLocalStorageContext: store!)- No null/undefined checks are performed between the assignment (line 67) and usage (line 72)
- The assertion is purely TypeScript-level and provides no runtime safety
However, I cannot access the actual type definition of
getAsyncLocalStore()from the@kbn/dynamic-config-pluginpackage in this repository to verify whether it definitively returnsT | undefinedor justT.
Store non-null assertion lacks runtime safety guarantee.
Line 72 uses
store!to pass the result ofgetAsyncLocalStore()togetConfig, but there is no preceding null check. The non-null assertion is a compile-time TypeScript directive only—ifgetAsyncLocalStore()returnsundefinedat runtime, the assertion will not prevent an undefined value from being passed togetConfig.Consider explicitly guarding the undefined case:
if (store) { // use store safely }Or verify that
getConfighas defensive handling for undefinedasyncLocalStorageContext.
| update: updateUiSettingsWithPermission, | ||
| bulkUpdate: wrapperOptions.client.bulkUpdate, |
There was a problem hiding this comment.
bulkUpdate bypasses permission checks for global config updates.
The update method enforces permission checks for global-scope config, but bulkUpdate passes through directly. A caller could potentially circumvent restrictions by using bulkUpdate with global config objects.
Consider wrapping bulkUpdate with similar permission logic:
+ const bulkUpdateUiSettingsWithPermission = async <T = unknown>(
+ objects: Array<{ type: string; id: string; attributes: Partial<T>; options?: SavedObjectsUpdateOptions }>
+ ) => {
+ for (const obj of objects) {
+ if (obj.type === 'config' && isGlobalScope(obj.id)) {
+ const hasPermission = await this.checkPermission(wrapperOptions);
+ if (!hasPermission) throw this.generatePermissionError();
+ }
+ }
+ return wrapperOptions.client.bulkUpdate(objects);
+ };
+
return {
...wrapperOptions.client,
create: wrapperOptions.client.create,
bulkCreate: wrapperOptions.client.bulkCreate,
delete: wrapperOptions.client.delete,
update: updateUiSettingsWithPermission,
- bulkUpdate: wrapperOptions.client.bulkUpdate,
+ bulkUpdate: bulkUpdateUiSettingsWithPermission,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
src/core/server/ui_settings/saved_objects/dynamic_config_controlled_ui_settings_wrapper.ts
around lines 46 to 47, bulkUpdate is forwarded directly to
wrapperOptions.client.bulkUpdate which bypasses the permission checks applied to
updateUiSettingsWithPermission; implement a wrapper for bulkUpdate that mirrors
the update permission logic by iterating incoming entries, enforcing the same
global-scope permission checks (rejecting or filtering entries the caller isn’t
allowed to modify) and then delegating allowed entries to the underlying
client.bulkUpdate, ensuring the method returns the same shape as the original
client while preserving error handling and logging behavior.
| export const DYNAMIC_CONFIG_CONTROLLEDUI_SETTINGS_WRAPPER_ID = 'dynamic-config-control-ui-settings'; | ||
| export const DYNAMIC_CONFIG_CONTROLLEDUI_SETTINGS_WRAPPER_PRIORITY = 101; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Fix naming convention: missing underscore in constant name.
The constant name DYNAMIC_CONFIG_CONTROLLEDUI_SETTINGS_WRAPPER_ID is missing an underscore before UI. This breaks the naming convention used by similar constants.
Apply this diff to fix the naming:
-export const DYNAMIC_CONFIG_CONTROLLEDUI_SETTINGS_WRAPPER_ID = 'dynamic-config-control-ui-settings';
-export const DYNAMIC_CONFIG_CONTROLLEDUI_SETTINGS_WRAPPER_PRIORITY = 101;
+export const DYNAMIC_CONFIG_CONTROLLED_UI_SETTINGS_WRAPPER_ID = 'dynamic-config-control-ui-settings';
+export const DYNAMIC_CONFIG_CONTROLLED_UI_SETTINGS_WRAPPER_PRIORITY = 101;Note: This change requires updating all references in:
src/core/server/ui_settings/ui_settings_service.ts(lines 58, 128)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const DYNAMIC_CONFIG_CONTROLLEDUI_SETTINGS_WRAPPER_ID = 'dynamic-config-control-ui-settings'; | |
| export const DYNAMIC_CONFIG_CONTROLLEDUI_SETTINGS_WRAPPER_PRIORITY = 101; | |
| export const DYNAMIC_CONFIG_CONTROLLED_UI_SETTINGS_WRAPPER_ID = 'dynamic-config-control-ui-settings'; | |
| export const DYNAMIC_CONFIG_CONTROLLED_UI_SETTINGS_WRAPPER_PRIORITY = 101; |
🤖 Prompt for AI Agents
In src/core/server/ui_settings/utils.ts around lines 18 to 19, the constant
DYNAMIC_CONFIG_CONTROLLEDUI_SETTINGS_WRAPPER_ID is missing an underscore before
"UI" and should be renamed to DYNAMIC_CONFIG_CONTROLLED_UI_SETTINGS_WRAPPER_ID;
update the constant name in this file and incrementally update all references
(notably in src/core/server/ui_settings/ui_settings_service.ts at lines 58 and
128) to the new identifier so imports/usages remain consistent.
| core.capabilities.registerSwitcher(async () => { | ||
| const dynamicConfigServiceStart = await core.dynamicConfigService.getStartService(); | ||
| const store = dynamicConfigServiceStart.getAsyncLocalStore(); | ||
| const client = dynamicConfigServiceStart.getClient(); | ||
|
|
||
| try { | ||
| const dynamicConfig = await client.getConfig( | ||
| { pluginConfigPath: 'uiSettings' }, | ||
| { asyncLocalStorageContext: store! } | ||
| ); | ||
|
|
||
| return { | ||
| globalScopeEditable: { | ||
| enabled: dynamicConfig.globalScopeEditable.enabled, | ||
| }, | ||
| }; | ||
| } catch (e) { | ||
| this.logger.error(e); | ||
| return {}; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Improve error handling to provide consistent fallback behavior.
When dynamic config retrieval fails, returning an empty object {} means the globalScopeEditable capability won't be set, potentially causing downstream code to receive undefined. This could lead to inconsistent permission checks.
Consider providing an explicit fallback:
} catch (e) {
this.logger.error(e);
- return {};
+ return {
+ globalScopeEditable: {
+ enabled: true, // Safe default: allow edits
+ },
+ };
}This ensures predictable behavior even when dynamic config is unavailable.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| core.capabilities.registerSwitcher(async () => { | |
| const dynamicConfigServiceStart = await core.dynamicConfigService.getStartService(); | |
| const store = dynamicConfigServiceStart.getAsyncLocalStore(); | |
| const client = dynamicConfigServiceStart.getClient(); | |
| try { | |
| const dynamicConfig = await client.getConfig( | |
| { pluginConfigPath: 'uiSettings' }, | |
| { asyncLocalStorageContext: store! } | |
| ); | |
| return { | |
| globalScopeEditable: { | |
| enabled: dynamicConfig.globalScopeEditable.enabled, | |
| }, | |
| }; | |
| } catch (e) { | |
| this.logger.error(e); | |
| return {}; | |
| } | |
| }); | |
| core.capabilities.registerSwitcher(async () => { | |
| const dynamicConfigServiceStart = await core.dynamicConfigService.getStartService(); | |
| const store = dynamicConfigServiceStart.getAsyncLocalStore(); | |
| const client = dynamicConfigServiceStart.getClient(); | |
| try { | |
| const dynamicConfig = await client.getConfig( | |
| { pluginConfigPath: 'uiSettings' }, | |
| { asyncLocalStorageContext: store! } | |
| ); | |
| return { | |
| globalScopeEditable: { | |
| enabled: dynamicConfig.globalScopeEditable.enabled, | |
| }, | |
| }; | |
| } catch (e) { | |
| this.logger.error(e); | |
| return { | |
| globalScopeEditable: { | |
| enabled: true, // Safe default: allow edits | |
| }, | |
| }; | |
| } | |
| }); |
🤖 Prompt for AI Agents
In src/plugins/advanced_settings/server/plugin.ts around lines 70 to 90, the
catch block returns an empty object which leaves globalScopeEditable undefined
for callers; change the error handling to log the error with context and return
a well-formed fallback object (e.g., { globalScopeEditable: { enabled: false }
}) so callers always receive the expected shape and a predictable default
permission value.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
changelogs/fragments/10959.yml (1)
1-2: Changelog entry is clear and accurate.The entry properly documents the feature addition with appropriate categorization and includes a reference to the PR for traceability.
Optional suggestion for enhanced clarity: Consider including the configuration key name (
uiSettings.globalScopeEditable.enabled) in the description if this changelog will be published in release notes where users need to understand what setting to configure. However, the current high-level description is appropriate for a changelog fragment if it's meant to be automatically aggregated.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
changelogs/fragments/10959.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (67)
- GitHub Check: Run functional tests on Windows (ciGroup4)
- GitHub Check: Run functional tests on Windows (ciGroup9)
- GitHub Check: Run functional tests on Windows (ciGroup12)
- GitHub Check: Run functional tests on Windows (ciGroup7)
- GitHub Check: Run functional tests on Windows (ciGroup8)
- GitHub Check: Run functional tests on Windows (ciGroup13)
- GitHub Check: Run functional tests on Windows (ciGroup11)
- GitHub Check: Run functional tests on Windows (ciGroup2)
- GitHub Check: Run functional tests on Windows (ciGroup6)
- GitHub Check: Run functional tests on Windows (ciGroup10)
- GitHub Check: Run functional tests on Windows (ciGroup1)
- GitHub Check: Run functional tests on Windows (ciGroup5)
- GitHub Check: Run functional tests on Linux (ciGroup13)
- GitHub Check: Run cypress tests (osd:ciGroup14)
- GitHub Check: Run functional tests on Windows (ciGroup3)
- GitHub Check: Run functional tests on Linux (ciGroup12)
- GitHub Check: Run cypress tests (osd:ciGroup16Explore)
- GitHub Check: Run cypress tests (osd:ciGroup13Explore)
- GitHub Check: Run functional tests on Linux (ciGroup11)
- GitHub Check: Run cypress tests (osd:ciGroup15Explore)
- GitHub Check: Run functional tests on Linux (ciGroup9)
- GitHub Check: Run cypress tests (osd:ciGroup10Explore)
- GitHub Check: Run cypress tests (osd:ciGroup17Explore)
- GitHub Check: Run functional tests on Linux (ciGroup10)
- GitHub Check: Run functional tests on Linux (ciGroup6)
- GitHub Check: Run functional tests on Linux (ciGroup7)
- GitHub Check: Run cypress tests (osd:ciGroup15)
- GitHub Check: Run cypress tests (osd:ciGroup14Explore)
- GitHub Check: Run functional tests on Linux (ciGroup3)
- GitHub Check: Run cypress tests (osd:ciGroup12)
- GitHub Check: Run functional tests on Linux (ciGroup4)
- GitHub Check: Run cypress tests (osd:ciGroup12Explore)
- GitHub Check: Run functional tests on Linux (ciGroup5)
- GitHub Check: Run functional tests on Linux (ciGroup8)
- GitHub Check: Run cypress tests (osd:ciGroup13)
- GitHub Check: Run functional tests on Linux (ciGroup1)
- GitHub Check: Run functional tests on Linux (ciGroup2)
- GitHub Check: Run cypress tests (osd:ciGroup10Slow)
- GitHub Check: Run cypress tests (osd:ciGroup6)
- GitHub Check: Run cypress tests (osd:ciGroup11)
- GitHub Check: Run cypress tests (osd:ciGroup10Fast)
- GitHub Check: Run cypress tests (osd:ciGroup1)
- GitHub Check: Run cypress tests (osd:ciGroup7)
- GitHub Check: Run cypress tests (osd:ciGroup5)
- GitHub Check: Run cypress tests (osd:ciGroup4)
- GitHub Check: Run cypress tests (osd:ciGroup3)
- GitHub Check: Run cypress tests (osd:ciGroup8)
- GitHub Check: Run cypress tests (osd:ciGroup9)
- GitHub Check: Run cypress tests (osd:ciGroup2)
- GitHub Check: Build min release artifacts on macOS ARM64
- GitHub Check: Build min release artifacts on Windows x64
- GitHub Check: Build min release artifacts on macOS x64
- GitHub Check: Build min release artifacts on Linux x64
- GitHub Check: Build and Verify on Linux (ciGroup1)
- GitHub Check: Build min release artifacts on Linux ARM64
- GitHub Check: Build and Verify on Windows (ciGroup4)
- GitHub Check: Run plugin functional tests on Linux
- GitHub Check: Build and Verify on Windows (ciGroup1)
- GitHub Check: Build and Verify on Windows (ciGroup2)
- GitHub Check: Run plugin functional tests on Windows
- GitHub Check: Build and Verify on Linux (ciGroup4)
- GitHub Check: Build and Verify on Windows (ciGroup3)
- GitHub Check: Build and Verify on Linux (ciGroup3)
- GitHub Check: Build and Verify on Linux (ciGroup2)
- GitHub Check: Lint and validate
- GitHub Check: lighthouse
- GitHub Check: bundle-analyzer
4 participants
Description
This pr integrates dynamic config to disable global settings updates for non-admin users
Issue #10960
Testing the changes
Add the
uiSettings.globalScopeEditable.enabledsetting in the YAML configuration. (If this value cannot be found in the config store, the dynamic config service will fall back to reading it from opensearch_dashboards.yml).Global scope ui settings are not able to edit
Changelog
Check List
yarn test:jestyarn test:jest_integrationSummary by CodeRabbit
New Features
Configuration
globalScopeEditableoption (runtime-controllable) to govern global UI settings editability.Tests
✏️ Tip: You can customize this high-level summary in your review settings.