Skip to content

Commit

Permalink
chore: remove remove-role command since it compromises the validity o…
Browse files Browse the repository at this point in the history
…f the auth repo due to an issue with our TUF fork
  • Loading branch information
renatav committed Oct 9, 2024
1 parent afa8c27 commit a130985
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 39 deletions.
3 changes: 2 additions & 1 deletion taf/api/dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
from taf.exceptions import TAFError
from taf.git import GitRepository
from taf.log import taf_logger
from taf.updater.updater import OperationType, UpdateConfig, clone_repository
from taf.taf.tools.repo import UpdateConfig
from taf.updater.updater import OperationType, clone_repository


@log_on_start(
Expand Down
2 changes: 1 addition & 1 deletion taf/api/roles.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
update_snapshot_and_timestamp,
update_target_metadata,
)
from taf.auth_repo import AuthenticationRepository
from taf.auth_repo import METADATA_DIRECTORY_NAME, AuthenticationRepository
from taf.constants import (
DEFAULT_ROLE_SETUP_PARAMS,
DEFAULT_RSA_SIGNATURE_SCHEME,
Expand Down
3 changes: 3 additions & 0 deletions taf/auth_repo.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,9 @@ def get_info_json(
else:
return self.get_json(head_commit, INFO_JSON_PATH)

def get_metadata_path(self, role):
return self.path / METADATA_DIRECTORY_NAME / f"{role}.json"

def is_commit_authenticated(self, target_name: str, commit: str) -> bool:
"""Checks if passed commit is ever authenticated for given target name."""
for auth_commit in self.all_commits_on_branch(reverse=False):
Expand Down
2 changes: 1 addition & 1 deletion taf/tools/cli/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from taf.utils import is_run_from_python_executable, on_rm_error


def catch_cli_exception(func=None, *, handle, print_error=False, remove_dir_on_error=False):
def catch_cli_exception(func=None, *, handle=TAFError, print_error=False, remove_dir_on_error=False):
if not func:
return partial(
catch_cli_exception,
Expand Down
2 changes: 2 additions & 0 deletions taf/tools/repo/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@ def latest_commit_and_branch_command():
@click.command(help="""Fetch and print the last validated commit hash and the default branch.
Integrated into the pre-push git hook""")
@find_repository
@catch_cli_exception
@click.option("--path", default=".", help="Authentication repository's location. If not specified, set to the current directory")
def latest_commit_and_branch(path):
auth_repo = AuthenticationRepository(path=path)
Expand All @@ -302,6 +303,7 @@ def latest_commit_and_branch(path):
def status_command():
@click.command(help="Prints the whole state of the library, including authentication repositories and its dependencies.")
@find_repository
@catch_cli_exception
@click.option("--path", default=".", help="Authentication repository's location. If not specified, set to the current directory")
@click.option("--library-dir", default=None, help="Path to the library's root directory. Determined based on the authentication repository's path if not provided.")
def status(path, library_dir):
Expand Down
75 changes: 39 additions & 36 deletions taf/tools/roles/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
from taf.api.roles import add_role, add_roles, list_keys_of_role, remove_role, add_signing_key
from taf.constants import DEFAULT_RSA_SIGNATURE_SCHEME
from taf.exceptions import TAFError
from taf.repository_utils import find_valid_repository
from taf.tools.cli import catch_cli_exception
from taf.tools.cli import catch_cli_exception, find_repository

from taf.api.roles import add_role_paths

Expand All @@ -13,6 +12,7 @@ def add_role_command():
Its parent role, number of signing keys and signatures threshold can also be defined.
Update and sign all metadata files and commit.
""")
@find_repository
@catch_cli_exception(handle=TAFError)
@click.argument("role")
@click.option("--path", default=".", help="Authentication repository's location. If not specified, set to the current directory")
Expand All @@ -26,9 +26,9 @@ def add_role_command():
@click.option("--no-commit", is_flag=True, default=False, help="Indicates that the changes should not be committed automatically")
@click.option("--prompt-for-keys", is_flag=True, default=False, help="Whether to ask the user to enter their key if not located inside the keystore directory")
def add(role, path, parent_role, delegated_path, keystore, keys_number, threshold, yubikey, scheme, no_commit, prompt_for_keys):
if not path:
print("Specify at least one path")
delegated_path = find_valid_repository(delegated_path)
if not delegated_path:
print("Specify at least one delegated path")
return

add_role(
path=path,
Expand Down Expand Up @@ -77,6 +77,7 @@ def add_multiple_roles_command():
"keystore": "keystore_path"
}
""")
@find_repository
@catch_cli_exception(handle=TAFError)
@click.option("--path", default=".", help="Authentication repository's location. If not specified, set to the current directory")
@click.argument("keys-description")
Expand All @@ -85,7 +86,6 @@ def add_multiple_roles_command():
@click.option("--no-commit", is_flag=True, default=False, help="Indicates that the changes should not be committed automatically")
@click.option("--prompt-for-keys", is_flag=True, default=False, help="Whether to ask the user to enter their key if not located inside the keystore directory")
def add_multiple(path, keystore, keys_description, scheme, no_commit, prompt_for_keys):
path = find_valid_repository(path)
add_roles(
path=path,
keystore=keystore,
Expand All @@ -99,6 +99,7 @@ def add_multiple(path, keystore, keys_description, scheme, no_commit, prompt_for

def add_role_paths_command():
@click.command(help="Add a new delegated target role, specifying which paths are delegated to the new role. Its parent role, number of signing keys and signatures threshold can also be defined. Update and sign all metadata files and commit.")
@find_repository
@catch_cli_exception(handle=TAFError)
@click.argument("role")
@click.option("--path", default=".", help="Authentication repository's location. If not specified, set to the current directory")
Expand All @@ -107,7 +108,6 @@ def add_role_paths_command():
@click.option("--no-commit", is_flag=True, default=False, help="Indicates that the changes should not be committed automatically")
@click.option("--prompt-for-keys", is_flag=True, default=False, help="Whether to ask the user to enter their key if not located inside the keystore directory")
def adding_role_paths(role, path, delegated_path, keystore, no_commit, prompt_for_keys):
path = find_valid_repository(path)
if not delegated_path:
print("Specify at least one path")
return
Expand All @@ -124,32 +124,35 @@ def adding_role_paths(role, path, delegated_path, keystore, no_commit, prompt_fo
return adding_role_paths


def remove_role_command():
@click.command(help="""Remove a delegated target role, and, optionally, its targets (depending on the remove-targets parameter).
If targets should also be deleted, target files are remove and their corresponding entires are removed
from repositoires.json. If targets should not get removed, the target files are signed using the
removed role's parent role
""")
@catch_cli_exception(handle=TAFError)
@click.argument("role")
@click.option("--path", default=".", help="Authentication repository's location. If not specified, set to the current directory")
@click.option("--keystore", default=None, help="Location of the keystore files")
@click.option("--scheme", default=DEFAULT_RSA_SIGNATURE_SCHEME, help="A signature scheme used for signing")
@click.option("--remove-targets/--no-remove-targets", default=True, help="Should targets delegated to this role also be removed. If not removed, they are signed by the parent role")
@click.option("--no-commit", is_flag=True, default=False, help="Indicates that the changes should not be committed automatically")
@click.option("--prompt-for-keys", is_flag=True, default=False, help="Whether to ask the user to enter their key if not located inside the keystore directory")
def remove(role, path, keystore, scheme, remove_targets, no_commit, prompt_for_keys):
path = find_valid_repository(path)
remove_role(
path=path,
role=role,
keystore=keystore,
scheme=scheme,
remove_targets=remove_targets,
commit=not no_commit,
prompt_for_keys=prompt_for_keys,
)
return remove
# commenting out this command since its execution leads to an invalid state
# this is a TUF bug (or better said, caused by using a newer version of the updater and old repository_tool)
# it will be addressed when we transition to metadata API
# def remove_role_command():
# @click.command(help="""Remove a delegated target role, and, optionally, its targets (depending on the remove-targets parameter).
# If targets should also be deleted, target files are remove and their corresponding entires are removed
# from repositoires.json. If targets should not get removed, the target files are signed using the
# removed role's parent role
# """)
# @find_repository
# @catch_cli_exception(handle=TAFError)
# @click.argument("role")
# @click.option("--path", default=".", help="Authentication repository's location. If not specified, set to the current directory")
# @click.option("--keystore", default=None, help="Location of the keystore files")
# @click.option("--scheme", default=DEFAULT_RSA_SIGNATURE_SCHEME, help="A signature scheme used for signing")
# @click.option("--remove-targets/--no-remove-targets", default=True, help="Should targets delegated to this role also be removed. If not removed, they are signed by the parent role")
# @click.option("--no-commit", is_flag=True, default=False, help="Indicates that the changes should not be committed automatically")
# @click.option("--prompt-for-keys", is_flag=True, default=False, help="Whether to ask the user to enter their key if not located inside the keystore directory")
# def remove(role, path, keystore, scheme, remove_targets, no_commit, prompt_for_keys):
# remove_role(
# path=path,
# role=role,
# keystore=keystore,
# scheme=scheme,
# remove_targets=remove_targets,
# commit=not no_commit,
# prompt_for_keys=prompt_for_keys,
# )
# return remove


def add_signing_key_command():
Expand All @@ -162,6 +165,7 @@ def add_signing_key_command():
necessary to specify its path as the pub_key parameter's value. If this option
is not used when calling this command, the key can be directly entered later.
""")
@find_repository
@catch_cli_exception(handle=TAFError)
@click.option("--path", default=".", help="Authentication repository's location. If not specified, set to the current directory")
@click.option("--role", multiple=True, help="A list of roles to whose list of signing keys the new key should be added")
Expand All @@ -172,7 +176,6 @@ def add_signing_key_command():
@click.option("--no-commit", is_flag=True, default=False, help="Indicates that the changes should not be committed automatically")
@click.option("--prompt-for-keys", is_flag=True, default=False, help="Whether to ask the user to enter their key if not located inside the keystore directory")
def adding_signing_key(path, role, pub_key_path, keystore, keys_description, scheme, no_commit, prompt_for_keys):
path = find_valid_repository(path)
if not role:
print("Specify at least one role")
return
Expand All @@ -195,11 +198,11 @@ def list_keys_command():
List all keys of the specified role. If certs directory exists and contains certificates exported from YubiKeys,
include additional information read from these certificates, like name or organization.
""")
@find_repository
@catch_cli_exception(handle=TAFError)
@click.argument("role")
@click.option("--path", default=".", help="Authentication repository's location. If not specified, set to the current directory")
def list_keys(role, path):
path = find_valid_repository(path)
key_infos = list_keys_of_role(
path=path,
role=role,
Expand All @@ -213,6 +216,6 @@ def attach_to_group(group):
group.add_command(add_role_command(), name='add')
group.add_command(add_multiple_roles_command(), name='add-multiple')
group.add_command(add_role_paths_command(), name='add-role-paths')
group.add_command(remove_role_command(), name='remove')
# group.add_command(remove_role_command(), name='remove')
group.add_command(add_signing_key_command(), name='add-signing-key')
group.add_command(list_keys_command(), name='list-keys')

0 comments on commit a130985

Please sign in to comment.