From 94d2b0b97c242174c6f1c08cb2da2d2d03d98bd4 Mon Sep 17 00:00:00 2001 From: Steven Daniele Date: Thu, 9 Apr 2020 10:52:33 -0400 Subject: [PATCH 1/3] fix: do not error on unknown os_family grain If the formula was ran on a system that reported an os_family that wasn't one of "Debian", "RedHat", "Arch", "Suse" then the map.jinja template would fail to render with "'NoneType' is not iterable. This occurs because grains.filter_by will return None when it fails match the grain to the input dictionary. The value is then blindly passed into a dict.update() which causes the failure. In this patch we ensure that the default values, as defined in defaults.yaml, are always applied when grain matching fails. --- firewalld/defaults.yaml | 25 +++++++++++++------------ firewalld/map.jinja | 23 ++++++++--------------- firewalld/osfamilymap.yaml | 13 +++++++++++++ 3 files changed, 34 insertions(+), 27 deletions(-) create mode 100644 firewalld/osfamilymap.yaml diff --git a/firewalld/defaults.yaml b/firewalld/defaults.yaml index bbabd6e..5c00c0d 100644 --- a/firewalld/defaults.yaml +++ b/firewalld/defaults.yaml @@ -1,18 +1,19 @@ # -*- coding: utf-8 -*- # vim: ft=yaml --- -firewalld: - enabled: true - package: firewalld - service: firewalld - config: /etc/firewalld.conf +default: + firewalld: + enabled: true + package: firewalld + service: firewalld + config: /etc/firewalld.conf - ipset: - manage: false - pkg: ipset + ipset: + manage: false + pkg: ipset - backend: - manage: false - pkg: nftables + backend: + manage: false + pkg: nftables - ipsets: {} + ipsets: {} diff --git a/firewalld/map.jinja b/firewalld/map.jinja index 8a8a394..4b952f6 100644 --- a/firewalld/map.jinja +++ b/firewalld/map.jinja @@ -3,25 +3,18 @@ {#- Start with defaults from defaults.yaml #} {% import_yaml "firewalld/defaults.yaml" as default_settings %} +{% import_yaml "firewalld/osfamilymap.yaml" as osfamilymap %} -{#- -Setup variable using grains['os_family'] based logic, only add key:values here -that differ from whats in defaults.yaml -#} -{% set os_family_map = salt['grains.filter_by']({ - 'Debian': {}, - 'RedHat': {}, - 'Arch': {}, - 'Suse': {}, - }, grain='os_family', merge=salt['pillar.get']('firewalld:lookup')) -%} - -{#- Merge the flavor_map to the default settings #} -{% do default_settings.firewalld.update(os_family_map) %} +{% set platform_defaults = salt['grains.filter_by'](default_settings, + default='default', + merge=salt['grains.filter_by'](osfamilymap, grain='os_family', + merge=salt['pillar.get']('firewalld:lookup') + ) +) %} {#- Merge in salt:lookup pillar #} {% set firewalld = salt['pillar.get']( 'firewalld', - default=default_settings.firewalld, + default=platform_defaults.firewalld, merge=True) %} diff --git a/firewalld/osfamilymap.yaml b/firewalld/osfamilymap.yaml new file mode 100644 index 0000000..810a01c --- /dev/null +++ b/firewalld/osfamilymap.yaml @@ -0,0 +1,13 @@ +# -*- coding: utf-8 -*- +# # vim: ft=yaml +# os_family defaults +# only add key:values here that differ from whats in defaults.yaml +--- +Debian: + firewalld: {} +RedHat: + firewalld: {} +Arch: + firewalld: {} +Suse: + firewalld: {} From afcf5e770085565b11c25e9af522b194bd67fc30 Mon Sep 17 00:00:00 2001 From: Steven Daniele Date: Mon, 13 Apr 2020 12:14:32 -0400 Subject: [PATCH 2/3] refactor: split default maps into separate files While the default maps are mostly empty this sets the groundwork for distribution specific defaults. The layout is based on the formula template. BREAKING CHANGE: `map.jinja` has been upgraded from using `pillar.get` to `config.get`. --- firewalld/defaults.yaml | 25 ++++++++++----------- firewalld/map.jinja | 30 ++++++++++++++++--------- firewalld/osarchmap.yaml | 35 +++++++++++++++++++++++++++++ firewalld/osfamilymap.yaml | 42 +++++++++++++++++++++++++---------- firewalld/osfingermap.yaml | 45 ++++++++++++++++++++++++++++++++++++++ firewalld/osmap.yaml | 33 ++++++++++++++++++++++++++++ 6 files changed, 176 insertions(+), 34 deletions(-) create mode 100644 firewalld/osarchmap.yaml create mode 100644 firewalld/osfingermap.yaml create mode 100644 firewalld/osmap.yaml diff --git a/firewalld/defaults.yaml b/firewalld/defaults.yaml index 5c00c0d..bbabd6e 100644 --- a/firewalld/defaults.yaml +++ b/firewalld/defaults.yaml @@ -1,19 +1,18 @@ # -*- coding: utf-8 -*- # vim: ft=yaml --- -default: - firewalld: - enabled: true - package: firewalld - service: firewalld - config: /etc/firewalld.conf +firewalld: + enabled: true + package: firewalld + service: firewalld + config: /etc/firewalld.conf - ipset: - manage: false - pkg: ipset + ipset: + manage: false + pkg: ipset - backend: - manage: false - pkg: nftables + backend: + manage: false + pkg: nftables - ipsets: {} + ipsets: {} diff --git a/firewalld/map.jinja b/firewalld/map.jinja index 4b952f6..f25fb36 100644 --- a/firewalld/map.jinja +++ b/firewalld/map.jinja @@ -3,18 +3,28 @@ {#- Start with defaults from defaults.yaml #} {% import_yaml "firewalld/defaults.yaml" as default_settings %} +{% import_yaml "firewalld/osarchmap.yaml" as osarchmap %} {% import_yaml "firewalld/osfamilymap.yaml" as osfamilymap %} +{% import_yaml "firewalld/osmap.yaml" as osmap %} +{% import_yaml "firewalld/osfingermap.yaml" as osfingermap %} -{% set platform_defaults = salt['grains.filter_by'](default_settings, - default='default', - merge=salt['grains.filter_by'](osfamilymap, grain='os_family', - merge=salt['pillar.get']('firewalld:lookup') +{% set _config = salt['config.get']('firewalld', default={}) %} + +{% set defaults = salt['grains.filter_by'](default_settings, + default='firewalld', + merge=salt['grains.filter_by'](osarchmap, grain='osarch', + merge=salt['grains.filter_by'](osfamilymap, grain='os_family', + merge=salt['grains.filter_by'](osmap, grain='os', + merge=salt['grains.filter_by'](osfingermap, grain='osfinger', + merge=salt['grains.filter_by'](_config, default='lookup') + ) + ) + ) ) ) %} -{#- Merge in salt:lookup pillar #} -{% set firewalld = salt['pillar.get']( - 'firewalld', - default=platform_defaults.firewalld, - merge=True) -%} +{% set firewalld = salt['grains.filter_by']( + {'defaults': defaults}, + default='defaults', + merge=_config +) %} diff --git a/firewalld/osarchmap.yaml b/firewalld/osarchmap.yaml new file mode 100644 index 0000000..ab3bc1f --- /dev/null +++ b/firewalld/osarchmap.yaml @@ -0,0 +1,35 @@ +# -*- coding: utf-8 -*- +# vim: ft=yaml +# +# Setup variables using grains['osarch'] based logic. +# You just need to add the key:values for an `osarch` that differ +# from `defaults.yaml`. +# Only add an `osarch` which is/will be supported by the formula. +# +# If you do not need to provide defaults via the `osarch` grain, +# you will need to provide at least an empty dict in this file, e.g. +# osarch: {} +--- +amd64: + arch: amd64 + +x86_64: + arch: amd64 + +386: + arch: 386 + +arm64: + arch: arm64 + +armv6l: + arch: armv6l + +armv7l: + arch: armv7l + +ppc64le: + arch: ppc64le + +s390x: + arch: s390x diff --git a/firewalld/osfamilymap.yaml b/firewalld/osfamilymap.yaml index 810a01c..1c3c6ff 100644 --- a/firewalld/osfamilymap.yaml +++ b/firewalld/osfamilymap.yaml @@ -1,13 +1,33 @@ # -*- coding: utf-8 -*- -# # vim: ft=yaml -# os_family defaults -# only add key:values here that differ from whats in defaults.yaml +# vim: ft=yaml +# +# Setup variables using grains['os_family'] based logic. +# You just need to add the key:values for an `os_family` that differ +# from `defaults.yaml` + `osarch.yaml`. +# Only add an `os_family` which is/will be supported by the formula. +# +# If you do not need to provide defaults via the `os_family` grain, +# you will need to provide at least an empty dict in this file, e.g. +# osfamilymap: {} --- -Debian: - firewalld: {} -RedHat: - firewalld: {} -Arch: - firewalld: {} -Suse: - firewalld: {} +Debian: {} + +RedHat: {} + +Suse: {} + +Gentoo: {} + +Arch: {} + +Alpine: {} + +FreeBSD: {} + +OpenBSD: {} + +Solaris: {} + +Windows: {} + +MacOS: {} diff --git a/firewalld/osfingermap.yaml b/firewalld/osfingermap.yaml new file mode 100644 index 0000000..090e844 --- /dev/null +++ b/firewalld/osfingermap.yaml @@ -0,0 +1,45 @@ +# -*- coding: utf-8 -*- +# vim: ft=yaml +# +# Setup variables using grains['osfinger'] based logic. +# You just need to add the key:values for an `osfinger` that differ +# from `defaults.yaml` + `osarch.yaml` + `os_family.yaml` + `osmap.yaml`. +# Only add an `osfinger` which is/will be supported by the formula. +# +# If you do not need to provide defaults via the `os_finger` grain, +# you will need to provide at least an empty dict in this file, e.g. +# osfingermap: {} +--- +# os: Debian +Debian-10: {} +Debian-9: {} +Debian-8: {} + +# os: Ubuntu +Ubuntu-18.04: {} +Ubuntu-16.04: {} + +# os: Fedora +Fedora-31: {} +Fedora-30: {} + +# os: CentOS +CentOS Linux-8: {} +CentOS Linux-7: {} +CentOS-6: {} + +# os: Amazon +Amazon Linux-2: {} +Amazon Linux AMI-2018: {} + +# os: SUSE +Leap-15: {} + +# os: FreeBSD +FreeBSD-12: {} + +# os: Windows +Windows-8.1: {} + +# os: Gentoo +Gentoo-2: {} diff --git a/firewalld/osmap.yaml b/firewalld/osmap.yaml new file mode 100644 index 0000000..de11a67 --- /dev/null +++ b/firewalld/osmap.yaml @@ -0,0 +1,33 @@ +# -*- coding: utf-8 -*- +# vim: ft=yaml +# +# Setup variables using grains['os'] based logic. +# You just need to add the key:values for an `os` that differ +# from `defaults.yaml` + `osarch.yaml` + `os_family.yaml`. +# Only add an `os` which is/will be supported by the formula. +# +# If you do not need to provide defaults via the `os` grain, +# you will need to provide at least an empty dict in this file, e.g. +# osmap: {} +--- +# os_family: Debian +Ubuntu: {} +Raspbian: {} + +# os_family: RedHat +Fedora: {} +CentOS: {} +Amazon: {} + +# os_family: Suse +SUSE: {} +openSUSE: {} + +# os_family: Gentoo +Funtoo: {} + +# os_family: Arch +Manjaro: {} + +# os_family: Solaris +SmartOS: {} From d1f7a3717184bc22fde6e04d8672fcce0a462c4b Mon Sep 17 00:00:00 2001 From: Imran Iqbal Date: Sat, 18 Apr 2020 14:57:43 +0100 Subject: [PATCH 3/3] test(yaml_dump_spec): update after splitting `map.jinja` --- test/integration/default/controls/yaml_dump_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/integration/default/controls/yaml_dump_spec.rb b/test/integration/default/controls/yaml_dump_spec.rb index d4eba39..b2f5d6a 100644 --- a/test/integration/default/controls/yaml_dump_spec.rb +++ b/test/integration/default/controls/yaml_dump_spec.rb @@ -11,6 +11,7 @@ control 'firewalld `map.jinja` YAML dump' do IndividualCalls: 'no' LogDenied: 'off' RFC3964_IPv4: 'yes' + arch: amd64 backend: manage: true pkg: nftables