Omit full fix (#79024)

* omit keyword should reset to context (#78917)

* omit keyword should reset to context

  ensure we use context/inheritance when calculating value,
  using default only when context is unavailable.

  fixes #75692

(cherry picked from commit 9650ddb11cd220845ca7996eeb620035f5e07a0a)

* fixes to FA inheritance (#78990)

finalized applies to all field attributes
fix getting parent value
also remove unused/needed extend/prepend signature
moar testing

(cherry picked from commit ff6e4da36addccb06001f7b05b1a9c04ae1d7984)

* setup role needs it's own info

8 files changed, 119 insertions, 15 deletions

diff --git a/changelogs/fragments/fix_omit_key.yml b/changelogs/fragments/fix_omit_key.yml
new file mode 100644
index 0000000000..13d22466f2
--- /dev/null
+++ b/changelogs/fragments/fix_omit_key.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - omit on keywords was resetting to default value, ignoring inheritance.
diff --git a/lib/ansible/playbook/base.py b/lib/ansible/playbook/base.py
index ae7957b0bc..f254767227 100644
--- a/lib/ansible/playbook/base.py
+++ b/lib/ansible/playbook/base.py
@@ -362,10 +362,7 @@ class FieldAttributeBase:
         # Resolve extended groups last, after caching the group in case they recursively refer to each other
         for include_group in include_groups:
             if not AnsibleCollectionRef.is_valid_fqcr(include_group):
-                include_group_collection = collection_name
                 include_group = collection_name + '.' + include_group
-            else:
-                include_group_collection = '.'.join(include_group.split('.')[0:2])
 
             dummy, group_actions = self._resolve_group(include_group, mandatory=False)
 
@@ -481,6 +478,20 @@ class FieldAttributeBase:
             value.post_validate(templar=templar)
         return value
 
+    def set_to_context(self, name):
+        ''' set to parent inherited value or Sentinel as appropriate'''
+
+        attribute = self.fattributes[name]
+        if isinstance(attribute, NonInheritableFieldAttribute):
+            # setting to sentinel will trigger 'default/default()' on getter
+            setattr(self, name, Sentinel)
+        else:
+            try:
+                setattr(self, name, self._get_parent_attribute(name, omit=True))
+            except AttributeError:
+                # mostly playcontext as only tasks/handlers/blocks really resolve parent
+                setattr(self, name, Sentinel)
+
     def post_validate(self, templar):
         '''
         we can't tell that everything is of the right type until we have
@@ -524,13 +535,10 @@ class FieldAttributeBase:
                     # if the attribute contains a variable, template it now
                     value = templar.template(getattr(self, name))
 
-                # if this evaluated to the omit value, set the value back to
-                # the default specified in the FieldAttribute and move on
+                # If this evaluated to the omit value, set the value back to inherited by context
+                # or default specified in the FieldAttribute and move on
                 if omit_value is not None and value == omit_value:
-                    if callable(attribute.default):
-                        setattr(self, name, attribute.default())
-                    else:
-                        setattr(self, name, attribute.default)
+                    self.set_to_context(name)
                     continue
 
                 # and make sure the attribute is of the type it should be
@@ -679,7 +687,7 @@ class FieldAttributeBase:
             if name in data:
                 setattr(self, name, data[name])
             else:
-                setattr(self, name, attribute.default)
+                self.set_to_context(name)
 
         # restore the UUID field
         setattr(self, '_uuid', data.get('uuid'))
diff --git a/lib/ansible/playbook/block.py b/lib/ansible/playbook/block.py
index 7a6080cd46..216d04e856 100644
--- a/lib/ansible/playbook/block.py
+++ b/lib/ansible/playbook/block.py
@@ -294,14 +294,20 @@ class Block(Base, Conditional, CollectionSearch, Taggable):
             for dep in dep_chain:
                 dep.set_loader(loader)
 
-    def _get_parent_attribute(self, attr, extend=False, prepend=False):
+    def _get_parent_attribute(self, attr, omit=False):
         '''
         Generic logic to get the attribute or parent attribute for a block value.
         '''
         extend = self.fattributes.get(attr).extend
         prepend = self.fattributes.get(attr).prepend
+
         try:
-            value = getattr(self, f'_{attr}', Sentinel)
+            # omit self, and only get parent values
+            if omit:
+                value = Sentinel
+            else:
+                value = getattr(self, f'_{attr}', Sentinel)
+
             # If parent is static, we can grab attrs from the parent
             # otherwise, defer to the grandparent
             if getattr(self._parent, 'statically_loaded', True):
diff --git a/lib/ansible/playbook/task.py b/lib/ansible/playbook/task.py
index 47072b069f..d0a4c6ecc8 100644
--- a/lib/ansible/playbook/task.py
+++ b/lib/ansible/playbook/task.py
@@ -457,14 +457,20 @@ class Task(Base, Conditional, Taggable, CollectionSearch):
         if self._parent:
             self._parent.set_loader(loader)
 
-    def _get_parent_attribute(self, attr, extend=False, prepend=False):
+    def _get_parent_attribute(self, attr, omit=False):
         '''
         Generic logic to get the attribute or parent attribute for a task value.
         '''
         extend = self.fattributes.get(attr).extend
         prepend = self.fattributes.get(attr).prepend
+
         try:
-            value = getattr(self, f'_{attr}', Sentinel)
+            # omit self, and only get parent values
+            if omit:
+                value = Sentinel
+            else:
+                value = getattr(self, f'_{attr}', Sentinel)
+
             # If parent is static, we can grab attrs from the parent
             # otherwise, defer to the grandparent
             if getattr(self._parent, 'statically_loaded', True):
diff --git a/test/integration/targets/omit/75692.yml b/test/integration/targets/omit/75692.yml
new file mode 100644
index 0000000000..b4000c978c
--- /dev/null
+++ b/test/integration/targets/omit/75692.yml
@@ -0,0 +1,31 @@
+- name: omit should reset to 'absent' or same context, not just 'default' value
+  hosts: testhost
+  gather_facts: false
+  become: yes
+  become_user: nobody
+  roles:
+    - name: setup_test_user
+      become: yes
+      become_user: root
+  tasks:
+    - shell: whoami
+      register: inherited
+
+    - shell: whoami
+      register: explicit_no
+      become: false
+
+    - shell: whoami
+      register: omited_inheritance
+      become: '{{ omit }}'
+
+    - shell: whoami
+      register: explicit_yes
+      become: yes
+
+    - name: ensure omit works with inheritance
+      assert:
+        that:
+          - inherited.stdout == omited_inheritance.stdout
+          - inherited.stdout == explicit_yes.stdout
+          - inherited.stdout != explicit_no.stdout
diff --git a/test/integration/targets/omit/C75692.yml b/test/integration/targets/omit/C75692.yml
new file mode 100644
index 0000000000..6e1215fbb5
--- /dev/null
+++ b/test/integration/targets/omit/C75692.yml
@@ -0,0 +1,44 @@
+- hosts: localhost
+  gather_facts: false
+  tasks:
+    - name: Make sure foo is gone
+      file:
+        path: foo
+        state: absent
+    - name: Create foo - should only be changed in first iteration
+      copy:
+        dest: foo
+        content: foo
+      check_mode: '{{ omit }}'
+      register: cmode
+      loop:
+        - 1
+        - 2
+
+    - when: ansible_check_mode
+      block:
+      - name: stat foo
+        stat: path=foo
+        register: foo
+        check_mode: off
+      - debug: var=foo
+      - name: validate expected outcomes when in check mode and file does not exist
+        assert:
+          that:
+            - cmode['results'][0] is changed
+            - cmode['results'][1] is changed
+        when: not foo['stat']['exists']
+
+      - name: validate expected outcomes when in check mode and file exists
+        assert:
+          that:
+            - cmode['results'][0] is not changed
+            - cmode['results'][1] is not changed
+        when: foo['stat']['exists']
+
+    - name: validate expected outcomes when not in check mode (file is always deleted)
+      assert:
+        that:
+          - cmode['results'][0] is changed
+          - cmode['results'][1] is not changed
+      when: not ansible_check_mode
diff --git a/test/integration/targets/omit/aliases b/test/integration/targets/omit/aliases
index 1d28bdb2aa..1bff31cb09 100644
--- a/test/integration/targets/omit/aliases
+++ b/test/integration/targets/omit/aliases
@@ -1,2 +1,3 @@
 shippable/posix/group5
+needs/target/setup_test_user
 context/controller
diff --git a/test/integration/targets/omit/runme.sh b/test/integration/targets/omit/runme.sh
index 962e1f0409..e2f3c0274d 100755
--- a/test/integration/targets/omit/runme.sh
+++ b/test/integration/targets/omit/runme.sh
@@ -2,4 +2,10 @@
 
 set -eux
 
-ansible-playbook 48673.yml -i ../../inventory -v "$@"
+# positive inheritance works
+ANSIBLE_ROLES_PATH=../ ansible-playbook 48673.yml 75692.yml  -i ../../inventory -v "$@"
+
+# ensure negative also works
+ansible-playbook -C C75692.yml -i ../../inventory -v "$@"	# expects 'foo' not to exist
+ansible-playbook C75692.yml -i ../../inventory -v "$@"		# creates 'foo'
+ansible-playbook -C C75692.yml -i ../../inventory -v "$@"	# expects 'foo' does exist