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.

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

Partial fix for issue #7698 - TlsManager support for client-side #7699

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
5 changes: 5 additions & 0 deletions common/tls/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@
<artifactId>hamcrest-all</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
</dependencies>

<build>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,14 @@ protected void reload(Optional<X509KeyManager> keyManager, Optional<X509TrustMan
trustManager.ifPresent(reloadableTrustManager::reload);
}

/**
* Initialize and set the {@link javax.net.ssl.SSLContext} on this manager instance.
*
* @param tlsConfig the tls configuration
* @param secureRandom the secure random
* @param keyManagers the key managers
* @param trustManagers the trust managers
*/
protected void initSslContext(TlsConfig tlsConfig,
SecureRandom secureRandom,
KeyManager[] keyManagers,
Expand Down Expand Up @@ -197,6 +205,15 @@ protected SecureRandom secureRandom(TlsConfig tlsConfig) {
return RANDOM.get();
}

/**
* Build the key manager factory.
*
* @param target the tls configuration
* @param secureRandom the secure random
* @param privateKey the private key for the key store
* @param certificates the certificates for the keystore
* @return a key manager factory instance
*/
protected KeyManagerFactory buildKmf(TlsConfig target,
SecureRandom secureRandom,
PrivateKey privateKey,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright (c) 2023 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.helidon.common.tls.spi;

import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;

import io.helidon.common.tls.TlsManager;

class TlsManagerCache {
private static final ConcurrentHashMap<Object, TlsManager> CACHE = new ConcurrentHashMap<>();

private TlsManagerCache() {
}

static TlsManager getOrCreate(Object configBean,
Copy link
Member

Choose a reason for hiding this comment

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

The signature should be:

static <T> TlsManager getOrCreate(T configBean, Function<T, TlsManager> creator)

if this would be the way to go, as that allows you to use a method referenc for the creator

Copy link
Member Author

Choose a reason for hiding this comment

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

pushed

Function<Object, TlsManager> creator) {
Objects.requireNonNull(configBean);
Objects.requireNonNull(creator);
return CACHE.computeIfAbsent(configBean, creator);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@

package io.helidon.common.tls.spi;

import java.util.Objects;
import java.util.function.Function;

import io.helidon.common.config.Config;
import io.helidon.common.config.ConfiguredProvider;
import io.helidon.common.tls.TlsManager;

Expand All @@ -24,4 +28,22 @@
*/
public interface TlsManagerProvider extends ConfiguredProvider<TlsManager> {

/**
* Provides the ability to have a unique {@link TlsManager} per unique {@link Config} instance provided.
*
* @param config the raw config instance
* @param configBean the config bean instance
* @param name the config bean instance name
* @param creator the creator to apply if not already in cache, which takes the config bean instance
danielkec marked this conversation as resolved.
Show resolved Hide resolved
* @return the tls manager instance from cache, defaulting to creation from the {@code creator} if not in cache
*/
static TlsManager getOrCreate(Config config,
Copy link
Member

Choose a reason for hiding this comment

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

why is there a name and config parameter, when theser are not used in the code? Please remove them
Also update the signature to what I wrote above

Copy link
Member Author

Choose a reason for hiding this comment

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

I was thinking we might need them for future use. I'll remove them but at least consider that.

Copy link
Member Author

Choose a reason for hiding this comment

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

pushed

String name,
Object configBean,
Function<Object, TlsManager> creator) {
Objects.requireNonNull(config);
Objects.requireNonNull(name);
return TlsManagerCache.getOrCreate(configBean, creator);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/*
* Copyright (c) 2023 Oracle and/or its affiliates.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package io.helidon.common.tls.spi;

import io.helidon.common.tls.ConfiguredTlsManager;
import io.helidon.common.tls.TlsManager;
import io.helidon.config.Config;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

import java.util.concurrent.atomic.AtomicInteger;

import static org.hamcrest.CoreMatchers.*;
import static org.hamcrest.MatcherAssert.assertThat;

class TlsManagerProviderTest {

@Test
void caching() {
TlsManager mock = Mockito.mock(TlsManager.class);
Config config = Config.create();
AtomicInteger count = new AtomicInteger();

// we are using "1" and "2" here abstractly to stand in for Config beans, which would hash properly
TlsManager manager1 = TlsManagerProvider.getOrCreate(config, "test", "1", (c) -> {
count.incrementAndGet();
return mock;
});
assertThat(manager1, sameInstance(mock));
assertThat(count.get(), is(1));

TlsManager manager2 = TlsManagerProvider.getOrCreate(config, "test", "1", (c) -> {
count.incrementAndGet();
return Mockito.mock(TlsManager.class);
});
assertThat(manager2, sameInstance(mock));
assertThat(count.get(), is(1));

TlsManager manager3 = TlsManagerProvider.getOrCreate(config, "test", "2", (c) -> {
count.incrementAndGet();
return Mockito.mock(TlsManager.class);
});
assertThat(manager3, notNullValue());
assertThat(manager3, not(sameInstance(mock)));
assertThat(count.get(), is(2));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,10 @@ public String configKey() {
public TlsManager create(Config config,
String name) {
OciCertificatesTlsManagerConfig cfg = OciCertificatesTlsManagerConfig.create(config);
return new DefaultOciCertificatesTlsManager(cfg, name, config);
return TlsManagerProvider.getOrCreate(config,
name,
cfg,
(c) -> new DefaultOciCertificatesTlsManager(cfg, name, config));
}

}