[playbook] error on empty, error on 'include' (remove two deprecations) (#74172)

Change:
- Remove two deprecated features
- We now error if a playbook is an empty list instead of just skipping
- We now error if using 'include' instead of 'import_playbook'

Test Plan:
- Added new tests for new errors

Tickets:
- Fixes #74133

Signed-off-by: Rick Elrod <rick@elrod.me>

* sanity & changelog

Signed-off-by: Rick Elrod <rick@elrod.me>

changelogs/fragments/playbook-deprecations.yml

@@ -0,0 +1,3 @@
+minor_changes:
+  - playbook - Error if a playbook is an empty list instead of just skipping
+  - playbook - Error if using ``include`` instead of ``import_playbook``

lib/ansible/constants.py

@@ -77,7 +77,6 @@ _ACTION_SETUP = add_internal_fqcns(('setup', ))
 _ACTION_HAS_CMD = add_internal_fqcns(('command', 'shell', 'script'))
 _ACTION_ALLOWS_RAW_ARGS = _ACTION_HAS_CMD + add_internal_fqcns(('raw', ))
 _ACTION_ALL_INCLUDES = _ACTION_INCLUDE + _ACTION_INCLUDE_TASKS + _ACTION_INCLUDE_ROLE
-_ACTION_ALL_IMPORT_PLAYBOOKS = _ACTION_INCLUDE + _ACTION_IMPORT_PLAYBOOK
 _ACTION_ALL_INCLUDE_IMPORT_TASKS = _ACTION_INCLUDE + _ACTION_INCLUDE_TASKS + _ACTION_IMPORT_TASKS
 _ACTION_ALL_PROPER_INCLUDE_IMPORT_ROLES = _ACTION_INCLUDE_ROLE + _ACTION_IMPORT_ROLE
 _ACTION_ALL_PROPER_INCLUDE_IMPORT_TASKS = _ACTION_INCLUDE_TASKS + _ACTION_IMPORT_TASKS

lib/ansible/playbook/__init__.py

@@ -79,8 +79,8 @@ class Playbook:
             self._loader.set_basedir(cur_basedir)
             raise AnsibleParserError("A playbook must be a list of plays, got a %s instead" % type(ds), obj=ds)
         elif not ds:
-            display.deprecated("Empty plays will currently be skipped, in the future they will cause a syntax error",
+            self._loader.set_basedir(cur_basedir)
-                               version='2.12', collection_name='ansible.builtin')
+            raise AnsibleParserError("A playbook must contain at least one play")
 
         # Parse the playbook entries. For plays, we simply parse them
         # using the Play() object, and includes are parsed using the
@@ -89,18 +89,15 @@ class Playbook:
             if not isinstance(entry, dict):
                 # restore the basedir in case this error is caught and handled
                 self._loader.set_basedir(cur_basedir)
-                raise AnsibleParserError("playbook entries must be either a valid play or an include statement", obj=entry)
+                raise AnsibleParserError("playbook entries must be either valid plays or 'import_playbook' statements", obj=entry)
 
-            if any(action in entry for action in C._ACTION_ALL_IMPORT_PLAYBOOKS):
+            if any(action in entry for action in C._ACTION_IMPORT_PLAYBOOK):
-                if any(action in entry for action in C._ACTION_INCLUDE):
-                    display.deprecated("'include' for playbook includes. You should use 'import_playbook' instead",
-                                       version="2.12", collection_name='ansible.builtin')
                 pb = PlaybookInclude.load(entry, basedir=self._basedir, variable_manager=variable_manager, loader=self._loader)
                 if pb is not None:
                     self._entries.extend(pb._entries)
                 else:
                     which = entry
-                    for k in C._ACTION_IMPORT_PLAYBOOK + C._ACTION_INCLUDE:
+                    for k in C._ACTION_IMPORT_PLAYBOOK:
                         if k in entry:
                             which = entry[k]
                             break

lib/ansible/playbook/playbook_include.py

@@ -140,7 +140,7 @@ class PlaybookInclude(Base, Conditional, Taggable):
             new_ds.ansible_pos = ds.ansible_pos
 
         for (k, v) in iteritems(ds):
-            if k in C._ACTION_ALL_IMPORT_PLAYBOOKS:
+            if k in C._ACTION_IMPORT_PLAYBOOK:
                 self._preprocess_import(ds, new_ds, k, v)
             else:
                 # some basic error checking, to make sure vars are properly

test/integration/targets/includes/include_on_playbook_should_fail.yml

@@ -0,0 +1 @@
+- include: test_includes3.yml

test/integration/targets/includes/runme.sh

@@ -5,3 +5,9 @@ set -eux
 ansible-playbook test_includes.yml -i ../../inventory "$@"
 
 ansible-playbook inherit_notify.yml "$@"
+
+echo "EXPECTED ERROR: Ensure we fail if using 'include' to include a playbook."
+set +e
+result="$(ansible-playbook -i ../../inventory include_on_playbook_should_fail.yml -v "$@" 2>&1)"
+set -e
+grep -q "ERROR! 'include' is not a valid attribute for a Play" <<< "$result"

test/integration/targets/includes/test_includes.yml

@@ -1,7 +1,7 @@
-- include: test_includes2.yml parameter1=asdf parameter2=jkl
+- import_playbook: test_includes2.yml parameter1=asdf parameter2=jkl
 
-- include: test_includes3.yml
+- import_playbook: test_includes3.yml
 
-- include: test_include_free.yml
+- import_playbook: test_include_free.yml
 
-- include: test_include_host_pinned.yml
+- import_playbook: test_include_host_pinned.yml

test/integration/targets/path_lookups/play.yml

@@ -10,31 +10,31 @@
     - copy: dest={{playbook_dir}}/files/testfile content='in files'
     - copy: dest={{playbook_dir}}/testfile content='in local'
 
-- include: testplay.yml
+- import_playbook: testplay.yml
   vars:
     remove: nothing
     role_out: in role files
     play_out: in files
 
-- include: testplay.yml
+- import_playbook: testplay.yml
   vars:
     remove: roles/showfile/files/testfile
     role_out: in role
     play_out: in files
 
-- include: testplay.yml
+- import_playbook: testplay.yml
   vars:
     remove: roles/showfile/testfile
     role_out: in role tasks
     play_out: in files
 
-- include: testplay.yml
+- import_playbook: testplay.yml
   vars:
     remove: roles/showfile/tasks/testfile
     role_out: in files
     play_out: in files
 
-- include: testplay.yml
+- import_playbook: testplay.yml
   vars:
     remove: files/testfile
     role_out: in local

test/integration/targets/playbook/empty.yml

@@ -0,0 +1 @@
+[]

test/integration/targets/playbook/runme.sh

@@ -7,3 +7,9 @@ ansible-playbook -i ../../inventory types.yml -v "$@"
 
 # test timeout
 ansible-playbook -i ../../inventory timeout.yml -v "$@"
+
+echo "EXPECTED ERROR: Ensure we fail properly if a playbook is an empty list."
+set +e
+result="$(ansible-playbook -i ../../inventory empty.yml -v "$@" 2>&1)"
+set -e
+grep -q "ERROR! A playbook must contain at least one play" <<< "$result"

test/sanity/ignore.txt

@@ -142,7 +142,6 @@ lib/ansible/modules/yum_repository.py validate-modules:doc-default-does-not-matc
 lib/ansible/modules/yum_repository.py validate-modules:parameter-type-not-in-doc
 lib/ansible/modules/yum_repository.py validate-modules:undocumented-parameter
 lib/ansible/parsing/vault/__init__.py pylint:blacklisted-name
-lib/ansible/playbook/__init__.py pylint:ansible-deprecated-version
 lib/ansible/playbook/base.py pylint:blacklisted-name
 lib/ansible/playbook/collectionsearch.py required-and-default-attributes  # https://github.com/ansible/ansible/issues/61460
 lib/ansible/playbook/conditional.py pylint:ansible-deprecated-version