You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "FrankYang0529 (via GitHub)" <gi...@apache.org> on 2024/08/18 14:00:24 UTC
[PR] Extract security-related helpers from scala.TestUtils to java class [kafka]
FrankYang0529 opened a new pull request, #16912:
URL: https://github.com/apache/kafka/pull/16912
*More detailed description of your change,
if necessary. The PR title and PR message become
the squashed commit message, so use a separate
comment to ping reviewers.*
*Summary of testing strategy (including rationale)
for the feature or bug fix. Unit and/or integration
tests are expected for any behaviour change and
system tests should be considered for larger changes.*
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16683: Extract security-related helpers from scala.TestUtils to java class [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 merged PR #16912:
URL: https://github.com/apache/kafka/pull/16912
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16683: Extract security-related helpers from scala.TestUtils to java class [kafka]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on PR #16912:
URL: https://github.com/apache/kafka/pull/16912#issuecomment-2354536192
Hi @chia7712, I rebased the code and all failed cases can pass on my laptop. Could you take a look when you have time? Thank you.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16683: Extract security-related helpers from scala.TestUtils to java class [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16912:
URL: https://github.com/apache/kafka/pull/16912#issuecomment-2350085048
@FrankYang0529 Could you please rebase code?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16683: Extract security-related helpers from scala.TestUtils to java class [kafka]
Posted by "FrankYang0529 (via GitHub)" <gi...@apache.org>.
FrankYang0529 commented on PR #16912:
URL: https://github.com/apache/kafka/pull/16912#issuecomment-2365162969
Hi @chia7712, thanks for the suggestion. I addressed all comments.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Extract security-related helpers from scala.TestUtils to java class [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16912:
URL: https://github.com/apache/kafka/pull/16912#issuecomment-2297594358
@FrankYang0529 please add jira number
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16683: Extract security-related helpers from scala.TestUtils to java class [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #16912:
URL: https://github.com/apache/kafka/pull/16912#discussion_r1769331209
##########
core/src/test/java/kafka/security/JaasTestUtils.java:
##########
@@ -269,4 +275,108 @@ private static void writeToFile(File file, List<JaasSection> jaasSections) throw
writer.write(String.join("", jaasSections.stream().map(Object::toString).toArray(String[]::new)));
}
}
+
+ public static boolean usesSslTransportLayer(SecurityProtocol securityProtocol) {
+ switch (securityProtocol) {
+ case SSL:
+ case SASL_SSL:
+ return true;
+ default:
+ return false;
+ }
+ }
+
+ public static boolean usesSaslAuthentication(SecurityProtocol securityProtocol) {
+ switch (securityProtocol) {
+ case SASL_PLAINTEXT:
+ case SASL_SSL:
+ return true;
+ default:
+ return false;
+ }
+ }
+
+ public static Properties sslConfigs(ConnectionMode mode,
+ boolean clientCert,
+ Optional<File> trustStoreFile,
+ String certAlias) throws Exception {
+ return sslConfigs(mode, clientCert, trustStoreFile, certAlias, SSL_CERTIFICATE_CN, TestSslUtils.DEFAULT_TLS_PROTOCOL_FOR_TESTS);
+ }
+
+ public static Properties sslConfigs(ConnectionMode mode,
+ boolean clientCert,
+ Optional<File> trustStoreFile,
+ String certAlias,
+ String certCn,
+ String tlsProtocol) throws Exception {
+ File trustStore = trustStoreFile.orElseThrow(() -> new Exception("SSL enabled but no trustStoreFile provided"));
+
+ TestSslUtils.SslConfigsBuilder sslConfigsBuilder = new TestSslUtils.SslConfigsBuilder(mode)
+ .useClientCert(clientCert)
+ .createNewTrustStore(trustStore)
+ .certAlias(certAlias)
+ .cn(certCn)
+ .tlsProtocol(tlsProtocol);
+
+ Properties sslProps = new Properties();
+ sslConfigsBuilder.build().forEach(sslProps::put);
+ return sslProps;
+ }
+
+ public static Properties producerSecurityConfigs(SecurityProtocol securityProtocol,
+ Optional<File> trustStoreFile,
+ Optional<Properties> saslProperties) throws Exception {
+ return securityConfigs(ConnectionMode.CLIENT, securityProtocol, trustStoreFile, "producer", SSL_CERTIFICATE_CN, saslProperties);
+ }
+
+ public static Properties consumerSecurityConfigs(SecurityProtocol securityProtocol, Optional<File> trustStoreFile, Optional<Properties> saslProperties) throws Exception {
+ return securityConfigs(ConnectionMode.CLIENT, securityProtocol, trustStoreFile, "consumer", SSL_CERTIFICATE_CN, saslProperties);
+ }
+
+ public static Properties adminClientSecurityConfigs(SecurityProtocol securityProtocol, Optional<File> trustStoreFile, Optional<Properties> saslProperties) throws Exception {
+ return securityConfigs(ConnectionMode.CLIENT, securityProtocol, trustStoreFile, "admin-client", SSL_CERTIFICATE_CN, saslProperties);
+ }
+
+ public static Properties securityConfigs(ConnectionMode connectionMode,
+ SecurityProtocol securityProtocol,
+ Optional<File> trustStoreFile,
+ String certAlias,
+ String certCn,
+ Optional<Properties> saslProperties) throws Exception {
+ return securityConfigs(connectionMode, securityProtocol, trustStoreFile, certAlias, certCn, saslProperties,
+ TestSslUtils.DEFAULT_TLS_PROTOCOL_FOR_TESTS, Optional.empty());
+ }
+ /**
+ * Returns security configuration options for broker or clients
+ *
+ * @param connectionMode Client or server mode
+ * @param securityProtocol Security protocol which indicates if SASL or SSL or both configs are included
+ * @param trustStoreFile Trust store file must be provided for SSL and SASL_SSL
+ * @param certAlias Alias of certificate in SSL key store
+ * @param certCn CN for certificate
+ * @param saslProperties SASL configs if security protocol is SASL_SSL or SASL_PLAINTEXT
+ * @param tlsProtocol TLS version
+ * @param needsClientCert If not empty, a flag which indicates if client certificates are required. By default
Review Comment:
`By default,`
##########
core/src/test/java/kafka/security/JaasTestUtils.java:
##########
@@ -269,4 +275,108 @@ private static void writeToFile(File file, List<JaasSection> jaasSections) throw
writer.write(String.join("", jaasSections.stream().map(Object::toString).toArray(String[]::new)));
}
}
+
+ public static boolean usesSslTransportLayer(SecurityProtocol securityProtocol) {
+ switch (securityProtocol) {
+ case SSL:
+ case SASL_SSL:
+ return true;
+ default:
+ return false;
+ }
+ }
+
+ public static boolean usesSaslAuthentication(SecurityProtocol securityProtocol) {
+ switch (securityProtocol) {
+ case SASL_PLAINTEXT:
+ case SASL_SSL:
+ return true;
+ default:
+ return false;
+ }
+ }
+
+ public static Properties sslConfigs(ConnectionMode mode,
+ boolean clientCert,
+ Optional<File> trustStoreFile,
+ String certAlias) throws Exception {
+ return sslConfigs(mode, clientCert, trustStoreFile, certAlias, SSL_CERTIFICATE_CN, TestSslUtils.DEFAULT_TLS_PROTOCOL_FOR_TESTS);
+ }
+
+ public static Properties sslConfigs(ConnectionMode mode,
+ boolean clientCert,
+ Optional<File> trustStoreFile,
+ String certAlias,
+ String certCn,
+ String tlsProtocol) throws Exception {
+ File trustStore = trustStoreFile.orElseThrow(() -> new Exception("SSL enabled but no trustStoreFile provided"));
+
+ TestSslUtils.SslConfigsBuilder sslConfigsBuilder = new TestSslUtils.SslConfigsBuilder(mode)
Review Comment:
```java
File trustStore = trustStoreFile.orElseThrow(() -> new Exception("SSL enabled but no trustStoreFile provided"));
Properties sslProps = new Properties();
sslProps.putAll(new TestSslUtils.SslConfigsBuilder(mode)
.useClientCert(clientCert)
.createNewTrustStore(trustStore)
.certAlias(certAlias)
.cn(certCn)
.tlsProtocol(tlsProtocol)
.build());
return sslProps;
```
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org