From 05021f11a29a0213c5aecddf8e7b907b7834214a Mon Sep 17 00:00:00 2001
From: "James E. Blair" <jeblair@redhat.com>
Date: Tue, 16 Jun 2020 11:15:10 -0700
Subject: [PATCH] Revert "Add Zookeeper TLS support"

This reverts commit 29825ac18b58145f007f64b2998357445b8fdd91.

We observed this issue in production:
https://github.com/python-zk/kazoo/issues/587

Revert until we find a fix.

Change-Id: Ib7b8e3b06770a83b39458d09d2b1e655bd94bd22
---
 inventory/service/group_vars/zookeeper.yaml   |   4 -
 .../library/make_nodepool_zk_hosts.py         |   2 +-
 playbooks/roles/nodepool-base/tasks/main.yaml |  12 --
 playbooks/roles/zk-ca/README.rst              |   4 -
 playbooks/roles/zk-ca/defaults/main.yaml      |   5 -
 playbooks/roles/zk-ca/tasks/main.yaml         |  49 ---------
 playbooks/roles/zk-ca/zk-ca.sh                | 104 ------------------
 .../zookeeper-compose/docker-compose.yaml     |   1 -
 playbooks/roles/zookeeper/tasks/main.yaml     |   8 --
 .../roles/zookeeper/templates/zoo.cfg.j2      |  10 +-
 playbooks/roles/zuul/tasks/main.yaml          |   7 --
 playbooks/roles/zuul/templates/zuul.conf.j2   |   6 +-
 testinfra/test_zookeeper.py                   |   2 +-
 13 files changed, 5 insertions(+), 209 deletions(-)
 delete mode 100644 playbooks/roles/zk-ca/README.rst
 delete mode 100644 playbooks/roles/zk-ca/defaults/main.yaml
 delete mode 100644 playbooks/roles/zk-ca/tasks/main.yaml
 delete mode 100755 playbooks/roles/zk-ca/zk-ca.sh

diff --git a/inventory/service/group_vars/zookeeper.yaml b/inventory/service/group_vars/zookeeper.yaml
index 9477ad2b9b..f74b283b0f 100644
--- a/inventory/service/group_vars/zookeeper.yaml
+++ b/inventory/service/group_vars/zookeeper.yaml
@@ -3,12 +3,8 @@ zookeeper_group: zookeeper
 zookeeper_uid: 10001
 zookeeper_gid: 10001
 iptables_extra_allowed_groups:
-  # Insecure (TODO: remove)
   - {'protocol': 'tcp', 'port': '2181', 'group': 'nodepool'}
   - {'protocol': 'tcp', 'port': '2181', 'group': 'zuul'}
-  # Secure
-  - {'protocol': 'tcp', 'port': '2281', 'group': 'nodepool'}
-  - {'protocol': 'tcp', 'port': '2281', 'group': 'zuul'}
   # Zookeeper election
   - {'protocol': 'tcp', 'port': '2888', 'group': 'zookeeper'}
   # Zookeeper leader
diff --git a/playbooks/roles/nodepool-base/library/make_nodepool_zk_hosts.py b/playbooks/roles/nodepool-base/library/make_nodepool_zk_hosts.py
index 2919c98521..e00803a282 100644
--- a/playbooks/roles/nodepool-base/library/make_nodepool_zk_hosts.py
+++ b/playbooks/roles/nodepool-base/library/make_nodepool_zk_hosts.py
@@ -31,7 +31,7 @@ def main():
         for host in p['zk_group']:
             zk_hosts.append(dict(
                 host=p['hostvars'][host]['ansible_host'],
-                port=2281
+                port=2181
             ))
         module.exit_json(hosts=zk_hosts, changed=True)
     except Exception as e:
diff --git a/playbooks/roles/nodepool-base/tasks/main.yaml b/playbooks/roles/nodepool-base/tasks/main.yaml
index 922832bbde..956a702ee5 100644
--- a/playbooks/roles/nodepool-base/tasks/main.yaml
+++ b/playbooks/roles/nodepool-base/tasks/main.yaml
@@ -26,14 +26,6 @@
     group: '{{ nodepool_group }}'
     mode: 0755
 
-- name: Generate ZooKeeper TLS cert
-  include_role:
-    name: zk-ca
-  vars:
-    zk_ca_cert_dir: /etc/nodepool
-    zk_ca_cert_dir_owner: '{{ nodepool_user }}'
-    zk_ca_cert_dir_group: '{{ nodepool_group }}'
-
 - name: Create nodepool log dir
   file:
     name: /var/log/nodepool
