-
Notifications
You must be signed in to change notification settings - Fork 25.3k
Remove escape hatch permitting incompatible builds #76513
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
Remove escape hatch permitting incompatible builds #76513
Conversation
Pinging @elastic/es-core-features (Team:Core/Features) |
...gin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java
Outdated
Show resolved
Hide resolved
...gin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java
Outdated
Show resolved
Hide resolved
...eprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java
Outdated
Show resolved
Hide resolved
Looking good, just a few comments to fix up first. Hopefully we can keep the security manager happy. |
@elasticmachine update branch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is pretty good. I left a suggestion on another way to handle the getProperty call, but otherwise LGTM
static String permitsHandshakesFromIncompatibleBuildsSupplier = | ||
System.getProperty(TransportService.PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If anything happens to cause this to line throw it would lead to NoClassDefFoundError
issues.
static String permitsHandshakesFromIncompatibleBuildsSupplier = | ||
System.getProperty(TransportService.PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY); | ||
|
||
static DeprecationIssue checkNoPermitHandshakeFromIncompatibleBuilds(final Settings settings, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking that we pass in the System::getProperty
method reference or () -> System.getProperty("......")
to this function, and call the function to obtain the setting value
ClusterState.EMPTY_STATE, | ||
new XPackLicenseState(Settings.EMPTY, () -> 0)); | ||
assertEquals(null, expectedNullIssue); | ||
NodeDeprecationChecks.permitsHandshakesFromIncompatibleBuildsSupplier = randomAlphaOfLengthBetween(1, 10); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then here we could just pass in a () -> randomAlphaOfLengthBetween(1, 10)
to the checkNoPermitHandshakeFromeIncompatibleBuilds
function and not worry about static variables
@elasticmachine run elasticsearch-ci/part-1 |
The system property "es.unsafely_permit_handshake_from_incompatible_builds" was deprecated and has been removed in 8.0. This adds a deprecation check for that property.
Relates to #42404 and #65753.