[TESTS] Update tests and add test for generate_artifacts (#311)

* [tests] use tmp_path instead of deprecated tmpdir
* cover some error cases
* [test] test cli displays help if verb missing
* add some fixme in the code
* exclude setup.py from coverage reporting
* add high lever testing for generate_artifacts verb

Signed-off-by: Mikael Arguedas <mikael.arguedas@gmail.com>

sros2/.coveragerc

@@ -2,3 +2,4 @@
 omit =
     # omit test directory
     test/*
+    setup.py

sros2/sros2/api/_artifact_generation.py

@@ -21,10 +21,12 @@
 from . import _policy
 
 
+# FIXME move away from mutable default (linter should complain about it)
 def generate_artifacts(
-        keystore_path: Optional[pathlib.Path] = None,
-        identity_names: List[str] = [],
-        policy_files: List[pathlib.Path] = []) -> None:
+    keystore_path: Optional[pathlib.Path] = None,
+    identity_names: List[str] = [],
+    policy_files: List[pathlib.Path] = []
+) -> None:
     if keystore_path is None:
         keystore_path = _utilities.get_keystore_path_from_env()
         if keystore_path is None:
@@ -37,6 +39,8 @@ def generate_artifacts(
     for identity in identity_names:
         keystore.create_enclave(keystore_path, identity)
     for policy_file in policy_files:
+        # FIXME load_policy should raise something else
+        # than RuntimeError and it should be caught here
         policy_tree = load_policy(policy_file)
         enclaves_element = policy_tree.find('enclaves')
         for enclave in enclaves_element:

sros2/test/policies/invalid_policy_missing_topics_tag.xml

@@ -0,0 +1,17 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<policy version="0.2.0"
+  xmlns:xi="http://www.w3.org/2001/XInclude">
+  <enclaves>
+    <enclave path="/test_enclave">
+      <profiles>
+        <profile ns="/" node="talker">
+          <xi:include href="common/node.xml"
+            xpointer="xpointer(/profile/*)"/>
+	  <!--topics publish="ALLOW" -->
+            <topic>chatter</topic>
+	  <!--/topics-->
+        </profile>
+      </profiles>
+    </enclave>
+  </enclaves>
+</policy>

sros2/test/sros2/commands/security/verbs/test_create_enclave.py

@@ -34,8 +34,8 @@
 
 # This fixture will run once for the entire module (as opposed to once per test)
 @pytest.fixture(scope='module')
-def enclave_keys_dir(tmpdir_factory) -> Path:
-    keystore_dir = Path(str(tmpdir_factory.mktemp('keystore')))
+def enclave_keys_dir(tmp_path_factory) -> Path:
+    keystore_dir = tmp_path_factory.mktemp('keystore')
 
     # First, create the keystore
     sros2.keystore.create_keystore(keystore_dir)
@@ -96,12 +96,11 @@ def test_create_enclave(enclave_keys_dir):
         assert (enclave_keys_dir / expected_file).is_file()
 
 
-def test_create_enclave_twice(tmpdir):
-    keystore_dir = Path(tmpdir)
-
+def test_create_enclave_twice(tmp_path):
     # First, create the keystore
-    sros2.keystore.create_keystore(keystore_dir)
-    assert keystore_dir.is_dir()
+    sros2.keystore.create_keystore(tmp_path)
+    assert tmp_path.is_dir()
+    keystore_dir = tmp_path
 
     # Now using that keystore, create an enclave
     assert cli.main(

sros2/test/sros2/commands/security/verbs/test_create_keystore.py

@@ -29,14 +29,14 @@
 
 # This fixture will run once for the entire module (as opposed to once per test)
 @pytest.fixture(scope='module')
-def keystore_dir(tmpdir_factory) -> Path:
-    keystore_dir = str(tmpdir_factory.mktemp('keystore'))
+def keystore_dir(tmp_path_factory) -> Path:
+    keystore_dir = tmp_path_factory.mktemp('keystore')
 
     # Create the keystore
-    assert cli.main(argv=['security', 'create_keystore', keystore_dir]) == 0
+    assert cli.main(argv=['security', 'create_keystore', str(keystore_dir)]) == 0
 
     # Return path to keystore directory
-    return Path(keystore_dir)
+    return keystore_dir
 
 
 def test_create_keystore(keystore_dir):
@@ -95,3 +95,12 @@ def test_governance_p7s(keystore_dir):
 def test_governance_xml(keystore_dir):
     # Validates valid XML
     ElementTree.parse(str(keystore_dir / 'enclaves' / 'governance.xml'))
+
+
+def test_create_keystore_twice_fails(tmp_path):
+    keystore_dir = tmp_path / 'keystore'
+    keystore_dir.mkdir()
+
+    # Create the keystore
+    assert cli.main(argv=['security', 'create_keystore', str(keystore_dir)]) == 0
+    assert cli.main(argv=['security', 'create_keystore', str(keystore_dir)]) == 1

sros2/test/sros2/commands/security/verbs/test_create_permission.py

@@ -30,8 +30,8 @@
 
 # This fixture will run once for the entire module (as opposed to once per test)
 @pytest.fixture(scope='module')
-def enclave_dir(tmpdir_factory, test_policy_dir) -> pathlib.Path:
-    keystore_dir = pathlib.Path(str(tmpdir_factory.mktemp('keystore')))
+def enclave_dir(tmp_path_factory, test_policy_dir) -> pathlib.Path:
+    keystore_dir = tmp_path_factory.mktemp('keystore')
 
     # First, create the keystore as well as an enclave for the talker
     sros2.keystore.create_keystore(keystore_dir)

sros2/test/sros2/commands/security/verbs/test_generate_artifacts.py

@@ -0,0 +1,126 @@
+# Copyright 2024 Mikael Arguedas
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from pathlib import Path
+
+import pytest
+
+from ros2cli import cli
+
+from sros2 import _utilities
+
+
+# Here we provide only very high level testing as this verb
+# is just a combination of calls to the others ones covered by precise tests
+
+# This fixture will run once for the entire module (as opposed to once per test)
+@pytest.fixture(scope='module')
+def keystore_dir(tmp_path_factory) -> Path:
+    keystore_dir = tmp_path_factory.mktemp('keystore')
+
+    # Create the keystore
+    assert cli.main(argv=['security', 'create_keystore', str(keystore_dir)]) == 0
+
+    # Return path to keystore directory
+    return keystore_dir
+
+
+def test_cli_keystore_args(capsys, tmp_path, monkeypatch, keystore_dir):
+    # invalid keystore
+    assert cli.main(argv=['security', 'generate_artifacts', '-k', str(tmp_path)]) == 0
+    output = capsys.readouterr().out.rstrip()
+    assert 'is not a valid keystore, creating new keystore' in output
+
+    assert cli.main(argv=['security', 'generate_artifacts', '-k', str(keystore_dir)]) == 0
+
+    # keystore from env
+    with monkeypatch.context() as m:
+        m.setenv(_utilities._KEYSTORE_DIR_ENV, str(keystore_dir))
+        assert cli.main(argv=['security', 'generate_artifacts']) == 0
+
+    # invalid keystore from env
+    tmp_keystore_folder = tmp_path
+    with monkeypatch.context() as m:
+        m.setenv(_utilities._KEYSTORE_DIR_ENV, str(tmp_keystore_folder / 'bar'))
+        assert cli.main(argv=['security', 'generate_artifacts']) == 0
+        output = capsys.readouterr().out.rstrip()
+        assert 'is not a valid keystore, creating new keystore' in output
+
+    # no keystore in args or in env
+    with monkeypatch.context() as m:
+        m.delenv(_utilities._KEYSTORE_DIR_ENV, raising=False)
+        assert cli.main(argv=['security', 'generate_artifacts']) == 1
+        output = capsys.readouterr().err.rstrip()
+        assert (
+            'Unable to generate artifacts: '
+            "'ROS_SECURITY_KEYSTORE' isn't pointing at a valid keystore"
+            in output
+        )
+
+
+def test_cli_enclave_args(keystore_dir):
+    # no enclaves
+    assert cli.main(argv=['security', 'generate_artifacts', '-k', str(keystore_dir)]) == 0
+
+    # 1 existing enclave and 1 to create
+    assert cli.main(
+        argv=['security', 'create_enclave', str(keystore_dir), '/test_enclave']) == 0
+    enclave_list = ['/test_enclave', '/test_enclave2']
+    command_args = ['security', 'generate_artifacts', '-k', str(keystore_dir)]
+    for name in enclave_list:
+        command_args.append('-e')
+        command_args.append(name)
+    assert cli.main(argv=command_args) == 0
+    expected_files = (
+        'cert.pem', 'governance.p7s', 'identity_ca.cert.pem', 'key.pem', 'permissions.p7s',
+        'permissions.xml', 'permissions_ca.cert.pem'
+    )
+    for enclave in enclave_list:
+        enclave_keys_dir = keystore_dir / 'enclaves' / enclave.lstrip('/')
+        assert len(list(enclave_keys_dir.iterdir())) == len(expected_files)
+
+        for expected_file in expected_files:
+            assert (enclave_keys_dir / expected_file).is_file()
+
+
+def test_cli_policies_args(capsys, keystore_dir, test_policy_dir):
+    enclave_list = ['/test_enclave', '/test_enclave2', '/minimal_action/minimal_action_server']
+    command_args = ['security', 'generate_artifacts', '-k', str(keystore_dir)]
+    for name in enclave_list:
+        command_args.append('-e')
+        command_args.append(name)
+    # Test an invalid policy file
+    retcode = cli.main(
+        argv=command_args + [
+            '-p', str(test_policy_dir / 'invalid_policy_missing_topics_tag.xml')
+        ]
+    )
+    assert "Element 'topic': This element is not expected." in retcode
+    # Test a valid policy file
+    assert cli.main(
+        argv=command_args + [
+            '-p', str(test_policy_dir / 'minimal_action.policy.xml')
+        ]
+    ) == 0
+    # ensure that missing enclaves have been created on the fly
+    for name in enclave_list:
+        assert Path(keystore_dir / 'enclaves' / name.lstrip('/')).is_dir()
+    # Test a valid set of policy files
+    assert cli.main(
+        argv=command_args + [
+            '-p', str(test_policy_dir / 'minimal_action.policy.xml'),
+            '-p', str(test_policy_dir / 'add_two_ints.policy.xml'),
+            '-p', str(test_policy_dir / 'talker_listener.policy.xml'),
+        ]
+    ) == 0

sros2/test/sros2/commands/security/verbs/test_list_enclaves.py

@@ -57,14 +57,14 @@ def test_list_enclaves_no_keys(capsys):
 def test_list_enclaves_uninitialized_keystore(capsys):
     with tempfile.TemporaryDirectory() as keystore_dir:
         # Verify that list_enclaves properly handles an uninitialized keystore
-        assert cli.main(argv=['security', 'list_enclaves', keystore_dir]) != 0
+        assert cli.main(argv=['security', 'list_enclaves', keystore_dir]) == 1
         assert (capsys.readouterr().err.strip() ==
                 f"Unable to list enclaves: '{keystore_dir}' is not a valid keystore")
 
 
 def test_list_enclaves_no_keystore(capsys):
     # Verify that list_enclaves properly handles a non-existent keystore
     keystore = os.path.join(tempfile.gettempdir(), 'non-existent')
-    assert cli.main(argv=['security', 'list_enclaves', keystore]) != 0
+    assert cli.main(argv=['security', 'list_enclaves', keystore]) == 1
     assert (capsys.readouterr().err.strip() ==
             f"Unable to list enclaves: '{keystore}' is not a valid keystore")

sros2/test/sros2/commands/security/verbs/test_no_verb.py

@@ -0,0 +1,21 @@
+# Copyright 2024 Mikael Arguedas
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from ros2cli import cli
+
+
+def test_no_verb(capsys):
+    assert cli.main(argv=['security']) == 0
+    output = capsys.readouterr().out.rstrip()
+    assert 'Call `ros2 security <command> -h` for more detailed usage.' in output

sros2/test/sros2/keystore/test_enclave.py

@@ -12,6 +12,16 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+from pathlib import Path
+
+import pytest
+
+from ros2cli import cli
+
+from sros2.errors import (
+    InvalidEnclaveNameError,
+    InvalidKeystoreError,
+)
 from sros2.keystore import _enclave
 
 
@@ -29,3 +39,23 @@ def test_is_key_name_valid():
     assert not _enclave._is_enclave_name_valid('foo/bar')
     assert not _enclave._is_enclave_name_valid('/42foo')
     assert not _enclave._is_enclave_name_valid('/foo/42bar')
+
+
+@pytest.fixture()
+def keystore_dir(tmp_path_factory) -> Path:
+    keystore_dir = tmp_path_factory.mktemp('keystore')
+
+    # Create the keystore
+    assert cli.main(argv=['security', 'create_keystore', str(keystore_dir)]) == 0
+
+    # Return path to keystore directory
+    return keystore_dir
+
+
+def test_create_enclave_invalid_arguments(keystore_dir):
+    with pytest.raises(InvalidKeystoreError):
+        _enclave.create_enclave(Path('foo/bar'), '/baz/foobar')
+    with pytest.raises(InvalidKeystoreError):
+        _enclave.create_enclave(Path('foo/bar'), 'baz/foobar')
+    with pytest.raises(InvalidEnclaveNameError):
+        _enclave.create_enclave(keystore_dir, 'baz/foobar')