@@ -70,10 +62,6 @@
   vars:
     new_config:
       zookeeper-servers: '{{ zk_hosts.hosts }}'
-      zookeeper-tls:
-        cert: "/etc/nodepool/certs/cert.pem"
-        key: "/etc/nodepool/keys/key.pem"
-        ca: "/etc/nodepool/certs/cacert.pem"
   set_fact:
     nodepool_config: "{{ nodepool_config | combine(new_config) }}"
 
diff --git a/playbooks/roles/zk-ca/README.rst b/playbooks/roles/zk-ca/README.rst
deleted file mode 100644
index e314c2ab20..0000000000
--- a/playbooks/roles/zk-ca/README.rst
+++ /dev/null
@@ -1,4 +0,0 @@
-Generate TLS certs for ZooKeeper
-
-This will copy the certs to the remote node into the /etc/zuul
-directory by default.
diff --git a/playbooks/roles/zk-ca/defaults/main.yaml b/playbooks/roles/zk-ca/defaults/main.yaml
deleted file mode 100644
index db0eb65f72..0000000000
--- a/playbooks/roles/zk-ca/defaults/main.yaml
+++ /dev/null
@@ -1,5 +0,0 @@
-zk_ca_root: /var/zk-ca
-zk_ca_server: "{{ inventory_hostname }}"
-zk_ca_cert_dir: /etc/zuul
-zk_ca_cert_dir_owner: 10001
-zk_ca_cert_dir_group: 10001
diff --git a/playbooks/roles/zk-ca/tasks/main.yaml b/playbooks/roles/zk-ca/tasks/main.yaml
deleted file mode 100644
index f76f82937b..0000000000
--- a/playbooks/roles/zk-ca/tasks/main.yaml
+++ /dev/null
@@ -1,49 +0,0 @@
-- name: Ensure zk-ca directory exists
-  delegate_to: localhost
-  file:
-    path: "{{ zk_ca_root }}"
-    state: directory
-
-# Run this in flock so that we can run it in plays for multiple target
-# hosts in parallel while serializing access to the CA files.
-- name: Run zk-ca.sh
-  delegate_to: localhost
-  script: "zk-ca.sh {{ zk_ca_root }} {{ zk_ca_server }}"
-  args:
-    executable: "flock {{ zk_ca_root }}/lock"
-
-- name: Ensure cert dir exists
-  file:
-    path: "{{ zk_ca_cert_dir }}/certs"
-    state: directory
-    owner: "{{ zk_ca_cert_dir_owner }}"
-    group: "{{ zk_ca_cert_dir_group }}"
-    mode: '0755'
-
-- name: Ensure keys dir exists
-  file:
-    path: "{{ zk_ca_cert_dir }}/keys"
-    state: directory
-    owner: "{{ zk_ca_cert_dir_owner }}"
-    group: "{{ zk_ca_cert_dir_group }}"
-    mode: '0700'
-
-- name: Copy TLS cacert into place
-  copy:
-    src: "/var/zk-ca/certs/cacert.pem"
-    dest: "{{ zk_ca_cert_dir }}/certs/cacert.pem"
-
-- name: Copy TLS cert into place
-  copy:
-    src: "/var/zk-ca/certs/{{ inventory_hostname }}.pem"
-    dest: "{{ zk_ca_cert_dir }}/certs/cert.pem"
-
-- name: Copy TLS key into place
-  copy:
-    src: "/var/zk-ca/keys/{{ inventory_hostname }}key.pem"
-    dest: "{{ zk_ca_cert_dir }}/keys/key.pem"
-
-- name: Copy TLS keystore into place
-  copy:
-    src: "/var/zk-ca/keystores/{{ inventory_hostname }}.pem"
-    dest: "{{ zk_ca_cert_dir }}/keys/keystore.pem"
diff --git a/playbooks/roles/zk-ca/zk-ca.sh b/playbooks/roles/zk-ca/zk-ca.sh
deleted file mode 100755
index 69c393bb02..0000000000
--- a/playbooks/roles/zk-ca/zk-ca.sh
+++ /dev/null
@@ -1,104 +0,0 @@
-#!/bin/sh -e
-
-# Copyright 2020 Red Hat, Inc
-#
-# Licensed under the Apache License, Version 2.0 (the "License"); you may
-# not use this file except in compliance with the License. You may obtain
-# a copy of the License at
-#
-#      http://www.apache.org/licenses/LICENSE-2.0
-#
-# Unless required by applicable law or agreed to in writing, software
-# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
-# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
-# License for the specific language governing permissions and limitations
-# under the License.
-
-# Manage a CA for Zookeeper
-
-CAROOT=$1
-SERVER=$2
-
-SUBJECT='/C=US/ST=California/L=Oakland/O=Company Name/OU=Org'
-TOOLSDIR=$(dirname $0)
-# Unlike the zk-ca.sh in zuul, we use the system openssl.cnf
-CONFIG=""
-
-make_ca() {
-    mkdir $CAROOT/demoCA
-    mkdir $CAROOT/demoCA/reqs
-    mkdir $CAROOT/demoCA/newcerts
-    mkdir $CAROOT/demoCA/crl
-    mkdir $CAROOT/demoCA/private
-    chmod 700 $CAROOT/demoCA/private
-    touch $CAROOT/demoCA/index.txt
-    touch $CAROOT/demoCA/index.txt.attr
-    mkdir $CAROOT/certs
-    mkdir $CAROOT/keys
-    mkdir $CAROOT/keystores
-    chmod 700 $CAROOT/keys
-    chmod 700 $CAROOT/keystores
-
-    openssl req $CONFIG -new -nodes -subj "$SUBJECT/CN=caroot" \
-            -keyout $CAROOT/demoCA/private/cakey.pem \
-            -out $CAROOT/demoCA/reqs/careq.pem
-    openssl ca $CONFIG -create_serial -days 3560 -batch -selfsign -extensions v3_ca \
-            -out $CAROOT/demoCA/cacert.pem \
-            -keyfile $CAROOT/demoCA/private/cakey.pem \
-            -infiles $CAROOT/demoCA/reqs/careq.pem
-    cp $CAROOT/demoCA/cacert.pem $CAROOT/certs
-}
-
-make_client() {
-    openssl req $CONFIG -new -nodes -subj "$SUBJECT/CN=client" \
-            -keyout $CAROOT/keys/clientkey.pem \
-            -out $CAROOT/demoCA/reqs/clientreq.pem
-    openssl ca $CONFIG -batch -policy policy_anything -days 3560 \
-            -out $CAROOT/certs/client.pem \
-            -infiles $CAROOT/demoCA/reqs/clientreq.pem
-}
-
-make_server() {
-    openssl req $CONFIG -new -nodes -subj "$SUBJECT/CN=$SERVER" \
-            -keyout $CAROOT/keys/${SERVER}key.pem \
-            -out $CAROOT/demoCA/reqs/${SERVER}req.pem
-    openssl ca $CONFIG -batch -policy policy_anything -days 3560 \
-            -out $CAROOT/certs/$SERVER.pem \
-            -infiles $CAROOT/demoCA/reqs/${SERVER}req.pem
-    cat $CAROOT/certs/$SERVER.pem $CAROOT/keys/${SERVER}key.pem \
-        > $CAROOT/keystores/$SERVER.pem
-}
-
-help() {
-    echo "$0 CAROOT [SERVER]"
-    echo
-    echo "  CAROOT is the path to a directory in which to store the CA"
-    echo "         and certificates."
-    echo "  SERVER is the FQDN of a server for which a certificate should"
-    echo "         be generated"
-}
-
-if [ ! -d "$CAROOT" ]; then
-    echo "CAROOT must be a directory"
-    help
-    exit 1
-fi
-
-cd $CAROOT
-CAROOT=`pwd`
-
-if [ ! -d "$CAROOT/demoCA" ]; then
-    echo 'Generate CA'
-    make_ca
-    echo 'Generate client certificate'
-    make_client
-fi
-
-if [ -f "$CAROOT/certs/$SERVER.pem" ]; then
-    echo "Certificate for $SERVER already exists"
-    exit 0
-fi
-
-if [ "$SERVER" != "" ]; then
-    make_server
-fi
diff --git a/playbooks/roles/zookeeper/files/zookeeper-compose/docker-compose.yaml b/playbooks/roles/zookeeper/files/zookeeper-compose/docker-compose.yaml
index 96e0ecbe00..ae6aa3a9c4 100644
--- a/playbooks/roles/zookeeper/files/zookeeper-compose/docker-compose.yaml
+++ b/playbooks/roles/zookeeper/files/zookeeper-compose/docker-compose.yaml
@@ -12,4 +12,3 @@ services:
       - "/var/zookeeper/data:/data"
       - "/var/zookeeper/datalog:/datalog"
       - "/var/zookeeper/logs:/logs"
