From f0b6e0cf6e0359e72d4a7dff2b6e6c8176e92bc0 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Tue, 12 Sep 2023 00:09:58 +0000 Subject: [PATCH 01/24] Support for retention policies on containers --- .../ApiService/Functions/QueueFileChanges.cs | 31 ++++++++++++++++--- .../onefuzzlib/NotificationOperations.cs | 5 ++- ...RententionPolicy.cs => RetentionPolicy.cs} | 21 ++++++++++++- 3 files changed, 49 insertions(+), 8 deletions(-) rename src/ApiService/ApiService/onefuzzlib/{RententionPolicy.cs => RetentionPolicy.cs} (54%) diff --git a/src/ApiService/ApiService/Functions/QueueFileChanges.cs b/src/ApiService/ApiService/Functions/QueueFileChanges.cs index acdd3e328d..5b5ef5fed8 100644 --- a/src/ApiService/ApiService/Functions/QueueFileChanges.cs +++ b/src/ApiService/ApiService/Functions/QueueFileChanges.cs @@ -54,6 +54,8 @@ public async Async.Task Run( return; } + var storageAccount = new ResourceIdentifier(topicElement.GetString()!); + try { // Setting isLastRetryAttempt to false will rethrow any exceptions // With the intention that the azure functions runtime will handle requeing @@ -61,7 +63,7 @@ public async Async.Task Run( // requeuing ourselves because azure functions doesn't support retry policies // for queue based functions. - var result = await FileAdded(fileChangeEvent, isLastRetryAttempt: false); + var result = await FileAdded(storageAccount, fileChangeEvent, isLastRetryAttempt: false); if (!result.IsOk && result.ErrorV.Code == ErrorCode.ADO_WORKITEM_PROCESSING_DISABLED) { await RequeueMessage(msg, TimeSpan.FromDays(1)); } @@ -71,16 +73,37 @@ public async Async.Task Run( } } - private async Async.Task FileAdded(JsonDocument fileChangeEvent, bool isLastRetryAttempt) { + private async Async.Task FileAdded(ResourceIdentifier storageAccount, JsonDocument fileChangeEvent, bool isLastRetryAttempt) { var data = fileChangeEvent.RootElement.GetProperty("data"); var url = data.GetProperty("url").GetString()!; var parts = url.Split("/").Skip(3).ToList(); - var container = parts[0]; + var container = Container.Parse(parts[0]); var path = string.Join('/', parts.Skip(1)); _log.LogInformation("file added : {Container} - {Path}", container, path); - return await _notificationOperations.NewFiles(Container.Parse(container), path, isLastRetryAttempt); + + await ApplyRetentionPolicy(storageAccount, container, path); + + return await _notificationOperations.NewFiles(container, path, isLastRetryAttempt); + } + + private async Async.Task ApplyRetentionPolicy(ResourceIdentifier storageAccount, Container container, string path) { + // default retention period can be applied to the container + // if one exists, we will set the expiry date on the newly-created blob, if it doesn't already have one + var account = await _storage.GetBlobServiceClientForAccount(storageAccount); + var containerClient = account.GetBlobContainerClient(container.String); + var containerProps = await containerClient.GetPropertiesAsync(); + var retentionPeriod = RetentionPolicyUtils.GetRetentionPeriodFromMetadata(containerProps.Value.Metadata); + if (retentionPeriod.HasValue) { + var blobClient = containerClient.GetBlobClient(path); + var tags = (await blobClient.GetTagsAsync()).Value.Tags; + var expiryDate = DateTime.UtcNow + retentionPeriod.Value; + var tag = RetentionPolicyUtils.CreateExpiryDateTag(DateOnly.FromDateTime(expiryDate)); + if (tags.TryAdd(tag.Key, tag.Value)) { + await blobClient.SetTagsAsync(tags); + } + } } private async Async.Task RequeueMessage(string msg, TimeSpan? visibilityTimeout = null) { diff --git a/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs b/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs index dc133e1fba..0eca5b1e00 100644 --- a/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs @@ -22,13 +22,12 @@ public NotificationOperations(ILogger log, IOnefuzzConte } public async Async.Task NewFiles(Container container, string filename, bool isLastRetryAttempt) { - var result = OneFuzzResultVoid.Ok; - // We don't want to store file added events for the events container because that causes an infinite loop if (container == WellKnownContainers.Events) { - return result; + return Result.Ok(); } + var result = OneFuzzResultVoid.Ok; var notifications = GetNotifications(container); var hasNotifications = await notifications.AnyAsync(); var reportOrRegression = await _context.Reports.GetReportOrRegression(container, filename, expectReports: hasNotifications); diff --git a/src/ApiService/ApiService/onefuzzlib/RententionPolicy.cs b/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs similarity index 54% rename from src/ApiService/ApiService/onefuzzlib/RententionPolicy.cs rename to src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs index 4052db93e1..a09b7ac80d 100644 --- a/src/ApiService/ApiService/onefuzzlib/RententionPolicy.cs +++ b/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs @@ -1,4 +1,6 @@ -namespace Microsoft.OneFuzz.Service; +using System.Xml; + +namespace Microsoft.OneFuzz.Service; public interface IRetentionPolicy { @@ -21,4 +23,21 @@ public static KeyValuePair CreateExpiryDateTag(DateOnly expiryDa } public static string CreateExpiredBlobTagFilter() => $@"""{EXPIRY_TAG}"" <= '{DateOnly.FromDateTime(DateTime.UtcNow)}'"; + + public const string RETENTION_KEY = "RetentionPeriod"; + + public static TimeSpan? GetRetentionPeriodFromMetadata(IDictionary? containerMetadata) { + if (containerMetadata is not null && + containerMetadata.TryGetValue(RETENTION_KEY, out var retentionString) && + !string.IsNullOrWhiteSpace(retentionString)) { + try { + return XmlConvert.ToTimeSpan(retentionString); + } catch { + // Log error: unable to convert xxx + return null; + } + } + + return null; + } } From c50c5c4a2f9426bc192778e60a5dd07a304728af Mon Sep 17 00:00:00 2001 From: George Pollard Date: Tue, 12 Sep 2023 00:16:15 +0000 Subject: [PATCH 02/24] Silence analysis --- src/ApiService/ApiService/Functions/QueueFileChanges.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ApiService/ApiService/Functions/QueueFileChanges.cs b/src/ApiService/ApiService/Functions/QueueFileChanges.cs index 5b5ef5fed8..1935302bc7 100644 --- a/src/ApiService/ApiService/Functions/QueueFileChanges.cs +++ b/src/ApiService/ApiService/Functions/QueueFileChanges.cs @@ -101,7 +101,7 @@ private async Async.Task ApplyRetentionPolicy(ResourceIdentifier storageAccount, var expiryDate = DateTime.UtcNow + retentionPeriod.Value; var tag = RetentionPolicyUtils.CreateExpiryDateTag(DateOnly.FromDateTime(expiryDate)); if (tags.TryAdd(tag.Key, tag.Value)) { - await blobClient.SetTagsAsync(tags); + _ = await blobClient.SetTagsAsync(tags); } } } From 6c2b4c1bc3ef2e7a3762ad3439f42cdac4981f67 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 14 Sep 2023 20:50:45 +0000 Subject: [PATCH 03/24] "namespace" metadata --- src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs b/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs index a09b7ac80d..762c246541 100644 --- a/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs +++ b/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs @@ -24,7 +24,7 @@ public static KeyValuePair CreateExpiryDateTag(DateOnly expiryDa public static string CreateExpiredBlobTagFilter() => $@"""{EXPIRY_TAG}"" <= '{DateOnly.FromDateTime(DateTime.UtcNow)}'"; - public const string RETENTION_KEY = "RetentionPeriod"; + public const string RETENTION_KEY = "OneFuzz_RetentionPeriod"; public static TimeSpan? GetRetentionPeriodFromMetadata(IDictionary? containerMetadata) { if (containerMetadata is not null && From 054a19f2180b040a3c48464a5bed1065a6099636 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 14 Sep 2023 21:46:38 +0000 Subject: [PATCH 04/24] Define retention periods from CLI --- .../ApiService/onefuzzlib/RetentionPolicy.cs | 3 +- src/cli/onefuzz/api.py | 3 +- src/cli/onefuzz/templates/__init__.py | 82 +++++++++--- src/cli/onefuzz/templates/afl.py | 10 +- src/cli/onefuzz/templates/libfuzzer.py | 123 +++++++++++++----- src/cli/onefuzz/templates/ossfuzz.py | 6 +- src/cli/onefuzz/templates/radamsa.py | 31 +++-- src/cli/onefuzz/templates/regression.py | 23 ++-- src/pytypes/onefuzztypes/models.py | 3 +- 9 files changed, 208 insertions(+), 76 deletions(-) diff --git a/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs b/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs index 762c246541..ddbdb7ae27 100644 --- a/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs +++ b/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs @@ -24,7 +24,8 @@ public static KeyValuePair CreateExpiryDateTag(DateOnly expiryDa public static string CreateExpiredBlobTagFilter() => $@"""{EXPIRY_TAG}"" <= '{DateOnly.FromDateTime(DateTime.UtcNow)}'"; - public const string RETENTION_KEY = "OneFuzz_RetentionPeriod"; + // NB: this must match the value used on the CLI side + public const string RETENTION_KEY = "onefuzz_retentionperiod"; public static TimeSpan? GetRetentionPeriodFromMetadata(IDictionary? containerMetadata) { if (containerMetadata is not null && diff --git a/src/cli/onefuzz/api.py b/src/cli/onefuzz/api.py index 6968192642..38ef3a8d53 100644 --- a/src/cli/onefuzz/api.py +++ b/src/cli/onefuzz/api.py @@ -16,6 +16,7 @@ from typing import Callable, Dict, List, Optional, Tuple, Type, TypeVar, Union from urllib.parse import urlparse from uuid import UUID +from onefuzz.templates import ContainerTemplate import semver from memoization import cached @@ -1018,7 +1019,7 @@ def create( job_id: UUID_EXPANSION, task_type: TaskType, target_exe: str, - containers: List[Tuple[enums.ContainerType, primitives.Container]], + containers: List[Tuple[enums.ContainerType, ContainerTemplate]], *, analyzer_env: Optional[Dict[str, str]] = None, analyzer_exe: Optional[str] = None, diff --git a/src/cli/onefuzz/templates/__init__.py b/src/cli/onefuzz/templates/__init__.py index c7bb21d41a..550ffc5b3d 100644 --- a/src/cli/onefuzz/templates/__init__.py +++ b/src/cli/onefuzz/templates/__init__.py @@ -6,6 +6,7 @@ import os import tempfile import zipfile +from datetime import timedelta from typing import Any, Dict, List, Optional, Tuple from uuid import uuid4 @@ -22,6 +23,28 @@ class StoppedEarly(Exception): pass +class ContainerTemplate: + def __init__( + self, + name: Container, + exists: bool, + *, + retention_period: Optional[timedelta] = None, + ): + self.name = name + self.retention_period = retention_period + # TODO: exists is not yet used/checked + self.exists = exists + + @staticmethod + def existing(name: Container): + return ContainerTemplate(name, True) + + @staticmethod + def fresh(name: Container, *, retention_period: Optional[timedelta] = None): + return ContainerTemplate(name, False, retention_period=retention_period) + + class JobHelper: def __init__( self, @@ -59,7 +82,7 @@ def __init__( self.wait_for_running: bool = False self.wait_for_stopped: bool = False - self.containers: Dict[ContainerType, Container] = {} + self.containers: Dict[ContainerType, ContainerTemplate] = {} self.tags: Dict[str, str] = {"project": project, "name": name, "build": build} if job is None: self.onefuzz.versions.check() @@ -71,6 +94,11 @@ def __init__( else: self.job = job + def add_existing_container( + self, container_type: ContainerType, container: Container + ) -> None: + self.containers[container_type] = ContainerTemplate.existing(container) + def define_containers(self, *types: ContainerType) -> None: """ Define default container set based on provided types @@ -79,13 +107,23 @@ def define_containers(self, *types: ContainerType) -> None: """ for container_type in types: - self.containers[container_type] = self.onefuzz.utils.build_container_name( + container_name = self.onefuzz.utils.build_container_name( container_type=container_type, project=self.project, name=self.name, build=self.build, platform=self.platform, ) + self.containers[container_type] = ContainerTemplate.fresh( + container_name, + retention_period=JobHelper._default_retention_period(container_type), + ) + + @staticmethod + def _default_retention_period(container_type: ContainerType) -> Optional[timedelta]: + if container_type == ContainerType.crashdumps: + return timedelta(days=90) + return None def get_unique_container_name(self, container_type: ContainerType) -> Container: return Container( @@ -97,11 +135,17 @@ def get_unique_container_name(self, container_type: ContainerType) -> Container: ) def create_containers(self) -> None: - for container_type, container_name in self.containers.items(): - self.logger.info("using container: %s", container_name) - self.onefuzz.containers.create( - container_name, metadata={"container_type": container_type.name} - ) + for container_type, container in self.containers.items(): + self.logger.info("using container: %s", container.name) + metadata = {"container_type": container_type.name} + if container.retention_period is not None: + # format as ISO8601 period + # NB: this must match the value used on the server side + metadata[ + "onefuzz_retentionperiod" + ] = f"P{container.retention_period.days}D" + + self.onefuzz.containers.create(container.name, metadata=metadata) def delete_container(self, container_name: Container) -> None: self.onefuzz.containers.delete(container_name) @@ -110,7 +154,7 @@ def setup_notifications(self, config: Optional[NotificationConfig]) -> None: if not config: return - containers: List[Container] = [] + containers: List[ContainerTemplate] = [] if ContainerType.unique_reports in self.containers: containers.append(self.containers[ContainerType.unique_reports]) else: @@ -121,7 +165,9 @@ def setup_notifications(self, config: Optional[NotificationConfig]) -> None: for container in containers: self.logger.info("creating notification config for %s", container) - self.onefuzz.notifications.create(container, config, replace_existing=True) + self.onefuzz.notifications.create( + container.name, config, replace_existing=True + ) def upload_setup( self, @@ -141,25 +187,25 @@ def upload_setup( self.logger.info("uploading setup dir `%s`" % setup_dir) self.onefuzz.containers.files.upload_dir( - self.containers[ContainerType.setup], setup_dir + self.containers[ContainerType.setup].name, setup_dir ) else: self.logger.info("uploading target exe `%s`" % target_exe) self.onefuzz.containers.files.upload_file( - self.containers[ContainerType.setup], target_exe + self.containers[ContainerType.setup].name, target_exe ) pdb_path = os.path.splitext(target_exe)[0] + ".pdb" if os.path.exists(pdb_path): pdb_name = os.path.basename(pdb_path) self.onefuzz.containers.files.upload_file( - self.containers[ContainerType.setup], pdb_path, pdb_name + self.containers[ContainerType.setup].name, pdb_path, pdb_name ) if setup_files: for filename in setup_files: self.logger.info("uploading %s", filename) self.onefuzz.containers.files.upload_file( - self.containers[ContainerType.setup], filename + self.containers[ContainerType.setup].name, filename ) def upload_inputs(self, path: Directory, read_only: bool = False) -> None: @@ -167,7 +213,9 @@ def upload_inputs(self, path: Directory, read_only: bool = False) -> None: container_type = ContainerType.inputs if read_only: container_type = ContainerType.readonly_inputs - self.onefuzz.containers.files.upload_dir(self.containers[container_type], path) + self.onefuzz.containers.files.upload_dir( + self.containers[container_type].name, path + ) def upload_inputs_zip(self, path: File) -> None: with tempfile.TemporaryDirectory() as tmp_dir: @@ -176,7 +224,7 @@ def upload_inputs_zip(self, path: File) -> None: self.logger.info("uploading inputs from zip: `%s`" % path) self.onefuzz.containers.files.upload_dir( - self.containers[ContainerType.inputs], Directory(tmp_dir) + self.containers[ContainerType.inputs].name, Directory(tmp_dir) ) @classmethod @@ -195,8 +243,8 @@ def wait_on( wait_for_files = [] self.to_monitor = { - self.containers[x]: len( - self.onefuzz.containers.files.list(self.containers[x]).files + self.containers[x].name: len( + self.onefuzz.containers.files.list(self.containers[x].name).files ) for x in wait_for_files } diff --git a/src/cli/onefuzz/templates/afl.py b/src/cli/onefuzz/templates/afl.py index d3019a19cf..71efae7092 100644 --- a/src/cli/onefuzz/templates/afl.py +++ b/src/cli/onefuzz/templates/afl.py @@ -11,7 +11,7 @@ from onefuzz.api import Command -from . import JobHelper +from . import ContainerTemplate, JobHelper class AFL(Command): @@ -98,7 +98,7 @@ def basic( if existing_inputs: self.onefuzz.containers.get(existing_inputs) - helper.containers[ContainerType.inputs] = existing_inputs + helper.add_existing_container(ContainerType.inputs, existing_inputs) else: helper.define_containers(ContainerType.inputs) @@ -112,7 +112,7 @@ def basic( if ( len( self.onefuzz.containers.files.list( - helper.containers[ContainerType.inputs] + helper.containers[ContainerType.inputs].name ).files ) == 0 @@ -130,7 +130,7 @@ def basic( self.onefuzz.containers.get(afl_container) containers = [ - (ContainerType.tools, afl_container), + (ContainerType.tools, ContainerTemplate.existing(afl_container)), (ContainerType.setup, helper.containers[ContainerType.setup]), (ContainerType.crashes, helper.containers[ContainerType.crashes]), (ContainerType.inputs, helper.containers[ContainerType.inputs]), @@ -140,7 +140,7 @@ def basic( containers.append( ( ContainerType.extra_setup, - helper.containers[ContainerType.extra_setup], + ContainerTemplate.existing(extra_setup_container), ) ) diff --git a/src/cli/onefuzz/templates/libfuzzer.py b/src/cli/onefuzz/templates/libfuzzer.py index 7716cfefed..13c171fcc4 100644 --- a/src/cli/onefuzz/templates/libfuzzer.py +++ b/src/cli/onefuzz/templates/libfuzzer.py @@ -14,7 +14,7 @@ from onefuzz.api import Command -from . import JobHelper +from . import ContainerTemplate, JobHelper LIBFUZZER_MAGIC_STRING = b"ERROR: libFuzzer" @@ -45,8 +45,8 @@ def _check_is_libfuzzer(self, target_exe: File) -> None: def _add_optional_containers( self, - dest: List[Tuple[ContainerType, Container]], - source: Dict[ContainerType, Container], + dest: List[Tuple[ContainerType, ContainerTemplate]], + source: Dict[ContainerType, ContainerTemplate], types: List[ContainerType], ) -> None: dest.extend([(c, source[c]) for c in types if c in source]) @@ -55,7 +55,7 @@ def _create_tasks( self, *, job: Job, - containers: Dict[ContainerType, Container], + containers: Dict[ContainerType, ContainerTemplate], pool_name: PoolName, target_exe: str, vm_count: int = 2, @@ -292,7 +292,9 @@ def _create_tasks( ] if tools is not None: - analysis_containers.append((ContainerType.tools, tools)) + analysis_containers.append( + (ContainerType.tools, ContainerTemplate.existing(tools)) + ) self._add_optional_containers( analysis_containers, @@ -428,15 +430,17 @@ def basic( ) if existing_inputs: - helper.containers[ContainerType.inputs] = existing_inputs + helper.add_existing_container(ContainerType.inputs, existing_inputs) else: helper.define_containers(ContainerType.inputs) if readonly_inputs: - helper.containers[ContainerType.readonly_inputs] = readonly_inputs + helper.add_existing_container( + ContainerType.readonly_inputs, readonly_inputs + ) if crashes: - helper.containers[ContainerType.crashes] = crashes + helper.add_existing_container(ContainerType.crashes, crashes) if analyzer_exe is not None: helper.define_containers(ContainerType.analysis) @@ -465,17 +469,19 @@ def basic( else: source_allowlist_blob_name = None - containers = helper.containers - if extra_setup_container is not None: - containers[ContainerType.extra_setup] = extra_setup_container + helper.add_existing_container( + ContainerType.extra_setup, extra_setup_container + ) if extra_output_container is not None: - containers[ContainerType.extra_output] = extra_output_container + helper.add_existing_container( + ContainerType.extra_output, extra_output_container + ) self._create_tasks( job=helper.job, - containers=containers, + containers=helper.containers, pool_name=pool_name, target_exe=target_exe_blob_name, vm_count=vm_count, @@ -601,14 +607,30 @@ def merge( merge_containers = [ (ContainerType.setup, helper.containers[ContainerType.setup]), - ( - ContainerType.unique_inputs, - output_container or helper.containers[ContainerType.unique_inputs], - ), ] + if output_container: + merge_containers.append( + ( + ContainerType.unique_inputs, + ContainerTemplate.existing(output_container), + ) + ) + else: + merge_containers.append( + ( + ContainerType.unique_inputs, + helper.containers[ContainerType.unique_inputs], + ) + ) + if extra_setup_container is not None: - merge_containers.append((ContainerType.extra_setup, extra_setup_container)) + merge_containers.append( + ( + ContainerType.extra_setup, + ContainerTemplate.existing(extra_setup_container), + ) + ) if inputs: merge_containers.append( @@ -616,7 +638,12 @@ def merge( ) if existing_inputs: for existing_container in existing_inputs: - merge_containers.append((ContainerType.inputs, existing_container)) + merge_containers.append( + ( + ContainerType.inputs, + ContainerTemplate.existing(existing_container), + ) + ) self.logger.info("creating libfuzzer_merge task") self.onefuzz.tasks.create( @@ -738,15 +765,17 @@ def dotnet( containers = helper.containers if existing_inputs: - helper.containers[ContainerType.inputs] = existing_inputs + helper.add_existing_container(ContainerType.inputs, existing_inputs) else: helper.define_containers(ContainerType.inputs) if readonly_inputs: - helper.containers[ContainerType.readonly_inputs] = readonly_inputs + helper.add_existing_container( + ContainerType.readonly_inputs, readonly_inputs + ) if crashes: - helper.containers[ContainerType.crashes] = crashes + helper.add_existing_container(ContainerType.crashes, crashes) # Assumes that `libfuzzer-dotnet` and supporting tools were uploaded upon deployment. fuzzer_tools_container = Container( @@ -758,11 +787,16 @@ def dotnet( (ContainerType.crashes, containers[ContainerType.crashes]), (ContainerType.crashdumps, containers[ContainerType.crashdumps]), (ContainerType.inputs, containers[ContainerType.inputs]), - (ContainerType.tools, fuzzer_tools_container), + (ContainerType.tools, ContainerTemplate.existing(fuzzer_tools_container)), ] if extra_setup_container is not None: - fuzzer_containers.append((ContainerType.extra_setup, extra_setup_container)) + fuzzer_containers.append( + ( + ContainerType.extra_setup, + ContainerTemplate.existing(extra_setup_container), + ) + ) helper.create_containers() helper.setup_notifications(notification_config) @@ -817,12 +851,15 @@ def dotnet( (ContainerType.setup, containers[ContainerType.setup]), (ContainerType.coverage, containers[ContainerType.coverage]), (ContainerType.readonly_inputs, containers[ContainerType.inputs]), - (ContainerType.tools, fuzzer_tools_container), + (ContainerType.tools, ContainerTemplate.existing(fuzzer_tools_container)), ] if extra_setup_container is not None: coverage_containers.append( - (ContainerType.extra_setup, extra_setup_container) + ( + ContainerType.extra_setup, + ContainerTemplate.existing(extra_setup_container), + ) ) self.logger.info("creating `dotnet_coverage` task") @@ -851,11 +888,16 @@ def dotnet( (ContainerType.reports, containers[ContainerType.reports]), (ContainerType.unique_reports, containers[ContainerType.unique_reports]), (ContainerType.no_repro, containers[ContainerType.no_repro]), - (ContainerType.tools, fuzzer_tools_container), + (ContainerType.tools, ContainerTemplate.existing(fuzzer_tools_container)), ] if extra_setup_container is not None: - report_containers.append((ContainerType.extra_setup, extra_setup_container)) + report_containers.append( + ( + ContainerType.extra_setup, + ContainerTemplate.existing(extra_setup_container), + ) + ) self.logger.info("creating `dotnet_crash_report` task") self.onefuzz.tasks.create( @@ -972,13 +1014,13 @@ def qemu_user( if existing_inputs: self.onefuzz.containers.get(existing_inputs) # ensure it exists - helper.containers[ContainerType.inputs] = existing_inputs + helper.add_existing_container(ContainerType.inputs, existing_inputs) else: helper.define_containers(ContainerType.inputs) if crashes: self.onefuzz.containers.get(crashes) - helper.containers[ContainerType.crashes] = crashes + helper.add_existing_container(ContainerType.crashes, crashes) fuzzer_containers = [ (ContainerType.setup, helper.containers[ContainerType.setup]), @@ -988,11 +1030,21 @@ def qemu_user( ] if extra_setup_container is not None: - fuzzer_containers.append((ContainerType.extra_setup, extra_setup_container)) + fuzzer_containers.append( + ( + ContainerType.extra_setup, + ContainerTemplate.existing(extra_setup_container), + ) + ) if readonly_inputs is not None: self.onefuzz.containers.get(readonly_inputs) # ensure it exists - fuzzer_containers.append((ContainerType.readonly_inputs, readonly_inputs)) + fuzzer_containers.append( + ( + ContainerType.readonly_inputs, + ContainerTemplate.existing(readonly_inputs), + ) + ) helper.create_containers() @@ -1090,7 +1142,12 @@ def qemu_user( ] if extra_setup_container is not None: - report_containers.append((ContainerType.extra_setup, extra_setup_container)) + report_containers.append( + ( + ContainerType.extra_setup, + ContainerTemplate.existing(extra_setup_container), + ) + ) self.logger.info("creating libfuzzer_crash_report task") self.onefuzz.tasks.create( diff --git a/src/cli/onefuzz/templates/ossfuzz.py b/src/cli/onefuzz/templates/ossfuzz.py index fde1da0708..3d4a4300c3 100644 --- a/src/cli/onefuzz/templates/ossfuzz.py +++ b/src/cli/onefuzz/templates/ossfuzz.py @@ -215,13 +215,15 @@ def libfuzzer( ) if extra_setup_container is not None: - helper.containers[ContainerType.extra_setup] = extra_setup_container + helper.add_existing_container( + ContainerType.extra_setup, extra_setup_container + ) helper.create_containers() helper.setup_notifications(notification_config) dst_sas = self.onefuzz.containers.get( - helper.containers[ContainerType.setup] + helper.containers[ContainerType.setup].name ).sas_url self._copy_exe(container_sas["build"], dst_sas, File(fuzzer)) self._copy_all(container_sas["base"], dst_sas) diff --git a/src/cli/onefuzz/templates/radamsa.py b/src/cli/onefuzz/templates/radamsa.py index ea0b57fdb3..47fac97012 100644 --- a/src/cli/onefuzz/templates/radamsa.py +++ b/src/cli/onefuzz/templates/radamsa.py @@ -11,7 +11,7 @@ from onefuzz.api import Command -from . import JobHelper +from . import ContainerTemplate, JobHelper class Radamsa(Command): @@ -94,7 +94,9 @@ def basic( if existing_inputs: self.onefuzz.containers.get(existing_inputs) - helper.containers[ContainerType.readonly_inputs] = existing_inputs + helper.add_existing_container( + ContainerType.readonly_inputs, existing_inputs + ) else: helper.define_containers(ContainerType.readonly_inputs) helper.create_containers() @@ -108,7 +110,7 @@ def basic( if ( len( self.onefuzz.containers.files.list( - helper.containers[ContainerType.readonly_inputs] + helper.containers[ContainerType.readonly_inputs].name ).files ) == 0 @@ -148,7 +150,7 @@ def basic( self.logger.info("creating radamsa task") containers = [ - (ContainerType.tools, tools), + (ContainerType.tools, ContainerTemplate.existing(tools)), (ContainerType.setup, helper.containers[ContainerType.setup]), (ContainerType.crashes, helper.containers[ContainerType.crashes]), ( @@ -158,7 +160,12 @@ def basic( ] if extra_setup_container is not None: - containers.append((ContainerType.extra_setup, extra_setup_container)) + containers.append( + ( + ContainerType.extra_setup, + ContainerTemplate.existing(extra_setup_container), + ) + ) fuzzer_task = self.onefuzz.tasks.create( helper.job.job_id, @@ -194,7 +201,12 @@ def basic( ] if extra_setup_container is not None: - report_containers.append((ContainerType.extra_setup, extra_setup_container)) + report_containers.append( + ( + ContainerType.extra_setup, + ContainerTemplate.existing(extra_setup_container), + ) + ) self.logger.info("creating generic_crash_report task") self.onefuzz.tasks.create( @@ -234,14 +246,17 @@ def basic( analysis_containers = [ (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.tools, tools), + (ContainerType.tools, ContainerTemplate.existing(tools)), (ContainerType.analysis, helper.containers[ContainerType.analysis]), (ContainerType.crashes, helper.containers[ContainerType.crashes]), ] if extra_setup_container is not None: analysis_containers.append( - (ContainerType.extra_setup, extra_setup_container) + ( + ContainerType.extra_setup, + ContainerTemplate.existing(extra_setup_container), + ) ) self.onefuzz.tasks.create( diff --git a/src/cli/onefuzz/templates/regression.py b/src/cli/onefuzz/templates/regression.py index 00d8a10c37..7847277f2c 100644 --- a/src/cli/onefuzz/templates/regression.py +++ b/src/cli/onefuzz/templates/regression.py @@ -12,7 +12,7 @@ from onefuzz.api import Command -from . import JobHelper +from . import ContainerTemplate, JobHelper class Regression(Command): @@ -222,12 +222,17 @@ def _create_job( ] if extra_setup_container: - containers.append((ContainerType.extra_setup, extra_setup_container)) + containers.append( + ( + ContainerType.extra_setup, + ContainerTemplate.existing(extra_setup_container), + ) + ) if crashes: - helper.containers[ - ContainerType.readonly_inputs - ] = helper.get_unique_container_name(ContainerType.readonly_inputs) + helper.containers[ContainerType.readonly_inputs] = ContainerTemplate.fresh( + helper.get_unique_container_name(ContainerType.readonly_inputs) + ) containers.append( ( ContainerType.readonly_inputs, @@ -239,7 +244,7 @@ def _create_job( if crashes: for file in crashes: self.onefuzz.containers.files.upload_file( - helper.containers[ContainerType.readonly_inputs], file + helper.containers[ContainerType.readonly_inputs].name, file ) helper.setup_notifications(notification_config) @@ -276,7 +281,7 @@ def _create_job( if task.error: raise Exception("task failed: %s", task.error) - container = helper.containers[ContainerType.regression_reports] + container = helper.containers[ContainerType.regression_reports].name for filename in self.onefuzz.containers.files.list(container).files: self.logger.info("checking file: %s", filename) if self._check_regression(container, File(filename)): @@ -287,4 +292,6 @@ def _create_job( delete_input_container and ContainerType.readonly_inputs in helper.containers ): - helper.delete_container(helper.containers[ContainerType.readonly_inputs]) + helper.delete_container( + helper.containers[ContainerType.readonly_inputs].name + ) diff --git a/src/pytypes/onefuzztypes/models.py b/src/pytypes/onefuzztypes/models.py index c888621600..31ebd836ec 100644 --- a/src/pytypes/onefuzztypes/models.py +++ b/src/pytypes/onefuzztypes/models.py @@ -3,7 +3,7 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -from datetime import datetime +from datetime import datetime, timedelta from typing import Any, Dict, Generic, List, Optional, TypeVar, Union from uuid import UUID, uuid4 @@ -192,6 +192,7 @@ class TaskVm(BaseModel): class TaskContainers(BaseModel): type: ContainerType name: Container + retention_period: Optional[timedelta] class TaskConfig(BaseModel): From 03e110954a2658b8c11574ab6ff049ea409b6cd9 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 14 Sep 2023 21:51:52 +0000 Subject: [PATCH 05/24] Remove additional value from model --- src/pytypes/onefuzztypes/models.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/pytypes/onefuzztypes/models.py b/src/pytypes/onefuzztypes/models.py index 31ebd836ec..ba8a1bb03c 100644 --- a/src/pytypes/onefuzztypes/models.py +++ b/src/pytypes/onefuzztypes/models.py @@ -192,7 +192,6 @@ class TaskVm(BaseModel): class TaskContainers(BaseModel): type: ContainerType name: Container - retention_period: Optional[timedelta] class TaskConfig(BaseModel): From bdab6f0ef8c4bc3c42eb0dbbc003160a32b67151 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 14 Sep 2023 22:08:07 +0000 Subject: [PATCH 06/24] Unused import --- src/pytypes/onefuzztypes/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pytypes/onefuzztypes/models.py b/src/pytypes/onefuzztypes/models.py index ba8a1bb03c..c888621600 100644 --- a/src/pytypes/onefuzztypes/models.py +++ b/src/pytypes/onefuzztypes/models.py @@ -3,7 +3,7 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -from datetime import datetime, timedelta +from datetime import datetime from typing import Any, Dict, Generic, List, Optional, TypeVar, Union from uuid import UUID, uuid4 From 5fe6658cd45da20bc63f4d5bd6d432dab8a8c176 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 14 Sep 2023 22:11:36 +0000 Subject: [PATCH 07/24] Add feature flag and perform update concurrently --- src/ApiService/ApiService/FeatureFlags.cs | 1 + .../ApiService/Functions/QueueFileChanges.cs | 40 +++++++++++-------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/src/ApiService/ApiService/FeatureFlags.cs b/src/ApiService/ApiService/FeatureFlags.cs index aa4bc87079..e74396e882 100644 --- a/src/ApiService/ApiService/FeatureFlags.cs +++ b/src/ApiService/ApiService/FeatureFlags.cs @@ -8,4 +8,5 @@ public static class FeatureFlagConstants { public const string EnableBlobRetentionPolicy = "EnableBlobRetentionPolicy"; public const string EnableDryRunBlobRetention = "EnableDryRunBlobRetention"; public const string EnableWorkItemCreation = "EnableWorkItemCreation"; + public const string EnableContainerRetentionPolicies = "EnableContainerRetentionPolicies"; } diff --git a/src/ApiService/ApiService/Functions/QueueFileChanges.cs b/src/ApiService/ApiService/Functions/QueueFileChanges.cs index 1935302bc7..5097f55507 100644 --- a/src/ApiService/ApiService/Functions/QueueFileChanges.cs +++ b/src/ApiService/ApiService/Functions/QueueFileChanges.cs @@ -1,5 +1,6 @@ using System.Text.Json; using System.Text.Json.Nodes; +using System.Threading.Tasks; using Azure.Core; using Microsoft.Azure.Functions.Worker; using Microsoft.Extensions.Logging; @@ -83,27 +84,34 @@ private async Async.Task FileAdded(ResourceIdentifier storage _log.LogInformation("file added : {Container} - {Path}", container, path); - await ApplyRetentionPolicy(storageAccount, container, path); + var (_, result) = await ( + ApplyRetentionPolicy(storageAccount, container, path), + _notificationOperations.NewFiles(container, path, isLastRetryAttempt)); - return await _notificationOperations.NewFiles(container, path, isLastRetryAttempt); + return result; } - private async Async.Task ApplyRetentionPolicy(ResourceIdentifier storageAccount, Container container, string path) { - // default retention period can be applied to the container - // if one exists, we will set the expiry date on the newly-created blob, if it doesn't already have one - var account = await _storage.GetBlobServiceClientForAccount(storageAccount); - var containerClient = account.GetBlobContainerClient(container.String); - var containerProps = await containerClient.GetPropertiesAsync(); - var retentionPeriod = RetentionPolicyUtils.GetRetentionPeriodFromMetadata(containerProps.Value.Metadata); - if (retentionPeriod.HasValue) { - var blobClient = containerClient.GetBlobClient(path); - var tags = (await blobClient.GetTagsAsync()).Value.Tags; - var expiryDate = DateTime.UtcNow + retentionPeriod.Value; - var tag = RetentionPolicyUtils.CreateExpiryDateTag(DateOnly.FromDateTime(expiryDate)); - if (tags.TryAdd(tag.Key, tag.Value)) { - _ = await blobClient.SetTagsAsync(tags); + private async Async.Task ApplyRetentionPolicy(ResourceIdentifier storageAccount, Container container, string path) { + if (await _context.FeatureManagerSnapshot.IsEnabledAsync(FeatureFlagConstants.EnableContainerRetentionPolicies)) { + // default retention period can be applied to the container + // if one exists, we will set the expiry date on the newly-created blob, if it doesn't already have one + var account = await _storage.GetBlobServiceClientForAccount(storageAccount); + var containerClient = account.GetBlobContainerClient(container.String); + var containerProps = await containerClient.GetPropertiesAsync(); + var retentionPeriod = RetentionPolicyUtils.GetRetentionPeriodFromMetadata(containerProps.Value.Metadata); + if (retentionPeriod.HasValue) { + var blobClient = containerClient.GetBlobClient(path); + var tags = (await blobClient.GetTagsAsync()).Value.Tags; + var expiryDate = DateTime.UtcNow + retentionPeriod.Value; + var tag = RetentionPolicyUtils.CreateExpiryDateTag(DateOnly.FromDateTime(expiryDate)); + if (tags.TryAdd(tag.Key, tag.Value)) { + _ = await blobClient.SetTagsAsync(tags); + return true; + } } } + + return false; } private async Async.Task RequeueMessage(string msg, TimeSpan? visibilityTimeout = null) { From 97f00c5883a9b4bbcf2c16a490723fb71aad72e2 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 14 Sep 2023 22:35:27 +0000 Subject: [PATCH 08/24] ContainerTemplate should be a template-only concept --- src/cli/onefuzz/api.py | 3 +- src/cli/onefuzz/templates/__init__.py | 9 +++++ src/cli/onefuzz/templates/afl.py | 22 +++++------ src/cli/onefuzz/templates/libfuzzer.py | 49 +++++++++++-------------- src/cli/onefuzz/templates/ossfuzz.py | 2 +- src/cli/onefuzz/templates/radamsa.py | 22 +++++------ src/cli/onefuzz/templates/regression.py | 18 ++++----- 7 files changed, 64 insertions(+), 61 deletions(-) diff --git a/src/cli/onefuzz/api.py b/src/cli/onefuzz/api.py index 38ef3a8d53..6968192642 100644 --- a/src/cli/onefuzz/api.py +++ b/src/cli/onefuzz/api.py @@ -16,7 +16,6 @@ from typing import Callable, Dict, List, Optional, Tuple, Type, TypeVar, Union from urllib.parse import urlparse from uuid import UUID -from onefuzz.templates import ContainerTemplate import semver from memoization import cached @@ -1019,7 +1018,7 @@ def create( job_id: UUID_EXPANSION, task_type: TaskType, target_exe: str, - containers: List[Tuple[enums.ContainerType, ContainerTemplate]], + containers: List[Tuple[enums.ContainerType, primitives.Container]], *, analyzer_env: Optional[Dict[str, str]] = None, analyzer_exe: Optional[str] = None, diff --git a/src/cli/onefuzz/templates/__init__.py b/src/cli/onefuzz/templates/__init__.py index 550ffc5b3d..f51b810cfc 100644 --- a/src/cli/onefuzz/templates/__init__.py +++ b/src/cli/onefuzz/templates/__init__.py @@ -99,6 +99,15 @@ def add_existing_container( ) -> None: self.containers[container_type] = ContainerTemplate.existing(container) + def container_name(self, container_type: ContainerType) -> Container: + return self.containers[container_type].name + + def container_names(self) -> Dict[ContainerType, Container]: + return { + container_type: container.name + for (container_type, container) in self.containers.items() + } + def define_containers(self, *types: ContainerType) -> None: """ Define default container set based on provided types diff --git a/src/cli/onefuzz/templates/afl.py b/src/cli/onefuzz/templates/afl.py index 71efae7092..e4d49233dc 100644 --- a/src/cli/onefuzz/templates/afl.py +++ b/src/cli/onefuzz/templates/afl.py @@ -11,7 +11,7 @@ from onefuzz.api import Command -from . import ContainerTemplate, JobHelper +from . import JobHelper class AFL(Command): @@ -130,17 +130,17 @@ def basic( self.onefuzz.containers.get(afl_container) containers = [ - (ContainerType.tools, ContainerTemplate.existing(afl_container)), - (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.crashes, helper.containers[ContainerType.crashes]), - (ContainerType.inputs, helper.containers[ContainerType.inputs]), + (ContainerType.tools, afl_container), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.crashes, helper.container_name(ContainerType.crashes)), + (ContainerType.inputs, helper.container_name(ContainerType.inputs)), ] if extra_setup_container is not None: containers.append( ( ContainerType.extra_setup, - ContainerTemplate.existing(extra_setup_container), + extra_setup_container, ) ) @@ -169,12 +169,12 @@ def basic( ) report_containers = [ - (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.crashes, helper.containers[ContainerType.crashes]), - (ContainerType.reports, helper.containers[ContainerType.reports]), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.crashes, helper.container_name(ContainerType.crashes)), + (ContainerType.reports, helper.container_name(ContainerType.reports)), ( ContainerType.unique_reports, - helper.containers[ContainerType.unique_reports], + helper.container_name(ContainerType.unique_reports), ), ] @@ -182,7 +182,7 @@ def basic( report_containers.append( ( ContainerType.extra_setup, - helper.containers[ContainerType.extra_setup], + helper.container_name(ContainerType.extra_setup), ) ) diff --git a/src/cli/onefuzz/templates/libfuzzer.py b/src/cli/onefuzz/templates/libfuzzer.py index 13c171fcc4..c1d228c848 100644 --- a/src/cli/onefuzz/templates/libfuzzer.py +++ b/src/cli/onefuzz/templates/libfuzzer.py @@ -45,8 +45,8 @@ def _check_is_libfuzzer(self, target_exe: File) -> None: def _add_optional_containers( self, - dest: List[Tuple[ContainerType, ContainerTemplate]], - source: Dict[ContainerType, ContainerTemplate], + dest: List[Tuple[ContainerType, Container]], + source: Dict[ContainerType, Container], types: List[ContainerType], ) -> None: dest.extend([(c, source[c]) for c in types if c in source]) @@ -55,7 +55,7 @@ def _create_tasks( self, *, job: Job, - containers: Dict[ContainerType, ContainerTemplate], + containers: Dict[ContainerType, Container], pool_name: PoolName, target_exe: str, vm_count: int = 2, @@ -481,7 +481,7 @@ def basic( self._create_tasks( job=helper.job, - containers=helper.containers, + containers=helper.container_names(), pool_name=pool_name, target_exe=target_exe_blob_name, vm_count=vm_count, @@ -606,21 +606,21 @@ def merge( target_exe_blob_name = helper.setup_relative_blob_name(target_exe, setup_dir) merge_containers = [ - (ContainerType.setup, helper.containers[ContainerType.setup]), + (ContainerType.setup, helper.container_name(ContainerType.setup)), ] if output_container: merge_containers.append( ( ContainerType.unique_inputs, - ContainerTemplate.existing(output_container), + output_container, ) ) else: merge_containers.append( ( ContainerType.unique_inputs, - helper.containers[ContainerType.unique_inputs], + helper.container_name(ContainerType.unique_inputs), ) ) @@ -628,22 +628,17 @@ def merge( merge_containers.append( ( ContainerType.extra_setup, - ContainerTemplate.existing(extra_setup_container), + extra_setup_container, ) ) if inputs: merge_containers.append( - (ContainerType.inputs, helper.containers[ContainerType.inputs]) + (ContainerType.inputs, helper.container_name(ContainerType.inputs)) ) if existing_inputs: for existing_container in existing_inputs: - merge_containers.append( - ( - ContainerType.inputs, - ContainerTemplate.existing(existing_container), - ) - ) + merge_containers.append((ContainerType.inputs, existing_container)) self.logger.info("creating libfuzzer_merge task") self.onefuzz.tasks.create( @@ -1023,17 +1018,17 @@ def qemu_user( helper.add_existing_container(ContainerType.crashes, crashes) fuzzer_containers = [ - (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.crashes, helper.containers[ContainerType.crashes]), - (ContainerType.crashdumps, helper.containers[ContainerType.crashdumps]), - (ContainerType.inputs, helper.containers[ContainerType.inputs]), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.crashes, helper.container_name(ContainerType.crashes)), + (ContainerType.crashdumps, helper.container_name(ContainerType.crashdumps)), + (ContainerType.inputs, helper.container_name(ContainerType.inputs)), ] if extra_setup_container is not None: fuzzer_containers.append( ( ContainerType.extra_setup, - ContainerTemplate.existing(extra_setup_container), + extra_setup_container, ) ) @@ -1042,7 +1037,7 @@ def qemu_user( fuzzer_containers.append( ( ContainerType.readonly_inputs, - ContainerTemplate.existing(readonly_inputs), + readonly_inputs, ) ) @@ -1131,21 +1126,21 @@ def qemu_user( ) report_containers = [ - (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.crashes, helper.containers[ContainerType.crashes]), - (ContainerType.reports, helper.containers[ContainerType.reports]), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.crashes, helper.container_name(ContainerType.crashes)), + (ContainerType.reports, helper.container_name(ContainerType.reports)), ( ContainerType.unique_reports, - helper.containers[ContainerType.unique_reports], + helper.container_name(ContainerType.unique_reports), ), - (ContainerType.no_repro, helper.containers[ContainerType.no_repro]), + (ContainerType.no_repro, helper.container_name(ContainerType.no_repro)), ] if extra_setup_container is not None: report_containers.append( ( ContainerType.extra_setup, - ContainerTemplate.existing(extra_setup_container), + extra_setup_container, ) ) diff --git a/src/cli/onefuzz/templates/ossfuzz.py b/src/cli/onefuzz/templates/ossfuzz.py index 3d4a4300c3..94024add83 100644 --- a/src/cli/onefuzz/templates/ossfuzz.py +++ b/src/cli/onefuzz/templates/ossfuzz.py @@ -247,7 +247,7 @@ def libfuzzer( self.onefuzz.template.libfuzzer._create_tasks( job=base_helper.job, - containers=helper.containers, + containers=helper.container_names(), pool_name=pool_name, target_exe=fuzzer_blob_name, vm_count=VM_COUNT, diff --git a/src/cli/onefuzz/templates/radamsa.py b/src/cli/onefuzz/templates/radamsa.py index 47fac97012..befcf9e31c 100644 --- a/src/cli/onefuzz/templates/radamsa.py +++ b/src/cli/onefuzz/templates/radamsa.py @@ -190,21 +190,21 @@ def basic( ) report_containers = [ - (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.crashes, helper.containers[ContainerType.crashes]), - (ContainerType.reports, helper.containers[ContainerType.reports]), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.crashes, helper.container_name(ContainerType.crashes)), + (ContainerType.reports, helper.container_name(ContainerType.reports)), ( ContainerType.unique_reports, - helper.containers[ContainerType.unique_reports], + helper.container_name(ContainerType.unique_reports), ), - (ContainerType.no_repro, helper.containers[ContainerType.no_repro]), + (ContainerType.no_repro, helper.container_name(ContainerType.no_repro)), ] if extra_setup_container is not None: report_containers.append( ( ContainerType.extra_setup, - ContainerTemplate.existing(extra_setup_container), + extra_setup_container, ) ) @@ -245,17 +245,17 @@ def basic( self.logger.info("creating custom analysis") analysis_containers = [ - (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.tools, ContainerTemplate.existing(tools)), - (ContainerType.analysis, helper.containers[ContainerType.analysis]), - (ContainerType.crashes, helper.containers[ContainerType.crashes]), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.tools, tools), + (ContainerType.analysis, helper.container_name(ContainerType.analysis)), + (ContainerType.crashes, helper.container_name(ContainerType.crashes)), ] if extra_setup_container is not None: analysis_containers.append( ( ContainerType.extra_setup, - ContainerTemplate.existing(extra_setup_container), + extra_setup_container, ) ) diff --git a/src/cli/onefuzz/templates/regression.py b/src/cli/onefuzz/templates/regression.py index 7847277f2c..0aa2550da6 100644 --- a/src/cli/onefuzz/templates/regression.py +++ b/src/cli/onefuzz/templates/regression.py @@ -207,17 +207,17 @@ def _create_job( ) containers = [ - (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.crashes, helper.containers[ContainerType.crashes]), - (ContainerType.reports, helper.containers[ContainerType.reports]), - (ContainerType.no_repro, helper.containers[ContainerType.no_repro]), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.crashes, helper.container_name(ContainerType.crashes)), + (ContainerType.reports, helper.container_name(ContainerType.reports)), + (ContainerType.no_repro, helper.container_name(ContainerType.no_repro)), ( ContainerType.unique_reports, - helper.containers[ContainerType.unique_reports], + helper.container_name(ContainerType.unique_reports), ), ( ContainerType.regression_reports, - helper.containers[ContainerType.regression_reports], + helper.container_name(ContainerType.regression_reports), ), ] @@ -225,7 +225,7 @@ def _create_job( containers.append( ( ContainerType.extra_setup, - ContainerTemplate.existing(extra_setup_container), + extra_setup_container, ) ) @@ -236,7 +236,7 @@ def _create_job( containers.append( ( ContainerType.readonly_inputs, - helper.containers[ContainerType.readonly_inputs], + helper.container_name(ContainerType.readonly_inputs), ) ) @@ -244,7 +244,7 @@ def _create_job( if crashes: for file in crashes: self.onefuzz.containers.files.upload_file( - helper.containers[ContainerType.readonly_inputs].name, file + helper.container_name(ContainerType.readonly_inputs), file ) helper.setup_notifications(notification_config) From 1818cb2c236d24884358c4d270a8aadc65103165 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 14 Sep 2023 22:41:30 +0000 Subject: [PATCH 09/24] Use container_name more consistently --- src/cli/onefuzz/templates/__init__.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/cli/onefuzz/templates/__init__.py b/src/cli/onefuzz/templates/__init__.py index f51b810cfc..4aac118cae 100644 --- a/src/cli/onefuzz/templates/__init__.py +++ b/src/cli/onefuzz/templates/__init__.py @@ -163,19 +163,19 @@ def setup_notifications(self, config: Optional[NotificationConfig]) -> None: if not config: return - containers: List[ContainerTemplate] = [] + containers: List[Container] = [] if ContainerType.unique_reports in self.containers: - containers.append(self.containers[ContainerType.unique_reports]) + containers.append(self.container_name(ContainerType.unique_reports)) else: - containers.append(self.containers[ContainerType.reports]) + containers.append(self.container_name(ContainerType.reports)) if ContainerType.regression_reports in self.containers: - containers.append(self.containers[ContainerType.regression_reports]) + containers.append(self.container_name(ContainerType.regression_reports)) for container in containers: self.logger.info("creating notification config for %s", container) self.onefuzz.notifications.create( - container.name, config, replace_existing=True + container, config, replace_existing=True ) def upload_setup( @@ -196,25 +196,25 @@ def upload_setup( self.logger.info("uploading setup dir `%s`" % setup_dir) self.onefuzz.containers.files.upload_dir( - self.containers[ContainerType.setup].name, setup_dir + self.container_name(ContainerType.setup), setup_dir ) else: self.logger.info("uploading target exe `%s`" % target_exe) self.onefuzz.containers.files.upload_file( - self.containers[ContainerType.setup].name, target_exe + self.container_name(ContainerType.setup), target_exe ) pdb_path = os.path.splitext(target_exe)[0] + ".pdb" if os.path.exists(pdb_path): pdb_name = os.path.basename(pdb_path) self.onefuzz.containers.files.upload_file( - self.containers[ContainerType.setup].name, pdb_path, pdb_name + self.container_name(ContainerType.setup), pdb_path, pdb_name ) if setup_files: for filename in setup_files: self.logger.info("uploading %s", filename) self.onefuzz.containers.files.upload_file( - self.containers[ContainerType.setup].name, filename + self.container_name(ContainerType.setup), filename ) def upload_inputs(self, path: Directory, read_only: bool = False) -> None: @@ -223,7 +223,7 @@ def upload_inputs(self, path: Directory, read_only: bool = False) -> None: if read_only: container_type = ContainerType.readonly_inputs self.onefuzz.containers.files.upload_dir( - self.containers[container_type].name, path + self.container_name(container_type), path ) def upload_inputs_zip(self, path: File) -> None: @@ -233,7 +233,7 @@ def upload_inputs_zip(self, path: File) -> None: self.logger.info("uploading inputs from zip: `%s`" % path) self.onefuzz.containers.files.upload_dir( - self.containers[ContainerType.inputs].name, Directory(tmp_dir) + self.container_name(ContainerType.inputs), Directory(tmp_dir) ) @classmethod @@ -252,8 +252,8 @@ def wait_on( wait_for_files = [] self.to_monitor = { - self.containers[x].name: len( - self.onefuzz.containers.files.list(self.containers[x].name).files + self.container_name(x): len( + self.onefuzz.containers.files.list(self.container_name(x)).files ) for x in wait_for_files } From afa426eded881771371659339bf66a78dc1a9a58 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 14 Sep 2023 22:42:42 +0000 Subject: [PATCH 10/24] Reformat --- src/cli/onefuzz/templates/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/cli/onefuzz/templates/__init__.py b/src/cli/onefuzz/templates/__init__.py index 4aac118cae..b283f55f17 100644 --- a/src/cli/onefuzz/templates/__init__.py +++ b/src/cli/onefuzz/templates/__init__.py @@ -174,9 +174,7 @@ def setup_notifications(self, config: Optional[NotificationConfig]) -> None: for container in containers: self.logger.info("creating notification config for %s", container) - self.onefuzz.notifications.create( - container, config, replace_existing=True - ) + self.onefuzz.notifications.create(container, config, replace_existing=True) def upload_setup( self, From 10e2dca4995f895c6de5347069df4e0c473bff67 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 14 Sep 2023 22:57:48 +0000 Subject: [PATCH 11/24] Typing fixes --- src/cli/examples/domato.py | 21 +++++---- src/cli/examples/honggfuzz.py | 19 ++++---- src/cli/onefuzz/templates/libfuzzer.py | 60 +++++++++++++------------- src/cli/onefuzz/templates/radamsa.py | 16 +++---- 4 files changed, 62 insertions(+), 54 deletions(-) diff --git a/src/cli/examples/domato.py b/src/cli/examples/domato.py index 7c2abc6301..4bdf2a297c 100755 --- a/src/cli/examples/domato.py +++ b/src/cli/examples/domato.py @@ -67,7 +67,7 @@ def upload_to_fuzzer_container(of: Onefuzz, fuzzer_name: str, fuzzer_url: str) - def upload_to_setup_container(of: Onefuzz, helper: JobHelper, setup_dir: str) -> None: - setup_sas = of.containers.get(helper.containers[ContainerType.setup]).sas_url + setup_sas = of.containers.get(helper.container_name(ContainerType.setup)).sas_url if AZCOPY_PATH is None: raise Exception("missing azcopy") command = [AZCOPY_PATH, "sync", setup_dir, setup_sas] @@ -143,13 +143,16 @@ def main() -> None: helper.create_containers() helper.setup_notifications(notification_config) upload_to_setup_container(of, helper, args.setup_dir) - add_setup_script(of, helper.containers[ContainerType.setup]) + add_setup_script(of, helper.container_name(ContainerType.setup)) containers = [ - (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.crashes, helper.containers[ContainerType.crashes]), - (ContainerType.reports, helper.containers[ContainerType.reports]), - (ContainerType.unique_reports, helper.containers[ContainerType.unique_reports]), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.crashes, helper.container_name(ContainerType.crashes)), + (ContainerType.reports, helper.container_name(ContainerType.reports)), + ( + ContainerType.unique_reports, + helper.container_name(ContainerType.unique_reports), + ), ] of.logger.info("Creating generic_crash_report task") @@ -164,11 +167,11 @@ def main() -> None: containers = [ (ContainerType.tools, Container(FUZZER_NAME)), - (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.crashes, helper.containers[ContainerType.crashes]), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.crashes, helper.container_name(ContainerType.crashes)), ( ContainerType.readonly_inputs, - helper.containers[ContainerType.readonly_inputs], + helper.container_name(ContainerType.readonly_inputs), ), ] diff --git a/src/cli/examples/honggfuzz.py b/src/cli/examples/honggfuzz.py index 225b7f7510..9466716d98 100644 --- a/src/cli/examples/honggfuzz.py +++ b/src/cli/examples/honggfuzz.py @@ -88,13 +88,16 @@ def main() -> None: if args.inputs: helper.upload_inputs(args.inputs) - add_setup_script(of, helper.containers[ContainerType.setup]) + add_setup_script(of, helper.container_name(ContainerType.setup)) containers = [ - (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.crashes, helper.containers[ContainerType.crashes]), - (ContainerType.reports, helper.containers[ContainerType.reports]), - (ContainerType.unique_reports, helper.containers[ContainerType.unique_reports]), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.crashes, helper.container_name(ContainerType.crashes)), + (ContainerType.reports, helper.container_name(ContainerType.reports)), + ( + ContainerType.unique_reports, + helper.container_name(ContainerType.unique_reports), + ), ] of.logger.info("Creating generic_crash_report task") @@ -109,11 +112,11 @@ def main() -> None: containers = [ (ContainerType.tools, Container("honggfuzz")), - (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.crashes, helper.containers[ContainerType.crashes]), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.crashes, helper.container_name(ContainerType.crashes)), ( ContainerType.inputs, - helper.containers[ContainerType.inputs], + helper.container_name(ContainerType.inputs), ), ] diff --git a/src/cli/onefuzz/templates/libfuzzer.py b/src/cli/onefuzz/templates/libfuzzer.py index c1d228c848..f487372121 100644 --- a/src/cli/onefuzz/templates/libfuzzer.py +++ b/src/cli/onefuzz/templates/libfuzzer.py @@ -14,7 +14,7 @@ from onefuzz.api import Command -from . import ContainerTemplate, JobHelper +from . import JobHelper LIBFUZZER_MAGIC_STRING = b"ERROR: libFuzzer" @@ -85,7 +85,7 @@ def _create_tasks( task_env: Optional[Dict[str, str]] = None, ) -> None: target_options = target_options or [] - regression_containers = [ + regression_containers: List[Tuple[ContainerType, Container]] = [ (ContainerType.setup, containers[ContainerType.setup]), (ContainerType.crashes, containers[ContainerType.crashes]), (ContainerType.unique_reports, containers[ContainerType.unique_reports]), @@ -129,7 +129,7 @@ def _create_tasks( task_env=task_env, ) - fuzzer_containers = [ + fuzzer_containers: List[Tuple[ContainerType, Container]] = [ (ContainerType.setup, containers[ContainerType.setup]), (ContainerType.crashes, containers[ContainerType.crashes]), (ContainerType.crashdumps, containers[ContainerType.crashdumps]), @@ -184,7 +184,7 @@ def _create_tasks( prereq_tasks = [fuzzer_task.task_id, regression_task.task_id] - coverage_containers = [ + coverage_containers: List[Tuple[ContainerType, Container]] = [ (ContainerType.setup, containers[ContainerType.setup]), (ContainerType.coverage, containers[ContainerType.coverage]), (ContainerType.readonly_inputs, containers[ContainerType.inputs]), @@ -245,7 +245,7 @@ def _create_tasks( task_env=task_env, ) - report_containers = [ + report_containers: List[Tuple[ContainerType, Container]] = [ (ContainerType.setup, containers[ContainerType.setup]), (ContainerType.crashes, containers[ContainerType.crashes]), (ContainerType.reports, containers[ContainerType.reports]), @@ -285,16 +285,14 @@ def _create_tasks( if analyzer_exe is not None: self.logger.info("creating custom analysis") - analysis_containers = [ + analysis_containers: List[Tuple[ContainerType, Container]] = [ (ContainerType.setup, containers[ContainerType.setup]), (ContainerType.analysis, containers[ContainerType.analysis]), (ContainerType.crashes, containers[ContainerType.crashes]), ] if tools is not None: - analysis_containers.append( - (ContainerType.tools, ContainerTemplate.existing(tools)) - ) + analysis_containers.append((ContainerType.tools, tools)) self._add_optional_containers( analysis_containers, @@ -757,8 +755,6 @@ def dotnet( ContainerType.no_repro, ) - containers = helper.containers - if existing_inputs: helper.add_existing_container(ContainerType.inputs, existing_inputs) else: @@ -778,18 +774,18 @@ def dotnet( ) fuzzer_containers = [ - (ContainerType.setup, containers[ContainerType.setup]), - (ContainerType.crashes, containers[ContainerType.crashes]), - (ContainerType.crashdumps, containers[ContainerType.crashdumps]), - (ContainerType.inputs, containers[ContainerType.inputs]), - (ContainerType.tools, ContainerTemplate.existing(fuzzer_tools_container)), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.crashes, helper.container_name(ContainerType.crashes)), + (ContainerType.crashdumps, helper.container_name(ContainerType.crashdumps)), + (ContainerType.inputs, helper.container_name(ContainerType.inputs)), + (ContainerType.tools, fuzzer_tools_container), ] if extra_setup_container is not None: fuzzer_containers.append( ( ContainerType.extra_setup, - ContainerTemplate.existing(extra_setup_container), + extra_setup_container, ) ) @@ -843,17 +839,20 @@ def dotnet( libfuzzer_dotnet_loader_dll = LIBFUZZER_DOTNET_LOADER_PATH coverage_containers = [ - (ContainerType.setup, containers[ContainerType.setup]), - (ContainerType.coverage, containers[ContainerType.coverage]), - (ContainerType.readonly_inputs, containers[ContainerType.inputs]), - (ContainerType.tools, ContainerTemplate.existing(fuzzer_tools_container)), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.coverage, helper.container_name(ContainerType.coverage)), + ( + ContainerType.readonly_inputs, + helper.container_name(ContainerType.inputs), + ), + (ContainerType.tools, fuzzer_tools_container), ] if extra_setup_container is not None: coverage_containers.append( ( ContainerType.extra_setup, - ContainerTemplate.existing(extra_setup_container), + extra_setup_container, ) ) @@ -878,19 +877,22 @@ def dotnet( ) report_containers = [ - (ContainerType.setup, containers[ContainerType.setup]), - (ContainerType.crashes, containers[ContainerType.crashes]), - (ContainerType.reports, containers[ContainerType.reports]), - (ContainerType.unique_reports, containers[ContainerType.unique_reports]), - (ContainerType.no_repro, containers[ContainerType.no_repro]), - (ContainerType.tools, ContainerTemplate.existing(fuzzer_tools_container)), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.crashes, helper.container_name(ContainerType.crashes)), + (ContainerType.reports, helper.container_name(ContainerType.reports)), + ( + ContainerType.unique_reports, + helper.container_name(ContainerType.unique_reports), + ), + (ContainerType.no_repro, helper.container_name(ContainerType.no_repro)), + (ContainerType.tools, fuzzer_tools_container), ] if extra_setup_container is not None: report_containers.append( ( ContainerType.extra_setup, - ContainerTemplate.existing(extra_setup_container), + extra_setup_container, ) ) diff --git a/src/cli/onefuzz/templates/radamsa.py b/src/cli/onefuzz/templates/radamsa.py index befcf9e31c..d9ec34e15f 100644 --- a/src/cli/onefuzz/templates/radamsa.py +++ b/src/cli/onefuzz/templates/radamsa.py @@ -3,7 +3,7 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -from typing import Dict, List, Optional +from typing import Dict, List, Optional, Tuple from onefuzztypes.enums import OS, ContainerType, TaskDebugFlag, TaskType from onefuzztypes.models import Job, NotificationConfig @@ -11,7 +11,7 @@ from onefuzz.api import Command -from . import ContainerTemplate, JobHelper +from . import JobHelper class Radamsa(Command): @@ -149,13 +149,13 @@ def basic( self.logger.info("creating radamsa task") - containers = [ - (ContainerType.tools, ContainerTemplate.existing(tools)), - (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.crashes, helper.containers[ContainerType.crashes]), + containers: List[Tuple[ContainerType, Container]] = [ + (ContainerType.tools, tools), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.crashes, helper.container_name(ContainerType.crashes)), ( ContainerType.readonly_inputs, - helper.containers[ContainerType.readonly_inputs], + helper.container_name(ContainerType.readonly_inputs), ), ] @@ -163,7 +163,7 @@ def basic( containers.append( ( ContainerType.extra_setup, - ContainerTemplate.existing(extra_setup_container), + extra_setup_container, ) ) From 0ed0c95c0552d23c940070381f1ce8fce3c182cf Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 14 Sep 2023 23:28:02 +0000 Subject: [PATCH 12/24] Missing return type --- src/cli/onefuzz/templates/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cli/onefuzz/templates/__init__.py b/src/cli/onefuzz/templates/__init__.py index b283f55f17..aed7dfdefb 100644 --- a/src/cli/onefuzz/templates/__init__.py +++ b/src/cli/onefuzz/templates/__init__.py @@ -7,7 +7,7 @@ import tempfile import zipfile from datetime import timedelta -from typing import Any, Dict, List, Optional, Tuple +from typing import Any, Dict, List, Optional, Self, Tuple from uuid import uuid4 from onefuzztypes.enums import OS, ContainerType, JobState, TaskState @@ -37,11 +37,11 @@ def __init__( self.exists = exists @staticmethod - def existing(name: Container): + def existing(name: Container) -> Self: return ContainerTemplate(name, True) @staticmethod - def fresh(name: Container, *, retention_period: Optional[timedelta] = None): + def fresh(name: Container, *, retention_period: Optional[timedelta] = None) -> Self: return ContainerTemplate(name, False, retention_period=retention_period) From d7ba6875a89bbf6bc1b73a9083ebbd3f2c473a0f Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 14 Sep 2023 23:29:16 +0000 Subject: [PATCH 13/24] Update examples --- .../llvm-source-coverage/source-coverage-libfuzzer.py | 10 +++++----- .../examples/llvm-source-coverage/source-coverage.py | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/cli/examples/llvm-source-coverage/source-coverage-libfuzzer.py b/src/cli/examples/llvm-source-coverage/source-coverage-libfuzzer.py index a8a6a91ac9..b8ab5f347a 100755 --- a/src/cli/examples/llvm-source-coverage/source-coverage-libfuzzer.py +++ b/src/cli/examples/llvm-source-coverage/source-coverage-libfuzzer.py @@ -74,15 +74,15 @@ def main() -> None: helper.create_containers() of.containers.files.upload_file( - helper.containers[ContainerType.tools], f"{args.tools}/source-coverage.sh" + helper.container_name(ContainerType.tools), f"{args.tools}/source-coverage.sh" ) containers = [ - (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.analysis, helper.containers[ContainerType.analysis]), - (ContainerType.tools, helper.containers[ContainerType.tools]), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.analysis, helper.container_name(ContainerType.analysis)), + (ContainerType.tools, helper.container_name(ContainerType.tools)), # note, analysis is typically for crashes, but this is analyzing inputs - (ContainerType.crashes, helper.containers[ContainerType.inputs]), + (ContainerType.crashes, helper.container_name(ContainerType.inputs)), ] of.logger.info("Creating generic_analysis task") diff --git a/src/cli/examples/llvm-source-coverage/source-coverage.py b/src/cli/examples/llvm-source-coverage/source-coverage.py index 749662caba..ae903cd3b5 100755 --- a/src/cli/examples/llvm-source-coverage/source-coverage.py +++ b/src/cli/examples/llvm-source-coverage/source-coverage.py @@ -61,15 +61,15 @@ def main() -> None: helper.upload_inputs(args.inputs) of.containers.files.upload_file( - helper.containers[ContainerType.tools], f"{args.tools}/source-coverage.sh" + helper.container_name(ContainerType.tools), f"{args.tools}/source-coverage.sh" ) containers = [ - (ContainerType.setup, helper.containers[ContainerType.setup]), - (ContainerType.analysis, helper.containers[ContainerType.analysis]), - (ContainerType.tools, helper.containers[ContainerType.tools]), + (ContainerType.setup, helper.container_name(ContainerType.setup)), + (ContainerType.analysis, helper.container_name(ContainerType.analysis)), + (ContainerType.tools, helper.container_name(ContainerType.tools)), # note, analysis is typically for crashes, but this is analyzing inputs - (ContainerType.crashes, helper.containers[ContainerType.inputs]), + (ContainerType.crashes, helper.container_name(ContainerType.inputs)), ] of.logger.info("Creating generic_analysis task") From f7bade699c67a0694a88bbf2d0f049aad3293c0a Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 14 Sep 2023 23:34:27 +0000 Subject: [PATCH 14/24] Bump CLI Python version --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 99e9bddd32..197a44b9a4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -137,7 +137,7 @@ jobs: shell: bash - uses: actions/setup-python@v4 with: - python-version: 3.7 + python-version: 3.10 - uses: actions/download-artifact@v3 with: name: artifact-onefuzztypes From 8649e3e69f8861a02927fff5caaddcc4f94fa766 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 14 Sep 2023 23:35:48 +0000 Subject: [PATCH 15/24] Bump others as well --- .github/workflows/ci.yml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 197a44b9a4..702b904842 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -123,7 +123,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: 3.7 + python-version: 3.10 - name: lint shell: bash run: src/ci/check-check-pr.sh @@ -190,7 +190,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: 3.8 + python-version: 3.10 - name: lint shell: bash run: | @@ -208,7 +208,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: 3.8 + python-version: 3.10 - name: lint shell: bash run: | @@ -224,7 +224,7 @@ jobs: - run: src/ci/set-versions.sh - uses: actions/setup-python@v4 with: - python-version: 3.7 + python-version: 3.10 - run: src/ci/onefuzztypes.sh - uses: actions/upload-artifact@v3 with: @@ -481,7 +481,7 @@ jobs: path: artifacts - uses: actions/setup-python@v4 with: - python-version: 3.7 + python-version: 3.10 - name: Lint shell: bash run: | From 8ea092ff95ef0bdfe11258b891505a1c33dae228 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 14 Sep 2023 23:37:10 +0000 Subject: [PATCH 16/24] quotes --- .github/workflows/ci.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 702b904842..5f07124dd7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -123,7 +123,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: 3.10 + python-version: "3.10" - name: lint shell: bash run: src/ci/check-check-pr.sh @@ -137,7 +137,7 @@ jobs: shell: bash - uses: actions/setup-python@v4 with: - python-version: 3.10 + python-version: "3.10" - uses: actions/download-artifact@v3 with: name: artifact-onefuzztypes @@ -190,7 +190,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: 3.10 + python-version: "3.10" - name: lint shell: bash run: | @@ -208,7 +208,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: 3.10 + python-version: "3.10" - name: lint shell: bash run: | @@ -224,7 +224,7 @@ jobs: - run: src/ci/set-versions.sh - uses: actions/setup-python@v4 with: - python-version: 3.10 + python-version: "3.10" - run: src/ci/onefuzztypes.sh - uses: actions/upload-artifact@v3 with: @@ -481,7 +481,7 @@ jobs: path: artifacts - uses: actions/setup-python@v4 with: - python-version: 3.10 + python-version: "3.10" - name: Lint shell: bash run: | From 0844174d5fc6fd90805548652e06aefcc04a6eaa Mon Sep 17 00:00:00 2001 From: George Pollard Date: Fri, 15 Sep 2023 00:02:28 +0000 Subject: [PATCH 17/24] Actually need 3.11 for Self --- .github/workflows/ci.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5f07124dd7..2ec8871974 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -123,7 +123,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.10" + python-version: "3.11" - name: lint shell: bash run: src/ci/check-check-pr.sh @@ -137,7 +137,7 @@ jobs: shell: bash - uses: actions/setup-python@v4 with: - python-version: "3.10" + python-version: "3.11" - uses: actions/download-artifact@v3 with: name: artifact-onefuzztypes @@ -190,7 +190,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.10" + python-version: "3.11" - name: lint shell: bash run: | @@ -208,7 +208,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.10" + python-version: "3.11" - name: lint shell: bash run: | @@ -224,7 +224,7 @@ jobs: - run: src/ci/set-versions.sh - uses: actions/setup-python@v4 with: - python-version: "3.10" + python-version: "3.11" - run: src/ci/onefuzztypes.sh - uses: actions/upload-artifact@v3 with: @@ -481,7 +481,7 @@ jobs: path: artifacts - uses: actions/setup-python@v4 with: - python-version: "3.10" + python-version: "3.11" - name: Lint shell: bash run: | From 6f362fed95c18ee13f6b54ce527523bcaece9564 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Fri, 15 Sep 2023 00:05:19 +0000 Subject: [PATCH 18/24] Revert "Actually need 3.11 for Self" This reverts commit 0844174d5fc6fd90805548652e06aefcc04a6eaa. --- .github/workflows/ci.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2ec8871974..5f07124dd7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -123,7 +123,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.11" + python-version: "3.10" - name: lint shell: bash run: src/ci/check-check-pr.sh @@ -137,7 +137,7 @@ jobs: shell: bash - uses: actions/setup-python@v4 with: - python-version: "3.11" + python-version: "3.10" - uses: actions/download-artifact@v3 with: name: artifact-onefuzztypes @@ -190,7 +190,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.11" + python-version: "3.10" - name: lint shell: bash run: | @@ -208,7 +208,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-python@v4 with: - python-version: "3.11" + python-version: "3.10" - name: lint shell: bash run: | @@ -224,7 +224,7 @@ jobs: - run: src/ci/set-versions.sh - uses: actions/setup-python@v4 with: - python-version: "3.11" + python-version: "3.10" - run: src/ci/onefuzztypes.sh - uses: actions/upload-artifact@v3 with: @@ -481,7 +481,7 @@ jobs: path: artifacts - uses: actions/setup-python@v4 with: - python-version: "3.11" + python-version: "3.10" - name: Lint shell: bash run: | From 1122b21c7d83a1eab933f6f7fe93249f708cbb3a Mon Sep 17 00:00:00 2001 From: George Pollard Date: Fri, 15 Sep 2023 00:06:12 +0000 Subject: [PATCH 19/24] Forward reference instead of Self --- src/cli/onefuzz/templates/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cli/onefuzz/templates/__init__.py b/src/cli/onefuzz/templates/__init__.py index aed7dfdefb..a1855460b2 100644 --- a/src/cli/onefuzz/templates/__init__.py +++ b/src/cli/onefuzz/templates/__init__.py @@ -7,7 +7,7 @@ import tempfile import zipfile from datetime import timedelta -from typing import Any, Dict, List, Optional, Self, Tuple +from typing import Any, Dict, List, Optional, Tuple from uuid import uuid4 from onefuzztypes.enums import OS, ContainerType, JobState, TaskState @@ -37,11 +37,11 @@ def __init__( self.exists = exists @staticmethod - def existing(name: Container) -> Self: + def existing(name: Container) -> 'ContainerTemplate': return ContainerTemplate(name, True) @staticmethod - def fresh(name: Container, *, retention_period: Optional[timedelta] = None) -> Self: + def fresh(name: Container, *, retention_period: Optional[timedelta] = None) -> 'ContainerTemplate': return ContainerTemplate(name, False, retention_period=retention_period) From fc64e602b87191768655b5b35708175d520d9e16 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Fri, 15 Sep 2023 00:57:36 +0000 Subject: [PATCH 20/24] ugh --- src/cli/onefuzz/templates/__init__.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/cli/onefuzz/templates/__init__.py b/src/cli/onefuzz/templates/__init__.py index a1855460b2..a88db46303 100644 --- a/src/cli/onefuzz/templates/__init__.py +++ b/src/cli/onefuzz/templates/__init__.py @@ -37,11 +37,13 @@ def __init__( self.exists = exists @staticmethod - def existing(name: Container) -> 'ContainerTemplate': + def existing(name: Container) -> "ContainerTemplate": return ContainerTemplate(name, True) @staticmethod - def fresh(name: Container, *, retention_period: Optional[timedelta] = None) -> 'ContainerTemplate': + def fresh( + name: Container, *, retention_period: Optional[timedelta] = None + ) -> "ContainerTemplate": return ContainerTemplate(name, False, retention_period=retention_period) From 30a3550fb89f9b388f39be2e85b4ff4c0a6a32d0 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Sun, 17 Sep 2023 21:40:05 +0000 Subject: [PATCH 21/24] Default-enable retention policy feature flag --- src/deployment/bicep-templates/feature-flags.bicep | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/deployment/bicep-templates/feature-flags.bicep b/src/deployment/bicep-templates/feature-flags.bicep index a845d69a9d..46fccb0856 100644 --- a/src/deployment/bicep-templates/feature-flags.bicep +++ b/src/deployment/bicep-templates/feature-flags.bicep @@ -89,4 +89,17 @@ resource enableWorkItemCreation 'Microsoft.AppConfiguration/configurationStores/ } } +resource enableContainerRetentionPolicies 'Microsoft.AppConfiguration/configurationStores/keyValues@2021-10-01-preview' = { + parent: featureFlags + name: '.appconfig.featureflag~2FEnableContainerRetentionPolicies' + properties: { + value: string({ + id: 'EnableContainerRetentionPolicies' + description: 'Enable retention policies on containers' + enabled: true + }) + contentType: 'application/vnd.microsoft.appconfig.ff+json;charset=utf-8' + } +} + output AppConfigEndpoint string = 'https://${appConfigName}.azconfig.io' From 54434c00cbf09feb194767801d658c94e50daead Mon Sep 17 00:00:00 2001 From: George Pollard Date: Mon, 18 Sep 2023 03:28:21 +0000 Subject: [PATCH 22/24] Add additional log method --- src/ApiService/ApiService/Functions/QueueFileChanges.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ApiService/ApiService/Functions/QueueFileChanges.cs b/src/ApiService/ApiService/Functions/QueueFileChanges.cs index 5097f55507..c460b20ec4 100644 --- a/src/ApiService/ApiService/Functions/QueueFileChanges.cs +++ b/src/ApiService/ApiService/Functions/QueueFileChanges.cs @@ -82,7 +82,7 @@ private async Async.Task FileAdded(ResourceIdentifier storage var container = Container.Parse(parts[0]); var path = string.Join('/', parts.Skip(1)); - _log.LogInformation("file added : {Container} - {Path}", container, path); + _log.LogInformation("file added : {Container} - {Path}", container.String, path); var (_, result) = await ( ApplyRetentionPolicy(storageAccount, container, path), @@ -106,6 +106,7 @@ private async Async.Task ApplyRetentionPolicy(ResourceIdentifier storageAc var tag = RetentionPolicyUtils.CreateExpiryDateTag(DateOnly.FromDateTime(expiryDate)); if (tags.TryAdd(tag.Key, tag.Value)) { _ = await blobClient.SetTagsAsync(tags); + _log.LogInformation("applied container retention policy ({Policy}) to {Path}", retentionPeriod.Value, path); return true; } } From 17d733c0a93f2f3023c07d3e8d94099a02e6cddc Mon Sep 17 00:00:00 2001 From: George Pollard Date: Mon, 18 Sep 2023 20:59:19 +0000 Subject: [PATCH 23/24] Renaming --- src/ApiService/ApiService/Functions/QueueFileChanges.cs | 2 +- src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ApiService/ApiService/Functions/QueueFileChanges.cs b/src/ApiService/ApiService/Functions/QueueFileChanges.cs index c460b20ec4..fa4d8fcf9e 100644 --- a/src/ApiService/ApiService/Functions/QueueFileChanges.cs +++ b/src/ApiService/ApiService/Functions/QueueFileChanges.cs @@ -98,7 +98,7 @@ private async Async.Task ApplyRetentionPolicy(ResourceIdentifier storageAc var account = await _storage.GetBlobServiceClientForAccount(storageAccount); var containerClient = account.GetBlobContainerClient(container.String); var containerProps = await containerClient.GetPropertiesAsync(); - var retentionPeriod = RetentionPolicyUtils.GetRetentionPeriodFromMetadata(containerProps.Value.Metadata); + var retentionPeriod = RetentionPolicyUtils.GetContainerRetentionPeriodFromMetadata(containerProps.Value.Metadata); if (retentionPeriod.HasValue) { var blobClient = containerClient.GetBlobClient(path); var tags = (await blobClient.GetTagsAsync()).Value.Tags; diff --git a/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs b/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs index ddbdb7ae27..9b1e9df596 100644 --- a/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs +++ b/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs @@ -25,11 +25,11 @@ public static KeyValuePair CreateExpiryDateTag(DateOnly expiryDa public static string CreateExpiredBlobTagFilter() => $@"""{EXPIRY_TAG}"" <= '{DateOnly.FromDateTime(DateTime.UtcNow)}'"; // NB: this must match the value used on the CLI side - public const string RETENTION_KEY = "onefuzz_retentionperiod"; + public const string CONTAINER_RETENTION_KEY = "onefuzz_retentionperiod"; - public static TimeSpan? GetRetentionPeriodFromMetadata(IDictionary? containerMetadata) { + public static TimeSpan? GetContainerRetentionPeriodFromMetadata(IDictionary? containerMetadata) { if (containerMetadata is not null && - containerMetadata.TryGetValue(RETENTION_KEY, out var retentionString) && + containerMetadata.TryGetValue(CONTAINER_RETENTION_KEY, out var retentionString) && !string.IsNullOrWhiteSpace(retentionString)) { try { return XmlConvert.ToTimeSpan(retentionString); From 386aabb81bb96366d6665a0c7e3f9e6bd57aff4d Mon Sep 17 00:00:00 2001 From: George Pollard Date: Mon, 18 Sep 2023 21:07:15 +0000 Subject: [PATCH 24/24] Better logging --- .../ApiService/Functions/QueueFileChanges.cs | 8 +++++--- src/ApiService/ApiService/OneFuzzTypes/Enums.cs | 1 + .../ApiService/onefuzzlib/RetentionPolicy.cs | 11 +++++------ src/pytypes/onefuzztypes/enums.py | 1 + 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/ApiService/ApiService/Functions/QueueFileChanges.cs b/src/ApiService/ApiService/Functions/QueueFileChanges.cs index fa4d8fcf9e..f1c4711f9d 100644 --- a/src/ApiService/ApiService/Functions/QueueFileChanges.cs +++ b/src/ApiService/ApiService/Functions/QueueFileChanges.cs @@ -99,14 +99,16 @@ private async Async.Task ApplyRetentionPolicy(ResourceIdentifier storageAc var containerClient = account.GetBlobContainerClient(container.String); var containerProps = await containerClient.GetPropertiesAsync(); var retentionPeriod = RetentionPolicyUtils.GetContainerRetentionPeriodFromMetadata(containerProps.Value.Metadata); - if (retentionPeriod.HasValue) { + if (!retentionPeriod.IsOk) { + _log.LogError("invalid retention period: {Error}", retentionPeriod.ErrorV); + } else if (retentionPeriod.OkV is TimeSpan period) { var blobClient = containerClient.GetBlobClient(path); var tags = (await blobClient.GetTagsAsync()).Value.Tags; - var expiryDate = DateTime.UtcNow + retentionPeriod.Value; + var expiryDate = DateTime.UtcNow + period; var tag = RetentionPolicyUtils.CreateExpiryDateTag(DateOnly.FromDateTime(expiryDate)); if (tags.TryAdd(tag.Key, tag.Value)) { _ = await blobClient.SetTagsAsync(tags); - _log.LogInformation("applied container retention policy ({Policy}) to {Path}", retentionPeriod.Value, path); + _log.LogInformation("applied container retention policy ({Policy}) to {Path}", period, path); return true; } } diff --git a/src/ApiService/ApiService/OneFuzzTypes/Enums.cs b/src/ApiService/ApiService/OneFuzzTypes/Enums.cs index 4739987e6b..4692debfe8 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Enums.cs +++ b/src/ApiService/ApiService/OneFuzzTypes/Enums.cs @@ -50,6 +50,7 @@ public enum ErrorCode { ADO_WORKITEM_PROCESSING_DISABLED = 494, ADO_VALIDATION_INVALID_PATH = 495, ADO_VALIDATION_INVALID_PROJECT = 496, + INVALID_RETENTION_PERIOD = 497, // NB: if you update this enum, also update enums.py } diff --git a/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs b/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs index 9b1e9df596..48d81df5c7 100644 --- a/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs +++ b/src/ApiService/ApiService/onefuzzlib/RetentionPolicy.cs @@ -27,18 +27,17 @@ public static KeyValuePair CreateExpiryDateTag(DateOnly expiryDa // NB: this must match the value used on the CLI side public const string CONTAINER_RETENTION_KEY = "onefuzz_retentionperiod"; - public static TimeSpan? GetContainerRetentionPeriodFromMetadata(IDictionary? containerMetadata) { + public static OneFuzzResult GetContainerRetentionPeriodFromMetadata(IDictionary? containerMetadata) { if (containerMetadata is not null && containerMetadata.TryGetValue(CONTAINER_RETENTION_KEY, out var retentionString) && !string.IsNullOrWhiteSpace(retentionString)) { try { - return XmlConvert.ToTimeSpan(retentionString); - } catch { - // Log error: unable to convert xxx - return null; + return Result.Ok(XmlConvert.ToTimeSpan(retentionString)); + } catch (Exception ex) { + return Error.Create(ErrorCode.INVALID_RETENTION_PERIOD, ex.Message); } } - return null; + return Result.Ok(null); } } diff --git a/src/pytypes/onefuzztypes/enums.py b/src/pytypes/onefuzztypes/enums.py index e2ec81eb15..317325de0b 100644 --- a/src/pytypes/onefuzztypes/enums.py +++ b/src/pytypes/onefuzztypes/enums.py @@ -304,6 +304,7 @@ class ErrorCode(Enum): ADO_VALIDATION_MISSING_PAT_SCOPES = 492 ADO_VALIDATION_INVALID_PATH = 495 ADO_VALIDATION_INVALID_PROJECT = 496 + INVALID_RETENTION_PERIOD = 497 # NB: if you update this enum, also update Enums.cs