No need to have these marker interfaces around when weäre not using them anywhere, all they do is hide a lot of code duplication actually. Removing them sets up the possible removal of hundreds of lines of downstream code it seems
Transport actions have associated request and response classes. However,
the base type restrictions are not necessary to duplicate when creating
a map of transport actions. Relatedly, the ActionHandler class doesn't
actually need strongly typed action type and classes since they are lost
when shoved into the node client map. This commit removes these type
restrictions and generic parameters.
This makes using usesDefaultDistribution in our test setup for explicit by requiring a reason why it's needed.
This is helpful as part of revisiting the need for all those usages in our code base.
This updates the gradle wrapper to 8.12
We addressed deprecation warnings due to the update that includes:
- Fix change in TestOutputEvent api
- Fix deprecation in groovy syntax
- Use latest ospackage plugin containing our fix
- Remove project usages at execution time
- Fix deprecated project references in repository-old-versions
XPackFeatureSet hasn't been used for many years. But the inner "Usage" class
is still used. This commit moves the Usage class up to its own file as
XPackFeatureUsage, and removes the defunct XPackFeatureSet interface.
closes#29736
The most relevant ES changes that upgrading to Lucene 10 requires are:
- use the appropriate IOContext
- Scorer / ScorerSupplier breaking changes
- Regex automaton are no longer determinized by default
- minimize moved to test classes
- introduce Elasticsearch900Codec
- adjust slicing code according to the added support for intra-segment concurrency
- disable intra-segment concurrency in tests
- adjust accessor methods for many Lucene classes that became a record
- adapt to breaking changes in the analysis area
Co-authored-by: Christoph Büscher <christophbuescher@posteo.de>
Co-authored-by: Mayya Sharipova <mayya.sharipova@elastic.co>
Co-authored-by: ChrisHegarty <chegar999@gmail.com>
Co-authored-by: Brian Seeders <brian.seeders@elastic.co>
Co-authored-by: Armin Braun <me@obrown.io>
Co-authored-by: Panagiotis Bailis <pmpailis@gmail.com>
Co-authored-by: Benjamin Trent <4357155+benwtrent@users.noreply.github.com>
In v9 the `getRestApiVersion()` method on `RestRequest`,
`XContentBuilder` and `XContentParser` can never return `V_7`, so we can
replace all the expressions of the form `$x$.getRestApiVersion() == V_7`
with `false`. This commit does that, and then refactors away the
resulting dead code using (largely) automated transformations.
This commit bumps the REST API version from 8 to 9. This effectively removes all support for REST API
compatibility with version 7 (though earlier commits already chipped away at some v7 support).
This also enables REST API compatibility support for version 8, providing support for v8 compatibility headers,
i.e. "application/vnd.elasticsearch+json;compatible-with=8" and no-op support (no errors) to accept v9
compatibility headers i.e. "application/vnd.elasticsearch+json;compatible-with=9".
see additional context in the GH PR #113151
A predicate to check whether the cluster supports a feature is available
to rest handlers defined in server. This commit adds that predicate to
plugins defining rest handlers as well.
A couple of children of `BroadCastResponse` are completely redundant,
adding no extra fields or separate serialization.
Removed them and replaced their use by the broadcast response itself.
Same as #101175, shorten `client().prepareIndex(index)` and
`client().prepareIndex().setIndex(index)` via a test utility.
Saves lots of code now and sets up some follow-up simplifcations.
Cleaning this up a little even though it's still quite horrible.
`.get()` in this API actually means `actionGet()` so to speak.
I think a good first step to cleaning this up is to at least reduce
the duplication though and save 1k lines.
* Avoid "this-escape" by making classes final
The "this-escape" compiler warning is intended to alert
developers to potential bugs in object initialization due to
subclassing. This class of bugs cannot occur when a class is
final. Here, we take cases where a class has no implementations
but generates a "this-escape" warning, and we make those
classes final rather than suppressing the compiler warning.
This makes the remaining suppressions more meaningful, since
they now indicate places where we may want to look for
initialization bugs.
In a few cases, making a class final meant changing some of its
protected fields and methods to private or default
accessibility.
Some classes with no implementations are mocked in testing.
Since making those classes final would involve non-trivial
rewrites of tests, I've left them alone.
* Spotless, remove redundant modifiers, clean up "protected" usage
* Revert a few more mocked classes
We'd like to make `SearchResponse` reference counted and pooled but there are around 6k
instances of tests that create a `SearchResponse` local variable that would need to be
released manually to avoid leaks in the tests.
This does away with about 10% of these spots by adding an override for `assertHitCount`
that handles the actual execution of the search request and its release automatically
and making use of it in all spots where the `.get()` on the request build could be inlined
semi-automatically and in a straight-forward fashion without other code changes.
This PR is migrating some of the ITs that use either the
`elasticsearch.legacy-java-rest-test` or the
`elasticsearch.legacy-yaml-rest-test` gradle test plugins to the new
`elasticsearch.internal-java-rest-test` and
`elasticsearch.internal-yaml-rest-test` equivalents. This is the list of
the affected ITs: * SamlAuthenticationIT * OperatorPrivilegesIT *
ProfileIT * SetSecurityUserProcessorWithWithSecurityDisabledIT *
AsyncSearchSecurityIT * SecurityRealmSmokeTestCase *
KibanaSystemIndexIT * KerberosAuthenticationIT * ReindexWithSecurityIT
and ReindexWithSecurityClientYamlTestSuiteIT *
ReloadSecureSettingsWithPasswordProtectedKeystoreRestIT * PermissionsIT
from slm:qa:with-security * Permissions IT from
runtime-fields:with-security * Permissions IT from ilm:qa:with-securiy
* GraphWithSecurityIT and GraphWithSecurityInsufficientRoleIT
Related: ES-6751
Another round of automated fixes to this, marking things that can be
made static as static. Saves some JIT cycles but also turns some lambdas
from capturing to non-capturing and makes the "utilityness" of some
classes visible.
Today subclasses of `HandledTransportAction` can specify the executor on
which they run, but the executor is optional and if omitted will use
`DIRECT_EXECUTOR_SERVICE`, which means the action runs on a transport
thread. This is a dangerous default behaviour because it makes it easy
to add new transport actions which implicitly run on a network thread,
which is very hard to pick up in reviews.
This commit makes the executor explicit in all callers, and marks the
dangerous methods for removal.
Adds @SuppressWarnings("this-escape") to all necessary places to that
Elasticsearch can compile with -Werror on JDK21
No investigation has been done to determine whether any of the cases
are a potential source of errors - we have simply suppressed all
existing occurrences.
Resolves: #99845
Remove temporary TransportService#registerRequestHandler functions
that take String instead of Executor and update the callers. This
is part of a larger change to pass Executor into request handlers,
rather than creating and stashing Strings that are only used to
eventually fetch appropriate Executors anyway.
Extensive testing changes were required to make TransportService and
ThreadPool mocks capable of looking up Executors from Strings for
TransportAction constructors. Added a new MockUtils class to
modularize the additional test setup.
Part of the changes for #97879
Remove temporary TransportService#registerRequestHandler functions
that take String instead of Executor and update the callers. This
is part of a larger change to pass Executor into request handlers,
rather than creating and stashing Strings that are only used to
eventually fetch appropriate Executors anyway.
Extensive testing changes were required to make TransportService and
ThreadPool mocks capable of looking up Executors from Strings for
TransportAction constructors. Added a new MockUtils class to
modularize the additional test setup.
Part of the changes for #97879
Today `TransportResponseHandler` returns the name of the executor, to be
looked up later in the `ThreadPool`. In fact most implementations can
supply their executor directly, avoiding the lookup, and this API change
allows the handler to choose to use an executor which does not belong to
the `ThreadPool` too.
Replacing the remaining usages that I could automatically replace
and a couple that I did by hand in this PR.
Also, added the same shortcut to the single node tests to save some
duplication there.
Continued refactoring of the license service. This commit follows #94261
which moved the LicenseService -> ClusterStateLicenseService and re-introduced
LicenseService as interface. That change did not move the settings to keep the diff small.
This change moves those settings to a new class LicenseSettings.
LicenseService will be re-introduced a subsequent commit.
The `ActionListener` interface implements its static methods using
various inner classes. These inner classes are public because interfaces
do not support package-private inner classes, although they are `final`
and only have `protected` constructors which prevent anyone else from
instantiating them. It's certainly a mistake for code in other packages
to access these things.
This commit moves the implementation details into a separate class to
hide them from the rest of the world. It also tightens up the calls to
`onFailure()` in a few places to better handle the possibility of this
method throwing an exception. Finally, it moves
`DelegatingActionListener` to a top level public class since this
utility is used in quite a number of places.
This commit adds a new test framework for configuring and orchestrating
test clusters for both Java and YAML REST testing. This will eventually
replace the existing "test-clusters" Gradle plugin and the build-time
cluster orchestration.
When parsing queries on the coordinating node, there is currently no way to share state between the different parsing methods (`fromXContent`). The only query that supports a parse context is bool query, which uses the context to track nested depth of queries, added with #66204. Such nested depth tracking mechanism is not 100% accurate as it tracks bool queries only, while there's many more query types that can hold other queries hence potentially cause stack overflow when deeply nested.
This change removes the parsing context that's specific to bool query, introduced with #66204, in favour of generalizing the nested depth tracking to all query types.
The generic tracking is introduced by wrapping the parser and overriding the method that parses named objects through the xcontent registry. Another way would have been to require a context argument when parsing queries, which would mean adding a context argument to all the QueryBuilder#fromXContent static methods. That would be a breaking change for plugins that provide custom queries, hence I went for trying out a different approach.
One aspect that this change requires and introduces is the distinction between parsing a top level query (which will wrap the parser, or it would create the context if we had one), as opposed to parsing an inner query, which goes ahead with the given parser and context. We already have this distinction as we have two different static methods in `AbstractQueryBuilder` but in practice only bool query makes the distinction being the only context-aware query.
In addition to generalizing tracking nested depth when parsing queries, we should be able to adopt this same strategy to track queries usage as part #90176 .
Given that the depth check is now more restrictive, as it counts all compound queries and not only bool, we have decided to raise the default limit to `30` to ensure that users are not going to hit the limit due to this change.
Previously `graph` checked if the request timed out, then spent some
time doing work, then passed the timeout on to the next request. Over
and over again. It's quite possible that the response may not have timed
out for the first check but would have timed out for the second check.
This manifests as the timeout being sent to the next hop being a
negative number of milliseconds. We don't allow this sort of thing.
This fixes this by moving the timeout check to the same spot it is read
for setting the timeout on the next request - we just check if its `> 0`
to find the timeouts.
This does keep the request running slightly longer after it's officially
timed out - but it's just long enough to prepare the next layer of
request. Usually microseconds. Which should be fine.
Closes#55396
This fixes references to project that makes the plugin incompatible with Gradle
configuration cache. We also remove custom xpackProject utility:
using xpackProject in certain situations can break configure configuration cache compatibility as it uses a mutual project object under the hood that is discouraged to use in some use cases (e.g. at execution time)
It always breaks compatibility with --configure-on-demand
using xpackProject uses the project of the :x-pack project. referencing other project objects from other subproject should avoided where possible to decouple (sub project configurations). There's a good explanation of why we want to decouple our project configurations as much as possible here: https://docs.gradle.org/current/userguide/multi_project_configuration_and_execution.html#sec:decoupled_projects
it adds little value over default out of the box gradle api (just use project(':x-pack:someProject') instead of xpackProject('someProject') Also in some occasions its even shorter. e.g. when this is used as xpackProject('someProject').path instead of just passing :x-pack:someProject
I'll try to put a bit more context in the PR description in the future to make the motivation behind these kind of changes more clear upfront
Related to #57918
In order to fix an error where large regexes in `include` or
`exclude` fields of the `terms` agg crash the node (#82923) I'd like to
centralize construction of the `RegExp` so we can test it for
large-ness in one spot. The trouble is, there are half a dozen ctors for
`IncludeExclude` and some take `String` and some take `RegExp` and some
take a sets of `String` and some take sets of `BytesRef`. It's all very
convenient for client code, but confusing to deal with. This removes all
but two of the ctors for `IncludeExclude` and mostly standardizes on one
that has:
```
String includeRe, String excludeRe, Set<BytesRef> includePrecise, Set<BytesRef> excludePecise
```
Now I can fix#82923 in a fairly simple follow up.
Try to represent immutable data with Java records introduced in JEP 395
Convert only existing immutable classes, no "POJO with setters to a record" refactorings.
This commit changes the superuser role (as used by the "elastic"
builtin user) so that it no longer has any sort of write access to
restricted indices (system indices).
This improves the safety and security of the cluster, as it means
that there are no out-of-the-box users or roles that can write to,
delete or close the security index.
Superusers can still read from (and monitor) system indices.
Other roles (and users) can still access system indices as specified
in their descriptor. These can be custom such as the
"_es_test_root" role used in the integration test suite, or builtin
roles such as kibana_system.
Fixes split packages between server and the LLRC (and HLRC), by renaming
the server package to a more appropriate name that represents the fact
that is in an internal client. That is, rename server's
org.elasticsearch.client to org.elasticsearch.client.internal.
This commit updates all deprecation message (except for REST
compatible API messages) in 8.0+ to be emit at warning level.
Currently none of these have been removed in future versions (yet) so they
should be logged at warning, not critical.
This commit also changes the default assertWarning to assert at warning level
and introduces a new assertCriticalWarning to assert critical warnings.