-      - "/var/zookeeper/tls:/tls"
diff --git a/playbooks/roles/zookeeper/tasks/main.yaml b/playbooks/roles/zookeeper/tasks/main.yaml
index 1acc60d29e..10ceaa2dba 100644
--- a/playbooks/roles/zookeeper/tasks/main.yaml
+++ b/playbooks/roles/zookeeper/tasks/main.yaml
@@ -27,14 +27,6 @@
     - data
     - datalog
     - logs
-    - tls
-- name: Generate ZooKeeper TLS cert
-  include_role:
-    name: zk-ca
-  vars:
-    zk_ca_cert_dir: /var/zookeeper/tls
-    zk_ca_cert_dir_owner: 10001
-    zk_ca_cert_dir_group: 10001
 - name: Write config
   template:
     src: zoo.cfg.j2
diff --git a/playbooks/roles/zookeeper/templates/zoo.cfg.j2 b/playbooks/roles/zookeeper/templates/zoo.cfg.j2
index 514b35cc26..63dff1b82a 100644
--- a/playbooks/roles/zookeeper/templates/zoo.cfg.j2
+++ b/playbooks/roles/zookeeper/templates/zoo.cfg.j2
@@ -22,13 +22,7 @@ autopurge.purgeInterval=6
 maxClientCnxns=60
 standaloneEnabled=true
 admin.enableServer=true
