Repository: ansible/ansible
Pull request: Case-insensitive set theory filters

##### SUMMARY

Fixes #74255

##### ISSUE TYPE

- Feature Pull Request

##### COMPONENT NAME

Filter plugin

##### ADDITIONAL INFORMATION

Change behavior of `unique` filter: the ansible fallback **is** case-sensitive, so don't fail when using it with `case_sensitive=True`.


Problem statement:
difference filter unexpectedly performs case-insensitive comparison
### Summary

When using `difference` filter with element that have just case-difference in the value of the filter, only one is kept.

### Issue Type

Feature Request

### Component Name

filters

### Ansible Version

```console
$ ansible --version
ansible 2.9.10
  config file = None
  configured module search path = ['/home/user/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/lib/python3.8/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.8.8 (default, Mar  4 2021, 21:24:42) [GCC 10.2.0]
```


### Configuration

```console
$ ansible-config dump --only-changed
```


### OS / Environment

RHEL 7

### Steps to Reproduce

```yaml
- hosts: localhost
  vars:
    list1:
      - a
      - A
      - b
      - B
    list2:
      - b
  tasks:
    - debug:
        msg: "{{ list1 | difference(list2) }}"
    - debug:
        msg: "{{ list1 | difference([]) }}"
```

```shell
ansible-playbook playbook.yml
```

### Expected Results

**All** the elements of `list1` that don’t exist in `list2` (as documented)

```
TASK [debug] ***********************
ok: [localhost] => {
    "msg": [
        "a",
        "A",
        "B"
    ]
}

TASK [debug] ***********************
ok: [localhost] => {
    "msg": [
        "a",
        "A",
        "b",
        "B"
    ]
}
```

### Actual Results

```console
TASK [debug] ***********************
ok: [localhost] => {
    "msg": [
        "a",
        "B"
    ]
}

TASK [debug] ***********************
ok: [localhost] => {
    "msg": [
        "a",
        "b"
    ]
}
```


### Code of Conduct

I agree to follow the Ansible Code of Conduct

Hints from issue discussion:
Files identified in the description:
None

If these files are incorrect, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
This is because `difference` filter is using `unique`
https://github.com/ansible/ansible/blob/fa0bccf6a1f09c0118b1609aadd10f85927f5edb/lib/ansible/plugins/filter/mathstuff.py#L103
without specifying `case_sensitive` which is `False` by default
https://github.com/ansible/ansible/blob/fa0bccf6a1f09c0118b1609aadd10f85927f5edb/lib/ansible/plugins/filter/mathstuff.py#L55
Changing this to a feature request. This would need to be added to several other tests as well to make the behavior consistent.

Commit message: Test multiple situations of 'unique' filter errors with fallback not handling specific parameters

