Merge pull request #69892 from phlogistonjohn/jjm-teuth-remotectl

smb: add a teuthology test that exercises the gRPC over TCP interface of smb remote-control

Reviewed-by: Avan Thakkar <athakkar@redhat.com>
Reviewed-by: Rabinarayan Panigrahi <rapanigr@redhat.com>

qa/suites/orch/cephadm/smb/tasks/deploy_smb_mgr_res_remotectl.yaml

@@ -0,0 +1,152 @@
+roles:
+# Test is for basic smb deployment & functionality. one node cluster is OK
+- - host.a
+  - mon.a
+  - mgr.x
+  - osd.0
+  - osd.1
+  - client.0
+# Reserve a host for acting as a domain controller
+- - host.b
+  - cephadm.exclude
+overrides:
+  ceph:
+    log-only-match:
+      - CEPHADM_
+tasks:
+- openssl_keys:
+    rcroot:
+      client: client.0
+      cn: rcroot
+      key-type: rsa:4096
+    remote-control:
+      client: client.0
+      ca: rcroot
+      cn: remote-control
+      add-san: true
+    remote-control-client:
+      client: client.0
+      ca: rcroot
+      cn: remote-control-client
+      add-san: true
+- smb.deploy_samba_ad_dc:
+    role: host.b
+- cephadm:
+    single_host_defaults: true
+
+- cephadm.shell:
+    host.a:
+      - ceph fs volume create cephfs
+- cephadm.wait_for_service:
+    service: mds.cephfs
+
+- cephadm.shell:
+    host.a:
+      # add subvolgroup & subvolumes for test
+      - cmd: ceph fs subvolumegroup create cephfs smb
+      - cmd: ceph fs subvolume create cephfs sv1 --group-name=smb --mode=0777
+      - cmd: ceph fs subvolume create cephfs sv2 --group-name=smb --mode=0777
+      # set up smb cluster and shares
+      - cmd: ceph mgr module enable smb
+      # TODO: replace sleep with poll of mgr state?
+      - cmd: sleep 30
+      - cmd: ceph smb apply -i -
+        stdin: |
+          # --- Begin Embedded YAML
+          - resource_type: ceph.smb.tls.credential
+            tls_credential_id: rc-cert-1
+            credential_type: cert
+            value: |
+              {{'cert@remote-control'|openssl_content|indent(4)}}
+          - resource_type: ceph.smb.tls.credential
+            tls_credential_id: rc-key-1
+            credential_type: key
+            value: |
+              {{'key@remote-control'|openssl_content|indent(4)}}
+          - resource_type: ceph.smb.tls.credential
+            tls_credential_id: rc-ca-cert-1
+            credential_type: cert
+            value: |
+              {{'cert@rcroot'|openssl_content|indent(4)}}
+          - resource_type: ceph.smb.cluster
+            cluster_id: modtest1
+            auth_mode: active-directory
+            domain_settings:
+              realm: DOMAIN1.SINK.TEST
+              join_sources:
+                - source_type: resource
+                  ref: join1-admin
+            custom_dns:
+              - "{{ctx.samba_ad_dc_ip}}"
+            remote_control:
+              cert: {ref: rc-cert-1}
+              key: {ref: rc-key-1}
+              ca_cert: {ref: rc-ca-cert-1}
+            placement:
+              count: 1
+          - resource_type: ceph.smb.join.auth
+            auth_id: join1-admin
+            auth:
+              username: Administrator
+              password: Passw0rd
+          - resource_type: ceph.smb.share
+            cluster_id: modtest1
+            share_id: share1
+            cephfs:
+              volume: cephfs
+              subvolumegroup: smb
+              subvolume: sv1
+              path: /
+          - resource_type: ceph.smb.share
+            cluster_id: modtest1
+            share_id: share2
+            cephfs:
+              volume: cephfs
+              subvolumegroup: smb
+              subvolume: sv2
+              path: /
+          # --- End Embedded YAML
+# Wait for the smb service to start
+- cephadm.wait_for_service:
+    service: smb.modtest1
+# Check if shares exist
+- template.exec:
+    host.b:
+      - sleep 30
+      - "{{ctx.samba_client_container_cmd|join(' ')}} smbclient -U DOMAIN1\\\\ckent%1115Rose. //{{'host.a'|role_to_remote|attr('ip_address')}}/share1 -c ls"
+      - "{{ctx.samba_client_container_cmd|join(' ')}} smbclient -U DOMAIN1\\\\ckent%1115Rose. //{{'host.a'|role_to_remote|attr('ip_address')}}/share2 -c ls"
+
+- smb.workunit:
+    admin_node: host.a
+    smb_nodes: [host.a]
+    smb_shares:
+      - share1
+      - share2
+    timeout: 1h
+    clients:
+      client.0:
+        - [default, ceph_smb_ctl_remote]
+
+- cephadm.shell:
+    host.a:
+      - cmd: ceph smb apply -i -
+        stdin: |
+          # --- Begin Embedded YAML
+          - resource_type: ceph.smb.cluster
+            cluster_id: modtest1
+            intent: removed
+          - resource_type: ceph.smb.join.auth
+            auth_id: join1-admin
+            intent: removed
+          - resource_type: ceph.smb.share
+            cluster_id: modtest1
+            share_id: share1
+            intent: removed
+          - resource_type: ceph.smb.share
+            cluster_id: modtest1
+            share_id: share2
+            intent: removed
+          # --- End Embedded YAML
+# Wait for the smb service to be removed
+- cephadm.wait_for_service_not_present:
+    service: smb.modtest1