-secureClientPort=2281
-ssl.keyStore.location=/tls/keys/keystore.pem
-ssl.trustStore.location=/tls/certs/cacert.pem
+clientPort=2181
 {% for host in groups['zookeeper'] %}
-server.{{ loop.index }}={{ (hostvars[host].public_v4) }}:2888:3888
+server.{{ loop.index }}={{ (hostvars[host].ansible_default_ipv4.address) }}:2888:3888
 {% endfor %}
-sslQuorum=true
-serverCnxnFactory=org.apache.zookeeper.server.NettyServerCnxnFactory
-ssl.quorum.keyStore.location=/tls/keys/keystore.pem
-ssl.quorum.trustStore.location=/tls/certs/cacert.pem
diff --git a/playbooks/roles/zuul/tasks/main.yaml b/playbooks/roles/zuul/tasks/main.yaml
index 1451caff02..4c1738a18c 100644
--- a/playbooks/roles/zuul/tasks/main.yaml
+++ b/playbooks/roles/zuul/tasks/main.yaml
@@ -21,13 +21,6 @@
     owner: "{{ zuul_user }}"
     group: "{{ zuul_group }}"
 
-- name: Generate ZooKeeper TLS cert
-  include_role:
-    name: zk-ca
-  vars:
-    zk_ca_cert_dir_owner: "{{ zuul_user_id }}"
-    zk_ca_cert_dir_group: "{{ zuul_group_id }}"
-
 - name: Create Zuul SSL dir
   file:
     state: directory
diff --git a/playbooks/roles/zuul/templates/zuul.conf.j2 b/playbooks/roles/zuul/templates/zuul.conf.j2
index ff86b24a12..eb8d512cb6 100644
--- a/playbooks/roles/zuul/templates/zuul.conf.j2
+++ b/playbooks/roles/zuul/templates/zuul.conf.j2
@@ -28,11 +28,7 @@ relative_priority=true
 user=zuul
 
 [zookeeper]
-hosts={% for host in groups['zookeeper'] %}{{ (hostvars[host].public_v4) }}:2281{% if not loop.last %},{% endif %}{% endfor %}
-
-tls_cert=/etc/zuul/certs/cert.pem
-tls_key=/etc/zuul/keys/key.pem
-tls_ca=/etc/zuul/certs/cacert.pem
+hosts={% for host in groups['zookeeper'] %}{{ (hostvars[host].ansible_default_ipv4.address) }}{% if not loop.last %},{% endif %}{% endfor %}
 
 session_timeout=40
 
diff --git a/testinfra/test_zookeeper.py b/testinfra/test_zookeeper.py
index 2770b38c49..0890bfaa46 100644
--- a/testinfra/test_zookeeper.py
+++ b/testinfra/test_zookeeper.py
@@ -22,5 +22,5 @@ def test_id_file(host):
     assert myid.content == b'1\n'
 
 def test_zk_listening(host):
-    zk = host.socket("tcp://0.0.0.0:2281")
+    zk = host.socket("tcp://0.0.0.0:2181")
     assert zk.is_listening