Skip to content

Mark apm_user for removal in "a future version"#87674

Merged
pugnascotia merged 9 commits intoelastic:mainfrom
sorenlouv:update-apm-user-deprecation
Jul 5, 2023
Merged

Mark apm_user for removal in "a future version"#87674
pugnascotia merged 9 commits intoelastic:mainfrom
sorenlouv:update-apm-user-deprecation

Conversation

@sorenlouv
Copy link
Copy Markdown
Contributor

Meta issue: elastic/kibana#116760
The apm_user role was marked as deprecated in 7.13 and was supposed to be removed in 8.0 but it didn't happen. Now we are aiming to remove the role in 9.0 and are updating the deprecation message.

All mentions of apm_user role have been removed from docs and in-product mentions in elastic/kibana#132790.

@sorenlouv sorenlouv mentioned this pull request Jun 14, 2022
@sorenlouv sorenlouv requested a review from ywangd June 14, 2022 23:13
@elasticsearchmachine elasticsearchmachine added external-contributor Pull request authored by a developer outside the Elasticsearch team v8.4.0 labels Jun 14, 2022
@sorenlouv sorenlouv added >deprecation :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC labels Jun 14, 2022
@elasticmachine elasticmachine added the Team:Security Meta label for security team label Jun 14, 2022
@elasticmachine
Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-security (Team:Security)

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

Hi @sqren, I've created a changelog YAML for you. Note that since this PR is labelled >deprecation, you need to update the changelog YAML to fill out the extended information sections.

ywangd
ywangd previously requested changes Jun 15, 2022
Copy link
Copy Markdown
Member

@ywangd ywangd left a comment

Choose a reason for hiding this comment

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

I think we should use the next major release instead of 9.0 since the major release strategy is not entirely clear. Also most places in the code use "the next major release/version", e.g.:

+ "will not permit mapping updates in the next major release - users who require access "

@bytebilly
Copy link
Copy Markdown
Contributor

I think we should use the next major release instead of 9.0 since the major release strategy is not entirely clear.

We could be even more generic with "a future major release". It was not so uncommon that we decided to keep a deprecated feature across majors, and in that case the message may be confusing.

@sorenlouv
Copy link
Copy Markdown
Contributor Author

sorenlouv commented Jun 20, 2022

I changed the deprecation notice to "This role will be removed in a future major release. Please use editor and viewer roles instead"

@ywangd
Copy link
Copy Markdown
Member

ywangd commented Jun 22, 2022

@elasticmachine update branch

@ywangd
Copy link
Copy Markdown
Member

ywangd commented Jun 22, 2022

@pugnascotia CI fails for this PR because it does not like the area label of deprecation. Existing ones are

"enum": [
"Cluster and node setting",
"Command line tool",
"CRUD",
"Index setting",
"JVM option",
"Java API",
"Logging",
"Mapping",
"Packaging",
"Painless",
"REST API",
"System requirement",
"Transform"
]

But none of them seems to fit the deprecation here which is for a builtin role. What would be your recommendation? Thanks!

@pugnascotia
Copy link
Copy Markdown
Contributor

It's fine to add a new area to that last. Obviously we don't want lots and lots of areas, but if we need a another one then let's just add it. "Authorization" or "Security" would be OK, something like that.

@elasticsearchmachine elasticsearchmachine changed the base branch from master to main July 22, 2022 23:06
@mark-vieira mark-vieira added v8.5.0 and removed v8.4.0 labels Jul 27, 2022
@csoulios csoulios added v8.6.0 and removed v8.5.0 labels Sep 21, 2022
@kingherc kingherc added v8.7.0 and removed v8.6.0 labels Nov 16, 2022
@rjernst rjernst added v8.8.0 and removed v8.7.0 labels Feb 8, 2023
@gmarouli gmarouli added v8.9.0 and removed v8.8.0 labels Apr 26, 2023
@sorenlouv
Copy link
Copy Markdown
Contributor Author

I dropped this but this is still something I'd like to merge.

@pugnascotia Afaict I added "Authorization" in 72b1163. Still, I'm seeing a bunch of failures.

What's the suggested way forward?

@pugnascotia
Copy link
Copy Markdown
Contributor

@elasticmachine update branch

@pugnascotia
Copy link
Copy Markdown
Contributor

@sqren I updated the branch but there are formatting problems. To fix them, update your local branch checkout and run:

./gradlew spotlessApply

Then commit and push. I don't believe we have automation for this at present.

@sorenlouv sorenlouv changed the title Mark apm_user for removal in 9.0 Mark apm_user for removal in "a future version" Jul 5, 2023
@pugnascotia pugnascotia dismissed ywangd’s stale review July 5, 2023 08:46

Request changes have been implemented.

@pugnascotia pugnascotia merged commit ed451a7 into elastic:main Jul 5, 2023
@sorenlouv sorenlouv deleted the update-apm-user-deprecation branch July 5, 2023 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>deprecation external-contributor Pull request authored by a developer outside the Elasticsearch team :Security/Authorization Roles, Privileges, DLS/FLS, RBAC/ABAC Team:Security Meta label for security team v8.10.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.