Merge pull request #15151 from ethereum/gh-token-download-benchmarks-script

Add authentication header to circleci api

Author: r0qs

.circleci/config.yml

@@ -1533,6 +1533,19 @@ jobs:
       - run:
           name: Summarize reports
           command: cat reports/externalTests/all-benchmarks.json | scripts/externalTests/summarize_benchmarks.sh > reports/externalTests/summarized-benchmarks.json
+      - store_artifacts:
+          path: reports/externalTests/all-benchmarks.json
+      - store_artifacts:
+          path: reports/externalTests/summarized-benchmarks.json
+      - run:
+          name: Check CircleCI token presence; Skip remaining steps if the token is not present.
+          command: |
+            # NOTE: download_benchmarks.py requires CIRCLECI_TOKEN environment variable to be set to
+            # a valid CircleCI API token to download the benchmark artifacts.
+            if [[ -z "$CIRCLECI_TOKEN" ]]; then
+              echo "Skipping download benchmarks..."
+              circleci-agent step halt
+            fi
       - run:
           name: Download reports from base branch
           command: |
@@ -1577,10 +1590,6 @@ jobs:
                 base-branch/all-benchmarks-*.json \
                 all-benchmarks.json > diff/benchmark-diff-all-table-inplace-absolute.md
             fi
-      - store_artifacts:
-          path: reports/externalTests/all-benchmarks.json
-      - store_artifacts:
-          path: reports/externalTests/summarized-benchmarks.json
       - store_artifacts:
           path: reports/externalTests/diff/
       - store_artifacts:

scripts/common/rest_api_helpers.py

@@ -1,3 +1,4 @@
+from os import environ
 from pathlib import Path
 from typing import List, Mapping, Optional
 import functools
@@ -42,13 +43,13 @@ class FileAlreadyExists(APIHelperError):
     pass
 
 
-def query_api(url: str, params: Mapping[str, str], debug_requests=False) -> dict:
+def query_api(url: str, params: Mapping[str, str], headers: dict, debug_requests=False) -> dict:
     if debug_requests:
         print(f'REQUEST URL: {url}')
         if len(params) > 0:
             print(f'QUERY: {params}')
 
-    response = requests.get(url, params=params, timeout=60)
+    response = requests.get(url, params=params, headers=headers, timeout=60)
     response.raise_for_status()
 
     if debug_requests:
@@ -63,11 +64,11 @@ def query_api(url: str, params: Mapping[str, str], debug_requests=False) -> dict
     return response.json()
 
 
-def download_file(url: str, target_path: Path, overwrite=False):
+def download_file(url: str, target_path: Path, headers: dict, overwrite=False):
     if not overwrite and target_path.exists():
         raise FileAlreadyExists(f"Refusing to overwrite existing file: '{target_path}'.")
 
-    with requests.get(url, stream=True, timeout=60) as request:
+    with requests.get(url, headers, stream=True, timeout=60) as request:
         with open(target_path, 'wb') as target_file:
             shutil.copyfileobj(request.raw, target_file)
 
@@ -86,6 +87,7 @@ def pull_request(self, pr_id: int) -> dict:
         return query_api(
             f'{self.BASE_URL}/repos/{self.project_slug}/pulls/{pr_id}',
             {},
+            {},
             self.debug_requests
         )
 
