Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add doPrivileged section in deprecation logger #81819

Merged
merged 6 commits into from
Dec 17, 2021

Conversation

pgomulka
Copy link
Contributor

Scripts using deprecation logger can trigger log files rolling over.
Scripts also run with a very limited permissions and without
doPrivileged section would cause SM exception

closes #81708

  • Have you signed the contributor license agreement?
  • Have you followed the contributor guidelines?
  • If submitting code, have you built your formula locally prior to submission with gradle check?
  • If submitting code, is your pull request against master? Unless there is a good reason otherwise, we prefer pull requests against master and will backport as needed.
  • If submitting code, have you checked that your submission is for an OS and architecture that we support?
  • If you are submitting this code for a class then read our policy for that.

Scripts using deprecation logger can trigger log files rolling over.
Scripts also run with a very limited permissions and without
doPrivileged section would cause SM exception

closes elastic#81708
@pgomulka pgomulka added :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache :Core/Infra/Logging Log management and logging utilities v8.0.0 v8.1.0 v7.16.2 labels Dec 16, 2021
@pgomulka pgomulka self-assigned this Dec 16, 2021
@elasticmachine elasticmachine added the Team:Core/Infra Meta label for core/infra team label Dec 16, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra (Team:Core/Infra)

final LoggerContextFactory originalFactory = LogManager.getFactory();
try {
AtomicBoolean supplierCalled = new AtomicBoolean(false);
// mocking the logger used inside DeprecationLogger requires heavy hacking...
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not super proud on how I had to set up that logging configuration but it is not far worse to what we had before https://github.com/elastic/elasticsearch/pull/37281/files#diff-70de5a6ba5c637e7f19c51341417760d6e957beb5a1fa5703049095ea2719ee0R322

@pgomulka pgomulka added the >bug label Dec 16, 2021
@elasticsearchmachine
Copy link
Collaborator

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

@pgomulka pgomulka removed the :Core/Infra/Scripting Scripting abstractions, Painless, and Mustache label Dec 16, 2021
Copy link
Contributor

@grcevski grcevski left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@jdconrad jdconrad left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you for the fast fix!

@pgomulka pgomulka added the auto-backport Automatically create backport pull requests when merged label Dec 17, 2021
@pgomulka pgomulka merged commit f954919 into elastic:master Dec 17, 2021
pgomulka added a commit to pgomulka/elasticsearch that referenced this pull request Dec 17, 2021
Scripts using deprecation logger can trigger log files rolling over.
Scripts also run with a very limited permissions and without
doPrivileged section would cause SM exception

closes elastic#81708
@elasticsearchmachine
Copy link
Collaborator

💔 Backport failed

Status Branch Result
8.0
7.16 Commit could not be cherrypicked due to conflicts

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

@droberts195
Copy link
Contributor

@pgomulka the 7.17 branch was created yesterday and it's inconsistent for this to go into 7.16 but not 7.17. I added the v7.17.0 label. Please can you make sure this gets into the 7.17 branch.

@pgomulka pgomulka removed the v7.16.2 label Dec 20, 2021
pgomulka added a commit to pgomulka/elasticsearch that referenced this pull request Dec 20, 2021
Scripts using deprecation logger can trigger log files rolling over.
Scripts also run with a very limited permissions and without
doPrivileged section would cause SM exception

closes elastic#81708
pgomulka added a commit that referenced this pull request Dec 20, 2021
Scripts using deprecation logger can trigger log files rolling over.
Scripts also run with a very limited permissions and without
doPrivileged section would cause SM exception

closes #81708
pgomulka added a commit that referenced this pull request Dec 23, 2021
#81922)

Scripts using deprecation logger can trigger log files rolling over.
Scripts also run with a very limited permissions and without
doPrivileged section would cause SM exception

closes #81708
backport #81819
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto-backport Automatically create backport pull requests when merged >bug :Core/Infra/Logging Log management and logging utilities Team:Core/Infra Meta label for core/infra team v7.17.0 v8.0.0-rc1 v8.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing doPriviliged in deprecation logger
7 participants