Skip to content

Fix SSLConfigurationReloaderTests failure tests #39408

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 1 commit into from
Feb 27, 2019

Conversation

jaymode
Copy link
Member

@jaymode jaymode commented Feb 26, 2019

This change fixes the tests that expect the reload of a
SSLConfiguration to fail. The tests relied on an incorrect assumption
that the reloader only called reload on for an SSLConfiguration if the
key and trust managers were successfully reloaded, but that is not the
case. This change removes the fail call with a wrapped call to the
original method and captures the exception and counts down a latch to
make these tests consistently tested.

Closes #39260

This change fixes the tests that expect the reload of a
SSLConfiguration to fail after changes made in elastic#30509. The tests relied
on the behavior that an SSLConfiguration only had reload called on it
after the key and trust managers had been created, but that is no
longer the case. This change removes the fail call with a wrapped call
to the original method and captures the exception and counts down a
latch to make these tests consistently tested rather than relying on
concurrency to catch a failure.

Closes elastic#39260
@jaymode jaymode added >test Issues or PRs that are addressing/adding tests v7.0.0 :Security/TLS SSL/TLS, Certificates v6.7.0 v8.0.0 v7.2.0 v6.6.2 labels Feb 26, 2019
@jaymode jaymode requested a review from jkakavas February 26, 2019 17:52
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

Copy link
Member

@jkakavas jkakavas left a comment

Choose a reason for hiding this comment

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

LGTM

@jaymode jaymode merged commit 3b9b4ce into elastic:master Feb 27, 2019
@jaymode jaymode deleted the fix_ssl_reload_tests branch February 27, 2019 15:17
jaymode added a commit that referenced this pull request Feb 27, 2019
This change fixes the tests that expect the reload of a
SSLConfiguration to fail. The tests relied on an incorrect assumption
that the reloader only called reload on for an SSLConfiguration if the
key and trust managers were successfully reloaded, but that is not the
case. This change removes the fail call with a wrapped call to the
original method and captures the exception and counts down a latch to
make these tests consistently tested.

Closes #39260
jaymode added a commit that referenced this pull request Feb 27, 2019
This change fixes the tests that expect the reload of a
SSLConfiguration to fail. The tests relied on an incorrect assumption
that the reloader only called reload on for an SSLConfiguration if the
key and trust managers were successfully reloaded, but that is not the
case. This change removes the fail call with a wrapped call to the
original method and captures the exception and counts down a latch to
make these tests consistently tested.

Closes #39260
jaymode added a commit that referenced this pull request Feb 27, 2019
This change fixes the tests that expect the reload of a
SSLConfiguration to fail. The tests relied on an incorrect assumption
that the reloader only called reload on for an SSLConfiguration if the
key and trust managers were successfully reloaded, but that is not the
case. This change removes the fail call with a wrapped call to the
original method and captures the exception and counts down a latch to
make these tests consistently tested.

Closes #39260
jaymode added a commit that referenced this pull request Feb 27, 2019
This change fixes the tests that expect the reload of a
SSLConfiguration to fail. The tests relied on an incorrect assumption
that the reloader only called reload on for an SSLConfiguration if the
key and trust managers were successfully reloaded, but that is not the
case. This change removes the fail call with a wrapped call to the
original method and captures the exception and counts down a latch to
make these tests consistently tested.

Closes #39260
@jpountz
Copy link
Contributor

jpountz commented Jul 5, 2019

@jaymode I'm assuming there is nothing left to backport and removed the backport pending label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Security/TLS SSL/TLS, Certificates >test Issues or PRs that are addressing/adding tests v6.8.2 v7.3.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SSLConfigurationReloaderTests.testReloadingPEMKeyConfigException failure
7 participants