[Fleet] Rename ingestManager plugin ID fleet#83200
Conversation
370adc9 to
fa9537c
Compare
fa9537c to
a48b2a6
Compare
cjcenizal
left a comment
There was a problem hiding this comment.
ES UI code LGTM, didn't test locally.
|
Pinging @elastic/apm-ui (Team:apm) |
|
Pinging @elastic/ingest-management (Team:Ingest Management) |
| @@ -186,15 +180,15 @@ export class IngestManagerPlugin | |||
| if (deps.features) { | |||
| deps.features.registerKibanaFeature({ | |||
| id: PLUGIN_ID, | |||
There was a problem hiding this comment.
question: @legrego IIRC changing feature ID is a breaking change (we changed the PLUGIN_ID) and we shouldn't make it in a minor upgrade, but I'm not sure what is our policy regarding features that belong to plugins that are in a beta?
There was a problem hiding this comment.
Heya Fleet team:
tl;dr I think we can rename in this specific case, please take the time to read this entire comment to understand the implications.
IIRC changing feature ID is a breaking change (we changed the PLUGIN_ID) and we shouldn't make it in a minor upgrade
That's exactly right. Changing a feature id will change the associated privilege ids that are registered with Elasticsearch on startup. For example, prior to this change, a user may have been assigned a custom role which grants access to Ingest Manager. The role would look something like this:
{
"name": "some-custom-role",
"cluster": [],
"index": [],
"run_as": [],
"applications": [
{
"application": "kibana-.kibana",
"privileges": [
"feature_ingestManager.all"
],
"resources": [
"space:default"
]
}
]
}The privilege that gets assigned to the role (feature_ingestManager.all) is derived in part from the feature id. Renaming your feature means that any role which previously granted access to your feature will no longer grant access, because Kibana doesn't have a feature called ingestManager anymore. The role should instead look like:
{
"name": "some-custom-role",
"cluster": [],
"index": [],
"run_as": [],
"applications": [
{
"application": "kibana-.kibana",
"privileges": [
"feature_fleet.all"
],
"resources": [
"space:default"
]
}
]
}The problem is that Kibana can't just go and rewrite all of these role definitions. We don't have any sort of migration system in place yet (#68814)
I'm not sure what is our policy regarding features that belong to plugins that are in a beta?
This might be our saving grace. I think we can allow the rename in this specific case, but only because this is a beta feature, and we explicitly tell users not to run this in production yet.
However, this is something that you need to document in the release notes. We have to at least give administrators a fighting chance of restoring access to their users, and not surprise them.
There was a problem hiding this comment.
I will add a release note 👍 ,
I do not think is so problematic here as an user is not able to use the Fleet plugin if he do not have the superuser role
There was a problem hiding this comment.
Sounds like y'all got all of the angles covered here, but I just wanted to mention that from various sources it looks like people are very positive about Fleet and Elastic Agent, and that Beta testers go out on a limb to try things out and give us early feedback. So as a general rule, I think it's important to have empathy for them and avoid introducing breaking changes that will essentially surprise and punish people who are trying to help us, if at all possible.
There was a problem hiding this comment.
I added the release note
skh
left a comment
There was a problem hiding this comment.
I tested this locally and didn't manage to break it. 👍
…e-ingest-manager-fleet-plugin
paul-tavares
left a comment
There was a problem hiding this comment.
Looks good Nicolas. Thanks for the changes to the Security Solution code 👍
| title: i18n.translate('xpack.fleet.oldAppTitle', { defaultMessage: 'Ingest Manager' }), | ||
| async mount(params: AppMountParameters) { | ||
| const [coreStart] = await core.getStartServices(); | ||
| coreStart.application.navigateToApp('fleet', { |
There was a problem hiding this comment.
Nice.
So this still show a left-nav (kibana) nav item, correct?
should there be a flag (in settings) to possibly remove it? (I'm thinking of non-existing fleet users that may download/install >= 7.11)
There was a problem hiding this comment.
the navlink is hidden with
navLinkStatus: AppNavLinkStatus.hidden,
|
@elastic/siem I would love to have a review here :) |
stephmilovic
left a comment
There was a problem hiding this comment.
LGTM on SIEM changes, thank you!
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Module Count
Async chunks
Page load bundle
Unknown metric groupsasync chunk count
History
To update your PR or re-run it, just comment with: |
* master: skip "Dashboards linked by a drilldown are both copied to a space" (elastic#83824) [alerts] adds action group and date to mustache template variables for actions (elastic#83195) skip flaky suite (elastic#79389) [DOCS] Reallocates limitations to point-of-use (elastic#79582) [Enterprise Search] Engine overview layout stub (elastic#83756) Disable exporting/importing of templates. Optimize pitch images a bit (elastic#83098) [DOCS] Consolidates plugins (elastic#83712) [ML] Space management UI (elastic#83320) test just part of the message to avoid updates (elastic#83703) [Data Table] Remove extra column in split mode (elastic#83193) Improve snapshot error messages (elastic#83785) skip flaky suite (elastic#83773) skip flaky suite (elastic#83771) skip flaky suite (elastic#65278) skip flaky suite (elastic#83793) [Task Manager] Ensures retries are inferred from the schedule of recurring tasks (elastic#83682) [index patterns] improve index pattern cache (elastic#83368) [Fleet] Rename ingestManager plugin ID fleet (elastic#83200) fixed pagination in connectors list (elastic#83638)
Summary
Last PR on the effort of renaming ingestManager Fleet
Rename
ingestManagerplugin IDfleetThis change the app path from
/app/ingestManagerto/app/fleetI changed the following plugins accordingly:
Also I renamed all the
IngestManagerPluginIngestManagerSetup, ..., toFleet*Release note breaking
The
ingestManagerplugin as been renamedfleet, this mean:/app/ingestManagerto/app/fleetfeature_ingestManager.*is not valid anymore and should be replaced byfeature_fleet.*