From d3928d1be0a915134f873c6ecea333ce23034dd9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javier=20B=C3=A9rtoli?= <javier@netmanagers.com.ar>
Date: Sat, 25 Aug 2018 18:27:37 -0300
Subject: [PATCH] Refactor ipset format, add backward compatibility

See https://github.com/saltstack-formulas/firewalld-formula/pull/21#pullrequestreview-146958098
---
 firewalld/defaults.yaml | 13 +++++++++++--
 firewalld/ipsets.sls    | 41 ++++++++++++++++++++++++++++++++++++-----
 pillar.example          |  6 ++++--
 3 files changed, 51 insertions(+), 9 deletions(-)

diff --git a/firewalld/defaults.yaml b/firewalld/defaults.yaml
index 3dfe739..73b7e7f 100644
--- a/firewalld/defaults.yaml
+++ b/firewalld/defaults.yaml
@@ -1,8 +1,17 @@
 # -*- coding: utf-8 -*-
 # vim: ft=yaml
 firewalld:
+  enabled: true
   package: firewalld
-  ipsetpackage: ipset
-  backendpackage: nftables
   service: firewalld
   config: /etc/firewalld.conf
+
+  ipset:
+    manage: true
+    pkg: ipset
+
+  backend:
+    manage: true
+    pkg: nftables
+
+  ipsets: {}
diff --git a/firewalld/ipsets.sls b/firewalld/ipsets.sls
index 1fba144..f16622d 100644
--- a/firewalld/ipsets.sls
+++ b/firewalld/ipsets.sls
@@ -4,10 +4,41 @@
 #
 {% from "firewalld/map.jinja" import firewalld with context %}
 
-{%- if salt['pillar.get']('firewalld:ipset') %}
+# Backward compatibility setting and deprecation notices
+{% set ipset_manage = false %}
+{% set ipset_pkg = 'ipset' %}
+{% set ipset_sets = firewalld.ipsets %}
+
+{% if firewalld.ipset is mapping %}
+  {% set ipset_manage = firewalld.ipset.manage %}
+  {% set ipset_pkg = firewalld.ipset.pkg %}
+{% else %}
+### Manage setting (old firewalld:ipset)
+firewalld-ipset-deprecated:
+  test.show_notification:
+    - text: |
+        'firewalld:ipset' format has changed and setting it as boolean is deprecated.
+        Set 'firewalld:ipset:manage' instead.
+        See firewalld/pillar.example for more information
+
+  {% set ipset_manage = firewalld.ipset %}
+{% endif %}
+
+### Package setting (old firewalld:ipsetpackage)
+{% if firewalld.ipsetpackage is defined %}
+firewalld-ipsetpackage-deprecated:
+  test.show_notification:
+    - text: |
+        'firewalld:ipsetpackage' is deprecated. Use 'firewalld:ipset:pkg' instead
+        See firewalld/pillar.example for more information
+
+  {% set ipset_pkg = firewalld.ipsetpackage %}
+{% endif %}
+
+{%- if ipset_manage %}
 package_ipset:
   pkg.installed:
-    - name: {{ firewalld.ipsetpackage }}
+    - name: {{ ipset_pkg }}
 
 directory_firewalld_ipsets:
   file.directory:            # make sure this is a directory
@@ -26,8 +57,8 @@ directory_firewalld_ipsets:
 #
 # This defines a ipset configuration, see firewalld.ipset (5) man page.
 #
-{% for k, v in salt['pillar.get']('firewalld:ipsets', {}).items() %}
-{% set z_name = v.name|default(k) %}
+  {% for k, v in ipset_sets.items() %}
+  {% set z_name = v.name|default(k) %}
 
 /etc/firewalld/ipsets/{{ z_name }}.xml:
   file.managed:
@@ -48,5 +79,5 @@ directory_firewalld_ipsets:
         name: {{ z_name }}
         ipset: {{ v }}
 
-{% endfor %}
+  {% endfor %}
 {%- endif %}
diff --git a/pillar.example b/pillar.example
index 9979838..2abd4db 100644
--- a/pillar.example
+++ b/pillar.example
@@ -1,7 +1,10 @@
 # FirewallD pillar examples:
 firewalld:
   enabled: True
-  ipset: True
+  ipset:
+    manage: True
+    pkg: ipset
+
   installbackend: False
   default_zone: public
 
@@ -51,7 +54,6 @@ firewalld:
       entries:
         - 10.0.0.1
 
-
   zones:
     public:
       short: Public