Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: reload TLS certificate without restarting server #5516

Merged
merged 14 commits into from
Jun 8, 2020
Merged

feat: reload TLS certificate without restarting server #5516

Changes from 1 commit
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
83e607a
feat: implement auto cert reload
vcrfxia May 29, 2020
dafa8e0
chore: checkstyle
vcrfxia May 31, 2020
0ffaf29
chore: minor rename
vcrfxia May 31, 2020
4e0c365
chore: use Vert.x worker rather than executor service
vcrfxia Jun 1, 2020
48929a9
chore: move logic for restarting server from ServerVerticle to Server
vcrfxia Jun 1, 2020
2849468
test: client trust store should always be valid
vcrfxia Jun 1, 2020
d5e0821
chore: findbugs
vcrfxia Jun 2, 2020
87917ea
fix: use separate thread for file watcher
vcrfxia Jun 2, 2020
7ff1082
test: prevent cert reload test from affecting other TLS tests
vcrfxia Jun 4, 2020
7221ead
Merge branch 'master' into auto-cert-reload
vcrfxia Jun 4, 2020
c700eb9
test: add logging to debug potential flakiness with shouldReloadCert()
vcrfxia Jun 4, 2020
b23542f
test: update assertion to be more informative on failure
vcrfxia Jun 4, 2020
1958270
refactor: file watcher extends thread
vcrfxia Jun 5, 2020
7a1d136
fix: configure cert reload before deploying instances
vcrfxia Jun 5, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
test: prevent cert reload test from affecting other TLS tests
  • Loading branch information
@vcrfxia
vcrfxia committed Jun 4, 2020
commit 7ff1082b15566cc52fb7aed1006efd40a199aa30
86 changes: 44 additions & 42 deletions ksqldb-rest-app/src/test/java/io/confluent/ksql/api/TlsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,47 +95,49 @@ public void shouldReloadCert() throws Exception {
assertThat(response.statusCode(), is(200));
assertThat(response.statusMessage(), is("OK"));

// When: load expired key store
ServerKeyStore.loadExpiredServerKeyStore();
assertThatEventually(
"Should fail to execute query with expired key store",
() -> {
// re-create client since server port changes on restart
this.client = createClient();

try {
// this should fail
sendRequest("/query-stream", requestBody.toBuffer());
return false;
} catch (Exception e) {
assertThat(e, instanceOf(ExecutionException.class)); // thrown from CompletableFuture.get()
assertThat(e.getMessage(),
containsString("javax.net.ssl.SSLHandshakeException: Failed to create SSL connection"));
return true;
}
},
is(true),
TimeUnit.SECONDS.toMillis(1),
TimeUnit.SECONDS.toMillis(1)
);

// When: load valid store
ServerKeyStore.loadValidServerKeyStore();
assertThatEventually(
"Should successfully execute query with valid key store",
() -> {
// re-create client since server port changes on restart
this.client = createClient();

try {
return sendRequest("/query-stream", requestBody.toBuffer()).statusCode();
} catch (Exception e) {
return 0;
}
},
is(200),
TimeUnit.SECONDS.toMillis(1),
TimeUnit.SECONDS.toMillis(1)
);
try {
// When: load expired key store
ServerKeyStore.loadExpiredServerKeyStore();
assertThatEventually(
"Should fail to execute query with expired key store",
() -> {
// re-create client since server port changes on restart
this.client = createClient();

try {
// this should fail
sendRequest("/query-stream", requestBody.toBuffer());
return false;
} catch (Exception e) {
assertThat(e,
instanceOf(ExecutionException.class)); // thrown from CompletableFuture.get()
return e.getMessage()
.contains("javax.net.ssl.SSLHandshakeException: Failed to create SSL connection");
}
},
is(true),
TimeUnit.SECONDS.toMillis(1),
TimeUnit.SECONDS.toMillis(1)
);
} finally { // restore cert regardless of failure above so as to not affect other tests
// When: load valid store
ServerKeyStore.loadValidServerKeyStore();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be good to test reloading the server more than once - i.e. changing the cert more than once, as there might be a bug in the filewatcher/reloading that only manifests after the reload has been done once.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test already reloads the server twice: once with a bad cert, and then again with a good cert. Are you suggesting we should repeat the cycle twice, for a total of four reloads?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, if you're already reloading more than once that seems fine :)

assertThatEventually(
"Should successfully execute query with valid key store",
() -> {
// re-create client since server port changes on restart
this.client = createClient();

try {
return sendRequest("/query-stream", requestBody.toBuffer()).statusCode();
} catch (Exception e) {
return 0;
}
},
is(200),
TimeUnit.SECONDS.toMillis(1),
TimeUnit.SECONDS.toMillis(1)
);
}
}
}