From 0a7d0c0be8c5bdb9a0fd33dfc6531786ff2f1123 Mon Sep 17 00:00:00 2001 From: Emmanuel Jillela Date: Tue, 25 Apr 2023 09:57:33 -0700 Subject: [PATCH] [Bug: 768] FX CLI: Separate create, cert gen commands This change separates existing command "fx collaborator.py generate-cert-request" command into two commands. "fx collaborator create -n {NAME} -d {DATA_PATH: optional}". "fx collaborator generate-cert-request -n {NAME}". Fixes #768 Signed-off-by: Emmanuel Jillela --- docs/running_the_federation.rst | 1 + openfl/interface/collaborator.py | 39 ++++++++++++++++++++------------ openfl/native/native.py | 8 ++++--- tests/github/pki_wrong_cn.py | 7 +++++- tests/github/test_graminize.sh | 3 ++- tests/github/utils.py | 11 +++++++-- 6 files changed, 48 insertions(+), 21 deletions(-) diff --git a/docs/running_the_federation.rst b/docs/running_the_federation.rst index fdad205be7..b4c0352ff7 100644 --- a/docs/running_the_federation.rst +++ b/docs/running_the_federation.rst @@ -1047,6 +1047,7 @@ Importing the Workspace .. code-block:: console + fx collaborator create -n {COL_LABEL} -d {DATA_PATH:optional} fx collaborator generate-cert-request -n {COL_LABEL} diff --git a/openfl/interface/collaborator.py b/openfl/interface/collaborator.py index a0c25b73d8..c5f4232060 100644 --- a/openfl/interface/collaborator.py +++ b/openfl/interface/collaborator.py @@ -61,6 +61,24 @@ def start_(plan, collaborator_name, data_config, secure): plan.get_collaborator(collaborator_name).run() +@collaborator.command(name='create') +@option('-n', '--collaborator_name', required=True, + help='The certified common name of the collaborator') +@option('-d', '--data_path', + help='The data path to be associated with the collaborator') +@option('-s', '--silent', help='Do not prompt', is_flag=True) +def create_(collaborator_name, data_path, silent): + """Creates a user for an experiment.""" + if data_path and is_directory_traversal(data_path): + echo('Data path is out of the openfl workspace scope.') + sys.exit(1) + + common_name = f'{collaborator_name}'.lower() + + # TODO: There should be some association with the plan made here as well + register_data_path(common_name, data_path=data_path, silent=silent) + + def register_data_path(collaborator_name, data_path=None, silent=False): """Register dataset path in the plan/data.yaml file. @@ -103,30 +121,26 @@ def register_data_path(collaborator_name, data_path=None, silent=False): d[collaborator_name] = dir_path # Write the data.yaml - with open(data_yaml, 'w', encoding='utf-8') as f: - for key, val in d.items(): - f.write(f'{key}{separator}{val}\n') + if isfile(data_yaml): + with open(data_yaml, 'w', encoding='utf-8') as f: + for key, val in d.items(): + f.write(f'{key}{separator}{val}\n') @collaborator.command(name='generate-cert-request') @option('-n', '--collaborator_name', required=True, help='The certified common name of the collaborator') -@option('-d', '--data_path', - help='The data path to be associated with the collaborator') @option('-s', '--silent', help='Do not prompt', is_flag=True) @option('-x', '--skip-package', help='Do not package the certificate signing request for export', is_flag=True) def generate_cert_request_(collaborator_name, - data_path, silent, skip_package): + silent, skip_package): """Generate certificate request for the collaborator.""" - if data_path and is_directory_traversal(data_path): - echo('Data path is out of the openfl workspace scope.') - sys.exit(1) - generate_cert_request(collaborator_name, data_path, silent, skip_package) + generate_cert_request(collaborator_name, silent, skip_package) -def generate_cert_request(collaborator_name, data_path, silent, skip_package): +def generate_cert_request(collaborator_name, silent, skip_package): """ Create collaborator certificate key pair. @@ -192,9 +206,6 @@ def generate_cert_request(collaborator_name, data_path, silent, skip_package): echo('This file should be sent to the certificate authority' ' (typically hosted by the aggregator) for signing') - # TODO: There should be some association with the plan made here as well - register_data_path(common_name, data_path=data_path, silent=silent) - def find_certificate_name(file_name): """Parse the collaborator name.""" diff --git a/openfl/native/native.py b/openfl/native/native.py index ae45fd091f..de05c076f2 100644 --- a/openfl/native/native.py +++ b/openfl/native/native.py @@ -200,6 +200,8 @@ def init(workspace_template: str = 'default', log_level: str = 'INFO', aggregator.certify(agg_fqdn, silent=True) data_path = 1 for col_name in col_names: + collaborator.create( + col_name, str(data_path), silent=True) collaborator.generate_cert_request( col_name, str(data_path), silent=True, skip_package=True) collaborator.certify(col_name, silent=True) @@ -208,7 +210,7 @@ def init(workspace_template: str = 'default', log_level: str = 'INFO', setup_logging(level=log_level, log_file=log_file) -def create_collaborator(plan, name, model, aggregator): +def get_collaborator(plan, name, model, aggregator): """ Create the collaborator. @@ -282,9 +284,9 @@ def run_experiment(collaborator_dict: dict, override_config: dict = None): aggregator = plan.get_aggregator() - # Create the collaborators + # get the collaborators collaborators = { - collaborator: create_collaborator( + collaborator: get_collaborator( plan, collaborator, collaborator_dict[collaborator], aggregator ) for collaborator in plan.authorized_cols } diff --git a/tests/github/pki_wrong_cn.py b/tests/github/pki_wrong_cn.py index f59fd4edaf..ee522bd300 100644 --- a/tests/github/pki_wrong_cn.py +++ b/tests/github/pki_wrong_cn.py @@ -26,9 +26,14 @@ def prepare_workspace(): ]) for col in ['one', 'two']: subprocess.check_call([ - 'fx', 'collaborator', 'generate-cert-request', + 'fx', 'collaborator', 'create', '-n', col, '-d', '1', + '-s' + ]) + subprocess.check_call([ + 'fx', 'collaborator', 'generate-cert-request', + '-n', col, '-s', '-x' ]) subprocess.check_call([ diff --git a/tests/github/test_graminize.sh b/tests/github/test_graminize.sh index af0e4381c5..b2b127db26 100755 --- a/tests/github/test_graminize.sh +++ b/tests/github/test_graminize.sh @@ -61,7 +61,8 @@ create_collaborator() { # Create collaborator certificate request cd ${COL_DIRECTORY}/${FED_WORKSPACE} - fx collaborator generate-cert-request -d ${DATA_PATH} -n ${COL} --silent # Remove '--silent' if you run this manually + fx collaborator create -d ${DATA_PATH} -n ${COL} --silent # Remove '--silent' if you run this manually + fx collaborator generate-cert-request -n ${COL} --silent # Remove '--silent' if you run this manually # Sign collaborator certificate cd ${FED_DIRECTORY} # Move back to the Aggregator diff --git a/tests/github/utils.py b/tests/github/utils.py index 5a812d4707..a22479cfae 100644 --- a/tests/github/utils.py +++ b/tests/github/utils.py @@ -21,9 +21,13 @@ def create_collaborator(col, workspace_root, data_path, archive_name, fed_worksp ) # Create collaborator certificate request + check_call( + ['fx', 'collaborator', 'create', '-d', data_path, '-n', col, '--silent'], + cwd=col_path / fed_workspace + ) # Remove '--silent' if you run this manually check_call( - ['fx', 'collaborator', 'generate-cert-request', '-d', data_path, '-n', col, '--silent'], + ['fx', 'collaborator', 'generate-cert-request', '-n', col, '--silent'], cwd=col_path / fed_workspace ) @@ -84,7 +88,10 @@ def create_signed_cert_for_collaborator(col, data_path): print(f'Certifying collaborator {col} with data path {data_path}...') # Create collaborator certificate request check_call([ - 'fx', 'collaborator', 'generate-cert-request', '-d', data_path, '-n', col, '--silent' + 'fx', 'collaborator', 'create', '-d', data_path, '-n', col, '--silent' + ]) + check_call([ + 'fx', 'collaborator', 'generate-cert-request', '-n', col, '--silent' ]) # Sign collaborator certificate check_call([