ipaservice: Handle smb services as other services.

In current implementation, when using `smb: yes`, only a small subset
of the attributes can be used in the playbook. This happened due the
use of `service_add_smb`, which adds a new service and does not modify
an existing one, and not coping with attributes not supported by this
IPA API call.

The implementation was modified so that a service with `smb: true` is
treated like any other service, which, in effect, simplified and fixed
service search, and allowed for the use of the same attributes as with
any service. Although simplified, when using `smb: true` an extra
query is done against the LDAP server, as a second `service_show` is
performed.

Tests have been updated to reflect the new imprlementation.

README-service.md

@@ -311,6 +311,8 @@ Variable | Description | Required
 `allow_retrieve_keytab_host` \| `ipaallowedtoperform_read_keys_host` | Hosts allowed to retrieve a keytab from of host. | no
 `allow_retrieve_keytab_hostgroup` \| `ipaallowedtoperform_read_keys_hostgroup` | Host groups allowed to retrieve a keytab of this host. | no
 `continue` | Continuous mode: don't stop on errors. Valid only if `state` is `absent`. Default: `no` (bool) | no
+`smb` | Service is an SMB service. If set, `cifs/` will be prefixed to the service name if needed. | no
+`netbiosname` | NETBIOS name for the SMB service. Only with `smb: yes`. | no
 `action` | Work on service or member level. It can be on of `member` or `service` and defaults to `service`. | no
 `state` | The state to ensure. It can be one of `present`, `absent`, or `disabled`, default: `present`. | no
 

plugins/modules/ipaservice.py

@@ -91,7 +91,7 @@ options:
     type: list
     aliases: ["krbprincipalname"]
   smb:
-    description: Add a SMB service. Can only be used with new services.
+    description: Add a SMB service.
     required: false
     type: bool
   netbiosname:
@@ -234,21 +234,11 @@ from ansible.module_utils.ansible_freeipa_module import temp_kinit, \
 import ipalib.errors
 
 
-def find_service(module, name, netbiosname):
+def find_service(module, name):
     _args = {
         "all": True,
     }
 
-    # Search for a SMB/cifs service.
-    if netbiosname is not None:
-        _result = api_command(
-            module, "service_find", to_text(netbiosname), _args)
-
-        for _res_find in _result.get('result', []):
-            for uid in _res_find.get('uid', []):
-                if uid.startswith("%s$@" % netbiosname):
-                    return _res_find
-
     try:
         _result = api_command(module, "service_show", to_text(name), _args)
     except ipalib.errors.NotFound:
@@ -287,6 +277,19 @@ def gen_args(pac_type, auth_ind, skip_host_check, force, requires_pre_auth,
     return _args
 
 
+def gen_args_smb(netbiosname, ok_as_delegate, ok_to_auth_as_delegate):
+    _args = {}
+
+    if netbiosname is not None:
+        _args['ipantflatname'] = netbiosname
+    if ok_as_delegate is not None:
+        _args['ipakrbokasdelegate'] = (ok_as_delegate)
+    if ok_to_auth_as_delegate is not None:
+        _args['ipakrboktoauthasdelegate'] = (ok_to_auth_as_delegate)
+
+    return _args
+
+
 def check_parameters(module, state, action, names, parameters):
     assert isinstance(parameters, dict)
 
@@ -310,15 +313,13 @@ def check_parameters(module, state, action, names, parameters):
         if action == 'service':
             invalid = ['delete_continue']
 
-            if parameters.get('smb', False):
-                invalid.extend(['force', 'auth_ind', 'skip_host_check',
-                                'requires_pre_auth', 'auth_ind', 'pac_type'])
-
-                for _invalid in invalid:
-                    if parameters.get(_invalid, False):
+            if (
+                not parameters.get('smb', False)
+                and parameters.get('netbiosname')
+            ):
                 module.fail_json(
-                            msg="Argument '%s' can not be used with SMB "
-                                "service." % _invalid)
+                    msg="Argument 'netbiosname' can not be used without "
+                        "SMB service.")
         else:
             invalid.append('delete_continue')
 
@@ -494,11 +495,9 @@ def main():
         commands = []
 
         for name in names:
-            res_find = find_service(ansible_module, name, netbiosname)
+            res_find = find_service(ansible_module, name)
 
             if state == "present":
-                # if service exists, 'smb' cannot be used.
-
                 if action == "service":
                     args = gen_args(
                         pac_type, auth_ind, skip_host_check, force,
@@ -507,12 +506,23 @@ def main():
                     if not has_skip_host_check and 'skip_host_check' in args:
                         del args['skip_host_check']
 
-                    if res_find is None:
                     if smb:
-                            if netbiosname is not None:
-                                args['ipantflatname'] = netbiosname
-                            commands.append([name, 'service_add_smb', args])
-                        else:
+                        if res_find is None:
+                            _name = "cifs/" + name
+                            res_find = find_service(ansible_module, _name)
+                            if res_find is None:
+                                _args = gen_args_smb(
+                                    netbiosname, ok_as_delegate,
+                                    ok_to_auth_as_delegate)
+                                commands.append(
+                                    [name, 'service_add_smb', _args])
+                                res_find = {}
+                            # service_add_smb will prefix 'name' with
+                            # "cifs/", so we will need to change it here,
+                            # so that service_mod, if called later, works.
+                            name = _name
+
+                    if res_find is None:
                         commands.append([name, 'service_add', args])
 
                         certificate_add = certificate or []

tests/service/test_service.yml

@@ -469,17 +469,49 @@
         ipaservice:
           ipaadmin_password: SomeADMINpassword
           name: "{{ host1_fqdn }}"
+          pac_type: NONE
           smb: yes
           netbiosname: SAMBASVC
         register: result
         failed_when: not result.changed or result.failed
 
       - name: Ensure SMB service is again.
+        ipaservice:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ host1_fqdn }}"
+          pac_type: NONE
+          smb: yes
+          netbiosname: SAMBASVC
+        register: result
+        failed_when: result.changed or result.failed
+
+      - name: Modify SMB service.
+        ipaservice:
+          ipaadmin_password: SomeADMINpassword
+          name: "{{ host1_fqdn }}"
+          smb: yes
+          netbiosname: SAMBASVC
+          allow_retrieve_keytab_user:
+            - user01
+            - user02
+          allow_retrieve_keytab_group:
+            - group01
+            - group02
+        register: result
+        failed_when: not result.changed or result.failed
+
+      - name: Modify SMB service, again.
         ipaservice:
           ipaadmin_password: SomeADMINpassword
           name: "{{ host1_fqdn }}"
           smb: yes
           netbiosname: SAMBASVC
+          allow_retrieve_keytab_user:
+            - user01
+            - user02
+          allow_retrieve_keytab_group:
+            - group01
+            - group02
         register: result
         failed_when: result.changed or result.failed