Entity store/entity maintainers min license#260170
Conversation
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
⏳ Build in-progress, with failures
Failed CI StepsTest Failures
History
cc @chennn1990 |
✅ Actions performedReview triggered.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe pull request consolidates the entity_store plugin by removing the constants.ts file and relocating its exports to the common module, updating all import references accordingly. It introduces licensing integration through a new required dependency on the licensing plugin. Entity maintainer tasks gain a minLicense field with gating logic that enforces license validation at runtime, defaulting to 'basic' level. The API_VERSIONS, ENTITY_STORE_ROUTES, and related constants are now centralized in the common module, reducing code fragmentation. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis pull request implements license-gating for entity maintainer tasks in the Entity Store plugin. The ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
x-pack/solutions/security/plugins/entity_store/server/domain/entity_maintainers/entity_maintainers_client.test.ts (1)
17-23:⚠️ Potential issue | 🟠 MajorMock missing
DEFAULT_ENTITY_MAINTAINER_MIN_LICENSE— tests pass incorrectly.The mock factory completely replaces the module, so the import on line 13 resolves to
undefined. All assertions usingminLicense: DEFAULT_ENTITY_MAINTAINER_MIN_LICENSEare actually testingminLicense: undefined.Proposed fix
jest.mock('../../tasks/entity_maintainers', () => ({ + DEFAULT_ENTITY_MAINTAINER_MIN_LICENSE: 'basic', getTaskId: jest.fn((id: string, namespace: string) => `${id}:${namespace}`), removeEntityMaintainer: jest.fn().mockResolvedValue(undefined), scheduleEntityMaintainerTask: jest.fn().mockResolvedValue(undefined), startEntityMaintainer: jest.fn().mockResolvedValue(undefined), stopEntityMaintainer: jest.fn().mockResolvedValue(undefined), }));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@x-pack/solutions/security/plugins/entity_store/server/domain/entity_maintainers/entity_maintainers_client.test.ts` around lines 17 - 23, The mock currently fully replaces the entity_maintainers module so DEFAULT_ENTITY_MAINTAINER_MIN_LICENSE becomes undefined in tests; update the jest.mock call to preserve or re-export DEFAULT_ENTITY_MAINTAINER_MIN_LICENSE (e.g., use jest.requireActual('../../tasks/entity_maintainers') and spread its exports, then override only getTaskId, removeEntityMaintainer, scheduleEntityMaintainerTask, startEntityMaintainer, and stopEntityMaintainer) so tests see the real DEFAULT_ENTITY_MAINTAINER_MIN_LICENSE value while the listed functions remain mocked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@x-pack/solutions/security/plugins/entity_store/server/domain/entity_maintainers/entity_maintainers_client.test.ts`:
- Around line 17-23: The mock currently fully replaces the entity_maintainers
module so DEFAULT_ENTITY_MAINTAINER_MIN_LICENSE becomes undefined in tests;
update the jest.mock call to preserve or re-export
DEFAULT_ENTITY_MAINTAINER_MIN_LICENSE (e.g., use
jest.requireActual('../../tasks/entity_maintainers') and spread its exports,
then override only getTaskId, removeEntityMaintainer,
scheduleEntityMaintainerTask, startEntityMaintainer, and stopEntityMaintainer)
so tests see the real DEFAULT_ENTITY_MAINTAINER_MIN_LICENSE value while the
listed functions remain mocked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 8aa248dd-962c-4ee9-9699-ceb295a55b70
📒 Files selected for processing (17)
x-pack/solutions/security/plugins/entity_store/common/constants.tsx-pack/solutions/security/plugins/entity_store/kibana.jsoncx-pack/solutions/security/plugins/entity_store/moon.ymlx-pack/solutions/security/plugins/entity_store/public/bulk_update_entities_api.tsx-pack/solutions/security/plugins/entity_store/public/hooks/useInstallEntityStoreV2.test.tsxx-pack/solutions/security/plugins/entity_store/public/hooks/useInstallEntityStoreV2.tsxx-pack/solutions/security/plugins/entity_store/public/search_entities_api.tsx-pack/solutions/security/plugins/entity_store/server/domain/entity_maintainers/entity_maintainers_client.test.tsx-pack/solutions/security/plugins/entity_store/server/domain/entity_maintainers/entity_maintainers_client.tsx-pack/solutions/security/plugins/entity_store/server/routes/apis/entity_maintainers/get_maintainers.tsx-pack/solutions/security/plugins/entity_store/server/tasks/entity_maintainers/entity_maintainers.test.tsx-pack/solutions/security/plugins/entity_store/server/tasks/entity_maintainers/entity_maintainers_registry.test.tsx-pack/solutions/security/plugins/entity_store/server/tasks/entity_maintainers/entity_maintainers_registry.tsx-pack/solutions/security/plugins/entity_store/server/tasks/entity_maintainers/index.tsx-pack/solutions/security/plugins/entity_store/server/tasks/entity_maintainers/types.tsx-pack/solutions/security/plugins/entity_store/server/types.tsx-pack/solutions/security/plugins/entity_store/tsconfig.json
💤 Files with no reviewable changes (1)
- x-pack/solutions/security/plugins/entity_store/common/constants.ts
…/github.com/chennn1990/kibana into entity-store/entity-maintainers-min-license
|
@maxcold FYI. |
| import { useEffect } from 'react'; | ||
| import { EntityStoreStatus } from '../../common'; | ||
| import { ENTITY_STORE_ROUTES, FF_ENABLE_ENTITY_STORE_V2 } from '../../common/constants'; | ||
| import { ENTITY_STORE_ROUTES, FF_ENABLE_ENTITY_STORE_V2 } from '../../common'; |
|
@uri-weisman awesome news, will keep an eye on this PR! |
## Summary Entity Store entity maintainers now respect Kibana license tier at run time - Each maintainer can declare an optional minimum license - If the active license is not enough, that run is skipped and maintainer state is left unchanged - When no minimum is set, the framework uses the lowest tier so behavior matches a Basic-style default - License is evaluated when the task runs, so tier changes apply without restarting Kibana The internal list-maintainers response includes the configured minimum license so callers can see requirements next to status Duplicate common constants were consolidated so shared plugin constants live in one place for public code ## Testing - Automated tests cover maintainer registration, license skip behavior, and list response shape for minimum license ## How to verify - Run the entity maintainer unit tests - Run your usual repo change-validation checks --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
## Summary Entity Store entity maintainers now respect Kibana license tier at run time - Each maintainer can declare an optional minimum license - If the active license is not enough, that run is skipped and maintainer state is left unchanged - When no minimum is set, the framework uses the lowest tier so behavior matches a Basic-style default - License is evaluated when the task runs, so tier changes apply without restarting Kibana The internal list-maintainers response includes the configured minimum license so callers can see requirements next to status Duplicate common constants were consolidated so shared plugin constants live in one place for public code ## Testing - Automated tests cover maintainer registration, license skip behavior, and list response shape for minimum license ## How to verify - Run the entity maintainer unit tests - Run your usual repo change-validation checks --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Summary
Entity Store entity maintainers now respect Kibana license tier at run time
The internal list-maintainers response includes the configured minimum license so callers can see requirements next to status
Duplicate common constants were consolidated so shared plugin constants live in one place for public code
Testing
How to verify