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.

Sign up for GitHub

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

[feat] Add hostname verification config to ClusterData #45

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

[feat] Add hostname verification config to ClusterData #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fb0cb1c
[feat] Add hostname verification config to ClusterData
michaeljmarshall Mar 1, 2023
2cff8e1
Update default to true for PIP
michaeljmarshall May 10, 2023
5a7cde1
Simplify geo-replication client configuration
michaeljmarshall May 10, 2023
a9af541
Simplify geo-replication client configuration again
michaeljmarshall May 10, 2023
6601828
Defer changing argument names to another PR
michaeljmarshall May 10, 2023
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
6 changes: 3 additions & 3 deletions pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1308,7 +1308,7 @@ public PulsarClient getReplicationClient(String cluster, Optional<ClusterData> c
data.getBrokerClientTrustCertsFilePath(),
data.getBrokerClientKeyFilePath(),
data.getBrokerClientCertificateFilePath(),
pulsar.getConfiguration().isTlsHostnameVerificationEnabled()
data.isTlsHostnameVerificationEnabled()
);
} else if (pulsar.getConfiguration().isBrokerClientTlsEnabled()) {
configTlsSettings(clientBuilder, serviceUrlTls,
Expand Down Expand Up @@ -1445,7 +1445,7 @@ public PulsarAdmin getClusterPulsarAdmin(String cluster, Optional<ClusterData> c
data.getBrokerClientTrustCertsFilePath(),
data.getBrokerClientKeyFilePath(),
data.getBrokerClientCertificateFilePath(),
pulsar.getConfiguration().isTlsHostnameVerificationEnabled()
data.isTlsHostnameVerificationEnabled()
);
} else if (conf.isBrokerClientTlsEnabled()) {
configAdminTlsSettings(builder,
Expand All @@ -1460,7 +1460,7 @@ public PulsarAdmin getClusterPulsarAdmin(String cluster, Optional<ClusterData> c
conf.getBrokerClientTrustCertsFilePath(),
conf.getBrokerClientKeyFilePath(),
conf.getBrokerClientCertificateFilePath(),
pulsar.getConfiguration().isTlsHostnameVerificationEnabled()
conf.isTlsHostnameVerificationEnabled()
);
}

Expand Down
4 changes: 4 additions & 0 deletions ...ar-client-admin-api/src/main/java/org/apache/pulsar/common/policies/data/ClusterData.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ public interface ClusterData {

boolean isTlsAllowInsecureConnection();

boolean isTlsHostnameVerificationEnabled();

boolean isBrokerClientTlsEnabledWithKeyStore();

String getBrokerClientTlsTrustStoreType();
Expand Down Expand Up @@ -97,6 +99,8 @@ interface Builder {

Builder tlsAllowInsecureConnection(boolean enabled);

Builder tlsHostnameVerificationEnabled(boolean enabled);

Builder brokerClientTlsEnabledWithKeyStore(boolean enabled);

Builder brokerClientTlsTrustStoreType(String trustStoreType);
Expand Down
7 changes: 7 additions & 0 deletions pulsar-client-tools/src/main/java/org/apache/pulsar/admin/cli/CmdClusters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,10 @@ abstract class ClusterDetailsCommand extends BaseCommand {
@Parameter(names = "--tls-allow-insecure", description = "Allow insecure tls connection", required = false)
protected Boolean tlsAllowInsecureConnection;

@Parameter(names = "--hostname-verification-enabled", description = "Enable hostname verification",
required = false)
protected Boolean tlsHostnameVerificationEnabled;

@Parameter(names = "--tls-enable-keystore",
description = "Whether use KeyStore type to authenticate", required = false)
protected Boolean brokerClientTlsEnabledWithKeyStore;
Expand Down Expand Up @@ -411,6 +415,9 @@ void processArguments() throws Exception {
if (tlsAllowInsecureConnection != null) {
builder.tlsAllowInsecureConnection(tlsAllowInsecureConnection);
}
if (tlsHostnameVerificationEnabled != null) {
builder.tlsHostnameVerificationEnabled(tlsHostnameVerificationEnabled);
}
if (brokerClientTlsEnabledWithKeyStore != null) {
builder.brokerClientTlsEnabledWithKeyStore(brokerClientTlsEnabledWithKeyStore);
}
Expand Down
13 changes: 13 additions & 0 deletions pulsar-common/src/main/java/org/apache/pulsar/common/policies/data/ClusterDataImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ public final class ClusterDataImpl implements ClusterData, Cloneable {
+ " authority."
)
private boolean tlsAllowInsecureConnection;
@ApiModelProperty(
name = "tlsHostnameVerificationEnabled",
value = "Enable TLS hostname verification"
)
private boolean tlsHostnameVerificationEnabled = true;
@ApiModelProperty(
name = "brokerClientTlsEnabledWithKeyStore",
value = "Whether internal client use KeyStore type to authenticate with other Pulsar brokers"
Expand Down Expand Up @@ -203,6 +208,7 @@ public ClusterDataImplBuilder clone() {
.peerClusterNames(peerClusterNames)
.brokerClientTlsEnabled(brokerClientTlsEnabled)
.tlsAllowInsecureConnection(tlsAllowInsecureConnection)
.tlsHostnameVerificationEnabled(tlsHostnameVerificationEnabled)
.brokerClientTlsEnabledWithKeyStore(brokerClientTlsEnabledWithKeyStore)
.brokerClientTlsTrustStoreType(brokerClientTlsTrustStoreType)
.brokerClientTlsTrustStore(brokerClientTlsTrustStore)
Expand Down Expand Up @@ -231,6 +237,7 @@ public static class ClusterDataImplBuilder implements ClusterData.Builder {
private LinkedHashSet<String> peerClusterNames;
private boolean brokerClientTlsEnabled = false;
private boolean tlsAllowInsecureConnection = false;
private boolean tlsHostnameVerificationEnabled = true;
private boolean brokerClientTlsEnabledWithKeyStore = false;
private String brokerClientTlsTrustStoreType = "JKS";
private String brokerClientTlsTrustStore;
Expand Down Expand Up @@ -303,6 +310,11 @@ public ClusterDataImplBuilder tlsAllowInsecureConnection(boolean tlsAllowInsecur
return this;
}

public ClusterDataImplBuilder tlsHostnameVerificationEnabled(boolean tlsHostnameVerificationEnabled) {
this.tlsHostnameVerificationEnabled = tlsHostnameVerificationEnabled;
return this;
}

public ClusterDataImplBuilder brokerClientTlsEnabledWithKeyStore(boolean brokerClientTlsEnabledWithKeyStore) {
this.brokerClientTlsEnabledWithKeyStore = brokerClientTlsEnabledWithKeyStore;
return this;
Expand Down Expand Up @@ -387,6 +399,7 @@ public ClusterDataImpl build() {
peerClusterNames,
brokerClientTlsEnabled,
tlsAllowInsecureConnection,
tlsHostnameVerificationEnabled,
brokerClientTlsEnabledWithKeyStore,
brokerClientTlsTrustStoreType,
brokerClientTlsTrustStore,
Expand Down
1 change: 1 addition & 0 deletions pulsar-common/src/test/java/org/apache/pulsar/common/policies/data/ClusterDataImplTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public void verifyClone() {
.peerClusterNames(new LinkedHashSet<>())
.brokerClientTlsEnabled(true)
.tlsAllowInsecureConnection(false)
.tlsHostnameVerificationEnabled(true)
.brokerClientTlsEnabledWithKeyStore(true)
.brokerClientTlsTrustStoreType("JKS")
.brokerClientTlsTrustStore("/my/trust/store")
Expand Down