Fixed permission issue that's causing backwards compatibility test ru…

…n to fail & made logic to run prepare mode tests more robust (#1049)

1. Changed overly permissive permissions to more fine grained permissions 
2. Started using pytest to identify markers instead of getattr()

.github/workflows/backwards_compatibility_marqo_execution.yml

@@ -41,7 +41,12 @@ on:
 
 jobs:
   Start-Runner:
-    permissions: write-all
+    permissions:
+      contents: read # This permission is necessary to read repository contents
+      actions: write # Used by machulav/ec2-github-runner@v2 for managing self-hosted runners. The workflow needs to create and manage GitHub Actions runners on EC2
+      id-token: write # Used by aws-actions/configure-aws-credentials@v4. Required for AWS authentication and OIDC token management
+      checks: write # Used implicitly by GitHub Actions to report job statuses and create check runs
+      statuses: write # Used implicitly by GitHub Actions to report job statuses and create check runs
     name: Start self-hosted EC2 runner
     runs-on: ubuntu-latest
     outputs:
@@ -126,6 +131,12 @@ jobs:
 
   Stop-Runner:
     name: Stop self-hosted EC2 runner
+    permissions:
+      contents: read # This permission is necessary to read repository contents
+      actions: write # Used by machulav/ec2-github-runner@v2 for managing self-hosted runners. The workflow needs to create and manage GitHub Actions runners on EC2
+      id-token: write # Used by aws-actions/configure-aws-credentials@v4. Required for AWS authentication and OIDC token management
+      checks: write # Used implicitly by GitHub Actions to report job statuses and create check runs
+      statuses: write # Used implicitly by GitHub Actions to report job statuses and create check runs
     needs:
       - Start-Runner # required to get output from the start-runner job
       - backwards_compatibility # required to wait when the main job is done

.github/workflows/backwards_compatibility_marqo_orchestrator.yml

@@ -54,7 +54,7 @@ jobs:
             echo "Image already exists in ECR, will not build and push again. Will be using the image digest from existing image"
             
             IMAGE_IDENTIFIER=$(echo "$IMAGE_DETAILS" | jq -r '.imageDetails[0].imageDigest')
-            REGISTRY_ID = "424082663841.dkr.ecr.us-east-1.amazonaws.com"
+            REGISTRY_ID="424082663841.dkr.ecr.us-east-1.amazonaws.com"
             FULL_IDENTIFIER="${REGISTRY_ID}/marqo-compatibility-tests@${IMAGE_IDENTIFIER}"
             echo "image_identifier=${FULL_IDENTIFIER}" >> $GITHUB_OUTPUT
           else
@@ -144,6 +144,12 @@ jobs:
         from_version: ${{ fromJson(needs.orchestrate.outputs.list) }}
     uses: ./.github/workflows/backwards_compatibility_marqo_execution.yml
     secrets: inherit
+    permissions:
+      contents: read # This permission is necessary to read repository contents
+      actions: write # Used by machulav/ec2-github-runner@v2 for managing self-hosted runners. The workflow needs to create and manage GitHub Actions runners on EC2
+      id-token: write # Used by aws-actions/configure-aws-credentials@v4. Required for AWS authentication and OIDC token management
+      checks: write # Used implicitly by GitHub Actions to report job statuses and create check runs
+      statuses: write # Used implicitly by GitHub Actions to report job statuses and create check runs
     with:
       from_version: ${{ matrix.from_version }}
       to_version: ${{ needs.orchestrate.outputs.to_version }}

tests/backwards_compatibility_tests/compatibility_test_runner.py

@@ -492,15 +492,39 @@ def full_test_run(to_version: str):
     run_test_mode(to_version)
 
 def run_prepare_mode(version_to_test_against: str):
+    version_to_test_against = semver.VersionInfo.parse(version_to_test_against)
     load_all_subclasses("tests.backwards_compatibility_tests")
     # Get all subclasses of `BaseCompatibilityTestCase` that match the `version_to_test_against` criterion
     # The below condition also checks if the test class is not marked to be skipped
-    tests = [test_class for test_class in BaseCompatibilityTestCase.__subclasses__()
-             if (getattr(test_class, 'marqo_version', '0') <= version_to_test_against and getattr(test_class, 'skip', False) == False)]
-    for test_class in tests:
-        test_class.setUpClass() #setUpClass will be used to create Marqo client
-        test_instance = test_class()
-        test_instance.prepare() #Prepare method will be used to create index and add documents
+    for test_class in BaseCompatibilityTestCase.__subclasses__():
+        markers = getattr(test_class, "pytestmark", [])
+        # Check for specific markers
+        marqo_version_marker = next( # Checks what version a compatibility test is marked with (ex: @pytest.mark.marqo_version('2.11.0')). If no version is marked, it will skip the test
+            (marker for marker in markers if marker.name == "marqo_version"),
+            None
+        )
+        skip_marker = next( # Checks if a compatibility test is marked with @pytest.mark.skip
+            (marker for marker in markers if marker.name == "skip"),
+            None
+        )
+        # To check for cases if a test case is not marked with marqo_version OR if it is marked with skip. In that case we skip running prepare mode on that test case.
+        if not marqo_version_marker or skip_marker:
+            if not marqo_version_marker:
+                logger.error(f"No marqo_version marker detected for class {test_class.__name__}, skipping prepare mode for this test class")
+            elif skip_marker:
+                logger.error(f"Detected 'skip' marker for class {test_class.__name__}, skipping prepare mode for this test class")
+            continue
+
+        marqo_version = marqo_version_marker.args[0]
+        logger.debug(f"Detected marqo_version '{marqo_version}' for class {test_class.__name__}")
+
+        if semver.VersionInfo.parse(marqo_version).compare(version_to_test_against) <= 0:
+            logger.debug(f"Running prepare mode on test: {test_class.__name__} with version: {marqo_version}")
+            test_class.setUpClass() #setUpClass will be used to create Marqo client
+            test_instance = test_class()
+            test_instance.prepare() #Prepare method will be used to create index and add documents
+        else: # Skip the test if the version_to_test_against is greater than the version the test is marked
+            logger.debug(f"Skipping testcase {test_class.__name__} with version {marqo_version} as it is greater than {version_to_test_against}")
 
 def construct_pytest_arguments(version_to_test_against):
     pytest_args = [

tests/backwards_compatibility_tests/conftest.py

@@ -27,6 +27,4 @@ def pytest_collection_modifyitems(config, items):
             logger.debug(f"Test version: {test_case_version}, v/s version supplied in pytest arguments: {version}")
             test_case_version = semver.VersionInfo.parse(test_case_version)
             if test_case_version > version:
-                item.add_marker(pytest.mark.skip(reason=f"Test requires marqo_version {test_case_version} which is not greater than version supplied {version}. Skipping."))
-
-    logger.debug(f"Total collected tests: {len(items)}")
+                item.add_marker(pytest.mark.skip(reason=f"Test requires marqo_version {test_case_version} which is not greater than version supplied {version}. Skipping."))