Skip to content
This repository has been archived by the owner on Nov 1, 2023. It is now read-only.

Support for retention policies on containers #3501

Merged
merged 27 commits into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
f0b6e0c
Support for retention policies on containers
Porges Sep 12, 2023
c50c5c4
Silence analysis
Porges Sep 12, 2023
6c2b4c1
"namespace" metadata
Porges Sep 14, 2023
054a19f
Define retention periods from CLI
Porges Sep 14, 2023
03e1109
Remove additional value from model
Porges Sep 14, 2023
bdab6f0
Unused import
Porges Sep 14, 2023
5fe6658
Add feature flag and perform update concurrently
Porges Sep 14, 2023
97f00c5
ContainerTemplate should be a template-only concept
Porges Sep 14, 2023
1818cb2
Use container_name more consistently
Porges Sep 14, 2023
afa426e
Reformat
Porges Sep 14, 2023
10e2dca
Typing fixes
Porges Sep 14, 2023
0ed0c95
Missing return type
Porges Sep 14, 2023
d7ba687
Update examples
Porges Sep 14, 2023
f7bade6
Bump CLI Python version
Porges Sep 14, 2023
8649e3e
Bump others as well
Porges Sep 14, 2023
8ea092f
quotes
Porges Sep 14, 2023
0844174
Actually need 3.11 for Self
Porges Sep 15, 2023
6f362fe
Revert "Actually need 3.11 for Self"
Porges Sep 15, 2023
1122b21
Forward reference instead of Self
Porges Sep 15, 2023
fc64e60
ugh
Porges Sep 15, 2023
b0744eb
Merge branch 'main' into container-retention-policy
Porges Sep 15, 2023
30a3550
Default-enable retention policy feature flag
Porges Sep 17, 2023
cb560e2
Merge branch 'main' into container-retention-policy
Porges Sep 17, 2023
54434c0
Add additional log method
Porges Sep 18, 2023
17d733c
Renaming
Porges Sep 18, 2023
386aabb
Better logging
Porges Sep 18, 2023
d45c1f3
Merge branch 'main' into container-retention-policy
Porges Sep 25, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ jobs:
- uses: actions/checkout@v3
- uses: actions/setup-python@v4
with:
python-version: 3.7
python-version: "3.10"
Porges marked this conversation as resolved.
Show resolved Hide resolved
- name: lint
shell: bash
run: src/ci/check-check-pr.sh
Expand All @@ -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
Expand Down Expand Up @@ -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: |
Expand All @@ -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: |
Expand All @@ -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:
Expand Down Expand Up @@ -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: |
Expand Down
1 change: 1 addition & 0 deletions src/ApiService/ApiService/FeatureFlags.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Porges marked this conversation as resolved.
Show resolved Hide resolved
}
39 changes: 35 additions & 4 deletions src/ApiService/ApiService/Functions/QueueFileChanges.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -54,14 +55,16 @@ public class QueueFileChanges {
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
// the message for us. The difference is for the poison queue, we're handling the
// 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));
}
Expand All @@ -71,16 +74,44 @@ public class QueueFileChanges {
}
}

private async Async.Task<OneFuzzResultVoid> FileAdded(JsonDocument fileChangeEvent, bool isLastRetryAttempt) {
private async Async.Task<OneFuzzResultVoid> 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);

var (_, result) = await (
ApplyRetentionPolicy(storageAccount, container, path),
_notificationOperations.NewFiles(container, path, isLastRetryAttempt));

return result;
}

private async Async.Task<bool> 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,12 @@ public NotificationOperations(ILogger<NotificationOperations> log, IOnefuzzConte

}
public async Async.Task<OneFuzzResultVoid> 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);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
namespace Microsoft.OneFuzz.Service;
using System.Xml;

namespace Microsoft.OneFuzz.Service;


public interface IRetentionPolicy {
Expand All @@ -21,4 +23,22 @@ public class RetentionPolicyUtils {
}

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 static TimeSpan? GetRetentionPeriodFromMetadata(IDictionary<string, string>? 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;
}
}
21 changes: 12 additions & 9 deletions src/cli/examples/domato.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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")
Expand All @@ -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),
),
]

Expand Down
19 changes: 11 additions & 8 deletions src/cli/examples/honggfuzz.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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),
),
]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
10 changes: 5 additions & 5 deletions src/cli/examples/llvm-source-coverage/source-coverage.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Loading
Loading