Skip to content

Replace hardcoded JKS keystore type with KeyStore.getDefaultType()#12506

Draft
mamccorm wants to merge 1 commit intostrimzi:mainfrom
mamccorm:use-default-keystore-type
Draft

Replace hardcoded JKS keystore type with KeyStore.getDefaultType()#12506
mamccorm wants to merge 1 commit intostrimzi:mainfrom
mamccorm:use-default-keystore-type

Conversation

@mamccorm
Copy link

@mamccorm mamccorm commented Mar 9, 2026

Type of change

Select the type of your PR

  • Refactoring

Description

In-memory keystores used for TrustManagerFactory/KeyManagerFactory do not need a specific type. Using the JVM's default allows FIPS-configured JVMs to use their preferred keystore type (e.g. BCFKS) instead of failing on JKS.

Checklist

Please go through this checklist and make sure all applicable tasks have been done

  • Write tests
  • Make sure all tests pass
  • Update documentation
  • Check RBAC rights for Kubernetes / OpenShift roles
  • Try your changes from Pod inside your Kubernetes and OpenShift cluster, not just locally
  • Reference relevant issue(s) and close them after merging
  • Update CHANGELOG.md
  • Supply screenshots for visual changes, such as Grafana dashboards

In-memory keystores used for TrustManagerFactory/KeyManagerFactory
do not need a specific type. Using the JVM's default allows
FIPS-configured JVMs to use their preferred keystore type (e.g. BCFKS)
instead of failing on JKS.

Signed-off-by: Mark McCormick <mark.mccormick@chainguard.dev>
@mamccorm mamccorm force-pushed the use-default-keystore-type branch from 95ea44c to ec2562a Compare March 9, 2026 12:20
@scholzj
Copy link
Member

scholzj commented Mar 9, 2026

I'm not convinced this makes sense. The code was designed and tested with JKS. Not with any default store type. So I think there can be a discussion about making it configurable and a module. But it should not blindly use any default store type. Also, keep in mind that this is just a small subset of places that work with certificates. So in terms of any compliance, I do not think it has any meaning.

@sanjayk0508
Copy link

these files have same occurrences too

  • certificate-manager/src/main/java/io/strimzi/certs/OpenSslCertManager.java
  • topic-operator/src/main/java/io/strimzi/operator/topic/cruisecontrol/CruiseControlClientImpl.java

@sanjayk0508
Copy link

sanjayk0508 commented Mar 9, 2026

@scholzj KeyStore.getDefaultType() reads from java.security properties, given that AbstractKafkaClientProperties.java uses KeyStore.getInstance(TRUSTSTORE_TYPE_CONFIG) as a configurable, ref - https://github.com/strimzi/strimzi-kafka-operator/blob/main/systemtest/src/main/java/io/strimzi/systemtest/kafkaclients/clientproperties/AbstractKafkaClientProperties.java#L165, since the PR is trying to apply the same idea to the in-memory keystores in PemTrustSet/PemAuthIdentity/KafkaAgentUtils, is the TRUSTSTORE_TYPE_CONFIG what you mean by "configurable and a module"?

@scholzj
Copy link
Member

scholzj commented Mar 9, 2026

@scholzj KeyStore.getDefaultType() reads from java.security properties, given that AbstractKafkaClientProperties.java uses KeyStore.getInstance(TRUSTSTORE_TYPE_CONFIG) as a configurable, ref - https://github.com/strimzi/strimzi-kafka-operator/blob/main/systemtest/src/main/java/io/strimzi/systemtest/kafkaclients/clientproperties/AbstractKafkaClientProperties.java#L165, since the PR is trying to apply the same idea to the in-memory keystores in PemTrustSet/PemAuthIdentity/KafkaAgentUtils, is the TRUSTSTORE_TYPE_CONFIG what you mean by "configurable and a module"?

I have no idea what the TRUSTSTORE_TYPE_CONFIG is here. But it is in a completely different part of the codebase.

@ppatierno
Copy link
Member

Hi,
I would like to have more insights about these changes and why you think they are needed. Maybe adding more context and information within the PR description would be a good start.
Saying that, I am not sure how it would be tested on environments with a different keystore type, given we are not assuming it's always "JKS" but could be something else.
Finally, the usage of TRUSTSTORE_TYPE_CONFIG is just within system tests. It's set to PKCS12 and used to set up truststore for clients during tests. I am even not sure it's strictly related to what you are trying to achieve.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants