From d23756398891d5dd8fbf0151661a79e6451ac968 Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Thu, 4 May 2023 12:14:23 -0700 Subject: [PATCH 1/3] Collect additional report field in an ExtensionData property (#3079) * Collect additional report field in an ExtensionData property * typo fix --- src/ApiService/ApiService/OneFuzzTypes/Model.cs | 2 ++ src/ApiService/Tests/ReportTests.cs | 10 ++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/ApiService/ApiService/OneFuzzTypes/Model.cs b/src/ApiService/ApiService/OneFuzzTypes/Model.cs index 6ebd0bea8a..9b0594a4e1 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Model.cs +++ b/src/ApiService/ApiService/OneFuzzTypes/Model.cs @@ -461,6 +461,8 @@ public record Report( string? OnefuzzVersion, Uri? ReportUrl ) : IReport, ITruncatable { + + [JsonExtensionData] public Dictionary? ExtensionData { get; set; } public Report Truncate(int maxLength) { return this with { Executable = Executable[..maxLength], diff --git a/src/ApiService/Tests/ReportTests.cs b/src/ApiService/Tests/ReportTests.cs index 9e838ff561..aaa453318e 100644 --- a/src/ApiService/Tests/ReportTests.cs +++ b/src/ApiService/Tests/ReportTests.cs @@ -36,7 +36,9 @@ void TestParseReport() { }, "tool_name": "libfuzzer", "tool_version": "1.2.3", - "onefuzz_version": "1.2.3" + "onefuzz_version": "1.2.3", + "extra_property1": "test", + "extra_property2": 5 } """; @@ -78,7 +80,11 @@ void TestParseReport() { """; var report = Reports.ParseReportOrRegression(testReport, new Uri("http://test")); - _ = Assert.IsType(report); + var reportInstance = Assert.IsType(report); + + Assert.Equal("test", reportInstance?.ExtensionData?["extra_property1"].GetString()); + Assert.Equal(5, reportInstance?.ExtensionData?["extra_property2"].GetInt32()); + var regression = Reports.ParseReportOrRegression(testRegresion, new Uri("http://test")); _ = Assert.IsType(regression); From bb1a54470ae2c83a3efd3251ff4f03991650955d Mon Sep 17 00:00:00 2001 From: Cheick Keita Date: Thu, 4 May 2023 12:50:46 -0700 Subject: [PATCH 2/3] Fix logic of marking task as failed (#3083) * Fix logic of markaing task as failed - Do not mark task as failed if it is already in the shutting down state - accumulate errors when setting task error to understand the context - refactor the Error record * fix tests * format * Fix build * Update src/ApiService/ApiService/onefuzzlib/ImageReference.cs Co-authored-by: George Pollard * Update src/ApiService/ApiService/onefuzzlib/ProxyOperations.cs Co-authored-by: Teo Voinea <58236992+tevoinea@users.noreply.github.com> --------- Co-authored-by: George Pollard Co-authored-by: Teo Voinea <58236992+tevoinea@users.noreply.github.com> --- .../ApiService/Functions/AgentCanSchedule.cs | 6 +-- .../ApiService/Functions/AgentEvents.cs | 49 ++++++++----------- .../ApiService/Functions/AgentRegistration.cs | 33 ++++++------- .../ApiService/Functions/Containers.cs | 12 ++--- .../ApiService/Functions/Download.cs | 8 +-- src/ApiService/ApiService/Functions/Jobs.cs | 20 +++----- .../Functions/Migrations/JinjaToScriban.cs | 2 +- src/ApiService/ApiService/Functions/Node.cs | 16 ++---- .../ApiService/Functions/NodeAddSshKey.cs | 2 +- .../ApiService/Functions/Notifications.cs | 6 +-- src/ApiService/ApiService/Functions/Pool.cs | 12 ++--- src/ApiService/ApiService/Functions/Proxy.cs | 4 +- .../ApiService/Functions/ReproVmss.cs | 6 +-- .../ApiService/Functions/Scaleset.cs | 24 ++++----- src/ApiService/ApiService/Functions/Tasks.cs | 16 +++--- .../ApiService/Functions/WebhookLogs.cs | 2 +- .../ApiService/Functions/WebhookPing.cs | 2 +- .../ApiService/Functions/Webhooks.cs | 6 +-- .../ApiService/OneFuzzTypes/Model.cs | 5 +- .../ApiService/OneFuzzTypes/ReturnTypes.cs | 4 +- .../TestHooks/InstanceConfigTestHooks.cs | 2 +- .../TestHooks/NodeOperationsTestHooks.cs | 2 +- .../onefuzzlib/EndpointAuthorization.cs | 30 ++++-------- .../ApiService/onefuzzlib/ImageReference.cs | 29 ++++++----- .../ApiService/onefuzzlib/JobOperations.cs | 2 +- .../ApiService/onefuzzlib/NodeOperations.cs | 27 +++++----- .../ApiService/onefuzzlib/NsgOperations.cs | 9 ++-- .../ApiService/onefuzzlib/ProxyOperations.cs | 6 +-- .../ApiService/onefuzzlib/ReproOperations.cs | 6 +-- .../ApiService/onefuzzlib/Request.cs | 17 +++---- .../onefuzzlib/ScalesetOperations.cs | 6 +-- .../ApiService/onefuzzlib/TaskOperations.cs | 6 +-- .../ApiService/onefuzzlib/VmssOperations.cs | 2 +- .../notifications/JinjaTemplateAdapter.cs | 2 +- .../notifications/NotificationsBase.cs | 2 +- .../Fakes/TestEndpointAuthorization.cs | 4 +- 36 files changed, 171 insertions(+), 216 deletions(-) diff --git a/src/ApiService/ApiService/Functions/AgentCanSchedule.cs b/src/ApiService/ApiService/Functions/AgentCanSchedule.cs index 8d74f2dec2..847c755970 100644 --- a/src/ApiService/ApiService/Functions/AgentCanSchedule.cs +++ b/src/ApiService/ApiService/Functions/AgentCanSchedule.cs @@ -35,11 +35,7 @@ private async Async.Task Post(HttpRequestData req) { _log.Warning($"Unable to find {canScheduleRequest.MachineId:Tag:MachineId}"); return await _context.RequestHandling.NotOk( req, - new Error( - ErrorCode.UNABLE_TO_FIND, - new string[] { - "unable to find node" - }), + Error.Create(ErrorCode.UNABLE_TO_FIND, "unable to find node"), canScheduleRequest.MachineId.ToString()); } diff --git a/src/ApiService/ApiService/Functions/AgentEvents.cs b/src/ApiService/ApiService/Functions/AgentEvents.cs index 5f68b8ce59..73319d06ba 100644 --- a/src/ApiService/ApiService/Functions/AgentEvents.cs +++ b/src/ApiService/ApiService/Functions/AgentEvents.cs @@ -35,7 +35,7 @@ private async Async.Task Post(HttpRequestData req) { NodeStateUpdate updateEvent => await OnStateUpdate(envelope.MachineId, updateEvent), WorkerEvent workerEvent => await OnWorkerEvent(envelope.MachineId, workerEvent), NodeEvent nodeEvent => await OnNodeEvent(envelope.MachineId, nodeEvent), - _ => new Error(ErrorCode.INVALID_REQUEST, new string[] { $"invalid node event: {envelope.Event.GetType().Name}" }), + _ => Error.Create(ErrorCode.INVALID_REQUEST, $"invalid node event: {envelope.Event.GetType().Name}"), }; if (error is Error e) { @@ -114,17 +114,17 @@ private async Async.Task Post(HttpRequestData req) { } else if (ev.State == NodeState.SettingUp) { if (ev.Data is NodeSettingUpEventData settingUpData) { if (!settingUpData.Tasks.Any()) { - return new Error(ErrorCode.INVALID_REQUEST, Errors: new string[] { - $"setup without tasks. machine_id: {machineId}", - }); + return Error.Create(ErrorCode.INVALID_REQUEST, + $"setup without tasks. machine_id: {machineId}" + ); } foreach (var taskId in settingUpData.Tasks) { var task = await _context.TaskOperations.GetByTaskId(taskId); if (task is null) { - return new Error( + return Error.Create( ErrorCode.INVALID_REQUEST, - Errors: new string[] { $"unable to find task: {taskId}" }); + $"unable to find task: {taskId}"); } _log.Info($"node starting task. {machineId:Tag:MachineId} {task.JobId:Tag:JobId} {task.TaskId:Tag:TaskId}"); @@ -154,7 +154,7 @@ private async Async.Task Post(HttpRequestData req) { if (ev.Data is NodeDoneEventData doneData) { if (doneData.Error is not null) { var errorText = EntityConverter.ToJsonString(doneData); - error = new Error(ErrorCode.TASK_FAILED, Errors: new string[] { errorText }); + error = Error.Create(ErrorCode.TASK_FAILED, errorText); _log.Error($"node 'done' {machineId:Tag:MachineId} - {errorText:Tag:Error}"); } } @@ -178,9 +178,9 @@ private async Async.Task Post(HttpRequestData req) { return await OnWorkerEventRunning(machineId, ev.Running); } - return new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: new string[] { "WorkerEvent should have either 'done' or 'running' set" }); + return Error.Create( + ErrorCode.INVALID_REQUEST, + "WorkerEvent should have either 'done' or 'running' set"); } private async Async.Task OnWorkerEventRunning(Guid machineId, WorkerRunningEvent running) { @@ -189,15 +189,11 @@ private async Async.Task Post(HttpRequestData req) { _context.NodeOperations.GetByMachineId(machineId)); if (task is null) { - return new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: new string[] { $"unable to find task: {running.TaskId}" }); + return Error.Create(ErrorCode.INVALID_REQUEST, $"unable to find task: {running.TaskId}"); } if (node is null) { - return new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: new string[] { $"unable to find node: {machineId}" }); + return Error.Create(ErrorCode.INVALID_REQUEST, $"unable to find node: {machineId}"); } if (!node.State.ReadyForReset()) { @@ -240,15 +236,11 @@ private async Async.Task Post(HttpRequestData req) { _context.NodeOperations.GetByMachineId(machineId)); if (task is null) { - return new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: new string[] { $"unable to find task: {done.TaskId}" }); + return Error.Create(ErrorCode.INVALID_REQUEST, $"unable to find task: {done.TaskId}"); } if (node is null) { - return new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: new string[] { $"unable to find node: {machineId}" }); + return Error.Create(ErrorCode.INVALID_REQUEST, $"unable to find node: {machineId}"); } // trim stdout/stderr if too long @@ -272,13 +264,12 @@ private async Async.Task Post(HttpRequestData req) { } else { await _context.TaskOperations.MarkFailed( task, - new Error( - Code: ErrorCode.TASK_FAILED, - Errors: new string[] { - $"task failed. exit_status:{done.ExitStatus}", - done.Stdout, - done.Stderr, - })); + Error.Create( + ErrorCode.TASK_FAILED, + $"task failed. exit_status:{done.ExitStatus}", + done.Stdout, + done.Stderr + )); // keep node if any keep options are set if ((task.Config.Debug?.Contains(TaskDebugFlag.KeepNodeOnFailure) == true) diff --git a/src/ApiService/ApiService/Functions/AgentRegistration.cs b/src/ApiService/ApiService/Functions/AgentRegistration.cs index ce380c7c78..6ad8cc69d8 100644 --- a/src/ApiService/ApiService/Functions/AgentRegistration.cs +++ b/src/ApiService/ApiService/Functions/AgentRegistration.cs @@ -40,9 +40,9 @@ private async Async.Task Get(HttpRequestData req) { if (machineId == Guid.Empty) { return await _context.RequestHandling.NotOk( req, - new Error( + Error.Create( ErrorCode.INVALID_REQUEST, - new string[] { "'machine_id' query parameter must be provided" }), + "'machine_id' query parameter must be provided"), "agent registration"); } @@ -50,9 +50,9 @@ private async Async.Task Get(HttpRequestData req) { if (agentNode is null) { return await _context.RequestHandling.NotOk( req, - new Error( + Error.Create( ErrorCode.INVALID_REQUEST, - new string[] { $"unable to find a registration associated with machine_id '{machineId}'" }), + $"unable to find a registration associated with machine_id '{machineId}'"), "agent registration"); } @@ -60,9 +60,9 @@ private async Async.Task Get(HttpRequestData req) { if (!pool.IsOk) { return await _context.RequestHandling.NotOk( req, - new Error( + Error.Create( ErrorCode.INVALID_REQUEST, - new string[] { "unable to find a pool associated with the provided machine_id" }), + "unable to find a pool associated with the provided machine_id"), "agent registration"); } @@ -101,18 +101,16 @@ private async Async.Task Post(HttpRequestData req) { if (machineId == Guid.Empty) { return await _context.RequestHandling.NotOk( req, - new Error( - ErrorCode.INVALID_REQUEST, - new string[] { "'machine_id' query parameter must be provided" }), + Error.Create( + ErrorCode.INVALID_REQUEST, "'machine_id' query parameter must be provided"), "agent registration"); } if (poolName is null) { return await _context.RequestHandling.NotOk( req, - new Error( - ErrorCode.INVALID_REQUEST, - new string[] { "'pool_name' query parameter must be provided" }), + Error.Create( + ErrorCode.INVALID_REQUEST, "'pool_name' query parameter must be provided"), "agent registration"); } @@ -124,9 +122,8 @@ private async Async.Task Post(HttpRequestData req) { if (!poolResult.IsOk) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: new[] { $"unable to find pool '{poolName}'" }), + Error.Create( + ErrorCode.INVALID_REQUEST, $"unable to find pool '{poolName}'"), "agent registration"); } @@ -140,9 +137,9 @@ private async Async.Task Post(HttpRequestData req) { if (os != null && pool.Os != os) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: new[] { $"OS mismatch: pool '{poolName}' is configured for '{pool.Os}', but agent is running '{os}'" }), + Error.Create( + ErrorCode.INVALID_REQUEST, + $"OS mismatch: pool '{poolName}' is configured for '{pool.Os}', but agent is running '{os}'"), "agent registration"); } diff --git a/src/ApiService/ApiService/Functions/Containers.cs b/src/ApiService/ApiService/Functions/Containers.cs index a7218e3b62..c3bedcfd66 100644 --- a/src/ApiService/ApiService/Functions/Containers.cs +++ b/src/ApiService/ApiService/Functions/Containers.cs @@ -39,9 +39,9 @@ private async Async.Task Get(HttpRequestData req) { if (container is null) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: new[] { "invalid container" }), + Error.Create( + ErrorCode.INVALID_REQUEST, + "invalid container"), context: get.Name.String); } @@ -101,9 +101,9 @@ private async Async.Task Post(HttpRequestData req) { if (sas is null) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: new[] { "invalid container" }), + Error.Create( + ErrorCode.INVALID_REQUEST, + "invalid container"), context: post.Name.String); } diff --git a/src/ApiService/ApiService/Functions/Download.cs b/src/ApiService/ApiService/Functions/Download.cs index 65b3fbcf7a..dac128b08b 100644 --- a/src/ApiService/ApiService/Functions/Download.cs +++ b/src/ApiService/ApiService/Functions/Download.cs @@ -25,9 +25,9 @@ private async Async.Task Get(HttpRequestData req) { if (queryContainer is null || !Container.TryParse(queryContainer, out var container)) { return await _context.RequestHandling.NotOk( req, - new Error( + Error.Create( ErrorCode.INVALID_REQUEST, - new string[] { "'container' query parameter must be provided and valid" }), + "'container' query parameter must be provided and valid"), "download"); } @@ -35,9 +35,9 @@ private async Async.Task Get(HttpRequestData req) { if (filename is null) { return await _context.RequestHandling.NotOk( req, - new Error( + Error.Create( ErrorCode.INVALID_REQUEST, - new string[] { "'filename' query parameter must be provided" }), + "'filename' query parameter must be provided"), "download"); } diff --git a/src/ApiService/ApiService/Functions/Jobs.cs b/src/ApiService/ApiService/Functions/Jobs.cs index 24579addcb..c7ee52bb5f 100644 --- a/src/ApiService/ApiService/Functions/Jobs.cs +++ b/src/ApiService/ApiService/Functions/Jobs.cs @@ -59,9 +59,7 @@ private async Task Post(HttpRequestData req) { if (containerSas is null) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.UNABLE_TO_CREATE_CONTAINER, - Errors: new string[] { "unable to create logs container " }), + Error.Create(ErrorCode.UNABLE_TO_CREATE_CONTAINER, "unable to create logs container "), "logs"); } @@ -73,9 +71,9 @@ private async Task Post(HttpRequestData req) { _logTracer.WithTag("HttpRequest", "POST").WithHttpStatus(r.ErrorV).Error($"failed to insert job {job.JobId:Tag:JobId}"); return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.UNABLE_TO_CREATE, - Errors: new string[] { "unable to create job " } + Error.Create( + ErrorCode.UNABLE_TO_CREATE, + "unable to create job" ), "job"); } @@ -95,9 +93,9 @@ private async Task Delete(HttpRequestData req) { if (job is null) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.INVALID_JOB, - Errors: new string[] { "no such job" }), + Error.Create( + ErrorCode.INVALID_JOB, + "no such job"), context: jobId.ToString()); } @@ -124,9 +122,7 @@ private async Task Get(HttpRequestData req) { if (job is null) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.INVALID_JOB, - Errors: new string[] { "no such job" }), + Error.Create(ErrorCode.INVALID_JOB, "no such job"), context: jobId.ToString()); } diff --git a/src/ApiService/ApiService/Functions/Migrations/JinjaToScriban.cs b/src/ApiService/ApiService/Functions/Migrations/JinjaToScriban.cs index 238976cab5..197c90ecae 100644 --- a/src/ApiService/ApiService/Functions/Migrations/JinjaToScriban.cs +++ b/src/ApiService/ApiService/Functions/Migrations/JinjaToScriban.cs @@ -72,7 +72,7 @@ await notifications.Select(notification => notification.NotificationId).ToListAs _log.Info($"Migrated notification: {notification.NotificationId} to jinja"); } else { failedNotificationIds.Add(notification.NotificationId); - _log.Error(new Error(ErrorCode.UNABLE_TO_UPDATE, new[] { r.ErrorV.Reason, r.ErrorV.Status.ToString() })); + _log.Error(Error.Create(ErrorCode.UNABLE_TO_UPDATE, r.ErrorV.Reason, r.ErrorV.Status.ToString())); } } catch (Exception ex) { failedNotificationIds.Add(notification.NotificationId); diff --git a/src/ApiService/ApiService/Functions/Node.cs b/src/ApiService/ApiService/Functions/Node.cs index 2bdb93d567..70231b90d2 100644 --- a/src/ApiService/ApiService/Functions/Node.cs +++ b/src/ApiService/ApiService/Functions/Node.cs @@ -38,9 +38,7 @@ private async Async.Task Get(HttpRequestData req) { if (node is null) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.UNABLE_TO_FIND, - Errors: new string[] { "unable to find node" }), + Error.Create(ErrorCode.UNABLE_TO_FIND, "unable to find node"), context: machineId.ToString()); } @@ -94,9 +92,7 @@ private async Async.Task Patch(HttpRequestData req) { if (node is null) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.UNABLE_TO_FIND, - Errors: new string[] { "unable to find node" }), + Error.Create(ErrorCode.UNABLE_TO_FIND, "unable to find node"), context: patch.MachineId.ToString()); } @@ -130,9 +126,7 @@ private async Async.Task Post(HttpRequestData req) { if (node is null) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.UNABLE_TO_FIND, - Errors: new string[] { "unable to find node" }), + Error.Create(ErrorCode.UNABLE_TO_FIND, "unable to find node"), context: post.MachineId.ToString()); } @@ -166,9 +160,7 @@ private async Async.Task Delete(HttpRequestData req) { if (node is null) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.UNABLE_TO_FIND, - new string[] { "unable to find node" }), + Error.Create(ErrorCode.UNABLE_TO_FIND, "unable to find node"), context: delete.MachineId.ToString()); } diff --git a/src/ApiService/ApiService/Functions/NodeAddSshKey.cs b/src/ApiService/ApiService/Functions/NodeAddSshKey.cs index 5a5fe058a1..1bce7c9fb2 100644 --- a/src/ApiService/ApiService/Functions/NodeAddSshKey.cs +++ b/src/ApiService/ApiService/Functions/NodeAddSshKey.cs @@ -30,7 +30,7 @@ private async Async.Task Post(HttpRequestData req) { if (node == null) { return await _context.RequestHandling.NotOk( req, - new Error(ErrorCode.UNABLE_TO_FIND, new[] { "unable to find node" }), + Error.Create(ErrorCode.UNABLE_TO_FIND, "unable to find node"), $"{request.OkV.MachineId}"); } diff --git a/src/ApiService/ApiService/Functions/Notifications.cs b/src/ApiService/ApiService/Functions/Notifications.cs index db1a6037ed..5013521a26 100644 --- a/src/ApiService/ApiService/Functions/Notifications.cs +++ b/src/ApiService/ApiService/Functions/Notifications.cs @@ -61,18 +61,18 @@ private async Async.Task Delete(HttpRequestData req) { var entries = await _context.NotificationOperations.SearchByPartitionKeys(new[] { $"{request.OkV.NotificationId}" }).ToListAsync(); if (entries.Count == 0) { - return await _context.RequestHandling.NotOk(req, new Error(ErrorCode.INVALID_REQUEST, new[] { "unable to find notification" }), context: "notification delete"); + return await _context.RequestHandling.NotOk(req, Error.Create(ErrorCode.INVALID_REQUEST, "unable to find notification"), context: "notification delete"); } if (entries.Count > 1) { - return await _context.RequestHandling.NotOk(req, new Error(ErrorCode.INVALID_REQUEST, new[] { "error identifying Notification" }), context: "notification delete"); + return await _context.RequestHandling.NotOk(req, Error.Create(ErrorCode.INVALID_REQUEST, "error identifying Notification"), context: "notification delete"); } var result = await _context.NotificationOperations.Delete(entries[0]); if (!result.IsOk) { var (status, error) = result.ErrorV; - return await _context.RequestHandling.NotOk(req, new Error(ErrorCode.UNABLE_TO_UPDATE, new[] { error }), "notification delete"); + return await _context.RequestHandling.NotOk(req, Error.Create(ErrorCode.UNABLE_TO_UPDATE, error), "notification delete"); } var response = req.CreateResponse(HttpStatusCode.OK); diff --git a/src/ApiService/ApiService/Functions/Pool.cs b/src/ApiService/ApiService/Functions/Pool.cs index d6d8170561..9d4ee7f5df 100644 --- a/src/ApiService/ApiService/Functions/Pool.cs +++ b/src/ApiService/ApiService/Functions/Pool.cs @@ -63,9 +63,7 @@ private async Task Post(HttpRequestData req) { if (pool.IsOk) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: new string[] { "pool with that name already exists" }), + Error.Create(ErrorCode.INVALID_REQUEST, "pool with that name already exists"), "PoolCreate"); } var newPool = await _context.PoolOperations.Create(name: create.Name, os: create.Os, architecture: create.Arch, managed: create.Managed, objectId: create.ObjectId); @@ -89,9 +87,7 @@ private async Task Patch(HttpRequestData req) { if (!pool.IsOk) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: new string[] { "pool with that name does not exist" }), + Error.Create(ErrorCode.INVALID_REQUEST, "pool with that name does not exist"), "PoolUpdate"); } @@ -100,9 +96,7 @@ private async Task Patch(HttpRequestData req) { if (updatePool.IsOk) { return await RequestHandling.Ok(req, await Populate(PoolToPoolResponse(updated), true)); } else { - return await _context.RequestHandling.NotOk(req, new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: new string[] { updatePool.ErrorV.Reason }), "PoolUpdate"); + return await _context.RequestHandling.NotOk(req, Error.Create(ErrorCode.INVALID_REQUEST, updatePool.ErrorV.Reason), "PoolUpdate"); } diff --git a/src/ApiService/ApiService/Functions/Proxy.cs b/src/ApiService/ApiService/Functions/Proxy.cs index 1071b000ac..17ab4d6ed3 100644 --- a/src/ApiService/ApiService/Functions/Proxy.cs +++ b/src/ApiService/ApiService/Functions/Proxy.cs @@ -63,7 +63,7 @@ private async Async.Task Get(HttpRequestData req) { machineId: machineId, dstPort: dstPort).ToListAsync(); if (!forwards.Any()) { - return await _context.RequestHandling.NotOk(req, new Error(ErrorCode.INVALID_REQUEST, new[] { "no forwards for scaleset and node" }), "debug_proxy get"); + return await _context.RequestHandling.NotOk(req, Error.Create(ErrorCode.INVALID_REQUEST, "no forwards for scaleset and node"), "debug_proxy get"); } var response = req.CreateResponse(); @@ -78,7 +78,7 @@ private async Async.Task Get(HttpRequestData req) { await r.WriteAsJsonAsync(new ProxyList(proxies)); return r; default: - return await _context.RequestHandling.NotOk(req, new Error(ErrorCode.INVALID_REQUEST, new[] { "ProxyGet must provide all or none of the following: scaleset_id, machine_id, dst_port" }), "debug_proxy get"); + return await _context.RequestHandling.NotOk(req, Error.Create(ErrorCode.INVALID_REQUEST, "ProxyGet must provide all or none of the following: scaleset_id, machine_id, dst_port"), "debug_proxy get"); } } diff --git a/src/ApiService/ApiService/Functions/ReproVmss.cs b/src/ApiService/ApiService/Functions/ReproVmss.cs index a5385746e7..a18e86433f 100644 --- a/src/ApiService/ApiService/Functions/ReproVmss.cs +++ b/src/ApiService/ApiService/Functions/ReproVmss.cs @@ -35,7 +35,7 @@ private async Async.Task Get(HttpRequestData req) { var vm = await _context.ReproOperations.SearchByPartitionKeys(new[] { $"{request.OkV.VmId}" }).FirstOrDefaultAsync(); if (vm == null) { - return await _context.RequestHandling.NotOk(req, new Error(ErrorCode.INVALID_REQUEST, new[] { "no such VM" }), $"{request.OkV.VmId}"); + return await _context.RequestHandling.NotOk(req, Error.Create(ErrorCode.INVALID_REQUEST, "no such VM"), $"{request.OkV.VmId}"); } var response = req.CreateResponse(HttpStatusCode.OK); @@ -98,7 +98,7 @@ private async Async.Task Delete(HttpRequestData req) { if (request.OkV.VmId == null) { return await _context.RequestHandling.NotOk( req, - new Error(ErrorCode.INVALID_REQUEST, new[] { "missing vm_id" }), + Error.Create(ErrorCode.INVALID_REQUEST, "missing vm_id"), context: "repro delete"); } @@ -107,7 +107,7 @@ private async Async.Task Delete(HttpRequestData req) { if (vm == null) { return await _context.RequestHandling.NotOk( req, - new Error(ErrorCode.INVALID_REQUEST, new[] { "no such vm" }), + Error.Create(ErrorCode.INVALID_REQUEST, "no such vm"), context: "repro delete"); } diff --git a/src/ApiService/ApiService/Functions/Scaleset.cs b/src/ApiService/ApiService/Functions/Scaleset.cs index 94b7a2c400..64f9ac52b9 100644 --- a/src/ApiService/ApiService/Functions/Scaleset.cs +++ b/src/ApiService/ApiService/Functions/Scaleset.cs @@ -70,9 +70,7 @@ private async Task Post(HttpRequestData req) { if (!pool.Managed) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.UNABLE_TO_CREATE, - Errors: new string[] { "scalesets can only be added to managed pools " }), + Error.Create(ErrorCode.UNABLE_TO_CREATE, "scalesets can only be added to managed pools "), context: "ScalesetCreate"); } @@ -96,9 +94,7 @@ private async Task Post(HttpRequestData req) { if (!validRegions.Contains(create.Region)) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.UNABLE_TO_CREATE, - Errors: new string[] { "invalid region" }), + Error.Create(ErrorCode.UNABLE_TO_CREATE, "invalid region"), context: "ScalesetCreate"); } @@ -109,9 +105,7 @@ private async Task Post(HttpRequestData req) { if (!availableSkus.Contains(create.VmSku)) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.UNABLE_TO_CREATE, - Errors: new string[] { $"The specified VM SKU '{create.VmSku}' is not available in the location ${region}" }), + Error.Create(ErrorCode.UNABLE_TO_CREATE, $"The specified VM SKU '{create.VmSku}' is not available in the location ${region}"), context: "ScalesetCreate"); } @@ -142,9 +136,9 @@ private async Task Post(HttpRequestData req) { _log.WithHttpStatus(inserted.ErrorV).Error($"failed to insert new scaleset {scaleset.ScalesetId:Tag:ScalesetId}"); return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.UNABLE_TO_CREATE, - new string[] { $"unable to insert scaleset: {inserted.ErrorV}" } + Error.Create( + ErrorCode.UNABLE_TO_CREATE, + $"unable to insert scaleset: {inserted.ErrorV}" ), context: "ScalesetCreate"); } @@ -191,9 +185,9 @@ private async Task Patch(HttpRequestData req) { if (!scaleset.State.CanUpdate()) { return await _context.RequestHandling.NotOk( req, - new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: new[] { $"scaleset must be in one of the following states to update: {string.Join(", ", ScalesetStateHelper.CanUpdateStates)}" }), + Error.Create( + ErrorCode.INVALID_REQUEST, + $"scaleset must be in one of the following states to update: {string.Join(", ", ScalesetStateHelper.CanUpdateStates)}"), "ScalesetUpdate"); } diff --git a/src/ApiService/ApiService/Functions/Tasks.cs b/src/ApiService/ApiService/Functions/Tasks.cs index ea7a461abe..7bc14e20f9 100644 --- a/src/ApiService/ApiService/Functions/Tasks.cs +++ b/src/ApiService/ApiService/Functions/Tasks.cs @@ -37,9 +37,9 @@ private async Async.Task Get(HttpRequestData req) { if (task == null) { return await _context.RequestHandling.NotOk( req, - new Error( + Error.Create( ErrorCode.INVALID_REQUEST, - new[] { "unable to find task" }), + "unable to find task"), "task get"); } @@ -101,7 +101,7 @@ private async Async.Task Post(HttpRequestData req) { if (!checkConfig.IsOk) { return await _context.RequestHandling.NotOk( req, - new Error(ErrorCode.INVALID_REQUEST, new[] { checkConfig.ErrorV.Error }), + Error.Create(ErrorCode.INVALID_REQUEST, checkConfig.ErrorV.Error), "task create"); } @@ -115,14 +115,14 @@ private async Async.Task Post(HttpRequestData req) { if (job == null) { return await _context.RequestHandling.NotOk( req, - new Error(ErrorCode.INVALID_REQUEST, new[] { "unable to find job" }), + Error.Create(ErrorCode.INVALID_REQUEST, "unable to find job"), cfg.JobId.ToString()); } if (job.State != JobState.Enabled && job.State != JobState.Init) { return await _context.RequestHandling.NotOk( req, - new Error(ErrorCode.UNABLE_TO_ADD_TASK_TO_JOB, new[] { $"unable to add a job in state {job.State}" }), + Error.Create(ErrorCode.UNABLE_TO_ADD_TASK_TO_JOB, $"unable to add a job in state {job.State}"), cfg.JobId.ToString()); } @@ -133,7 +133,7 @@ private async Async.Task Post(HttpRequestData req) { if (prereq == null) { return await _context.RequestHandling.NotOk( req, - new Error(ErrorCode.INVALID_REQUEST, new[] { "unable to find task " }), + Error.Create(ErrorCode.INVALID_REQUEST, "unable to find task "), "task create prerequisite"); } } @@ -165,8 +165,8 @@ private async Async.Task Delete(HttpRequestData req) { var task = await _context.TaskOperations.GetByTaskId(request.OkV.TaskId); if (task == null) { - return await _context.RequestHandling.NotOk(req, new Error(ErrorCode.INVALID_REQUEST, new[] { "unable to find task" - }), "task delete"); + return await _context.RequestHandling.NotOk(req, Error.Create(ErrorCode.INVALID_REQUEST, "unable to find task" + ), "task delete"); } diff --git a/src/ApiService/ApiService/Functions/WebhookLogs.cs b/src/ApiService/ApiService/Functions/WebhookLogs.cs index cbed30d08e..140d16784e 100644 --- a/src/ApiService/ApiService/Functions/WebhookLogs.cs +++ b/src/ApiService/ApiService/Functions/WebhookLogs.cs @@ -36,7 +36,7 @@ private async Async.Task Post(HttpRequestData req) { var webhook = await _context.WebhookOperations.GetByWebhookId(request.OkV.WebhookId); if (webhook == null) { - return await _context.RequestHandling.NotOk(req, new Error(ErrorCode.INVALID_REQUEST, new[] { "unable to find webhook" }), "webhook log"); + return await _context.RequestHandling.NotOk(req, Error.Create(ErrorCode.INVALID_REQUEST, "unable to find webhook"), "webhook log"); } _log.Info($"getting webhook logs: {request.OkV.WebhookId:Tag:WebhookId}"); diff --git a/src/ApiService/ApiService/Functions/WebhookPing.cs b/src/ApiService/ApiService/Functions/WebhookPing.cs index e73fe9f81e..623c0bed5c 100644 --- a/src/ApiService/ApiService/Functions/WebhookPing.cs +++ b/src/ApiService/ApiService/Functions/WebhookPing.cs @@ -36,7 +36,7 @@ private async Async.Task Post(HttpRequestData req) { var webhook = await _context.WebhookOperations.GetByWebhookId(request.OkV.WebhookId); if (webhook == null) { - return await _context.RequestHandling.NotOk(req, new Error(ErrorCode.INVALID_REQUEST, new[] { "unable to find webhook" }), "webhook ping"); + return await _context.RequestHandling.NotOk(req, Error.Create(ErrorCode.INVALID_REQUEST, "unable to find webhook"), "webhook ping"); } _log.Info($"pinging webhook : {request.OkV.WebhookId:Tag:WebhookId}"); diff --git a/src/ApiService/ApiService/Functions/Webhooks.cs b/src/ApiService/ApiService/Functions/Webhooks.cs index 7b8209cde8..1ac1d30cb7 100644 --- a/src/ApiService/ApiService/Functions/Webhooks.cs +++ b/src/ApiService/ApiService/Functions/Webhooks.cs @@ -40,7 +40,7 @@ private async Async.Task Get(HttpRequestData req) { var webhook = await _context.WebhookOperations.GetByWebhookId(request.OkV.WebhookId.Value); if (webhook == null) { - return await _context.RequestHandling.NotOk(req, new Error(ErrorCode.INVALID_REQUEST, new[] { "unable to find webhook" }), "webhook get"); + return await _context.RequestHandling.NotOk(req, Error.Create(ErrorCode.INVALID_REQUEST, "unable to find webhook"), "webhook get"); } var response = req.CreateResponse(HttpStatusCode.OK); @@ -73,7 +73,7 @@ private async Async.Task Patch(HttpRequestData req) { var webhook = await _context.WebhookOperations.GetByWebhookId(request.OkV.WebhookId); if (webhook == null) { - return await _context.RequestHandling.NotOk(req, new Error(ErrorCode.INVALID_REQUEST, new[] { "unable to find webhook" }), "webhook update"); + return await _context.RequestHandling.NotOk(req, Error.Create(ErrorCode.INVALID_REQUEST, "unable to find webhook"), "webhook update"); } var updated = webhook with { @@ -134,7 +134,7 @@ private async Async.Task Delete(HttpRequestData req) { var webhook = await _context.WebhookOperations.GetByWebhookId(request.OkV.WebhookId); if (webhook == null) { - return await _context.RequestHandling.NotOk(req, new Error(ErrorCode.INVALID_REQUEST, new[] { "unable to find webhook" }), "webhook delete"); + return await _context.RequestHandling.NotOk(req, Error.Create(ErrorCode.INVALID_REQUEST, "unable to find webhook"), "webhook delete"); } var r = await _context.WebhookOperations.Delete(webhook); diff --git a/src/ApiService/ApiService/OneFuzzTypes/Model.cs b/src/ApiService/ApiService/OneFuzzTypes/Model.cs index 9b0594a4e1..c29dbcc0c9 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/Model.cs +++ b/src/ApiService/ApiService/OneFuzzTypes/Model.cs @@ -166,7 +166,10 @@ public record Proxy bool Outdated ) : StatefulEntityBase(State); -public record Error(ErrorCode Code, string[]? Errors = null) { +public record Error(ErrorCode Code, List? Errors) { + public static Error Create(ErrorCode code, params string[] errors) { + return new Error(code, errors.ToList()); + } public sealed override string ToString() { var errorsString = Errors != null ? string.Join("", Errors) : string.Empty; return $"Error {{ Code = {Code}, Errors = {errorsString} }}"; diff --git a/src/ApiService/ApiService/OneFuzzTypes/ReturnTypes.cs b/src/ApiService/ApiService/OneFuzzTypes/ReturnTypes.cs index 1b6644bcb6..1035d03bad 100644 --- a/src/ApiService/ApiService/OneFuzzTypes/ReturnTypes.cs +++ b/src/ApiService/ApiService/OneFuzzTypes/ReturnTypes.cs @@ -49,7 +49,7 @@ public readonly struct OneFuzzResult { private OneFuzzResult(T_Ok ok) => (OkV, ErrorV, IsOk) = (ok, null, true); - private OneFuzzResult(ErrorCode errorCode, string[] errors) => (OkV, ErrorV, IsOk) = (default, new Error(errorCode, errors), false); + private OneFuzzResult(ErrorCode errorCode, string[] errors) => (OkV, ErrorV, IsOk) = (default, new Error(errorCode, errors.ToList()), false); private OneFuzzResult(Error err) => (OkV, ErrorV, IsOk) = (default, err, false); @@ -72,7 +72,7 @@ public readonly struct OneFuzzResultVoid { public OneFuzzResultVoid() => (ErrorV, IsOk) = (null, true); - private OneFuzzResultVoid(ErrorCode errorCode, string[] errors) => (ErrorV, IsOk) = (new Error(errorCode, errors), false); + private OneFuzzResultVoid(ErrorCode errorCode, string[] errors) => (ErrorV, IsOk) = (new Error(errorCode, errors.ToList()), false); private OneFuzzResultVoid(Error err) => (ErrorV, IsOk) = (err, false); diff --git a/src/ApiService/ApiService/TestHooks/InstanceConfigTestHooks.cs b/src/ApiService/ApiService/TestHooks/InstanceConfigTestHooks.cs index e3ec334152..70120049f0 100644 --- a/src/ApiService/ApiService/TestHooks/InstanceConfigTestHooks.cs +++ b/src/ApiService/ApiService/TestHooks/InstanceConfigTestHooks.cs @@ -26,7 +26,7 @@ public async Task Get([HttpTrigger(AuthorizationLevel.Anonymou if (config is null) { _log.Error($"Instance config is null"); - Error err = new(ErrorCode.INVALID_REQUEST, new[] { "Instance config is null" }); + Error err = Error.Create(ErrorCode.INVALID_REQUEST, "Instance config is null"); var resp = req.CreateResponse(HttpStatusCode.InternalServerError); await resp.WriteAsJsonAsync(err); return resp; diff --git a/src/ApiService/ApiService/TestHooks/NodeOperationsTestHooks.cs b/src/ApiService/ApiService/TestHooks/NodeOperationsTestHooks.cs index 29234e2d2a..ee35a721b7 100644 --- a/src/ApiService/ApiService/TestHooks/NodeOperationsTestHooks.cs +++ b/src/ApiService/ApiService/TestHooks/NodeOperationsTestHooks.cs @@ -10,7 +10,7 @@ #if DEBUG namespace ApiService.TestHooks { - sealed record MarkTasks(Node node, Error? error); + sealed record MarkTasks(Node node, Error error); public class NodeOperationsTestHooks { private readonly ILogTracer _log; diff --git a/src/ApiService/ApiService/onefuzzlib/EndpointAuthorization.cs b/src/ApiService/ApiService/onefuzzlib/EndpointAuthorization.cs index 2abceb30d8..011e522310 100644 --- a/src/ApiService/ApiService/onefuzzlib/EndpointAuthorization.cs +++ b/src/ApiService/ApiService/onefuzzlib/EndpointAuthorization.cs @@ -75,9 +75,9 @@ public async Async.Task Reject(HttpRequestData req, UserInfo t return await _context.RequestHandling.NotOk( req, - new Error( + Error.Create( ErrorCode.UNAUTHORIZED, - new string[] { reason ?? "Unrecognized agent" } + reason ?? "Unrecognized agent" ), "token verification", HttpStatusCode.Unauthorized @@ -92,9 +92,9 @@ public async Async.Task CheckRequireAdmins(HttpRequestData re var config = await _context.ConfigOperations.Fetch(); if (config is null) { - return new Error( - Code: ErrorCode.INVALID_CONFIGURATION, - Errors: new string[] { "no instance configuration found " }); + return Error.Create( + ErrorCode.INVALID_CONFIGURATION, + "no instance configuration found "); } return CheckRequireAdminsImpl(config, tokenResult.OkV.UserInfo); @@ -117,9 +117,7 @@ private static OneFuzzResultVoid CheckRequireAdminsImpl(InstanceConfig config, U } if (config.Admins is null) { - return new Error( - Code: ErrorCode.UNAUTHORIZED, - Errors: new string[] { "pool modification disabled " }); + return Error.Create(ErrorCode.UNAUTHORIZED, "pool modification disabled "); } if (userInfo.ObjectId is Guid objectId) { @@ -127,13 +125,9 @@ private static OneFuzzResultVoid CheckRequireAdminsImpl(InstanceConfig config, U return OneFuzzResultVoid.Ok; } - return new Error( - Code: ErrorCode.UNAUTHORIZED, - Errors: new string[] { "not authorized to manage instance" }); + return Error.Create(ErrorCode.UNAUTHORIZED, "not authorized to manage instance"); } else { - return new Error( - Code: ErrorCode.UNAUTHORIZED, - Errors: new string[] { "user had no Object ID" }); + return Error.Create(ErrorCode.UNAUTHORIZED, "user had no Object ID"); } } @@ -157,16 +151,12 @@ public async Async.Task CheckAccess(HttpRequestData req) { var allowed = await membershipChecker.IsMember(rule.AllowedGroupsIds, memberId); if (!allowed) { _log.Error($"unauthorized access: {memberId:Tag:MemberId} is not authorized to access {path:Tag:Path}"); - return new Error( - Code: ErrorCode.UNAUTHORIZED, - Errors: new string[] { "not approved to use this endpoint" }); + return Error.Create(ErrorCode.UNAUTHORIZED, "not approved to use this endpoint"); } else { return OneFuzzResultVoid.Ok; } } catch (Exception ex) { - return new Error( - Code: ErrorCode.UNAUTHORIZED, - Errors: new string[] { "unable to interact with graph", ex.Message }); + return Error.Create(ErrorCode.UNAUTHORIZED, "unable to interact with graph", ex.Message); } } diff --git a/src/ApiService/ApiService/onefuzzlib/ImageReference.cs b/src/ApiService/ApiService/onefuzzlib/ImageReference.cs index a3de78ee03..68b26c3aaa 100644 --- a/src/ApiService/ApiService/onefuzzlib/ImageReference.cs +++ b/src/ApiService/ApiService/onefuzzlib/ImageReference.cs @@ -22,7 +22,7 @@ public abstract record ImageReference { public static ImageReference MustParse(string image) { var result = TryParse(image); if (!result.IsOk) { - var msg = string.Join(", ", result.ErrorV.Errors ?? Array.Empty()); + var msg = result.ErrorV.Errors != null ? string.Join(", ", result.ErrorV.Errors) : string.Empty; throw new ArgumentException(msg, nameof(image)); } @@ -46,18 +46,17 @@ public static OneFuzzResult TryParse(string image) { } else if (identifier.ResourceType == ImageResource.ResourceType) { result = new Image(identifier); } else { - return new Error( + return Error.Create( ErrorCode.INVALID_IMAGE, - new[] { $"Unknown image resource type: {identifier.ResourceType}" }); + $"Unknown image resource type: {identifier.ResourceType}"); } } catch (FormatException) { // not an ARM identifier, try to parse a marketplace image: var imageParts = image.Split(":"); // The python code would throw if more than 4 parts are found in the split if (imageParts.Length != 4) { - return new Error( - Code: ErrorCode.INVALID_IMAGE, - new[] { $"Expected 4 ':' separated parts in '{image}'" }); + return Error.Create( + ErrorCode.INVALID_IMAGE, $"Expected 4 ':' separated parts in '{image}'"); } result = new Marketplace( @@ -112,10 +111,10 @@ protected override async Task> GetOsUncached(ArmClient armClie if (resource.Value.Data.OSType is OperatingSystemTypes os) { return OneFuzzResult.Ok(Enum.Parse(os.ToString(), ignoreCase: true)); } else { - return new Error(ErrorCode.INVALID_IMAGE, new[] { "Specified image had no OSType" }); + return Error.Create(ErrorCode.INVALID_IMAGE, "Specified image had no OSType"); } } catch (Exception ex) when (ex is RequestFailedException) { - return new Error(ErrorCode.INVALID_IMAGE, new[] { ex.ToString() }); + return Error.Create(ErrorCode.INVALID_IMAGE, ex.ToString()); } } } @@ -129,10 +128,10 @@ protected override async Task> GetOsUncached(ArmClient armClie if (resource.Value.Data.OSType is OperatingSystemTypes os) { return OneFuzzResult.Ok(Enum.Parse(os.ToString(), ignoreCase: true)); } else { - return new Error(ErrorCode.INVALID_IMAGE, new[] { "Specified image had no OSType" }); + return Error.Create(ErrorCode.INVALID_IMAGE, "Specified image had no OSType"); } } catch (Exception ex) when (ex is RequestFailedException) { - return new Error(ErrorCode.INVALID_IMAGE, new[] { ex.ToString() }); + return Error.Create(ErrorCode.INVALID_IMAGE, ex.ToString()); } } } @@ -145,10 +144,10 @@ protected override async Task> GetOsUncached(ArmClient armClie if (resource.Value.Data.OSType is OperatingSystemTypes os) { return OneFuzzResult.Ok(Enum.Parse(os.ToString(), ignoreCase: true)); } else { - return new Error(ErrorCode.INVALID_IMAGE, new[] { "Specified image had no OSType" }); + return Error.Create(ErrorCode.INVALID_IMAGE, "Specified image had no OSType"); } } catch (Exception ex) when (ex is RequestFailedException) { - return new Error(ErrorCode.INVALID_IMAGE, new[] { ex.ToString() }); + return Error.Create(ErrorCode.INVALID_IMAGE, ex.ToString()); } } } @@ -162,10 +161,10 @@ protected override async Task> GetOsUncached(ArmClient armClie if (resource.Value.Data.OSType is OperatingSystemTypes os) { return OneFuzzResult.Ok(Enum.Parse(os.ToString(), ignoreCase: true)); } else { - return new Error(ErrorCode.INVALID_IMAGE, new[] { "Specified image had no OSType" }); + return Error.Create(ErrorCode.INVALID_IMAGE, "Specified image had no OSType"); } } catch (Exception ex) when (ex is RequestFailedException) { - return new Error(ErrorCode.INVALID_IMAGE, new[] { ex.ToString() }); + return Error.Create(ErrorCode.INVALID_IMAGE, ex.ToString()); } } } @@ -178,7 +177,7 @@ protected override async Task> GetOsUncached(ArmClient armClie var os = resource.Value.Data.StorageProfile.OSDisk.OSType.ToString(); return OneFuzzResult.Ok(Enum.Parse(os.ToString(), ignoreCase: true)); } catch (Exception ex) when (ex is RequestFailedException) { - return new Error(ErrorCode.INVALID_IMAGE, new[] { ex.ToString() }); + return Error.Create(ErrorCode.INVALID_IMAGE, ex.ToString()); } } } diff --git a/src/ApiService/ApiService/onefuzzlib/JobOperations.cs b/src/ApiService/ApiService/onefuzzlib/JobOperations.cs index ded5d2fe19..36185777f0 100644 --- a/src/ApiService/ApiService/onefuzzlib/JobOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/JobOperations.cs @@ -80,7 +80,7 @@ public async Async.Task StopNeverStartedJobs() { await foreach (var job in QueryAsync(filter)) { await foreach (var task in _context.TaskOperations.QueryAsync(Query.PartitionKey(job.JobId.ToString()))) { - await _context.TaskOperations.MarkFailed(task, new Error(ErrorCode.TASK_FAILED, new[] { "job never not start" })); + await _context.TaskOperations.MarkFailed(task, Error.Create(ErrorCode.TASK_FAILED, "job never not start")); } _logTracer.Info($"stopping job that never started: {job.JobId:Tag:JobId}"); diff --git a/src/ApiService/ApiService/onefuzzlib/NodeOperations.cs b/src/ApiService/ApiService/onefuzzlib/NodeOperations.cs index a1e3cf156c..0a357f0f31 100644 --- a/src/ApiService/ApiService/onefuzzlib/NodeOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/NodeOperations.cs @@ -48,7 +48,7 @@ IAsyncEnumerable SearchStates(Guid? poolId = default, IAsyncEnumerable GetDeadNodes(Guid scaleSetId, TimeSpan expirationPeriod); - Async.Task MarkTasksStoppedEarly(Node node, Error? error = null); + Async.Task MarkTasksStoppedEarly(Node node, Error? error); static readonly TimeSpan NODE_EXPIRATION_TIME = TimeSpan.FromHours(1.0); static readonly TimeSpan NODE_REIMAGE_TIME = TimeSpan.FromDays(6.0); @@ -213,18 +213,18 @@ public async Task CanProcessNewWork(Node node) { var scaleset = scalesetResult.OkV; if (!scaleset.State.IsAvailable()) { - return CanProcessNewWorkResponse.NotAllowed($"scaleset not available for work. Scaleset state '{scaleset.State}'"); ; + return CanProcessNewWorkResponse.NotAllowed($"scaleset not available for work. Scaleset state '{scaleset.State}'"); } } var poolResult = await _context.PoolOperations.GetByName(node.PoolName); if (!poolResult.IsOk) { - return CanProcessNewWorkResponse.NotAllowed("invalid pool"); ; + return CanProcessNewWorkResponse.NotAllowed("invalid pool"); } var pool = poolResult.OkV; if (!PoolStateHelper.Available.Contains(pool.State)) { - return CanProcessNewWorkResponse.NotAllowed("pool is not available for work"); ; + return CanProcessNewWorkResponse.NotAllowed("pool is not available for work"); } return CanProcessNewWorkResponse.Allowed(); @@ -585,14 +585,19 @@ public IAsyncEnumerable SearchByPoolName(PoolName poolName) { } - public async Async.Task MarkTasksStoppedEarly(Node node, Error? error = null) { - if (error is null) { - error = new Error(ErrorCode.TASK_FAILED, new[] { $"node reimaged during task execution. machine_id: {node.MachineId}" }); - } - + public async Async.Task MarkTasksStoppedEarly(Node node, Error? error) { await foreach (var entry in _context.NodeTasksOperations.GetByMachineId(node.MachineId)) { var task = await _context.TaskOperations.GetByTaskId(entry.TaskId); - if (task is not null) { + if (task is not null && !TaskStateHelper.ShuttingDown(task.State)) { + var message = $"Node {node.MachineId} stopping while the task state is '{task.State}'"; + if (error is not null) { + if (error.Errors == null) { + error = error with { Errors = new List() }; + } + error.Errors.Add(message); + } else { + error = Error.Create(ErrorCode.TASK_FAILED, message); + } await _context.TaskOperations.MarkFailed(task, error); } if (!node.DebugKeepNode) { @@ -605,7 +610,7 @@ public async Async.Task MarkTasksStoppedEarly(Node node, Error? error = null) { } public new async Async.Task Delete(Node node) { - await MarkTasksStoppedEarly(node); + await MarkTasksStoppedEarly(node, Error.Create(ErrorCode.INVALID_NODE, "node is being deleted")); await _context.NodeTasksOperations.ClearByMachineId(node.MachineId); await _context.NodeMessageOperations.ClearMessages(node.MachineId); var r = await base.Delete(node); diff --git a/src/ApiService/ApiService/onefuzzlib/NsgOperations.cs b/src/ApiService/ApiService/onefuzzlib/NsgOperations.cs index f2acc562b3..2c0164fae8 100644 --- a/src/ApiService/ApiService/onefuzzlib/NsgOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/NsgOperations.cs @@ -37,15 +37,14 @@ public NsgOperations(ILogTracer logTracer, IOnefuzzContext context) { public async Async.Task> AssociateSubnet(string name, VirtualNetworkResource vnet, SubnetResource subnet) { var nsg = await GetNsg(name); if (nsg == null) { - return OneFuzzResult.Error(new Error(ErrorCode.UNABLE_TO_FIND, - new[] { $"cannot associate subnet. nsg {name} not found" })); + return OneFuzzResult.Error(Error.Create(ErrorCode.UNABLE_TO_FIND, + $"cannot associate subnet. nsg {name} not found")); } if (nsg.Data.Location != vnet.Data.Location) { - return OneFuzzResult.Error(new Error(ErrorCode.UNABLE_TO_UPDATE, - new[] { + return OneFuzzResult.Error(Error.Create(ErrorCode.UNABLE_TO_UPDATE, $"subnet and nsg have to be in the same region. nsg {nsg.Data.Name} {nsg.Data.Location}, subnet: {subnet.Data.Name} {subnet.Data}" - })); + )); } if (subnet.Data.NetworkSecurityGroup != null && subnet.Data.NetworkSecurityGroup.Id == nsg.Id) { diff --git a/src/ApiService/ApiService/onefuzzlib/ProxyOperations.cs b/src/ApiService/ApiService/onefuzzlib/ProxyOperations.cs index fc3b67842e..15a7d5b869 100644 --- a/src/ApiService/ApiService/onefuzzlib/ProxyOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/ProxyOperations.cs @@ -212,8 +212,8 @@ public async Async.Task Init(Proxy proxy) { } private async System.Threading.Tasks.Task SetProvisionFailed(Proxy proxy, VirtualMachineInstanceView? instanceView) { - var errors = GetErrors(proxy, instanceView).ToArray(); - return await SetFailed(proxy, new Error(ErrorCode.PROXY_FAILED, errors)); + var errors = GetErrors(proxy, instanceView); + return await SetFailed(proxy, new Error(ErrorCode.PROXY_FAILED, errors.ToList())); } private async Task SetFailed(Proxy proxy, Error error) { @@ -259,7 +259,7 @@ public async Task ExtensionsLaunch(Proxy proxy) { var vm = GetVm(proxy, config); var vmData = await _context.VmOperations.GetVm(vm.Name); if (vmData is null) { - return await SetFailed(proxy, new Error(ErrorCode.PROXY_FAILED, new[] { "azure not able to find vm" })); + return await SetFailed(proxy, Error.Create(ErrorCode.PROXY_FAILED, "azure not able to find vm")); } if (vmData.ProvisioningState == "Failed") { diff --git a/src/ApiService/ApiService/onefuzzlib/ReproOperations.cs b/src/ApiService/ApiService/onefuzzlib/ReproOperations.cs index a840618ba3..ec16b4f028 100644 --- a/src/ApiService/ApiService/onefuzzlib/ReproOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/ReproOperations.cs @@ -180,9 +180,9 @@ public async Async.Task ExtensionsLaunch(Repro repro) { if (vmData == null) { return await _context.ReproOperations.SetError( repro, - new Error( + Error.Create( ErrorCode.VM_CREATE_FAILED, - new string[] { "failed before launching extensions" })); + "failed before launching extensions")); } if (vmData.ProvisioningState == "Failed") { var failedVmData = await _context.VmOperations.GetVmWithInstanceView(vm.Name); @@ -232,7 +232,7 @@ public async Async.Task SetFailed(Repro repro, VirtualMachineInstanceView repro, new Error( ErrorCode.VM_CREATE_FAILED, - errors)); + errors.ToList())); } public async Task BuildReproScript(Repro repro) { diff --git a/src/ApiService/ApiService/onefuzzlib/Request.cs b/src/ApiService/ApiService/onefuzzlib/Request.cs index 4e19d32f85..12acb7278c 100644 --- a/src/ApiService/ApiService/onefuzzlib/Request.cs +++ b/src/ApiService/ApiService/onefuzzlib/Request.cs @@ -98,9 +98,9 @@ public static async Async.Task> ParseRequest(HttpRequestData } if (errors.Any()) { - return new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: errors.ToArray()); + return Error.Create( + ErrorCode.INVALID_REQUEST, + errors.ToArray()); } } @@ -109,9 +109,9 @@ public static async Async.Task> ParseRequest(HttpRequestData if (Validator.TryValidateObject(t, validationContext, validationResults, validateAllProperties: true)) { return OneFuzzResult.Ok(t); } else { - return new Error( - Code: ErrorCode.INVALID_REQUEST, - Errors: validationResults.Select(vr => vr.ToString()).ToArray()); + return Error.Create( + ErrorCode.INVALID_REQUEST, + validationResults.Select(vr => vr.ToString()).ToArray()); } } else { return OneFuzzResult.Error( @@ -154,13 +154,12 @@ public static Error ConvertError(Exception exception) { } } - return new Error( + return Error.Create( ErrorCode.INVALID_REQUEST, - new string[] { exception.Message, exception.Source ?? string.Empty, exception.StackTrace ?? string.Empty - } + ); } diff --git a/src/ApiService/ApiService/onefuzzlib/ScalesetOperations.cs b/src/ApiService/ApiService/onefuzzlib/ScalesetOperations.cs index 83df5fc47b..4ad05abf41 100644 --- a/src/ApiService/ApiService/onefuzzlib/ScalesetOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/ScalesetOperations.cs @@ -291,7 +291,7 @@ public async Async.Task Setup(Scaleset scaleset) { if (scaleset.Auth is null) { _logTracer.Error($"Scaleset Auth is missing for scaleset {scaleset.ScalesetId:Tag:ScalesetId}"); - return await SetFailed(scaleset, new Error(ErrorCode.UNABLE_TO_CREATE, new[] { "missing required auth" })); + return await SetFailed(scaleset, Error.Create(ErrorCode.UNABLE_TO_CREATE, "missing required auth")); } var vmss = await _context.VmssOperations.GetVmss(scaleset.ScalesetId); @@ -455,7 +455,7 @@ public async Async.Task Init(Scaleset scaleset) { return await SetFailed(scaleset, imageOsResult.ErrorV); } else if (imageOsResult.OkV != pool.Os) { _logTracer.Error($"got invalid OS: {imageOsResult.OkV:Tag:ActualOs} for scaleset: {scaleset.ScalesetId:Tag:ScalesetId} expected OS {pool.Os:Tag:ExpectedOs}"); - return await SetFailed(scaleset, new Error(ErrorCode.INVALID_REQUEST, new[] { $"invalid os (got: {imageOsResult.OkV} needed: {pool.Os})" })); + return await SetFailed(scaleset, Error.Create(ErrorCode.INVALID_REQUEST, $"invalid os (got: {imageOsResult.OkV} needed: {pool.Os})")); } else { return await SetState(scaleset, ScalesetState.Setup); } @@ -605,7 +605,7 @@ where x.State.ReadyForReset() _log.Info($"{errorMessage} {deadNode.MachineId:Tag:MachineId} {deadNode.ScalesetId:Tag:ScalesetId}"); - var error = new Error(ErrorCode.TASK_FAILED, new[] { $"{errorMessage} scaleset_id {deadNode.ScalesetId} last heartbeat:{deadNode.Heartbeat}" }); + var error = Error.Create(ErrorCode.TASK_FAILED, $"{errorMessage} scaleset_id {deadNode.ScalesetId} last heartbeat:{deadNode.Heartbeat}"); await _context.NodeOperations.MarkTasksStoppedEarly(deadNode, error); toReimage[deadNode.MachineId] = await _context.NodeOperations.ToReimage(deadNode, true); } diff --git a/src/ApiService/ApiService/onefuzzlib/TaskOperations.cs b/src/ApiService/ApiService/onefuzzlib/TaskOperations.cs index 6be5d4bafa..61755f7626 100644 --- a/src/ApiService/ApiService/onefuzzlib/TaskOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/TaskOperations.cs @@ -106,7 +106,7 @@ public async Async.Task MarkStopping(Task task) { } if (!task.State.HasStarted()) { - await MarkFailed(task, new Error(Code: ErrorCode.TASK_FAILED, Errors: new[] { "task never started" })); + await MarkFailed(task, Error.Create(ErrorCode.TASK_FAILED, "task never started")); } else { _ = await SetState(task, TaskState.Stopping); } @@ -133,7 +133,7 @@ private async Async.Task MarkDependantsFailed(Task task, List? taskInJob = foreach (var t in taskInJob) { if (t.Config.PrereqTasks != null) { if (t.Config.PrereqTasks.Contains(task.TaskId)) { - await MarkFailed(t, new Error(ErrorCode.TASK_FAILED, new[] { $"prerequisite task failed. task_id:{t.TaskId}" }), taskInJob); + await MarkFailed(t, Error.Create(ErrorCode.TASK_FAILED, $"prerequisite task failed. task_id:{t.TaskId}"), taskInJob); } } } @@ -264,7 +264,7 @@ public async Async.Task CheckPrereqTasks(Task task) { // if a prereq task fails, then mark this task as failed if (t == null) { - await MarkFailed(task, new Error(ErrorCode.INVALID_REQUEST, Errors: new[] { "unable to find prereq task" })); + await MarkFailed(task, Error.Create(ErrorCode.INVALID_REQUEST, "unable to find prereq task")); return false; } diff --git a/src/ApiService/ApiService/onefuzzlib/VmssOperations.cs b/src/ApiService/ApiService/onefuzzlib/VmssOperations.cs index 5cefac36ef..77efd5eec9 100644 --- a/src/ApiService/ApiService/onefuzzlib/VmssOperations.cs +++ b/src/ApiService/ApiService/onefuzzlib/VmssOperations.cs @@ -235,7 +235,7 @@ public async Async.Task> GetInstanceId(Guid name, Guid vmI try { return OneFuzzResult.Ok(await GetInstanceIdForVmId(name, vmId)); } catch { - return new Error(ErrorCode.UNABLE_TO_FIND, new string[] { $"unable to find scaleset machine: {name}:{vmId}" }); + return Error.Create(ErrorCode.UNABLE_TO_FIND, $"unable to find scaleset machine: {name}:{vmId}"); } } diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/JinjaTemplateAdapter.cs b/src/ApiService/ApiService/onefuzzlib/notifications/JinjaTemplateAdapter.cs index ef51e409f8..c538dfca95 100644 --- a/src/ApiService/ApiService/onefuzzlib/notifications/JinjaTemplateAdapter.cs +++ b/src/ApiService/ApiService/onefuzzlib/notifications/JinjaTemplateAdapter.cs @@ -189,7 +189,7 @@ public static async Async.Task ValidateScribanTempla new List { TaskDebugFlag.KeepNodeOnCompletion }, true ), - new Error(ErrorCode.UNABLE_TO_FIND, new string[] { "some error message" }), + Error.Create(ErrorCode.UNABLE_TO_FIND, "some error message"), new Authentication("password", "public key", "private key"), DateTimeOffset.UtcNow, DateTimeOffset.UtcNow, diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/NotificationsBase.cs b/src/ApiService/ApiService/onefuzzlib/notifications/NotificationsBase.cs index 60fa70a2bd..9ff790bda5 100644 --- a/src/ApiService/ApiService/onefuzzlib/notifications/NotificationsBase.cs +++ b/src/ApiService/ApiService/onefuzzlib/notifications/NotificationsBase.cs @@ -17,7 +17,7 @@ public NotificationsBase(ILogTracer logTracer, IOnefuzzContext context) { public async Async.Task LogFailedNotification(Report report, Exception error, Guid notificationId) { _logTracer.Error($"notification failed: notification_id:{notificationId:Tag:NotificationId} job_id:{report.JobId:Tag:JobId} task_id:{report.TaskId:Tag:TaskId} err:{error.Message:Tag:Error}"); - Error? err = new Error(ErrorCode.NOTIFICATION_FAILURE, new string[] { $"{error}" }); + Error? err = Error.Create(ErrorCode.NOTIFICATION_FAILURE, $"{error}"); await _context.Events.SendEvent(new EventNotificationFailed( NotificationId: notificationId, JobId: report.JobId, diff --git a/src/ApiService/IntegrationTests/Fakes/TestEndpointAuthorization.cs b/src/ApiService/IntegrationTests/Fakes/TestEndpointAuthorization.cs index 8980326e63..0d20c42bf6 100644 --- a/src/ApiService/IntegrationTests/Fakes/TestEndpointAuthorization.cs +++ b/src/ApiService/IntegrationTests/Fakes/TestEndpointAuthorization.cs @@ -36,9 +36,9 @@ public override Task CallIf( return _context.RequestHandling.NotOk( req, - new Error( + Error.Create( ErrorCode.UNAUTHORIZED, - new string[] { "Unrecognized agent" } + "Unrecognized agent" ), "token verification", HttpStatusCode.Unauthorized From 4044e5cc1a287953a2d0fe17c959e5289ff3ebe0 Mon Sep 17 00:00:00 2001 From: Marc Greisen Date: Thu, 4 May 2023 13:30:07 -0700 Subject: [PATCH 3/3] Fix typo in parameter name and set default value to true. (#3085) --- src/agent/onefuzz/src/az_copy.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/agent/onefuzz/src/az_copy.rs b/src/agent/onefuzz/src/az_copy.rs index 10b3d7432c..45b222d3ff 100644 --- a/src/agent/onefuzz/src/az_copy.rs +++ b/src/agent/onefuzz/src/az_copy.rs @@ -190,7 +190,7 @@ async fn retry_az_impl(mode: Mode, src: &OsStr, dst: &OsStr, args: &[&str]) -> R pub async fn sync(src: impl AsRef, dst: impl AsRef, delete_dst: bool) -> Result<()> { let args = if delete_dst { - vec!["--delete_destination"] + vec!["--delete-destination=true"] } else { vec![] };