Refactors ClientHelper to combine header logic#30620
Merged
colings86 merged 2 commits intoelastic:masterfrom May 16, 2018
colings86:refactor/clientHelpers
Merged
Refactors ClientHelper to combine header logic#30620colings86 merged 2 commits intoelastic:masterfrom colings86:refactor/clientHelpers
colings86 merged 2 commits intoelastic:masterfrom
colings86:refactor/clientHelpers
Conversation
colings86
added
review
v7.0.0
>refactoring
:Security/Authorization
Collaborator
|
Pinging @elastic/es-security |
polyfractal
approved these changes
May 15, 2018
Contributor
|
LGTMToo :) |
spinscale
approved these changes
May 15, 2018
Contributor
spinscale
left a comment
spinscale
left a comment
There was a problem hiding this comment.
LGTM.
You could remove Watcher.HEADER_FILTERS and replace with the ClientHelper ones, or I can do that as a follow up.
This change removes all the `*ClientHelper` classes which were repeating logic between plugins and instead adds `ClientHelper.executeWithHeaders()` and `ClientHelper.executeWithHeadersAsync()` methods to centralise the logic for executing requests with stored security headers.
Contributor
Author
|
@elasticmachine retest this please |
colings86
added a commit
that referenced
this pull request
May 16, 2018
* Refactors ClientHelper to combine header logic This change removes all the `*ClientHelper` classes which were repeating logic between plugins and instead adds `ClientHelper.executeWithHeaders()` and `ClientHelper.executeWithHeadersAsync()` methods to centralise the logic for executing requests with stored security headers. * Removes Watcher headers constant x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/tran sport/actions/put/TransportPutWatchActionTests.java /Users/colings86/dev/work/git/elasticsearch/.git/worktrees/elasticsearch -6.x/CHERRY_PICK_HEAD x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ClientHelp er.java x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MlClien tHelper.java x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/MlMetad ata.java x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/datafee d/DatafeedUpdate.java x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ClientHelp erTests.java x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/action/Transpo rtPreviewDatafeedAction.java x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extra ctor/aggregation/AggregationDataExtractor.java x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extra ctor/chunked/ChunkedDataExtractor.java x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extra ctor/scroll/ScrollDataExtractor.java x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/datafeed/extra ctor/scroll/ScrollDataExtractorFactory.java x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/MlClientHelper Tests.java x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/Ro llupClientHelper.java x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/job/Ro llupJobTask.java x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/Ro llupClientHelperTests.java x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watc her.java x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/Watc herClientHelper.java x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/acti ons/index/ExecutableIndexAction.java x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/inpu t/search/ExecutableSearchInput.java x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/tran sform/search/ExecutableSearchTransform.java x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/tran sport/actions/put/TransportPutWatchAction.java x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/Watc herClientHelperTests.java x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/tran sport/actions/put/TransportPutWatchActionTests.java
jasontedor
added a commit
to jasontedor/elasticsearch
that referenced
this pull request
May 16, 2018
…ngs-to-true * elastic/master: (34 commits) Test: increase search logging for LicensingTests Adjust serialization version in IndicesOptions [TEST] Fix compilation Remove version argument in RangeFieldType (elastic#30411) Remove unused DirectoryUtils class. (elastic#30582) Mitigate date histogram slowdowns with non-fixed timezones. (elastic#30534) Add a MovingFunction pipeline aggregation, deprecate MovingAvg agg (elastic#29594) Removes AwaitsFix on IndicesOptionsTests Template upgrades should happen in a system context (elastic#30621) Fix bug in BucketMetrics path traversal (elastic#30632) Fixes IndiceOptionsTests to serialise correctly (elastic#30644) S3 repo plugin populate SettingsFilter (elastic#30652) mute IndicesOptionsTests.testSerialization Rest High Level client: Add List Tasks (elastic#29546) Refactors ClientHelper to combine header logic (elastic#30620) [ML] Wait for ML indices in rolling upgrade tests (elastic#30615) Watcher: Ensure secrets integration tests also run triggered watch (elastic#30478) Move allocation awareness attributes to list setting (elastic#30626) [Docs] Update code snippet in has-child-query.asciidoc (elastic#30510) Replace custom reloadable Key/TrustManager (elastic#30509) ...
ywelsch
pushed a commit
to ywelsch/elasticsearch
that referenced
this pull request
May 23, 2018
* Refactors ClientHelper to combine header logic This change removes all the `*ClientHelper` classes which were repeating logic between plugins and instead adds `ClientHelper.executeWithHeaders()` and `ClientHelper.executeWithHeadersAsync()` methods to centralise the logic for executing requests with stored security headers. * Removes Watcher headers constant
Contributor
|
@colings86 Can the "backport pending" label be removed now? |
Contributor
Author
|
Yes, sorry it got forgotten but thanks very much for catching this @jpountz |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change removes all the
*ClientHelperclasses which wererepeating logic between plugins and instead adds
ClientHelper.executeWithHeaders()andClientHelper.executeWithHeadersAsync()methods to centralise thelogic for executing requests with stored security headers.
I have request reviews from everyone I think this refractor affects and also @jaymode since this is really a security change in essence.