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

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 `_`.

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
 ==========
 

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

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

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

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')))

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'}

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:
+    choices: ['no', False]
-    - 'no'
   environment:
     description:
     - List of dictionaries of environment variables and their values to use when

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 <thedude at bri1 dot com>
-#
-# 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"|(?P<rgb>rgb(?P<red>[0-5])(?P<green>[0-5])(?P<blue>[0-5]))"
                        r"|gray(?P<gray>[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'):

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}")