From f43831407b6de4866286dc24c496b6822100c15b Mon Sep 17 00:00:00 2001
From: Rafael Guterres Jeffman <rjeffman@redhat.com>
Date: Mon, 25 Oct 2021 12:31:24 -0300
Subject: [PATCH] ipaservice: Fix idempotent behavior for principal aliases.
When creating the lists to add/remove principal aliases, if the realm
was not specified, the alias would be used as it did not matched the
existing one, which has the realm part.
This patch fixes the add/del list creation by adding the current API
realm to each alias that does not have the realm part and then use
this modified list to be compared against the existing principal list.
This change also allows the use of the whole list in a single call to
the IPA API to add/remove the principals, instead of a call for every
one item in the list.
---
plugins/modules/ipaservice.py | 63 ++++++----
.../test_service_without_skip_host_check.yml | 112 ++++++++++++++++++
2 files changed, 151 insertions(+), 24 deletions(-)
diff --git a/plugins/modules/ipaservice.py b/plugins/modules/ipaservice.py
index b69a95cb..3f30bd23 100644
--- a/plugins/modules/ipaservice.py
+++ b/plugins/modules/ipaservice.py
@@ -224,7 +224,8 @@ RETURN = """
from ansible.module_utils.ansible_freeipa_module import \
IPAAnsibleModule, compare_args_ipa, encode_certificate, \
- gen_add_del_lists, ipalib_errors
+ gen_add_del_lists, gen_add_list, gen_intersection_list, ipalib_errors, \
+ api_get_realm, to_text
def find_service(module, name):
@@ -492,6 +493,30 @@ def main():
for name in names:
res_find = find_service(ansible_module, name)
+ res_principals = []
+
+ if principal and res_find:
+ # When comparing principals to the existing ones,
+ # the REALM is needded, and are added here for those
+ # that do not have it.
+ principal = [
+ p if "@" in p
+ else "%s@%s" % (p, api_get_realm())
+ for p in principal
+ ]
+ principal = list(set(principal))
+
+ # Create list of existing principal aliases as strings
+ # to compare with provided ones.
+ canonicalname = {
+ to_text(p)
+ for p in res_find.get("krbcanonicalname", [])
+ }
+ res_principals = [
+ to_text(elem)
+ for elem in res_find.get("krbprincipalname", [])
+ ]
+ res_principals = list(set(res_principals) - canonicalname)
if state == "present":
if action == "service":
@@ -576,8 +601,8 @@ def main():
host_add, host_del = gen_add_del_lists(
host, res_find.get('managedby_host', []))
- principal_add, principal_del = gen_add_del_lists(
- principal, res_find.get("principal"))
+ principal_add, principal_del = \
+ gen_add_del_lists(principal, res_principals)
(allow_create_keytab_user_add,
allow_create_keytab_user_del) = \
@@ -646,7 +671,7 @@ def main():
certificate_del = []
host_add = host or []
host_del = []
- principal_add = principal or []
+ principal_add = gen_add_list(principal, res_principals)
principal_del = []
allow_create_keytab_user_add = \
@@ -674,21 +699,12 @@ def main():
allow_retrieve_keytab_hostgroup or []
allow_retrieve_keytab_hostgroup_del = []
- # Add principals
- for _principal in principal_add:
+ if principal_add:
commands.append([name, "service_add_principal",
- {
- "krbprincipalname":
- _principal,
- }])
-
- # Remove principals
- for _principal in principal_del:
+ {"krbprincipalname": principal_add}])
+ if principal_del:
commands.append([name, "service_remove_principal",
- {
- "krbprincipalname":
- _principal,
- }])
+ {"krbprincipalname": principal_del}])
for _certificate in certificate_add:
commands.append([name, "service_add_cert",
@@ -776,13 +792,12 @@ def main():
ansible_module.fail_json(msg="No service '%s'" % name)
# Remove principals
- if principal is not None:
- for _principal in principal:
- commands.append([name, "service_remove_principal",
- {
- "krbprincipalname":
- _principal,
- }])
+ principal_del = gen_intersection_list(
+ principal, res_principals)
+ if principal_del:
+ commands.append([name, "service_remove_principal",
+ {"krbprincipalname": principal_del}])
+
# Remove certificates
if certificate is not None:
existing = res_find.get('usercertificate', [])
diff --git a/tests/service/test_service_without_skip_host_check.yml b/tests/service/test_service_without_skip_host_check.yml
index 0dc0fe45..763a56aa 100644
--- a/tests/service/test_service_without_skip_host_check.yml
+++ b/tests/service/test_service_without_skip_host_check.yml
@@ -347,6 +347,118 @@
register: result
failed_when: result.changed or result.failed
+ # tests for upstream issue #663
+ - name: Ensure service is present with principal alias.
+ ipaservice:
+ ipaadmin_password: SomeADMINpassword
+ name: "mysvc/{{ host1_fqdn }}"
+ principal: "asvc/{{ host1_fqdn }}"
+ register: result
+ failed_when: result.failed or not result.changed
+
+ - name: Ensure service is present with principal alias, again.
+ ipaservice:
+ ipaadmin_password: SomeADMINpassword
+ name: "mysvc/{{ host1_fqdn }}"
+ principal: "asvc/{{ host1_fqdn }}"
+ register: result
+ failed_when: result.failed or result.changed
+
+ - name: Ensure service is present with different principal alias.
+ ipaservice:
+ ipaadmin_password: SomeADMINpassword
+ name: "mysvc/{{ host1_fqdn }}"
+ principal: "HTTP/{{ host1_fqdn }}"
+ force: yes
+ register: result
+ failed_when: result.failed or not result.changed
+
+ - name: Ensure service is presennt with different principal alias, again.
+ ipaservice:
+ ipaadmin_password: SomeADMINpassword
+ name: "mysvc/{{ host1_fqdn }}"
+ principal: "HTTP/{{ host1_fqdn }}"
+ force: yes
+ register: result
+ failed_when: result.failed or result.changed
+
+ - name: Ensure service member principal alias is present.
+ ipaservice:
+ ipaadmin_password: SomeADMINpassword
+ name: "mysvc/{{ host1_fqdn }}"
+ principal: "asvc/{{ host1_fqdn }}"
+ action: member
+ register: result
+ failed_when: result.failed or not result.changed
+
+ - name: Ensure service member principal alias is present, again.
+ ipaservice:
+ ipaadmin_password: SomeADMINpassword
+ name: "mysvc/{{ host1_fqdn }}"
+ principal: "asvc/{{ host1_fqdn }}"
+ action: member
+ register: result
+ failed_when: result.failed or result.changed
+
+ - name: Ensure service member principal alias is absent.
+ ipaservice:
+ ipaadmin_password: SomeADMINpassword
+ name: "mysvc/{{ host1_fqdn }}"
+ principal: "asvc/{{ host1_fqdn }}"
+ action: member
+ state: absent
+ register: result
+ failed_when: result.failed or not result.changed
+
+ - name: Ensure service member principal alias is absent, again.
+ ipaservice:
+ ipaadmin_password: SomeADMINpassword
+ name: "mysvc/{{ host1_fqdn }}"
+ principal: "asvc/{{ host1_fqdn }}"
+ action: member
+ state: absent
+ register: result
+ failed_when: result.failed or result.changed
+
+ - name: Ensure service is present with multiple principal aliases.
+ ipaservice:
+ ipaadmin_password: SomeADMINpassword
+ name: "mysvc/{{ host1_fqdn }}"
+ principal:
+ - "HTTP/{{ host1_fqdn }}"
+ - "asvc/{{ host1_fqdn }}"
+ register: result
+ failed_when: result.failed or not result.changed
+
+ - name: Ensure service is present with multiple principal aliases, again.
+ ipaservice:
+ ipaadmin_password: SomeADMINpassword
+ name: "mysvc/{{ host1_fqdn }}"
+ principal:
+ - "HTTP/{{ host1_fqdn }}"
+ - "asvc/{{ host1_fqdn }}"
+ register: result
+ failed_when: result.failed or result.changed
+
+ - name: Ensure service is with multiple principal aliases is absent.
+ ipaservice:
+ ipaadmin_password: SomeADMINpassword
+ name: "mysvc/{{ host1_fqdn }}"
+ continue: yes
+ state: absent
+ register: result
+ failed_when: result.failed or not result.changed
+
+ - name: Ensure service is with multiple principal aliases is absent, again.
+ ipaservice:
+ ipaadmin_password: SomeADMINpassword
+ name: "mysvc/{{ host1_fqdn }}"
+ continue: yes
+ state: absent
+ register: result
+ failed_when: result.failed or result.changed
+ # end of tests for upstream issue #663
+
# cleanup
- name: Cleanup test environment
include_tasks: env_cleanup.yml
--
GitLab