Patch to review:
diff --git a/changelogs/fragments/74256-set-theory-filters-behavior.yml b/changelogs/fragments/74256-set-theory-filters-behavior.yml
new file mode 100644
index 00000000000000..8793c31bdc83ac
--- /dev/null
+++ b/changelogs/fragments/74256-set-theory-filters-behavior.yml
@@ -0,0 +1,6 @@
+breaking_changes:
+  - intersect, difference, symmetric_difference, union filters - the default behavior
+    is now to be case-sensitive (https://github.com/ansible/ansible/issues/74255)
+  - unique filter - the default behavior is now to fail if Jinja2's filter fails and
+    explicit ``case_sensitive=False`` as the Ansible's fallback is case-sensitive
+    (https://github.com/ansible/ansible/pull/74256)
diff --git a/docs/docsite/rst/porting_guides/porting_guide_core_2.12.rst b/docs/docsite/rst/porting_guides/porting_guide_core_2.12.rst
index 189b30e3108583..5cf9b222514423 100644
--- a/docs/docsite/rst/porting_guides/porting_guide_core_2.12.rst
+++ b/docs/docsite/rst/porting_guides/porting_guide_core_2.12.rst
@@ -47,6 +47,13 @@ Deprecated
         when: not teardown | bool
 
 
+Breaking Changes
+================
+
+* ``unique`` filter with Jinja2 < 2.10 is case-sensitive and now raise coherently an error if ``case_sensitive=False`` instead of when ``case_sensitive=True``.
+* Set theory filters (``intersect``, ``difference``, ``symmetric_difference`` and ``union``) are now case-sensitive. Explicitly use ``case_sensitive=False`` to keep previous behavior. Note: with Jinja2 < 2.10, the filters were already case-sensitive by default.
+
+
 Modules
 =======
 
diff --git a/lib/ansible/plugins/filter/mathstuff.py b/lib/ansible/plugins/filter/mathstuff.py
index 77baa7ef006722..1a545538892752 100644
--- a/lib/ansible/plugins/filter/mathstuff.py
+++ b/lib/ansible/plugins/filter/mathstuff.py
@@ -52,17 +52,19 @@
 
 
 @environmentfilter
-def unique(environment, a, case_sensitive=False, attribute=None):
+# Use case_sensitive=None as a sentinel value, so we raise an error only when
+# explicitly set and cannot be handle (by Jinja2 w/o 'unique' or fallback version)
+def unique(environment, a, case_sensitive=None, attribute=None):
 
     def _do_fail(e):
-        if case_sensitive or attribute:
+        if case_sensitive is False or attribute:
             raise AnsibleFilterError("Jinja2's unique filter failed and we cannot fall back to Ansible's version "
                                      "as it does not support the parameters supplied", orig_exc=e)
 
     error = e = None
     try:
         if HAS_UNIQUE:
-            c = list(do_unique(environment, a, case_sensitive=case_sensitive, attribute=attribute))
+            c = list(do_unique(environment, a, case_sensitive=bool(case_sensitive), attribute=attribute))
     except TypeError as e:
         error = e
         _do_fail(e)
@@ -74,8 +76,8 @@ def _do_fail(e):
     if not HAS_UNIQUE or error:
 
         # handle Jinja2 specific attributes when using Ansible's version
-        if case_sensitive or attribute:
-            raise AnsibleFilterError("Ansible's unique filter does not support case_sensitive nor attribute parameters, "
+        if case_sensitive is False or attribute:
+            raise AnsibleFilterError("Ansible's unique filter does not support case_sensitive=False nor attribute parameters, "
                                      "you need a newer version of Jinja2 that provides their version of the filter.")
 
         c = []
@@ -91,7 +93,7 @@ def intersect(environment, a, b):
     if isinstance(a, Hashable) and isinstance(b, Hashable):
         c = set(a) & set(b)
     else:
-        c = unique(environment, [x for x in a if x in b])
+        c = unique(environment, [x for x in a if x in b], True)
     return c
 
 
@@ -100,7 +102,7 @@ def difference(environment, a, b):
     if isinstance(a, Hashable) and isinstance(b, Hashable):
         c = set(a) - set(b)
     else:
-        c = unique(environment, [x for x in a if x not in b])
+        c = unique(environment, [x for x in a if x not in b], True)
     return c
 
 
@@ -119,7 +121,7 @@ def union(environment, a, b):
     if isinstance(a, Hashable) and isinstance(b, Hashable):
         c = set(a) | set(b)
     else:
-        c = unique(environment, a + b)
+        c = unique(environment, a + b, True)
     return c
 
 
diff --git a/test/integration/targets/filter_mathstuff/runme.sh b/test/integration/targets/filter_mathstuff/runme.sh
new file mode 100755
index 00000000000000..36503003153b36
--- /dev/null
+++ b/test/integration/targets/filter_mathstuff/runme.sh
@@ -0,0 +1,17 @@
+#!/usr/bin/env bash
+
+set -eux
+
+export ANSIBLE_ROLES_PATH=../
+
+ansible-playbook runme.yml "$@"
+
+source virtualenv.sh
+
+# Install Jinja < 2.10 since we want to test the fallback to Ansible's custom
+# unique filter. Jinja < 2.10 does not have do_unique so we will trigger the
+# fallback.
+pip install 'jinja2 < 2.10'
+
+# Run the playbook again in the venv with Jinja < 2.10
+ansible-playbook runme.yml "$@"
diff --git a/test/integration/targets/filter_mathstuff/runme.yml b/test/integration/targets/filter_mathstuff/runme.yml
new file mode 100644
index 00000000000000..a1eaef7a2a1f0b
--- /dev/null
+++ b/test/integration/targets/filter_mathstuff/runme.yml
@@ -0,0 +1,4 @@
+- hosts: localhost
+  gather_facts: false
+  roles:
+    - { role: filter_mathstuff }
diff --git a/test/integration/targets/filter_mathstuff/tasks/main.yml b/test/integration/targets/filter_mathstuff/tasks/main.yml
index 2a708be1d18406..93a65727f05677 100644
--- a/test/integration/targets/filter_mathstuff/tasks/main.yml
+++ b/test/integration/targets/filter_mathstuff/tasks/main.yml
@@ -1,6 +1,6 @@
-- name: Verify unique's fallback's exception throwing for case_sensitive=True
+- name: Verify unique's fallback's exception throwing for case_sensitive=False
   set_fact:
-    unique_fallback_exc1: '{{ [{"foo": "bar", "moo": "cow"}]|unique(case_sensitive=True) }}'
+    unique_fallback_exc1: '{{ [{"foo": "bar", "moo": "cow"}]|unique(case_sensitive=False) }}'
   ignore_errors: true
   tags: unique
   register: unique_fallback_exc1_res
@@ -67,6 +67,11 @@
       - '[1,2,3]|intersect([3,2,1]) == [1,2,3]'
       - '(1,2,3)|intersect((4,5,6))|list == []'
       - '(1,2,3)|intersect((3,4,5,6))|list == [3]'
+      - '["a","A","b"]|intersect(["B","c","C"]) == []'
+      - '["a","A","b"]|intersect(["b","B","c","C"]) == ["b"]'
+      - '["a","A","b"]|intersect(["b","A","a"]) == ["a","A","b"]'
+      - '("a","A","b")|intersect(("B","c","C"))|list == []'
+      - '("a","A","b")|intersect(("b","B","c","C"))|list == ["b"]'
 
 - name: Verify difference
   tags: difference
@@ -77,6 +82,11 @@
       - '[1,2,3]|difference([3,2,1]) == []'
       - '(1,2,3)|difference((4,5,6))|list == [1,2,3]'
       - '(1,2,3)|difference((3,4,5,6))|list == [1,2]'
+      - '["a","A","b"]|difference(["B","c","C"]) == ["a","A","b"]'
+      - '["a","A","b"]|difference(["b","B","c","C"]) == ["a","A"]'
+      - '["a","A","b"]|difference(["b","A","a"]) == []'
+      - '("a","A","b")|difference(("B","c","C"))|list|sort(case_sensitive=True) == ["A","a","b"]'
+      - '("a","A","b")|difference(("b","B","c","C"))|list|sort(case_sensitive=True) == ["A","a"]'
 
 - name: Verify symmetric_difference
   tags: symmetric_difference
@@ -87,6 +97,11 @@
       - '[1,2,3]|symmetric_difference([3,2,1]) == []'
       - '(1,2,3)|symmetric_difference((4,5,6))|list == [1,2,3,4,5,6]'
       - '(1,2,3)|symmetric_difference((3,4,5,6))|list == [1,2,4,5,6]'
+      - '["a","A","b"]|symmetric_difference(["B","c","C"]) == ["a","A","b","B","c","C"]'
+      - '["a","A","b"]|symmetric_difference(["b","B","c","C"]) == ["a","A","B","c","C"]'
+      - '["a","A","b"]|symmetric_difference(["b","A","a"]) == []'
+      - '("a","A","b")|symmetric_difference(("B","c","C"))|list|sort(case_sensitive=True) == ["A","B","C","a","b","c"]'
+      - '("a","A","b")|symmetric_difference(("b","B","c","C"))|list|sort(case_sensitive=True) == ["A","B","C","a","c"]'
 
 - name: Verify union
   tags: union
@@ -97,6 +112,11 @@
       - '[1,2,3]|union([3,2,1]) == [1,2,3]'
       - '(1,2,3)|union((4,5,6))|list == [1,2,3,4,5,6]'
       - '(1,2,3)|union((3,4,5,6))|list == [1,2,3,4,5,6]'
+      - '["a","A","b"]|union(["B","c","C"]) == ["a","A","b","B","c","C"]'
+      - '["a","A","b"]|union(["b","B","c","C"]) == ["a","A","b","B","c","C"]'
+      - '["a","A","b"]|union(["b","A","a"]) == ["a","A","b"]'
+      - '("a","A","b")|union(("B","c","C"))|list|sort(case_sensitive=True) == ["A","B","C","a","b","c"]'
+      - '("a","A","b")|union(("b","B","c","C"))|list|sort(case_sensitive=True) == ["A","B","C","a","b","c"]'
 
 - name: Verify min
   tags: min


Code review comments:
diff_hunk: @@ -47,6 +47,13 @@ Deprecated
         when: not teardown | bool
 
 
+Breaking Changes
+================
path: docs/docsite/rst/porting_guides/porting_guide_core_2.12.rst
text: @user1:
I'd just put this under the `Plugins` section instead of introducing a new section. Things in the porting guide are already expected to be breaking changes/things that need to be ported. :)

I'll fix this. Everything else looks reasonable to me.