From 73e041a4b06c82a01f3b0df3d4da8985e8f513ea Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 11 Apr 2023 15:37:17 -0700 Subject: [PATCH 01/15] update the ado logic to consume the list of existing items once --- .../onefuzzlib/notifications/Ado.cs | 80 +++++++++---------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs b/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs index d79105e1d0..1e4ba73dd9 100644 --- a/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs +++ b/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs @@ -201,7 +201,7 @@ public async IAsyncEnumerable ExistingWorkItems() { } } - var query = "select [System.Id] from WorkItems"; + var query = "select [System.Id] from WorkItems order by [System.Id]"; if (parts != null && parts.Any()) { query += " where " + string.Join(" AND ", parts); } @@ -327,47 +327,47 @@ private async Async.Task CreateNew() { } public async Async.Task Process((string, string)[] notificationInfo) { - var matchingWorkItems = await ExistingWorkItems().ToListAsync(); - - var nonDuplicateWorkItems = matchingWorkItems - .Where(wi => !IsADODuplicateWorkItem(wi)) - .ToList(); - - if (nonDuplicateWorkItems.Count > 1) { - var nonDuplicateWorkItemIds = nonDuplicateWorkItems.Select(wi => wi.Id); - var matchingWorkItemIds = matchingWorkItems.Select(wi => wi.Id); - - var extraTags = new List<(string, string)> { - ("NonDuplicateWorkItemIds", JsonSerializer.Serialize(nonDuplicateWorkItemIds)), - ("MatchingWorkItemIds", JsonSerializer.Serialize(matchingWorkItemIds)) - }; - extraTags.AddRange(notificationInfo); - - _logTracer.WithTags(extraTags).Info($"Found more than 1 matching, non-duplicate work item"); - foreach (var workItem in nonDuplicateWorkItems) { - _ = await UpdateExisting(workItem, notificationInfo); + var matchingItems = ExistingWorkItems() + .Select(wi => new { IsDuplicate = !IsADODuplicateWorkItem(wi), wi }); + + var updated = false; + WorkItem? oldest = null; + await foreach (var workItem in matchingItems) { + oldest ??= workItem.wi; + + _logTracer.WithTags(new List<(string, string)> { ("MatchingWorkItemIds", $"{workItem.wi.Id}") }).Info($"Found matching work item"); + if (workItem.IsDuplicate) { + + continue; } - } else if (nonDuplicateWorkItems.Count == 1) { - _ = await UpdateExisting(nonDuplicateWorkItems.Single(), notificationInfo); - } else if (matchingWorkItems.Any()) { - // We have matching work items but all are duplicates - _logTracer.WithTags(notificationInfo).Info($"All matching work items were duplicates, re-opening the oldest one"); - var oldestWorkItem = matchingWorkItems.OrderBy(wi => wi.Id).First(); - var stateChanged = await UpdateExisting(oldestWorkItem, notificationInfo); - if (stateChanged) { - // add a comment if we re-opened the bug - _ = await _client.AddCommentAsync( - new CommentCreate() { - Text = "This work item was re-opened because OneFuzz could only find related work items that are marked as duplicate." - }, - _project, - (int)oldestWorkItem.Id!); + _logTracer.WithTags(new List<(string, string)> { ("NonDuplicateWorkItemId", $"{workItem.wi.Id}") }).Info($"Found matching non-duplicate work item"); + _ = await UpdateExisting(workItem.wi, notificationInfo); + updated = true; + } + + if (!updated) { + if (oldest != null) { + // We have matching work items but all are duplicates + _logTracer.WithTags(notificationInfo) + .Info($"All matching work items were duplicates, re-opening the oldest one"); + var oldestWorkItem = oldest; + var stateChanged = await UpdateExisting(oldestWorkItem, notificationInfo); + if (stateChanged) { + // add a comment if we re-opened the bug + _ = await _client.AddCommentAsync( + new CommentCreate() { + Text = + "This work item was re-opened because OneFuzz could only find related work items that are marked as duplicate." + }, + _project, + (int)oldestWorkItem.Id!); + } + } else { + // We never saw a work item like this before, it must be new + var entry = await CreateNew(); + var adoEventType = "AdoNewItem"; + _logTracer.WithTags(notificationInfo).Event($"{adoEventType} {entry.Id:Tag:WorkItemId}"); } - } else { - // We never saw a work item like this before, it must be new - var entry = await CreateNew(); - var adoEventType = "AdoNewItem"; - _logTracer.WithTags(notificationInfo).Event($"{adoEventType} {entry.Id:Tag:WorkItemId}"); } } From a8afa1835c1651be2f7863ef30a69f60bc5fcb45 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 11 Apr 2023 18:21:57 -0700 Subject: [PATCH 02/15] format --- src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs b/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs index 60833c532a..4b45770d32 100644 --- a/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs +++ b/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs @@ -338,10 +338,10 @@ public async Async.Task Process((string, string)[] notificationInfo) { WorkItem? oldest = null; await foreach (var workItem in matchingItems) { oldest ??= workItem.wi; - + _logTracer.WithTags(new List<(string, string)> { ("MatchingWorkItemIds", $"{workItem.wi.Id}") }).Info($"Found matching work item"); if (workItem.IsDuplicate) { - + continue; } _logTracer.WithTags(new List<(string, string)> { ("NonDuplicateWorkItemId", $"{workItem.wi.Id}") }).Info($"Found matching non-duplicate work item"); From 7e3708be9e143a98f813d5085d6cd1008127d80f Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 12 Apr 2023 10:19:02 -0700 Subject: [PATCH 03/15] Update src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs Co-authored-by: Teo Voinea <58236992+tevoinea@users.noreply.github.com> --- src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs b/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs index 4b45770d32..164cbe6efd 100644 --- a/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs +++ b/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs @@ -332,7 +332,7 @@ private async Async.Task CreateNew() { public async Async.Task Process((string, string)[] notificationInfo) { var matchingItems = ExistingWorkItems(notificationInfo) - .Select(wi => new { IsDuplicate = !IsADODuplicateWorkItem(wi), wi }); + .Select(wi => new { IsDuplicate = IsADODuplicateWorkItem(wi), wi }); var updated = false; WorkItem? oldest = null; From d81e48300a5f90b6f47c5cb0f5d7f576b9015f94 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 17 Apr 2023 17:44:28 -0700 Subject: [PATCH 04/15] Adding a notification testing endpoint --- .../ApiService/Functions/Notifications.cs | 2 +- .../ApiService/Functions/NotificationsTest.cs | 41 ++++++++++ .../ApiService/OneFuzzTypes/Model.cs | 4 + .../ApiService/OneFuzzTypes/Requests.cs | 6 ++ .../ApiService/OneFuzzTypes/Responses.cs | 5 ++ .../onefuzzlib/NotificationOperations.cs | 41 +++++----- .../ApiService/onefuzzlib/Reports.cs | 25 ++++-- .../onefuzzlib/notifications/Ado.cs | 33 ++++---- .../onefuzzlib/notifications/GithubIssues.cs | 7 +- .../onefuzzlib/notifications/Teams.cs | 5 +- src/cli/onefuzz/debug.py | 76 ++++++++++++++----- src/pytypes/onefuzztypes/models.py | 1 + src/pytypes/onefuzztypes/requests.py | 7 ++ src/pytypes/onefuzztypes/responses.py | 5 ++ 14 files changed, 195 insertions(+), 63 deletions(-) create mode 100644 src/ApiService/ApiService/Functions/NotificationsTest.cs diff --git a/src/ApiService/ApiService/Functions/Notifications.cs b/src/ApiService/ApiService/Functions/Notifications.cs index 99fb5810bc..db1a6037ed 100644 --- a/src/ApiService/ApiService/Functions/Notifications.cs +++ b/src/ApiService/ApiService/Functions/Notifications.cs @@ -22,7 +22,7 @@ private async Async.Task Get(HttpRequestData req) { return await _context.RequestHandling.NotOk(req, request.ErrorV, "notification search"); } - var entries = request.OkV switch { { Container: null, NotificationId: null } => _context.NotificationOperations.SearchAll(), { Container: var c, NotificationId: null } => _context.NotificationOperations.SearchByRowKeys(c.Select(x => x.String)), { Container: var _, NotificationId: var n } => new[] { await _context.NotificationOperations.GetNotification(n.Value) }.ToAsyncEnumerable(), + var entries = request.OkV switch { { Container: null, NotificationId: null } => _context.NotificationOperations.SearchAll(), { Container: var c, NotificationId: null } => _context.NotificationOperations.SearchByRowKeys(c.Select(x => x.String)), { Container: var _, NotificationId: var n } => new[] { await _context.NotificationOperations.GetNotification(n.Value) }.ToAsyncEnumerable() }; var response = req.CreateResponse(HttpStatusCode.OK); diff --git a/src/ApiService/ApiService/Functions/NotificationsTest.cs b/src/ApiService/ApiService/Functions/NotificationsTest.cs new file mode 100644 index 0000000000..16a1a5c982 --- /dev/null +++ b/src/ApiService/ApiService/Functions/NotificationsTest.cs @@ -0,0 +1,41 @@ +using System.Net; +using Microsoft.Azure.Functions.Worker; +using Microsoft.Azure.Functions.Worker.Http; + +namespace Microsoft.OneFuzz.Service.Functions; + +public class NotificationsTest { + private readonly ILogTracer _log; + private readonly IEndpointAuthorization _auth; + private readonly IOnefuzzContext _context; + + public NotificationsTest(ILogTracer log, IEndpointAuthorization auth, IOnefuzzContext context) { + _log = log; + _auth = auth; + _context = context; + } + + private async Async.Task Post(HttpRequestData req) { + _log.WithTag("HttpRequest", "GET").Info($"Notification test"); + var request = await RequestHandling.ParseRequest(req); + if (!request.IsOk) { + return await _context.RequestHandling.NotOk(req, request.ErrorV, "notification search"); + } + + var notificationTest = request.OkV; + var result = await _context.NotificationOperations.TriggerNotification(notificationTest.Notification.Container, notificationTest.Notification, + notificationTest.Report, isLastRetryAttempt: true); + var response = req.CreateResponse(HttpStatusCode.OK); + await response.WriteAsJsonAsync(new NotificationTestResponse(result.IsOk, result.ErrorV?.ToString())); + return response; + } + + + [Function("NotificationsTest")] + public Async.Task Run([HttpTrigger(AuthorizationLevel.Anonymous, "POST", Route = "notifications/test")] HttpRequestData req) { + return _auth.CallIfUser(req, r => r.Method switch { + "POST" => Post(r), + _ => throw new InvalidOperationException("Unsupported HTTP method"), + }); + } +} diff --git a/src/ApiService/ApiService/OneFuzzTypes/Model.cs b/src/ApiService/ApiService/OneFuzzTypes/Model.cs index 6580fcce08..6ebd0bea8a 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Model.cs +++ b/src/ApiService/ApiService/OneFuzzTypes/Model.cs @@ -527,6 +527,10 @@ public RegressionReport Truncate(int maxLength) { } } +public record UnknownReportType( + Uri? ReportUrl +) : IReport; + [JsonConverter(typeof(NotificationTemplateConverter))] #pragma warning disable CA1715 public interface NotificationTemplate { diff --git a/src/ApiService/ApiService/OneFuzzTypes/Requests.cs b/src/ApiService/ApiService/OneFuzzTypes/Requests.cs index 508d7a9ea2..8cb4f4473e 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Requests.cs +++ b/src/ApiService/ApiService/OneFuzzTypes/Requests.cs @@ -129,6 +129,12 @@ public record NotificationSearch( Guid? NotificationId ) : BaseRequest; + +public record NotificationTest( + Report Report, + Notification Notification +) : BaseRequest; + public record NotificationGet( [property: Required] Guid NotificationId ) : BaseRequest; diff --git a/src/ApiService/ApiService/OneFuzzTypes/Responses.cs b/src/ApiService/ApiService/OneFuzzTypes/Responses.cs index 2c7909299f..d2a7e99377 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Responses.cs +++ b/src/ApiService/ApiService/OneFuzzTypes/Responses.cs @@ -205,3 +205,8 @@ List FailedNotificationIds public record JinjaToScribanMigrationDryRunResponse( List NotificationIdsToUpdate ) : BaseResponse(); + +public record NotificationTestResponse( + bool Success, + string? Error = null +) : BaseResponse(); diff --git a/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs b/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs index 631a5545ba..191c82c80a 100644 --- a/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs @@ -10,6 +10,9 @@ public interface INotificationOperations : IOrm { IAsyncEnumerable<(Task, IEnumerable)> GetQueueTasks(); Async.Task> Create(Container container, NotificationTemplate config, bool replaceExisting); Async.Task GetNotification(Guid notifificationId); + + System.Threading.Tasks.Task TriggerNotification(Container container, + Notification notification, IReport? reportOrRegression, bool isLastRetryAttempt = false); } public class NotificationOperations : Orm, INotificationOperations { @@ -26,26 +29,11 @@ public async Async.Task NewFiles(Container container, string filename, bool isLa var done = new List(); await foreach (var notification in notifications) { if (done.Contains(notification.Config)) { - continue; + return; } done.Add(notification.Config); - - if (notification.Config is TeamsTemplate teamsTemplate) { - await _context.Teams.NotifyTeams(teamsTemplate, container, filename, reportOrRegression!, notification.NotificationId); - } - - if (reportOrRegression == null) { - continue; - } - - if (notification.Config is AdoTemplate adoTemplate) { - await _context.Ado.NotifyAdo(adoTemplate, container, filename, reportOrRegression, isLastRetryAttempt, notification.NotificationId); - } - - if (notification.Config is GithubIssuesTemplate githubIssuesTemplate) { - await _context.GithubIssues.GithubIssue(githubIssuesTemplate, container, filename, reportOrRegression, notification.NotificationId); - } + _ = await TriggerNotification(container, notification, reportOrRegression, isLastRetryAttempt); } } @@ -74,6 +62,25 @@ public async Async.Task NewFiles(Container container, string filename, bool isLa } } + public async System.Threading.Tasks.Task TriggerNotification(Container container, + Notification notification, IReport? reportOrRegression, bool isLastRetryAttempt = false) { + switch (notification.Config) { + case TeamsTemplate teamsTemplate: + await _context.Teams.NotifyTeams(teamsTemplate, container, reportOrRegression!, + notification.NotificationId); + break; + case AdoTemplate adoTemplate when reportOrRegression is not null: + return await _context.Ado.NotifyAdo(adoTemplate, container, reportOrRegression, isLastRetryAttempt, + notification.NotificationId); + case GithubIssuesTemplate githubIssuesTemplate when reportOrRegression is not null: + await _context.GithubIssues.GithubIssue(githubIssuesTemplate, container, reportOrRegression, + notification.NotificationId); + break; + } + + return OneFuzzResultVoid.Ok; + } + public IAsyncEnumerable GetNotifications(Container container) { return SearchByRowKeys(new[] { container.String }); } diff --git a/src/ApiService/ApiService/onefuzzlib/Reports.cs b/src/ApiService/ApiService/onefuzzlib/Reports.cs index dd0f7e741e..2e85d5c77d 100644 --- a/src/ApiService/ApiService/onefuzzlib/Reports.cs +++ b/src/ApiService/ApiService/onefuzzlib/Reports.cs @@ -35,7 +35,17 @@ public Reports(ILogTracer log, IContainers containers) { return null; } - var blob = await _containers.GetBlob(container, fileName, StorageType.Corpus); + var containerClient = await _containers.FindContainer(container, StorageType.Corpus); + if (containerClient == null) { + if (expectReports) { + _log.Error($"get_report invalid container: {filePath:Tag:FilePath}"); + } + return null; + } + + Uri reportUrl = containerClient.GetBlobClient(fileName).Uri; + + var blob = (await containerClient.GetBlobClient(fileName).DownloadContentAsync()).Value.Content; if (blob == null) { if (expectReports) { @@ -44,11 +54,9 @@ public Reports(ILogTracer log, IContainers containers) { return null; } - var reportUrl = await _containers.GetFileUrl(container, fileName, StorageType.Corpus); - var reportOrRegression = ParseReportOrRegression(blob.ToString(), reportUrl); - if (reportOrRegression == null && expectReports) { + if (reportOrRegression is UnknownReportType && expectReports) { _log.Error($"unable to parse report ({filePath:Tag:FilePath}) as a report or regression"); } @@ -64,7 +72,7 @@ public Reports(ILogTracer log, IContainers containers) { } } - public static IReport? ParseReportOrRegression(string content, Uri? reportUrl) { + public static IReport ParseReportOrRegression(string content, Uri reportUrl) { var regressionReport = TryDeserialize(content); if (regressionReport is { CrashTestResult: { } }) { return regressionReport with { ReportUrl = reportUrl }; @@ -73,12 +81,17 @@ public Reports(ILogTracer log, IContainers containers) { if (report is { CrashType: { } }) { return report with { ReportUrl = reportUrl }; } - return null; + return new UnknownReportType(reportUrl); } } public interface IReport { Uri? ReportUrl { init; + get; + } + public string FileName() { + var segments = (this.ReportUrl ?? throw new ArgumentException()).Segments.Skip(2); + return string.Concat(segments); } }; diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs b/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs index 164cbe6efd..02824dd13a 100644 --- a/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs +++ b/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs @@ -8,17 +8,18 @@ namespace Microsoft.OneFuzz.Service; public interface IAdo { - public Async.Task NotifyAdo(AdoTemplate config, Container container, string filename, IReport reportable, bool isLastRetryAttempt, Guid notificationId); + public Async.Task NotifyAdo(AdoTemplate config, Container container, IReport reportable, bool isLastRetryAttempt, Guid notificationId); } public class Ado : NotificationsBase, IAdo { public Ado(ILogTracer logTracer, IOnefuzzContext context) : base(logTracer, context) { } - public async Async.Task NotifyAdo(AdoTemplate config, Container container, string filename, IReport reportable, bool isLastRetryAttempt, Guid notificationId) { + public async Async.Task NotifyAdo(AdoTemplate config, Container container, IReport reportable, bool isLastRetryAttempt, Guid notificationId) { + var filename = reportable.FileName(); if (reportable is RegressionReport) { _logTracer.Info($"ado integration does not support regression report. container:{container:Tag:Container} filename:{filename:Tag:Filename}"); - return; + return OneFuzzResultVoid.Ok; } var report = (Report)reportable; @@ -44,8 +45,11 @@ public async Async.Task NotifyAdo(AdoTemplate config, Container container, strin } else { _logTracer.WithTags(notificationInfo).Exception(e, $"Failed to process ado notification"); await LogFailedNotification(report, e, notificationId); + return OneFuzzResultVoid.Error(ErrorCode.NOTIFICATION_FAILURE, + $"Failed to process ado notification : exception: {e}"); } } + return OneFuzzResultVoid.Ok; } private static bool IsTransient(Exception e) { @@ -331,30 +335,25 @@ private async Async.Task CreateNew() { } public async Async.Task Process((string, string)[] notificationInfo) { - var matchingItems = ExistingWorkItems(notificationInfo) - .Select(wi => new { IsDuplicate = IsADODuplicateWorkItem(wi), wi }); - var updated = false; - WorkItem? oldest = null; - await foreach (var workItem in matchingItems) { - oldest ??= workItem.wi; - - _logTracer.WithTags(new List<(string, string)> { ("MatchingWorkItemIds", $"{workItem.wi.Id}") }).Info($"Found matching work item"); - if (workItem.IsDuplicate) { - + WorkItem? oldestWorkItem = null; + await foreach (var workItem in ExistingWorkItems(notificationInfo)) { + // work items are ordered by id, so the oldest one is the first one + oldestWorkItem ??= workItem; + _logTracer.WithTags(new List<(string, string)> { ("MatchingWorkItemIds", $"{workItem.Id}") }).Info($"Found matching work item"); + if (IsADODuplicateWorkItem(workItem)) { continue; } - _logTracer.WithTags(new List<(string, string)> { ("NonDuplicateWorkItemId", $"{workItem.wi.Id}") }).Info($"Found matching non-duplicate work item"); - _ = await UpdateExisting(workItem.wi, notificationInfo); + _logTracer.WithTags(new List<(string, string)> { ("NonDuplicateWorkItemId", $"{workItem.Id}") }).Info($"Found matching non-duplicate work item"); + _ = await UpdateExisting(workItem, notificationInfo); updated = true; } if (!updated) { - if (oldest != null) { + if (oldestWorkItem != null) { // We have matching work items but all are duplicates _logTracer.WithTags(notificationInfo) .Info($"All matching work items were duplicates, re-opening the oldest one"); - var oldestWorkItem = oldest; var stateChanged = await UpdateExisting(oldestWorkItem, notificationInfo); if (stateChanged) { // add a comment if we re-opened the bug diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/GithubIssues.cs b/src/ApiService/ApiService/onefuzzlib/notifications/GithubIssues.cs index 88020d0144..05f778cceb 100644 --- a/src/ApiService/ApiService/onefuzzlib/notifications/GithubIssues.cs +++ b/src/ApiService/ApiService/onefuzzlib/notifications/GithubIssues.cs @@ -3,7 +3,7 @@ namespace Microsoft.OneFuzz.Service; public interface IGithubIssues { - Async.Task GithubIssue(GithubIssuesTemplate config, Container container, string filename, IReport? reportable, Guid notificationId); + Async.Task GithubIssue(GithubIssuesTemplate config, Container container, IReport reportable, Guid notificationId); } public class GithubIssues : NotificationsBase, IGithubIssues { @@ -11,8 +11,9 @@ public class GithubIssues : NotificationsBase, IGithubIssues { public GithubIssues(ILogTracer logTracer, IOnefuzzContext context) : base(logTracer, context) { } - public async Async.Task GithubIssue(GithubIssuesTemplate config, Container container, string filename, IReport? reportable, Guid notificationId) { - if (reportable == null) { + public async Async.Task GithubIssue(GithubIssuesTemplate config, Container container, IReport reportable, Guid notificationId) { + var filename = reportable.FileName(); + if (filename == null) { return; } diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/Teams.cs b/src/ApiService/ApiService/onefuzzlib/notifications/Teams.cs index 404c02f176..2d22db97fe 100644 --- a/src/ApiService/ApiService/onefuzzlib/notifications/Teams.cs +++ b/src/ApiService/ApiService/onefuzzlib/notifications/Teams.cs @@ -4,7 +4,7 @@ namespace Microsoft.OneFuzz.Service; public interface ITeams { - Async.Task NotifyTeams(TeamsTemplate config, Container container, string filename, IReport reportOrRegression, Guid notificationId); + Async.Task NotifyTeams(TeamsTemplate config, Container container, IReport reportOrRegression, Guid notificationId); } public class Teams : ITeams { @@ -53,10 +53,11 @@ private async Async.Task SendTeamsWebhook(TeamsTemplate config, string title, IL } } - public async Async.Task NotifyTeams(TeamsTemplate config, Container container, string filename, IReport reportOrRegression, Guid notificationId) { + public async Async.Task NotifyTeams(TeamsTemplate config, Container container, IReport reportOrRegression, Guid notificationId) { var facts = new List>(); string? text = null; var title = string.Empty; + var filename = reportOrRegression.FileName(); if (reportOrRegression is Report report) { var task = await _context.TaskOperations.GetByJobIdAndTaskId(report.JobId, report.TaskId); diff --git a/src/cli/onefuzz/debug.py b/src/cli/onefuzz/debug.py index aefefc3ea0..d51eb448f4 100644 --- a/src/cli/onefuzz/debug.py +++ b/src/cli/onefuzz/debug.py @@ -12,6 +12,7 @@ from typing import Any, Dict, List, Optional, Set, Tuple, Union from urllib.parse import urlparse from uuid import UUID +import uuid import jmespath from azure.applicationinsights import ApplicationInsightsDataClient @@ -19,12 +20,13 @@ from azure.identity import AzureCliCredential from azure.storage.blob import ContainerClient from onefuzztypes import models, requests +from onefuzztypes import responses from onefuzztypes.enums import ContainerType, TaskType from onefuzztypes.models import BlobRef, Job, NodeAssignment, Report, Task, TaskConfig from onefuzztypes.primitives import Container, Directory, PoolName from onefuzztypes.responses import TemplateValidationResponse -from onefuzz.api import UUID_EXPANSION, Command, Onefuzz +from onefuzz.api import UUID_EXPANSION, Command, Endpoint, Onefuzz from .azure_identity_credential_adapter import AzureIdentityCredentialAdapter from .backend import wait @@ -775,6 +777,8 @@ def task( """Inject a report into the specified crash reporting task""" task = self.onefuzz.tasks.get(task_id) + task_id = task.task_id + job_id = task.job_id crashes = self._get_container(task, ContainerType.crashes) reports = self._get_container(task, report_container_type) @@ -792,13 +796,60 @@ def task( handle.write("") self.onefuzz.containers.files.upload_file(crashes, file_path, crash_name) - report = Report( - input_blob=BlobRef( - account=self._get_storage_account(crashes), - container=crashes, - name=crash_name, + input_blob_ref = BlobRef( + account=self._get_storage_account(crashes), + container=crashes, + name=crash_name, + ) + + report = self._create_report( + job_id, task_id, task.config.task.target_exe, input_blob_ref + ) + + with tempfile.TemporaryDirectory() as tempdir: + file_path = os.path.join(tempdir, "report.json") + with open(file_path, "w") as handle: + handle.write(report.json()) + + self.onefuzz.containers.files.upload_file( + reports, file_path, crash_name + ".json" + ) + + def test_template( + self, task_id: UUID_EXPANSION, template: models.NotificationConfig + ) -> responses.NotificationTestResponse: + """Test a notification template""" + endpoint = Endpoint(self.onefuzz) + task = self.onefuzz.tasks.get(task_id) + task_id = task.task_id + job_id = task.job_id + input_blob_ref = BlobRef( + account="dummy-storage-account", + container="test-notification-crashes", + name="fake-crash-sample", + ) + + report = self._create_report(job_id, task_id, "fake_target.exe", input_blob_ref) + report.report_url = "https://fuzz7tkgjsiivmq6i.blob.core.windows.net/dummy-reports/dummy-report.json" + + return endpoint._req_model( + "POST", + responses.NotificationTestResponse, + data=requests.NotificationTest( + report=report, + notification=models.Notification( + container=models.Container("test-notification-reports"), + notification_id=uuid.uuid4(), + config=template.config, + ), ), - executable=task.config.task.target_exe, + alternate_endpoint="notifications/test", + ) + + def _create_report(self, job_id, task_id, target_exe, input_blob_ref) -> Report: + return Report( + input_blob=input_blob_ref, + executable=target_exe, crash_type="fake crash report", crash_site="fake crash site", call_stack=["#0 fake", "#1 call", "#2 stack"], @@ -806,7 +857,7 @@ def task( input_sha256=EMPTY_SHA256, asan_log="fake asan log", task_id=task_id, - job_id=task.job_id, + job_id=job_id, minimized_stack=[], minimized_stack_function_names=[], tool_name="libfuzzer", @@ -814,15 +865,6 @@ def task( onefuzz_version="1.2.3", ) - with tempfile.TemporaryDirectory() as tempdir: - file_path = os.path.join(tempdir, "report.json") - with open(file_path, "w") as handle: - handle.write(report.json()) - - self.onefuzz.containers.files.upload_file( - reports, file_path, crash_name + ".json" - ) - class Debug(Command): """Debug running jobs""" diff --git a/src/pytypes/onefuzztypes/models.py b/src/pytypes/onefuzztypes/models.py index 8f2b0e784e..57094dffff 100644 --- a/src/pytypes/onefuzztypes/models.py +++ b/src/pytypes/onefuzztypes/models.py @@ -231,6 +231,7 @@ class Report(BaseModel): minimized_stack_function_names_sha256: Optional[str] minimized_stack_function_lines: Optional[List[str]] minimized_stack_function_lines_sha256: Optional[str] + report_url: Optional[str] class NoReproReport(BaseModel): diff --git a/src/pytypes/onefuzztypes/requests.py b/src/pytypes/onefuzztypes/requests.py index 7283ecf5ca..645ea8e095 100644 --- a/src/pytypes/onefuzztypes/requests.py +++ b/src/pytypes/onefuzztypes/requests.py @@ -25,9 +25,11 @@ InstanceConfig, NotificationConfig, TemplateRenderContext, + Report, ) from .primitives import Container, PoolName, Region from .webhooks import WebhookMessageFormat +from onefuzztypes import models class BaseRequest(BaseModel): @@ -267,4 +269,9 @@ class JinjaToScribanMigrationPost(BaseModel): dry_run: bool = Field(default=False) +class NotificationTest(BaseModel): + report: Report + notification: models.Notification + + _check_hotfix() diff --git a/src/pytypes/onefuzztypes/responses.py b/src/pytypes/onefuzztypes/responses.py index fd42387dba..7a04eaa003 100644 --- a/src/pytypes/onefuzztypes/responses.py +++ b/src/pytypes/onefuzztypes/responses.py @@ -99,3 +99,8 @@ class JinjaToScribanMigrationResponse(BaseResponse): class JinjaToScribanMigrationDryRunResponse(BaseResponse): notification_ids_to_update: List[UUID] + + +class NotificationTestResponse(BaseResponse): + success: bool + error: Optional[str] From 9feb638f5aa21025ae724dd386dc54272b19ab7c Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 17 Apr 2023 20:15:50 -0700 Subject: [PATCH 05/15] fix tests --- src/ApiService/ApiService/OneFuzzTypes/Requests.cs | 4 ++-- src/ApiService/ApiService/onefuzzlib/Secrets.cs | 12 +++++------- src/ApiService/Tests/ReportTests.cs | 2 +- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/ApiService/ApiService/OneFuzzTypes/Requests.cs b/src/ApiService/ApiService/OneFuzzTypes/Requests.cs index 8cb4f4473e..a673b5f7ea 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Requests.cs +++ b/src/ApiService/ApiService/OneFuzzTypes/Requests.cs @@ -131,8 +131,8 @@ public record NotificationSearch( public record NotificationTest( - Report Report, - Notification Notification + [property: Required] Report Report, + [property: Required] Notification Notification ) : BaseRequest; public record NotificationGet( diff --git a/src/ApiService/ApiService/onefuzzlib/Secrets.cs b/src/ApiService/ApiService/onefuzzlib/Secrets.cs index ffb4857cab..1a69837529 100644 --- a/src/ApiService/ApiService/onefuzzlib/Secrets.cs +++ b/src/ApiService/ApiService/onefuzzlib/Secrets.cs @@ -56,13 +56,11 @@ public virtual async Task> SaveToKeyvault(SecretData secretD } public async Task GetSecretStringValue(SecretData data) { - - if (data.Secret is SecretAddress secretAddress) { - var secret = await GetSecret(secretAddress.Url); - return secret.Value; - } else { - return data.Secret.ToString(); - } + return (data.Secret) switch { + SecretAddress secretAddress => (await GetSecret(secretAddress.Url)).Value, + SecretValue sValue => sValue.Value?.ToString(), + _ => data.Secret.ToString(), + }; } public Uri GetKeyvaultAddress() { diff --git a/src/ApiService/Tests/ReportTests.cs b/src/ApiService/Tests/ReportTests.cs index 3ffa7b2873..9e838ff561 100644 --- a/src/ApiService/Tests/ReportTests.cs +++ b/src/ApiService/Tests/ReportTests.cs @@ -84,7 +84,7 @@ void TestParseReport() { _ = Assert.IsType(regression); var noReport = Reports.ParseReportOrRegression("{}", new Uri("http://test")); - Assert.Null(noReport); + _ = Assert.IsType(noReport); From bb5e7ba053b0ba3de16f8a691dc0dfa82d022274 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 17 Apr 2023 20:16:59 -0700 Subject: [PATCH 06/15] format --- src/pytypes/onefuzztypes/requests.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/pytypes/onefuzztypes/requests.py b/src/pytypes/onefuzztypes/requests.py index 645ea8e095..5d7547f955 100644 --- a/src/pytypes/onefuzztypes/requests.py +++ b/src/pytypes/onefuzztypes/requests.py @@ -8,6 +8,8 @@ from pydantic import AnyHttpUrl, BaseModel, Field, root_validator +from onefuzztypes import models + from ._monkeypatch import _check_hotfix from .consts import ONE_HOUR, SEVEN_DAYS from .enums import ( @@ -24,12 +26,11 @@ AutoScaleConfig, InstanceConfig, NotificationConfig, - TemplateRenderContext, Report, + TemplateRenderContext, ) from .primitives import Container, PoolName, Region from .webhooks import WebhookMessageFormat -from onefuzztypes import models class BaseRequest(BaseModel): From 7ec83ffeb9dcf0966b8729fbc1eba6218d991a37 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 17 Apr 2023 20:20:48 -0700 Subject: [PATCH 07/15] regen docs --- docs/webhook_events.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/webhook_events.md b/docs/webhook_events.md index 8b011a928a..020846aae7 100644 --- a/docs/webhook_events.md +++ b/docs/webhook_events.md @@ -238,6 +238,10 @@ If webhook is set to have Event Grid message format then the payload will look a "title": "Onefuzz Version", "type": "string" }, + "report_url": { + "title": "Report Url", + "type": "string" + }, "scariness_description": { "title": "Scariness Description", "type": "string" @@ -2158,6 +2162,10 @@ If webhook is set to have Event Grid message format then the payload will look a "title": "Onefuzz Version", "type": "string" }, + "report_url": { + "title": "Report Url", + "type": "string" + }, "scariness_description": { "title": "Scariness Description", "type": "string" @@ -6578,6 +6586,10 @@ If webhook is set to have Event Grid message format then the payload will look a "title": "Onefuzz Version", "type": "string" }, + "report_url": { + "title": "Report Url", + "type": "string" + }, "scariness_description": { "title": "Scariness Description", "type": "string" From e228034762a6f53a1cdc20862cff11c2c301cf0c Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 17 Apr 2023 20:25:35 -0700 Subject: [PATCH 08/15] update logic --- src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs b/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs index 191c82c80a..77db07622d 100644 --- a/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs @@ -29,7 +29,7 @@ public async Async.Task NewFiles(Container container, string filename, bool isLa var done = new List(); await foreach (var notification in notifications) { if (done.Contains(notification.Config)) { - return; + continue; } done.Add(notification.Config); From 2e9849da238e0527374d8bceee50c27211c13619 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 17 Apr 2023 20:48:16 -0700 Subject: [PATCH 09/15] format --- src/cli/onefuzz/debug.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cli/onefuzz/debug.py b/src/cli/onefuzz/debug.py index d51eb448f4..9e9966535e 100644 --- a/src/cli/onefuzz/debug.py +++ b/src/cli/onefuzz/debug.py @@ -8,19 +8,18 @@ import os import tempfile import time +import uuid from datetime import datetime from typing import Any, Dict, List, Optional, Set, Tuple, Union from urllib.parse import urlparse from uuid import UUID -import uuid import jmespath from azure.applicationinsights import ApplicationInsightsDataClient from azure.applicationinsights.models import QueryBody from azure.identity import AzureCliCredential from azure.storage.blob import ContainerClient -from onefuzztypes import models, requests -from onefuzztypes import responses +from onefuzztypes import models, requests, responses from onefuzztypes.enums import ContainerType, TaskType from onefuzztypes.models import BlobRef, Job, NodeAssignment, Report, Task, TaskConfig from onefuzztypes.primitives import Container, Directory, PoolName From 64a8c23693ed7e40566d43ee6680798207c77553 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 17 Apr 2023 20:45:53 -0700 Subject: [PATCH 10/15] fix dummy name --- src/cli/onefuzz/debug.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cli/onefuzz/debug.py b/src/cli/onefuzz/debug.py index 9e9966535e..ef961285fe 100644 --- a/src/cli/onefuzz/debug.py +++ b/src/cli/onefuzz/debug.py @@ -829,7 +829,7 @@ def test_template( ) report = self._create_report(job_id, task_id, "fake_target.exe", input_blob_ref) - report.report_url = "https://fuzz7tkgjsiivmq6i.blob.core.windows.net/dummy-reports/dummy-report.json" + report.report_url = "https://dummy-container.blob.core.windows.net/dummy-reports/dummy-report.json" return endpoint._req_model( "POST", From a8558d4a32f652ce32667fb166f8c7a62f119c6e Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 17 Apr 2023 21:07:04 -0700 Subject: [PATCH 11/15] mypy fix --- src/cli/onefuzz/debug.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/cli/onefuzz/debug.py b/src/cli/onefuzz/debug.py index ef961285fe..d058349e20 100644 --- a/src/cli/onefuzz/debug.py +++ b/src/cli/onefuzz/debug.py @@ -776,8 +776,7 @@ def task( """Inject a report into the specified crash reporting task""" task = self.onefuzz.tasks.get(task_id) - task_id = task.task_id - job_id = task.job_id + crashes = self._get_container(task, ContainerType.crashes) reports = self._get_container(task, report_container_type) @@ -802,7 +801,7 @@ def task( ) report = self._create_report( - job_id, task_id, task.config.task.target_exe, input_blob_ref + task.job_id, task.task_id, task.config.task.target_exe, input_blob_ref ) with tempfile.TemporaryDirectory() as tempdir: @@ -820,15 +819,15 @@ def test_template( """Test a notification template""" endpoint = Endpoint(self.onefuzz) task = self.onefuzz.tasks.get(task_id) - task_id = task.task_id - job_id = task.job_id input_blob_ref = BlobRef( account="dummy-storage-account", container="test-notification-crashes", name="fake-crash-sample", ) - report = self._create_report(job_id, task_id, "fake_target.exe", input_blob_ref) + report = self._create_report( + task.job_id, task.task_id, "fake_target.exe", input_blob_ref + ) report.report_url = "https://dummy-container.blob.core.windows.net/dummy-reports/dummy-report.json" return endpoint._req_model( @@ -837,7 +836,7 @@ def test_template( data=requests.NotificationTest( report=report, notification=models.Notification( - container=models.Container("test-notification-reports"), + container=Container("test-notification-reports"), notification_id=uuid.uuid4(), config=template.config, ), @@ -845,7 +844,9 @@ def test_template( alternate_endpoint="notifications/test", ) - def _create_report(self, job_id, task_id, target_exe, input_blob_ref) -> Report: + def _create_report( + self, job_id: UUID, task_id: UUID, target_exe: str, input_blob_ref: BlobRef + ) -> Report: return Report( input_blob=input_blob_ref, executable=target_exe, From 59b041f3fda7a1578abdb79f3ebeee7f8a240f82 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Mon, 17 Apr 2023 21:15:29 -0700 Subject: [PATCH 12/15] make mypy happy --- src/cli/onefuzz/debug.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/cli/onefuzz/debug.py b/src/cli/onefuzz/debug.py index d058349e20..7cf181c23a 100644 --- a/src/cli/onefuzz/debug.py +++ b/src/cli/onefuzz/debug.py @@ -800,6 +800,7 @@ def task( name=crash_name, ) + assert task.config.task.target_exe is not None report = self._create_report( task.job_id, task.task_id, task.config.task.target_exe, input_blob_ref ) From 25ddb73947fb743750a66f35fcc72ccfc54825be Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 18 Apr 2023 09:09:17 -0700 Subject: [PATCH 13/15] bandit fix --- src/cli/onefuzz/debug.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cli/onefuzz/debug.py b/src/cli/onefuzz/debug.py index 7cf181c23a..455750edcf 100644 --- a/src/cli/onefuzz/debug.py +++ b/src/cli/onefuzz/debug.py @@ -800,9 +800,9 @@ def task( name=crash_name, ) - assert task.config.task.target_exe is not None + target_exe = task.config.task.target_exe if task.config.task.target_exe else "" report = self._create_report( - task.job_id, task.task_id, task.config.task.target_exe, input_blob_ref + task.job_id, task.task_id, target_exe, input_blob_ref ) with tempfile.TemporaryDirectory() as tempdir: From 75f5ccf14b2335e757066f5b89382a7fd3dce6b0 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Tue, 18 Apr 2023 09:18:22 -0700 Subject: [PATCH 14/15] renaming --- src/cli/onefuzz/debug.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cli/onefuzz/debug.py b/src/cli/onefuzz/debug.py index 455750edcf..4562d2d2e8 100644 --- a/src/cli/onefuzz/debug.py +++ b/src/cli/onefuzz/debug.py @@ -815,7 +815,7 @@ def task( ) def test_template( - self, task_id: UUID_EXPANSION, template: models.NotificationConfig + self, task_id: UUID_EXPANSION, notificationConfig: models.NotificationConfig ) -> responses.NotificationTestResponse: """Test a notification template""" endpoint = Endpoint(self.onefuzz) @@ -839,7 +839,7 @@ def test_template( notification=models.Notification( container=Container("test-notification-reports"), notification_id=uuid.uuid4(), - config=template.config, + config=notificationConfig.config, ), ), alternate_endpoint="notifications/test", From 8035095bae74728965ebd3631c121d0a9e5c5627 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Wed, 19 Apr 2023 10:11:49 -0700 Subject: [PATCH 15/15] address PR Comment --- .../ApiService/onefuzzlib/notifications/GithubIssues.cs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/GithubIssues.cs b/src/ApiService/ApiService/onefuzzlib/notifications/GithubIssues.cs index 05f778cceb..05d8333117 100644 --- a/src/ApiService/ApiService/onefuzzlib/notifications/GithubIssues.cs +++ b/src/ApiService/ApiService/onefuzzlib/notifications/GithubIssues.cs @@ -13,9 +13,6 @@ public GithubIssues(ILogTracer logTracer, IOnefuzzContext context) public async Async.Task GithubIssue(GithubIssuesTemplate config, Container container, IReport reportable, Guid notificationId) { var filename = reportable.FileName(); - if (filename == null) { - return; - } if (reportable is RegressionReport) { _logTracer.Info($"github issue integration does not support regression reports. {container:Tag:Container} - {filename:Tag:Filename}");