Skip to content

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

Merged
merged 9 commits into from
Sep 3, 2021

Conversation

masseyke
Copy link
Member

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.

@masseyke masseyke added the Team:Data Management Meta label for data/management team label Aug 13, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (Team:Core/Features)

@masseyke masseyke linked an issue Aug 13, 2021 that may be closed by this pull request
80 tasks
@masseyke masseyke requested a review from jbaiera August 20, 2021 18:03
@jbaiera
Copy link
Member

jbaiera commented Aug 26, 2021

Looking good, just a few comments to fix up first. Hopefully we can keep the security manager happy.

@jbaiera jbaiera self-requested a review August 31, 2021 19:38
@masseyke
Copy link
Member Author

masseyke commented Sep 1, 2021

@elasticmachine update branch

Copy link
Member

@jbaiera jbaiera left a 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

Comment on lines 674 to 675
static String permitsHandshakesFromIncompatibleBuildsSupplier =
System.getProperty(TransportService.PERMIT_HANDSHAKES_FROM_INCOMPATIBLE_BUILDS_KEY);
Copy link
Member

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,
Copy link
Member

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);
Copy link
Member

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

@masseyke
Copy link
Member Author

masseyke commented Sep 2, 2021

@elasticmachine run elasticsearch-ci/part-1

@masseyke masseyke merged commit d7f992b into elastic:7.x Sep 3, 2021
@masseyke masseyke deleted the feature/unsafe-handshake-deprecation branch September 3, 2021 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>non-issue Team:Data Management Meta label for data/management team v7.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Deprecation Info API checks for 8.0
6 participants