Skip to content

Commit

Permalink
Kanan/semantic validation (microsoft#3386)
Browse files Browse the repository at this point in the history
* Throw exceptoins for missing area/iteration paths

* Comment out feature flag check for work item creation

* Revert "Comment out feature flag check for work item creation"

This reverts commit ad96169.

* Add some extra info to the exceptions to distinguish them

* Add logging when a validation error is encountered

* Add call to validate in NotificationsTest for testing

* Fix area and iteration path validation

* Update error messages

* Fix ArgumentNullException

* Add extra information to path check failure

* Request classification nodes with depth equal to number of parts in the path

* Pass tree structure as parameter instead of always using Areas

* Remove '[PAT]' from error messages

* Require project name in Area/IterationPath during validation

* PR comments

* Sneak in a one-line fix for empty comments

* Change context messsage in response from notiifcation test
  • Loading branch information
kananb committed Aug 28, 2023
1 parent 10bb79a commit 2a88838
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 3 deletions.
5 changes: 5 additions & 0 deletions src/ApiService/ApiService/Functions/NotificationsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@ public class NotificationsTest {
}

var notificationTest = request.OkV;
var validConfig = await notificationTest.Notification.Config.Validate();
if (!validConfig.IsOk) {
return await _context.RequestHandling.NotOk(req, validConfig.ErrorV, context: "notification test");
}

var result = await _context.NotificationOperations.TriggerNotification(notificationTest.Notification.Container, notificationTest.Notification,
notificationTest.Report, isLastRetryAttempt: true);
var response = req.CreateResponse(HttpStatusCode.OK);
Expand Down
1 change: 1 addition & 0 deletions src/ApiService/ApiService/OneFuzzTypes/Enums.cs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ public enum ErrorCode {
ADO_VALIDATION_UNEXPECTED_ERROR = 491,
ADO_VALIDATION_MISSING_PAT_SCOPES = 492,
ADO_WORKITEM_PROCESSING_DISABLED = 494,
ADO_VALIDATION_INVALID_PATH = 495,
// NB: if you update this enum, also update enums.py
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ public NotificationOperations(ILogger<NotificationOperations> log, IOnefuzzConte
if (await _context.FeatureManagerSnapshot.IsEnabledAsync(FeatureFlagConstants.SemanticNotificationConfigValidation)) {
var validConfig = await config.Validate();
if (!validConfig.IsOk) {
_logTracer.LogError($"Error(s) ocurred during template validation: {{title}}{Environment.NewLine}{{errors}}", validConfig.ErrorV.Title, string.Join(Environment.NewLine, validConfig.ErrorV.Errors ?? new()));
return OneFuzzResult<Notification>.Error(validConfig.ErrorV);
}
}
Expand Down
58 changes: 55 additions & 3 deletions src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,38 @@ public class Ado : NotificationsBase, IAdo {
return errorCodes.Any(errorStr.Contains);
}

private static async Async.Task<OneFuzzResultVoid> ValidatePath(string project, string path, TreeStructureGroup structureGroup, WorkItemTrackingHttpClient client) {
var pathType = (structureGroup == TreeStructureGroup.Areas) ? "Area" : "Iteration";
var pathParts = path.Split('\\');
if (!string.Equals(pathParts[0], project, StringComparison.OrdinalIgnoreCase)) {
return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_INVALID_PATH, new string[] {
$"Path \"{path}\" is invalid. It must start with the project name, \"{project}\".",
$"Example: \"{project}\\{path}\".",
});
}

var current = await client.GetClassificationNodeAsync(project, structureGroup, depth: pathParts.Length - 1);
if (current == null) {
return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_INVALID_PATH, new string[] {
$"{pathType} Path \"{path}\" is invalid. \"{project}\" is not a valid project.",
});
}

foreach (var part in pathParts.Skip(1)) {
var child = current.Children?.FirstOrDefault(x => string.Equals(x.Name, part, StringComparison.OrdinalIgnoreCase));
if (child == null) {
return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_INVALID_PATH, new string[] {
$"{pathType} Path \"{path}\" is invalid. \"{part}\" is not a valid child of \"{current.Name}\".",
$"Valid children of \"{current.Name}\" are: [{string.Join(',', current.Children?.Select(x => $"\"{x.Name}\"") ?? new List<string>())}].",
});
}

current = child;
}

return OneFuzzResultVoid.Ok;
}

public static async Async.Task<OneFuzzResultVoid> Validate(AdoTemplate config) {
// Validate PAT is valid for the base url
VssConnection connection;
Expand Down Expand Up @@ -124,10 +156,9 @@ public class Ado : NotificationsBase, IAdo {
return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_INVALID_PAT, "Auth token is missing or invalid");
}

var witClient = await connection.GetClientAsync<WorkItemTrackingHttpClient>();
try {
// Validate unique_fields are part of the project's valid fields
var witClient = await connection.GetClientAsync<WorkItemTrackingHttpClient>();

// The set of valid fields for this project according to ADO
var projectValidFields = await GetValidFields(witClient, config.Project);

Expand Down Expand Up @@ -162,6 +193,27 @@ public class Ado : NotificationsBase, IAdo {
});
}

try {
// Validate AreaPath and IterationPath exist
if (config.AdoFields.TryGetValue("System.AreaPath", out var areaPathString)) {
var validateAreaPath = await ValidatePath(config.Project, areaPathString, TreeStructureGroup.Areas, witClient);
if (!validateAreaPath.IsOk) {
return validateAreaPath;
}
}
if (config.AdoFields.TryGetValue("System.IterationPath", out var iterationPathString)) {
var validateIterationPath = await ValidatePath(config.Project, iterationPathString, TreeStructureGroup.Iterations, witClient);
if (!validateIterationPath.IsOk) {
return validateIterationPath;
}
}
} catch (Exception e) {
return OneFuzzResultVoid.Error(ErrorCode.ADO_VALIDATION_UNEXPECTED_ERROR, new string[] {
"Failed to query and validate against the classification nodes for this project",
$"Exception: {e}",
});
}

return OneFuzzResultVoid.Ok;
}

Expand Down Expand Up @@ -362,7 +414,7 @@ public sealed class AdoConnector {
return false;
}

if (_config.OnDuplicate.Comment != null) {
if (!string.IsNullOrEmpty(_config.OnDuplicate.Comment)) {
var comment = _config.OnDuplicate.Comment;
_ = await _client.AddCommentAsync(
new CommentCreate() {
Expand Down
1 change: 1 addition & 0 deletions src/pytypes/onefuzztypes/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,7 @@ class ErrorCode(Enum):
ADO_VALIDATION_UNEXPECTED_HTTP_EXCEPTION = 490
ADO_VALIDATION_UNEXPECTED_ERROR = 491
ADO_VALIDATION_MISSING_PAT_SCOPES = 492
ADO_VALIDATION_INVALID_PATH = 495
# NB: if you update this enum, also update Enums.cs


Expand Down

0 comments on commit 2a88838

Please sign in to comment.