Deprecating data_frame_transforms roles #117519
Conversation
dan-rubinstein added >deprecation :ml 
| Documentation preview: |
elasticsearchmachine added the external-contributor 
| Hi @dan-rubinstein, I've created a changelog YAML for you. Note that since this PR is labelled |
dan-rubinstein removed the external-contributor | Pinging @elastic/ml-core (Team:ML) |
| @elasticmachine merge upstream |
mattc58 requested review from jfreden and n1v0lg and removed request for jfreden 
| "manage_behavioral_analytics", | ||
| "manage_ccr", | ||
| "manage_connector", | ||
| "manage_data_frame_transforms", |
There was a problem hiding this comment.
I don't see mention of removing these privileges in the breaking changes request -- we only mention roles. The manage_data_frame_transforms and monitor_data_frame_transforms privileges are indeed deprecated according to our docs (monitor_data_frame_transforms has already been removed from docs it seems), but we don't issue any deprecation warnings around these privileges (currently there's no clean, built-in support for that). These privileges can be assigned to user-defined roles and user-created API keys so they are not just scoped to the built-in roles we're removing here.
I think it's likely still a good idea to remove the privileges in 9.0 but I prefer that we do this in a separate PR and after making sure that it's worth the trouble.
@dan-rubinstein could you revert the changes around privileges in this PR? That'll also take care of some of the CI failures. I can open a follow up PR with the privileges removed -- since it requires a PR against the Serverless repo as well, it'll be easier for me to author both PRs than only one of them.
There was a problem hiding this comment.
I can revert the changes for this file - to clarify though, would you prefer that we revert the change entirely, or would you prefer that we set the permissions as empty via Set.of()?
There was a problem hiding this comment.
@prwhelan let's revert it entirely -- I synced on this with the ES sec team before the holidays and the preference from our end would be to follow the deprecate, then remove approach, i.e., implement a deprecation warning for the privilege, then remove in a later version. This would delay the removal of the privilege since we'd first need a mechanism for issuing deprecation warnings for privileges (that's on the ES security team to implement).
Would the following be acceptable for the ML team? Remove the roles now, wait for ES security to implement a deprecation mechanism, deprecate the privilege, then remove it in some later ES version? This way things are less disruptive for any customers that may still be relying on the privilege.
Just a preference: if you have confidence that only very few (if any) customers still use this privilege, or feel strongly about making it non-functional sooner, I'm also good with making it a NOOP via Set.of().
Either way though, let's do whatever we do around the privilege in a separate PR to keep things focused.
There was a problem hiding this comment.
Okay, is there an issue we can track for deprecating and removing the privilege? I imagine we'll need to wait to the v10 upgrade to delete it, in which case I want to record it in some way
There was a problem hiding this comment.
Yup -- I'll create Jira for deprecating this particular privilege and Slack it to you (it's a private link that external people won't have access to).
docs/changelog/117519.yaml Outdated
| pr: 117519 | ||
| summary: Deprecating `data_frame_transforms` roles | ||
| area: Machine Learning | ||
| type: deprecation |
There was a problem hiding this comment.
We want a >breaking label on the PR instead of a >deprecation label. That'll also update some of the contents of this changelog IIRC.
After updating the label, lets tweak the title and summary here and the PR title to "Removing data_frame_transforms roles".
| Hi @dan-rubinstein, I've updated the changelog YAML for you. Note that since this PR is labelled |
| @elasticmachine update branch |
| "my_admin_role": { <4> | ||
| "type": "action_request_validation_exception", | ||
| "reason": "Validation Failed: 1: unknown cluster privilege [bad_cluster_privilege]. a privilege must be either one of the predefined cluster privilege names [manage_own_api_key,manage_data_stream_global_retention,monitor_data_stream_global_retention,none,cancel_task,cross_cluster_replication,cross_cluster_search,delegate_pki,grant_api_key,manage_autoscaling,manage_index_templates,manage_logstash_pipelines,manage_oidc,manage_saml,manage_search_application,manage_search_query_rules,manage_search_synonyms,manage_service_account,manage_token,manage_user_profile,monitor_connector,monitor_enrich,monitor_inference,monitor_ml,monitor_rollup,monitor_snapshot,monitor_stats,monitor_text_structure,monitor_watcher,post_behavioral_analytics_event,read_ccr,read_connector_secrets,read_fleet_secrets,read_ilm,read_pipeline,read_security,read_slm,transport_client,write_connector_secrets,write_fleet_secrets,create_snapshot,manage_behavioral_analytics,manage_ccr,manage_connector,manage_enrich,manage_ilm,manage_inference,manage_ml,manage_rollup,manage_slm,manage_watcher,monitor_data_frame_transforms,monitor_transform,manage_api_key,manage_ingest_pipelines,manage_pipeline,manage_data_frame_transforms,manage_transform,manage_security,monitor,manage,all] or a pattern over one of the available cluster actions;" | ||
| "reason": "Validation Failed: 1: unknown cluster privilege [bad_cluster_privilege]. a privilege must be either one of the predefined cluster privilege names [manage_own_api_key,manage_data_stream_global_retention,monitor_data_stream_global_retention,none,cancel_task,cross_cluster_replication,cross_cluster_search,delegate_pki,grant_api_key,manage_autoscaling,manage_index_templates,manage_logstash_pipelines,manage_oidc,manage_saml,manage_search_application,manage_search_query_rules,manage_search_synonyms,manage_service_account,manage_token,manage_user_profile,monitor_connector,monitor_enrich,monitor_inference,monitor_ml,monitor_rollup,monitor_snapshot,monitor_stats,monitor_text_structure,monitor_watcher,post_behavioral_analytics_event,read_ccr,read_connector_secrets,read_fleet_secrets,read_ilm,read_pipeline,read_security,read_slm,transport_client,write_connector_secrets,write_fleet_secrets,create_snapshot,manage_behavioral_analytics,manage_ccr,manage_connector,manage_enrich,manage_ilm,manage_inference,manage_ml,manage_rollup,manage_slm,manage_watcher,monitor_transform,manage_api_key,manage_ingest_pipelines,manage_pipeline,manage_transform,manage_security,monitor,manage,all] or a pattern over one of the available cluster actions;" |
Co-authored-by: Nikolaj Volgushev <n1v0lg@users.noreply.github.com>
This reverts commit 4599256.
| @elasticmachine update branch |
| There are no new commits on the base branch. |
| @dan-rubinstein is this PR relevant to the serverless changelog? [FYI this question is based on 9.0 breaking changes] |
7 participants
Issue - https://github.com/elastic/ml-team/issues/919
Description
The
data_frame_transforms_adminanddata_frame_transforms_userroles have been deprecated for some time now. The roles are still created to ensure backwards compatibility but they have a deprecation tag in all documentation and a critical deprecation warning is going to be added to 8.18 for these roles as part of this in-progress change. This change removes all references to these roles from code and documentation including the logic to create them. This will only be added to 9.0. Users will have migrated their transforms to the non-deprecated equivalent roles as part of handling the critical deprecation warning mentioned above during their migration from 8.18->9. Any users that were unable to do so, will be able to call the transforms update API to update their roles and get their transforms working again.Testing
[MERGE BLOCKER] Note: This change is blocked on both the deprecation warning change and approval on the breaking change request.Note [Updated]: The deprecation warning change has been merged and backported and the breaking change request has been approved. This change no longer has any merge blockers.