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

Pass allow security manager flag in gradle test policy setup plugin #111725

Conversation

mark-vieira
Copy link
Contributor

The java.security.manager=allow system property is required when running tests on newer Java versions with the security manager deprecated. As such, it should be set in our GradleTestPolicySetupPlugin so that it's done for external plugin authors.

@mark-vieira mark-vieira requested a review from a team as a code owner August 8, 2024 22:42
@elasticsearchmachine elasticsearchmachine added Team:Delivery Meta label for Delivery team v8.16.0 labels Aug 8, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-delivery (Team:Delivery)

Copy link
Member

@rjernst rjernst left a comment

Choose a reason for hiding this comment

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

This looks fine, but I suspect we need more. Other stuff we do in ElasticsearchTestBasePlugin is needed like setting up java.library.path/jna.library.path

@mark-vieira
Copy link
Contributor Author

This looks fine, but I suspect we need more. Other stuff we do in ElasticsearchTestBasePlugin is needed like setting up java.library.path/jna.library.path

Probably, it's likely our example plugin tests just aren't exercising that code.

@mark-vieira mark-vieira added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Aug 8, 2024
Copy link
Contributor

@john-wagster john-wagster left a comment

Choose a reason for hiding this comment

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

LGTM, fixes an issues I accidentally stumbled upon when trying to run yaml tests against the plugin build with jdk 22.

@elasticsearchmachine elasticsearchmachine merged commit e235880 into elastic:main Aug 8, 2024
16 checks passed
@mark-vieira mark-vieira deleted the example-plugin-security-manager-fix branch August 8, 2024 23:49
mark-vieira added a commit to mark-vieira/elasticsearch that referenced this pull request Aug 8, 2024
…lastic#111725)

The `java.security.manager=allow` system property is required when
running tests on newer Java versions with the security manager
deprecated. As such, it should be set in our
`GradleTestPolicySetupPlugin` so that it's done for external plugin
authors.
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.15

elasticsearchmachine pushed a commit that referenced this pull request Aug 9, 2024
…111725) (#111728)

The `java.security.manager=allow` system property is required when
running tests on newer Java versions with the security manager
deprecated. As such, it should be set in our
`GradleTestPolicySetupPlugin` so that it's done for external plugin
authors.
weizijun added a commit to weizijun/elasticsearch that referenced this pull request Aug 9, 2024
* upstream/main: (22 commits)
  Prune changelogs after 8.15.0 release
  Bump versions after 8.15.0 release
  EIS integration (elastic#111154)
  Skip LOOKUP/INLINESTATS cases unless on snapshot (elastic#111755)
  Always enforce strict role validation (elastic#111056)
  Mute org.elasticsearch.xpack.esql.analysis.VerifierTests testUnsupportedAndMultiTypedFields elastic#111753
  [ML] Force time shift integration test (elastic#111620)
  ESQL: Add tests for sort, where with unsupported type (elastic#111737)
  [ML] Force time shift documentation (elastic#111668)
  Fix remote cluster credential secure settings reload   (elastic#111535)
  ESQL: Fix for overzealous validation in case of invalid mapped fields (elastic#111475)
  Pass allow security manager flag in gradle test policy setup plugin (elastic#111725)
  Rename streamContent/Separator to bulkContent/Separator (elastic#111716)
  Mute org.elasticsearch.tdigest.ComparisonTests testSparseGaussianDistribution elastic#111721
  Remove 8.14 from branches.json
  Only emit product origin in deprecation log if present (elastic#111683)
  Forward port release notes for v8.15.0 (elastic#111714)
  [ES|QL] Combine Disjunctive CIDRMatch (elastic#111501)
  ESQL: Remove qualifier from attrs (elastic#110581)
  Force using the last centroid during merging (elastic#111644)
  ...

# Conflicts:
#	server/src/main/java/org/elasticsearch/TransportVersions.java
#	x-pack/plugin/inference/src/main/java/org/elasticsearch/xpack/inference/InferenceNamedWriteablesProvider.java
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this pull request Sep 4, 2024
…lastic#111725)

The `java.security.manager=allow` system property is required when
running tests on newer Java versions with the security manager
deprecated. As such, it should be set in our
`GradleTestPolicySetupPlugin` so that it's done for external plugin
authors.
davidkyle pushed a commit to davidkyle/elasticsearch that referenced this pull request Sep 5, 2024
…lastic#111725)

The `java.security.manager=allow` system property is required when
running tests on newer Java versions with the security manager
deprecated. As such, it should be set in our
`GradleTestPolicySetupPlugin` so that it's done for external plugin
authors.
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!) :Delivery/Build Build or test infrastructure >non-issue Team:Delivery Meta label for Delivery team v8.15.1 v8.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants