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

Wrap a call to QatZipper with AccessController.doPrivileged. #211

Merged
merged 3 commits into from
Jan 10, 2025

Conversation

mulugetam
Copy link
Contributor

A customer reached out to inform us that they were unable to use qat_deflate and qat_lz4. Upon investigation, I discovered that isQATAvailable() was returning false due to a java.security permission fail in here. This behavior is unexpected, as my initial PR did not require it (as far as I can remember).

This PR addresses the issue by adding the necessary permission to the qat-java codebase.

@sarthakaggarwal97

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Ubuntu <ubuntu@ip-172-31-60-200.us-west-2.compute.internal>
Signed-off-by: Mulugeta Mammo <mulugeta.mammo@intel.com>
Copy link
Collaborator

@reta reta left a comment

Choose a reason for hiding this comment

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

@mulugetam this permission in general should never be granted (especially, in case of codecs), and usually indicates the issue in other places (caller site). I believe you have to wrap the call into AccessController.doPriveledged instead

@mulugetam
Copy link
Contributor Author

@reta will do that.

Signed-off-by: Mulugeta Mammo <mulugeta.mammo@intel.com>
@mulugetam mulugetam changed the title Grant qat-java a permission to modify thread. Wrap a call to QatZipper with AccessController.doPrivileged. Jan 10, 2025
@reta
Copy link
Collaborator

reta commented Jan 10, 2025

Thanks @mulugetam , I see the failing CI check, will fix it first thing tomorrow (if you have an opportunity, please apply the changes from opensearch-project/ml-commons#3223, thank you)

@reta
Copy link
Collaborator

reta commented Jan 10, 2025

#212

@reta
Copy link
Collaborator

reta commented Jan 10, 2025

@mulugetam could you please rebase? thank you!

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
@mulugetam
Copy link
Contributor Author

Just did. Thank you @reta

env:
ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION: true

options: ${{ needs.Get-CI-Image-Tag.outputs.ci-image-start-options }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting, this change is in main already ...

@reta reta merged commit 9de7fd3 into opensearch-project:main Jan 10, 2025
9 checks passed
@reta reta added bug Something isn't working backport 2.x labels Jan 10, 2025
opensearch-trigger-bot bot pushed a commit that referenced this pull request Jan 10, 2025
* Grant qat-java a permission to modify arbitrary thread.

Signed-off-by: Ubuntu <ubuntu@ip-172-31-60-200.us-west-2.compute.internal>
Signed-off-by: Mulugeta Mammo <mulugeta.mammo@intel.com>

* Wrap a QatZipper() inside AccessController.doPrivileged().

Signed-off-by: Mulugeta Mammo <mulugeta.mammo@intel.com>

* Fix GitHib action workflows (#212)

Signed-off-by: Andriy Redko <andriy.redko@aiven.io>

---------

Signed-off-by: Ubuntu <ubuntu@ip-172-31-60-200.us-west-2.compute.internal>
Signed-off-by: Mulugeta Mammo <mulugeta.mammo@intel.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Co-authored-by: Ubuntu <ubuntu@ip-172-31-60-200.us-west-2.compute.internal>
Co-authored-by: Andriy Redko <andriy.redko@aiven.io>
(cherry picked from commit 9de7fd3)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
reta added a commit that referenced this pull request Jan 10, 2025
…214)

* Grant qat-java a permission to modify arbitrary thread.




* Wrap a QatZipper() inside AccessController.doPrivileged().



* Fix GitHib action workflows (#212)



---------






(cherry picked from commit 9de7fd3)

Signed-off-by: Ubuntu <ubuntu@ip-172-31-60-200.us-west-2.compute.internal>
Signed-off-by: Mulugeta Mammo <mulugeta.mammo@intel.com>
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Ubuntu <ubuntu@ip-172-31-60-200.us-west-2.compute.internal>
Co-authored-by: Andriy Redko <andriy.redko@aiven.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants