Test Code changes due to TLS Parameter changes in openfl (securefeder…

…atedai#1170) * Change the input params Signed-off-by: Chaurasiya, Payal <[email protected]> * use_tls and require_client_auth Signed-off-by: Chaurasiya, Payal <[email protected]> * remove not Signed-off-by: Chaurasiya, Payal <[email protected]> * Small change in use-tls Signed-off-by: Chaurasiya, Payal <[email protected]> * Review comments Signed-off-by: Chaurasiya, Payal <[email protected]> * action needed for boolean Signed-off-by: Chaurasiya, Payal <[email protected]> * Small fix Signed-off-by: Chaurasiya, Payal <[email protected]> * Small fix Signed-off-by: Chaurasiya, Payal <[email protected]> * Review comments Signed-off-by: Chaurasiya, Payal <[email protected]> --------- Signed-off-by: Chaurasiya, Payal <[email protected]> Co-authored-by: Karan Shah <[email protected]>
payalcha · Nov 28, 2024 · 00f2755 · 00f2755
1 parent 5a59342
commit 00f2755
Show file tree

Hide file tree

Showing 5 changed files with 93 additions and 61 deletions.
diff --git a/.github/workflows/task_runner_e2e.yml b/.github/workflows/task_runner_e2e.yml
@@ -163,3 +163,70 @@ jobs:
         with:
           name: tr_non_tls_${{ env.MODEL_NAME }}_python${{ env.PYTHON_VERSION }}_${{ github.run_id }}
           path: result.tar
+
+  test_with_no_client_auth:
+    name: tr_no_client_auth
+    runs-on: ubuntu-22.04
+    timeout-minutes: 120 # 2 hours
+    strategy:
+      matrix:
+        # Testing non TLS scenario only for torch_cnn_mnist model and python 3.10
+        # If required, this can be extended to other models and python versions
+        model_name: ["keras_cnn_mnist"]
+        python_version: ["3.10"]
+      fail-fast: false # do not immediately fail if one of the combinations fail
+
+    env:
+      MODEL_NAME: ${{ matrix.model_name }}
+      PYTHON_VERSION: ${{ matrix.python_version }}
+
+    steps:
+      - name: Checkout OpenFL repository
+        id: checkout_openfl
+        uses: actions/[email protected]
+        with:
+          fetch-depth: 2 # needed for detecting changes
+          submodules: "true"
+          token: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Set up Python
+        id: setup_python
+        uses: actions/setup-python@v3
+        with:
+          python-version: ${{ env.PYTHON_VERSION }}
+
+      - name: Install dependencies
+        id: install_dependencies
+        run: |
+          python -m pip install --upgrade pip
+          pip install .
+          pip install -r test-requirements.txt
+
+      - name: Run Task Runner E2E tests without TLS
+        id: run_tests
+        run: |
+          python -m pytest -s tests/end_to_end/test_suites/task_runner_tests.py \
+          -m ${{ env.MODEL_NAME }} --model_name ${{ env.MODEL_NAME }} \
+          --num_rounds ${{ env.NUM_ROUNDS }} --num_collaborators ${{ env.NUM_COLLABORATORS }} --disable_client_auth
+          echo "Task runner end to end test run completed"
+
+      - name: Print test summary
+        id: print_test_summary
+        if: ${{ always() }}
+        run: |
+          export PYTHONPATH="$PYTHONPATH:."
+          python tests/end_to_end/utils/summary_helper.py
+          echo "Test summary printed"
+
+      - name: Tar files
+        id: tar_files
+        if: ${{ always() }}
+        run: tar -cvf result.tar results
+
+      - name: Upload Artifacts
+        id: upload_artifacts
+        uses: actions/upload-artifact@v4
+        if: ${{ always() }}
+        with:
+          name: tr_no_client_auth_${{ env.MODEL_NAME }}_python${{ env.PYTHON_VERSION }}_${{ github.run_id }}
+          path: result.tar
diff --git a/tests/end_to_end/conftest.py b/tests/end_to_end/conftest.py
@@ -17,10 +17,9 @@
 # Define a named tuple to store the objects for model owner, aggregator, and collaborators
 federation_fixture = collections.namedtuple(
     "federation_fixture",
-    "model_owner, aggregator, collaborators, model_name, disable_client_auth, disable_tls, workspace_path, results_dir, num_rounds",
+    "model_owner, aggregator, collaborators, model_name, require_client_auth, use_tls, workspace_path, results_dir, num_rounds",
 )
 
-
 def pytest_addoption(parser):
     """
     Add custom command line options to the pytest parser.
@@ -29,44 +28,12 @@ def pytest_addoption(parser):
     """
     parser.addini("results_dir", "Directory to store test results", default="results")
     parser.addini("log_level", "Logging level", default="DEBUG")
-    parser.addoption(
-        "--results_dir", action="store", type=str, default="results", help="Results directory"
-    )
-    parser.addoption(
-        "--num_collaborators",
-        action="store",
-        type=int,
-        default=constants.NUM_COLLABORATORS,
-        help="Number of collaborators",
-    )
-    parser.addoption(
-        "--num_rounds",
-        action="store",
-        type=int,
-        default=constants.NUM_ROUNDS,
-        help="Number of rounds to train",
-    )
-    parser.addoption(
-        "--model_name",
-        action="store",
-        type=str,
-        help="Model name",
-    )
-    parser.addoption(
-        "--disable_client_auth",
-        action="store_true",
-        help="Disable client authentication",
-    )
-    parser.addoption(
-        "--disable_tls",
-        action="store_true",
-        help="Disable TLS for communication",
-    )
-    parser.addoption(
-        "--log_memory_usage",
-        action="store_true",
-        help="Enable memory log in collaborators and aggregator",
-    )
+    parser.addoption("--num_collaborators")
+    parser.addoption("--num_rounds")
+    parser.addoption("--model_name")
+    parser.addoption("--disable_client_auth", action="store_true")
+    parser.addoption("--disable_tls", action="store_true")
+    parser.addoption("--log_memory_usage", action="store_true")
 
 
 @pytest.fixture(scope="session", autouse=True)
@@ -234,20 +201,20 @@ def fx_federation(request, pytestconfig):
     args = parse_arguments()
     # Use the model name from the test case name if not provided as a command line argument
     model_name = args.model_name if args.model_name else request.node.name.split("test_")[1]
-    results_dir = args.results_dir or pytestconfig.getini("results_dir")
+    results_dir = pytestconfig.getini("results_dir")
     num_collaborators = args.num_collaborators
     num_rounds = args.num_rounds
-    disable_client_auth = args.disable_client_auth
-    disable_tls = args.disable_tls
+    require_client_auth = not args.disable_client_auth
+    use_tls = not args.disable_tls
     log_memory_usage = args.log_memory_usage
 
     log.info(
         f"Running federation setup using Task Runner API on single machine with below configurations:\n"
         f"\tNumber of collaborators: {num_collaborators}\n"
         f"\tNumber of rounds: {num_rounds}\n"
         f"\tModel name: {model_name}\n"
-        f"\tClient authentication: {not disable_client_auth}\n"
-        f"\tTLS: {not disable_tls}\n"
+        f"\tClient authentication: {require_client_auth}\n"
+        f"\tTLS: {use_tls}\n"
         f"\tMemory Logs: {log_memory_usage}"
     )
 
@@ -270,16 +237,14 @@ def fx_federation(request, pytestconfig):
         model_owner.modify_plan(
             new_rounds=num_rounds,
             num_collaborators=num_collaborators,
-            disable_client_auth=disable_client_auth,
-            disable_tls=disable_tls,
+            require_client_auth=require_client_auth,
+            use_tls=use_tls,
         )
     except Exception as e:
         log.error(f"Failed to modify the plan: {e}")
         raise e
 
-    # For TLS enabled (default) scenario: when the workspace is certified, the collaborators are registered as well
-    # For TLS disabled scenario: collaborators need to be registered explicitly
-    if args.disable_tls:
+    if not use_tls:
         log.info("Disabling TLS for communication")
         try:
             model_owner.register_collaborators(num_collaborators)
@@ -321,8 +286,8 @@ def fx_federation(request, pytestconfig):
         aggregator=aggregator,
         collaborators=collaborators,
         model_name=model_name,
-        disable_client_auth=disable_client_auth,
-        disable_tls=disable_tls,
+        require_client_auth=require_client_auth,
+        use_tls=use_tls,
         workspace_path=workspace_path,
         results_dir=results_dir,
         num_rounds=num_rounds,

diff --git a/tests/end_to_end/models/participants.py b/tests/end_to_end/models/participants.py
@@ -114,14 +114,14 @@ def certify_collaborator(self, collaborator_name):
             raise e
         return True
 
-    def modify_plan(self, new_rounds=None, num_collaborators=None, disable_client_auth=False, disable_tls=False):
+    def modify_plan(self, new_rounds=None, num_collaborators=None, require_client_auth=True, use_tls=True):
         """
         Modify the plan to train the model
         Args:
             new_rounds (int): Number of rounds to train
             num_collaborators (int): Number of collaborators
-            disable_client_auth (bool): Disable client authentication
-            disable_tls (bool): Disable TLS communication
+            require_client_auth (bool): Enable client authentication
+            use_tls (bool): Enable TLS communication
         Returns:
             bool: True if successful, else False
         """
@@ -139,8 +139,9 @@ def modify_plan(self, new_rounds=None, num_collaborators=None, disable_client_au
         data["collaborator"]["settings"]["log_memory_usage"] = self.log_memory_usage
 
         data["data_loader"]["settings"]["collaborator_count"] = int(self.num_collaborators)
-        data["network"]["settings"]["disable_client_auth"] = disable_client_auth
-        data["network"]["settings"]["tls"] = not disable_tls
+        data["network"]["settings"]["require_client_auth"] = require_client_auth
+        data["network"]["settings"]["use_tls"] = use_tls
+
 
         with open(self.plan_path, "w+") as write_file:
             yaml.dump(data, write_file)

diff --git a/tests/end_to_end/test_suites/task_runner_tests.py b/tests/end_to_end/test_suites/task_runner_tests.py
@@ -17,7 +17,7 @@ def test_torch_cnn_mnist(fx_federation):
     log.info("Testing torch_cnn_mnist model")
 
     # Setup PKI for trusted communication within the federation
-    if not fx_federation.disable_tls:
+    if fx_federation.use_tls:
         assert fed_helper.setup_pki(fx_federation), "Failed to setup PKI for trusted communication"
 
     # Start the federation
@@ -32,7 +32,7 @@ def test_keras_cnn_mnist(fx_federation):
     log.info("Testing keras_cnn_mnist model")
 
     # Setup PKI for trusted communication within the federation
-    if not fx_federation.disable_tls:
+    if fx_federation.use_tls:
         assert fed_helper.setup_pki(fx_federation), "Failed to setup PKI for trusted communication"
 
     # Start the federation
@@ -50,7 +50,7 @@ def test_torch_cnn_histology(fx_federation):
     log.info("Testing torch_cnn_histology model")
 
     # Setup PKI for trusted communication within the federation
-    if not fx_federation.disable_tls:
+    if fx_federation.use_tls:
         assert fed_helper.setup_pki(fx_federation), "Failed to setup PKI for trusted communication"
 
     # Start the federation

diff --git a/tests/end_to_end/utils/conftest_helper.py b/tests/end_to_end/utils/conftest_helper.py
@@ -27,7 +27,6 @@ def parse_arguments():
     """
     try:
         parser = argparse.ArgumentParser(description="Provide the required arguments to run the tests")
-        parser.add_argument("--results_dir", type=str, required=False, default="results", help="Directory to store the results")
         parser.add_argument("--num_collaborators", type=int, default=2, help="Number of collaborators")
         parser.add_argument("--num_rounds", type=int, default=5, help="Number of rounds to train")
         parser.add_argument("--model_name", type=str, help="Model name")