From 56d676969713b8932d283b3a7db813b0e430387c Mon Sep 17 00:00:00 2001 From: Ivan Barba Date: Tue, 21 Jul 2026 07:22:08 +0000 Subject: [PATCH 1/4] Migrate Android artifact fetcher to V4 client API --- .../platforms/android/fetch_artifact.py | 222 +++++++++--------- .../platforms/android/fetch_artifact_test.py | 109 +++++++++ 2 files changed, 218 insertions(+), 113 deletions(-) create mode 100644 src/clusterfuzz/_internal/tests/core/platforms/android/fetch_artifact_test.py diff --git a/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py b/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py index e86cced06c6..58f52a4d885 100644 --- a/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py +++ b/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py @@ -28,6 +28,8 @@ from clusterfuzz._internal.config import db_config from clusterfuzz._internal.google_cloud_utils import storage from clusterfuzz._internal.metrics import logs +from clusterfuzz._internal.platforms.android.android_build_v4_api import \ + AndroidBuildV4Api from clusterfuzz._internal.system import environment from . import adb @@ -38,6 +40,11 @@ # Maximum number of retries for artifact access. MAX_RETRIES = 5 +ANDROID_BUILD_API_SCOPES = [ + 'https://www.googleapis.com/auth/androidbuild.internal', + 'https://www.googleapis.com/auth/cloud-platform' +] + STABLE_CUTTLEFISH_BUILD = { 'bid': '11655237', 'branch': 'git_main', @@ -64,7 +71,7 @@ def _use_v4(): return False -def execute_request_with_retries(request): +def _execute_request_with_retries(request): """Executes request and retries on failure.""" result = None for _ in range(MAX_RETRIES): @@ -77,8 +84,8 @@ def execute_request_with_retries(request): return result -def download_artifact(client, bid, target, attempt_id, name, output_directory, - output_filename): +def _download_artifact(client, bid, target, attempt_id, name, output_directory, + output_filename): """Download one artifact.""" logs.info('reached download_artifact') logs.info('artifact to download: %s' % name) @@ -96,12 +103,12 @@ def download_artifact(client, bid, target, attempt_id, name, output_directory, artifact_name=name) if _use_v4(): - artifact_query = client.buildartifacts().get( - buildId=bid, target=target, attemptId=attempt_id, resourceId=name) + artifact = client.get_artifact_metadata(bid, target, attempt_id, name) else: artifact_query = client.buildartifact().get( buildId=bid, target=target, attemptId=attempt_id, resourceId=name) - artifact = execute_request_with_retries(artifact_query) + artifact = _execute_request_with_retries(artifact_query) + if artifact is None: logs.error( 'AndroidBuildAPI download_artifact failed: artifact metadata ' @@ -131,22 +138,6 @@ def download_artifact(client, bid, target, attempt_id, name, output_directory, if size >= DEFAULT_CHUNK_SIZE: chunksize = DEFAULT_CHUNK_SIZE - # Just like get, except get_media. - logs.info( - 'AndroidBuildAPI download_artifact media download started.', - api_version=version_tag, - operation='download_artifact_media', - build_id=bid, - target=target, - attempt_id=attempt_id, - artifact_name=name) - if _use_v4(): - dl_request = client.buildartifacts().get_media( - buildId=bid, target=target, attemptId=attempt_id, resourceId=name) - else: - dl_request = client.buildartifact().get_media( - buildId=bid, target=target, attemptId=attempt_id, resourceId=name) - if output_filename: file_name = output_filename else: @@ -172,20 +163,49 @@ def download_artifact(client, bid, target, attempt_id, name, output_directory, logs.info('Output dir: %s' % output_dir) if not os.path.exists(output_dir): logs.info(f'Creating directory {output_dir}') - os.mkdir(output_dir) + os.makedirs(output_dir, exist_ok=True) + + # Just like get, except get_media. + logs.info( + 'AndroidBuildAPI download_artifact media download started.', + api_version=version_tag, + operation='download_artifact_media', + build_id=bid, + target=target, + attempt_id=attempt_id, + artifact_name=name) + + if _use_v4(): + success = client.download_artifact_file(bid, target, attempt_id, name, + output_path) + if not success: + logs.error( + 'AndroidBuildAPI download_artifact failed for V4.', + api_version=version_tag, + operation='download_artifact', + build_id=bid, + target=target, + attempt_id=attempt_id, + artifact_name=name, + output_path=output_path, + status='failed') + return None + else: + dl_request = client.buildartifact().get_media( + buildId=bid, target=target, attemptId=attempt_id, resourceId=name) - with io.FileIO(output_path, mode='wb') as file_handle: - downloader = apiclient.http.MediaIoBaseDownload( - file_handle, dl_request, chunksize=chunksize) - done = False + with io.FileIO(output_path, mode='wb') as file_handle: + downloader = apiclient.http.MediaIoBaseDownload( + file_handle, dl_request, chunksize=chunksize) + done = False - while not done: - status, done = downloader.next_chunk() - if status: - size_completed = int(status.resumable_progress) - if size != 0: - percent_completed = (size_completed * 100.0) / size - logs.info('%.1f%% complete.' % percent_completed) + while not done: + status, done = downloader.next_chunk() + if status: + size_completed = int(status.resumable_progress) + if size != 0: + percent_completed = (size_completed * 100.0) / size + logs.info('%.1f%% complete.' % percent_completed) logs.info( 'AndroidBuildAPI download_artifact completed successfully.', @@ -200,12 +220,17 @@ def download_artifact(client, bid, target, attempt_id, name, output_directory, return output_path -def get_artifacts_for_build(client, - bid: str, - target: str, - attempt_id: str = 'latest', - regexp: Optional[str] = None) -> List[str]: +def _get_artifacts_for_build(client, + bid: str, + target: str, + attempt_id: str = 'latest', + regexp: Optional[str] = None) -> List[str]: """Return list of artifacts for a given build.""" + if not regexp: + logs.warning( + 'Regexp is empty, returning early to avoid querying all artifacts.') + return [] + version_tag = 'V4' if _use_v4() else 'V3' logs.info( 'AndroidBuildAPI get_artifacts_for_build started.', @@ -216,46 +241,27 @@ def get_artifacts_for_build(client, attempt_id=attempt_id, regexp=regexp) - if _use_v4(): - if not regexp: - request = client.buildartifacts().list( - buildId=bid, target=target, attemptId=attempt_id) - else: - request = client.buildartifacts().list( - buildId=bid, - target=target, - attemptId=attempt_id, - nameRegexp=regexp, - maxResults=100) - else: - if not regexp: - request = client.buildartifact().list( - buildId=bid, target=target, attemptId=attempt_id) - else: - request = client.buildartifact().list( - buildId=bid, - target=target, - attemptId=attempt_id, - nameRegexp=regexp, - maxResults=100) - - request_str = (f'{request.uri}, {request.method}, ' - f'{request.body}, {request.methodId}') - artifacts = [] - results = [] - while request: - result = execute_request_with_retries(request) - if not result: - break - results.append(result) - if result and 'artifacts' in result: - for artifact in result['artifacts']: - artifacts.append(artifact) - if _use_v4(): - request = client.buildartifacts().list_next(request, result) - else: + + if _use_v4(): + artifacts = client.list_artifacts(bid, target, attempt_id, regexp=regexp) + else: + request = client.buildartifact().list( + buildId=bid, + target=target, + attemptId=attempt_id, + nameRegexp=regexp, + maxResults=100) + + while request: + result = _execute_request_with_retries(request) + if not result: + break + results.append(result) + if result and 'artifacts' in result: + for artifact in result['artifacts']: + artifacts.append(artifact) request = client.buildartifact().list_next(request, result) logs.info( @@ -271,13 +277,13 @@ def get_artifacts_for_build(client, if not artifacts: logs.error(f'No artifact found for target {target}, build id {bid}.\n' - f'request {request_str}, results {results}') + f'results {results}') adb.bad_state_reached() return artifacts -def get_client(): +def _get_client(): """Return client with connection to build apiary.""" # Connect using build apiary service account credentials. build_apiary_service_account_private_key = db_config.get_value( @@ -289,20 +295,15 @@ def get_client(): credentials = ServiceAccountCredentials.from_json_keyfile_dict( json.loads(build_apiary_service_account_private_key), - scopes='https://www.googleapis.com/auth/androidbuild.internal') + scopes=ANDROID_BUILD_API_SCOPES) + + logs.info( + 'AndroidBuildAPI client initialization started.', + api_version='V4' if _use_v4() else 'V3') + if _use_v4(): - logs.info( - 'AndroidBuildAPI client initialization started.', api_version='V4') - client = apiclient.discovery.build( - 'androidbuildinternal', - 'v4', - discoveryServiceUrl= - 'https://androidbuild-pa.googleapis.com/$discovery/rest?version=v4', - credentials=credentials, - static_discovery=False) + client = AndroidBuildV4Api.create_authenticated(credentials) else: - logs.info( - 'AndroidBuildAPI client initialization started.', api_version='V3') client = apiclient.discovery.build( 'androidbuildinternal', 'v3', @@ -312,7 +313,7 @@ def get_client(): return client -def get_stable_build_info(): +def _get_stable_build_info(): """Return stable artifact for cuttlefish branch and target.""" logs.info('Reached get_stable_build_info') stable_build_info = STABLE_CUTTLEFISH_BUILD @@ -332,14 +333,14 @@ def get_stable_build_info(): def get_latest_artifact_info(branch, target, signed=False, stable_build=False): """Return latest artifact for a branch and target.""" - client = get_client() + client = _get_client() if not client: return None # TODO(https://github.com/google/clusterfuzz/issues/3950) # After stabilizing the Cuttlefish image, revert this if environment.is_android_cuttlefish() and stable_build: - build_info = get_stable_build_info() + build_info = _get_stable_build_info() # Use tip-of-tree build if 'bid' is missing or 0. # Setting 'bid' to 0 in stable_build_info.json # allows for easy switching between stable build @@ -355,14 +356,9 @@ def get_latest_artifact_info(branch, target, signed=False, stable_build=False): branch=branch, target=target, signed=signed) + if _use_v4(): - request = client.builds().list( # pylint: disable=no-member - buildType='submitted', - branch=branch, - target=target, - successful=True, - maxResults=1, - signed=signed) + builds = client.list_builds(branch, target, signed) else: request = client.build().list( # pylint: disable=no-member buildType='submitted', @@ -371,11 +367,9 @@ def get_latest_artifact_info(branch, target, signed=False, stable_build=False): successful=True, maxResults=1, signed=signed) - request_str = (f'{request.uri}, {request.method}, ' - f'{request.body}, {request.methodId}') + builds = _execute_request_with_retries(request) - builds = execute_request_with_retries(request) - if not builds: + if not builds or 'builds' not in builds or not builds['builds']: logs.error( 'AndroidBuildAPI get_latest_artifact_info failed: no builds found.', api_version=version_tag, @@ -383,13 +377,15 @@ def get_latest_artifact_info(branch, target, signed=False, stable_build=False): branch=branch, target=target, signed=signed, - status='failed', - request_str=request_str) + status='failed') return None build = builds['builds'][0] bid = build['buildId'] - target = build['target']['name'] + if _use_v4(): + target = build.get('target', {}).get('name', build.get('target')) + else: + target = build['target']['name'] logs.info( 'AndroidBuildAPI get_latest_artifact_info completed.', @@ -405,12 +401,12 @@ def get_latest_artifact_info(branch, target, signed=False, stable_build=False): def get(bid, target, regex, output_directory, output_filename=None): """Return artifact for a given build id, target and file regex.""" - client = get_client() + client = _get_client() if not client: return None # Run the script to fetch the artifact. - return run_script( + return _run_script( client=client, bid=bid, target=target, @@ -419,9 +415,9 @@ def get(bid, target, regex, output_directory, output_filename=None): output_filename=output_filename) -def run_script(client, bid, target, regex, output_directory, output_filename): +def _run_script(client, bid, target, regex, output_directory, output_filename): """Download artifacts as specified.""" - artifacts = get_artifacts_for_build( + artifacts = _get_artifacts_for_build( client=client, bid=bid, target=target, attempt_id='latest', regexp=regex) if not artifacts: logs.error(f'Artifact could not be fetched for target {target}, ' @@ -439,7 +435,7 @@ def run_script(client, bid, target, regex, output_directory, output_filename): continue if regex.match(artifact_name): - loop_result = download_artifact( + loop_result = _download_artifact( client=client, bid=bid, target=target, diff --git a/src/clusterfuzz/_internal/tests/core/platforms/android/fetch_artifact_test.py b/src/clusterfuzz/_internal/tests/core/platforms/android/fetch_artifact_test.py new file mode 100644 index 00000000000..a1458d33eff --- /dev/null +++ b/src/clusterfuzz/_internal/tests/core/platforms/android/fetch_artifact_test.py @@ -0,0 +1,109 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Tests for fetch_artifact.py.""" + +import unittest +from unittest import mock + +from clusterfuzz._internal.platforms.android import fetch_artifact +from clusterfuzz._internal.tests.test_libs import helpers + +class FetchArtifactTest(unittest.TestCase): + """Tests for fetch_artifact.""" + + def setUp(self): + helpers.patch(self, [ + 'clusterfuzz._internal.platforms.android.fetch_artifact._get_client', + 'clusterfuzz._internal.platforms.android.fetch_artifact._use_v4', + 'clusterfuzz._internal.platforms.android.fetch_artifact._execute_request_with_retries', + ]) + self.mock_client = mock.MagicMock() + self.mock._get_client.return_value = self.mock_client + + def test_get_latest_artifact_v4_success(self): + """Tests get_latest_artifact_info (V4). Expects extraction of {bid, branch, target} when list_builds returns data.""" + self.mock._use_v4.return_value = True + self.mock_client.list_builds.return_value = { + 'builds': [{ + 'buildId': '123', + 'target': { + 'name': 'test_target' + } + }] + } + + result = fetch_artifact.get_latest_artifact_info('branch1', 'target1') + self.assertEqual(result, {'bid': '123', 'branch': 'branch1', 'target': 'test_target'}) + self.mock_client.list_builds.assert_called_once_with('branch1', 'target1', False) + + def test_get_latest_artifact_v4_failure_no_builds(self): + """Tests get_latest_artifact_info (V4) returns None gracefully when no builds are found.""" + self.mock._use_v4.return_value = True + self.mock_client.list_builds.return_value = {} + + result = fetch_artifact.get_latest_artifact_info('branch1', 'target1') + self.assertIsNone(result) + + def test_get_latest_artifact_v3_success(self): + """Tests get_latest_artifact_info (V3). Expects extraction of {bid, branch, target} via legacy HTTP execution.""" + self.mock._use_v4.return_value = False + + mock_request = mock.MagicMock() + self.mock_client.build().list.return_value = mock_request + + self.mock._execute_request_with_retries.return_value = { + 'builds': [{ + 'buildId': '456', + 'target': { + 'name': 'test_target2' + } + }] + } + + result = fetch_artifact.get_latest_artifact_info('branch2', 'target2', signed=True) + self.assertEqual(result, {'bid': '456', 'branch': 'branch2', 'target': 'test_target2'}) + + self.mock_client.build().list.assert_called_once_with( + buildType='submitted', + branch='branch2', + target='target2', + successful=True, + maxResults=1, + signed=True) + self.mock._execute_request_with_retries.assert_called_once_with(mock_request) + + def test_get_latest_artifact_v3_failure_no_builds(self): + """Tests get_latest_artifact_info (V3) returns None gracefully when the payload is empty.""" + self.mock._use_v4.return_value = False + + mock_request = mock.MagicMock() + self.mock_client.build().list.return_value = mock_request + self.mock._execute_request_with_retries.return_value = {'builds': []} + + result = fetch_artifact.get_latest_artifact_info('branch2', 'target2', signed=True) + self.assertIsNone(result) + + def test_get_latest_artifact_client_auth_failure(self): + """Tests get_latest_artifact_info exits early and returns None when client auth fails.""" + self.mock._get_client.return_value = None + + result = fetch_artifact.get_latest_artifact_info('branch1', 'target1') + self.assertIsNone(result) + + def test_get_artifacts_for_build_empty_regexp(self): + """Tests _get_artifacts_for_build returns [] returning early when regexp is empty, bypassing API calls.""" + result = fetch_artifact._get_artifacts_for_build(self.mock_client, 'bid', 'target', regexp='') + self.assertEqual(result, []) + self.mock_client.list_artifacts.assert_not_called() + self.mock_client.buildartifact().list.assert_not_called() From c088acd70af5324287f94f705681359a286bb8d2 Mon Sep 17 00:00:00 2001 From: Ivan Barba Date: Tue, 21 Jul 2026 07:48:24 +0000 Subject: [PATCH 2/4] Uses correct service account credentials at construction --- .../platforms/android/fetch_artifact.py | 15 ++++++-- .../platforms/android/fetch_artifact_test.py | 38 +++++++++++++------ 2 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py b/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py index 58f52a4d885..5ada8465b5d 100644 --- a/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py +++ b/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py @@ -23,6 +23,7 @@ from typing import Optional import apiclient +from google.oauth2 import service_account from oauth2client.service_account import ServiceAccountCredentials from clusterfuzz._internal.config import db_config @@ -293,17 +294,23 @@ def _get_client(): 'Android build apiary credentials are not set, skip artifact fetch.') return None - credentials = ServiceAccountCredentials.from_json_keyfile_dict( - json.loads(build_apiary_service_account_private_key), - scopes=ANDROID_BUILD_API_SCOPES) + key_dict = json.loads(build_apiary_service_account_private_key) logs.info( 'AndroidBuildAPI client initialization started.', api_version='V4' if _use_v4() else 'V3') if _use_v4(): - client = AndroidBuildV4Api.create_authenticated(credentials) + try: + credentials = service_account.Credentials.from_service_account_info( + key_dict, scopes=ANDROID_BUILD_API_SCOPES) + client = AndroidBuildV4Api.create_authenticated(credentials) + except Exception as e: + logs.error(f'Failed to initialize AndroidBuildV4Api: {e}') + return None else: + credentials = ServiceAccountCredentials.from_json_keyfile_dict( + key_dict, scopes=ANDROID_BUILD_API_SCOPES) client = apiclient.discovery.build( 'androidbuildinternal', 'v3', diff --git a/src/clusterfuzz/_internal/tests/core/platforms/android/fetch_artifact_test.py b/src/clusterfuzz/_internal/tests/core/platforms/android/fetch_artifact_test.py index a1458d33eff..e01de1ba822 100644 --- a/src/clusterfuzz/_internal/tests/core/platforms/android/fetch_artifact_test.py +++ b/src/clusterfuzz/_internal/tests/core/platforms/android/fetch_artifact_test.py @@ -13,12 +13,15 @@ # limitations under the License. """Tests for fetch_artifact.py.""" +# pylint: disable=protected-access + import unittest from unittest import mock from clusterfuzz._internal.platforms.android import fetch_artifact from clusterfuzz._internal.tests.test_libs import helpers + class FetchArtifactTest(unittest.TestCase): """Tests for fetch_artifact.""" @@ -44,8 +47,13 @@ def test_get_latest_artifact_v4_success(self): } result = fetch_artifact.get_latest_artifact_info('branch1', 'target1') - self.assertEqual(result, {'bid': '123', 'branch': 'branch1', 'target': 'test_target'}) - self.mock_client.list_builds.assert_called_once_with('branch1', 'target1', False) + self.assertEqual(result, { + 'bid': '123', + 'branch': 'branch1', + 'target': 'test_target' + }) + self.mock_client.list_builds.assert_called_once_with( + 'branch1', 'target1', False) def test_get_latest_artifact_v4_failure_no_builds(self): """Tests get_latest_artifact_info (V4) returns None gracefully when no builds are found.""" @@ -58,10 +66,10 @@ def test_get_latest_artifact_v4_failure_no_builds(self): def test_get_latest_artifact_v3_success(self): """Tests get_latest_artifact_info (V3). Expects extraction of {bid, branch, target} via legacy HTTP execution.""" self.mock._use_v4.return_value = False - + mock_request = mock.MagicMock() self.mock_client.build().list.return_value = mock_request - + self.mock._execute_request_with_retries.return_value = { 'builds': [{ 'buildId': '456', @@ -71,9 +79,14 @@ def test_get_latest_artifact_v3_success(self): }] } - result = fetch_artifact.get_latest_artifact_info('branch2', 'target2', signed=True) - self.assertEqual(result, {'bid': '456', 'branch': 'branch2', 'target': 'test_target2'}) - + result = fetch_artifact.get_latest_artifact_info( + 'branch2', 'target2', signed=True) + self.assertEqual(result, { + 'bid': '456', + 'branch': 'branch2', + 'target': 'test_target2' + }) + self.mock_client.build().list.assert_called_once_with( buildType='submitted', branch='branch2', @@ -81,17 +94,19 @@ def test_get_latest_artifact_v3_success(self): successful=True, maxResults=1, signed=True) - self.mock._execute_request_with_retries.assert_called_once_with(mock_request) + self.mock._execute_request_with_retries.assert_called_once_with( + mock_request) def test_get_latest_artifact_v3_failure_no_builds(self): """Tests get_latest_artifact_info (V3) returns None gracefully when the payload is empty.""" self.mock._use_v4.return_value = False - + mock_request = mock.MagicMock() self.mock_client.build().list.return_value = mock_request self.mock._execute_request_with_retries.return_value = {'builds': []} - result = fetch_artifact.get_latest_artifact_info('branch2', 'target2', signed=True) + result = fetch_artifact.get_latest_artifact_info( + 'branch2', 'target2', signed=True) self.assertIsNone(result) def test_get_latest_artifact_client_auth_failure(self): @@ -103,7 +118,8 @@ def test_get_latest_artifact_client_auth_failure(self): def test_get_artifacts_for_build_empty_regexp(self): """Tests _get_artifacts_for_build returns [] returning early when regexp is empty, bypassing API calls.""" - result = fetch_artifact._get_artifacts_for_build(self.mock_client, 'bid', 'target', regexp='') + result = fetch_artifact._get_artifacts_for_build( + self.mock_client, 'bid', 'target', regexp='') self.assertEqual(result, []) self.mock_client.list_artifacts.assert_not_called() self.mock_client.buildartifact().list.assert_not_called() From f64347fcb069c3b0d1d682e078918a58195ebc0c Mon Sep 17 00:00:00 2001 From: Ivan Barba Date: Tue, 21 Jul 2026 19:01:53 +0000 Subject: [PATCH 3/4] Simplify validations & Adds a TODO --- .../platforms/android/android_build_v4_api.py | 5 ++++- .../_internal/platforms/android/fetch_artifact.py | 15 ++++++++------- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/src/clusterfuzz/_internal/platforms/android/android_build_v4_api.py b/src/clusterfuzz/_internal/platforms/android/android_build_v4_api.py index 4b46a0ac7e7..bd46eddc41f 100644 --- a/src/clusterfuzz/_internal/platforms/android/android_build_v4_api.py +++ b/src/clusterfuzz/_internal/platforms/android/android_build_v4_api.py @@ -165,7 +165,10 @@ def list_builds(self, branch: str, target: str, url = f'{self.BASE_URL}/v4/builds' try: response = self._request(url, params=params) - return response.json() + data = response.json() + if data.get('builds') and len(data['builds']) > 0: + return data + return None except (requests.exceptions.RequestException, google.auth.exceptions.GoogleAuthError, ValueError) as e: logs.error( diff --git a/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py b/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py index 5ada8465b5d..a3fa414ecc8 100644 --- a/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py +++ b/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py @@ -166,6 +166,7 @@ def _download_artifact(client, bid, target, attempt_id, name, output_directory, logs.info(f'Creating directory {output_dir}') os.makedirs(output_dir, exist_ok=True) + # TODO(b/537368595) Remove unnecesary logging. # Just like get, except get_media. logs.info( 'AndroidBuildAPI download_artifact media download started.', @@ -229,7 +230,9 @@ def _get_artifacts_for_build(client, """Return list of artifacts for a given build.""" if not regexp: logs.warning( - 'Regexp is empty, returning early to avoid querying all artifacts.') + 'Regexp is empty, returning early to avoid querying all artifacts.', + bid=bid, + target=target) return [] version_tag = 'V4' if _use_v4() else 'V3' @@ -374,9 +377,10 @@ def get_latest_artifact_info(branch, target, signed=False, stable_build=False): successful=True, maxResults=1, signed=signed) - builds = _execute_request_with_retries(request) + res = _execute_request_with_retries(request) + builds = res if (res and res.get('builds')) else None - if not builds or 'builds' not in builds or not builds['builds']: + if not builds: logs.error( 'AndroidBuildAPI get_latest_artifact_info failed: no builds found.', api_version=version_tag, @@ -389,10 +393,7 @@ def get_latest_artifact_info(branch, target, signed=False, stable_build=False): build = builds['builds'][0] bid = build['buildId'] - if _use_v4(): - target = build.get('target', {}).get('name', build.get('target')) - else: - target = build['target']['name'] + target = build['target']['name'] logs.info( 'AndroidBuildAPI get_latest_artifact_info completed.', From fffb1bbcc3a099ad4f8ec6de1e575101e23bebc9 Mon Sep 17 00:00:00 2001 From: Ivan Barba Date: Thu, 23 Jul 2026 09:53:29 -0600 Subject: [PATCH 4/4] [Android V4 api Migration] Creates Feature Flag (#5373) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Overview As per requested, we created a new feature flag to disable definitely the API build use cases(used trough `fetch_artifact.py`), this feature flag is enabled by default, because that's the current API & ClusterFuzz workflow. But can be disabled by adding the feature flag to the DB and marking it as enabled=false. ## Changes - Creates new feature_flag, and enables it by default in `fetch_artifact.py` - Logs warnings when the feature flag is disabled in `fetch_artifact.py` - Safely exists the related methods in `symbols_downloader` and `flash.py` which consume the `fetch_artifact.py` - Moved up the `adb.bad_state_reach()` method from the `fetch_artifact.py` module to its callers, this way we can now handle better the None values and raised errors from the API being turned off by the feature flag. ## Tests After the initial tests in the PR parents, i tested this changes once more in dev(including all 3 prs), looking specifically for `utils.fetch_url` or errors inherent to the return value from this method. Basically since my changes landed in dev the following error groups where seen: image Discarded the errors following this logic: - 403 Errors are expected to appear as per the changes that were previously there to avoid them are no longer in dev - Some error groups only appeared on previous clusterfuzz versions - Other errors are expected(like the queue size limit or bad revision errors) - There are errors from corpus pruning & fuzzer not found, this have been seen for a while now, not related to my changes - Lastly there are some errors in a cron_job, specifically in [this file ](https://github.com/google/clusterfuzz/blob/master/src/clusterfuzz/_internal/cron/load_bigquery_stats.py)but this one doesn't make usage of `fetch_url` and nor does the method that triggered the error #### PR review 1. [**#5370** Add Android Build API V4 REST client and tests ](https://github.com/google/clusterfuzz/pull/5370) 2. [#5371 Migrate fetch_artifact to use new V4 API](https://github.com/google/clusterfuzz/pull/5371) 3. 👉 [#5373 Creates Feature Flag for Android V4 API](https://github.com/google/clusterfuzz/pull/5373) *(This PR)* Please only approve, I'll merge this PR's to avoid any possible merge conflict that can appear in the process :D --- .../_internal/base/feature_flags.py | 1 + .../platforms/android/fetch_artifact.py | 28 +++++++++-- .../_internal/platforms/android/flash.py | 7 ++- .../platforms/android/symbols_downloader.py | 11 ++++- .../platforms/android/fetch_artifact_test.py | 49 +++++++++++++++++++ 5 files changed, 90 insertions(+), 6 deletions(-) diff --git a/src/clusterfuzz/_internal/base/feature_flags.py b/src/clusterfuzz/_internal/base/feature_flags.py index 5dfeccae1c1..9660745990c 100644 --- a/src/clusterfuzz/_internal/base/feature_flags.py +++ b/src/clusterfuzz/_internal/base/feature_flags.py @@ -44,6 +44,7 @@ class FeatureFlags(Enum): ENABLE_FUZZ_FOR_BOTS = 'enable_fuzz_for_bots' STORAGE_THREADED_OPS_FUZZ_TARGETS = 'storage_threaded_ops_fuzz_targets' + CALL_ANDROID_API = 'call_android_api' @property def flag(self): diff --git a/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py b/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py index a3fa414ecc8..961e012bc50 100644 --- a/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py +++ b/src/clusterfuzz/_internal/platforms/android/fetch_artifact.py @@ -26,6 +26,7 @@ from google.oauth2 import service_account from oauth2client.service_account import ServiceAccountCredentials +from clusterfuzz._internal.base import feature_flags from clusterfuzz._internal.config import db_config from clusterfuzz._internal.google_cloud_utils import storage from clusterfuzz._internal.metrics import logs @@ -33,8 +34,6 @@ AndroidBuildV4Api from clusterfuzz._internal.system import environment -from . import adb - # 20 MB default chunk size. DEFAULT_CHUNK_SIZE = 20 * 1024 * 1024 @@ -72,6 +71,20 @@ def _use_v4(): return False +def _call_android_api_enabled(): + """Return True if we should call the Android Build API, enabled by default, + Disabled always if invoked in a uworker + """ + if environment.is_uworker(): + logs.info('AndroidBuildAPI access disabled for uworker.') + return False + + flag = feature_flags.FeatureFlags.CALL_ANDROID_API.flag + if flag is None: + return True + return flag.enabled + + def _execute_request_with_retries(request): """Executes request and retries on failure.""" result = None @@ -282,7 +295,6 @@ def _get_artifacts_for_build(client, if not artifacts: logs.error(f'No artifact found for target {target}, build id {bid}.\n' f'results {results}') - adb.bad_state_reached() return artifacts @@ -343,6 +355,11 @@ def _get_stable_build_info(): def get_latest_artifact_info(branch, target, signed=False, stable_build=False): """Return latest artifact for a branch and target.""" + if not _call_android_api_enabled(): + logs.warning( + 'Android build API is disabled by feature flag call_android_api.') + return None + client = _get_client() if not client: return None @@ -409,6 +426,11 @@ def get_latest_artifact_info(branch, target, signed=False, stable_build=False): def get(bid, target, regex, output_directory, output_filename=None): """Return artifact for a given build id, target and file regex.""" + if not _call_android_api_enabled(): + logs.warning( + 'Android build API is disabled by feature flag call_android_api.') + return None + client = _get_client() if not client: return None diff --git a/src/clusterfuzz/_internal/platforms/android/flash.py b/src/clusterfuzz/_internal/platforms/android/flash.py index b292b1491e4..283a80120d3 100644 --- a/src/clusterfuzz/_internal/platforms/android/flash.py +++ b/src/clusterfuzz/_internal/platforms/android/flash.py @@ -85,7 +85,7 @@ def download_latest_build(build_info, image_regexes, image_directory): logs.error('Failed to download artifact %s for ' 'branch %s and target %s.' % (image_file_paths, build_info['branch'], target)) - return + adb.bad_state_reached() for file_path in image_file_paths: if file_path.endswith('.zip') or file_path.endswith('.tar.gz'): @@ -97,7 +97,10 @@ def boot_stable_build_cuttlefish(branch, target, image_directory): """Boot cuttlefish instance using stable build id fetched from gcs.""" build_info = fetch_artifact.get_latest_artifact_info( branch, target, stable_build=True) - download_latest_build(build_info, FLASH_CUTTLEFISH_REGEXES, image_directory) + if not build_info: + logs.error('Unable to fetch stable build info for cuttlefish.') + else: + download_latest_build(build_info, FLASH_CUTTLEFISH_REGEXES, image_directory) adb.recreate_cuttlefish_device() adb.connect_to_cuttlefish_device() diff --git a/src/clusterfuzz/_internal/platforms/android/symbols_downloader.py b/src/clusterfuzz/_internal/platforms/android/symbols_downloader.py index 112d86fc215..55a906b5498 100644 --- a/src/clusterfuzz/_internal/platforms/android/symbols_downloader.py +++ b/src/clusterfuzz/_internal/platforms/android/symbols_downloader.py @@ -196,7 +196,12 @@ def download_trusty_symbols_if_needed(symbols_directory, app_name, bid): branch = 'polygon-trusty-whitechapel-master' if not bid: - bid = fetch_artifact.get_latest_artifact_info(branch, ab_target)['bid'] + build_info = fetch_artifact.get_latest_artifact_info(branch, ab_target) + if not build_info: + logs.error(f'Unable to fetch build info for branch {branch} ' + f'and target {ab_target}.') + return + bid = build_info['bid'] artifact_filename = f'{ab_target}-{bid}.syms.zip' symbols_archive_path = os.path.join(symbols_directory, artifact_filename) @@ -204,6 +209,10 @@ def download_trusty_symbols_if_needed(symbols_directory, app_name, bid): download_artifact_if_needed(bid, symbols_directory, symbols_archive_path, [ab_target], artifact_filename, None) + if not os.path.exists(symbols_archive_path): + logs.error(f'Unable to locate symbols archive {symbols_archive_path}.') + return + with zipfile.ZipFile(symbols_archive_path, 'r') as symbols_zipfile: for filepath in symbols_zipfile.namelist(): if f'{app_name}.syms.elf' in filepath: diff --git a/src/clusterfuzz/_internal/tests/core/platforms/android/fetch_artifact_test.py b/src/clusterfuzz/_internal/tests/core/platforms/android/fetch_artifact_test.py index e01de1ba822..5417a64e229 100644 --- a/src/clusterfuzz/_internal/tests/core/platforms/android/fetch_artifact_test.py +++ b/src/clusterfuzz/_internal/tests/core/platforms/android/fetch_artifact_test.py @@ -18,6 +18,7 @@ import unittest from unittest import mock +from clusterfuzz._internal.base import feature_flags from clusterfuzz._internal.platforms.android import fetch_artifact from clusterfuzz._internal.tests.test_libs import helpers @@ -29,10 +30,12 @@ def setUp(self): helpers.patch(self, [ 'clusterfuzz._internal.platforms.android.fetch_artifact._get_client', 'clusterfuzz._internal.platforms.android.fetch_artifact._use_v4', + 'clusterfuzz._internal.platforms.android.fetch_artifact._call_android_api_enabled', 'clusterfuzz._internal.platforms.android.fetch_artifact._execute_request_with_retries', ]) self.mock_client = mock.MagicMock() self.mock._get_client.return_value = self.mock_client + self.mock._call_android_api_enabled.return_value = True def test_get_latest_artifact_v4_success(self): """Tests get_latest_artifact_info (V4). Expects extraction of {bid, branch, target} when list_builds returns data.""" @@ -116,6 +119,13 @@ def test_get_latest_artifact_client_auth_failure(self): result = fetch_artifact.get_latest_artifact_info('branch1', 'target1') self.assertIsNone(result) + def test_get_latest_artifact_disabled_by_feature_flag(self): + """Tests get_latest_artifact_info exits early and returns None when disabled by feature flag.""" + self.mock._call_android_api_enabled.return_value = False + + result = fetch_artifact.get_latest_artifact_info('branch1', 'target1') + self.assertIsNone(result) + def test_get_artifacts_for_build_empty_regexp(self): """Tests _get_artifacts_for_build returns [] returning early when regexp is empty, bypassing API calls.""" result = fetch_artifact._get_artifacts_for_build( @@ -123,3 +133,42 @@ def test_get_artifacts_for_build_empty_regexp(self): self.assertEqual(result, []) self.mock_client.list_artifacts.assert_not_called() self.mock_client.buildartifact().list.assert_not_called() + + +class CallAndroidApiEnabledTest(unittest.TestCase): + """Tests for _call_android_api_enabled.""" + + def setUp(self): + helpers.patch(self, [ + 'clusterfuzz._internal.system.environment.is_uworker', + ]) + self.mock.is_uworker.return_value = False + + def test_is_uworker_returns_false(self): + self.mock.is_uworker.return_value = True + self.assertFalse(fetch_artifact._call_android_api_enabled()) + + def test_flag_none_returns_true(self): + with mock.patch.object( + feature_flags.FeatureFlags, 'flag', + new_callable=mock.PropertyMock) as mock_flag: + mock_flag.return_value = None + self.assertTrue(fetch_artifact._call_android_api_enabled()) + + def test_flag_enabled_returns_true(self): + mock_flag_obj = mock.MagicMock() + mock_flag_obj.enabled = True + with mock.patch.object( + feature_flags.FeatureFlags, 'flag', + new_callable=mock.PropertyMock) as mock_flag: + mock_flag.return_value = mock_flag_obj + self.assertTrue(fetch_artifact._call_android_api_enabled()) + + def test_flag_disabled_returns_false(self): + mock_flag_obj = mock.MagicMock() + mock_flag_obj.enabled = False + with mock.patch.object( + feature_flags.FeatureFlags, 'flag', + new_callable=mock.PropertyMock) as mock_flag: + mock_flag.return_value = mock_flag_obj + self.assertFalse(fetch_artifact._call_android_api_enabled())