Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable query logging default #128

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Disable query logging default #128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
633cbb3
Disable query logging default
ixs May 3, 2019
5685a5e
Fix missing logging_config for Arch, RedHat and SuSE.
ixs May 3, 2019
1ce4378
Deprecation check and test fixup
ixs May 3, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions bind/config.sls
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
5 changes: 2 additions & 3 deletions bind/files/named.conf.local.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,10 +132,10 @@ view {{ view }} {
};
{%- endfor %}

{%- if salt['pillar.get']('bind:config:enable_logging', True) %}
{%- if salt['pillar.get']('bind:config:use_extensive_logging', False) %}
include "{{ map.logging_config }}";
{% else %}
{#- 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";
Expand All @@ -144,7 +144,6 @@ logging {
category queries { querylog; };
};
{%- endif %}
{%- endif %}

{%- if salt['pillar.get']('bind:controls', False) %}
controls {
Expand Down
3 changes: 3 additions & 0 deletions bind/map.jinja
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
'zones_source_dir': 'zones',
'config': '/etc/named.conf',
'local_config': '/etc/named.conf.local',
'logging_config': '/etc/named/logging.conf',
'default_config': '/etc/sysconfig/named',
'named_directory': '/var/named',
'chroot_dir': '',
Expand Down Expand Up @@ -63,6 +64,7 @@
'zones_source_dir': 'zones',
'config': '/etc/named.conf',
'local_config': '/etc/named.conf.local',
'logging_config': '/etc/named.conf.logging',
'named_directory': '/var/named',
'chroot_dir': '',
'log_dir': '/var/log/named',
Expand Down Expand Up @@ -102,6 +104,7 @@
'zones_source_dir': 'zones',
'config': '/etc/named.conf',
'local_config': '/etc/named.d/named.conf.local',
'logging_config': '/etc/named.d/logging.conf',
'default_config': '/etc/sysconfig/named',
'named_directory': '/var/lib/named',
'chroot_dir': '/var/lib/named',
Expand Down
8 changes: 4 additions & 4 deletions pillar.example
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ bind:
user: root # File & Directory user
group: named # File & Directory group
mode: 640 # File & Directory mode
enable_logging: true # Enable basic query logging
use_extensive_logging: # Enable extensive config for logging. Partial example. For proposed settings please refer to
channel: # https://kb.isc.org/article/AA-01526/0/BIND-Logging-some-basic-recommendations.html
default_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
file: default
size: '200m' # size of a individual file (default 20m)
versions: '10' # how many files will be stored (default 3)
Expand Down
2 changes: 1 addition & 1 deletion test/integration/default/config_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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\};/ }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you're moving the logging config to another file, can you either add some tests to make sure the logging_config file/s are created properly, or open an issue so we don't forget to add tests for this? Perhaps I can find some time to add them

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Let me rework this a bit...

# Match acl1
its('content') { should match /acl\ client1\ \{\n\ \ 127\.0\.0\.0\/8;\n\ \ 10\.20\.0\.0\/16;\n\};/ }
# Match acl2
Expand Down