@@ -108,11 +110,12 @@ def paginated_query_api_iterator(self, url: str, params: Mapping[str, str], max_
 
         page_count = 0
         next_page_token = None
+        headers = {'Circle-Token': str(environ.get('CIRCLECI_TOKEN'))} if 'CIRCLECI_TOKEN' in environ else {}
         while max_pages is None or page_count < max_pages:
             if next_page_token is not None:
                 params = {**params, 'page-token': next_page_token}
 
-            json_response = query_api(url, params, self.debug_requests)
+            json_response = query_api(url, params, headers, self.debug_requests)
 
             yield json_response['items']
             next_page_token = json_response['next_page_token']

scripts/externalTests/download_benchmarks.py

@@ -2,7 +2,9 @@
 
 from argparse import ArgumentParser, Namespace
 from enum import Enum, unique
+from os import environ
 from pathlib import Path
+from textwrap import dedent
 from typing import Mapping, Optional
 import sys
 
@@ -28,8 +30,13 @@ class Status(Enum):
 
 def process_commandline() -> Namespace:
     script_description = (
-        "Downloads benchmark results attached as artifacts to the c_ext_benchmarks job on CircleCI. "
-        "If no options are specified, downloads results for the currently checked out git branch."
+        """
+        Downloads benchmark results attached as artifacts to the c_ext_benchmarks job on CircleCI.
+        If no options are specified, downloads results for the currently checked out git branch.
+
+        The script requires the CIRCLECI_TOKEN environment variable to be set with a valid CircleCI API token.
+        You can generate a new token at https://app.circleci.com/settings/user/tokens.
+        """
     )
 
     parser = ArgumentParser(description=script_description)
@@ -96,9 +103,11 @@ def download_benchmark_artifact(
             print(f"Missing artifact: {artifact_path}.")
         return False
 
+    headers = {'Circle-Token': str(environ.get('CIRCLECI_TOKEN'))} if 'CIRCLECI_TOKEN' in environ else {}
     download_file(
         artifacts[artifact_path]['url'],
         Path(f'{benchmark_name}-{branch}-{commit_hash[:8]}.json'),
+        headers,
         overwrite,
     )
 
@@ -162,6 +171,13 @@ def download_benchmarks(
 
 def main():
     try:
+        if 'CIRCLECI_TOKEN' not in environ:
+            raise RuntimeError(
+               dedent(""" \
+                   CIRCLECI_TOKEN environment variable required but not set.
+                   Please generate a new token at https://app.circleci.com/settings/user/tokens and set CIRCLECI_TOKEN.
+                   """)
+            )
         options = process_commandline()
         return download_benchmarks(
             options.branch,

test/scripts/test_externalTests_benchmark_downloader.py

@@ -3,6 +3,8 @@
 from pathlib import Path
 from unittest import TestCase
 from unittest.mock import call, Mock, patch
+import os
+import requests
 
 # NOTE: This test file file only works with scripts/ added to PYTHONPATH so pylint can't find the imports
 # pragma pylint: disable=import-error
@@ -31,7 +33,7 @@ def _git_run_command_mock(command):
         "If you have updated the code, please remember to add matching command fixtures above."
     )
 
-def _requests_get_mock(url, params, timeout):
+def _requests_get_mock(url, params, headers, timeout):
     response_mock = Mock()
 
     if url == 'https://api.github.com/repos/ethereum/solidity/pulls/12818':
@@ -155,25 +157,37 @@ def _requests_get_mock(url, params, timeout):
         return response_mock
 
     if url == 'https://circleci.com/api/v2/project/gh/ethereum/solidity/1018023/artifacts':
-        response_mock.json.return_value = {
-            "next_page_token": None,
-            "items": [
-                {
-                    "path": "reports/externalTests/all-benchmarks.json",
-                    "url": "https://circle-artifacts.com/0/reports/externalTests/all-benchmarks.json"
-                },
-                {
-                    "path": "reports/externalTests/summarized-benchmarks.json",
-                    "url": "https://circle-artifacts.com/0/reports/externalTests/summarized-benchmarks.json"
-                }
-            ]
-        }
+        if (
+            os.environ.get('CIRCLECI_TOKEN') == 'valid_token' and
+            headers.get('Circle-Token') == os.environ.get('CIRCLECI_TOKEN')
+        ):
+            response_mock.json.return_value = {
+                "next_page_token": None,
+                "items": [
+                    {
+                        "path": "reports/externalTests/all-benchmarks.json",
+                        "url": "https://circle-artifacts.com/0/reports/externalTests/all-benchmarks.json"
+                    },
+                    {
+                        "path": "reports/externalTests/summarized-benchmarks.json",
+                        "url": "https://circle-artifacts.com/0/reports/externalTests/summarized-benchmarks.json"
+                    }
+                ]
+            }
+        else:
+            response_mock.status_code = 401
+            response_mock.json.return_value = {
+                "message": "Unauthorized"
+            }
+            error = requests.exceptions.HTTPError(f"401 Client Error: Unauthorized for url: {url}")
+            response_mock.raise_for_status.side_effect = error
         return response_mock
 
     raise RuntimeError(
         "The test tried to perform an unexpected GET request.\n"
         f"URL: {url}\n" +
         (f"query: {params}\n" if len(params) > 0 else "") +
+        (f"headers: {headers}\n" if len(headers) > 0 else "") +
         f"timeout: {timeout}\n" +
         "If you have updated the code, please remember to add matching response fixtures above."
     )
@@ -186,17 +200,20 @@ def setUp(self):
     @patch('externalTests.download_benchmarks.download_file')
     @patch('requests.get', _requests_get_mock)
     @patch('common.git_helpers.run_git_command',_git_run_command_mock)
+    @patch.dict(os.environ, {'CIRCLECI_TOKEN': 'valid_token'})
     def test_download_benchmarks(download_file_mock):
         download_benchmarks(None, None, None, silent=True)
         download_file_mock.assert_has_calls([
             call(
                 'https://circle-artifacts.com/0/reports/externalTests/summarized-benchmarks.json',
                 Path('summarized-benchmarks-benchmark-downloader-fa1ddc6f.json'),
+                {'Circle-Token': 'valid_token'},
                 False
             ),
             call(
                 'https://circle-artifacts.com/0/reports/externalTests/all-benchmarks.json',
                 Path('all-benchmarks-benchmark-downloader-fa1ddc6f.json'),
+                {'Circle-Token': 'valid_token'},
                 False
             ),
         ])
@@ -205,17 +222,20 @@ def test_download_benchmarks(download_file_mock):
     @patch('externalTests.download_benchmarks.download_file')
     @patch('requests.get', _requests_get_mock)
     @patch('common.git_helpers.run_git_command',_git_run_command_mock)
+    @patch.dict(os.environ, {'CIRCLECI_TOKEN': 'valid_token'})
     def test_download_benchmarks_branch(download_file_mock):
         download_benchmarks('develop', None, None, silent=True)
         download_file_mock.assert_has_calls([
             call(
                 'https://circle-artifacts.com/0/reports/externalTests/summarized-benchmarks.json',
                 Path('summarized-benchmarks-develop-43f29c00.json'),
+                {'Circle-Token': 'valid_token'},
                 False
             ),
             call(
                 'https://circle-artifacts.com/0/reports/externalTests/all-benchmarks.json',
                 Path('all-benchmarks-develop-43f29c00.json'),
+                {'Circle-Token': 'valid_token'},
                 False
             ),
         ])
@@ -224,17 +244,20 @@ def test_download_benchmarks_branch(download_file_mock):
     @patch('externalTests.download_benchmarks.download_file')
     @patch('requests.get', _requests_get_mock)
     @patch('common.git_helpers.run_git_command',_git_run_command_mock)
+    @patch.dict(os.environ, {'CIRCLECI_TOKEN': 'valid_token'})
     def test_download_benchmarks_pr(download_file_mock):
         download_benchmarks(None, 12818, None, silent=True)
         download_file_mock.assert_has_calls([
             call(
                 'https://circle-artifacts.com/0/reports/externalTests/summarized-benchmarks.json',
                 Path('summarized-benchmarks-benchmark-downloader-fa1ddc6f.json'),
+                {'Circle-Token': 'valid_token'},
                 False
             ),
             call(
                 'https://circle-artifacts.com/0/reports/externalTests/all-benchmarks.json',
                 Path('all-benchmarks-benchmark-downloader-fa1ddc6f.json'),
+                {'Circle-Token': 'valid_token'},
                 False
             ),
         ])
@@ -243,17 +266,30 @@ def test_download_benchmarks_pr(download_file_mock):
     @patch('externalTests.download_benchmarks.download_file')
     @patch('requests.get', _requests_get_mock)
     @patch('common.git_helpers.run_git_command',_git_run_command_mock)
+    @patch.dict(os.environ, {'CIRCLECI_TOKEN': 'valid_token'})
     def test_download_benchmarks_base_of_pr(download_file_mock):
         download_benchmarks(None, None, 12818, silent=True)
         download_file_mock.assert_has_calls([
             call(
                 'https://circle-artifacts.com/0/reports/externalTests/summarized-benchmarks.json',
                 Path('summarized-benchmarks-develop-43f29c00.json'),
+                {'Circle-Token': 'valid_token'},
                 False
             ),
             call(
                 'https://circle-artifacts.com/0/reports/externalTests/all-benchmarks.json',
                 Path('all-benchmarks-develop-43f29c00.json'),
+                {'Circle-Token': 'valid_token'},
                 False
             ),
         ])
+
+    # NOTE: No circleci token is set in the environment
+    @patch('externalTests.download_benchmarks.download_file')
+    @patch('requests.get', _requests_get_mock)
+    @patch('common.git_helpers.run_git_command',_git_run_command_mock)
+    def test_download_benchmarks_unauthorized_request(self, _):
+        with self.assertRaises(requests.exceptions.HTTPError) as manager:
+            download_benchmarks(None, None, None, silent=True)
+
+        self.assertIn('401 Client Error: Unauthorized', str(manager.exception))