From 4a09b85cbd3740d340f6f7b89b2d3956e9922956 Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Wed, 6 Nov 2024 22:25:07 +0000 Subject: [PATCH 1/9] ofg: prepare use of cached images Signed-off-by: David Korczynski --- infra/build/functions/build_lib.py | 13 ++++++++++--- infra/build/functions/build_project.py | 23 ++++++++++++++--------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/infra/build/functions/build_lib.py b/infra/build/functions/build_lib.py index a5766cf022d9..025de95bf7fd 100644 --- a/infra/build/functions/build_lib.py +++ b/infra/build/functions/build_lib.py @@ -387,7 +387,8 @@ def get_docker_build_step(image_names, directory, use_buildkit_cache=False, src_root='oss-fuzz', - architecture='x86_64'): + architecture='x86_64', + cache_name=''): """Returns the docker build step.""" assert len(image_names) >= 1 directory = os.path.join(src_root, directory) @@ -404,6 +405,9 @@ def get_docker_build_step(image_names, _make_image_name_architecture_specific(image_name, architecture) for image_name in image_names ] + if cache_name: + args.extend(['-build-arg', f'CACHE_IMAGE={cache_name}']) + for image_name in image_names: args.extend(['--tag', image_name]) @@ -437,7 +441,8 @@ def get_project_image_steps( # pylint: disable=too-many-arguments language, config, architectures=None, - experiment=False): + experiment=False, + cache_name=''): """Returns GCB steps to build OSS-Fuzz project image.""" if architectures is None: architectures = [] @@ -453,9 +458,11 @@ def get_project_image_steps( # pylint: disable=too-many-arguments if config.test_image_suffix: steps.extend(get_pull_test_images_steps(config.test_image_suffix)) src_root = 'oss-fuzz' if not experiment else '.' + docker_build_step = get_docker_build_step([image], os.path.join('projects', name), - src_root=src_root) + src_root=src_root, + cache_name=cache_name) steps.append(docker_build_step) srcmap_step_id = get_srcmap_step_id() steps.extend([{ diff --git a/infra/build/functions/build_project.py b/infra/build/functions/build_project.py index 44c00a87a906..4894229bc80c 100755 --- a/infra/build/functions/build_project.py +++ b/infra/build/functions/build_project.py @@ -354,17 +354,22 @@ def get_build_steps_for_project(project, return [] timestamp = get_datetime_now().strftime('%Y%m%d%H%M') + + # if we use caching, then we need to use the right name. We assume that + # there is only a single sanitizer. if use_caching: - # Use cached built image. - build_steps = [] + cache_name = ('us-central1-docker.pkg.dev/oss-fuzz/oss-fuzz-gen/' + f'{project.name}-ofg-cached-{project.sanitizers[0]}') else: - build_steps = build_lib.get_project_image_steps( - project.name, - project.image, - project.fuzzing_language, - config=config, - architectures=project.architectures, - experiment=config.experiment) + cache_name = '' + build_steps = build_lib.get_project_image_steps( + project.name, + project.image, + project.fuzzing_language, + config=config, + architectures=project.architectures, + experiment=config.experiment, + cache_name=cache_name) # Sort engines to make AFL first to test if libFuzzer has an advantage in # finding bugs first since it is generally built first. From 856b661c95f85fe94f2021cb5637a910a44d8ac7 Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Wed, 6 Nov 2024 22:26:35 +0000 Subject: [PATCH 2/9] nit Signed-off-by: David Korczynski --- infra/build/functions/build_lib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infra/build/functions/build_lib.py b/infra/build/functions/build_lib.py index 025de95bf7fd..576d1e9a8549 100644 --- a/infra/build/functions/build_lib.py +++ b/infra/build/functions/build_lib.py @@ -406,7 +406,7 @@ def get_docker_build_step(image_names, for image_name in image_names ] if cache_name: - args.extend(['-build-arg', f'CACHE_IMAGE={cache_name}']) + args.extend(['--build-arg', f'CACHE_IMAGE={cache_name}']) for image_name in image_names: args.extend(['--tag', image_name]) From c30bd8ce00440dc4751f725e90a7a2817ee9716b Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Wed, 6 Nov 2024 22:33:05 +0000 Subject: [PATCH 3/9] nit Signed-off-by: David Korczynski --- infra/build/functions/build_project.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infra/build/functions/build_project.py b/infra/build/functions/build_project.py index 4894229bc80c..616abf044ca1 100755 --- a/infra/build/functions/build_project.py +++ b/infra/build/functions/build_project.py @@ -359,7 +359,7 @@ def get_build_steps_for_project(project, # there is only a single sanitizer. if use_caching: cache_name = ('us-central1-docker.pkg.dev/oss-fuzz/oss-fuzz-gen/' - f'{project.name}-ofg-cached-{project.sanitizers[0]}') + f'{project.name}-ofg-cached-{project.sanitizers[0]}') else: cache_name = '' build_steps = build_lib.get_project_image_steps( From 8ce98fec1224e40b12832f45131f3a56d60b8a1e Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Wed, 6 Nov 2024 22:58:13 +0000 Subject: [PATCH 4/9] cache_name -> cache_image Signed-off-by: David Korczynski --- infra/build/functions/build_lib.py | 8 ++++---- infra/build/functions/build_project.py | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/infra/build/functions/build_lib.py b/infra/build/functions/build_lib.py index 576d1e9a8549..9a0e4e8b8c91 100644 --- a/infra/build/functions/build_lib.py +++ b/infra/build/functions/build_lib.py @@ -388,7 +388,7 @@ def get_docker_build_step(image_names, use_buildkit_cache=False, src_root='oss-fuzz', architecture='x86_64', - cache_name=''): + cache_image=''): """Returns the docker build step.""" assert len(image_names) >= 1 directory = os.path.join(src_root, directory) @@ -405,7 +405,7 @@ def get_docker_build_step(image_names, _make_image_name_architecture_specific(image_name, architecture) for image_name in image_names ] - if cache_name: + if cache_image: args.extend(['--build-arg', f'CACHE_IMAGE={cache_name}']) for image_name in image_names: @@ -442,7 +442,7 @@ def get_project_image_steps( # pylint: disable=too-many-arguments config, architectures=None, experiment=False, - cache_name=''): + cache_image=''): """Returns GCB steps to build OSS-Fuzz project image.""" if architectures is None: architectures = [] @@ -462,7 +462,7 @@ def get_project_image_steps( # pylint: disable=too-many-arguments docker_build_step = get_docker_build_step([image], os.path.join('projects', name), src_root=src_root, - cache_name=cache_name) + cache_image=cache_image) steps.append(docker_build_step) srcmap_step_id = get_srcmap_step_id() steps.extend([{ diff --git a/infra/build/functions/build_project.py b/infra/build/functions/build_project.py index 616abf044ca1..337a1c46e9b7 100755 --- a/infra/build/functions/build_project.py +++ b/infra/build/functions/build_project.py @@ -358,10 +358,10 @@ def get_build_steps_for_project(project, # if we use caching, then we need to use the right name. We assume that # there is only a single sanitizer. if use_caching: - cache_name = ('us-central1-docker.pkg.dev/oss-fuzz/oss-fuzz-gen/' - f'{project.name}-ofg-cached-{project.sanitizers[0]}') + cache_image = ('us-central1-docker.pkg.dev/oss-fuzz/oss-fuzz-gen/' + f'{project.name}-ofg-cached-{project.sanitizers[0]}') else: - cache_name = '' + cache_image = '' build_steps = build_lib.get_project_image_steps( project.name, project.image, @@ -369,7 +369,7 @@ def get_build_steps_for_project(project, config=config, architectures=project.architectures, experiment=config.experiment, - cache_name=cache_name) + cache_image=cache_image) # Sort engines to make AFL first to test if libFuzzer has an advantage in # finding bugs first since it is generally built first. From 3352e3ff90ba4d4a19e07abf53ad4462cd483984 Mon Sep 17 00:00:00 2001 From: David Korczynski Date: Wed, 6 Nov 2024 23:04:40 +0000 Subject: [PATCH 5/9] reuse cached_image Signed-off-by: David Korczynski --- infra/build/functions/build_project.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/infra/build/functions/build_project.py b/infra/build/functions/build_project.py index 337a1c46e9b7..f8a2e9f6f9b1 100755 --- a/infra/build/functions/build_project.py +++ b/infra/build/functions/build_project.py @@ -358,8 +358,8 @@ def get_build_steps_for_project(project, # if we use caching, then we need to use the right name. We assume that # there is only a single sanitizer. if use_caching: - cache_image = ('us-central1-docker.pkg.dev/oss-fuzz/oss-fuzz-gen/' - f'{project.name}-ofg-cached-{project.sanitizers[0]}') + project.cached_sanitizer=project.sanitizers[0] + cache_image = project.cached_image() else: cache_image = '' build_steps = build_lib.get_project_image_steps( From 08cf6a89f0d4e698e4d11511c535c495bd6fb222 Mon Sep 17 00:00:00 2001 From: Oliver Chang Date: Thu, 7 Nov 2024 10:17:17 +1100 Subject: [PATCH 6/9] fixes --- infra/build/functions/build_lib.py | 2 +- infra/build/functions/build_project.py | 15 +++++++++------ 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/infra/build/functions/build_lib.py b/infra/build/functions/build_lib.py index 9a0e4e8b8c91..1ad5c7d39b5a 100644 --- a/infra/build/functions/build_lib.py +++ b/infra/build/functions/build_lib.py @@ -442,7 +442,7 @@ def get_project_image_steps( # pylint: disable=too-many-arguments config, architectures=None, experiment=False, - cache_image=''): + cache_image=None): """Returns GCB steps to build OSS-Fuzz project image.""" if architectures is None: architectures = [] diff --git a/infra/build/functions/build_project.py b/infra/build/functions/build_project.py index f8a2e9f6f9b1..d6fee4cd5cb1 100755 --- a/infra/build/functions/build_project.py +++ b/infra/build/functions/build_project.py @@ -190,8 +190,10 @@ def image(self): return f'gcr.io/{build_lib.IMAGE_PROJECT}/{self.name}' - def cached_image(self, sanitizer): - return _CACHED_IMAGE.format(name=self.real_name, sanitizer=sanitizer) + @property + def cached_image(self): + return _CACHED_IMAGE.format( + name=self.real_name, sanitizer=self.cached_sanitizer) def get_last_step_id(steps): @@ -355,13 +357,14 @@ def get_build_steps_for_project(project, timestamp = get_datetime_now().strftime('%Y%m%d%H%M') - # if we use caching, then we need to use the right name. We assume that + # If we use caching, then we need to use the right name. We assume that # there is only a single sanitizer. if use_caching: - project.cached_sanitizer=project.sanitizers[0] - cache_image = project.cached_image() + project.cached_sanitizer = project.sanitizers[0] + cache_image = project.cached_image else: - cache_image = '' + cache_image = None + build_steps = build_lib.get_project_image_steps( project.name, project.image, From 74cd34d7c9aed0540ae0b5ab4c57d5ae1eb94140 Mon Sep 17 00:00:00 2001 From: Oliver Chang Date: Thu, 7 Nov 2024 10:52:52 +1100 Subject: [PATCH 7/9] various fixes --- infra/build/functions/build_lib.py | 2 +- infra/build/functions/build_project.py | 3 --- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/infra/build/functions/build_lib.py b/infra/build/functions/build_lib.py index 1ad5c7d39b5a..de0e3efac0bd 100644 --- a/infra/build/functions/build_lib.py +++ b/infra/build/functions/build_lib.py @@ -406,7 +406,7 @@ def get_docker_build_step(image_names, for image_name in image_names ] if cache_image: - args.extend(['--build-arg', f'CACHE_IMAGE={cache_name}']) + args.extend(['--build-arg', f'CACHE_IMAGE={cache_image}']) for image_name in image_names: args.extend(['--tag', image_name]) diff --git a/infra/build/functions/build_project.py b/infra/build/functions/build_project.py index d6fee4cd5cb1..441612775b62 100755 --- a/infra/build/functions/build_project.py +++ b/infra/build/functions/build_project.py @@ -185,9 +185,6 @@ def sanitizers(self): @property def image(self): """Returns the docker image for the project.""" - if self.cached_sanitizer: - return self.cached_image(self.cached_sanitizer) - return f'gcr.io/{build_lib.IMAGE_PROJECT}/{self.name}' @property From a717d7a5b3b3f8da83501deae73cafbe25245f77 Mon Sep 17 00:00:00 2001 From: Oliver Chang Date: Thu, 7 Nov 2024 11:26:25 +1100 Subject: [PATCH 8/9] print dockerfile --- infra/build/functions/build_lib.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/infra/build/functions/build_lib.py b/infra/build/functions/build_lib.py index de0e3efac0bd..21a827aac250 100644 --- a/infra/build/functions/build_lib.py +++ b/infra/build/functions/build_lib.py @@ -459,6 +459,14 @@ def get_project_image_steps( # pylint: disable=too-many-arguments steps.extend(get_pull_test_images_steps(config.test_image_suffix)) src_root = 'oss-fuzz' if not experiment else '.' + steps.append({ + 'name': 'ubuntu', + 'args': [ + 'bash', '-c', + f'cat {src_root}/projects/{name}/Dockerfile' + ], + }) + docker_build_step = get_docker_build_step([image], os.path.join('projects', name), src_root=src_root, From 1ea1d1a34f100a61dab442c9e113e1bcbd789225 Mon Sep 17 00:00:00 2001 From: Oliver Chang Date: Thu, 7 Nov 2024 12:40:34 +1100 Subject: [PATCH 9/9] format --- infra/build/functions/build_lib.py | 5 +---- infra/build/functions/build_project.py | 4 ++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/infra/build/functions/build_lib.py b/infra/build/functions/build_lib.py index 21a827aac250..58183ef664d5 100644 --- a/infra/build/functions/build_lib.py +++ b/infra/build/functions/build_lib.py @@ -461,10 +461,7 @@ def get_project_image_steps( # pylint: disable=too-many-arguments steps.append({ 'name': 'ubuntu', - 'args': [ - 'bash', '-c', - f'cat {src_root}/projects/{name}/Dockerfile' - ], + 'args': ['bash', '-c', f'cat {src_root}/projects/{name}/Dockerfile'], }) docker_build_step = get_docker_build_step([image], diff --git a/infra/build/functions/build_project.py b/infra/build/functions/build_project.py index 441612775b62..3878bd086b6c 100755 --- a/infra/build/functions/build_project.py +++ b/infra/build/functions/build_project.py @@ -189,8 +189,8 @@ def image(self): @property def cached_image(self): - return _CACHED_IMAGE.format( - name=self.real_name, sanitizer=self.cached_sanitizer) + return _CACHED_IMAGE.format(name=self.real_name, + sanitizer=self.cached_sanitizer) def get_last_step_id(steps):