Skip to content

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
Contributor

@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