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

Updates to Infinispan 14.0.19.Final #36445

Merged
merged 1 commit into from
Oct 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion bom/application/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@
<rest-assured.version>5.3.0</rest-assured.version>
<junit.jupiter.version>5.10.0</junit.jupiter.version>
<junit-pioneer.version>1.5.0</junit-pioneer.version>
<infinispan.version>14.0.17.Final</infinispan.version>
<infinispan.version>14.0.19.Final</infinispan.version>
<infinispan.protostream.version>4.6.5.Final</infinispan.protostream.version>
<caffeine.version>3.1.5</caffeine.version>
<netty.version>4.1.100.Final</netty.version>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ public void infinispanConnectionConfiguration() {
assertThat(configuration.security().ssl().provider()).isEqualTo("SSL_prov");
assertThat(configuration.security().ssl().protocol()).isEqualTo("SSL_protocol");
assertThat(configuration.security().ssl().ciphers()).containsExactlyInAnyOrder("SSL_cipher1", "SSL_cipher2");
assertThat(configuration.security().ssl().hostnameValidation()).isTrue();
assertThat(configuration.security().ssl().sniHostName()).isEqualTo("sniHostName");
assertThat(configuration.clusters()).extracting("clusterName", "clientIntelligence")
.containsExactly(tuple("bsite", ClientIntelligence.BASIC));
assertThat(configuration.clusters()).hasSize(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ quarkus.infinispan-client.trust-store-type=JCEKS
quarkus.infinispan-client.ssl-provider=SSL_prov
quarkus.infinispan-client.ssl-protocol=SSL_protocol
quarkus.infinispan-client.ssl-ciphers=SSL_cipher1,SSL_cipher2
quarkus.infinispan-client.ssl-host-name-validation=true
quarkus.infinispan-client.sni-host-name=sniHostName

quarkus.infinispan-client.backup-cluster.bsite.hosts=bsite1:32111
quarkus.infinispan-client.backup-cluster.bsite.client-intelligence=BASIC

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,15 @@ private ConfigurationBuilder builderFromProperties(String infinispanClientName,
infinispanClientRuntimeConfig.sslCiphers.get().stream().collect(Collectors.joining(" ")));
}

if (infinispanClientRuntimeConfig.sslHostNameValidation.isPresent()) {
properties.put(ConfigurationProperties.SSL_HOSTNAME_VALIDATION,
infinispanClientRuntimeConfig.sslHostNameValidation.get());
}

if (infinispanClientRuntimeConfig.sniHostName.isPresent()) {
Copy link
Member

Choose a reason for hiding this comment

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

Here, you already know that sniHostName is not set, while hostname and validation is enabled. You could throw configuration exception. IMO it would be nicer than having it thrown during bean creating on startup event. But I take it you don't want to repeat validation that is already in the Infinispan client?

It's really just FYI, nothing else.

21:07:04,706 INFO  [app] 21:07:03,928 Failed to start application (with profile [prod]): java.lang.RuntimeException: Failed to start quarkus
21:07:04,706 INFO  [app] 	at io.quarkus.runner.ApplicationImpl.doStart(Unknown Source)
21:07:04,706 INFO  [app] 	at io.quarkus.runtime.Application.start(Application.java:101)
21:07:04,707 INFO  [app] 	at io.quarkus.runtime.ApplicationLifecycleManager.run(ApplicationLifecycleManager.java:111)
21:07:04,707 INFO  [app] 	at io.quarkus.runtime.Quarkus.run(Quarkus.java:71)
21:07:04,707 INFO  [app] 	at io.quarkus.runtime.Quarkus.run(Quarkus.java:44)
21:07:04,707 INFO  [app] 	at io.quarkus.runtime.Quarkus.run(Quarkus.java:124)
21:07:04,707 INFO  [app] 	at io.quarkus.runner.GeneratedMain.main(Unknown Source)
21:07:04,707 INFO  [app] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
21:07:04,707 INFO  [app] 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
21:07:04,707 INFO  [app] 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
21:07:04,707 INFO  [app] 	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
21:07:04,708 INFO  [app] 	at io.quarkus.bootstrap.runner.QuarkusEntryPoint.doRun(QuarkusEntryPoint.java:61)
21:07:04,708 INFO  [app] 	at io.quarkus.bootstrap.runner.QuarkusEntryPoint.main(QuarkusEntryPoint.java:32)
21:07:04,708 INFO  [app] Caused by: jakarta.enterprise.inject.CreationException: Error creating synthetic bean [ddee68cd3f60c96290f55bc237588d70e5c96aab]: org.infinispan.commons.CacheConfigurationException: ISPN004112: The SNI hostname is required when hostname validation is enabled
21:07:04,708 INFO  [app] 	at org.infinispan.client.hotrod.RemoteCacheManager_ddee68cd3f60c96290f55bc237588d70e5c96aab_Synthetic_Bean.doCreate(Unknown Source)
21:07:04,708 INFO  [app] 	at org.infinispan.client.hotrod.RemoteCacheManager_ddee68cd3f60c96290f55bc237588d70e5c96aab_Synthetic_Bean.create(Unknown Source)
21:07:04,708 INFO  [app] 	at org.infinispan.client.hotrod.RemoteCacheManager_ddee68cd3f60c96290f55bc237588d70e5c96aab_Synthetic_Bean.create(Unknown Source)
21:07:04,708 INFO  [app] 	at io.quarkus.arc.impl.AbstractSharedContext.createInstanceHandle(AbstractSharedContext.java:113)
21:07:04,709 INFO  [app] 	at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:37)
21:07:04,709 INFO  [app] 	at io.quarkus.arc.impl.AbstractSharedContext$1.get(AbstractSharedContext.java:34)
21:07:04,709 INFO  [app] 	at io.quarkus.arc.impl.LazyValue.get(LazyValue.java:32)
21:07:04,709 INFO  [app] 	at io.quarkus.arc.impl.ComputingCache.computeIfAbsent(ComputingCache.java:69)
21:07:04,709 INFO  [app] 	at io.quarkus.arc.impl.AbstractSharedContext.get(AbstractSharedContext.java:34)
21:07:04,709 INFO  [app] 	at io.quarkus.arc.impl.ClientProxies.getApplicationScopedDelegate(ClientProxies.java:21)
21:07:04,709 INFO  [app] 	at org.infinispan.client.hotrod.RemoteCacheManager_ddee68cd3f60c96290f55bc237588d70e5c96aab_Synthetic_ClientProxy.arc$delegate(Unknown Source)
21:07:04,709 INFO  [app] 	at org.infinispan.client.hotrod.RemoteCacheManager_ddee68cd3f60c96290f55bc237588d70e5c96aab_Synthetic_ClientProxy.administration(Unknown Source)
21:07:04,709 INFO  [app] 	at io.quarkus.ts.messaging.infinispan.grpc.kafka.InfinispanPopulated.onStart(InfinispanPopulated.java:28)
21:07:04,710 INFO  [app] 	at io.quarkus.ts.messaging.infinispan.grpc.kafka.InfinispanPopulated_Observer_onStart_acc4e7f81539047dcf8d1cef111350cdc8ea75f8.notify(Unknown Source)
21:07:04,710 INFO  [app] 	at io.quarkus.arc.impl.EventImpl$Notifier.notifyObservers(EventImpl.java:346)
21:07:04,710 INFO  [app] 	at io.quarkus.arc.impl.EventImpl$Notifier.notify(EventImpl.java:328)
21:07:04,710 INFO  [app] 	at io.quarkus.arc.impl.EventImpl.fire(EventImpl.java:82)
21:07:04,710 INFO  [app] 	at io.quarkus.arc.runtime.ArcRecorder.fireLifecycleEvent(ArcRecorder.java:155)
21:07:04,710 INFO  [app] 	at io.quarkus.arc.runtime.ArcRecorder.handleLifecycleEvents(ArcRecorder.java:106)
21:07:04,710 INFO  [app] 	at io.quarkus.deployment.steps.LifecycleEventsBuildStep$startupEvent1144526294.deploy_0(Unknown Source)
21:07:04,711 INFO  [app] 	at io.quarkus.deployment.steps.LifecycleEventsBuildStep$startupEvent1144526294.deploy(Unknown Source)
21:07:04,711 INFO  [app] 	... 13 more
21:07:04,711 INFO  [app] Caused by: org.infinispan.commons.CacheConfigurationException: ISPN004112: The SNI hostname is required when hostname validation is enabled
21:07:04,711 INFO  [app] 	at org.infinispan.client.hotrod.configuration.SslConfigurationBuilder.validate(SslConfigurationBuilder.java:260)
21:07:04,711 INFO  [app] 	at org.infinispan.client.hotrod.configuration.SecurityConfigurationBuilder.validate(SecurityConfigurationBuilder.java:51)
21:07:04,711 INFO  [app] 	at org.infinispan.client.hotrod.configuration.ConfigurationBuilder.validate(ConfigurationBuilder.java:596)
21:07:04,711 INFO  [app] 	at org.infinispan.client.hotrod.configuration.ConfigurationBuilder.build(ConfigurationBuilder.java:664)
21:07:04,711 INFO  [app] 	at org.infinispan.client.hotrod.configuration.ConfigurationBuilder.build(ConfigurationBuilder.java:659)
21:07:04,712 INFO  [app] 	at io.quarkus.infinispan.client.runtime.InfinispanClientProducer.initialize(InfinispanClientProducer.java:109)
21:07:04,712 INFO  [app] 	at io.quarkus.infinispan.client.runtime.InfinispanClientProducer.getNamedRemoteCacheManager(InfinispanClientProducer.java:428)
21:07:04,712 INFO  [app] 	at io.quarkus.infinispan.client.runtime.InfinispanRecorder$1.apply(InfinispanRecorder.java:36)
21:07:04,712 INFO  [app] 	at io.quarkus.infinispan.client.runtime.InfinispanRecorder$1.apply(InfinispanRecorder.java:33)
21:07:04,712 INFO  [app] 	at io.quarkus.infinispan.client.runtime.InfinispanRecorder$InfinispanClientSupplier.get(InfinispanRecorder.java:82)
21:07:04,712 INFO  [app] 	at io.quarkus.arc.runtime.ArcRecorder$4.apply(ArcRecorder.java:129)
21:07:04,712 INFO  [app] 	at io.quarkus.arc.runtime.ArcRecorder$4.apply(ArcRecorder.java:126)
21:07:04,712 INFO  [app] 	at org.infinispan.client.hotrod.RemoteCacheManager_ddee68cd3f60c96290f55bc237588d70e5c96aab_Synthetic_Bean.createSynthetic(Unknown Source)
21:07:04,713 INFO  [app] 	... 34 more

Copy link
Member Author

@karesti karesti Oct 17, 2023

Choose a reason for hiding this comment

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

This kind of validation depends on Infinispan, is not Quarkus logic. I won't duplicate that here, since this logic can change for security or any other reasons in Infinispan.

Copy link
Member

Choose a reason for hiding this comment

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

this logic can change for security or any other reasons in Infinispan.

But you document it, how can you document it in Quarkus and say it can change in Infinispan? It will leave Quarkus documentation inconsistent.

Copy link
Member

Choose a reason for hiding this comment

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

Anyway, I think I get your point, thanks for your answer and this PR.

properties.put(ConfigurationProperties.SNI_HOST_NAME, infinispanClientRuntimeConfig.sniHostName.get());
}