qa/tasks/smb.py

@@ -349,6 +349,7 @@ def _workunit_commands(
 def workunit(ctx, config):
     """Workunit wrapper with special behaviors for smb."""
     from . import workunit
+    from teuthology import misc
 
     _config = copy.deepcopy(config)
     clients = _config.get('clients') or {}
@@ -375,6 +376,7 @@ def workunit(ctx, config):
     )
     _ssh_keys_config = config.get('ssh_keys', {})
     _config['enable_ssh_keys'] = _ssh_keys_config not in (False, None)
+    _config['testdir'] = misc.get_testdir(ctx)
     log.info('Passing workunit config: %r', _config)
     with contextlib.ExitStack() as estack:
         if _config['enable_ssh_keys']:
@@ -415,6 +417,8 @@ def write_metadata_file(ctx, config, *, roles=None):
             _node_info(ctx, node, client_name=node)
             for node in config.get('clients')
         ]
+    if config.get('testdir'):
+        obj['testdir'] = config['testdir']
     data = json.dumps(obj)
     log.debug('smb metadata: %r', obj)
 

qa/tasks/template.py

@@ -79,6 +79,36 @@ def _role_to_remote(rctx, role):
     return None
 
 
+@jinja2.pass_context
+def _openssl_content(rctx, cert):
+    """Return SSL certificate contents managed by 'openssl_keys' role."""
+    ctx = rctx["ctx"]
+    ssl_certificates = getattr(ctx, 'ssl_certificates', None)
+    if not ssl_certificates:
+        log.info("No ssl_certificates object in context")
+        return None
+    what = 'cert'
+    if cert.startswith('key@'):
+        what = 'key'
+        cert = cert[4:]
+    elif cert.startswith('cert@'):
+        cert = cert[5:]
+
+    cert_obj = ssl_certificates.get(cert)
+    if not cert_obj:
+        log.info("No cert matching %r found", cert)
+        return None
+    if what == 'cert':
+        cfn = cert_obj.certificate
+    elif what == 'key':
+        cfn = cert_obj.key
+    else:
+        raise ValueError('invalid cert object')
+    content = cert_obj.remote.read_file(cfn)
+    log.info("Read %d from %s %s", len(content), what, cert)
+    return content.decode()
+
+
 def _vip_vars(rctx):
     """For backwards compat with the vip.subst_vip function."""
     # Make it possible to replace subst_vip in vip.py.
