From ac6ec9f99a7e6c5328b93c8a0697d929d5512ae1 Mon Sep 17 00:00:00 2001
From: Dmitry Tantsur <dtantsur@protonmail.com>
Date: Tue, 1 Sep 2020 15:25:49 +0200
Subject: [PATCH] Create our own firewalld zone and use it on real bare metal

Modifying the public zone is questionable, let's use our own zone.
Also let's make sure network_interface actually belongs to it.

Conflicts:
	playbooks/roles/bifrost-ironic-install/tasks/bootstrap.yml

Change-Id: I63f5fa4845aa8f1c90a0c73dd78deb45aaaa4fd1
(cherry picked from commit 710e0db06885b13b398d6fe209893cc732a43d41)
---
 doc/source/install/index.rst                  |  7 +++
 .../bifrost-ironic-install/defaults/main.yml  |  2 +
 .../tasks/bootstrap.yml                       | 23 ++-------
 .../tasks/inspector_bootstrap.yml             |  2 +-
 .../bifrost-ironic-install/tasks/install.yml  |  4 ++
 .../tasks/setup_firewalld.yml                 | 50 +++++++++++++++++++
 .../firewalld-zone-d8c72fb5924a4916.yaml      | 11 ++++
 7 files changed, 79 insertions(+), 20 deletions(-)
 create mode 100644 playbooks/roles/bifrost-ironic-install/tasks/setup_firewalld.yml
 create mode 100644 releasenotes/notes/firewalld-zone-d8c72fb5924a4916.yaml

diff --git a/doc/source/install/index.rst b/doc/source/install/index.rst
index bd289670f..a01267a99 100644
--- a/doc/source/install/index.rst
+++ b/doc/source/install/index.rst
@@ -62,8 +62,15 @@ For the machine that hosts Bifrost you'll need to figure out:
 
 * The network interface you're going to use for communication between the bare
   metal machines and the Bifrost services.
+
+  On systems using firewalld (CentOS and RHEL currently), a new zone
+  ``bifrost`` will be created, and the network interface will be moved to it.
+  DHCP, PXE and API services will only be added to this zone. If you need any
+  of them available in other zones, you need to configure firewall yourself.
+
 * Pool of IP addresses for DHCP (must be within the network configured on the
   chosen network interface).
+
 * Whether you want the services to use authentication via Keystone_.
 
 For each machine that is going to be enrolled in the Bare Metal service you'll
diff --git a/playbooks/roles/bifrost-ironic-install/defaults/main.yml b/playbooks/roles/bifrost-ironic-install/defaults/main.yml
index ed0b8047a..4693b5289 100644
--- a/playbooks/roles/bifrost-ironic-install/defaults/main.yml
+++ b/playbooks/roles/bifrost-ironic-install/defaults/main.yml
@@ -49,6 +49,8 @@ network_interface: "virbr0"
 ans_network_interface: "{{ network_interface | replace('-', '_') }}"
 internal_interface: "{{ hostvars[inventory_hostname]['ansible_' + ans_network_interface]['ipv4'] }}"
 internal_ip: "{{ internal_interface['address'] }}"
+# Our own firewalld zone, only applies when testing is false.
+firewalld_internal_zone: bifrost
 
 # Normally this would setting would be http in a bifrost installation
 # without TLS. This setting allows a user to override the setting in case
diff --git a/playbooks/roles/bifrost-ironic-install/tasks/bootstrap.yml b/playbooks/roles/bifrost-ironic-install/tasks/bootstrap.yml
index 26f98becb..313772c14 100644
--- a/playbooks/roles/bifrost-ironic-install/tasks/bootstrap.yml
+++ b/playbooks/roles/bifrost-ironic-install/tasks/bootstrap.yml
@@ -27,23 +27,8 @@
     enable_venv: true
   when: lookup('env', 'VENV') | length > 0
 
-- block:
-    - name: "Ask systemd to reload configuration"
-      systemd:
-        daemon_reload: yes
-
-    - name: "Enable firewalld"
-      service:
-        name: firewalld
-        state: started
-        enabled: yes
-
-    - name: "Disable iptables (if enabled)"
-      service:
-        name: iptables
-        state: stopped
-        enabled: no
-      ignore_errors: true
+- name: "Setup firewalld"
+  include_tasks: setup_firewalld.yml
   when: ansible_distribution in ["CentOS", "RedHat"]
 
 # NOTE(sean-k-mooney) only the RabbitMQ server and MySQL db are started