builder.withProperties(properties);

if (infinispanClientRuntimeConfig.tracingPropagationEnabled.isPresent()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,19 @@ public class InfinispanClientRuntimeConfig {
@ConfigItem
Optional<List<String>> sslCiphers;

/**
* Do SSL hostname validation.
* Defaults to true.
karesti marked this conversation as resolved.
Show resolved Hide resolved
*/
@ConfigItem
Optional<Boolean> sslHostNameValidation;
Copy link
Member

Choose a reason for hiding this comment

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

It's unclear to me, why exactly is this optional when no default value is set via SmallRye Config. When no default value is set, then value is either true of false, no? I think setting default value would be clearer.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's the default value in Infinispan

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure I should write this, because I don't think it is important at all, I just thought you could think about it when refactoring stuff in the future:

  • you document default to true, therefore whether it is default in Infinispan or not, it must be default in Quarkus, therefore saying I only set this value when user does means:
    • you are using Optional when not necessary, you can't just delete default value here because there is none
    • you are documenting default value in description instead of default value
    • boolean is either true or false, so not setting it to the Infinispan client later means you trust you will keep consistency and not forget when anything changes. you are not gaining anything by not setting it, just risking future inconsistency. That is - writing description and having default somewhere else than in Quarkus.


/**
* SNI host name. Mandatory when SSL is enabled and host name validation is true.
*/
@ConfigItem
Optional<String> sniHostName;

/**
* Whether a tracing propagation is enabled in case the Opentelemetry extension is present.
* By default the propagation of the context is propagated from the client to the Infinispan Server.
Expand Down
Loading