@@ -106,6 +136,7 @@ def transform(ctx, config, target, **ctx_vars):
         loader = jinja2.BaseLoader()
         jenv = jinja2.Environment(loader=loader)
         jenv.filters["role_to_remote"] = _role_to_remote
+        jenv.filters["openssl_content"] = _openssl_content
         setattr(ctx, "_jinja_env", jenv)
     rctx = dict(
         ctx=ctx, config=config, cluster_name=config.get("cluster", "")

qa/workunits/smb/smb_tests.sh

@@ -23,5 +23,5 @@ fi
 trap cleanup EXIT
 
 cd "${HERE}"
-"${VENV}/bin/${PY}" -m pip install pytest 'smbprotocol<1.17.0'
+"${VENV}/bin/${PY}" -m pip install pytest 'smbprotocol<1.17.0' grpcio grpcio-reflection
 "${VENV}/bin/${PY}" -m pytest -v "$@"

qa/workunits/smb/tests/cephutil.py

@@ -56,6 +56,9 @@ def cephadm_shell_cmd(
         f'/home/{smb_cfg.ssh_user}/cephtest/cephadm',
         'shell',
     ]
+    volumes = kwargs.pop('volumes', [])
+    for v in volumes:
+        cmd.extend(['-v', v])
     cmd += list(args)
     proc = subprocess.run(cmd, **kwargs)
     if load is LoadJSON.BOTH:

qa/workunits/smb/tests/pytest.ini

@@ -6,4 +6,5 @@ markers =
     hosts_access: Host access tests
     rate_limiting: Rate limit tests
     ceph_smb_ctl_local: Local/container test of ceph-smb-ctl tool
+    ceph_smb_ctl_remote: Remote access test of ceph-smb-ctl tool
     domain: Domain integration tests

qa/workunits/smb/tests/smbutil.py

@@ -1,5 +1,6 @@
 import base64
 import contextlib
+import os
 import pathlib
 import time
 
@@ -89,6 +90,10 @@ class SMBTestConf:
         # it for now until we really need to check
         return self.clients()[0]
 
+    @property
+    def testdir(self):
+        return self._data.get('testdir') or os.path.expanduser('~/cephtest')
+
 
 @contextlib.contextmanager
 def connection(conf, share):
@@ -190,7 +195,7 @@ def get_share_by_id(smb_cfg, cluster_id, share_id):
     return None
 
 
-def apply_share_config(smb_cfg, share):
+def apply_share_config(smb_cfg, share, immediate=False):
     """Apply share configuration via the apply command."""
     jres = cephutil.cephadm_shell_cmd(
         smb_cfg,
@@ -209,5 +214,6 @@ def apply_share_config(smb_cfg, share):
     assert resources_ret['resource_type'] == 'ceph.smb.share'
     # sleep to ensure the settings got applied in smbd
     # TODO: make this more dynamic somehow
-    time.sleep(60)
+    if not immediate:
+        time.sleep(60)
     return resources_ret

qa/workunits/smb/tests/test_ceph_smb_ctl.py

@@ -1,3 +1,4 @@
+import pathlib
 import time
 
 import pytest
@@ -8,14 +9,12 @@ import smbutil
 CEPH_SMB_CTL = 'ceph-smb-ctl'
 
 
-@pytest.mark.ceph_smb_ctl_local
-class TestCephSMBCtlLocal:
+class RemoteCtlBase:
+    def _rcontrol(self, smb_cfg, args, **kwargs):
+        raise NotImplementedError()
+
     def test_get_info(self, smb_cfg):
-        jres = cephutil.cephadm_shell_cmd(
-            smb_cfg,
-            [CEPH_SMB_CTL, 'info'],
-            load_json=True,
-        )
+        jres = self._rcontrol(smb_cfg, ['info'], load_json=True)
         assert jres.returncode == 0
         assert jres.obj
         assert 'samba_info' in jres.obj
@@ -23,11 +22,7 @@ class TestCephSMBCtlLocal:
         assert 'clustered' in jres.obj['samba_info']
 
     def test_status_empty(self, smb_cfg):
-        jres = cephutil.cephadm_shell_cmd(
-            smb_cfg,
-            [CEPH_SMB_CTL, 'status'],
-            load_json=True,
-        )
+        jres = self._rcontrol(smb_cfg, ['status'], load_json=True)
         assert jres.returncode == 0
         assert jres.obj
         assert 'server_timestamp' in jres.obj
@@ -41,11 +36,7 @@ class TestCephSMBCtlLocal:
         with smbutil.connection(smb_cfg, share_name) as sharep:
             sharep.listdir()  # trigger a tree connect in client lib
             time.sleep(0.2)
-            jres = cephutil.cephadm_shell_cmd(
-                smb_cfg,
-                [CEPH_SMB_CTL, 'status'],
-                load_json=True,
-            )
+            jres = self._rcontrol(smb_cfg, ['status'], load_json=True)
         assert jres.returncode == 0
         assert jres.obj
         assert 'server_timestamp' in jres.obj
@@ -60,28 +51,23 @@ class TestCephSMBCtlLocal:
 
     def test_config_dump_samba(self, smb_cfg):
         share_name = smbutil.get_shares(smb_cfg)[0]['name']
-        res = cephutil.cephadm_shell_cmd(
-            smb_cfg,
-            [CEPH_SMB_CTL, 'config-dump', 'samba'],
-            capture_output=True,
-            text=True,
+        res = self._rcontrol(
+            smb_cfg, ['config-dump', 'samba'], capture_output=True, text=True
         )
         assert res.returncode == 0
         assert f'[{share_name}]' in res.stdout
 
     def test_config_dump_sambacc(self, smb_cfg):
-        jres = cephutil.cephadm_shell_cmd(
-            smb_cfg,
-            [CEPH_SMB_CTL, 'config-dump', 'sambacc'],
-            load_json=True,
+        jres = self._rcontrol(
+            smb_cfg, ['config-dump', 'sambacc'], load_json=True
         )
         assert jres.returncode == 0
         assert jres.obj
 
     def test_config_dump_cmp_hash(self, smb_cfg):
-        res = cephutil.cephadm_shell_cmd(
+        res = self._rcontrol(
             smb_cfg,
-            [CEPH_SMB_CTL, 'config-dump', '--sha256', 'samba'],
+            ['config-dump', '--sha256', 'samba'],
             capture_output=True,
             text=True,
         )
@@ -93,9 +79,9 @@ class TestCephSMBCtlLocal:
         alg, digest = parts[-1].split(':', 1)
         assert alg == 'sha256'
 
-        jres = cephutil.cephadm_shell_cmd(
+        jres = self._rcontrol(
             smb_cfg,
-            [CEPH_SMB_CTL, 'config-summary', '--sha256', 'samba'],
+            ['config-summary', '--sha256', 'samba'],
             load_json=True,
         )
         assert jres.returncode == 0
@@ -104,9 +90,9 @@ class TestCephSMBCtlLocal:
         assert jres.obj['digest']['config_digest'] == digest
 
     def test_config_shares_list(self, smb_cfg):
-        jres = cephutil.cephadm_shell_cmd(
+        jres = self._rcontrol(
             smb_cfg,
-            [CEPH_SMB_CTL, 'config-shares-list', 'samba'],
+            ['config-shares-list', 'samba'],
             load_json=True,
         )
         assert jres.returncode == 0
@@ -118,43 +104,182 @@ class TestCephSMBCtlLocal:
         # the shares list in sambacc should always map exactly to the
         # shares list in samba itself
         prev_names = jres.obj
-        jres = cephutil.cephadm_shell_cmd(
+        jres = self._rcontrol(
             smb_cfg,
-            [CEPH_SMB_CTL, 'config-shares-list', 'sambacc'],
+            ['config-shares-list', 'sambacc'],
             load_json=True,
         )
         assert jres.returncode == 0
         assert sorted(jres.obj) == sorted(prev_names)
 
     def test_debug_get_set(self, smb_cfg):
-        jres = cephutil.cephadm_shell_cmd(
+        jres = self._rcontrol(
             smb_cfg,
-            [CEPH_SMB_CTL, 'get-debug-level', 'smb'],
+            ['get-debug-level', 'smb'],
             load_json=True,
         )
         assert jres.returncode == 0, "get-debug-level smb failed"
         assert "debug_level" in jres.obj
         orig_debug_level = jres.obj["debug_level"]
 
-        jres = cephutil.cephadm_shell_cmd(
+        jres = self._rcontrol(
             smb_cfg,
-            [CEPH_SMB_CTL, 'set-debug-level', 'smb', "10"],
+            ['set-debug-level', 'smb', "10"],
             load_json=True,
         )
         assert jres.returncode == 0, "set-debug-level smb 10 failed"
 
-        jres = cephutil.cephadm_shell_cmd(
+        jres = self._rcontrol(
             smb_cfg,
-            [CEPH_SMB_CTL, 'get-debug-level', 'smb'],
+            ['get-debug-level', 'smb'],
             load_json=True,
         )
         assert jres.returncode == 0, "get-debug-level smb failed"
         assert "debug_level" in jres.obj
         assert jres.obj["debug_level"] == "10"
 
-        jres = cephutil.cephadm_shell_cmd(
+        jres = self._rcontrol(
             smb_cfg,
-            [CEPH_SMB_CTL, 'set-debug-level', 'smb', orig_debug_level],
+            ['set-debug-level', 'smb', orig_debug_level],
             load_json=True,
         )
         assert jres.returncode == 0, "set-debug-level smb orig level failed"
+
+
+@pytest.mark.ceph_smb_ctl_local
+class TestCephSMBCtlLocal(RemoteCtlBase):
+    def _rcontrol(self, smb_cfg, args, **kwargs):
+        return cephutil.cephadm_shell_cmd(
+            smb_cfg,
+            [CEPH_SMB_CTL] + args,
+            **kwargs,
+        )
+
+
+@pytest.mark.ceph_smb_ctl_remote
+class TestCephSMBCtlRemote(RemoteCtlBase):
+    def _rcontrol(self, smb_cfg, args, **kwargs):
+        grpc_host = f"{smb_cfg.server.ip_address}:54445"
+        ca_dir = pathlib.Path(smb_cfg.testdir) / 'ca'
+        ca_mnt = pathlib.Path('/tls')
+        assert (ca_dir / 'remote-control-client.crt').is_file()
+        assert (ca_dir / 'remote-control-client.key').is_file()
+        assert (ca_dir / 'rcroot.crt').is_file()
+        _args = [
+            CEPH_SMB_CTL,
+            f'--address={grpc_host}',
+            f"--tls-cert={ca_mnt}/remote-control-client.crt",
+            f"--tls-key={ca_mnt}/remote-control-client.key",
+            f"--tls-ca-cert={ca_mnt}/rcroot.crt",
+        ]
+        return cephutil.cephadm_shell_cmd(
+            smb_cfg,
+            _args + args,
+            volumes=[f'{ca_dir}:{ca_mnt}:ro'],
+            **kwargs,
+        )
+
+
+def _get_obj(value):
+    return value.to_simplified()
+
+
+@pytest.mark.ceph_smb_ctl_remote
+class TestCephSMBCtlAPI:
+    def _client(self, smb_cfg):
+        # alter paths to include python-common
+        import sys
+
+        curr = pathlib.Path('.').absolute()
+        while curr.parent != pathlib.Path('/'):
+            pcomm = curr / 'src/python-common'
+            if pcomm.is_dir():
+                sys.path.append(str(pcomm))
+                break
+            curr = curr.parent
+
+        # import the needed packges
+        try:
+            import ceph.smb.ctl.client as rclient
+            import ceph.smb.ctl.config as rconfig
+        except ImportError:
+            pytest.skip('failed to import ceph.smb.ctl.client OR dependency')
+
+        # set up a grpc client w/in the test
+        grpc_host = f"{smb_cfg.server.ip_address}:54445"
+        ca_dir = pathlib.Path(smb_cfg.testdir) / 'ca'
+        tls_cert = ca_dir / 'remote-control-client.crt'
+        tls_key = ca_dir / 'remote-control-client.key'
+        tls_ca_cert = ca_dir / 'rcroot.crt'
+        assert tls_cert.is_file()
+        assert tls_key.is_file()
+        assert tls_ca_cert.is_file()
+
+        cc = rconfig.Config(
+            address=grpc_host,
+            channel_type=rconfig.ChannelType.SECURE,
+            tls_cert=rconfig.TLSPath.create(tls_cert),
+            tls_key=rconfig.TLSPath.create(tls_key),
+            tls_ca_cert=rconfig.TLSPath.create(tls_ca_cert),
+        )
+        return rclient.Client(cc)
+
+    def test_get_info(self, smb_cfg):
+        obj = _get_obj(self._client(smb_cfg).info())
+        assert 'samba_info' in obj
+        assert 'version' in obj['samba_info']
+        assert 'clustered' in obj['samba_info']
+
+    def test_status_empty(self, smb_cfg):
+        obj = _get_obj(self._client(smb_cfg).status())
+        assert 'server_timestamp' in obj
+        assert 'sessions' in obj
+        assert 'tree_connections' in obj
+        assert not obj['sessions']
+        assert not obj['tree_connections']
+
+    def test_status_connected(self, smb_cfg):
+        share_name = smbutil.get_shares(smb_cfg)[0]['name']
+        with smbutil.connection(smb_cfg, share_name) as sharep:
+            sharep.listdir()  # trigger a tree connect in client lib
+            time.sleep(0.2)
+            obj = _get_obj(self._client(smb_cfg).status())
+        assert obj
+        assert 'server_timestamp' in obj
+        assert 'sessions' in obj
+        assert 'tree_connections' in obj
+        assert len(obj['sessions']) == 1
+        assert len(obj['tree_connections']) == 1
+        assert (
+            obj['sessions'][0]['session_id']
+            == obj['tree_connections'][0]['session_id']
+        )
+
+    def test_config_summary_change_poll(self, smb_cfg):
+        source = 'CONFIG_FOR_SAMBA'
+        obj = _get_obj(self._client(smb_cfg).config_summary(source))
+        assert 'digest' in obj
+        assert 'config_digest' in obj['digest']
+        prev_digest = obj['digest']['config_digest']
+        time.sleep(0.2)
+
+        # no change
+        obj = _get_obj(self._client(smb_cfg).config_summary(source))
+        assert prev_digest == obj['digest']['config_digest']
+
+        # change
+        share_cfg = smbutil.get_shares(smb_cfg)[0]
+        orig_comment = share_cfg.get('comment', '')
+        share_cfg['comment'] = 'Foo bar baz asdf bar baz foo'
+        assert share_cfg['comment'] != orig_comment
+        smbutil.apply_share_config(smb_cfg, share_cfg, immediate=True)
+
+        changed = False
+        for _ in range(0, 150):
+            time.sleep(0.5)
+            obj = _get_obj(self._client(smb_cfg).config_summary(source))
+            changed = prev_digest != obj['digest']['config_digest']
+            if changed:
+                break
+
+        assert changed, "config digest never changed"