Merge pull request #550 from openlawlibrary/renatav/usability-improve…

…ments

Usability improvements

CHANGELOG.md

@@ -13,6 +13,32 @@ and this project adheres to [Semantic Versioning][semver].
 
 ### Fixed
 
+
+## [0.31.2] - 10/16/2024
+
+### Added
+
+- Added a function for exporting `keys-description.json` ([550])
+- Added support for cloning a new dependency when adding it to `dependencies.json` if it is not on disk ([550])
+- Clean up authentication repository if an error occurs while running a cli command ([550])
+
+### Changed
+
+- Return a non-zero exit code with `sys.exit` when updater fails ([550])
+- Rework addition of a new role and target repositories. Use `custom.json` files ([550])
+
+
+### Fixed
+
+- Minor `conf init` and detection of the authentication repository fixes ([550])
+- Replace `info` logging calls with `notice` in API functions ([550])
+- Use `mirrors.json` urls when cloning dependencies ([551])
+
+
+[551]: https://github.com/openlawlibrary/taf/pull/551
+[550]: https://github.com/openlawlibrary/taf/pull/550
+
+
 ## [0.31.1] - 10/03/2024
 
 ### Added
@@ -1278,7 +1304,8 @@ and this project adheres to [Semantic Versioning][semver].
 
 [keepachangelog]: https://keepachangelog.com/en/1.0.0/
 [semver]: https://semver.org/spec/v2.0.0.html
-[unreleased]: https://github.com/openlawlibrary/taf/compare/v0.31.1...HEAD
+[unreleased]: https://github.com/openlawlibrary/taf/compare/v0.31.2...HEAD
+[0.31.2]: https://github.com/openlawlibrary/taf/compare/v0.31.1...v0.31.2
 [0.31.1]: https://github.com/openlawlibrary/taf/compare/v0.31.0...v0.31.1
 [0.31.0]: https://github.com/openlawlibrary/taf/compare/v0.30.2...0.31.0
 [0.30.2]: https://github.com/openlawlibrary/taf/compare/v0.30.1...v0.30.2

taf/api/conf.py

@@ -1,8 +1,10 @@
 from shutil import Error, copytree
+import shutil
 from typing import Optional
 from pathlib import Path
 from taf.api.keystore import generate_keys
 from taf.log import taf_logger
