From 1202dd000f10b0e8959019484f1c3b3f9628fc67 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Thu, 14 Jan 2021 15:11:30 -0500 Subject: [PATCH] Allow restricting config values to enumerated list (#73162) * Allow restricting config values to enumerated list * dont document internal entries * also ignore private defs for ansible-config * remove invalid value from tests * added porting entry --- .../allow_restricted_config_values.yml | 3 +++ .../porting_guide_base_2.11.rst | 6 ++++++ .../command_plugins/dump_config.py | 8 +++++++- lib/ansible/cli/config.py | 4 ++-- lib/ansible/config/base.yml | 20 ++++++++++++++++--- lib/ansible/config/manager.py | 13 +++++++++++- lib/ansible/constants.py | 13 ++++++++++++ .../plugins/doc_fragments/shell_windows.py | 3 +-- lib/ansible/utils/color.py | 17 +--------------- test/integration/targets/collections/runme.sh | 2 +- 10 files changed, 63 insertions(+), 26 deletions(-) create mode 100644 changelogs/fragments/allow_restricted_config_values.yml diff --git a/changelogs/fragments/allow_restricted_config_values.yml b/changelogs/fragments/allow_restricted_config_values.yml new file mode 100644 index 00000000000..d770db9b6ca --- /dev/null +++ b/changelogs/fragments/allow_restricted_config_values.yml @@ -0,0 +1,3 @@ +minor_changes: + - Now 'choices' keyword in config definitions also restricts valid values for the entry. + - Internal config entries will not be documented, to mark an entry as internal it must start with `_`. diff --git a/docs/docsite/rst/porting_guides/porting_guide_base_2.11.rst b/docs/docsite/rst/porting_guides/porting_guide_base_2.11.rst index 0a4fc006bcc..259a1f70104 100644 --- a/docs/docsite/rst/porting_guides/porting_guide_base_2.11.rst +++ b/docs/docsite/rst/porting_guides/porting_guide_base_2.11.rst @@ -31,6 +31,12 @@ Command Line ``~/.ansible/galaxy_token``) or (insecurely) via the ``--token`` argument to ``ansible-galaxy``. +Other: +====== + +* The configuration system now validates the ``choices`` field, so any settings that currently violate it and are currently ignored will now cause an error. + For example, `ANSIBLE_COLLECTIONS_ON_ANSIBLE_VERSION_MISMATCH=0` will now cause an error (valid chioces are 'ignore', 'warn' or 'error'. + Deprecated ========== diff --git a/hacking/build_library/build_ansible/command_plugins/dump_config.py b/hacking/build_library/build_ansible/command_plugins/dump_config.py index 7811f4658cb..33591b47310 100644 --- a/hacking/build_library/build_ansible/command_plugins/dump_config.py +++ b/hacking/build_library/build_ansible/command_plugins/dump_config.py @@ -26,7 +26,13 @@ DEFAULT_TEMPLATE_DIR = pathlib.Path(__file__).parents[4] / 'docs/templates' def fix_description(config_options): '''some descriptions are strings, some are lists. workaround it...''' - for config_key in config_options: + for config_key in list(config_options.keys()): + + # drop internal entries + if config_key.startswith('_'): + del config_options[config_key] + continue + description = config_options[config_key].get('description', []) if isinstance(description, list): desc_list = description diff --git a/lib/ansible/cli/config.py b/lib/ansible/cli/config.py index a3f844562da..286aad7eff7 100644 --- a/lib/ansible/cli/config.py +++ b/lib/ansible/cli/config.py @@ -159,7 +159,7 @@ class ConfigCLI(CLI): ''' list all current configs reading lib/constants.py and shows env and config file setting names ''' - self.pager(to_text(yaml.dump(self.config.get_configuration_definitions(), Dumper=AnsibleDumper), errors='surrogate_or_strict')) + self.pager(to_text(yaml.dump(self.config.get_configuration_definitions(ignore_private=True), Dumper=AnsibleDumper), errors='surrogate_or_strict')) def execute_dump(self): ''' @@ -167,7 +167,7 @@ class ConfigCLI(CLI): ''' # FIXME: deal with plugins, not just base config text = [] - defaults = self.config.get_configuration_definitions().copy() + defaults = self.config.get_configuration_definitions(ignore_private=True).copy() for setting in self.config.data.get_settings(): if setting.name in defaults: defaults[setting.name] = setting diff --git a/lib/ansible/config/base.yml b/lib/ansible/config/base.yml index 7f36be41df8..616514b71f2 100644 --- a/lib/ansible/config/base.yml +++ b/lib/ansible/config/base.yml @@ -275,15 +275,19 @@ COLLECTIONS_ON_ANSIBLE_VERSION_MISMATCH: ini: [{key: collections_on_ansible_version_mismatch, section: defaults}] choices: [error, warning, ignore] default: warning +_COLOR_DEFAULTS: &color + name: placeholder for color settings' defaults + choices: ['black', 'bright gray', 'blue', 'white', 'green', 'bright blue', 'cyan', 'bright green', 'red', 'bright cyan', 'purple', 'bright red', 'yellow', 'bright purple', 'dark gray', 'bright yellow', 'magenta', 'bright magenta', 'normal'] COLOR_CHANGED: + <<: *color name: Color for 'changed' task status default: yellow description: Defines the color to use on 'Changed' task status env: [{name: ANSIBLE_COLOR_CHANGED}] ini: - {key: changed, section: colors} - yaml: {key: display.colors.changed} COLOR_CONSOLE_PROMPT: + <<: *color name: "Color for ansible-console's prompt task status" default: white description: Defines the default color to use for ansible-console @@ -292,22 +296,23 @@ COLOR_CONSOLE_PROMPT: - {key: console_prompt, section: colors} version_added: "2.7" COLOR_DEBUG: + <<: *color name: Color for debug statements default: dark gray description: Defines the color to use when emitting debug messages env: [{name: ANSIBLE_COLOR_DEBUG}] ini: - {key: debug, section: colors} - yaml: {key: display.colors.debug} COLOR_DEPRECATE: + <<: *color name: Color for deprecation messages default: purple description: Defines the color to use when emitting deprecation messages env: [{name: ANSIBLE_COLOR_DEPRECATE}] ini: - {key: deprecate, section: colors} - yaml: {key: display.colors.deprecate} COLOR_DIFF_ADD: + <<: *color name: Color for diff added display default: green description: Defines the color to use when showing added lines in diffs @@ -316,6 +321,7 @@ COLOR_DIFF_ADD: - {key: diff_add, section: colors} yaml: {key: display.colors.diff.add} COLOR_DIFF_LINES: + <<: *color name: Color for diff lines display default: cyan description: Defines the color to use when showing diffs @@ -323,6 +329,7 @@ COLOR_DIFF_LINES: ini: - {key: diff_lines, section: colors} COLOR_DIFF_REMOVE: + <<: *color name: Color for diff removed display default: red description: Defines the color to use when showing removed lines in diffs @@ -330,6 +337,7 @@ COLOR_DIFF_REMOVE: ini: - {key: diff_remove, section: colors} COLOR_ERROR: + <<: *color name: Color for error messages default: red description: Defines the color to use when emitting error messages @@ -338,6 +346,7 @@ COLOR_ERROR: - {key: error, section: colors} yaml: {key: colors.error} COLOR_HIGHLIGHT: + <<: *color name: Color for highlighting default: white description: Defines the color to use for highlighting @@ -345,6 +354,7 @@ COLOR_HIGHLIGHT: ini: - {key: highlight, section: colors} COLOR_OK: + <<: *color name: Color for 'ok' task status default: green description: Defines the color to use when showing 'OK' task status @@ -352,6 +362,7 @@ COLOR_OK: ini: - {key: ok, section: colors} COLOR_SKIP: + <<: *color name: Color for 'skip' task status default: cyan description: Defines the color to use when showing 'Skipped' task status @@ -359,6 +370,7 @@ COLOR_SKIP: ini: - {key: skip, section: colors} COLOR_UNREACHABLE: + <<: *color name: Color for 'unreachable' host state default: bright red description: Defines the color to use on 'Unreachable' status @@ -366,6 +378,7 @@ COLOR_UNREACHABLE: ini: - {key: unreachable, section: colors} COLOR_VERBOSE: + <<: *color name: Color for verbose messages default: blue description: Defines the color to use when emitting verbose messages. i.e those that show with '-v's. @@ -373,6 +386,7 @@ COLOR_VERBOSE: ini: - {key: verbose, section: colors} COLOR_WARN: + <<: *color name: Color for warning messages default: bright purple description: Defines the color to use when emitting warning messages diff --git a/lib/ansible/config/manager.py b/lib/ansible/config/manager.py index 7ef437a6139..fbc3bcde604 100644 --- a/lib/ansible/config/manager.py +++ b/lib/ansible/config/manager.py @@ -384,7 +384,7 @@ class ConfigManager(object): return ret - def get_configuration_definitions(self, plugin_type=None, name=None): + def get_configuration_definitions(self, plugin_type=None, name=None, ignore_private=False): ''' just list the possible settings, either base or for specific plugins or plugin ''' ret = {} @@ -395,6 +395,11 @@ class ConfigManager(object): else: ret = self._plugins.get(plugin_type, {}).get(name, {}) + if ignore_private: + for cdef in list(ret.keys()): + if cdef.startswith('_'): + del ret[cdef] + return ret def _loop_entries(self, container, entry_list): @@ -536,6 +541,12 @@ class ConfigManager(object): raise AnsibleOptionsError('Invalid type for configuration option %s: %s' % (to_native(_get_entry(plugin_type, plugin_name, config)), to_native(e))) + # deal with restricted values + if value is not None and 'choices' in defs[config] and defs[config]['choices'] is not None: + if value not in defs[config]['choices']: + raise AnsibleOptionsError('Invalid value "%s" for configuration option "%s", valid values are: %s' % + (value, to_native(_get_entry(plugin_type, plugin_name, config)), defs[config]['choices'])) + # deal with deprecation of the setting if 'deprecated' in defs[config] and origin != 'default': self.DEPRECATED.append((config, defs[config].get('deprecated'))) diff --git a/lib/ansible/constants.py b/lib/ansible/constants.py index e6c660f1dbe..825e1096c54 100644 --- a/lib/ansible/constants.py +++ b/lib/ansible/constants.py @@ -61,6 +61,19 @@ class _DeprecatedSequenceConstant(Sequence): # CONSTANTS ### yes, actual ones +# http://nezzen.net/2008/06/23/colored-text-in-python-using-ansi-escape-sequences/ +COLOR_CODES = { + 'black': u'0;30', 'bright gray': u'0;37', + 'blue': u'0;34', 'white': u'1;37', + 'green': u'0;32', 'bright blue': u'1;34', + 'cyan': u'0;36', 'bright green': u'1;32', + 'red': u'0;31', 'bright cyan': u'1;36', + 'purple': u'0;35', 'bright red': u'1;31', + 'yellow': u'0;33', 'bright purple': u'1;35', + 'dark gray': u'1;30', 'bright yellow': u'1;33', + 'magenta': u'0;35', 'bright magenta': u'1;35', + 'normal': u'0', +} REJECT_EXTS = ('.pyc', '.pyo', '.swp', '.bak', '~', '.rpm', '.md', '.txt', '.rst') BOOL_TRUE = BOOLEANS_TRUE COLLECTION_PTYPE_COMPAT = {'module': 'modules'} diff --git a/lib/ansible/plugins/doc_fragments/shell_windows.py b/lib/ansible/plugins/doc_fragments/shell_windows.py index ee43f7ca72f..ddb573fc71b 100644 --- a/lib/ansible/plugins/doc_fragments/shell_windows.py +++ b/lib/ansible/plugins/doc_fragments/shell_windows.py @@ -38,8 +38,7 @@ options: - Windows only supports C(no) as an option. type: bool default: 'no' - choices: - - 'no' + choices: ['no', False] environment: description: - List of dictionaries of environment variables and their values to use when diff --git a/lib/ansible/utils/color.py b/lib/ansible/utils/color.py index 8762b44f3f8..be8fb004e48 100644 --- a/lib/ansible/utils/color.py +++ b/lib/ansible/utils/color.py @@ -51,21 +51,6 @@ if C.ANSIBLE_FORCE_COLOR: # domain - there is no license except that you must leave this header. # # Copyright (C) 2008 Brian Nez -# -# http://nezzen.net/2008/06/23/colored-text-in-python-using-ansi-escape-sequences/ - -codeCodes = { - 'black': u'0;30', 'bright gray': u'0;37', - 'blue': u'0;34', 'white': u'1;37', - 'green': u'0;32', 'bright blue': u'1;34', - 'cyan': u'0;36', 'bright green': u'1;32', - 'red': u'0;31', 'bright cyan': u'1;36', - 'purple': u'0;35', 'bright red': u'1;31', - 'yellow': u'0;33', 'bright purple': u'1;35', - 'dark gray': u'1;30', 'bright yellow': u'1;33', - 'magenta': u'0;35', 'bright magenta': u'1;35', - 'normal': u'0', -} def parsecolor(color): @@ -74,7 +59,7 @@ def parsecolor(color): r"|(?Prgb(?P[0-5])(?P[0-5])(?P[0-5]))" r"|gray(?P[0-9]+)", color) if not matches: - return codeCodes[color] + return C.COLOR_CODES[color] if matches.group('color'): return u'38;5;%d' % int(matches.group('color')) if matches.group('rgb'): diff --git a/test/integration/targets/collections/runme.sh b/test/integration/targets/collections/runme.sh index a9b05447449..53641cc5510 100755 --- a/test/integration/targets/collections/runme.sh +++ b/test/integration/targets/collections/runme.sh @@ -6,7 +6,7 @@ export ANSIBLE_COLLECTIONS_PATH=$PWD/collection_root_user:$PWD/collection_root_s export ANSIBLE_GATHERING=explicit export ANSIBLE_GATHER_SUBSET=minimal export ANSIBLE_HOST_PATTERN_MISMATCH=error -export ANSIBLE_COLLECTIONS_ON_ANSIBLE_VERSION_MISMATCH=0 +unset ANSIBLE_COLLECTIONS_ON_ANSIBLE_VERSION_MISMATCH # FUTURE: just use INVENTORY_PATH as-is once ansible-test sets the right dir ipath=../../$(basename "${INVENTORY_PATH:-../../inventory}")