Skip to content

Commit 8837201

Browse files
committed
remove client ports, adapt readiness probe & tests
1 parent 44261e9 commit 8837201

File tree

3 files changed

+18
-53
lines changed

3 files changed

+18
-53
lines changed

rust/operator-binary/src/crd/security.rs

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -65,12 +65,11 @@ pub struct ZookeeperSecurity {
6565
impl ZookeeperSecurity {
6666
// ports
6767
pub const CLIENT_PORT: u16 = 2181;
68-
pub const CLIENT_PORT_NAME: &'static str = "clientPort";
68+
pub const SECURE_CLIENT_PORT: u16 = 2282;
69+
pub const ADMIN_PORT: u16 = 8080;
6970
// directories
7071
pub const QUORUM_TLS_DIR: &'static str = "/stackable/quorum_tls";
7172
pub const QUORUM_TLS_MOUNT_DIR: &'static str = "/stackable/quorum_tls_mount";
72-
pub const SECURE_CLIENT_PORT: u16 = 2282;
73-
pub const SECURE_CLIENT_PORT_NAME: &'static str = "secureClientPort";
7473
pub const SERVER_CNXN_FACTORY: &'static str = "serverCnxnFactory";
7574
pub const SERVER_TLS_DIR: &'static str = "/stackable/server_tls";
7675
pub const SERVER_TLS_MOUNT_DIR: &'static str = "/stackable/server_tls_mount";
@@ -220,42 +219,6 @@ impl ZookeeperSecurity {
220219

221220
// Server TLS
222221
if self.tls_enabled() {
223-
// We set only the clientPort and portUnification here because otherwise there is a port bind exception
224-
// See: https://issues.apache.org/jira/browse/ZOOKEEPER-4276
225-
// --> Normally we would like to only set the secureClientPort (check out commented code below)
226-
// What we tried:
227-
// 1) Set clientPort and secureClientPort will fail with
228-
// "static.config different from dynamic config .. "
229-
// config.insert(
230-
// Self::CLIENT_PORT_NAME.to_string(),
231-
// CLIENT_PORT.to_string(),
232-
// );
233-
// config.insert(
234-
// Self::SECURE_CLIENT_PORT_NAME.to_string(),
235-
// SECURE_CLIENT_PORT.to_string(),
236-
// );
237-
238-
// 2) Setting only secureClientPort will config in the above mentioned bind exception.
239-
// The NettyFactory tries to bind multiple times on the secureClientPort.
240-
// config.insert(
241-
// Self::SECURE_CLIENT_PORT_NAME.to_string(),
242-
// self.client_port(.to_string()),
243-
// );
244-
245-
// 3) Using the clientPort and portUnification still allows plaintext connection without
246-
// authentication, but at least TLS and authentication works when connecting securely.
247-
config.insert(
248-
Self::CLIENT_PORT_NAME.to_string(),
249-
self.client_port().to_string(),
250-
);
251-
config.insert("client.portUnification".to_string(), "true".to_string());
252-
// TODO: Remove clientPort and portUnification (above) in favor of secureClientPort once the bug is fixed
253-
// config.insert(
254-
// Self::SECURE_CLIENT_PORT_NAME.to_string(),
255-
// self.client_port(.to_string()),
256-
// );
257-
// END TICKET
258-
259222
config.insert(
260223
Self::SSL_HOST_NAME_VERIFICATION.to_string(),
261224
"true".to_string(),
@@ -278,11 +241,6 @@ impl ZookeeperSecurity {
278241
{
279242
config.insert(Self::SSL_CLIENT_AUTH.to_string(), "need".to_string());
280243
}
281-
} else {
282-
config.insert(
283-
Self::CLIENT_PORT_NAME.to_string(),
284-
self.client_port().to_string(),
285-
);
286244
}
287245

288246
config

rust/operator-binary/src/zk_controller.rs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -573,12 +573,20 @@ fn build_server_rolegroup_config_map(
573573
.into_iter()
574574
.flatten()
575575
.map(|pod| {
576+
let port = if zookeeper_security.tls_enabled() {
577+
// The docs state for TLS-only server
578+
// server.5 = <host>:<leader_port>:<election_port>;;<secureClientPort> (TLS-only server)
579+
format!(";{port}", port = zookeeper_security.client_port())
580+
} else {
581+
// The docs state for non-TLS server
582+
// server.5 = <host>:<leader_port>:<election_port>;<clientPort> (TLS-only server)
583+
format!("{port}", port = zookeeper_security.client_port())
584+
};
576585
(
577586
format!("server.{id}", id = pod.zookeeper_myid),
578587
format!(
579-
"{internal_fqdn}:{ZOOKEEPER_LEADER_PORT}:{ZOOKEEPER_ELECTION_PORT};{client_port}",
588+
"{internal_fqdn}:{ZOOKEEPER_LEADER_PORT}:{ZOOKEEPER_ELECTION_PORT};{port}",
580589
internal_fqdn = pod.internal_fqdn(cluster_info),
581-
client_port = zookeeper_security.client_port()
582590
),
583591
)
584592
})
@@ -859,11 +867,10 @@ fn build_server_rolegroup_statefulset(
859867
command: Some(vec![
860868
"bash".to_string(),
861869
"-c".to_string(),
862-
// We don't have telnet or netcat in the container images, but
863-
// we can use Bash's virtual /dev/tcp filesystem to accomplish the same thing
870+
// NOTE: the adminPort property is currently generated in the product config machinery properties.yaml.
864871
format!(
865-
"exec 3<>/dev/tcp/127.0.0.1/{} && echo srvr >&3 && grep '^Mode: ' <&3",
866-
zookeeper_security.client_port()
872+
r#"curl -s http://127.0.0.1:{admin_port}/commands/mntr | grep -q '"server_state"[ ]*:[ ]*"\(leader\|follower\)"'"#,
873+
admin_port = ZookeeperSecurity::ADMIN_PORT
867874
),
868875
]),
869876
}),

tests/templates/kuttl/smoke/test_tls.sh.j2

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,12 @@ echo "Start TLS testing..."
1818
############################################################################
1919
# Test the plaintext unsecured connection
2020
############################################################################
21-
if ! /stackable/zookeeper/bin/zkCli.sh -server "${SERVER}" ls / &> /dev/null;
21+
if /stackable/zookeeper/bin/zkCli.sh -server "${SERVER}" ls / &> /dev/null;
2222
then
23-
echo "[ERROR] Could not establish unsecure connection!"
23+
echo "[ERROR] Could establish unsecure connection!"
2424
exit 1
2525
fi
26-
echo "[SUCCESS] Unsecure client connection established!"
26+
echo "[SUCCESS] Could not establish unsecure connection!"
2727

2828
############################################################################
2929
# We set the correct client tls credentials and expect to be able to connect

0 commit comments

Comments
 (0)