From 18a66e291dad71128a32d662aa808213acefe0e9 Mon Sep 17 00:00:00 2001 From: David Shrewsbury Date: Mon, 13 Apr 2020 12:53:05 -0400 Subject: [PATCH] Force collections to be static (#68723) * Force collections to be static Templating of collection names does not work at all. Force them to be static so that a warning is generated for the user. * Add collectionsearch unit test and fix for reviews New unit test validates the new _load_collections() code and moves the new check to the end of the method. * Change unit test to pytest * Adjust unit test to use capsys instead of monkeypatch * Fix pep8 error * Add changelog fragment Closes #68704 --- .../68723-force-static-collections.yaml | 3 ++ lib/ansible/playbook/collectionsearch.py | 16 +++++++- test/units/playbook/test_collectionsearch.py | 38 +++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 changelogs/fragments/68723-force-static-collections.yaml create mode 100644 test/units/playbook/test_collectionsearch.py diff --git a/changelogs/fragments/68723-force-static-collections.yaml b/changelogs/fragments/68723-force-static-collections.yaml new file mode 100644 index 00000000000..c6aaf86f715 --- /dev/null +++ b/changelogs/fragments/68723-force-static-collections.yaml @@ -0,0 +1,3 @@ +bugfixes: +- Force collection names to be static so that a warning is generated because + templating currently does not work (see https://github.com/ansible/ansible/issues/68704). diff --git a/lib/ansible/playbook/collectionsearch.py b/lib/ansible/playbook/collectionsearch.py index 93b80a86657..994e2e13e4c 100644 --- a/lib/ansible/playbook/collectionsearch.py +++ b/lib/ansible/playbook/collectionsearch.py @@ -7,6 +7,10 @@ __metaclass__ = type from ansible.module_utils.six import string_types from ansible.playbook.attribute import FieldAttribute from ansible.utils.collection_loader import AnsibleCollectionLoader +from ansible.template import is_template, Environment +from ansible.utils.display import Display + +display = Display() def _ensure_default_collection(collection_list=None): @@ -32,7 +36,8 @@ def _ensure_default_collection(collection_list=None): class CollectionSearch: # this needs to be populated before we can resolve tasks/roles/etc - _collections = FieldAttribute(isa='list', listof=string_types, priority=100, default=_ensure_default_collection) + _collections = FieldAttribute(isa='list', listof=string_types, priority=100, default=_ensure_default_collection, + always_post_validate=True, static=True) def _load_collections(self, attr, ds): # this will only be called if someone specified a value; call the shared value @@ -41,4 +46,13 @@ class CollectionSearch: if not ds: # don't return an empty collection list, just return None return None + # This duplicates static attr checking logic from post_validate() + # because if the user attempts to template a collection name, it will + # error before it ever gets to the post_validate() warning. + env = Environment() + for collection_name in ds: + if is_template(collection_name, env): + display.warning('"collections" is not templatable, but we found: %s, ' + 'it will not be templated and will be used "as is".' % (collection_name)) + return ds diff --git a/test/units/playbook/test_collectionsearch.py b/test/units/playbook/test_collectionsearch.py new file mode 100644 index 00000000000..fe480d3de1e --- /dev/null +++ b/test/units/playbook/test_collectionsearch.py @@ -0,0 +1,38 @@ +# (c) 2020 Ansible Project +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +from ansible.playbook.collectionsearch import CollectionSearch + +import pytest + + +def test_collection_static_warning(capsys): + """Test that collection name is not templated. + + Also, make sure that users see the warning message for the referenced name. + """ + + collection_name = 'foo.{{bar}}' + cs = CollectionSearch() + assert collection_name in cs._load_collections(None, [collection_name]) + + std_out, std_err = capsys.readouterr() + assert '[WARNING]: "collections" is not templatable, but we found: %s' % collection_name in std_err + assert '' == std_out