ipa_otptoken: fix wrong return value string to bool (#7795)

ipa_data is return ipatokendisable in boolean format and the module expects it as a string this behavior causes a lack of idempotency and the get_diff module will fail in the second run.
ansible-collections · Jan 16, 2024 · 31de16c · 31de16c
1 parent 32ec751
commit 31de16c
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 12 deletions.
diff --git a/changelogs/fragments/7797-ipa-fix-otp-idempotency.yml b/changelogs/fragments/7797-ipa-fix-otp-idempotency.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - ipa_otptoken - the module expect ``ipatokendisabled`` as string but the ``ipatokendisabled`` value is returned as a boolean (https://github.com/ansible-collections/community.general/pull/7795).
diff --git a/plugins/modules/ipa_otptoken.py b/plugins/modules/ipa_otptoken.py
@@ -237,7 +237,7 @@ def get_otptoken_dict(ansible_to_ipa, uniqueid=None, newuniqueid=None, otptype=N
     if owner is not None:
         otptoken[ansible_to_ipa['owner']] = owner
     if enabled is not None:
-        otptoken[ansible_to_ipa['enabled']] = 'FALSE' if enabled else 'TRUE'
+        otptoken[ansible_to_ipa['enabled']] = False if enabled else True
     if notbefore is not None:
         otptoken[ansible_to_ipa['notbefore']] = notbefore + 'Z'
     if notafter is not None:

diff --git a/tests/unit/plugins/modules/test_ipa_otptoken.py b/tests/unit/plugins/modules/test_ipa_otptoken.py
@@ -104,7 +104,7 @@ def test_add_new_all_default(self):
             {
                 'method': 'otptoken_add',
                 'name': 'NewToken1',
-                'item': {'ipatokendisabled': 'FALSE',
+                'item': {'ipatokendisabled': False,
                          'all': True}
             }
         )
@@ -130,7 +130,7 @@ def test_add_new_all_default_with_aliases(self):
             {
                 'method': 'otptoken_add',
                 'name': 'NewToken1',
-                'item': {'ipatokendisabled': 'FALSE',
+                'item': {'ipatokendisabled': False,
                          'all': True}
             }
         )
@@ -176,7 +176,7 @@ def test_add_new_all_specified(self):
                          'ipatokenotpkey': 'KRSXG5CTMVRXEZLUGE======',
                          'description': 'Test description',
                          'ipatokenowner': 'pinky',
-                         'ipatokendisabled': 'FALSE',
+                         'ipatokendisabled': False,
                          'ipatokennotbefore': '20200101010101Z',
                          'ipatokennotafter': '20900101010101Z',
                          'ipatokenvendor': 'Acme',
@@ -220,7 +220,7 @@ def test_already_existing_no_change_all_specified(self):
                         'ipatokenotpkey': [{'__base64__': 'VGVzdFNlY3JldDE='}],
                         'description': ['Test description'],
                         'ipatokenowner': ['pinky'],
-                        'ipatokendisabled': ['FALSE'],
+                        'ipatokendisabled': [False],
                         'ipatokennotbefore': ['20200101010101Z'],
                         'ipatokennotafter': ['20900101010101Z'],
                         'ipatokenvendor': ['Acme'],
@@ -271,7 +271,7 @@ def test_already_existing_one_change_all_specified(self):
                         'ipatokenotpkey': [{'__base64__': 'VGVzdFNlY3JldDE='}],
                         'description': ['Test description'],
                         'ipatokenowner': ['pinky'],
-                        'ipatokendisabled': ['FALSE'],
+                        'ipatokendisabled': [False],
                         'ipatokennotbefore': ['20200101010101Z'],
                         'ipatokennotafter': ['20900101010101Z'],
                         'ipatokenvendor': ['Acme'],
@@ -296,7 +296,7 @@ def test_already_existing_one_change_all_specified(self):
                 'name': 'NewToken1',
                 'item': {'description': 'Test description',
                          'ipatokenowner': 'brain',
-                         'ipatokendisabled': 'FALSE',
+                         'ipatokendisabled': False,
                          'ipatokennotbefore': '20200101010101Z',
                          'ipatokennotafter': '20900101010101Z',
                          'ipatokenvendor': 'Acme',
@@ -335,7 +335,7 @@ def test_already_existing_all_valid_change_all_specified(self):
                         'ipatokenotpkey': [{'__base64__': 'VGVzdFNlY3JldDE='}],
                         'description': ['Test description'],
                         'ipatokenowner': ['pinky'],
-                        'ipatokendisabled': ['FALSE'],
+                        'ipatokendisabled': [False],
                         'ipatokennotbefore': ['20200101010101Z'],
                         'ipatokennotafter': ['20900101010101Z'],
                         'ipatokenvendor': ['Acme'],
@@ -360,7 +360,7 @@ def test_already_existing_all_valid_change_all_specified(self):
                 'name': 'NewToken1',
                 'item': {'description': 'New Test description',
                          'ipatokenowner': 'pinky',
-                         'ipatokendisabled': 'TRUE',
+                         'ipatokendisabled': True,
                          'ipatokennotbefore': '20200101010102Z',
                          'ipatokennotafter': '20900101010102Z',
                          'ipatokenvendor': 'NewAcme',
@@ -384,7 +384,7 @@ def test_delete_existing_token(self):
                         'ipatokenotpkey': [{'__base64__': 'KRSXG5CTMVRXEZLUGE======'}],
                         'description': ['Test description'],
                         'ipatokenowner': ['pinky'],
-                        'ipatokendisabled': ['FALSE'],
+                        'ipatokendisabled': [False],
                         'ipatokennotbefore': ['20200101010101Z'],
                         'ipatokennotafter': ['20900101010101Z'],
                         'ipatokenvendor': ['Acme'],
@@ -425,7 +425,7 @@ def test_disable_existing_token(self):
                         'ipatokenotpkey': [{'__base64__': 'KRSXG5CTMVRXEZLUGE======'}],
                         'description': ['Test description'],
                         'ipatokenowner': ['pinky'],
-                        'ipatokendisabled': ['FALSE'],
+                        'ipatokendisabled': [False],
                         'ipatokennotbefore': ['20200101010101Z'],
                         'ipatokennotafter': ['20900101010101Z'],
                         'ipatokenvendor': ['Acme'],
@@ -448,7 +448,7 @@ def test_disable_existing_token(self):
             {
                 'method': 'otptoken_mod',
                 'name': 'NewToken1',
-                'item': {'ipatokendisabled': 'TRUE',
+                'item': {'ipatokendisabled': True,
                           'all': True}
             }
         )
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		bugfixes:
		- ipa_otptoken - the module expect ``ipatokendisabled`` as string but the ``ipatokendisabled`` value is returned as a boolean (https://github.com/ansible-collections/community.general/pull/7795).