Skip to content

Conversation

@albertzaharovits
Copy link
Contributor

@albertzaharovits albertzaharovits commented Jun 6, 2022

The Closeable#close method on Realms was never called upon node shutdown.
This is a problem when realms (eg OIDC) create non-daemon threads that expect
to be stopped when the close method is invoked. Specifically, it is a problem
on Windows where the graceful shutdown is implemented by terminatting all
non-daemon threads (see Bootstrap#stop and Bootstrap#initializeNatives).

Closes #86286

@albertzaharovits albertzaharovits self-assigned this Jun 6, 2022
@albertzaharovits albertzaharovits added the :Security/Security Security issues without another label label Jun 6, 2022
@elasticsearchmachine
Copy link
Collaborator

Hi @albertzaharovits, I've created a changelog YAML for you.

@albertzaharovits albertzaharovits marked this pull request as ready for review June 6, 2022 19:43
@elasticmachine elasticmachine added the Team:Security Meta label for security team label Jun 6, 2022
@elasticmachine
Copy link
Collaborator

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

@tvernum
Copy link
Contributor

tvernum commented Jun 7, 2022

I think this should be tagged v8.2.3 as well (if we can get it approved & merged quickly)

Copy link
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.

LGTM

…ecurity/authc/Realms.java Co-authored-by: Tim Vernum <tim@adjective.org>
@albertzaharovits
Copy link
Contributor Author

albertzaharovits commented Jun 7, 2022

I think this should be tagged v8.2.3 as well (if we can get it approved & merged quickly)

Yes, added the label and will merge it there as well.

@albertzaharovits
Copy link
Contributor Author

@elasticmachine update branch

@albertzaharovits albertzaharovits added auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) auto-backport-and-merge labels Jun 7, 2022
@albertzaharovits albertzaharovits changed the title Security plugin close releasable realms Security plugin close realms Jun 7, 2022
@albertzaharovits
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/part-1

@elasticsearchmachine elasticsearchmachine merged commit 9c02afd into elastic:master Jun 7, 2022
@albertzaharovits albertzaharovits deleted the fix_close_realms branch June 7, 2022 08:29
albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this pull request Jun 7, 2022
The `Closeable#close` method on `Realm`s was never called upon node shutdown. This is a problem when realms (eg OIDC) create non-daemon threads that expect to be stopped when the `close` method is invoked. Specifically, it is a problem on Windows where the graceful shutdown is implemented by terminatting all non-daemon threads (see `Bootstrap#stop` and `Bootstrap#initializeNatives`). Closes elastic#86286
albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this pull request Jun 7, 2022
The `Closeable#close` method on `Realm`s was never called upon node shutdown. This is a problem when realms (eg OIDC) create non-daemon threads that expect to be stopped when the `close` method is invoked. Specifically, it is a problem on Windows where the graceful shutdown is implemented by terminatting all non-daemon threads (see `Bootstrap#stop` and `Bootstrap#initializeNatives`). Closes elastic#86286
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
7.17 Commit could not be cherrypicked due to conflicts
8.2
8.3

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 87429

elasticsearchmachine pushed a commit that referenced this pull request Jun 7, 2022
* Security plugin close realms (#87429) The `Closeable#close` method on `Realm`s was never called upon node shutdown. This is a problem when realms (eg OIDC) create non-daemon threads that expect to be stopped when the `close` method is invoked. Specifically, it is a problem on Windows where the graceful shutdown is implemented by terminatting all non-daemon threads (see `Bootstrap#stop` and `Bootstrap#initializeNatives`). Closes #86286 * Add the extension point in LocalStateSecurity
elasticsearchmachine pushed a commit that referenced this pull request Jun 7, 2022
* Security plugin close realms (#87429) The `Closeable#close` method on `Realm`s was never called upon node shutdown. This is a problem when realms (eg OIDC) create non-daemon threads that expect to be stopped when the `close` method is invoked. Specifically, it is a problem on Windows where the graceful shutdown is implemented by terminatting all non-daemon threads (see `Bootstrap#stop` and `Bootstrap#initializeNatives`). Closes #86286 * IOUtils rename around
albertzaharovits added a commit to albertzaharovits/elasticsearch that referenced this pull request Jun 7, 2022
The `Closeable#close` method on `Realm`s was never called upon node shutdown. This is a problem when realms (eg OIDC) create non-daemon threads that expect to be stopped when the `close` method is invoked. Specifically, it is a problem on Windows where the graceful shutdown is implemented by terminatting all non-daemon threads (see `Bootstrap#stop` and `Bootstrap#initializeNatives`). Closes elastic#86286
elasticsearchmachine pushed a commit that referenced this pull request Jun 7, 2022
* [7.17] Security plugin close realms (#87429) The `Closeable#close` method on `Realm`s was never called upon node shutdown. This is a problem when realms (eg OIDC) create non-daemon threads that expect to be stopped when the `close` method is invoked. Specifically, it is a problem on Windows where the graceful shutdown is implemented by terminatting all non-daemon threads (see `Bootstrap#stop` and `Bootstrap#initializeNatives`). Closes #86286 * Merge fallout * Spotless * More compilation fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) >bug :Security/Security Security issues without another label Team:Security Meta label for security team v7.17.5 v8.2.3 v8.3.1 v8.4.0

6 participants