+from taf.utils import read_input_dict
 
 
 def init(
@@ -17,11 +19,11 @@ def init(
         taf_directory = Path(".taf")
 
     if taf_directory.exists() and taf_directory.is_dir():
-        taf_logger.info(".taf directory already exists.")
+        taf_logger.log("NOTICE", ".taf directory already exists.")
     else:
         # Create the .taf directory
         taf_directory.mkdir(exist_ok=True)
-        taf_logger.info("Generated .taf directory")
+        taf_logger.log("NOTICE", "Generated .taf directory")
 
     # Create the config.toml file
     config_file_path = taf_directory / "config.toml"
@@ -32,7 +34,15 @@ def init(
     keystore_directory.mkdir(exist_ok=True)
 
     # If any of these parameters exist you can assume the user wants to generate keys
-    if not keystore and not roles_key_infos:
+
+    # check if keystore already exists
+    roles_key_infos_dict = read_input_dict(roles_key_infos)
+    keystore = (
+        keystore or (roles_key_infos and roles_key_infos_dict.get("keystore")) or None
+    )
+    should_generate_keys = False
+    keystore_path = Path(keystore) if keystore else None
+    if not keystore:
         # Prompt the user if they want to run the generate_keys function
         while True:
             use_keystore = (
@@ -43,9 +53,9 @@ def init(
             if use_keystore in ["y", "n"]:
                 should_generate_keys = use_keystore == "y"
                 break
-    if should_generate_keys or (keystore and not roles_key_infos):
-        # First check if the user already specified keystore
-        if not keystore:
+
+        if should_generate_keys:
+            # First check if the user already specified keystore
             copy_keystore = (
                 input(
                     "Do you want to load an existing keystore from another location? [y/N]: "
@@ -61,32 +71,33 @@ def init(
                     keystore_path = Path(keystore_input)
                     if keystore_path.exists() and keystore_path.is_dir():
                         keystore = keystore_input  # Assign the string path to the keystore variable
+                        should_generate_keys = (
+                            False  # no need to generate keys, they will be copied
+                        )
                         break
                     else:
                         taf_logger.error(
                             f"Provided keystore path {keystore} is invalid."
                         )
-        # Check if keystore is specified now. If so copy the keys
-        if keystore:
-            try:
-                copytree(keystore, keystore_directory, dirs_exist_ok=True)
-                taf_logger.info(
-                    f"Copied keystore from {keystore} to {keystore_directory}"
-                )
-            except FileNotFoundError:
-                taf_logger.error(f"Provided keystore path {keystore} not found.")
-            except Error as e:
-                taf_logger.error(f"Error occurred while copying keystore: {e}")
+    # Check if keystore is specified now. If so copy the keys
+    if keystore and keystore_path and keystore_path.is_dir():
+        try:
+            copytree(keystore, keystore_directory, dirs_exist_ok=True)
+            taf_logger.log(
+                "NOTICE", f"Copied keystore from {keystore} to {keystore_directory}"
+            )
+        except FileNotFoundError:
+            taf_logger.error(f"Provided keystore path {keystore} not found.")
+        except Error as e:
+            taf_logger.error(f"Error occurred while copying keystore: {e}")
 
-        # If there is no keystore path specified, ask for keys description and generate keys
-        elif not roles_key_infos:
-            roles_key_infos = input(
-                "Enter the path to the keys description JSON file (can be left empty): "
-            ).strip()
-            if not roles_key_infos:
-                roles_key_infos = "."
-    if roles_key_infos:
-        generate_keys(taf_directory, str(keystore_directory), roles_key_infos)
-        taf_logger.info(
-            f"Successfully generated keys inside the {keystore_directory} directory"
+    if should_generate_keys:
+        generate_keys(keystore_directory, roles_key_infos)
+        taf_logger.log(
+            "NOTICE",
+            f"Successfully generated keys inside the {keystore_directory} directory",
         )
+
+    if roles_key_infos is not None and Path(roles_key_infos).is_file():
+        infos_config_path = (taf_directory / Path(roles_key_infos).name).absolute()
+        shutil.copy(str(roles_key_infos), str(infos_config_path))

taf/api/dependencies.py

@@ -17,6 +17,8 @@
 from taf.exceptions import TAFError
 from taf.git import GitRepository
 from taf.log import taf_logger
+from taf.updater.updater import OperationType, clone_repository
+import taf.updater.updater as updater
 
 
 @log_on_start(
@@ -38,6 +40,7 @@ def add_dependency(
     out_of_band_commit: str,
     keystore: str,
     dependency_path: Optional[str] = None,
+    dependency_url: Optional[str] = None,
     library_dir: Optional[str] = None,
     scheme: Optional[str] = DEFAULT_RSA_SIGNATURE_SCHEME,
     custom: Optional[Dict] = None,
@@ -82,7 +85,12 @@ def add_dependency(
 
     auth_repo = AuthenticationRepository(path=path)
     if not auth_repo.is_git_repository_root:
-        print(f"{path} is not a git repository!")
+        taf_logger.error(f"{path} is not a git repository!")
+        return
+
+    dependencies_json = repositoriesdb.load_dependencies_json(auth_repo)
+    if dependencies_json is not None and dependency_name in dependencies_json:
+        taf_logger.log("NOTICE", f"{dependency_name} already added")
         return
     if library_dir is None:
         library_dir = str(auth_repo.path.parent.parent)
@@ -92,6 +100,26 @@ def add_dependency(
     else:
         dependency = GitRepository(library_dir, dependency_name)
 
+    if not dependency.is_git_repository and dependency_url is not None:
+        taf_logger.log(
+            "NOTICE", f"{dependency.path} does not exist. Cloning from {dependency_url}"
+        )
+        config = updater.UpdateConfig(
+            operation=OperationType.CLONE,
+            url=dependency_url,
+            path=Path(library_dir, dependency_name),
+            library_dir=library_dir,
+            strict=False,
+            bare=False,
+            no_deps=False,
+        )  # type: ignore
+        try:
+            clone_repository(config)
+            dependency.default_branch = dependency._determine_default_branch()
+        except Exception as e:
+            taf_logger.error(f"Dependency clone failed due to error {e}.")
+            return
+
     if dependency.is_git_repository:
         branch_name, out_of_band_commit = _determine_out_of_band_data(
             dependency, branch_name, out_of_band_commit, no_prompt
@@ -124,9 +152,9 @@ def add_dependency(
         dependencies[dependency_name]["custom"] = custom
 
     # update content of repositories.json before updating targets metadata
-    Path(auth_repo.path, repositoriesdb.DEPENDENCIES_JSON_PATH).write_text(
-        json.dumps(dependencies_json, indent=4)
-    )
+    dependencies_path = Path(auth_repo.path, repositoriesdb.DEPENDENCIES_JSON_PATH)
+    dependencies_path.parent.mkdir(exist_ok=True)
+    Path(dependencies_path).write_text(json.dumps(dependencies_json, indent=4))
 
     removed_targets_data: Dict = {}
     added_targets_data: Dict = {repositoriesdb.DEPENDENCIES_JSON_NAME: {}}

taf/api/keystore.py

@@ -1,5 +1,5 @@
 from logging import INFO
-from typing import Optional
+from typing import Optional, Union
 from logdecorator import log_on_start, log_on_end
 from pathlib import Path
 from taf.models.types import RolesKeysData
@@ -15,6 +15,7 @@
 from taf.log import taf_logger
 from taf.models.types import RolesIterator
 from taf.models.converter import from_dict
+from taf.exceptions import KeystoreError
 
 
 @log_on_start(INFO, "Generating '{key_path:s}'", logger=taf_logger)
@@ -40,14 +41,21 @@ def _generate_rsa_key(key_path: str, password: str, bits: Optional[int] = None)
     Returns:
         None
     """
-    if password:
-        generate_and_write_rsa_keypair(filepath=key_path, bits=bits, password=password)
-    else:
-        generate_and_write_unencrypted_rsa_keypair(filepath=key_path, bits=bits)
+    try:
+        if password:
+            generate_and_write_rsa_keypair(
+                filepath=key_path, bits=bits, password=password
+            )
+        else:
+            generate_and_write_unencrypted_rsa_keypair(filepath=key_path, bits=bits)
+        taf_logger.log("NOTICE", f"Generated key {key_path}")
+    except Exception:
+        taf_logger.error(f"An error occurred while generating rsa key {key_path}")
+        raise KeystoreError(f"An error occurred while generating rsa key {key_path}")
 
 
 def generate_keys(
-    auth_repo_path: Path, keystore: Optional[str], roles_key_infos: str
+    keystore: Optional[Union[str, Path]], roles_key_infos: Optional[str]
 ) -> None:
     """
     Generate public and private keys and writes them to disk. Names of keys correspond to names
@@ -68,15 +76,20 @@ def generate_keys(
 
     Returns:
         None
+
+    Raises:
+        KeystoreError if an error occurs while initializing the keystore directory or generating a key
     """
     if keystore is None:
-        taf_directory = find_taf_directory(auth_repo_path)
+        taf_directory = find_taf_directory(Path())
         if taf_directory:
             keystore = str(taf_directory / "keystore")
         else:
             keystore = "./keystore"
+
+    taf_logger.log("NOTICE", f"Generating keys in {str(Path(keystore).absolute())}")
     roles_key_infos_dict, keystore, _ = _initialize_roles_and_keystore(
-        roles_key_infos, keystore
+        roles_key_infos, str(keystore)
     )
 
     roles_keys_data = from_dict(roles_key_infos_dict, RolesKeysData)
@@ -86,7 +99,7 @@ def generate_keys(
                 key_name = get_key_name(role.name, key_num, role.number)
                 if keystore is not None:
                     password = input(
-                        "Enter keystore password and press ENTER (can be left empty)"
+                        f"Enter {key_name} keystore password and press ENTER (can be left empty)"
                     )
                     key_path = str(Path(keystore, key_name))
                     _generate_rsa_key(key_path, password, role.length)

taf/api/repository.py

@@ -20,6 +20,7 @@
 from taf.exceptions import TAFError
 from taf.keys import load_sorted_keys_of_new_roles
 import taf.repositoriesdb as repositoriesdb
+from taf.api.utils._conf import find_keystore
 from taf.utils import ensure_pre_push_hook
 from tuf.repository_tool import create_new_repository
 from taf.log import taf_logger
@@ -65,6 +66,10 @@ def create_repository(
     if not _check_if_can_create_repository(auth_repo):
         return
 
+    if not keystore and auth_repo.path is not None:
+        keystore_path = find_keystore(auth_repo.path)
+        if keystore_path is not None:
+            keystore = str(keystore_path)
     roles_key_infos_dict, keystore, skip_prompt = _initialize_roles_and_keystore(
         roles_key_infos, keystore
     )