chibenwa commented on PR #1559:
URL: https://github.com/apache/james-project/pull/1559#issuecomment-1547154365
> ...I initially had that in mind and asked about dedicated module, to
handle it, to which you replied with "Each protocol module can independently
define the bindings without needing a centralized place at all..." hence I
followed your guidance :-)
Well, the above quoted statement is compatible with "one route to reload all
certificates" and do not mean "one route per protocol" ;-)
> TBH, in our downstream project I used that approach and simply rely on
injected Set<AbstractServerFactory> to reload certificates for all
servers/protocols :-)
This, would look great to me, indeed!
> I'll add it later on, though would be nice to clarify/confirm endpoints -
do we want to have it directly or under `/server/?
Is:
```
POST /servers?reloadSSLCertificate
```
Ok with you?
Else what do you propose?
> I would also say that we lack some tests. Did you think of a potential
testing strategy for this changeset?
`server/container/guice/common/src/main/java/org/apache/james/TemporaryJamesServer.java`
defines a way to programmatically override configuration files, and thus test
numerous configuration set up within the same maven project.
The way it works is writing the desired configuration to a temporary file
and use it upon application startup.
We could easily...
- 1. Have `keystore` and `keystore2` as test resources
- 2. Have `smtpserver.xml` pointing to
`<keystore>file://conf/keystore</keystore>` (default)
- 3. Start james
- 4. Manually modify the smtpserver.xml temporary file (bit tricky but
doable) to use `<keystore>file://conf/keystore2</keystore>`
- 5. relead the cetrificate
- 6. Ensure that a SSL client is now presented with the newer certificate.
The easiest way to achieve (4.) would be to enhance `TemporaryJamesServer`
in order to handle renames upon copying a resource to its configuration
folder...
Eg:
```
private void copyResources() {
configurationFileNames
.forEach(this::copyResource);
}
public void copyResource(String resourceName) {
copyResource(resourceName, resourceName);
}
public void copyResource(String resourceName, String targetName) {
var resolvedResource =
Paths.get(configurationFolder.getAbsolutePath()).resolve(targetName);
try (OutputStream outputStream = new
FileOutputStream(resolvedResource.toFile())) {
URL resource =
ClassLoader.getSystemClassLoader().getResource(resourceName);
if (resource != null) {
try (InputStream stream = resource.openStream()) {
stream.transferTo(outputStream);
}
} else {
throw new RuntimeException("Failed to load configuration
resource " + resourceName);
}
} catch (IOException e) {
throw new RuntimeException(e);
}
}
```
Then a test could look like:
```
class SSLCertificateReloadTest {
private static final List<String> BASE_CONFIGURATION_FILE_NAMES =
ImmutableList.of("dnsservice.xml",
"dnsservice.xml",
"imapserver.xml",
"jwt_publickey",
"lmtpserver.xml",
"mailetcontainer.xml",
"mailrepositorystore.xml",
"managesieveserver.xml",
"pop3server.xml",
"smtpserver.xml");
private GuiceJamesServer jamesServer;
private TemporaryJamesServer temporaryJamesServer;
@BeforeEach
void beforeEach(@TempDir Path workingPath) throws Exception {
temporaryJamesServer = new
TemporaryJamesServer(workingPath.toFile(), BASE_CONFIGURATION_FILE_NAMES);
jamesServer = temporaryJamesServer.getJamesServer()
.combineWith(IN_MEMORY_SERVER_AGGREGATE_MODULE)
.combineWith(new UsersRepositoryModuleChooser(new
MemoryUsersRepositoryModule())
.chooseModules(UsersRepositoryModuleChooser.Implementation.DEFAULT))
.overrideWith(new JMXServerModule(),
binder ->
binder.bind(ListeningMessageSearchIndex.class).toInstance(mock(ListeningMessageSearchIndex.class)));
}
@AfterEach
void afterEach() {
if (jamesServer != null && jamesServer.isStarted()) {
jamesServer.stop();
}
}
@Test
void jamesCliShouldFailWhenNotGiveAuthCredential() throws Exception {
// Start james with certificated within smtpserver.xml
jamesServer.start();
// change certificate
temporaryJamesServer.copyResource("smtpserver-new-certificates.xml",
"smtpserver.xml");
// Reload certificates
// XPOST /servers?reloadSSLCertificate
// Open an SSL connection and check certificate
}
}
```
(Possible test location: `memory-webadmin-integration-test` module?)
We could also add a test ensuring reloading ssl certificate do not close
existing connections.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
