diff --git a/nvflare/private/fed/server/job_meta_validator.py b/nvflare/private/fed/server/job_meta_validator.py index 919d99fb85..9af2cb3495 100644 --- a/nvflare/private/fed/server/job_meta_validator.py +++ b/nvflare/private/fed/server/job_meta_validator.py @@ -55,7 +55,7 @@ def validate(self, job_name: str, job_data: bytes) -> Tuple[bool, str, dict]: self._validate_resource(job_name, meta) self._validate_mandatory_clients(job_name, meta, clients) except ValueError as e: - return False, secure_format_exception(e), meta + return False, str(e), meta return True, "", meta diff --git a/nvflare/private/fed/server/server_engine.py b/nvflare/private/fed/server/server_engine.py index 73fc51d997..c1fe9a8cb2 100644 --- a/nvflare/private/fed/server/server_engine.py +++ b/nvflare/private/fed/server/server_engine.py @@ -756,8 +756,6 @@ def check_client_resources(self, job: Job, resource_reqs, fl_ctx: FLContext) -> def _make_message_for_check_resource(self, job, resource_requirements, fl_ctx): request = Message(topic=TrainingTopic.CHECK_RESOURCE, body=resource_requirements) request.set_header(RequestHeader.JOB_ID, job.job_id) - request.set_header(RequestHeader.REQUIRE_AUTHZ, "true") - request.set_header(RequestHeader.ADMIN_COMMAND, AdminCommandNames.CHECK_RESOURCES) set_message_security_data(request, job, fl_ctx) return request diff --git a/tests/integration_test/data/test_configs/authorization/test_diff_job_config.yml b/tests/integration_test/data/test_configs/authorization/test_diff_job_config.yml index 50f73993ce..ce6bbb30e7 100644 --- a/tests/integration_test/data/test_configs/authorization/test_diff_job_config.yml +++ b/tests/integration_test/data/test_configs/authorization/test_diff_job_config.yml @@ -16,7 +16,7 @@ tests: "data": [ { "type": "error", - "data": "ValueError: Multiple apps to be deployed to following sites ['site-1'] for job duplicate_clients" + "data": "Multiple apps to be deployed to following sites ['site-1'] for job duplicate_clients" } ] - "trigger": @@ -37,7 +37,7 @@ tests: "data": [ { "type": "error", - "data": "ValueError: Multiple apps to be deployed to following sites ['server'] for job duplicate_server" + "data": "Multiple apps to be deployed to following sites ['server'] for job duplicate_server" } ] - "trigger": @@ -58,7 +58,7 @@ tests: "data": [ { "type": "error", - "data": "ValueError: value for key site-1 in resource spec is expecting a dictionary" + "data": "value for key site-1 in resource spec is expecting a dictionary" } ] - "trigger": @@ -79,7 +79,7 @@ tests: "data": [ { "type": "error", - "data": "ValueError: Mandatory clients {'site-1'} are not in the deploy_map for job mandatory_not_met" + "data": "Mandatory clients {'site-1'} are not in the deploy_map for job mandatory_not_met" } ] - "trigger": @@ -100,7 +100,7 @@ tests: "data": [ { "type": "error", - "data": "ValueError: min_clients -1 must be positive for job min_clients_value_range" + "data": "min_clients -1 must be positive for job min_clients_value_range" } ] - "trigger": @@ -121,7 +121,7 @@ tests: "data": [ { "type": "error", - "data": "ValueError: App 'sag2' in deploy_map doesn't exist for job missing_app" + "data": "App 'sag2' in deploy_map doesn't exist for job missing_app" } ] - "trigger": @@ -142,7 +142,7 @@ tests: "data": [ { "type": "error", - "data": "ValueError: App 'sag2' will be deployed to client but client config is missing" + "data": "App 'sag2' will be deployed to client but client config is missing" } ] - "trigger": @@ -163,7 +163,7 @@ tests: "data": [ { "type": "error", - "data": "ValueError: App 'sag1' will be deployed to server but server config is missing" + "data": "App 'sag1' will be deployed to server but server config is missing" } ] - "trigger": @@ -184,7 +184,7 @@ tests: "data": [ { "type": "error", - "data": "ValueError: No site is specified in deploy_map for job no_deployment" + "data": "No site is specified in deploy_map for job no_deployment" } ] - "trigger": @@ -205,7 +205,7 @@ tests: "data": [ { "type": "error", - "data": "ValueError: min 4 clients required for job not_enough_clients, found 2." + "data": "min 4 clients required for job not_enough_clients, found 2." } ] - "trigger": @@ -215,21 +215,27 @@ tests: "result": "type": "run_state" "data": {} - - test_name: "submit valid_app_as_job" + - test_name: "submit old_app" event_sequence: - "trigger": "type": "server_log" "data": "Server started" - "actions": [ "submit_job valid_app_as_job" ] + "actions": [ "submit_job old_app" ] "result": - "type": "job_submit_success" + "type": "admin_api_response" + "data": [ + { + "type": "error", + "data": "meta.[json|conf|yml] not existing for job old_app, possible in legacy job format. Please upgrade the job structure." + } + ] - "trigger": - "type": "run_state" - "data": { "run_finished": True } - "actions": [ "ensure_current_job_done" ] + "type": "server_log" + "data": "Server started" + "actions": [ "mark_test_done" ] "result": "type": "run_state" - "data": { "run_finished": True } + "data": {} - test_name: "submit valid_job" event_sequence: - "trigger": diff --git a/tests/unit_test/data/jobs/min_clients_value_range/meta.json b/tests/unit_test/data/jobs/min_clients_value_range/meta.json index b43de1b963..459f3318f9 100644 --- a/tests/unit_test/data/jobs/min_clients_value_range/meta.json +++ b/tests/unit_test/data/jobs/min_clients_value_range/meta.json @@ -1,7 +1,7 @@ { "name": "sag", "resource_spec": {}, - "deploy_map": {"sag": ["server","site-1", "site-2"]}, + "deploy_map": {"sag": ["server", "site-1", "site-2"]}, "min_clients" : -1 } diff --git a/tests/unit_test/data/jobs/valid_app_as_job/config/config_fed_client.json b/tests/unit_test/data/jobs/old_app/config/config_fed_client.json similarity index 100% rename from tests/unit_test/data/jobs/valid_app_as_job/config/config_fed_client.json rename to tests/unit_test/data/jobs/old_app/config/config_fed_client.json diff --git a/tests/unit_test/data/jobs/valid_app_as_job/config/config_fed_server.json b/tests/unit_test/data/jobs/old_app/config/config_fed_server.json similarity index 100% rename from tests/unit_test/data/jobs/valid_app_as_job/config/config_fed_server.json rename to tests/unit_test/data/jobs/old_app/config/config_fed_server.json diff --git a/tests/unit_test/data/jobs/valid_job_wo_job_name/meta.json b/tests/unit_test/data/jobs/valid_job_wo_job_name/meta.json new file mode 100644 index 0000000000..429910d029 --- /dev/null +++ b/tests/unit_test/data/jobs/valid_job_wo_job_name/meta.json @@ -0,0 +1,10 @@ +{ + "resource_spec": {}, + "deploy_map": { + "sag": [ + "server", + "site-1", + "site-2" + ] + } +} diff --git a/tests/unit_test/data/jobs/valid_job_wo_job_name/sag/config/config_fed_client.json b/tests/unit_test/data/jobs/valid_job_wo_job_name/sag/config/config_fed_client.json new file mode 100755 index 0000000000..7e2649a5c2 --- /dev/null +++ b/tests/unit_test/data/jobs/valid_job_wo_job_name/sag/config/config_fed_client.json @@ -0,0 +1,17 @@ +{ + "format_version": 2, + "executors": [ + { + "tasks": [ + "train" + ], + "executor": { + "path": "nvflare.app_common.np.np_trainer.NPTrainer", + "args": {} + } + } + ], + "task_result_filters": [], + "task_data_filters": [], + "components": [] +} \ No newline at end of file diff --git a/tests/unit_test/data/jobs/valid_job_wo_job_name/sag/config/config_fed_server.json b/tests/unit_test/data/jobs/valid_job_wo_job_name/sag/config/config_fed_server.json new file mode 100755 index 0000000000..df0fa1f1c7 --- /dev/null +++ b/tests/unit_test/data/jobs/valid_job_wo_job_name/sag/config/config_fed_server.json @@ -0,0 +1,44 @@ +{ + "format_version": 2, + "server": { + "heart_beat_timeout": 600 + }, + "task_data_filters": [], + "task_result_filters": [], + "components": [ + { + "id": "persistor", + "path": "nvflare.app_common.np.np_model_persistor.NPModelPersistor", + "args": {} + }, + { + "id": "shareable_generator", + "path": "nvflare.app_common.shareablegenerators.full_model_shareable_generator.FullModelShareableGenerator", + "args": {} + }, + { + "id": "aggregator", + "path": "nvflare.app_common.aggregators.intime_accumulate_model_aggregator.InTimeAccumulateWeightedAggregator", + "args": { + "expected_data_kind": "WEIGHTS" + } + } + ], + "workflows": [ + { + "id": "scatter_and_gather", + "path": "nvflare.app_common.workflows.scatter_and_gather.ScatterAndGather", + "args": { + "min_clients": 2, + "num_rounds": 1, + "start_round": 0, + "wait_time_after_min_received": 0, + "aggregator_id": "aggregator", + "persistor_id": "persistor", + "shareable_generator_id": "shareable_generator", + "train_task_name": "train", + "train_timeout": 6000 + } + } + ] +} \ No newline at end of file diff --git a/tests/unit_test/private/fed/server/job_meta_validator_test.py b/tests/unit_test/private/fed/server/job_meta_validator_test.py index 6d7b21e032..5a0b84aabf 100644 --- a/tests/unit_test/private/fed/server/job_meta_validator_test.py +++ b/tests/unit_test/private/fed/server/job_meta_validator_test.py @@ -21,7 +21,6 @@ import pytest from nvflare.apis.fl_constant import JobConstants -from nvflare.apis.utils.job_utils import convert_legacy_zipped_app_to_job from nvflare.fuel.utils.zip_utils import get_all_file_paths, normpath_for_zip, split_path from nvflare.private.fed.server.job_meta_validator import JobMetaValidator @@ -52,7 +51,7 @@ def _zip_job_with_meta(folder_name: str, meta: str) -> bytes: bio = io.BytesIO() _zip_directory_with_meta(job_path, folder_name, meta, bio) zip_data = bio.getvalue() - return convert_legacy_zipped_app_to_job(zip_data) + return zip_data META_WITH_VALID_DEPLOY_MAP = [ @@ -78,7 +77,7 @@ def _zip_job_with_meta(folder_name: str, meta: str) -> bytes: VALID_JOBS = [ pytest.param("valid_job", id="valid_job"), pytest.param("valid_job_deployment_all_idle", id="valid_job_deployment_all_idle"), - pytest.param("valid_app_as_job", id="valid_app_wo_meta"), + pytest.param("valid_job_wo_job_name", id="valid_job_wo_job_name"), ] @@ -93,6 +92,7 @@ def _zip_job_with_meta(folder_name: str, meta: str) -> bytes: pytest.param("missing_server_in_deployment", id="missing_server_in_deploy_map"), pytest.param("no_deployment", id="no_deployment"), pytest.param("not_enough_clients", id="not_enough_clients"), + pytest.param("old_app", id="old_app"), ] @@ -139,29 +139,8 @@ def test_invalid_min_clients_value_range(self, min_clients): """ self._assert_invalid(job_name, meta) - def test_deploy_map_config_non_exists_app(self): - job_name = "valid_job" - # valid_job folder contains sag app, not hello-pt app - meta = """ - { - "resource_spec": { "site-a": {"gpu": 1}, "site-b": {"gpu": 1}}, - "deploy_map": {"hello-pt": ["server","site-a", "site-b"]} - } - """ - self._assert_invalid(job_name, meta) - - def test_meta_missing_job_folder_name(self): - job_name = "valid_job" - meta = """ - { - "resource_spec": { "site-a": {"gpu": 1}, "site-b": {"gpu": 1}}, - "deploy_map": {"sag": ["server","site-a", "site-b"]} - } - """ - self._assert_valid(job_name, meta) - - def _assert_valid(self, job_name: str, meta: str = ""): - data = _zip_job_with_meta(job_name, meta) + def _assert_valid(self, job_name: str): + data = _zip_job_with_meta(job_name, "") valid, error, meta = self.validator.validate(job_name, data) assert valid assert error == ""