@@ -358,7 +343,7 @@
 - name: "Enable services in firewalld"
   firewalld:
     service: "{{ item }}"
-    zone: "{{ 'libvirt' if testing | bool else 'public' }}"
+    zone: "{{ 'libvirt' if testing | bool else firewalld_internal_zone }}"
     state: enabled
     permanent: yes
     immediate: yes
@@ -371,7 +356,7 @@
 - name: "Enable ports in firewalld"
   firewalld:
     port: "{{ item }}/tcp"
-    zone: "{{ 'libvirt' if testing | bool else 'public' }}"
+    zone: "{{ 'libvirt' if testing | bool else firewalld_internal_zone }}"
     state: enabled
     permanent: yes
     immediate: yes
diff --git a/playbooks/roles/bifrost-ironic-install/tasks/inspector_bootstrap.yml b/playbooks/roles/bifrost-ironic-install/tasks/inspector_bootstrap.yml
index 0a3fa5a62..984265546 100644
--- a/playbooks/roles/bifrost-ironic-install/tasks/inspector_bootstrap.yml
+++ b/playbooks/roles/bifrost-ironic-install/tasks/inspector_bootstrap.yml
@@ -132,7 +132,7 @@
 - name: "Inspector - Enable port in firewalld"
   firewalld:
     port: "5050/tcp"
-    zone: "{{ 'libvirt' if testing else 'public' }}"
+    zone: "{{ 'libvirt' if testing | bool else firewalld_internal_zone }}"
     state: enabled
     permanent: yes
     immediate: yes
diff --git a/playbooks/roles/bifrost-ironic-install/tasks/install.yml b/playbooks/roles/bifrost-ironic-install/tasks/install.yml
index c508efde2..d37f9fbd5 100644
--- a/playbooks/roles/bifrost-ironic-install/tasks/install.yml
+++ b/playbooks/roles/bifrost-ironic-install/tasks/install.yml
@@ -22,6 +22,10 @@
     name: "{{ required_packages }}"
     state: present
 
+- name: "Ask systemd to reload configuration"
+  systemd:
+    daemon_reload: yes
+
 # NOTE(TheJulia) While we don't necessarilly require /opt/stack any longer
 # and it should already be created by the Ansible setup, we will leave this
 # here for the time being.
diff --git a/playbooks/roles/bifrost-ironic-install/tasks/setup_firewalld.yml b/playbooks/roles/bifrost-ironic-install/tasks/setup_firewalld.yml
new file mode 100644
index 000000000..3c960eda0
--- /dev/null
+++ b/playbooks/roles/bifrost-ironic-install/tasks/setup_firewalld.yml
@@ -0,0 +1,50 @@
+# 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.
+---
+- name: "Enable firewalld"
+  service:
+    name: firewalld
+    state: started
+    enabled: yes
+
+- name: "Disable iptables (if enabled)"
+  service:
+    name: iptables
+    state: stopped
+    enabled: no
+  ignore_errors: true
+
+- name: "Create a firewalld zone"
+  firewalld:
+    zone: "{{ firewalld_internal_zone }}"
+    state: present
+    permanent: yes
+  register: new_zone_result
+  when: not testing | bool
+
+- name: "Reload firewalld if needed"
+  service:
+    name: firewalld
+    state: reloaded
+  when:
+    - new_zone_result is defined
+    - new_zone_result.changed
+
+- name: "Add the network interface to the new zone"
+  firewalld:
+    zone: "{{ firewalld_internal_zone }}"
+    interface: "{{ network_interface }}"
+    state: enabled
+    permanent: yes
+    immediate: yes
+  when: not testing | bool
diff --git a/releasenotes/notes/firewalld-zone-d8c72fb5924a4916.yaml b/releasenotes/notes/firewalld-zone-d8c72fb5924a4916.yaml
new file mode 100644
index 000000000..48d445c47
--- /dev/null
+++ b/releasenotes/notes/firewalld-zone-d8c72fb5924a4916.yaml
@@ -0,0 +1,11 @@
+---
+fixes:
+  - |
+    Instead of modifying the ``public`` firewalld zone, creates a new zone
+    ``bifrost`` and puts the ``network_interface`` in it. Set
+    ``firewalld_internal_zone=public`` to revert to the previous behavior.
+upgrade:
+  - |
+    Bifrost no longer adds ironic and ironic-inspector endpoints to the public
+    firewalld zone, the operator has to do it explicitly if external access
+    is expected.