Sematically validate notification configs (microsoft#2850)

* Add new command * Update remaining jinja templates and references to use scriban * Add ado template validation * Validate ado and github templates * Remove unnecessary function * Update src/ApiService/ApiService/OneFuzzTypes/Model.cs Co-authored-by: Cheick Keita <kcheick@gmail.com> --------- Co-authored-by: Cheick Keita <kcheick@gmail.com>
chkeita · Feb 16, 2023 · 4349fae · 4349fae
1 parent 2a191e1
commit 4349fae
Show file tree

Hide file tree

Showing 6 changed files with 126 additions and 22 deletions.
diff --git a/src/ApiService/ApiService/FeatureFlags.cs b/src/ApiService/ApiService/FeatureFlags.cs
@@ -3,4 +3,5 @@
 public static class FeatureFlagConstants {
     public const string EnableScribanOnly = "EnableScribanOnly";
     public const string EnableNodeDecommissionStrategy = "EnableNodeDecommissionStrategy";
+    public const string EnableValidateNotificationConfigSemantics = "EnableValidateNotificationConfigSemantics";
 }
diff --git a/src/ApiService/ApiService/OneFuzzTypes/Model.cs b/src/ApiService/ApiService/OneFuzzTypes/Model.cs
@@ -1,6 +1,7 @@
 using System.Reflection;
 using System.Text.Json;
 using System.Text.Json.Serialization;
+using System.Threading.Tasks;
 using Microsoft.OneFuzz.Service.OneFuzzLib.Orm;
 using Endpoint = System.String;
 using GroupId = System.Guid;
@@ -536,6 +537,7 @@ public RegressionReport Truncate(int maxLength) {
 #pragma warning disable CA1715
 public interface NotificationTemplate {
 #pragma warning restore CA1715
+    Async.Task<OneFuzzResultVoid> Validate();
 }
 
 
@@ -637,10 +639,19 @@ public record AdoTemplate(
     Dictionary<string, string> AdoFields,
     ADODuplicateTemplate OnDuplicate,
     string? Comment = null
-    ) : NotificationTemplate;
-
-public record TeamsTemplate(SecretData<string> Url) : NotificationTemplate;
+    ) : NotificationTemplate {
+    public async Task<OneFuzzResultVoid> Validate() {
+        return await Ado.Validate(this);
+    }
+}
 
+public record TeamsTemplate(SecretData<string> Url) : NotificationTemplate {
+    public Task<OneFuzzResultVoid> Validate() {
+        // The only way we can validate in the current state is to send a test webhook
+        // Maybe there's a teams nuget package we can pull in to help validate
+        return Async.Task.FromResult(OneFuzzResultVoid.Ok);
+    }
+}
 
 public record GithubAuth(string User, string PersonalAccessToken);
 
@@ -668,7 +679,11 @@ public record GithubIssuesTemplate(
     List<string> Assignees,
     List<string> Labels,
     GithubIssueDuplicate OnDuplicate
-    ) : NotificationTemplate;
+    ) : NotificationTemplate {
+    public async Task<OneFuzzResultVoid> Validate() {
+        return await GithubIssues.Validate(this);
+    }
+}
 
 public record Repro(
     [PartitionKey][RowKey] Guid VmId,

diff --git a/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs b/src/ApiService/ApiService/onefuzzlib/NotificationOperations.cs
@@ -96,6 +96,13 @@ public async Async.Task<OneFuzzResult<Notification>> Create(Container container,
             return OneFuzzResult<Notification>.Error(ErrorCode.INVALID_REQUEST, "The notification config is not a valid scriban template");
         }
 
+        if (await _context.FeatureManagerSnapshot.IsEnabledAsync(FeatureFlagConstants.EnableValidateNotificationConfigSemantics)) {
+            var validConfig = await config.Validate();
+            if (!validConfig.IsOk) {
+                return OneFuzzResult<Notification>.Error(validConfig.ErrorV);
+            }
+        }
+
         if (replaceExisting) {
             var existing = this.SearchByRowKeys(new[] { container.String });
             await foreach (var existingEntry in existing) {
@@ -138,7 +145,6 @@ private async Async.Task<NotificationTemplate> HideSecrets(NotificationTemplate
 
     public async Async.Task<Task?> GetRegressionReportTask(RegressionReport report) {
         if (report.CrashTestResult.CrashReport != null) {
-
             return await _context.TaskOperations.GetByJobIdAndTaskId(report.CrashTestResult.CrashReport.JobId, report.CrashTestResult.CrashReport.TaskId);
         }
         if (report.CrashTestResult.NoReproReport != null) {

diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs b/src/ApiService/ApiService/onefuzzlib/notifications/Ado.cs
@@ -2,13 +2,13 @@
 using Microsoft.TeamFoundation.WorkItemTracking.WebApi;
 using Microsoft.TeamFoundation.WorkItemTracking.WebApi.Models;
 using Microsoft.VisualStudio.Services.Common;
+using Microsoft.VisualStudio.Services.WebApi;
 using Microsoft.VisualStudio.Services.WebApi.Patch.Json;
 
 namespace Microsoft.OneFuzz.Service;
 
 public interface IAdo {
     public Async.Task NotifyAdo(AdoTemplate config, Container container, string filename, IReport reportable, bool isLastRetryAttempt, Guid notificationId);
-
 }
 
 public class Ado : NotificationsBase, IAdo {
@@ -61,6 +61,55 @@ private static bool IsTransient(Exception e) {
         return errorCodes.Any(code => errorStr.Contains(code));
     }
 
+    public static async Async.Task<OneFuzzResultVoid> Validate(AdoTemplate config) {
+        // Validate PAT is valid for the base url
+        VssConnection connection;
+        if (config.AuthToken.Secret is SecretValue<string> token) {
+            try {
+                connection = new VssConnection(config.BaseUrl, new VssBasicCredential(string.Empty, token.Value));
+                await connection.ConnectAsync();
+            } catch {
+                return OneFuzzResultVoid.Error(ErrorCode.INVALID_CONFIGURATION, $"Failed to connect to {config.BaseUrl} using the provided token");
+            }
+        } else {
+            return OneFuzzResultVoid.Error(ErrorCode.INVALID_CONFIGURATION, "Auth token is missing or invalid");
+        }
+
+        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);
+
+            var configFields = config.UniqueFields.Select(field => field.ToLowerInvariant()).ToHashSet();
+            var validConfigFields = configFields.Intersect(projectValidFields.Keys).ToHashSet();
+
+            if (!validConfigFields.SetEquals(configFields)) {
+                var invalidFields = configFields.Except(validConfigFields);
+                return OneFuzzResultVoid.Error(ErrorCode.INVALID_CONFIGURATION, new[]
+                    {
+                        $"The following unique fields are not valid fields for this project: {string.Join(',', invalidFields)}",
+                        "You can find the valid fields for your project by following these steps: https://learn.microsoft.com/en-us/azure/devops/boards/work-items/work-item-fields?view=azure-devops#review-fields"
+                    }
+                );
+            }
+        } catch {
+            return OneFuzzResultVoid.Error(ErrorCode.INVALID_CONFIGURATION, "Failed to query and compare the valid fields for this project");
+        }
+
+        return OneFuzzResultVoid.Ok;
+    }
+
+    private static WorkItemTrackingHttpClient GetAdoClient(Uri baseUrl, string token) {
+        return new WorkItemTrackingHttpClient(baseUrl, new VssBasicCredential("PAT", token));
+    }
+
+    private static async Async.Task<Dictionary<string, WorkItemField>> GetValidFields(WorkItemTrackingHttpClient client, string? project) {
+        return (await client.GetFieldsAsync(project, expand: GetFieldsExpand.ExtensionFields))
+            .ToDictionary(field => field.ReferenceName.ToLowerInvariant());
+    }
+
     sealed class AdoConnector {
         private readonly AdoTemplate _config;
         private readonly Renderer _renderer;
@@ -75,13 +124,11 @@ public static async Async.Task<AdoConnector> AdoConnectorCreator(IOnefuzzContext
 
             var authToken = await context.SecretsOperations.GetSecretStringValue(config.AuthToken);
             var client = GetAdoClient(config.BaseUrl, authToken!);
-            return new AdoConnector(container, filename, config, report, renderer, project!, client, instanceUrl, logTracer);
+            return new AdoConnector(config, renderer, project!, client, instanceUrl, logTracer);
         }
 
-        private static WorkItemTrackingHttpClient GetAdoClient(Uri baseUrl, string token) {
-            return new WorkItemTrackingHttpClient(baseUrl, new VssBasicCredential("PAT", token));
-        }
-        public AdoConnector(Container container, string filename, AdoTemplate config, Report report, Renderer renderer, string project, WorkItemTrackingHttpClient client, Uri instanceUrl, ILogTracer logTracer) {
+
+        public AdoConnector(AdoTemplate config, Renderer renderer, string project, WorkItemTrackingHttpClient client, Uri instanceUrl, ILogTracer logTracer) {
             _config = config;
             _renderer = renderer;
             _project = project;
@@ -112,7 +159,7 @@ public async IAsyncEnumerable<WorkItem> ExistingWorkItems() {
             }
 
             var project = filters.TryGetValue("system.teamproject", out var value) ? value : null;
-            var validFields = await GetValidFields(project);
+            var validFields = await GetValidFields(_client, project);
 
             var postQueryFilter = new Dictionary<string, string>();
             /*
@@ -235,11 +282,6 @@ public async Async.Task<bool> UpdateExisting(WorkItem item, (string, string)[] n
             return stateUpdated;
         }
 
-        private async Async.Task<Dictionary<string, WorkItemField>> GetValidFields(string? project) {
-            return (await _client.GetFieldsAsync(project, expand: GetFieldsExpand.ExtensionFields))
-                .ToDictionary(field => field.ReferenceName.ToLowerInvariant());
-        }
-
         private async Async.Task<WorkItem> CreateNew() {
             var (taskType, document) = await RenderNew();
             var entry = await _client.CreateWorkItemAsync(document, _project, taskType);

diff --git a/src/ApiService/ApiService/onefuzzlib/notifications/GithubIssues.cs b/src/ApiService/ApiService/onefuzzlib/notifications/GithubIssues.cs
@@ -30,6 +30,35 @@ public async Async.Task GithubIssue(GithubIssuesTemplate config, Container conta
         }
     }
 
+    public static async Async.Task<OneFuzzResultVoid> Validate(GithubIssuesTemplate config) {
+        // Validate PAT is valid
+        GitHubClient gh;
+        if (config.Auth.Secret is SecretValue<GithubAuth> auth) {
+            try {
+                gh = GetGitHubClient(auth.Value.User, auth.Value.PersonalAccessToken);
+                var _ = await gh.User.Get(auth.Value.User);
+            } catch {
+                return OneFuzzResultVoid.Error(ErrorCode.INVALID_CONFIGURATION, $"Failed to login to github.com with user {auth.Value.User} and the provided Personal Access Token");
+            }
+        } else {
+            return OneFuzzResultVoid.Error(ErrorCode.INVALID_CONFIGURATION, $"GithubAuth is missing or invalid");
+        }
+
+        try {
+            var _ = await gh.Repository.Get(config.Organization, config.Repository);
+        } catch {
+            return OneFuzzResultVoid.Error(ErrorCode.INVALID_CONFIGURATION, $"Failed to access repository: {config.Organization}/{config.Repository}");
+        }
+
+        return OneFuzzResultVoid.Ok;
+    }
+
+    private static GitHubClient GetGitHubClient(string user, string pat) {
+        return new GitHubClient(new ProductHeaderValue("OneFuzz")) {
+            Credentials = new Credentials(user, pat)
+        };
+    }
+
     private async Async.Task Process(GithubIssuesTemplate config, Container container, string filename, Report report) {
         var renderer = await Renderer.ConstructRenderer(_context, container, filename, report, _logTracer);
         var handler = await GithubConnnector.GithubConnnectorCreator(config, container, filename, renderer, _context.Creds.GetInstanceUrl(), _context, _logTracer);
@@ -48,14 +77,12 @@ public static async Async.Task<GithubConnnector> GithubConnnectorCreator(GithubI
                 SecretValue<GithubAuth> sv => sv.Value,
                 _ => throw new ArgumentException($"Unexpected secret type {config.Auth.Secret.GetType()}")
             };
-            return new GithubConnnector(config, container, filename, renderer, instanceUrl, auth!, logTracer);
+            return new GithubConnnector(config, renderer, instanceUrl, auth!, logTracer);
         }
 
-        public GithubConnnector(GithubIssuesTemplate config, Container container, string filename, Renderer renderer, Uri instanceUrl, GithubAuth auth, ILogTracer logTracer) {
+        public GithubConnnector(GithubIssuesTemplate config, Renderer renderer, Uri instanceUrl, GithubAuth auth, ILogTracer logTracer) {
             _config = config;
-            _gh = new GitHubClient(new ProductHeaderValue("OneFuzz")) {
-                Credentials = new Credentials(auth.User, auth.PersonalAccessToken)
-            };
+            _gh = GetGitHubClient(auth.User, auth.PersonalAccessToken);
             _renderer = renderer;
             _instanceUrl = instanceUrl;
             _logTracer = logTracer;

diff --git a/src/deployment/bicep-templates/feature-flags.bicep b/src/deployment/bicep-templates/feature-flags.bicep
@@ -24,4 +24,17 @@ resource configStoreFeatureflag 'Microsoft.AppConfiguration/configurationStores/
   }
 }
 
+resource validateNotificationConfigSemantics 'Microsoft.AppConfiguration/configurationStores/keyValues@2021-10-01-preview' = {
+  parent: featureFlags
+  name: '.appconfig.featureflag~2FEnableValidateNotificationConfigSemantics'
+  properties: {
+    value: string({
+      id: 'EnableScribanOnly'
+      description: 'Check notification configs for valid PATs and fields'
+      enabled: true
+    })
+    contentType: 'application/vnd.microsoft.appconfig.ff+json;charset=utf-8'
+  }
+}
+
 output AppConfigEndpoint string = 'https://${appConfigName}.azconfig.io'