Find dependencies changes (demisto#31986)

* fixing the script

* fix artifacts path

* why space in dir name whyyy

* Apply suggestions from code review

Co-authored-by: Shahaf Ben Yakir <[email protected]>

---------

Co-authored-by: Shahaf Ben Yakir <[email protected]>

.gitlab/ci/.gitlab-ci.on-push.yml

@@ -168,6 +168,21 @@ validate-content-conf:
     - cp "./Tests/conf.json" "${ARTIFACTS_FOLDER_SERVER_TYPE}/conf.json"
     - section_end "Copy conf.json To Server Type Artifacts Folder"
 
+    - section_start "Find dependencies changes" --collapsed
+    - |
+      if [[ -z $BUCKET_UPLOAD || $TEST_UPLOAD == "false" ]]; then
+        source ./Tests/scripts/get_previous_master_sha.sh
+        if [[ -z $PREVIOUS_MASTER_SHA ]]; then
+          echo "WARNING: failed to detect previous master SHA, skipping find dependencies changes"
+        else
+          echo "Finding pack dependencies diff against $PREVIOUS_MASTER_SHA"
+          python Tests/scripts/find_pack_dependencies_changes.py --gitlab-token $GITLAB_API_TOKEN --master-sha $PREVIOUS_MASTER_SHA --job-name $CI_JOB_NAME --artifacts-folder "$ARTIFACTS_FOLDER_SERVER_TYPE"
+        fi
+      else
+        echo "Test upload flow - skipping find dependencies changes"
+      fi
+    - section_end "Find dependencies changes"
+
     - section_start "Replace Cortex XSOAR" --collapsed
     - |
       if [[ $MARKETPLACE_VERSION == "marketplacev2" ||  $MARKETPLACE_VERSION == "xpanse" ]];

Tests/scripts/find_pack_dependencies_changes.py

@@ -96,21 +96,29 @@ def get_diff(args: Namespace) -> dict:  # pragma: no cover
     absolute_artifacts_folder = Path(args.artifacts_folder)
     relative_artifacts_folder = absolute_artifacts_folder.relative_to(ARTIFACTS_DIR_LOCATION)
     gitlab_client = GitlabClient(args.gitlab_token)
-    previous = gitlab_client.get_packs_dependencies_json(
-        args.master_sha,
-        args.job_name,
-        relative_artifacts_folder / PACKS_DEPENDENCIES_FILENAME,
+    master_packs_dependencies_json = json.loads(
+        gitlab_client.get_artifact_file(
+            args.master_sha,
+            args.job_name,
+            relative_artifacts_folder / PACKS_DEPENDENCIES_FILENAME,
+            ref="master",
+        )
     )
-    current = json.loads((absolute_artifacts_folder / PACKS_DEPENDENCIES_FILENAME).read_text())
-    return compare(previous, current)
+    current_packs_dependencies_json = json.loads(
+        (absolute_artifacts_folder / PACKS_DEPENDENCIES_FILENAME).read_text()
+    )
+    return compare(master_packs_dependencies_json, current_packs_dependencies_json)
 
 
 def main():  # pragma: no cover
-    args = parse_args()
-    diff = get_diff(args)
-    diff_file = Path(args.artifacts_folder) / DIFF_FILENAME
-    logger.info(f"Dumping the diff to an artifact file: {diff_file}")
-    diff_file.write_text(json.dumps(diff, indent=4))
+    try:
+        args = parse_args()
+        diff = get_diff(args)
+        diff_file = Path(args.artifacts_folder) / DIFF_FILENAME
+        logger.info(f"Dumping the diff to an artifact file: {diff_file}")
+        diff_file.write_text(json.dumps(diff, indent=4))
+    except Exception as e:
+        logger.warning(f"Skipping pack dependencies calculation: \n{e}")
 
 
 if __name__ == '__main__':

Tests/scripts/gitlab_client.py

@@ -1,5 +1,3 @@
-import enum
-import json
 import os
 from tempfile import mkdtemp
 import zipfile
@@ -14,13 +12,6 @@
 PROJECT_ID = os.getenv("CI_PROJECT_ID", "1061")
 
 
-class GetArtifactErrors(str, enum.Enum):
-    NO_PIPELINES = "No pipelines for this SHA"
-    NO_JOB = "No jobs with the specified name"
-    NO_ARTIFACTS = "No artifacts in the specified job"
-    NO_FILE_IN_ARTIFACTS = "The specified file does not exist in the artifacts"
-
-
 class GitlabClient:
     def __init__(self, gitlab_token: str) -> None:
         self.base_url = f"{API_BASE_URL}/projects/{PROJECT_ID}"
@@ -40,8 +31,18 @@ def _get(
             return response.json()
         return response
 
-    def get_pipelines_by_sha(self, commit_sha: str) -> list:
-        return self._get(f"pipelines?sha={commit_sha}", to_json=True)
+    def get_pipelines(
+            self,
+            commit_sha: str = None,
+            ref: str = None,
+            sort: str = "asc",
+    ) -> list:
+        params = {
+            "sha": commit_sha,
+            "ref": ref,
+            "sort": sort,
+        }
+        return self._get("pipelines", params=params, to_json=True)
 
     def get_job_id_by_name(self, pipeline_id: str, job_name: str) -> str | None:
         response: list = self._get(f"pipelines/{pipeline_id}/jobs", to_json=True)
@@ -71,45 +72,43 @@ def get_artifact_file(
         commit_sha: str,
         job_name: str,
         artifact_filepath: Path,
+        ref: str = None,
     ) -> str:
         """Gets an artifact file data as text.
 
         Args:
             commit_sha (str): A commit SHA
             job_name (str): A job name
             artifact_filepath (Path): The artifact file path
+            ref (str): The branch name.
 
         Raises:
-            Exception: An exception message specifying the reasons for not returning the file data.
+            Exception: An exception message specifying the reasons for not returning the file data,
+            for each pipeline triggered for the given commit SHA.
 
         Returns:
             str: The artifact text data.
         """
-        pipeline_ids = [p["id"] for p in self.get_pipelines_by_sha(commit_sha)]
-        pid_to_err = {}
-        for pipeline_id in pipeline_ids:
-            if job_id := self.get_job_id_by_name(pipeline_id, job_name):
-                try:
-                    bundle_path = self.download_and_extract_artifacts_bundle(job_id)
-                    return (bundle_path / artifact_filepath).read_text()
-                except requests.HTTPError:
-                    pid_to_err[pipeline_id] = GetArtifactErrors.NO_ARTIFACTS.value
-                except FileNotFoundError:
-                    pid_to_err[pipeline_id] = GetArtifactErrors.NO_FILE_IN_ARTIFACTS.value
-            else:
-                pid_to_err[pipeline_id] = GetArtifactErrors.NO_JOB.value
-
-        raise Exception(
-            f"Could not extract {artifact_filepath.name} from any pipeline of SHA {commit_sha}. "
-            f"Err: {GetArtifactErrors.NO_PIPELINES.value if not pipeline_ids else pid_to_err}"
-        )
-
-    def get_packs_dependencies_json(
-        self,
-        commit_sha: str,
-        job_name: str,
-        packs_dependencies_filepath: Path,
-    ) -> dict:
-        return json.loads(
-            self.get_artifact_file(commit_sha, job_name, packs_dependencies_filepath)
-        )
+        try:
+            pipelines = self.get_pipelines(commit_sha=commit_sha, ref=ref)
+            if not pipelines:
+                raise Exception("No pipelines found for this SHA")
+            errors = []
+            for pipeline in pipelines:
+                pid = pipeline["id"]
+                if job_id := self.get_job_id_by_name(pid, job_name):
+                    try:
+                        bundle_path = self.download_and_extract_artifacts_bundle(job_id)
+                        return (bundle_path / artifact_filepath).read_text()
+                    except requests.HTTPError:
+                        errors.append(f"Pipeline #{pid}: No artifacts in job {job_name}")
+                    except FileNotFoundError:
+                        errors.append(f"Pipeline #{pid}: The file {artifact_filepath} does not exist in the artifacts")
+                else:
+                    errors.append(f"Pipeline #{pid}: No job with the name {job_name}")
+            raise Exception("\n".join(errors))
+
+        except Exception as e:
+            raise Exception(
+                f"Could not extract {artifact_filepath.name} from any pipeline with SHA {commit_sha}:\n{e}"
+            )

Tests/scripts/infrastructure_tests/gitlab_client_test.py

@@ -6,7 +6,7 @@
 
 import pytest
 
-from Tests.scripts.gitlab_client import GitlabClient, GetArtifactErrors
+from Tests.scripts.gitlab_client import GitlabClient
 
 
 SHA = "mock_sha"
@@ -34,23 +34,23 @@ def mock_artifacts_api_response(
     return mock_bytes.getvalue()
 
 
-def test_get_packs_dependencies(
+def test_get_artifact_file(
     client: GitlabClient,
     requests_mock,
 ) -> None:
     """
         Given:
             - A Gitlab Client
             - A Commit SHA
-            - The job name in which a packs_dependencies.json should be stored as an artifact
+            - The job name in which a packs_dependencies.json file should be stored as an artifact
         When:
-            - Calling get_packs_dependencies_json()
+            - Calling get_artifact_file()
         Then:
             - Ensure the response is the expected data.
     """
     packs_dependencies_json: dict = {}
     requests_mock.get(
-        f"{client.base_url}/pipelines?sha={SHA}",
+        f"{client.base_url}/pipelines",
         json=[{"id": "mock_pipeline_id"}],
     )
     requests_mock.get(
@@ -61,11 +61,11 @@ def test_get_packs_dependencies(
         f"{client.base_url}/jobs/mock_job_id/artifacts",
         content=mock_artifacts_api_response(packs_dependencies_json),
     )
-    assert client.get_packs_dependencies_json(
+    assert json.loads(client.get_artifact_file(
         SHA,
         JOB_NAME,
         PACKS_DEPENDENCIES_FILEPATH,
-    ) == packs_dependencies_json
+    )) == packs_dependencies_json
 
 
 @pytest.mark.parametrize(
@@ -75,54 +75,54 @@ def test_get_packs_dependencies(
             [],
             None,
             None,
-            GetArtifactErrors.NO_PIPELINES,
+            "No pipelines",
             id="No Pipelines",
         ),
         pytest.param(
             [{"id": "mock_pipeline_id"}],
             [{"id": "mock_job_id", "name": "some_job"}],
             None,
-            GetArtifactErrors.NO_JOB,
+            "No job",
             id="No Job",
         ),
         pytest.param(
             [{"id": "mock_pipeline_id"}],
             [{"id": "mock_job_id", "name": JOB_NAME}],
             {"status_code": 404},
-            GetArtifactErrors.NO_ARTIFACTS,
+            "No artifacts",
             id="No artifacts",
         ),
         pytest.param(
             [{"id": "mock_pipeline_id"}],
             [{"id": "mock_job_id", "name": JOB_NAME}],
             {"content": mock_artifacts_api_response(data=None)},
-            GetArtifactErrors.NO_FILE_IN_ARTIFACTS,
+            "does not exist in the artifacts",
             id="No pack_dependencies.json file in artifacts",
         ),
     ]
 )
-def test_get_packs_dependencies_bad(
+def test_get_artifact_file_bad(
     client: GitlabClient,
     requests_mock: Any,
     pipelines_mock_response: list | None,
     jobs_mock_response: list | None,
     artifacts_mock_repsonse: dict | None,
-    expected_err: GetArtifactErrors,
+    expected_err: str,
 ) -> None:
     """
         Given:
             - A Gitlab Client
             - A Commit SHA
-            - The job name in which a packs_dependencies.json should be stored as an artifact
+            - The job name in which a packs_dependencies.json file should be stored as an artifact
             - A marketplace version
             - Test cases for different Gitlab API responses.
         When:
-            - Calling get_packs_dependencies_json()
+            - Calling get_artifact_file()
         Then:
             - Ensure an exception is raised for all test cases.
     """
     requests_mock.get(
-        f"{client.base_url}/pipelines?sha={SHA}",
+        f"{client.base_url}/pipelines",
         json=pipelines_mock_response,
     )
     requests_mock.get(
@@ -135,9 +135,9 @@ def test_get_packs_dependencies_bad(
             **artifacts_mock_repsonse,
         )
     with pytest.raises(Exception) as e:
-        client.get_packs_dependencies_json(
+        client.get_artifact_file(
             SHA,
             JOB_NAME,
             PACKS_DEPENDENCIES_FILEPATH,
         )
-    assert expected_err.value in str(e)
+    assert expected_err in str(e)