[TEST] Make SSL restrictions update atomic #31050
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tvernum
added
>test
|
Pinging @elastic/es-security |
jaymode
requested changes
Jun 4, 2018
| @@ -133,7 +132,8 @@ public Settings nodeSettings(int nodeOrdinal) { | |||
|
|
|||
| private void writeRestrictions(String trustedPattern) { | |||
| try { | |||
| Files.write(restrictionsPath, Collections.singleton("trust.subject_name: \"" + trustedPattern + "\"")); | |||
| Files.write(restrictionsTmpPath, Collections.singleton("trust.subject_name: \"" + trustedPattern + "\"")); | |||
| Files.move(restrictionsTmpPath, restrictionsPath, StandardCopyOption.ATOMIC_MOVE); | |||
There was a problem hiding this comment.
I think we should add REPLACE_EXISTING and handling of AtomicMoveNotSupportedException. SecurityFiles has code for this.
tvernum
added a commit
that referenced
this pull request
Jun 15, 2018
SSLTrustRestrictionsTests updates the restrictions YML file during the test run to change the set of restrictions. This update was small, but it wasn't atomic. If the yml file is reloaded while empty or invalid, then it causes all SSL certificates to be considered invalid (until it is reloaded again), which could break the sniffing/administrative client that runs underneath the tests.
tlrx
added a commit
that referenced
this pull request
Jun 15, 2018
* 6.x: 6d711fa (origin/6.x, 6.x) Uncouple persistent task state and status (#31031) f0f16b7 [TEST] Make SSL restrictions update atomic (#31050) 652193f Describe how to add a plugin in Dockerfile (#31340) bb568ab TEST: getCapturedRequestsAndClear should be atomic (#31312) 6f94914 Do not set vm.max_map_count when unnecessary (#31285) c12f3c7 Painless: Fix bug for static method calls on interfaces (#31348) 21128e2 QA: Fix resolution of default distribution (#31351) df17a83 Build: Fix the license in the pom zip and tar (#31336) 3e76b15 Treat ack timeout more like a publish timeout (#31303)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SSLTrustRestrictionsTestsupdates the restrictions YML file during the test run to change the set of restrictions. This update was small, but it wasn't atomic.If the yml file is reloaded while empty or invalid, then it causes all SSL certificates to be considered invalid (until it is reloaded again), which could break the sniffing/administrative client that runs underneath the tests.
Relates: #29989