From 7e2652a1e242a5c6578bfa2d60af31f383569219 Mon Sep 17 00:00:00 2001
From: Andreas Thienemann <andreas@bawue.net>
Date: Sat, 4 May 2019 01:55:47 +0200
Subject: [PATCH] Deprecation check and test fixup

Add a deprecation message if enable_logging is used instead of enable_query_log.
Abort if both enable_logging/enable_query_log is enabled with use_extensive_logging
as these are mutually exclusive now.

Fixup tests to _not_ look for the querylog line.
---
 bind/config.sls                         | 12 ++++++++++++
 bind/files/named.conf.local.jinja       |  3 ++-
 pillar.example                          |  2 +-
 test/integration/default/config_spec.rb |  2 +-
 4 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/bind/config.sls b/bind/config.sls
index e351d0b6..d4d60b1b 100644
--- a/bind/config.sls
+++ b/bind/config.sls
@@ -83,6 +83,18 @@ bind_config:
     - watch_in:
       - service: bind
 
+{%- if salt['pillar.get']('bind:config:enable_logging') is not none %}
+bind_local_config_deprecated_logging:
+  test.show_notification:
+    - text: Pillar data contains enable_logging. This parameter is deprecated and has been renamed enable_query_log.
+{%- endif %}
+
+{%- if (salt['pillar.get']('bind:config:enable_logging') or salt['pillar.get']('bind:config:enable_query_log')) and salt['pillar.get']('bind:config:use_extensive_logging') %}
+bind_local_config_logging_extensive_fail:
+  test.fail_without_changes:
+    - name: Pillar data uses enable_logging/enable_query_log and use_extensive_logging. These are mutually exclusive.
+{%- endif %}
+
 bind_local_config:
   file.managed:
     - name: {{ map.local_config }}
diff --git a/bind/files/named.conf.local.jinja b/bind/files/named.conf.local.jinja
index ddac4239..da936681 100644
--- a/bind/files/named.conf.local.jinja
+++ b/bind/files/named.conf.local.jinja
@@ -134,7 +134,8 @@ view {{ view }} {
 
 {%- if salt['pillar.get']('bind:config:use_extensive_logging', False) %}
 include "{{ map.logging_config }}";
-{% elif salt['pillar.get']('bind:config:enable_logging', False) %}
+{#- bind:config:enable_logging is deprecated. TODO: Remove the or part of the condition below after a while. #}
+{% elif salt['pillar.get']('bind:config:enable_query_log', False) or salt['pillar.get']('bind:config:enable_logging', False) %}
 logging {
   channel "querylog" {
     file "{{ map.log_dir }}/query.log";
diff --git a/pillar.example b/pillar.example
index 08f6374a..34dd0a42 100644
--- a/pillar.example
+++ b/pillar.example
@@ -28,7 +28,7 @@ bind:
     user: root                                    # File & Directory user
     group: named                                  # File & Directory group
     mode: 640                                     # File & Directory mode
-    enable_logging: true                          # Enable basic query logging in $log_dir/query.log
+    enable_query_log: true                        # Enable basic query logging in $log_dir/query.log
     use_extensive_logging:                        # Alternatively, enable much more extensive config for logging.
       channel:                                    # Partial example. For proposed settings please refer to
         default_log:                              # https://kb.isc.org/article/AA-01526/0/BIND-Logging-some-basic-recommendations.html
diff --git a/test/integration/default/config_spec.rb b/test/integration/default/config_spec.rb
index 1751621e..0d192c0d 100644
--- a/test/integration/default/config_spec.rb
+++ b/test/integration/default/config_spec.rb
@@ -147,7 +147,7 @@
     # Match 100.51.198 reverse zone from pillar
     its('content') { should match /^zone\ "100\.51\.198\.in-addr\.arpa"\ {\n\ \ type\ master;\n\ \ file\ "#{zones_directory}\/100\.51\.198\.in-addr\.arpa";\n\ \ \n\ \ notify\ no;\n\};/ }
     # Match logging
-    its('content') { should match /^logging\ \{\n\ \ channel\ "querylog"\ {\n\ \ \ \ file\ "#{log_directory}\/query\.log";\n\ \ \ \ print-time\ yes;\n\ \ \};\n\ \ category\ queries\ \{\ querylog;\ \};\n\};/ }
+    its('content') { should not match /^logging\ \{\n\ \ channel\ "querylog"\ {\n\ \ \ \ file\ "#{log_directory}\/query\.log";\n\ \ \ \ print-time\ yes;\n\ \ \};\n\ \ category\ queries\ \{\ querylog;\ \};\n\};/ }
     # Match acl1
     its('content') { should match /acl\ client1\ \{\n\ \ 127\.0\.0\.0\/8;\n\ \ 10\.20\.0\.0\/16;\n\};/ }
     # Match acl2