From 383be21ff4783d559fc36df5392f79807602633f Mon Sep 17 00:00:00 2001 From: Dan Tavori <38749041+dantavori@users.noreply.github.com> Date: Thu, 1 Feb 2024 10:37:40 +0200 Subject: [PATCH] Find dependencies changes (#31986) * fixing the script * fix artifacts path * why space in dir name whyyy * Apply suggestions from code review Co-authored-by: Shahaf Ben Yakir <44666568+ShahafBenYakir@users.noreply.github.com> --------- Co-authored-by: Shahaf Ben Yakir <44666568+ShahafBenYakir@users.noreply.github.com> --- .gitlab/ci/.gitlab-ci.on-push.yml | 15 ++++ .../scripts/find_pack_dependencies_changes.py | 30 ++++--- Tests/scripts/get_previous_master_sha.sh | 0 Tests/scripts/gitlab_client.py | 79 +++++++++---------- .../gitlab_client_test.py | 36 ++++----- 5 files changed, 91 insertions(+), 69 deletions(-) mode change 100644 => 100755 Tests/scripts/get_previous_master_sha.sh diff --git a/.gitlab/ci/.gitlab-ci.on-push.yml b/.gitlab/ci/.gitlab-ci.on-push.yml index 04bc1a6b3e88..29cbeeefbe2a 100644 --- a/.gitlab/ci/.gitlab-ci.on-push.yml +++ b/.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" ]]; diff --git a/Tests/scripts/find_pack_dependencies_changes.py b/Tests/scripts/find_pack_dependencies_changes.py index dda528a3f4ba..4f5b124e7d36 100644 --- a/Tests/scripts/find_pack_dependencies_changes.py +++ b/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__': diff --git a/Tests/scripts/get_previous_master_sha.sh b/Tests/scripts/get_previous_master_sha.sh old mode 100644 new mode 100755 diff --git a/Tests/scripts/gitlab_client.py b/Tests/scripts/gitlab_client.py index e16ac34fa78a..62f9cdbf69e4 100644 --- a/Tests/scripts/gitlab_client.py +++ b/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,6 +72,7 @@ def get_artifact_file( commit_sha: str, job_name: str, artifact_filepath: Path, + ref: str = None, ) -> str: """Gets an artifact file data as text. @@ -78,38 +80,35 @@ def get_artifact_file( 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}" + ) diff --git a/Tests/scripts/infrastructure_tests/gitlab_client_test.py b/Tests/scripts/infrastructure_tests/gitlab_client_test.py index 33fc931f7aca..43e729505971 100644 --- a/Tests/scripts/infrastructure_tests/gitlab_client_test.py +++ b/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,7 +34,7 @@ def mock_artifacts_api_response( return mock_bytes.getvalue() -def test_get_packs_dependencies( +def test_get_artifact_file( client: GitlabClient, requests_mock, ) -> None: @@ -42,15 +42,15 @@ def test_get_packs_dependencies( 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)