From 85404f12d00cb997eb808ed86f7a7030984c00e2 Mon Sep 17 00:00:00 2001 From: BobSilent Date: Mon, 5 Aug 2019 23:36:07 +0200 Subject: [PATCH 01/20] inital changes in directives parser --- src/aggregator-ruleng/DirectivesParser.cs | 26 +++++++++++++++++------ src/aggregator-ruleng/RuleEngine.cs | 2 ++ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/aggregator-ruleng/DirectivesParser.cs b/src/aggregator-ruleng/DirectivesParser.cs index d10ccb9f..efe866f6 100644 --- a/src/aggregator-ruleng/DirectivesParser.cs +++ b/src/aggregator-ruleng/DirectivesParser.cs @@ -9,11 +9,11 @@ namespace aggregator.Engine /// internal class DirectivesParser { - int firstCodeLine = 0; + private int firstCodeLine = 0; private readonly IAggregatorLogger logger; - string[] ruleCode; - List references = new List(); - List imports = new List(); + private readonly string[] ruleCode; + private readonly List references = new List(); + private readonly List imports = new List(); internal DirectivesParser(IAggregatorLogger logger, string[] ruleCode) { @@ -21,6 +21,7 @@ internal DirectivesParser(IAggregatorLogger logger, string[] ruleCode) this.logger = logger; //defaults + Impersonate = false; Language = Languages.Csharp; References = references; Imports = imports; @@ -91,6 +92,18 @@ internal bool Parse() } break; + case "onbehalfofinitiator": + if (parts.Length < 2) + { + Impersonate = true; + } + else + { + logger.WriteWarning($"Invalid import directive {directive}"); + return false; + } + break; + default: logger.WriteWarning($"Unrecognized directive {directive}"); return false; @@ -117,8 +130,9 @@ internal enum Languages Csharp } + internal bool Impersonate { get; private set; } internal Languages Language { get; private set; } - internal IReadOnlyList References { get; private set; } - internal IReadOnlyList Imports { get; private set; } + internal IReadOnlyList References { get; } + internal IReadOnlyList Imports { get; } } } diff --git a/src/aggregator-ruleng/RuleEngine.cs b/src/aggregator-ruleng/RuleEngine.cs index ffb09873..f8c60e64 100644 --- a/src/aggregator-ruleng/RuleEngine.cs +++ b/src/aggregator-ruleng/RuleEngine.cs @@ -25,6 +25,7 @@ public class RuleEngine private readonly IAggregatorLogger logger; private readonly Script roslynScript; private readonly SaveMode saveMode; + private readonly bool impersonateChanges; public RuleEngine(IAggregatorLogger logger, string[] ruleCode, SaveMode mode, bool dryRun) { @@ -43,6 +44,7 @@ public RuleEngine(IAggregatorLogger logger, string[] ruleCode, SaveMode mode, bo if (directives.Language == DirectivesParser.Languages.Csharp) { + impersonateChanges = directives.Impersonate; var references = LoadReferences(directives); var imports = GetImports(directives); From 1cdb4a82924d30a98a6ac6e09098c146b43ad264 Mon Sep 17 00:00:00 2001 From: BobSilent Date: Mon, 2 Sep 2019 13:32:42 +0200 Subject: [PATCH 02/20] provide impersonation in workitemstore --- src/aggregator-ruleng/CoreFieldRefNames.cs | 3 +- src/aggregator-ruleng/RuleEngine.cs | 2 +- src/aggregator-ruleng/WorkItemStore.cs | 51 +++++++++++++++++----- src/aggregator-ruleng/WorkItemWrapper.cs | 6 +++ src/unittests-ruleng/WorkItemStoreTests.cs | 2 +- 5 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/aggregator-ruleng/CoreFieldRefNames.cs b/src/aggregator-ruleng/CoreFieldRefNames.cs index 337c5247..b3a96536 100644 --- a/src/aggregator-ruleng/CoreFieldRefNames.cs +++ b/src/aggregator-ruleng/CoreFieldRefNames.cs @@ -1,5 +1,5 @@ /* - * TODO replace this class with references to + * TODO replace this class with references to using Microsoft.TeamFoundation.WorkItemTracking.Common; using Microsoft.TeamFoundation.WorkItemTracking.Common.Constants; */ @@ -31,6 +31,7 @@ internal class CoreFieldRefNames public const string IterationPath = "System.IterationPath"; public const string Reason = "System.Reason"; public const string RelatedLinkCount = "System.RelatedLinkCount"; + public const string RevisedBy = "System.RevisedBy"; public const string RevisedDate = "System.RevisedDate"; public const string AuthorizedDate = "System.AuthorizedDate"; public const string Tags = "System.Tags"; diff --git a/src/aggregator-ruleng/RuleEngine.cs b/src/aggregator-ruleng/RuleEngine.cs index f8c60e64..b150c83b 100644 --- a/src/aggregator-ruleng/RuleEngine.cs +++ b/src/aggregator-ruleng/RuleEngine.cs @@ -142,7 +142,7 @@ public async Task ExecuteAsync(Guid projectId, WorkItemData workItemPayl } logger.WriteVerbose($"Post-execution, save any change (mode {saveMode})..."); - var saveRes = await store.SaveChanges(saveMode, !DryRun, cancellationToken); + var saveRes = await store.SaveChanges(saveMode, !DryRun, impersonateChanges, cancellationToken); if (saveRes.created + saveRes.updated > 0) { logger.WriteInfo($"Changes saved to Azure DevOps (mode {saveMode}): {saveRes.created} created, {saveRes.updated} updated."); diff --git a/src/aggregator-ruleng/WorkItemStore.cs b/src/aggregator-ruleng/WorkItemStore.cs index 9a9ab51f..18c6be6c 100644 --- a/src/aggregator-ruleng/WorkItemStore.cs +++ b/src/aggregator-ruleng/WorkItemStore.cs @@ -8,6 +8,7 @@ using System.Threading.Tasks; using Microsoft.TeamFoundation.Work.WebApi.Contracts; +using Microsoft.VisualStudio.Services.WebApi; using Microsoft.VisualStudio.Services.WebApi.Patch.Json; @@ -20,6 +21,9 @@ public class WorkItemStore private readonly Lazy>> _lazyGetWorkItemCategories; private readonly Lazy>> _lazyGetBacklogWorkItemTypesAndStates; + private readonly IdentityRef _polluterIdentity; + + public WorkItemStore(EngineContext context) { _context = context; @@ -31,7 +35,9 @@ public WorkItemStore(EngineContext context) public WorkItemStore(EngineContext context, WorkItem workItem) : this(context) { //initialize tracker with initial work item - _ = new WorkItemWrapper(_context, workItem); + var wrapper = new WorkItemWrapper(_context, workItem); + //store initiator identity + _polluterIdentity = wrapper.RevisedBy; } public WorkItemWrapper GetWorkItem(int id) @@ -130,28 +136,47 @@ public async Task> GetBacklogWorkItemType return await _lazyGetBacklogWorkItemTypesAndStates.Value; } - public async Task<(int created, int updated)> SaveChanges(SaveMode mode, bool commit, CancellationToken cancellationToken) + + private void ImpersonateChanges() + { + var workItems = _context.Tracker.GetChangedWorkItems(); + + var changedWorkItems = workItems.Created + .Concat(workItems.Updated); + + foreach (var workItem in changedWorkItems) + { + workItem.ChangedBy = _polluterIdentity; + } + } + + public async Task<(int created, int updated)> SaveChanges(SaveMode mode, bool commit, bool impersonate, CancellationToken cancellationToken) { + if (impersonate) + { + ImpersonateChanges(); + } + switch (mode) { case SaveMode.Default: _context.Logger.WriteVerbose($"No save mode specified, assuming {SaveMode.TwoPhases}."); goto case SaveMode.TwoPhases; case SaveMode.Item: - var resultItem = await SaveChanges_ByItem(commit, cancellationToken); + var resultItem = await SaveChanges_ByItem(commit, impersonate, cancellationToken); return resultItem; case SaveMode.Batch: - var resultBatch = await SaveChanges_Batch(commit, cancellationToken); + var resultBatch = await SaveChanges_Batch(commit, impersonate, cancellationToken); return resultBatch; case SaveMode.TwoPhases: - var resultTwoPhases = await SaveChanges_TwoPhases(commit, cancellationToken); + var resultTwoPhases = await SaveChanges_TwoPhases(commit, impersonate, cancellationToken); return resultTwoPhases; default: throw new InvalidOperationException($"Unsupported save mode: {mode}."); } } - private async Task<(int created, int updated)> SaveChanges_ByItem(bool commit, CancellationToken cancellationToken) + private async Task<(int created, int updated)> SaveChanges_ByItem(bool commit, bool impersonate, CancellationToken cancellationToken) { int created = 0; int updated = 0; @@ -166,6 +191,7 @@ public async Task> GetBacklogWorkItemType item.Changes, _context.ProjectName, item.WorkItemType, + bypassRules: impersonate, cancellationToken: cancellationToken ); } @@ -198,6 +224,7 @@ public async Task> GetBacklogWorkItemType _ = await _clients.WitClient.UpdateWorkItemAsync( item.Changes, item.Id, + bypassRules: impersonate, cancellationToken: cancellationToken ); } @@ -212,7 +239,7 @@ public async Task> GetBacklogWorkItemType return (created, updated); } - private async Task<(int created, int updated)> SaveChanges_Batch(bool commit, CancellationToken cancellationToken) + private async Task<(int created, int updated)> SaveChanges_Batch(bool commit, bool impersonate, CancellationToken cancellationToken) { // see https://github.com/redarrowlabs/vsts-restapi-samplecode/blob/master/VSTSRestApiSamples/WorkItemTracking/Batch.cs // and https://docs.microsoft.com/en-us/rest/api/vsts/wit/workitembatchupdate?view=vsts-rest-4.1 @@ -230,7 +257,7 @@ public async Task> GetBacklogWorkItemType var request = _clients.WitClient.CreateWorkItemBatchRequest(_context.ProjectName, item.WorkItemType, item.Changes, - bypassRules: false, + bypassRules: impersonate, suppressNotifications: false); batchRequests.Add(request); } @@ -241,7 +268,7 @@ public async Task> GetBacklogWorkItemType var request = _clients.WitClient.CreateWorkItemBatchRequest(item.Id, item.Changes, - bypassRules: false, + bypassRules: impersonate, suppressNotifications: false); batchRequests.Add(request); } @@ -270,7 +297,7 @@ private static bool IsSuccessStatusCode(int statusCode) //TODO no error handling here? SaveChanges_Batch has at least the DryRun support and error handling //TODO Improve complex handling with ReplaceIdAndResetChanges and RemapIdReferences - private async Task<(int created, int updated)> SaveChanges_TwoPhases(bool commit, CancellationToken cancellationToken) + private async Task<(int created, int updated)> SaveChanges_TwoPhases(bool commit, bool impersonate, CancellationToken cancellationToken) { // see https://github.com/redarrowlabs/vsts-restapi-samplecode/blob/master/VSTSRestApiSamples/WorkItemTracking/Batch.cs // and https://docs.microsoft.com/en-us/rest/api/vsts/wit/workitembatchupdate?view=vsts-rest-4.1 @@ -298,7 +325,7 @@ private static bool IsSuccessStatusCode(int statusCode) var request = _clients.WitClient.CreateWorkItemBatchRequest(_context.ProjectName, item.WorkItemType, document, - bypassRules: false, + bypassRules: impersonate, suppressNotifications: false); batchRequests.Add(request); } @@ -330,7 +357,7 @@ private static bool IsSuccessStatusCode(int statusCode) var request = _clients.WitClient.CreateWorkItemBatchRequest(item.Id, item.Changes, - bypassRules: false, + bypassRules: impersonate, suppressNotifications: false); batchRequests.Add(request); } diff --git a/src/aggregator-ruleng/WorkItemWrapper.cs b/src/aggregator-ruleng/WorkItemWrapper.cs index 16bf9dfe..a62b4fcd 100644 --- a/src/aggregator-ruleng/WorkItemWrapper.cs +++ b/src/aggregator-ruleng/WorkItemWrapper.cs @@ -292,6 +292,12 @@ public int RelatedLinkCount set => SetFieldValue(CoreFieldRefNames.RelatedLinkCount, value); } + public IdentityRef RevisedBy + { + get => GetFieldValue(CoreFieldRefNames.RevisedBy); + set => SetFieldValue(CoreFieldRefNames.RevisedBy, value); + } + public DateTime? RevisedDate { get => GetFieldValue(CoreFieldRefNames.RevisedDate); diff --git a/src/unittests-ruleng/WorkItemStoreTests.cs b/src/unittests-ruleng/WorkItemStoreTests.cs index 62d652af..c9b12153 100644 --- a/src/unittests-ruleng/WorkItemStoreTests.cs +++ b/src/unittests-ruleng/WorkItemStoreTests.cs @@ -90,7 +90,7 @@ public async Task NewWorkItem_Succeeds() var wi = sut.NewWorkItem("Task"); wi.Title = "Brand new"; - var save = await sut.SaveChanges(SaveMode.Default, false, CancellationToken.None); + var save = await sut.SaveChanges(SaveMode.Default, false, false, CancellationToken.None); Assert.NotNull(wi); Assert.True(wi.IsNew); From 857d27f87f3a9b671f97cb20f1b7894132720c0d Mon Sep 17 00:00:00 2001 From: BobSilent Date: Mon, 2 Sep 2019 17:30:28 +0200 Subject: [PATCH 03/20] beautify rulewrapper --- src/aggregator-cli/Rules/AggregatorRules.cs | 2 +- src/aggregator-function/RuleWrapper.cs | 45 +++++++++++++-------- 2 files changed, 29 insertions(+), 18 deletions(-) diff --git a/src/aggregator-cli/Rules/AggregatorRules.cs b/src/aggregator-cli/Rules/AggregatorRules.cs index 6d743dcf..7d234a8a 100644 --- a/src/aggregator-cli/Rules/AggregatorRules.cs +++ b/src/aggregator-cli/Rules/AggregatorRules.cs @@ -96,7 +96,7 @@ internal async Task AddAsync(InstanceName instance, string name, string fi { _logger.WriteInfo($"Validate rule file {filePath}"); - var ruleContent = await File.ReadAllLinesAsync(filePath); + var ruleContent = await File.ReadAllLinesAsync(filePath, cancellationToken); var engineLogger = new EngineWrapperLogger(_logger); try diff --git a/src/aggregator-function/RuleWrapper.cs b/src/aggregator-function/RuleWrapper.cs index 9e163b35..9b88c62f 100644 --- a/src/aggregator-function/RuleWrapper.cs +++ b/src/aggregator-function/RuleWrapper.cs @@ -50,20 +50,12 @@ internal async Task ExecuteAsync(WorkItemEventContext eventContext, Canc logger.WriteInfo($"Connected to Azure DevOps"); using (var clientsContext = new AzureDevOpsClientsContext(devops)) { - string ruleFilePath = Path.Combine(functionDirectory, $"{ruleName}.rule"); - if (!File.Exists(ruleFilePath)) + string ruleFilePath = GetRuleFilePath(); + if (string.IsNullOrWhiteSpace(ruleFilePath)) { - logger.WriteError($"Rule code not found at {ruleFilePath}"); return "Rule file not found!"; } - - logger.WriteVerbose($"Rule code found at {ruleFilePath}"); - string[] ruleCode; - using (var fileStream = File.OpenRead(ruleFilePath)) - { - var reader = new StreamReader(fileStream); - ruleCode = await ReadAllLinesAsync(reader); - } + string[] ruleCode = await ReadAllLinesAsync(ruleFilePath); var engine = new Engine.RuleEngine(logger, ruleCode, configuration.SaveMode, configuration.DryRun); @@ -72,16 +64,35 @@ internal async Task ExecuteAsync(WorkItemEventContext eventContext, Canc } } - private static async Task ReadAllLinesAsync(TextReader streamReader) + private static async Task ReadAllLinesAsync(string ruleFilePath) + { + using (var fileStream = File.OpenRead(ruleFilePath)) + { + using (var streamReader = new StreamReader(fileStream)) + { + var lines = new List(); + string line; + while ((line = await streamReader.ReadLineAsync()) != null) + { + lines.Add(line); + } + + return lines.ToArray(); + } + } + } + + private string GetRuleFilePath() { - var lines = new List(); - string line; - while ((line = await streamReader.ReadLineAsync()) != null) + string ruleFilePath = Path.Combine(functionDirectory, $"{ruleName}.rule"); + if (!File.Exists(ruleFilePath)) { - lines.Add(line); + logger.WriteError($"Rule code not found at {ruleFilePath}"); + return string.Empty; } - return lines.ToArray(); + logger.WriteVerbose($"Rule code found at {ruleFilePath}"); + return ruleFilePath; } } } From e0ff5219a710d27e8d8abafe202a775709e3fee8 Mon Sep 17 00:00:00 2001 From: BobSilent Date: Tue, 3 Sep 2019 18:10:01 +0200 Subject: [PATCH 04/20] Parse impersonation directive from rulecode --- src/aggregator-ruleng/DirectivesParser.cs | 138 ----------------- src/aggregator-ruleng/IAggregatorLogger.cs | 30 ++++ .../Language/IRuleDirectives.cs | 13 ++ .../Language/RuleDirectives.cs | 34 +++++ .../Language/RuleDirectivesExtensions.cs | 51 +++++++ .../Language/RuleFileParser.cs | 140 ++++++++++++++++++ .../Language/RuleLanguage.cs | 7 + src/aggregator-ruleng/RuleEngine.cs | 79 +++++----- src/unittests-ruleng/RuleFileParserTests.cs | 53 +++++++ 9 files changed, 367 insertions(+), 178 deletions(-) delete mode 100644 src/aggregator-ruleng/DirectivesParser.cs create mode 100644 src/aggregator-ruleng/Language/IRuleDirectives.cs create mode 100644 src/aggregator-ruleng/Language/RuleDirectives.cs create mode 100644 src/aggregator-ruleng/Language/RuleDirectivesExtensions.cs create mode 100644 src/aggregator-ruleng/Language/RuleFileParser.cs create mode 100644 src/aggregator-ruleng/Language/RuleLanguage.cs create mode 100644 src/unittests-ruleng/RuleFileParserTests.cs diff --git a/src/aggregator-ruleng/DirectivesParser.cs b/src/aggregator-ruleng/DirectivesParser.cs deleted file mode 100644 index efe866f6..00000000 --- a/src/aggregator-ruleng/DirectivesParser.cs +++ /dev/null @@ -1,138 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Text; - -namespace aggregator.Engine -{ - /// - /// Scan the top lines of a script looking for directives, whose lines start with a . - /// - internal class DirectivesParser - { - private int firstCodeLine = 0; - private readonly IAggregatorLogger logger; - private readonly string[] ruleCode; - private readonly List references = new List(); - private readonly List imports = new List(); - - internal DirectivesParser(IAggregatorLogger logger, string[] ruleCode) - { - this.ruleCode = ruleCode; - this.logger = logger; - - //defaults - Impersonate = false; - Language = Languages.Csharp; - References = references; - Imports = imports; - } - - /// - /// Grab directives - /// - internal bool Parse() - { - while (firstCodeLine < ruleCode.Length - && ruleCode[firstCodeLine].Length > 0 - && ruleCode[firstCodeLine][0] == '.') - { - string directive = ruleCode[firstCodeLine].Substring(1); - var parts = directive.Split('='); - - switch (parts[0].ToLowerInvariant()) - { - case "lang": - case "language": - if (parts.Length < 2) - { - logger.WriteWarning($"Unrecognized directive {directive}"); - return false; - } - else - { - switch (parts[1].ToUpperInvariant()) - { - case "C#": - case "CS": - case "CSHARP": - Language = Languages.Csharp; - break; - default: - logger.WriteWarning($"Unrecognized language {parts[1]}"); - return false; - } - } - break; - - case "r": - case "ref": - case "reference": - if (parts.Length < 2) - { - logger.WriteWarning($"Invalid reference directive {directive}"); - return false; - } - else - { - references.Add(parts[1]); - } - break; - - case "import": - case "imports": - case "namespace": - if (parts.Length < 2) - { - logger.WriteWarning($"Invalid import directive {directive}"); - return false; - } - else - { - imports.Add(parts[1]); - } - break; - - case "onbehalfofinitiator": - if (parts.Length < 2) - { - Impersonate = true; - } - else - { - logger.WriteWarning($"Invalid import directive {directive}"); - return false; - } - break; - - default: - logger.WriteWarning($"Unrecognized directive {directive}"); - return false; - }//switch - - firstCodeLine++; - }//while - return true; - } - - internal string GetRuleCode() - { - StringBuilder sb = new StringBuilder(); - // Keep directive lines commented out, to maintain source location of rule code for diagnostics. - for(int i=0; i References { get; } - internal IReadOnlyList Imports { get; } - } -} diff --git a/src/aggregator-ruleng/IAggregatorLogger.cs b/src/aggregator-ruleng/IAggregatorLogger.cs index c7214022..4a329b04 100644 --- a/src/aggregator-ruleng/IAggregatorLogger.cs +++ b/src/aggregator-ruleng/IAggregatorLogger.cs @@ -14,4 +14,34 @@ public interface IAggregatorLogger void WriteError(string message); } + + public static class AggregatorLoggerExtensions + { + public static void WriteVerbose(this IAggregatorLogger logger, string[] messages) + { + WriteMultiple(logger.WriteVerbose, messages); + } + public static void WriteInfo(this IAggregatorLogger logger, string[] messages) + { + WriteMultiple(logger.WriteInfo, messages); + } + + public static void WriteWarning(this IAggregatorLogger logger, string[] messages) + { + WriteMultiple(logger.WriteWarning, messages); + } + + public static void WriteError(this IAggregatorLogger logger, string[] messages) + { + WriteMultiple(logger.WriteError, messages); + } + + private static void WriteMultiple(Action logAction, string[] messages) + { + foreach (var message in messages) + { + logAction(message); + } + } + } } diff --git a/src/aggregator-ruleng/Language/IRuleDirectives.cs b/src/aggregator-ruleng/Language/IRuleDirectives.cs new file mode 100644 index 00000000..25b37b85 --- /dev/null +++ b/src/aggregator-ruleng/Language/IRuleDirectives.cs @@ -0,0 +1,13 @@ +using System.Collections.Generic; + + +namespace aggregator.Engine.Language { + public interface IRuleDirectives + { + bool Impersonate { get; } + RuleLanguage Language { get; } + IReadOnlyList References { get; } + IReadOnlyList Imports { get; } + IReadOnlyList RuleCode { get; } + } +} \ No newline at end of file diff --git a/src/aggregator-ruleng/Language/RuleDirectives.cs b/src/aggregator-ruleng/Language/RuleDirectives.cs new file mode 100644 index 00000000..dde335dc --- /dev/null +++ b/src/aggregator-ruleng/Language/RuleDirectives.cs @@ -0,0 +1,34 @@ +using System.Collections.Generic; +using System.Collections.ObjectModel; + + +namespace aggregator.Engine.Language { + internal class RuleDirectives : IRuleDirectives + { + public RuleDirectives() + { + Impersonate = false; + Language = RuleLanguage.Unknown; + References = new List(); + Imports = new List(); + RuleCode = new List(); + } + + public bool Impersonate { get; internal set; } + + public RuleLanguage Language { get; internal set; } + + IReadOnlyList IRuleDirectives.References => new ReadOnlyCollection(References); + + IReadOnlyList IRuleDirectives.Imports => new ReadOnlyCollection(Imports); + + IReadOnlyList IRuleDirectives.RuleCode => new ReadOnlyCollection(RuleCode); + + + public IList References { get; } + + public IList Imports { get; } + + public IList RuleCode { get; } + } +} \ No newline at end of file diff --git a/src/aggregator-ruleng/Language/RuleDirectivesExtensions.cs b/src/aggregator-ruleng/Language/RuleDirectivesExtensions.cs new file mode 100644 index 00000000..4f6e0246 --- /dev/null +++ b/src/aggregator-ruleng/Language/RuleDirectivesExtensions.cs @@ -0,0 +1,51 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; + + +namespace aggregator.Engine.Language { + public static class RuleDirectivesExtensions + { + internal static string GetRuleCode(this IRuleDirectives ruleDirectives) + { + return string.Join(Environment.NewLine, ruleDirectives.RuleCode); + } + + internal static bool IsValid(this IRuleDirectives ruleDirectives) + { + return ruleDirectives.IsSupportedLanguage() && ruleDirectives.RuleCode.Any(); + } + + internal static bool IsCSharp(this IRuleDirectives ruleDirectives) + { + return ruleDirectives.Language == RuleLanguage.Csharp; + } + + internal static bool IsSupportedLanguage(this IRuleDirectives ruleDirectives) + { + return ruleDirectives.Language != RuleLanguage.Unknown; + } + + + internal static string LanguageAsString(this IRuleDirectives ruleDirectives) + { + switch (ruleDirectives.Language) + { + case RuleLanguage.Csharp: + return "C#"; + case RuleLanguage.Unknown: + return "Unknown"; + default: + throw new ArgumentOutOfRangeException(); + } + } + + internal static IEnumerable LoadAssemblyReferences(this IRuleDirectives ruleDirectives) + { + return ruleDirectives.References + .Select(reference => new AssemblyName(reference)) + .Select(Assembly.Load); + } + } +} \ No newline at end of file diff --git a/src/aggregator-ruleng/Language/RuleFileParser.cs b/src/aggregator-ruleng/Language/RuleFileParser.cs new file mode 100644 index 00000000..eee1e088 --- /dev/null +++ b/src/aggregator-ruleng/Language/RuleFileParser.cs @@ -0,0 +1,140 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; + +using Microsoft.VisualStudio.Services.Common; + + +namespace aggregator.Engine.Language +{ + /// + /// Scan the top lines of a script looking for directives, whose lines start with a dot + /// + public static class RuleFileParser + { + public static (IRuleDirectives ruleDirectives, bool result, string[] messages) ReadFile(string ruleFilePath) + { + var content = File.ReadAllLines(ruleFilePath); + return Read(content); + } + + /// + /// Grab directives + /// + internal static (IRuleDirectives ruleDirectives, bool parseSuccess, string[] messages) Read(string[] ruleCode) + { + var messages = new List(); + var directiveLineIndex = 0; + var ruleDirectives = new RuleDirectives() + { + Language = RuleLanguage.Csharp + }; + + while (directiveLineIndex < ruleCode.Length + && ruleCode[directiveLineIndex].Length > 0 + && ruleCode[directiveLineIndex][0] == '.') + { + string directive = ruleCode[directiveLineIndex].Substring(1); + var parts = directive.Split('='); + + switch (parts[0].ToLowerInvariant()) + { + case "lang": + case "language": + if (parts.Length < 2) + { + messages.Add($"Invalid language directive {directive}"); + ruleDirectives.Language = RuleLanguage.Unknown; + } + else + { + switch (parts[1].ToUpperInvariant()) + { + case "C#": + case "CS": + case "CSHARP": + ruleDirectives.Language = RuleLanguage.Csharp; + break; + default: + { + messages.Add($"Unrecognized language {parts[1]}"); + ruleDirectives.Language = RuleLanguage.Unknown; + break; + } + } + } + break; + + case "r": + case "ref": + case "reference": + if (parts.Length < 2) + { + messages.Add($"Invalid reference directive {directive}"); + } + else + { + ruleDirectives.References.Add(parts[1]); + } + break; + + case "import": + case "imports": + case "namespace": + if (parts.Length < 2) + { + messages.Add($"Invalid import directive {directive}"); + } + else + { + ruleDirectives.Imports.Add(parts[1]); + } + break; + + case "onbehalfofinitiator": + if (parts.Length < 2) + { + ruleDirectives.Impersonate = true; + } + else + { + messages.Add($"Invalid onbehalfofinitiator directive {directive}"); + } + break; + + default: + { + messages.Add($"Unrecognized directive {directive}"); + break; + } + }//switch + + directiveLineIndex++; + }//while + + ruleDirectives.RuleCode.AddRange(ruleCode.Skip(directiveLineIndex)); + return (ruleDirectives, messages.Count == 0, messages.ToArray()); + } + + public static void WriteFile(string ruleFilePath, IRuleDirectives ruleDirectives) + { + var content = Write(ruleDirectives); + + File.WriteAllLines(ruleFilePath, content); + } + + public static string[] Write(IRuleDirectives ruleDirectives) + { + var content = new List + { + ruleDirectives.LanguageAsString() + }; + + content.AddRange(ruleDirectives.References.Select(reference => $".reference={reference}")); + content.AddRange(ruleDirectives.Imports.Select(import => $".import={import}")); + content.AddRange(ruleDirectives.RuleCode); + + return content.ToArray(); + } + } +} diff --git a/src/aggregator-ruleng/Language/RuleLanguage.cs b/src/aggregator-ruleng/Language/RuleLanguage.cs new file mode 100644 index 00000000..dbe2ebce --- /dev/null +++ b/src/aggregator-ruleng/Language/RuleLanguage.cs @@ -0,0 +1,7 @@ +namespace aggregator.Engine.Language { + public enum RuleLanguage + { + Csharp, + Unknown + } +} \ No newline at end of file diff --git a/src/aggregator-ruleng/RuleEngine.cs b/src/aggregator-ruleng/RuleEngine.cs index b150c83b..39c59278 100644 --- a/src/aggregator-ruleng/RuleEngine.cs +++ b/src/aggregator-ruleng/RuleEngine.cs @@ -5,6 +5,9 @@ using System.Reflection; using System.Threading; using System.Threading.Tasks; + +using aggregator.Engine.Language; + using Microsoft.CodeAnalysis.CSharp.Scripting; using Microsoft.CodeAnalysis.Scripting; @@ -35,27 +38,29 @@ public RuleEngine(IAggregatorLogger logger, string[] ruleCode, SaveMode mode, bo this.saveMode = mode; this.DryRun = dryRun; - var directives = new DirectivesParser(logger, ruleCode); - if (!directives.Parse()) + (IRuleDirectives ruleDirectives, bool success, string[] messages) = RuleFileParser.Read(ruleCode); + if (!success || !ruleDirectives.IsValid()) { + logger.WriteWarning(messages.Any() ? $"RuleFileParser issues: {string.Join(",",messages)}" : "No RuleFileParser issues found!"); State = EngineState.Error; return; } - if (directives.Language == DirectivesParser.Languages.Csharp) - { - impersonateChanges = directives.Impersonate; - var references = LoadReferences(directives); - var imports = GetImports(directives); + impersonateChanges = ruleDirectives.Impersonate; + + var references = new HashSet(DefaultAssemblyReferences().Concat(ruleDirectives.LoadAssemblyReferences())); + var imports = new HashSet(DefaultImports().Concat(ruleDirectives.Imports)); - var scriptOptions = ScriptOptions.Default - .WithEmitDebugInformation(true) - .WithReferences(references) - // Add namespaces - .WithImports(imports); + var scriptOptions = ScriptOptions.Default + .WithEmitDebugInformation(true) + .WithReferences(references) + // Add namespaces + .WithImports(imports); + if (ruleDirectives.IsCSharp()) + { this.roslynScript = CSharpScript.Create( - code: directives.GetRuleCode(), + code: ruleDirectives.GetRuleCode(), options: scriptOptions, globalsType: typeof(Globals)); } @@ -66,38 +71,32 @@ public RuleEngine(IAggregatorLogger logger, string[] ruleCode, SaveMode mode, bo } } - private static IEnumerable LoadReferences(DirectivesParser directives) + private static IEnumerable DefaultAssemblyReferences() { - var types = new List() { - typeof(object), - typeof(System.Linq.Enumerable), - typeof(System.Collections.Generic.CollectionExtensions), - typeof(Microsoft.VisualStudio.Services.WebApi.IdentityRef), - typeof(WorkItemWrapper) - }; - var references = types.ConvertAll(t => t.Assembly); - // user references - foreach (var reference in directives.References) - { - var name = new AssemblyName(reference); - references.Add(Assembly.Load(name)); - } - - return references.Distinct(); + var types = new List() + { + typeof(object), + typeof(System.Linq.Enumerable), + typeof(System.Collections.Generic.CollectionExtensions), + typeof(Microsoft.VisualStudio.Services.WebApi.IdentityRef), + typeof(WorkItemWrapper) + }; + + return types.Select(t => t.Assembly); } - private static IEnumerable GetImports(DirectivesParser directives) + private static IEnumerable DefaultImports() { var imports = new List - { - "System", - "System.Linq", - "System.Collections.Generic", - "Microsoft.VisualStudio.Services.WebApi", - "aggregator.Engine" - }; - imports.AddRange(directives.Imports); - return imports.Distinct(); + { + "System", + "System.Linq", + "System.Collections.Generic", + "Microsoft.VisualStudio.Services.WebApi", + "aggregator.Engine" + }; + + return imports; } /// diff --git a/src/unittests-ruleng/RuleFileParserTests.cs b/src/unittests-ruleng/RuleFileParserTests.cs new file mode 100644 index 00000000..2d41c73b --- /dev/null +++ b/src/unittests-ruleng/RuleFileParserTests.cs @@ -0,0 +1,53 @@ +using System; +using System.Collections.Generic; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +using aggregator; +using aggregator.Engine; +using aggregator.Engine.Language; + +using Microsoft.TeamFoundation.WorkItemTracking.WebApi.Models; + +using NSubstitute; + +using Xunit; + + +namespace unittests_ruleng +{ + public class RuleFileParserTests + { + [Fact] + public void RuleLanguageDefaultsCSharp_Succeeds() + { + + string ruleCode = @" +return $""Hello { self.WorkItemType } #{ self.Id } - { self.Title }!""; +"; + + (IRuleDirectives directives, bool _, string[] __) = RuleFileParser.Read(ruleCode.Mince()); + + Assert.Empty(directives.References); + Assert.Empty(directives.Imports); + Assert.NotEmpty(directives.RuleCode); + Assert.Equal(RuleLanguage.Csharp, directives.Language); + } + + [Theory] + [InlineData(".language=CSharp", RuleLanguage.Csharp, true)] + [InlineData(".language=C#", RuleLanguage.Csharp, true)] + [InlineData(".language=CS", RuleLanguage.Csharp, true)] + [InlineData(".language=", RuleLanguage.Unknown, false)] + + public void RuleLanguageDirectiveParse_Succeeds(string ruleCode, RuleLanguage expectedLanguage, bool expectedParsingSuccess) + { + (IRuleDirectives directives, bool parsingSuccess, string[] __) = RuleFileParser.Read(ruleCode.Mince()); + + Assert.Equal(expectedLanguage, directives.Language); + Assert.Equal(expectedParsingSuccess, parsingSuccess); + } + + } +} From d5e0506662061db0f6104e380b396cde1673af8e Mon Sep 17 00:00:00 2001 From: BobSilent Date: Wed, 4 Sep 2019 10:42:37 +0200 Subject: [PATCH 05/20] move azure function context to function handler --- .../AzureFunctionHandler.cs | 18 ++++++++++++- src/aggregator-function/RuleWrapper.cs | 26 +++---------------- 2 files changed, 20 insertions(+), 24 deletions(-) diff --git a/src/aggregator-function/AzureFunctionHandler.cs b/src/aggregator-function/AzureFunctionHandler.cs index d78d8043..473e7de1 100644 --- a/src/aggregator-function/AzureFunctionHandler.cs +++ b/src/aggregator-function/AzureFunctionHandler.cs @@ -2,6 +2,7 @@ using Microsoft.Extensions.Logging; using Newtonsoft.Json; using System; +using System.IO; using System.Linq; using System.Net; using System.Net.Http; @@ -88,9 +89,11 @@ public async Task RunAsync(HttpRequestMessage req, Cancella configuration = InvokeOptions.ExtendFromUrl(configuration, req.RequestUri); var logger = new ForwarderLogger(_log); - var wrapper = new RuleWrapper(configuration, logger, _context.FunctionName, _context.FunctionDirectory); try { + var ruleFilePath = GetRuleFilePath(_context.FunctionName, _context.FunctionDirectory); + var wrapper = new RuleWrapper(configuration, logger, ruleFilePath); + string execResult = await wrapper.ExecuteAsync(eventContext, cancellationToken); if (string.IsNullOrEmpty(execResult)) @@ -151,6 +154,19 @@ private static WorkItemEventContext CreateContextFromEvent(WebHookEvent eventDat } } + private string GetRuleFilePath(string functionDirectory, string ruleName) + { + string ruleFilePath = Path.Combine(functionDirectory, $"{ruleName}.rule"); + if (!File.Exists(ruleFilePath)) + { + _log.LogError($"Rule code not found at {ruleFilePath}"); + throw new FileNotFoundException($"Rule code not found at expected path: '{ruleFilePath}'"); + } + + _log.LogDebug($"Rule code found at {ruleFilePath}"); + return ruleFilePath; + } + private static T GetCustomAttribute() where T : Attribute { diff --git a/src/aggregator-function/RuleWrapper.cs b/src/aggregator-function/RuleWrapper.cs index 9b88c62f..81b09657 100644 --- a/src/aggregator-function/RuleWrapper.cs +++ b/src/aggregator-function/RuleWrapper.cs @@ -16,15 +16,13 @@ internal class RuleWrapper { private readonly AggregatorConfiguration configuration; private readonly IAggregatorLogger logger; - private readonly string ruleName; - private readonly string functionDirectory; + private readonly string ruleFilePath; - public RuleWrapper(AggregatorConfiguration configuration, IAggregatorLogger logger, string ruleName, string functionDirectory) + public RuleWrapper(AggregatorConfiguration configuration, IAggregatorLogger logger, string ruleFilePath) { this.configuration = configuration; this.logger = logger; - this.ruleName = ruleName; - this.functionDirectory = functionDirectory; + this.ruleFilePath = ruleFilePath; } internal async Task ExecuteAsync(WorkItemEventContext eventContext, CancellationToken cancellationToken) @@ -50,11 +48,6 @@ internal async Task ExecuteAsync(WorkItemEventContext eventContext, Canc logger.WriteInfo($"Connected to Azure DevOps"); using (var clientsContext = new AzureDevOpsClientsContext(devops)) { - string ruleFilePath = GetRuleFilePath(); - if (string.IsNullOrWhiteSpace(ruleFilePath)) - { - return "Rule file not found!"; - } string[] ruleCode = await ReadAllLinesAsync(ruleFilePath); var engine = new Engine.RuleEngine(logger, ruleCode, configuration.SaveMode, configuration.DryRun); @@ -81,18 +74,5 @@ private static async Task ReadAllLinesAsync(string ruleFilePath) } } } - - private string GetRuleFilePath() - { - string ruleFilePath = Path.Combine(functionDirectory, $"{ruleName}.rule"); - if (!File.Exists(ruleFilePath)) - { - logger.WriteError($"Rule code not found at {ruleFilePath}"); - return string.Empty; - } - - logger.WriteVerbose($"Rule code found at {ruleFilePath}"); - return ruleFilePath; - } } } From 445131328ac51574036ed83af069f454cd4e4d99 Mon Sep 17 00:00:00 2001 From: BobSilent Date: Wed, 4 Sep 2019 23:51:46 +0200 Subject: [PATCH 06/20] update documentation and add possibility to configuring a rule or a mapping to run impersonated --- doc/command-examples.md | 2 +- doc/rule-language.md | 14 ++++++++++++-- doc/setup.md | 8 ++++++-- .../Mappings/AggregatorMappings.cs | 9 +++++---- src/aggregator-cli/Mappings/MapRuleCommand.cs | 5 ++++- src/aggregator-cli/Rules/AggregatorRules.cs | 16 ++++++++-------- src/aggregator-cli/Rules/InvokeRuleCommand.cs | 7 +++++-- src/aggregator-function/RuleWrapper.cs | 2 +- src/aggregator-ruleng/Language/RuleFileParser.cs | 16 +++++++++++----- src/aggregator-ruleng/RuleEngine.cs | 4 ++-- src/aggregator-shared/AggregatorConfiguration.cs | 1 + src/aggregator-shared/InvokeOptions.cs | 5 +++++ 12 files changed, 61 insertions(+), 28 deletions(-) diff --git a/doc/command-examples.md b/doc/command-examples.md index 59a2f63e..09e1fdd2 100644 --- a/doc/command-examples.md +++ b/doc/command-examples.md @@ -64,7 +64,7 @@ This is the last step: gluing Azure DevOps to the Rule hosted in Azure Functions ```Batchfile map.rule --verbose --project SampleProject --event workitem.created --instance my1 --rule test1 -map.rule --verbose --project SampleProject --event workitem.updated --instance my1 --rule test2 +map.rule --verbose --project SampleProject --event workitem.updated --instance my1 --rule test2 --impersonate map.rule --verbose --project SampleProject --event workitem.created --instance my3 --resourceGroup myRG1 --rule test3 ``` The same rule can be triggered by multiple events from different Azure DevOps projects. Currently only these events are supported: diff --git a/doc/rule-language.md b/doc/rule-language.md index d4995d57..ef3eb2b4 100644 --- a/doc/rule-language.md +++ b/doc/rule-language.md @@ -7,7 +7,9 @@ They are parsed by Aggregator and removed before compiling the code. `.lang=C#` `.language=Csharp` -Currently the only supported language is C#. You can use the `.lang` directive to specify the programming language used by the rule. +Currently the only supported language is C#. +You can use the `.lang` directive to specify the programming language used by the rule. +If no language is specified: C# is default. ## reference directive Loads the specified assembly in the Rule execution context @@ -19,8 +21,16 @@ Example Equivalent to C# namespace `.import=System.Collections.Generic` +## impersonate directive +Aggregator uses credentials for accessing Azure DevOps. By default the changes which +were saved back to Azure DevOps are done with the credentials provided for accessing +Azure DevOps. +In order to do the changes on behalf of the account who initiated an event, which Aggregator will handle, +specify +`.impersonate=onBehalfOfInitiator` - +**Attention:** To use this the identify accessing Azure DevOps needs special permissions, +see [Rule Examples](setup.md#azure-devops-personal-access-token--PAT-). # WorkItem Object diff --git a/doc/setup.md b/doc/setup.md index a476b1cd..2025a477 100644 --- a/doc/setup.md +++ b/doc/setup.md @@ -64,8 +64,12 @@ In Azure Portal you can check the permissons in the IAM menu for the selected Re ## Azure DevOps Personal Access Token (PAT) -A PAT has the same or less permissions than the person that creates it. -We recommend that the PAT is issued by an Azure DevOps Organization Administrator. +A PAT has the same or less permissions than the person/identity that creates it. +We recommend that the PAT is issued by an Azure DevOps Organization Administrator Identity. + +When using the [impersonate directive](rule-language.md#impersonate-directive) +the used identity for creating the PAT must have the permission: +"Bypass rules on work item updates" Aggregator needs the following Scopes: diff --git a/src/aggregator-cli/Mappings/AggregatorMappings.cs b/src/aggregator-cli/Mappings/AggregatorMappings.cs index cb2e0425..668cf148 100644 --- a/src/aggregator-cli/Mappings/AggregatorMappings.cs +++ b/src/aggregator-cli/Mappings/AggregatorMappings.cs @@ -71,7 +71,7 @@ internal class EventFilters public IEnumerable Fields { get; set; } } - internal async Task AddAsync(string projectName, string @event, EventFilters filters, InstanceName instance, string ruleName, CancellationToken cancellationToken) + internal async Task AddAsync(string projectName, string @event, EventFilters filters, InstanceName instance, string ruleName, bool impersonateExecution, CancellationToken cancellationToken) { logger.WriteVerbose($"Reading Azure DevOps project data..."); var projectClient = devops.GetClient(); @@ -80,7 +80,7 @@ internal async Task AddAsync(string projectName, string @event, EventFilte var rules = new AggregatorRules(azure, logger); logger.WriteVerbose($"Retrieving {ruleName} Function Key..."); - (string ruleUrl, string ruleKey) = await rules.GetInvocationUrlAndKey(instance, ruleName, cancellationToken); + (string ruleUrl, string ruleKey) = await rules.GetInvocationUrlAndKey(instance, ruleName, impersonateExecution, cancellationToken); logger.WriteInfo($"{ruleName} Function Key retrieved."); var serviceHooksClient = devops.GetClient(); @@ -217,8 +217,9 @@ internal async Task RemoveRuleEventAsync(string @event, InstanceName insta if (rule != "*") { ruleSubs = ruleSubs - .Where(s => s.ConsumerInputs.GetValue("url", "").StartsWith( - AggregatorRules.GetInvocationUrl(instance, rule))); + .Where(s => s.ConsumerInputs + .GetValue("url", "") + .StartsWith(AggregatorRules.GetInvocationUrl(instance, rule))); } foreach (var ruleSub in ruleSubs) diff --git a/src/aggregator-cli/Mappings/MapRuleCommand.cs b/src/aggregator-cli/Mappings/MapRuleCommand.cs index b6a836fb..0fab3290 100644 --- a/src/aggregator-cli/Mappings/MapRuleCommand.cs +++ b/src/aggregator-cli/Mappings/MapRuleCommand.cs @@ -25,6 +25,9 @@ class MapRuleCommand : CommandBase [Option('r', "rule", Required = true, HelpText = "Aggregator rule name.")] public string Rule { get; set; } + [Option("impersonate", Required = false, HelpText = "Do rule changes on behalf of the person triggered the rule execution. See wiki for details, requires special account privileges.")] + public bool ImpersonateExecution { get; set; } + // event filters: but cannot make AreaPath & Tag work //[Option("filterAreaPath", Required = false, HelpText = "Filter Azure DevOps event to include only Work Items under the specified Area Path.")] public string FilterAreaPath { get; set; } @@ -58,7 +61,7 @@ internal override async Task RunAsync(CancellationToken cancellationToken) }; var instance = new InstanceName(Instance, ResourceGroup); - var id = await mappings.AddAsync(Project, Event, filters, instance, Rule, cancellationToken); + var id = await mappings.AddAsync(Project, Event, filters, instance, Rule, ImpersonateExecution, cancellationToken); return id.Equals(Guid.Empty) ? 1 : 0; } } diff --git a/src/aggregator-cli/Rules/AggregatorRules.cs b/src/aggregator-cli/Rules/AggregatorRules.cs index 7d234a8a..0820c5e0 100644 --- a/src/aggregator-cli/Rules/AggregatorRules.cs +++ b/src/aggregator-cli/Rules/AggregatorRules.cs @@ -55,12 +55,12 @@ internal async Task> ListAsync(InstanceName instance, } } - internal static string GetInvocationUrl(InstanceName instance, string rule) + internal static string GetInvocationUrl(InstanceName instance, string rule, bool impersonateExecution = false) { - return $"{instance.FunctionAppUrl}/api/{rule}"; + return $"{instance.FunctionAppUrl}/api/{rule}{(impersonateExecution? "?execute=impersonated" : "" )}"; } - internal async Task<(string url, string key)> GetInvocationUrlAndKey(InstanceName instance, string rule, CancellationToken cancellationToken) + internal async Task<(string url, string key)> GetInvocationUrlAndKey(InstanceName instance, string rule, bool impersonateExecution = false, CancellationToken cancellationToken = default) { var instances = new AggregatorInstances(_azure, _logger); var kudu = new KuduApi(instance, _azure, _logger); @@ -80,7 +80,7 @@ internal static string GetInvocationUrl(InstanceName instance, string rule) var js = new JsonSerializer(); var secret = js.Deserialize(jtr); - (string url, string key) invocation = (GetInvocationUrl(instance, rule), secret.Key); + (string url, string key) invocation = (GetInvocationUrl(instance, rule, impersonateExecution), secret.Key); return invocation; } } @@ -289,7 +289,7 @@ internal async Task UpdateAsync(InstanceName instance, string name, string return ok; } - internal async Task InvokeLocalAsync(string projectName, string @event, int workItemId, string ruleFilePath, bool dryRun, SaveMode saveMode, CancellationToken cancellationToken) + internal async Task InvokeLocalAsync(string projectName, string @event, int workItemId, string ruleFilePath, bool dryRun, SaveMode saveMode, bool impersonateExecution, CancellationToken cancellationToken) { if (!File.Exists(ruleFilePath)) { @@ -332,7 +332,7 @@ internal async Task InvokeLocalAsync(string projectName, string @event, in var ruleCode = await File.ReadAllLinesAsync(ruleFilePath, cancellationToken); var engineLogger = new EngineWrapperLogger(_logger); - var engine = new Engine.RuleEngine(engineLogger, ruleCode, saveMode, dryRun: dryRun); + var engine = new Engine.RuleEngine(engineLogger, ruleCode, saveMode, dryRun: dryRun, impersonateExecution); var workItem = await clientsContext.WitClient.GetWorkItemAsync(projectName, workItemId, expand: WorkItemExpand.All, cancellationToken: cancellationToken); string result = await engine.ExecuteAsync(teamProjectId, workItem, clientsContext, cancellationToken); @@ -343,11 +343,11 @@ internal async Task InvokeLocalAsync(string projectName, string @event, in } } - internal async Task InvokeRemoteAsync(string account, string project, string @event, int workItemId, InstanceName instance, string ruleName, bool dryRun, SaveMode saveMode, CancellationToken cancellationToken) + internal async Task InvokeRemoteAsync(string account, string project, string @event, int workItemId, InstanceName instance, string ruleName, bool dryRun, SaveMode saveMode, bool impersonateExecution, CancellationToken cancellationToken) { // build the request ... _logger.WriteVerbose($"Retrieving {ruleName} Function Key..."); - var (ruleUrl, ruleKey) = await GetInvocationUrlAndKey(instance, ruleName, cancellationToken); + var (ruleUrl, ruleKey) = await GetInvocationUrlAndKey(instance, ruleName, impersonateExecution, cancellationToken); _logger.WriteInfo($"{ruleName} Function Key retrieved."); ruleUrl = InvokeOptions.AppendToUrl(ruleUrl, dryRun, saveMode); diff --git a/src/aggregator-cli/Rules/InvokeRuleCommand.cs b/src/aggregator-cli/Rules/InvokeRuleCommand.cs index a7dc53de..edd104ed 100644 --- a/src/aggregator-cli/Rules/InvokeRuleCommand.cs +++ b/src/aggregator-cli/Rules/InvokeRuleCommand.cs @@ -31,6 +31,9 @@ class InvokeRuleCommand : CommandBase [Option('m', "saveMode", Required = false, HelpText = "Save behaviour.")] public SaveMode SaveMode { get; set; } + [Option("impersonate", Required = false, HelpText = "Do rule changes on behalf of the person triggered the rule execution. See wiki for details, requires special account privileges.")] + public bool ImpersonateExecution { get; set; } + [Option('a', "account", SetName = "Remote", Required = true, HelpText = "Azure DevOps account name.")] public string Account { get; set; } @@ -53,14 +56,14 @@ internal override async Task RunAsync(CancellationToken cancellationToken) var rules = new AggregatorRules(context.Azure, context.Logger); if (Local) { - bool ok = await rules.InvokeLocalAsync(Project, Event, WorkItemId, Source, DryRun, SaveMode, cancellationToken); + bool ok = await rules.InvokeLocalAsync(Project, Event, WorkItemId, Source, DryRun, SaveMode, ImpersonateExecution, cancellationToken); return ok ? 0 : 1; } else { var instance = new InstanceName(Instance, ResourceGroup); context.Logger.WriteWarning("Untested feature!"); - bool ok = await rules.InvokeRemoteAsync(Account, Project, Event, WorkItemId, instance, Name, DryRun, SaveMode, cancellationToken); + bool ok = await rules.InvokeRemoteAsync(Account, Project, Event, WorkItemId, instance, Name, DryRun, SaveMode, ImpersonateExecution, cancellationToken); return ok ? 0 : 1; } } diff --git a/src/aggregator-function/RuleWrapper.cs b/src/aggregator-function/RuleWrapper.cs index 81b09657..b345ed59 100644 --- a/src/aggregator-function/RuleWrapper.cs +++ b/src/aggregator-function/RuleWrapper.cs @@ -50,7 +50,7 @@ internal async Task ExecuteAsync(WorkItemEventContext eventContext, Canc { string[] ruleCode = await ReadAllLinesAsync(ruleFilePath); - var engine = new Engine.RuleEngine(logger, ruleCode, configuration.SaveMode, configuration.DryRun); + var engine = new Engine.RuleEngine(logger, ruleCode, configuration.SaveMode, configuration.DryRun, configuration.Impersonate); return await engine.ExecuteAsync(eventContext.ProjectId, eventContext.WorkItemPayload, clientsContext, cancellationToken); } diff --git a/src/aggregator-ruleng/Language/RuleFileParser.cs b/src/aggregator-ruleng/Language/RuleFileParser.cs index eee1e088..145b008d 100644 --- a/src/aggregator-ruleng/Language/RuleFileParser.cs +++ b/src/aggregator-ruleng/Language/RuleFileParser.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.IO; using System.Linq; @@ -91,14 +92,14 @@ internal static (IRuleDirectives ruleDirectives, bool parseSuccess, string[] mes } break; - case "onbehalfofinitiator": + case "impersonate": if (parts.Length < 2) { - ruleDirectives.Impersonate = true; + messages.Add($"Invalid impersonate directive {directive}"); } else { - messages.Add($"Invalid onbehalfofinitiator directive {directive}"); + ruleDirectives.Impersonate = string.Equals("onBehalfOfInitiator", parts[1].TrimEnd(), StringComparison.OrdinalIgnoreCase); } break; @@ -127,9 +128,14 @@ public static string[] Write(IRuleDirectives ruleDirectives) { var content = new List { - ruleDirectives.LanguageAsString() + $".language={ruleDirectives.LanguageAsString()}" }; + if (ruleDirectives.Impersonate) + { + content.Add($".impersonate=onBehalfOfInitiator"); + } + content.AddRange(ruleDirectives.References.Select(reference => $".reference={reference}")); content.AddRange(ruleDirectives.Imports.Select(import => $".import={import}")); content.AddRange(ruleDirectives.RuleCode); diff --git a/src/aggregator-ruleng/RuleEngine.cs b/src/aggregator-ruleng/RuleEngine.cs index 39c59278..ff696033 100644 --- a/src/aggregator-ruleng/RuleEngine.cs +++ b/src/aggregator-ruleng/RuleEngine.cs @@ -30,7 +30,7 @@ public class RuleEngine private readonly SaveMode saveMode; private readonly bool impersonateChanges; - public RuleEngine(IAggregatorLogger logger, string[] ruleCode, SaveMode mode, bool dryRun) + public RuleEngine(IAggregatorLogger logger, string[] ruleCode, SaveMode mode, bool dryRun, bool executeImpersonated = false) { State = EngineState.Unknown; @@ -46,7 +46,7 @@ public RuleEngine(IAggregatorLogger logger, string[] ruleCode, SaveMode mode, bo return; } - impersonateChanges = ruleDirectives.Impersonate; + impersonateChanges = ruleDirectives.Impersonate || executeImpersonated; var references = new HashSet(DefaultAssemblyReferences().Concat(ruleDirectives.LoadAssemblyReferences())); var imports = new HashSet(DefaultImports().Concat(ruleDirectives.Imports)); diff --git a/src/aggregator-shared/AggregatorConfiguration.cs b/src/aggregator-shared/AggregatorConfiguration.cs index 6e19a24c..324511ac 100644 --- a/src/aggregator-shared/AggregatorConfiguration.cs +++ b/src/aggregator-shared/AggregatorConfiguration.cs @@ -48,5 +48,6 @@ public void Write(Microsoft.Azure.Management.AppService.Fluent.IWebApp webApp) public string DevOpsToken { get; set; } public SaveMode SaveMode { get; set; } public bool DryRun { get; internal set; } + public bool Impersonate { get; internal set; } } } diff --git a/src/aggregator-shared/InvokeOptions.cs b/src/aggregator-shared/InvokeOptions.cs index 1c31b0a2..ae579bb1 100644 --- a/src/aggregator-shared/InvokeOptions.cs +++ b/src/aggregator-shared/InvokeOptions.cs @@ -21,6 +21,11 @@ public static AggregatorConfiguration ExtendFromUrl(AggregatorConfiguration conf configuration.SaveMode = saveMode; } + if (string.Equals(parameters["execute"], "impersonated", StringComparison.OrdinalIgnoreCase)) + { + configuration.Impersonate = true; + } + return configuration; } } From dd5740239a70713b9f0d584bdce456fdff36434c Mon Sep 17 00:00:00 2001 From: BobSilent Date: Thu, 5 Sep 2019 15:57:13 +0200 Subject: [PATCH 07/20] extend tests --- src/unittests-ruleng/RuleFileParserTests.cs | 77 +++++++++++++++------ 1 file changed, 56 insertions(+), 21 deletions(-) diff --git a/src/unittests-ruleng/RuleFileParserTests.cs b/src/unittests-ruleng/RuleFileParserTests.cs index 2d41c73b..9a301df9 100644 --- a/src/unittests-ruleng/RuleFileParserTests.cs +++ b/src/unittests-ruleng/RuleFileParserTests.cs @@ -1,16 +1,4 @@ -using System; -using System.Collections.Generic; -using System.Text; -using System.Threading; -using System.Threading.Tasks; - -using aggregator; -using aggregator.Engine; -using aggregator.Engine.Language; - -using Microsoft.TeamFoundation.WorkItemTracking.WebApi.Models; - -using NSubstitute; +using aggregator.Engine.Language; using Xunit; @@ -22,32 +10,79 @@ public class RuleFileParserTests [Fact] public void RuleLanguageDefaultsCSharp_Succeeds() { - string ruleCode = @" return $""Hello { self.WorkItemType } #{ self.Id } - { self.Title }!""; "; - (IRuleDirectives directives, bool _, string[] __) = RuleFileParser.Read(ruleCode.Mince()); + (IRuleDirectives directives, bool parsingSuccess, string[] __) = RuleFileParser.Read(ruleCode.Mince()); Assert.Empty(directives.References); Assert.Empty(directives.Imports); + Assert.False(directives.Impersonate); Assert.NotEmpty(directives.RuleCode); Assert.Equal(RuleLanguage.Csharp, directives.Language); + Assert.True(parsingSuccess); } [Theory] - [InlineData(".language=CSharp", RuleLanguage.Csharp, true)] - [InlineData(".language=C#", RuleLanguage.Csharp, true)] - [InlineData(".language=CS", RuleLanguage.Csharp, true)] - [InlineData(".language=", RuleLanguage.Unknown, false)] + [InlineData(".language=CSharp")] + [InlineData(".language=C#")] + [InlineData(".language=CS")] + public void RuleLanguageDirectiveParse_Succeeds(string ruleCode, RuleLanguage expectedLanguage = RuleLanguage.Csharp) + { + (IRuleDirectives directives, bool parsingSuccess, string[] __) = RuleFileParser.Read(ruleCode.Mince()); + + Assert.True(parsingSuccess); + Assert.Equal(expectedLanguage, directives.Language); + } - public void RuleLanguageDirectiveParse_Succeeds(string ruleCode, RuleLanguage expectedLanguage, bool expectedParsingSuccess) + [Theory] + [InlineData(".language=")] + [InlineData(".lang=WHAT")] + [InlineData(".lang=C#\r\n.unrecognized=directive\r\nreturn string.Empty;\r\n", RuleLanguage.Csharp)] + public void RuleLanguageDirectiveParse_Fails(string ruleCode, RuleLanguage expectedLanguage = RuleLanguage.Unknown) { (IRuleDirectives directives, bool parsingSuccess, string[] __) = RuleFileParser.Read(ruleCode.Mince()); + Assert.False(parsingSuccess); Assert.Equal(expectedLanguage, directives.Language); - Assert.Equal(expectedParsingSuccess, parsingSuccess); } + + [Theory] + [InlineData(".r=System.Xml.XDocument", 1)] + [InlineData(".ref=System.Xml.XDocument", 1)] + [InlineData(".reference=System.Xml.XDocument", 1)] + public void RuleReferenceDirectiveParse_Succeeds(string ruleCode, int expectedReferenceCount) + { + (IRuleDirectives directives, bool parsingSuccess, string[] __) = RuleFileParser.Read(ruleCode.Mince()); + + Assert.True(parsingSuccess); + Assert.Equal(expectedReferenceCount, directives.References.Count); + } + + [Theory] + [InlineData(".import=System.Diagnostics", 1)] + [InlineData(".imports=System.Diagnostics", 1)] + [InlineData(".namespace=System.Diagnostics", 1)] + public void RuleImportDirectiveParse_Succeeds(string ruleCode, int expectedImportCount) + { + (IRuleDirectives directives, bool parsingSuccess, string[] __) = RuleFileParser.Read(ruleCode.Mince()); + + Assert.True(parsingSuccess); + Assert.Equal(expectedImportCount, directives.Imports.Count); + } + + [Fact] + public void RuleImpersonateDirectiveParse_Succeeds() + { + string ruleCode = @".impersonate=onBehalfOfInitiator +"; + + (IRuleDirectives directives, bool parsingSuccess, string[] __) = RuleFileParser.Read(ruleCode.Mince()); + + Assert.True(parsingSuccess); + Assert.True(directives.Impersonate); + } } } From 646cf1b37bfc0df9bb59694771e346f16ad22197 Mon Sep 17 00:00:00 2001 From: BobSilent Date: Thu, 5 Sep 2019 17:50:44 +0200 Subject: [PATCH 08/20] minor fix RuleFileParser --- src/aggregator-ruleng/Language/RuleFileParser.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/aggregator-ruleng/Language/RuleFileParser.cs b/src/aggregator-ruleng/Language/RuleFileParser.cs index 145b008d..9c5f78b4 100644 --- a/src/aggregator-ruleng/Language/RuleFileParser.cs +++ b/src/aggregator-ruleng/Language/RuleFileParser.cs @@ -45,7 +45,6 @@ internal static (IRuleDirectives ruleDirectives, bool parseSuccess, string[] mes if (parts.Length < 2) { messages.Add($"Invalid language directive {directive}"); - ruleDirectives.Language = RuleLanguage.Unknown; } else { From 66b83c2a721ab518686e3b5b6307d52082da879b Mon Sep 17 00:00:00 2001 From: BobSilent Date: Fri, 6 Sep 2019 13:54:08 +0200 Subject: [PATCH 09/20] RuleMapping display impersonation configuration in list command --- src/aggregator-cli/Instances/InstanceName.cs | 4 +- .../Mappings/AggregatorMappings.cs | 17 +++---- .../Mappings/MappingOutputData.cs | 16 ++++--- src/aggregator-cli/Rules/AggregatorRules.cs | 11 +++-- .../AzureFunctionHandler.cs | 4 +- src/aggregator-shared/InvokeOptions.cs | 46 +++++++++++++++---- 6 files changed, 65 insertions(+), 33 deletions(-) diff --git a/src/aggregator-cli/Instances/InstanceName.cs b/src/aggregator-cli/Instances/InstanceName.cs index 6d73b107..d75adbee 100644 --- a/src/aggregator-cli/Instances/InstanceName.cs +++ b/src/aggregator-cli/Instances/InstanceName.cs @@ -35,9 +35,9 @@ public static InstanceName FromFunctionAppName(string appName, string resourceGr } // used only in mappings.ListAsync - public static InstanceName FromFunctionAppUrl(string url) + public static InstanceName FromFunctionAppUrl(Uri url) { - string host = new Uri(url).Host; + string host = url.Host; host = host.Substring(0, host.IndexOf('.')); return new InstanceName(host.Remove(host.Length - functionAppSuffix.Length), null); } diff --git a/src/aggregator-cli/Mappings/AggregatorMappings.cs b/src/aggregator-cli/Mappings/AggregatorMappings.cs index 668cf148..528749fa 100644 --- a/src/aggregator-cli/Mappings/AggregatorMappings.cs +++ b/src/aggregator-cli/Mappings/AggregatorMappings.cs @@ -8,6 +8,7 @@ using Microsoft.Azure.Management.Fluent; using System.Threading; using Microsoft.VisualStudio.Services.FormInput; +using aggregator; namespace aggregator.cli { @@ -53,11 +54,11 @@ internal async Task> ListAsync(InstanceName insta continue; } // HACK need to factor the URL<->rule_name - string ruleUrl = subscription.ConsumerInputs.GetValue("url","/"); - string ruleName = ruleUrl.Substring(ruleUrl.LastIndexOf('/')); - string ruleFullName = InstanceName.FromFunctionAppUrl(ruleUrl).PlainName + ruleName; + Uri ruleUrl = new Uri(subscription.ConsumerInputs.GetValue("url","https://google.com")); + string ruleName = ruleUrl.Segments.LastOrDefault() ?? string.Empty; + string ruleFullName = $"{InstanceName.FromFunctionAppUrl(ruleUrl).PlainName}/{ruleName}"; result.Add( - new MappingOutputData(instance, ruleFullName, foundProject.Name, subscription.EventType, subscription.Status.ToString()) + new MappingOutputData(instance, ruleFullName, ruleUrl.IsImpersonatationEnabled(), foundProject.Name, subscription.EventType, subscription.Status.ToString()) ); } return result; @@ -80,7 +81,7 @@ internal async Task AddAsync(string projectName, string @event, EventFilte var rules = new AggregatorRules(azure, logger); logger.WriteVerbose($"Retrieving {ruleName} Function Key..."); - (string ruleUrl, string ruleKey) = await rules.GetInvocationUrlAndKey(instance, ruleName, impersonateExecution, cancellationToken); + (Uri ruleUrl, string ruleKey) = await rules.GetInvocationUrlAndKey(instance, ruleName, impersonateExecution, cancellationToken); logger.WriteInfo($"{ruleName} Function Key retrieved."); var serviceHooksClient = devops.GetClient(); @@ -108,7 +109,7 @@ internal async Task AddAsync(string projectName, string @event, EventFilte new InputFilterCondition { InputId = "url", - InputValue = ruleUrl, + InputValue = ruleUrl.ToString(), Operator = InputFilterOperator.Equals, CaseSensitive = false } @@ -132,7 +133,7 @@ internal async Task AddAsync(string projectName, string @event, EventFilte ConsumerActionId = "httpRequest", ConsumerInputs = new Dictionary { - { "url", ruleUrl }, + { "url", ruleUrl.ToString() }, { "httpHeaders", $"x-functions-key:{ruleKey}" }, // careful with casing! { "resourceDetailsToSend", "all" }, @@ -219,7 +220,7 @@ internal async Task RemoveRuleEventAsync(string @event, InstanceName insta ruleSubs = ruleSubs .Where(s => s.ConsumerInputs .GetValue("url", "") - .StartsWith(AggregatorRules.GetInvocationUrl(instance, rule))); + .StartsWith(AggregatorRules.GetInvocationUrl(instance, rule).ToString())); } foreach (var ruleSub in ruleSubs) diff --git a/src/aggregator-cli/Mappings/MappingOutputData.cs b/src/aggregator-cli/Mappings/MappingOutputData.cs index d9ccbbfd..0bbf31f6 100644 --- a/src/aggregator-cli/Mappings/MappingOutputData.cs +++ b/src/aggregator-cli/Mappings/MappingOutputData.cs @@ -2,16 +2,18 @@ { internal class MappingOutputData : ILogDataObject { - string instanceName; - string rule; - string project; - string @event; - string status; + private string instanceName; + private string rule; + private bool executeImpersonated; + private string project; + private string @event; + private string status; - internal MappingOutputData(InstanceName instance, string rule, string project, string @event, string status) + internal MappingOutputData(InstanceName instance, string rule, bool executeImpersonated, string project, string @event, string status) { this.instanceName = instance.PlainName; this.rule = rule; + this.executeImpersonated = executeImpersonated; this.project = project; this.@event = @event; this.status = status; @@ -19,7 +21,7 @@ internal MappingOutputData(InstanceName instance, string rule, string project, s public string AsHumanReadable() { - return $"Project {project} invokes rule {rule} for {@event} (status {status})"; + return $"Project {project} invokes rule {rule}{(executeImpersonated ? " (impersonated)" : string.Empty)} for {@event} (status {status})"; } } } diff --git a/src/aggregator-cli/Rules/AggregatorRules.cs b/src/aggregator-cli/Rules/AggregatorRules.cs index 0820c5e0..c7584b21 100644 --- a/src/aggregator-cli/Rules/AggregatorRules.cs +++ b/src/aggregator-cli/Rules/AggregatorRules.cs @@ -55,12 +55,13 @@ internal async Task> ListAsync(InstanceName instance, } } - internal static string GetInvocationUrl(InstanceName instance, string rule, bool impersonateExecution = false) + internal static Uri GetInvocationUrl(InstanceName instance, string rule, bool impersonateExecution = false) { - return $"{instance.FunctionAppUrl}/api/{rule}{(impersonateExecution? "?execute=impersonated" : "" )}"; + var url = $"{instance.FunctionAppUrl}/api/{rule}"; + return new Uri(url).AddToUrl(impersonate: impersonateExecution); } - internal async Task<(string url, string key)> GetInvocationUrlAndKey(InstanceName instance, string rule, bool impersonateExecution = false, CancellationToken cancellationToken = default) + internal async Task<(Uri url, string key)> GetInvocationUrlAndKey(InstanceName instance, string rule, bool impersonateExecution = false, CancellationToken cancellationToken = default) { var instances = new AggregatorInstances(_azure, _logger); var kudu = new KuduApi(instance, _azure, _logger); @@ -80,7 +81,7 @@ internal static string GetInvocationUrl(InstanceName instance, string rule, bool var js = new JsonSerializer(); var secret = js.Deserialize(jtr); - (string url, string key) invocation = (GetInvocationUrl(instance, rule, impersonateExecution), secret.Key); + (Uri url, string key) invocation = (GetInvocationUrl(instance, rule, impersonateExecution), secret.Key); return invocation; } } @@ -350,7 +351,7 @@ internal async Task InvokeRemoteAsync(string account, string project, stri var (ruleUrl, ruleKey) = await GetInvocationUrlAndKey(instance, ruleName, impersonateExecution, cancellationToken); _logger.WriteInfo($"{ruleName} Function Key retrieved."); - ruleUrl = InvokeOptions.AppendToUrl(ruleUrl, dryRun, saveMode); + ruleUrl = ruleUrl.AddToUrl(dryRun, saveMode); string baseUrl = $"https://dev.azure.com/{account}"; Guid teamProjectId = Guid.Empty; diff --git a/src/aggregator-function/AzureFunctionHandler.cs b/src/aggregator-function/AzureFunctionHandler.cs index 473e7de1..05439e7d 100644 --- a/src/aggregator-function/AzureFunctionHandler.cs +++ b/src/aggregator-function/AzureFunctionHandler.cs @@ -85,8 +85,8 @@ public async Task RunAsync(HttpRequestMessage req, Cancella .AddJsonFile("local.settings.json", optional: true, reloadOnChange: true) .AddEnvironmentVariables() .Build(); - var configuration = AggregatorConfiguration.Read(config); - configuration = InvokeOptions.ExtendFromUrl(configuration, req.RequestUri); + var configuration = AggregatorConfiguration.Read(config) + .UpdateFromUrl(req.RequestUri); var logger = new ForwarderLogger(_log); try diff --git a/src/aggregator-shared/InvokeOptions.cs b/src/aggregator-shared/InvokeOptions.cs index ae579bb1..5a5b62df 100644 --- a/src/aggregator-shared/InvokeOptions.cs +++ b/src/aggregator-shared/InvokeOptions.cs @@ -1,32 +1,60 @@ using System; +using System.Collections.Specialized; +using System.Globalization; + +using Microsoft.WindowsAzure.Storage.Core; + namespace aggregator { public static class InvokeOptions { - public static string AppendToUrl(string ruleUrl, bool dryRun, SaveMode saveMode) + public static Uri AddToUrl(this Uri ruleUrl, bool dryRun = false, SaveMode saveMode = SaveMode.Default, bool impersonate = false) { - return ruleUrl + FormattableString.Invariant($"?dryRun={dryRun}&saveMode={saveMode}"); + var queryBuilder = new UriQueryBuilder(); + queryBuilder.Add("dryRun", dryRun.ToString(CultureInfo.InvariantCulture)); + queryBuilder.Add("saveMode", saveMode.ToString()); + + if (impersonate) + { + queryBuilder.Add("execute", "impersonated"); + } + + return queryBuilder.AddToUri(ruleUrl); } - public static AggregatorConfiguration ExtendFromUrl(AggregatorConfiguration configuration, Uri requestUri) + public static AggregatorConfiguration UpdateFromUrl(this AggregatorConfiguration configuration, Uri requestUri) { var parameters = System.Web.HttpUtility.ParseQueryString(requestUri.Query); - bool dryRun = bool.TryParse(parameters["dryRun"], out dryRun) && dryRun; - configuration.DryRun = dryRun; + configuration.DryRun = IsDryRunEnabled(parameters); if (Enum.TryParse(parameters["saveMode"], out SaveMode saveMode)) { configuration.SaveMode = saveMode; } - if (string.Equals(parameters["execute"], "impersonated", StringComparison.OrdinalIgnoreCase)) - { - configuration.Impersonate = true; - } + configuration.Impersonate = IsImpersonatationEnabled(parameters); return configuration; } + + public static bool IsImpersonatationEnabled(this Uri ruleUrl) + { + var parameters = System.Web.HttpUtility.ParseQueryString(ruleUrl.Query); + + return IsImpersonatationEnabled(parameters); + } + + private static bool IsImpersonatationEnabled(NameValueCollection parameters) + { + return string.Equals(parameters["execute"], "impersonated", StringComparison.OrdinalIgnoreCase); + } + + private static bool IsDryRunEnabled(NameValueCollection parameters) + { + bool dryRun = bool.TryParse(parameters["dryRun"], out dryRun) && dryRun; + return dryRun; + } } } From b8e61222c1658a742bb9ddab0752901526664ba9 Mon Sep 17 00:00:00 2001 From: BobSilent Date: Sat, 7 Sep 2019 23:29:27 +0200 Subject: [PATCH 10/20] align code and revert GetInvocationUrl changes --- .../Mappings/AggregatorMappings.cs | 19 ++++++------ src/aggregator-cli/Rules/AggregatorRules.cs | 31 ++++++++++--------- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/aggregator-cli/Mappings/AggregatorMappings.cs b/src/aggregator-cli/Mappings/AggregatorMappings.cs index 528749fa..9e2a39e2 100644 --- a/src/aggregator-cli/Mappings/AggregatorMappings.cs +++ b/src/aggregator-cli/Mappings/AggregatorMappings.cs @@ -14,7 +14,7 @@ namespace aggregator.cli { internal class AggregatorMappings { - private VssConnection devops; + private readonly VssConnection devops; private readonly IAzure azure; private readonly ILogger logger; @@ -81,10 +81,10 @@ internal async Task AddAsync(string projectName, string @event, EventFilte var rules = new AggregatorRules(azure, logger); logger.WriteVerbose($"Retrieving {ruleName} Function Key..."); - (Uri ruleUrl, string ruleKey) = await rules.GetInvocationUrlAndKey(instance, ruleName, impersonateExecution, cancellationToken); + (Uri ruleUrl, string ruleKey) = await rules.GetInvocationUrlAndKey(instance, ruleName, cancellationToken); logger.WriteInfo($"{ruleName} Function Key retrieved."); - var serviceHooksClient = devops.GetClient(); + ruleUrl = ruleUrl.AddToUrl(impersonate: impersonateExecution); // check if the subscription already exists and bail out var query = new SubscriptionsQuery { @@ -119,6 +119,7 @@ internal async Task AddAsync(string projectName, string @event, EventFilte }; cancellationToken.ThrowIfCancellationRequested(); + var serviceHooksClient = devops.GetClient(); var queryResult = await serviceHooksClient.QuerySubscriptionsAsync(query); if (queryResult.Results.Any()) { @@ -202,7 +203,7 @@ internal async Task RemoveRuleEventAsync(string @event, InstanceName insta instance.FunctionAppUrl)); if (@event != "*") { - ruleSubs = ruleSubs.Where(s => s.EventType == @event); + ruleSubs = ruleSubs.Where(s => string.Equals(s.EventType, @event, StringComparison.OrdinalIgnoreCase)); } if (projectName != "*") @@ -212,15 +213,15 @@ internal async Task RemoveRuleEventAsync(string @event, InstanceName insta var project = await projectClient.GetProject(projectName); logger.WriteInfo($"Project {projectName} data read."); - ruleSubs = ruleSubs.Where(s => s.PublisherInputs["projectId"] == project.Id.ToString()); + ruleSubs = ruleSubs.Where(s => string.Equals(s.PublisherInputs["projectId"], project.Id.ToString(), StringComparison.OrdinalIgnoreCase)); } if (rule != "*") { - ruleSubs = ruleSubs - .Where(s => s.ConsumerInputs - .GetValue("url", "") - .StartsWith(AggregatorRules.GetInvocationUrl(instance, rule).ToString())); + var invocationUrl = AggregatorRules.GetInvocationUrl(instance, rule).ToString(); + ruleSubs = ruleSubs.Where(s => s.ConsumerInputs + .GetValue("url", "") + .StartsWith(invocationUrl, StringComparison.OrdinalIgnoreCase)); } foreach (var ruleSub in ruleSubs) diff --git a/src/aggregator-cli/Rules/AggregatorRules.cs b/src/aggregator-cli/Rules/AggregatorRules.cs index c7584b21..c6b49360 100644 --- a/src/aggregator-cli/Rules/AggregatorRules.cs +++ b/src/aggregator-cli/Rules/AggregatorRules.cs @@ -55,15 +55,14 @@ internal async Task> ListAsync(InstanceName instance, } } - internal static Uri GetInvocationUrl(InstanceName instance, string rule, bool impersonateExecution = false) + internal static Uri GetInvocationUrl(InstanceName instance, string rule) { var url = $"{instance.FunctionAppUrl}/api/{rule}"; - return new Uri(url).AddToUrl(impersonate: impersonateExecution); + return new Uri(url); } - internal async Task<(Uri url, string key)> GetInvocationUrlAndKey(InstanceName instance, string rule, bool impersonateExecution = false, CancellationToken cancellationToken = default) + internal async Task<(Uri url, string key)> GetInvocationUrlAndKey(InstanceName instance, string rule, CancellationToken cancellationToken = default) { - var instances = new AggregatorInstances(_azure, _logger); var kudu = new KuduApi(instance, _azure, _logger); // see https://github.com/projectkudu/kudu/wiki/Functions-API @@ -81,7 +80,7 @@ internal static Uri GetInvocationUrl(InstanceName instance, string rule, bool im var js = new JsonSerializer(); var secret = js.Deserialize(jtr); - (Uri url, string key) invocation = (GetInvocationUrl(instance, rule, impersonateExecution), secret.Key); + (Uri url, string key) invocation = (GetInvocationUrl(instance, rule), secret.Key); return invocation; } } @@ -149,19 +148,23 @@ private static async Task> PackagingFilesAsync(strin var assembly = Assembly.GetExecutingAssembly(); - using (var stream = assembly.GetManifestResourceStream("aggregator.cli.Rules.function.json")) // TODO we can deserialize a KuduFunctionConfig instead of using a fixed file... + using (var stream = assembly.GetManifestResourceStream("aggregator.cli.Rules.function.json")) { - var reader = new StreamReader(stream); - var content = await reader.ReadToEndAsync(); - inMemoryFiles.Add("function.json", content); + using (var reader = new StreamReader(stream)) + { + var content = await reader.ReadToEndAsync(); + inMemoryFiles.Add("function.json", content); + } } using (var stream = assembly.GetManifestResourceStream("aggregator.cli.Rules.run.csx")) { - var reader = new StreamReader(stream); - var content = await reader.ReadToEndAsync(); - inMemoryFiles.Add("run.csx", content); + using (var reader = new StreamReader(stream)) + { + var content = await reader.ReadToEndAsync(); + inMemoryFiles.Add("run.csx", content); + } } return inMemoryFiles; @@ -348,10 +351,10 @@ internal async Task InvokeRemoteAsync(string account, string project, stri { // build the request ... _logger.WriteVerbose($"Retrieving {ruleName} Function Key..."); - var (ruleUrl, ruleKey) = await GetInvocationUrlAndKey(instance, ruleName, impersonateExecution, cancellationToken); + var (ruleUrl, ruleKey) = await GetInvocationUrlAndKey(instance, ruleName, cancellationToken); _logger.WriteInfo($"{ruleName} Function Key retrieved."); - ruleUrl = ruleUrl.AddToUrl(dryRun, saveMode); + ruleUrl = ruleUrl.AddToUrl(dryRun, saveMode, impersonateExecution); string baseUrl = $"https://dev.azure.com/{account}"; Guid teamProjectId = Guid.Empty; From 731bc69f4a565a830d290bb5504342b4a3cd399d Mon Sep 17 00:00:00 2001 From: BobSilent Date: Wed, 11 Sep 2019 11:38:22 +0200 Subject: [PATCH 11/20] simplify AzureFunctionHandler and add intial test for bogus test message --- src/aggregator-cli.sln | 6 + .../AzureFunctionHandler.cs | 137 +++++++++--------- .../AzureFunctionHandlerTests.cs | 66 +++++++++ .../unittests-function.csproj | 27 ++++ .../TestData/ExampleTestData.cs | 13 +- src/unittests-ruleng/unittests-ruleng.csproj | 13 +- 6 files changed, 181 insertions(+), 81 deletions(-) create mode 100644 src/unittests-function/AzureFunctionHandlerTests.cs create mode 100644 src/unittests-function/unittests-function.csproj diff --git a/src/aggregator-cli.sln b/src/aggregator-cli.sln index 65141d68..e682f1e9 100644 --- a/src/aggregator-cli.sln +++ b/src/aggregator-cli.sln @@ -35,6 +35,8 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "unittests-core", "unittests-core\unittests-core.csproj", "{6AD42A49-2EA4-4659-855D-0C011D368794}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "unittests-function", "unittests-function\unittests-function.csproj", "{A875638A-8E95-47BE-AEDD-BAD5113692B3}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -69,6 +71,10 @@ Global {6AD42A49-2EA4-4659-855D-0C011D368794}.Debug|Any CPU.Build.0 = Debug|Any CPU {6AD42A49-2EA4-4659-855D-0C011D368794}.Release|Any CPU.ActiveCfg = Release|Any CPU {6AD42A49-2EA4-4659-855D-0C011D368794}.Release|Any CPU.Build.0 = Release|Any CPU + {A875638A-8E95-47BE-AEDD-BAD5113692B3}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {A875638A-8E95-47BE-AEDD-BAD5113692B3}.Debug|Any CPU.Build.0 = Debug|Any CPU + {A875638A-8E95-47BE-AEDD-BAD5113692B3}.Release|Any CPU.ActiveCfg = Release|Any CPU + {A875638A-8E95-47BE-AEDD-BAD5113692B3}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/src/aggregator-function/AzureFunctionHandler.cs b/src/aggregator-function/AzureFunctionHandler.cs index 05439e7d..5ba49853 100644 --- a/src/aggregator-function/AzureFunctionHandler.cs +++ b/src/aggregator-function/AzureFunctionHandler.cs @@ -35,109 +35,91 @@ public async Task RunAsync(HttpRequestMessage req, Cancella { _log.LogDebug($"Context: {_context.InvocationId} {_context.FunctionName} {_context.FunctionDirectory} {_context.FunctionAppDirectory}"); + var ruleName = _context.FunctionName; var aggregatorVersion = GetCustomAttribute()?.InformationalVersion; - - try - { - var rule = _context.FunctionName; - _log.LogInformation($"Aggregator v{aggregatorVersion} executing rule '{rule}'"); - } - catch (Exception ex) - { - _log.LogWarning($"Failed parsing request headers: {ex.Message}"); - } - + _log.LogInformation($"Aggregator v{aggregatorVersion} executing rule '{ruleName}'"); cancellationToken.ThrowIfCancellationRequested(); // Get request body - var jsonContent = await req.Content.ReadAsStringAsync(); - if (string.IsNullOrWhiteSpace(jsonContent)) + var eventData = await GetWebHookEvent(req); + if (eventData == null) { - _log.LogWarning($"Failed parsing request body: empty"); - - var resp = new HttpResponseMessage(HttpStatusCode.BadRequest) - { - Content = new StringContent("Request body is empty") - }; - return resp; + return req.CreateErrorResponse(HttpStatusCode.BadRequest, "Request body is empty"); } - var data = JsonConvert.DeserializeObject(jsonContent); - // sanity check - if (!DevOpsEvents.IsValidEvent(data.EventType) - || data.PublisherId != DevOpsEvents.PublisherId) + if (!DevOpsEvents.IsValidEvent(eventData.EventType) + || eventData.PublisherId != DevOpsEvents.PublisherId) { - return req.CreateResponse(HttpStatusCode.BadRequest, new - { - error = "Not a good Azure DevOps post..." - }); + return req.CreateErrorResponse(HttpStatusCode.BadRequest, "Not a good Azure DevOps post..."); } - var eventContext = CreateContextFromEvent(data); + var eventContext = CreateContextFromEvent(eventData); if (eventContext.IsTestEvent()) { - return RespondToTestEventMessage(req, aggregatorVersion); + return req.CreateTestEventResponse(aggregatorVersion, ruleName); } - var config = new ConfigurationBuilder() - .SetBasePath(_context.FunctionAppDirectory) - .AddJsonFile("local.settings.json", optional: true, reloadOnChange: true) - .AddEnvironmentVariables() - .Build(); - var configuration = AggregatorConfiguration.Read(config) + var configContext = GetConfigurationContext(); + var configuration = AggregatorConfiguration.Read(configContext) .UpdateFromUrl(req.RequestUri); var logger = new ForwarderLogger(_log); - try + using (_log.BeginScope($"WorkItem #{eventContext.WorkItemPayload.WorkItem.Id}")) { - var ruleFilePath = GetRuleFilePath(_context.FunctionName, _context.FunctionDirectory); - var wrapper = new RuleWrapper(configuration, logger, ruleFilePath); + try + { + var ruleFilePath = GetRuleFilePath(_context.FunctionName, _context.FunctionDirectory); + var wrapper = new RuleWrapper(configuration, logger, ruleFilePath); - string execResult = await wrapper.ExecuteAsync(eventContext, cancellationToken); + string execResult = await wrapper.ExecuteAsync(eventContext, cancellationToken); - if (string.IsNullOrEmpty(execResult)) - { - var resp = new HttpResponseMessage(HttpStatusCode.OK); - return resp; + if (string.IsNullOrEmpty(execResult)) + { + return req.CreateResponse(HttpStatusCode.OK); + } + else + { + _log.LogInformation($"Returning '{execResult}' from '{ruleName}'"); + return req.CreateResponse(HttpStatusCode.OK, execResult); + } } - else + catch (Exception ex) { - _log.LogInformation($"Returning '{execResult}' from '{_context.FunctionName}'"); - - var resp = new HttpResponseMessage(HttpStatusCode.OK) - { - Content = new StringContent(execResult) - }; - return resp; + _log.LogWarning($"Rule '{ruleName}' failed: {ex.Message}"); + return req.CreateErrorResponse(HttpStatusCode.NotImplemented, ex); } } - catch (Exception ex) - { - _log.LogWarning($"Rule '{_context.FunctionName}' failed: {ex.Message}"); + } - var resp = new HttpResponseMessage(HttpStatusCode.NotImplemented) - { - Content = new StringContent(ex.Message) - }; - return resp; - } + + private IConfigurationRoot GetConfigurationContext() + { + var config = new ConfigurationBuilder() + .SetBasePath(_context.FunctionAppDirectory) + .AddJsonFile("local.settings.json", optional: true, reloadOnChange: true) + .AddEnvironmentVariables() + .Build(); + return config; } - private HttpResponseMessage RespondToTestEventMessage(HttpRequestMessage req, string aggregatorVersion) + + private async Task GetWebHookEvent(HttpRequestMessage req) { - var resp = req.CreateResponse(HttpStatusCode.OK, new - { - message = $"Hello from Aggregator v{aggregatorVersion} executing rule '{_context.FunctionName}'" - }); - resp.Headers.Add("X-Aggregator-Version", aggregatorVersion); - resp.Headers.Add("X-Aggregator-Rule", _context.FunctionName); - return resp; + var jsonContent = await req.Content.ReadAsStringAsync(); + if (string.IsNullOrWhiteSpace(jsonContent)) + { + _log.LogWarning($"Failed parsing request body: empty"); + return null; + } + + var data = JsonConvert.DeserializeObject(jsonContent); + return data; } private static WorkItemEventContext CreateContextFromEvent(WebHookEvent eventData) { - var collectionUrl = eventData.ResourceContainers.GetValueOrDefault("collection")?.BaseUrl; + var collectionUrl = eventData.ResourceContainers.GetValueOrDefault("collection")?.BaseUrl ?? "https://example.com"; var teamProjectId = eventData.ResourceContainers.GetValueOrDefault("project")?.Id ?? Guid.Empty; var resourceObject = eventData.Resource as JObject; @@ -176,4 +158,19 @@ private static T GetCustomAttribute() .FirstOrDefault() as T; } } + + + internal static class HttpResponseMessageExtensions + { + public static HttpResponseMessage CreateTestEventResponse(this HttpRequestMessage req, string aggregatorVersion, string ruleName) + { + var resp = req.CreateResponse(HttpStatusCode.OK, new + { + message = $"Hello from Aggregator v{aggregatorVersion} executing rule '{ruleName}'" + }); + resp.Headers.Add("X-Aggregator-Version", aggregatorVersion); + resp.Headers.Add("X-Aggregator-Rule", ruleName); + return resp; + } + } } diff --git a/src/unittests-function/AzureFunctionHandlerTests.cs b/src/unittests-function/AzureFunctionHandlerTests.cs new file mode 100644 index 00000000..b3162242 --- /dev/null +++ b/src/unittests-function/AzureFunctionHandlerTests.cs @@ -0,0 +1,66 @@ +using System; +using System.Linq; +using System.Net; +using System.Net.Http; +using System.Text; +using System.Threading; +using aggregator; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using NSubstitute; +using unittests_ruleng.TestData; +using Xunit; +using ExecutionContext = Microsoft.Azure.WebJobs.ExecutionContext; + +namespace unittests_function +{ + public class AzureFunctionHandlerTests + { + private readonly ILogger logger; + private readonly ExecutionContext context; + private readonly HttpRequestMessage request; + + public AzureFunctionHandlerTests() + { + logger = Substitute.For(); + context = Substitute.For(); + context.InvocationId = Guid.Empty; + context.FunctionName = "TestRule"; + context.FunctionDirectory = ""; + context.FunctionAppDirectory = ""; + + request = new HttpRequestMessage(HttpMethod.Post, "http://localhost/"); + var services = new ServiceCollection() + .AddMvc() + .AddWebApiConventions() + .Services + .BuildServiceProvider(); + + request.Properties.Add(nameof(HttpContext), new DefaultHttpContext + { + RequestServices = services + }); + } + + [Fact] + public async void HandleTestEvent_ReturnAggregatorInformation_Succeeds() + { + request.Content = new StringContent(ExampleTestData.TestEventAsString, Encoding.UTF8, "application/json"); + + var handler = new AzureFunctionHandler(logger, context); + var response = await handler.RunAsync(request, CancellationToken.None); + + Assert.True(response.IsSuccessStatusCode); + Assert.True(response.Headers.TryGetValues("X-Aggregator-Version", out var versions)); + Assert.Single(versions); + + Assert.True(response.Headers.TryGetValues("X-Aggregator-Rule", out var rules)); + Assert.Equal("TestRule", rules.Single()); + + var content = await response.Content.ReadAsStringAsync(); + Assert.StartsWith("{\"message\":\"Hello from Aggregator v", content); + Assert.EndsWith("executing rule 'TestRule'\"}", content); + } + } +} diff --git a/src/unittests-function/unittests-function.csproj b/src/unittests-function/unittests-function.csproj new file mode 100644 index 00000000..904ac5ef --- /dev/null +++ b/src/unittests-function/unittests-function.csproj @@ -0,0 +1,27 @@ + + + + netcoreapp2.1 + unittests_function + + false + + + + + + + + + all + runtime; build; native; contentfiles; analyzers; buildtransitive + + + + + + + + + + diff --git a/src/unittests-ruleng/TestData/ExampleTestData.cs b/src/unittests-ruleng/TestData/ExampleTestData.cs index dcc3cc33..568db758 100644 --- a/src/unittests-ruleng/TestData/ExampleTestData.cs +++ b/src/unittests-ruleng/TestData/ExampleTestData.cs @@ -4,15 +4,16 @@ using System.Reflection; using Microsoft.TeamFoundation.Work.WebApi.Contracts; using Microsoft.TeamFoundation.WorkItemTracking.WebApi.Models; +using Microsoft.VisualStudio.Services.ServiceHooks.WebApi; using Newtonsoft.Json; namespace unittests_ruleng.TestData { - class Helper + internal static class Helper { - private static string GetEmbeddedResourceContent(string resourceName) + internal static string GetEmbeddedResourceContent(string resourceName) { Assembly assembly = Assembly.GetExecutingAssembly(); var fullName = assembly.GetManifestResourceNames() @@ -43,13 +44,13 @@ internal static string[] GetFromResource(string resourceName) } } - static class ExampleRuleCode + internal static class ExampleRuleCode { public static string[] ActivateParent => Helper.GetFromResource("advanced.activate-parent.rulecode"); public static string[] ResolveParent => Helper.GetFromResource("advanced.resolve-parent.rulecode"); } - static class ExampleTestData + public static class ExampleTestData { public static WorkItem DeltedWorkItem => Helper.GetFromResource("DeletedWorkItem.json"); public static WorkItem WorkItem => Helper.GetFromResource("WorkItem.22.json"); @@ -67,5 +68,9 @@ static class ExampleTestData public static ProcessConfiguration ProcessConfigDefaultAgile => Helper.GetFromResource("WorkClient.ProcessConfiguration.Agile.json"); public static ProcessConfiguration ProcessConfigDefaultScrum => Helper.GetFromResource("WorkClient.ProcessConfiguration.Scrum.json"); public static WorkItemStateColor[] WorkItemStateColorDefault => Helper.GetFromResource("WitClient.WorkItemStateColor.EpicFeatureUserStory.json"); + + + public static WebHookEvent TestEvent => Helper.GetFromResource("TestEvent.json"); + public static string TestEventAsString => Helper.GetEmbeddedResourceContent("TestEvent.json"); } } diff --git a/src/unittests-ruleng/unittests-ruleng.csproj b/src/unittests-ruleng/unittests-ruleng.csproj index f9faff2b..4000457b 100644 --- a/src/unittests-ruleng/unittests-ruleng.csproj +++ b/src/unittests-ruleng/unittests-ruleng.csproj @@ -21,6 +21,7 @@ + @@ -40,12 +41,16 @@ - + + + + + @@ -56,10 +61,4 @@ - - - - - - From 098918b4fcd11eb47b446f00b01d24ef1e2dc960 Mon Sep 17 00:00:00 2001 From: BobSilent Date: Tue, 10 Sep 2019 13:43:53 +0200 Subject: [PATCH 12/20] fix: RevisedBy not set, use ChangedBy of workitem --- src/aggregator-ruleng/WorkItemStore.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/aggregator-ruleng/WorkItemStore.cs b/src/aggregator-ruleng/WorkItemStore.cs index 18c6be6c..932b346d 100644 --- a/src/aggregator-ruleng/WorkItemStore.cs +++ b/src/aggregator-ruleng/WorkItemStore.cs @@ -21,7 +21,7 @@ public class WorkItemStore private readonly Lazy>> _lazyGetWorkItemCategories; private readonly Lazy>> _lazyGetBacklogWorkItemTypesAndStates; - private readonly IdentityRef _polluterIdentity; + private readonly IdentityRef _triggerIdentity; public WorkItemStore(EngineContext context) @@ -36,8 +36,8 @@ public WorkItemStore(EngineContext context, WorkItem workItem) : this(context) { //initialize tracker with initial work item var wrapper = new WorkItemWrapper(_context, workItem); - //store initiator identity - _polluterIdentity = wrapper.RevisedBy; + //store event initiator identity + _triggerIdentity = wrapper.ChangedBy; } public WorkItemWrapper GetWorkItem(int id) @@ -146,7 +146,7 @@ private void ImpersonateChanges() foreach (var workItem in changedWorkItems) { - workItem.ChangedBy = _polluterIdentity; + workItem.ChangedBy = _triggerIdentity; } } From d59741877a9203753166948ee1de25083fdbec4f Mon Sep 17 00:00:00 2001 From: BobSilent Date: Sun, 8 Sep 2019 23:55:40 +0200 Subject: [PATCH 13/20] reduce compiler messages --- src/aggregator-cli/Rules/AggregatorRules.cs | 1 - src/aggregator-ruleng/RuleEngine.cs | 6 +++--- src/aggregator-ruleng/WorkItemStore.cs | 5 ++--- src/unittests-ruleng/RuleFileParserTests.cs | 12 ++++++------ 4 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/aggregator-cli/Rules/AggregatorRules.cs b/src/aggregator-cli/Rules/AggregatorRules.cs index c6b49360..f6907ae1 100644 --- a/src/aggregator-cli/Rules/AggregatorRules.cs +++ b/src/aggregator-cli/Rules/AggregatorRules.cs @@ -19,7 +19,6 @@ namespace aggregator.cli { internal class AggregatorRules { - private static readonly Random Randomizer = new Random((int)DateTime.UtcNow.Ticks); private readonly IAzure _azure; private readonly ILogger _logger; diff --git a/src/aggregator-ruleng/RuleEngine.cs b/src/aggregator-ruleng/RuleEngine.cs index ff696033..cc9e4c8a 100644 --- a/src/aggregator-ruleng/RuleEngine.cs +++ b/src/aggregator-ruleng/RuleEngine.cs @@ -141,10 +141,10 @@ public async Task ExecuteAsync(Guid projectId, WorkItemData workItemPayl } logger.WriteVerbose($"Post-execution, save any change (mode {saveMode})..."); - var saveRes = await store.SaveChanges(saveMode, !DryRun, impersonateChanges, cancellationToken); - if (saveRes.created + saveRes.updated > 0) + var (created, updated) = await store.SaveChanges(saveMode, !DryRun, impersonateChanges, cancellationToken); + if (created + updated > 0) { - logger.WriteInfo($"Changes saved to Azure DevOps (mode {saveMode}): {saveRes.created} created, {saveRes.updated} updated."); + logger.WriteInfo($"Changes saved to Azure DevOps (mode {saveMode}): {created} created, {updated} updated."); } else { diff --git a/src/aggregator-ruleng/WorkItemStore.cs b/src/aggregator-ruleng/WorkItemStore.cs index 932b346d..ce4c0e00 100644 --- a/src/aggregator-ruleng/WorkItemStore.cs +++ b/src/aggregator-ruleng/WorkItemStore.cs @@ -139,10 +139,9 @@ public async Task> GetBacklogWorkItemType private void ImpersonateChanges() { - var workItems = _context.Tracker.GetChangedWorkItems(); + var (created, updated, _, _) = _context.Tracker.GetChangedWorkItems(); - var changedWorkItems = workItems.Created - .Concat(workItems.Updated); + var changedWorkItems = created.Concat(updated); foreach (var workItem in changedWorkItems) { diff --git a/src/unittests-ruleng/RuleFileParserTests.cs b/src/unittests-ruleng/RuleFileParserTests.cs index 9a301df9..57f33025 100644 --- a/src/unittests-ruleng/RuleFileParserTests.cs +++ b/src/unittests-ruleng/RuleFileParserTests.cs @@ -14,7 +14,7 @@ public void RuleLanguageDefaultsCSharp_Succeeds() return $""Hello { self.WorkItemType } #{ self.Id } - { self.Title }!""; "; - (IRuleDirectives directives, bool parsingSuccess, string[] __) = RuleFileParser.Read(ruleCode.Mince()); + (IRuleDirectives directives, bool parsingSuccess, _) = RuleFileParser.Read(ruleCode.Mince()); Assert.Empty(directives.References); Assert.Empty(directives.Imports); @@ -30,7 +30,7 @@ public void RuleLanguageDefaultsCSharp_Succeeds() [InlineData(".language=CS")] public void RuleLanguageDirectiveParse_Succeeds(string ruleCode, RuleLanguage expectedLanguage = RuleLanguage.Csharp) { - (IRuleDirectives directives, bool parsingSuccess, string[] __) = RuleFileParser.Read(ruleCode.Mince()); + (IRuleDirectives directives, bool parsingSuccess, _) = RuleFileParser.Read(ruleCode.Mince()); Assert.True(parsingSuccess); Assert.Equal(expectedLanguage, directives.Language); @@ -42,7 +42,7 @@ public void RuleLanguageDirectiveParse_Succeeds(string ruleCode, RuleLanguage ex [InlineData(".lang=C#\r\n.unrecognized=directive\r\nreturn string.Empty;\r\n", RuleLanguage.Csharp)] public void RuleLanguageDirectiveParse_Fails(string ruleCode, RuleLanguage expectedLanguage = RuleLanguage.Unknown) { - (IRuleDirectives directives, bool parsingSuccess, string[] __) = RuleFileParser.Read(ruleCode.Mince()); + (IRuleDirectives directives, bool parsingSuccess, _) = RuleFileParser.Read(ruleCode.Mince()); Assert.False(parsingSuccess); Assert.Equal(expectedLanguage, directives.Language); @@ -55,7 +55,7 @@ public void RuleLanguageDirectiveParse_Fails(string ruleCode, RuleLanguage expec [InlineData(".reference=System.Xml.XDocument", 1)] public void RuleReferenceDirectiveParse_Succeeds(string ruleCode, int expectedReferenceCount) { - (IRuleDirectives directives, bool parsingSuccess, string[] __) = RuleFileParser.Read(ruleCode.Mince()); + (IRuleDirectives directives, bool parsingSuccess, _) = RuleFileParser.Read(ruleCode.Mince()); Assert.True(parsingSuccess); Assert.Equal(expectedReferenceCount, directives.References.Count); @@ -67,7 +67,7 @@ public void RuleReferenceDirectiveParse_Succeeds(string ruleCode, int expectedRe [InlineData(".namespace=System.Diagnostics", 1)] public void RuleImportDirectiveParse_Succeeds(string ruleCode, int expectedImportCount) { - (IRuleDirectives directives, bool parsingSuccess, string[] __) = RuleFileParser.Read(ruleCode.Mince()); + (IRuleDirectives directives, bool parsingSuccess, _) = RuleFileParser.Read(ruleCode.Mince()); Assert.True(parsingSuccess); Assert.Equal(expectedImportCount, directives.Imports.Count); @@ -79,7 +79,7 @@ public void RuleImpersonateDirectiveParse_Succeeds() string ruleCode = @".impersonate=onBehalfOfInitiator "; - (IRuleDirectives directives, bool parsingSuccess, string[] __) = RuleFileParser.Read(ruleCode.Mince()); + (IRuleDirectives directives, bool parsingSuccess, _) = RuleFileParser.Read(ruleCode.Mince()); Assert.True(parsingSuccess); Assert.True(directives.Impersonate); From 06fd8701b5b175e0875965b5bac56df3fc050720 Mon Sep 17 00:00:00 2001 From: BobSilent Date: Thu, 12 Sep 2019 16:55:46 +0200 Subject: [PATCH 14/20] improve logging for batch request handling --- src/aggregator-ruleng/WorkItemStore.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/aggregator-ruleng/WorkItemStore.cs b/src/aggregator-ruleng/WorkItemStore.cs index ce4c0e00..4a793e38 100644 --- a/src/aggregator-ruleng/WorkItemStore.cs +++ b/src/aggregator-ruleng/WorkItemStore.cs @@ -373,6 +373,7 @@ private static bool IsSuccessStatusCode(int statusCode) return (created, updated); } + private async Task> ExecuteBatchRequest(IList batchRequests, CancellationToken cancellationToken) { if (!batchRequests.Any()) return Enumerable.Empty(); From e13937906457e076ca14ad6522c62901c47b3776 Mon Sep 17 00:00:00 2001 From: BobSilent Date: Sun, 8 Sep 2019 23:45:45 +0200 Subject: [PATCH 15/20] Refactor RuleWrapper and RuleEngine to follow single responsibility principle --- src/aggregator-cli/Rules/AggregatorRules.cs | 32 +-- .../AzureFunctionHandler.cs | 24 +- .../AzureFunctionRuleProvider.cs | 53 +++++ src/aggregator-function/RuleWrapper.cs | 78 ------- src/aggregator-ruleng/IAggregatorLogger.cs | 31 ++- src/aggregator-ruleng/IRuleProvider.cs | 9 + .../Language/RuleFileParser.cs | 76 +++++-- src/aggregator-ruleng/RuleEngine.cs | 150 +++---------- src/aggregator-ruleng/RuleExecutor.cs | 53 +++++ src/aggregator-ruleng/ScriptedRuleWrapper.cs | 205 ++++++++++++++++++ src/unittests-ruleng/RuleEngineTests.cs | 38 ---- src/unittests-ruleng/RuleFileParserTests.cs | 33 ++- src/unittests-ruleng/RuleTests.cs | 131 ++++------- .../ScriptedRuleWrapperTests.cs | 53 +++++ 14 files changed, 588 insertions(+), 378 deletions(-) create mode 100644 src/aggregator-function/AzureFunctionRuleProvider.cs delete mode 100644 src/aggregator-function/RuleWrapper.cs create mode 100644 src/aggregator-ruleng/IRuleProvider.cs create mode 100644 src/aggregator-ruleng/RuleExecutor.cs create mode 100644 src/aggregator-ruleng/ScriptedRuleWrapper.cs delete mode 100644 src/unittests-ruleng/RuleEngineTests.cs create mode 100644 src/unittests-ruleng/ScriptedRuleWrapperTests.cs diff --git a/src/aggregator-cli/Rules/AggregatorRules.cs b/src/aggregator-cli/Rules/AggregatorRules.cs index f6907ae1..8b31cb37 100644 --- a/src/aggregator-cli/Rules/AggregatorRules.cs +++ b/src/aggregator-cli/Rules/AggregatorRules.cs @@ -8,6 +8,7 @@ using System.Text; using System.Threading; using System.Threading.Tasks; +using aggregator.Engine.Language; using Microsoft.Azure.Management.Fluent; using Microsoft.TeamFoundation.Core.WebApi; using Microsoft.TeamFoundation.WorkItemTracking.WebApi.Models; @@ -95,13 +96,12 @@ internal async Task AddAsync(InstanceName instance, string name, string fi { _logger.WriteInfo($"Validate rule file {filePath}"); - var ruleContent = await File.ReadAllLinesAsync(filePath, cancellationToken); - var engineLogger = new EngineWrapperLogger(_logger); + var (ruleDirectives, _) = await RuleFileParser.ReadFile(filePath, engineLogger, cancellationToken); try { - var ruleEngine = new Engine.RuleEngine(engineLogger, ruleContent, SaveMode.Batch, true); - (var success, var diagnostics) = ruleEngine.VerifyRule(); + var rule = new Engine.ScriptedRuleWrapper(name, ruleDirectives, engineLogger); + (var success, var diagnostics) = rule.Verify(); if (success) { _logger.WriteInfo($"Rule file is valid"); @@ -125,8 +125,8 @@ internal async Task AddAsync(InstanceName instance, string name, string fi } _logger.WriteVerbose($"Layout rule files"); - var inMemoryFiles = await PackagingFilesAsync(name, filePath); - _logger.WriteInfo($"Packaging {filePath} into rule {name} complete."); + var inMemoryFiles = await PackagingFilesAsync(name, ruleDirectives); + _logger.WriteInfo($"Packaging rule {name} complete."); _logger.WriteVerbose($"Uploading rule files to {instance.PlainName}"); bool ok = await UploadRuleFilesAsync(instance, name, inMemoryFiles, cancellationToken); @@ -138,12 +138,12 @@ internal async Task AddAsync(InstanceName instance, string name, string fi return ok; } - private static async Task> PackagingFilesAsync(string name, string filePath) + private static async Task> PackagingFilesAsync(string name, IRuleDirectives ruleDirectives) { - var inMemoryFiles = new Dictionary(); - - var ruleContent = await File.ReadAllTextAsync(filePath); - inMemoryFiles.Add($"{name}.rule", ruleContent); + var inMemoryFiles = new Dictionary + { + { $"{name}.rule", string.Join(Environment.NewLine, RuleFileParser.Write(ruleDirectives)) } + }; var assembly = Assembly.GetExecutingAssembly(); @@ -332,13 +332,17 @@ internal async Task InvokeLocalAsync(string projectName, string @event, in using (var clientsContext = new AzureDevOpsClientsContext(devops)) { _logger.WriteVerbose($"Rule code found at {ruleFilePath}"); - var ruleCode = await File.ReadAllLinesAsync(ruleFilePath, cancellationToken); + var (ruleDirectives, _) = await RuleFileParser.ReadFile(ruleFilePath, cancellationToken); + var rule = new Engine.ScriptedRuleWrapper(Path.GetFileNameWithoutExtension(ruleFilePath), ruleDirectives) + { + ImpersonateExecution = impersonateExecution + }; var engineLogger = new EngineWrapperLogger(_logger); - var engine = new Engine.RuleEngine(engineLogger, ruleCode, saveMode, dryRun: dryRun, impersonateExecution); + var engine = new Engine.RuleEngine(engineLogger, saveMode, dryRun: dryRun); var workItem = await clientsContext.WitClient.GetWorkItemAsync(projectName, workItemId, expand: WorkItemExpand.All, cancellationToken: cancellationToken); - string result = await engine.ExecuteAsync(teamProjectId, workItem, clientsContext, cancellationToken); + string result = await engine.RunAsync(rule, teamProjectId, workItem, clientsContext, cancellationToken); _logger.WriteInfo($"Rule returned '{result}'"); return true; diff --git a/src/aggregator-function/AzureFunctionHandler.cs b/src/aggregator-function/AzureFunctionHandler.cs index 5ba49853..1203671c 100644 --- a/src/aggregator-function/AzureFunctionHandler.cs +++ b/src/aggregator-function/AzureFunctionHandler.cs @@ -2,7 +2,6 @@ using Microsoft.Extensions.Logging; using Newtonsoft.Json; using System; -using System.IO; using System.Linq; using System.Net; using System.Net.Http; @@ -65,14 +64,14 @@ public async Task RunAsync(HttpRequestMessage req, Cancella .UpdateFromUrl(req.RequestUri); var logger = new ForwarderLogger(_log); + var ruleProvider = new AzureFunctionRuleProvider(logger, _context.FunctionDirectory); + var ruleExecutor = new RuleExecutor(logger, configuration); using (_log.BeginScope($"WorkItem #{eventContext.WorkItemPayload.WorkItem.Id}")) { try { - var ruleFilePath = GetRuleFilePath(_context.FunctionName, _context.FunctionDirectory); - var wrapper = new RuleWrapper(configuration, logger, ruleFilePath); - - string execResult = await wrapper.ExecuteAsync(eventContext, cancellationToken); + var rule = await ruleProvider.GetRule(ruleName); + var execResult = await ruleExecutor.ExecuteAsync(rule, eventContext, cancellationToken); if (string.IsNullOrEmpty(execResult)) { @@ -80,7 +79,7 @@ public async Task RunAsync(HttpRequestMessage req, Cancella } else { - _log.LogInformation($"Returning '{execResult}' from '{ruleName}'"); + _log.LogInformation($"Returning '{execResult}' from '{rule.Name}'"); return req.CreateResponse(HttpStatusCode.OK, execResult); } } @@ -136,19 +135,6 @@ private static WorkItemEventContext CreateContextFromEvent(WebHookEvent eventDat } } - private string GetRuleFilePath(string functionDirectory, string ruleName) - { - string ruleFilePath = Path.Combine(functionDirectory, $"{ruleName}.rule"); - if (!File.Exists(ruleFilePath)) - { - _log.LogError($"Rule code not found at {ruleFilePath}"); - throw new FileNotFoundException($"Rule code not found at expected path: '{ruleFilePath}'"); - } - - _log.LogDebug($"Rule code found at {ruleFilePath}"); - return ruleFilePath; - } - private static T GetCustomAttribute() where T : Attribute { diff --git a/src/aggregator-function/AzureFunctionRuleProvider.cs b/src/aggregator-function/AzureFunctionRuleProvider.cs new file mode 100644 index 00000000..54c6f210 --- /dev/null +++ b/src/aggregator-function/AzureFunctionRuleProvider.cs @@ -0,0 +1,53 @@ +using System; +using System.IO; +using System.Linq; +using System.Threading.Tasks; +using aggregator.Engine; +using aggregator.Engine.Language; + +namespace aggregator +{ + internal class AzureFunctionRuleProvider : IRuleProvider + { + private const string SCRIPT_RULE_NAME_PATTERN = "*.rule"; + + private readonly IAggregatorLogger _logger; + private readonly string _rulesPath; + + public AzureFunctionRuleProvider(IAggregatorLogger logger, string functionDirectory) + { + _logger = logger; + _rulesPath = functionDirectory; + } + + /// + public async Task GetRule(string ruleName) + { + var ruleFilePath = GetRuleFilePath(ruleName); + var (ruleDirectives, _) = await RuleFileParser.ReadFile(ruleFilePath); + + return new ScriptedRuleWrapper(ruleName, ruleDirectives); + } + + private string GetRuleFilePath(string ruleName) + { + bool IsRequestedRule(string filePath) + { + return string.Equals(ruleName, Path.GetFileNameWithoutExtension(filePath), StringComparison.OrdinalIgnoreCase); + } + + var ruleFilePath = Directory.EnumerateFiles(_rulesPath, SCRIPT_RULE_NAME_PATTERN, SearchOption.TopDirectoryOnly) + .First(IsRequestedRule); + + if (ruleFilePath == null) + { + var errorMsg = $"Rule code file '{ruleName}.rule' not found at expected Path {_rulesPath}"; + _logger.WriteError(errorMsg); + throw new FileNotFoundException(errorMsg); + } + + _logger.WriteVerbose($"Rule code found at {ruleFilePath}"); + return ruleFilePath; + } + } +} \ No newline at end of file diff --git a/src/aggregator-function/RuleWrapper.cs b/src/aggregator-function/RuleWrapper.cs deleted file mode 100644 index b345ed59..00000000 --- a/src/aggregator-function/RuleWrapper.cs +++ /dev/null @@ -1,78 +0,0 @@ -using System; -using System.Collections.Generic; -using System.IO; -using System.Threading; -using System.Threading.Tasks; -using aggregator.Engine; -using Microsoft.VisualStudio.Services.Common; -using Microsoft.VisualStudio.Services.WebApi; - -namespace aggregator -{ - /// - /// Contains Aggregator specific code with no reference to Rule triggering - /// - internal class RuleWrapper - { - private readonly AggregatorConfiguration configuration; - private readonly IAggregatorLogger logger; - private readonly string ruleFilePath; - - public RuleWrapper(AggregatorConfiguration configuration, IAggregatorLogger logger, string ruleFilePath) - { - this.configuration = configuration; - this.logger = logger; - this.ruleFilePath = ruleFilePath; - } - - internal async Task ExecuteAsync(WorkItemEventContext eventContext, CancellationToken cancellationToken) - { - logger.WriteVerbose($"Connecting to Azure DevOps using {configuration.DevOpsTokenType}..."); - var clientCredentials = default(VssCredentials); - if (configuration.DevOpsTokenType == DevOpsTokenType.PAT) - { - clientCredentials = new VssBasicCredential(configuration.DevOpsTokenType.ToString(), configuration.DevOpsToken); - } - else - { - logger.WriteError($"Azure DevOps Token type {configuration.DevOpsTokenType} not supported!"); - throw new ArgumentOutOfRangeException(nameof(configuration.DevOpsTokenType)); - } - - cancellationToken.ThrowIfCancellationRequested(); - - // TODO improve from https://github.com/Microsoft/vsts-work-item-migrator - using (var devops = new VssConnection(eventContext.CollectionUri, clientCredentials)) - { - await devops.ConnectAsync(cancellationToken); - logger.WriteInfo($"Connected to Azure DevOps"); - using (var clientsContext = new AzureDevOpsClientsContext(devops)) - { - string[] ruleCode = await ReadAllLinesAsync(ruleFilePath); - - var engine = new Engine.RuleEngine(logger, ruleCode, configuration.SaveMode, configuration.DryRun, configuration.Impersonate); - - return await engine.ExecuteAsync(eventContext.ProjectId, eventContext.WorkItemPayload, clientsContext, cancellationToken); - } - } - } - - private static async Task ReadAllLinesAsync(string ruleFilePath) - { - using (var fileStream = File.OpenRead(ruleFilePath)) - { - using (var streamReader = new StreamReader(fileStream)) - { - var lines = new List(); - string line; - while ((line = await streamReader.ReadLineAsync()) != null) - { - lines.Add(line); - } - - return lines.ToArray(); - } - } - } - } -} diff --git a/src/aggregator-ruleng/IAggregatorLogger.cs b/src/aggregator-ruleng/IAggregatorLogger.cs index 4a329b04..454872a1 100644 --- a/src/aggregator-ruleng/IAggregatorLogger.cs +++ b/src/aggregator-ruleng/IAggregatorLogger.cs @@ -15,6 +15,35 @@ public interface IAggregatorLogger void WriteError(string message); } + /// + /// an emtpy logger implementation + /// + public class NullLogger : IAggregatorLogger + { + /// + public void WriteVerbose(string message) + { + } + + + /// + public void WriteInfo(string message) + { + } + + + /// + public void WriteWarning(string message) + { + } + + + /// + public void WriteError(string message) + { + } + } + public static class AggregatorLoggerExtensions { public static void WriteVerbose(this IAggregatorLogger logger, string[] messages) @@ -44,4 +73,4 @@ private static void WriteMultiple(Action logAction, string[] messages) } } } -} +} \ No newline at end of file diff --git a/src/aggregator-ruleng/IRuleProvider.cs b/src/aggregator-ruleng/IRuleProvider.cs new file mode 100644 index 00000000..633f8b25 --- /dev/null +++ b/src/aggregator-ruleng/IRuleProvider.cs @@ -0,0 +1,9 @@ +using System.Threading.Tasks; + +namespace aggregator.Engine +{ + public interface IRuleProvider + { + Task GetRule(string name); + } +} \ No newline at end of file diff --git a/src/aggregator-ruleng/Language/RuleFileParser.cs b/src/aggregator-ruleng/Language/RuleFileParser.cs index 9c5f78b4..dcfb1086 100644 --- a/src/aggregator-ruleng/Language/RuleFileParser.cs +++ b/src/aggregator-ruleng/Language/RuleFileParser.cs @@ -2,6 +2,8 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using System.Threading; +using System.Threading.Tasks; using Microsoft.VisualStudio.Services.Common; @@ -13,18 +15,29 @@ namespace aggregator.Engine.Language /// public static class RuleFileParser { - public static (IRuleDirectives ruleDirectives, bool result, string[] messages) ReadFile(string ruleFilePath) + public static async Task<(IRuleDirectives ruleDirectives, bool result)> ReadFile(string ruleFilePath, CancellationToken cancellationToken = default) { - var content = File.ReadAllLines(ruleFilePath); - return Read(content); + return await ReadFile(ruleFilePath, new NullLogger(), cancellationToken); + } + + public static async Task<(IRuleDirectives ruleDirectives, bool result)> ReadFile(string ruleFilePath, IAggregatorLogger logger, CancellationToken cancellationToken = default) + { + var content = await ReadAllLinesAsync(ruleFilePath, cancellationToken); + return Read(content, logger); } /// /// Grab directives /// - internal static (IRuleDirectives ruleDirectives, bool parseSuccess, string[] messages) Read(string[] ruleCode) + public static (IRuleDirectives ruleDirectives, bool parseSuccess) Read(string[] ruleCode, IAggregatorLogger logger = default) { - var messages = new List(); + var parsingIssues = false; + void FailParsingWithMessage(string message) + { + logger?.WriteWarning(message); + parsingIssues = true; + } + var directiveLineIndex = 0; var ruleDirectives = new RuleDirectives() { @@ -44,7 +57,7 @@ internal static (IRuleDirectives ruleDirectives, bool parseSuccess, string[] mes case "language": if (parts.Length < 2) { - messages.Add($"Invalid language directive {directive}"); + FailParsingWithMessage($"Invalid language directive {directive}"); } else { @@ -57,7 +70,7 @@ internal static (IRuleDirectives ruleDirectives, bool parseSuccess, string[] mes break; default: { - messages.Add($"Unrecognized language {parts[1]}"); + FailParsingWithMessage($"Unrecognized language {parts[1]}"); ruleDirectives.Language = RuleLanguage.Unknown; break; } @@ -70,7 +83,7 @@ internal static (IRuleDirectives ruleDirectives, bool parseSuccess, string[] mes case "reference": if (parts.Length < 2) { - messages.Add($"Invalid reference directive {directive}"); + FailParsingWithMessage($"Invalid reference directive {directive}"); } else { @@ -83,7 +96,7 @@ internal static (IRuleDirectives ruleDirectives, bool parseSuccess, string[] mes case "namespace": if (parts.Length < 2) { - messages.Add($"Invalid import directive {directive}"); + FailParsingWithMessage($"Invalid import directive {directive}"); } else { @@ -94,7 +107,7 @@ internal static (IRuleDirectives ruleDirectives, bool parseSuccess, string[] mes case "impersonate": if (parts.Length < 2) { - messages.Add($"Invalid impersonate directive {directive}"); + FailParsingWithMessage($"Invalid impersonate directive {directive}"); } else { @@ -104,7 +117,7 @@ internal static (IRuleDirectives ruleDirectives, bool parseSuccess, string[] mes default: { - messages.Add($"Unrecognized directive {directive}"); + FailParsingWithMessage($"Unrecognized directive {directive}"); break; } }//switch @@ -113,14 +126,15 @@ internal static (IRuleDirectives ruleDirectives, bool parseSuccess, string[] mes }//while ruleDirectives.RuleCode.AddRange(ruleCode.Skip(directiveLineIndex)); - return (ruleDirectives, messages.Count == 0, messages.ToArray()); + var parseSuccessful = !parsingIssues; + return (ruleDirectives, parseSuccessful); } - public static void WriteFile(string ruleFilePath, IRuleDirectives ruleDirectives) + public static async Task WriteFile(string ruleFilePath, IRuleDirectives ruleDirectives, CancellationToken cancellationToken = default) { var content = Write(ruleDirectives); - File.WriteAllLines(ruleFilePath, content); + await WriteAllLinesAsync(ruleFilePath, content, cancellationToken); } public static string[] Write(IRuleDirectives ruleDirectives) @@ -141,5 +155,39 @@ public static string[] Write(IRuleDirectives ruleDirectives) return content.ToArray(); } + + private static async Task ReadAllLinesAsync(string ruleFilePath, CancellationToken cancellationToken) + { + using (var fileStream = File.OpenRead(ruleFilePath)) + { + using (var streamReader = new StreamReader(fileStream)) + { + var lines = new List(); + string line; + while ((line = await streamReader.ReadLineAsync().ConfigureAwait(false)) != null) + { + cancellationToken.ThrowIfCancellationRequested(); + lines.Add(line); + } + + return lines.ToArray(); + } + } + } + + private static async Task WriteAllLinesAsync(string ruleFilePath, IEnumerable ruleContent, CancellationToken cancellationToken) + { + using (var fileStream = File.OpenWrite(ruleFilePath)) + { + using (var streamWriter = new StreamWriter(fileStream)) + { + foreach (var line in ruleContent) + { + cancellationToken.ThrowIfCancellationRequested(); + await streamWriter.WriteLineAsync(line).ConfigureAwait(false); + } + } + } + } } } diff --git a/src/aggregator-ruleng/RuleEngine.cs b/src/aggregator-ruleng/RuleEngine.cs index cc9e4c8a..7b3ffd6b 100644 --- a/src/aggregator-ruleng/RuleEngine.cs +++ b/src/aggregator-ruleng/RuleEngine.cs @@ -13,105 +13,40 @@ namespace aggregator.Engine { - public enum EngineState + internal interface IRuleEngine { - Unknown, - Success, - Error + Task RunAsync(IRule rule, Guid projectId, WorkItemData workItemPayload, IClientsContext clients, CancellationToken cancellationToken = default); } - /// - /// Entry point to execute rules, independent of environment - /// - public class RuleEngine + public abstract class RuleEngineBase : IRuleEngine { - private readonly IAggregatorLogger logger; - private readonly Script roslynScript; - private readonly SaveMode saveMode; - private readonly bool impersonateChanges; + protected IAggregatorLogger logger; - public RuleEngine(IAggregatorLogger logger, string[] ruleCode, SaveMode mode, bool dryRun, bool executeImpersonated = false) - { - State = EngineState.Unknown; - - this.logger = logger; - this.saveMode = mode; - this.DryRun = dryRun; - - (IRuleDirectives ruleDirectives, bool success, string[] messages) = RuleFileParser.Read(ruleCode); - if (!success || !ruleDirectives.IsValid()) - { - logger.WriteWarning(messages.Any() ? $"RuleFileParser issues: {string.Join(",",messages)}" : "No RuleFileParser issues found!"); - State = EngineState.Error; - return; - } + protected SaveMode saveMode; - impersonateChanges = ruleDirectives.Impersonate || executeImpersonated; + protected bool dryRun; - var references = new HashSet(DefaultAssemblyReferences().Concat(ruleDirectives.LoadAssemblyReferences())); - var imports = new HashSet(DefaultImports().Concat(ruleDirectives.Imports)); - - var scriptOptions = ScriptOptions.Default - .WithEmitDebugInformation(true) - .WithReferences(references) - // Add namespaces - .WithImports(imports); - - if (ruleDirectives.IsCSharp()) - { - this.roslynScript = CSharpScript.Create( - code: ruleDirectives.GetRuleCode(), - options: scriptOptions, - globalsType: typeof(Globals)); - } - else - { - logger.WriteError($"Cannot execute rule: language is not supported."); - State = EngineState.Error; - } - } - private static IEnumerable DefaultAssemblyReferences() + protected RuleEngineBase(IAggregatorLogger logger, SaveMode saveMode, bool dryRun) { - var types = new List() - { - typeof(object), - typeof(System.Linq.Enumerable), - typeof(System.Collections.Generic.CollectionExtensions), - typeof(Microsoft.VisualStudio.Services.WebApi.IdentityRef), - typeof(WorkItemWrapper) - }; - - return types.Select(t => t.Assembly); + this.logger = logger; + this.saveMode = saveMode; + this.dryRun = dryRun; } - private static IEnumerable DefaultImports() + public async Task RunAsync(IRule rule, Guid projectId, WorkItemData workItemPayload, IClientsContext clients, CancellationToken cancellationToken = default) { - var imports = new List - { - "System", - "System.Linq", - "System.Collections.Generic", - "Microsoft.VisualStudio.Services.WebApi", - "aggregator.Engine" - }; - - return imports; + var executionContext = CreateRuleExecutionContext(projectId, workItemPayload, clients); + + var result = await ExecuteRuleAsync(rule, executionContext, cancellationToken); + + return result; } - /// - /// State is used by unit tests - /// - public EngineState State { get; private set; } - public bool DryRun { get; } + protected abstract Task ExecuteRuleAsync(IRule rule, Globals executionContext, CancellationToken cancellationToken = default); - public async Task ExecuteAsync(Guid projectId, WorkItemData workItemPayload, IClientsContext clients, CancellationToken cancellationToken) + protected Globals CreateRuleExecutionContext(Guid projectId, WorkItemData workItemPayload, IClientsContext clients) { - if (State == EngineState.Error) - { - return string.Empty; - } - var workItem = workItemPayload.WorkItem; var context = new EngineContext(clients, projectId, workItem.GetTeamProject(), logger); var store = new WorkItemStore(context, workItem); @@ -126,50 +61,33 @@ public async Task ExecuteAsync(Guid projectId, WorkItemData workItemPayl store = store, logger = logger }; + return globals; + } + } - logger.WriteInfo($"Executing Rule..."); - var result = await roslynScript.RunAsync(globals, cancellationToken); - if (result.Exception != null) - { - logger.WriteError($"Rule failed with {result.Exception}"); - State = EngineState.Error; - } - else - { - logger.WriteInfo($"Rule succeeded with {result.ReturnValue ?? "no return value"}"); - State = EngineState.Success; - } - - logger.WriteVerbose($"Post-execution, save any change (mode {saveMode})..."); - var (created, updated) = await store.SaveChanges(saveMode, !DryRun, impersonateChanges, cancellationToken); - if (created + updated > 0) - { - logger.WriteInfo($"Changes saved to Azure DevOps (mode {saveMode}): {created} created, {updated} updated."); - } - else - { - logger.WriteInfo($"No changes saved to Azure DevOps."); - } - return result.ReturnValue; + public class RuleEngine : RuleEngineBase + { + public RuleEngine(IAggregatorLogger logger, SaveMode saveMode, bool dryRun) : base(logger, saveMode, dryRun) + { } - public (bool success, ImmutableArray diagnostics) VerifyRule() + protected override async Task ExecuteRuleAsync(IRule rule, Globals executionContext, CancellationToken cancellationToken = default) { - ImmutableArray diagnostics = roslynScript.Compile(); - (bool, ImmutableArray) result; - if (diagnostics.Any()) + var result = await rule.ApplyAsync(executionContext, cancellationToken); + + var store = executionContext.store; + var (created, updated) = await store.SaveChanges(saveMode, !dryRun, rule.ImpersonateExecution, cancellationToken); + if (created + updated > 0) { - State = EngineState.Error; - result = (false, diagnostics); + logger.WriteInfo($"Changes saved to Azure DevOps (mode {saveMode}): {created} created, {updated} updated."); } else { - State = EngineState.Success; - result = (true, ImmutableArray.Create()); + logger.WriteInfo($"No changes saved to Azure DevOps."); } - return result; + return result.Value; } } } diff --git a/src/aggregator-ruleng/RuleExecutor.cs b/src/aggregator-ruleng/RuleExecutor.cs new file mode 100644 index 00000000..d325cc4f --- /dev/null +++ b/src/aggregator-ruleng/RuleExecutor.cs @@ -0,0 +1,53 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.VisualStudio.Services.Common; +using Microsoft.VisualStudio.Services.WebApi; + +namespace aggregator.Engine +{ + public class RuleExecutor + { + protected readonly AggregatorConfiguration configuration; + + protected readonly IAggregatorLogger logger; + + public RuleExecutor(IAggregatorLogger logger, AggregatorConfiguration configuration) + { + this.configuration = configuration; + this.logger = logger; + } + + public async Task ExecuteAsync(IRule rule, WorkItemEventContext eventContext, CancellationToken cancellationToken) + { + logger.WriteVerbose($"Connecting to Azure DevOps using {configuration.DevOpsTokenType}..."); + var clientCredentials = default(VssCredentials); + if (configuration.DevOpsTokenType == DevOpsTokenType.PAT) + { + clientCredentials = new VssBasicCredential(configuration.DevOpsTokenType.ToString(), configuration.DevOpsToken); + } + else + { + logger.WriteError($"Azure DevOps Token type {configuration.DevOpsTokenType} not supported!"); + throw new ArgumentOutOfRangeException(nameof(configuration.DevOpsTokenType)); + } + + cancellationToken.ThrowIfCancellationRequested(); + + // TODO improve from https://github.com/Microsoft/vsts-work-item-migrator + using (var devops = new VssConnection(eventContext.CollectionUri, clientCredentials)) + { + await devops.ConnectAsync(cancellationToken); + logger.WriteInfo($"Connected to Azure DevOps"); + using (var clientsContext = new AzureDevOpsClientsContext(devops)) + { + var engine = new RuleEngine(logger, configuration.SaveMode, configuration.DryRun); + + var ruleResult = await engine.RunAsync(rule, eventContext.ProjectId, eventContext.WorkItemPayload, clientsContext, cancellationToken); + logger.WriteInfo(ruleResult); + return ruleResult; + } + } + } + } +} \ No newline at end of file diff --git a/src/aggregator-ruleng/ScriptedRuleWrapper.cs b/src/aggregator-ruleng/ScriptedRuleWrapper.cs new file mode 100644 index 00000000..1592f045 --- /dev/null +++ b/src/aggregator-ruleng/ScriptedRuleWrapper.cs @@ -0,0 +1,205 @@ +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Linq; +using System.Reflection; +using System.Text; +using System.Threading; +using System.Threading.Tasks; +using aggregator.Engine.Language; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp.Scripting; +using Microsoft.CodeAnalysis.Scripting; + +namespace aggregator.Engine +{ + public enum RuleExecutionOutcome + { + Unknown, + Success, + Error + } + + public interface IRuleResult + { + /// + /// Result Value Message + /// + string Value { get; } + + /// + /// Execution Outcome + /// + RuleExecutionOutcome Outcome { get; } + } + + + public class RuleResult : IRuleResult + { + /// + public string Value { get; set; } + + /// + public RuleExecutionOutcome Outcome { get; set; } + } + + public interface IRule + { + /// + /// RuleName + /// + string Name { get; } + + /// + /// The history will show the changes made by person who triggered the event + /// Assumes PAT or Account Permission is high enough + /// + bool ImpersonateExecution { get; set; } + + + Task ApplyAsync(Globals executionContext, CancellationToken cancellationToken); + } + + /// + /// CSharp Scripted Rule Facade + /// + public class ScriptedRuleWrapper : IRule + { + private Script _roslynScript; + private readonly IAggregatorLogger _logger; + private readonly bool? _ruleFileParseSuccess; + + /// + public string Name { get; } + + /// + public bool ImpersonateExecution { get; set; } + + internal IRuleDirectives RuleDirectives { get; set; } + + private ScriptedRuleWrapper(string ruleName, IAggregatorLogger logger) + { + _logger = logger; + Name = ruleName; + } + + internal ScriptedRuleWrapper(string ruleName, string[] ruleCode) : this(ruleName, new NullLogger()) + { + var parseResult = RuleFileParser.Read(ruleCode); + _ruleFileParseSuccess = parseResult.parseSuccess; + + Initialize(parseResult.ruleDirectives); + } + + public ScriptedRuleWrapper(string ruleName, IRuleDirectives ruleDirectives) : this(ruleName, ruleDirectives, new NullLogger()) + { + } + + public ScriptedRuleWrapper(string ruleName, IRuleDirectives ruleDirectives, IAggregatorLogger logger) : this(ruleName, logger) + { + Initialize(ruleDirectives); + } + + private void Initialize(IRuleDirectives ruleDirectives) + { + RuleDirectives = ruleDirectives; + ImpersonateExecution = RuleDirectives.Impersonate; + + var references = new HashSet(DefaultAssemblyReferences().Concat(RuleDirectives.LoadAssemblyReferences())); + var imports = new HashSet(DefaultImports().Concat(RuleDirectives.Imports)); + + var scriptOptions = ScriptOptions.Default + .WithEmitDebugInformation(true) + .WithReferences(references) + // Add namespaces + .WithImports(imports); + + if (RuleDirectives.IsCSharp()) + { + _roslynScript = CSharpScript.Create( + code: RuleDirectives.GetRuleCode(), + options: scriptOptions, + globalsType: typeof(Globals)); + } + else + { + _logger.WriteError($"Cannot execute rule: language is not supported."); + } + } + + private static IEnumerable DefaultAssemblyReferences() + { + var types = new List() + { + typeof(object), + typeof(System.Linq.Enumerable), + typeof(System.Collections.Generic.CollectionExtensions), + typeof(Microsoft.VisualStudio.Services.WebApi.IdentityRef), + typeof(WorkItemWrapper) + }; + + return types.Select(t => t.Assembly); + } + + private static IEnumerable DefaultImports() + { + var imports = new List + { + "System", + "System.Linq", + "System.Collections.Generic", + "Microsoft.VisualStudio.Services.WebApi", + "aggregator.Engine" + }; + + return imports; + } + + /// + public async Task ApplyAsync(Globals executionContext, CancellationToken cancellationToken) + { + var result = await _roslynScript.RunAsync(executionContext, cancellationToken); + + if (result.Exception != null) + { + _logger.WriteError($"Rule failed with {result.Exception}"); + return new RuleResult() + { + Outcome = RuleExecutionOutcome.Error, + Value = result.Exception.ToString() + }; + } + + _logger.WriteInfo($"Rule succeeded with {result.ReturnValue ?? "no return value"}"); + return new RuleResult() + { + Outcome = RuleExecutionOutcome.Success, + Value = result.ReturnValue // ?? string.Empty + }; + } + + public (bool success, ImmutableArray diagnostics) Verify() + { + if (_ruleFileParseSuccess.HasValue && !_ruleFileParseSuccess.Value) + { + return (false, ImmutableArray.Create()); + } + + // if parsing succeeded try to compile the script and look for errors + ImmutableArray diagnostics = _roslynScript.Compile(); + (bool, ImmutableArray) result; + if (diagnostics.Any()) + { + result = (false, diagnostics); + } + else + { + result = (true, ImmutableArray.Create()); + } + + return result; + } + + } +} + diff --git a/src/unittests-ruleng/RuleEngineTests.cs b/src/unittests-ruleng/RuleEngineTests.cs deleted file mode 100644 index 876e9fac..00000000 --- a/src/unittests-ruleng/RuleEngineTests.cs +++ /dev/null @@ -1,38 +0,0 @@ -using aggregator; -using aggregator.Engine; -using NSubstitute; -using Xunit; - -namespace unittests_ruleng -{ - public class RuleEngineTests - { - [Fact] - public void GivenAnValidRule_WhenTheRuleIsVerified_ThenTheResult_ShouldBeSuccessfull() - { - //Given - var engine = new RuleEngine(Substitute.For(), new [] { "" }, SaveMode.Batch, true); - - //When - var result = engine.VerifyRule(); - - //Then - Assert.True(result.success); - Assert.Empty(result.diagnostics); - } - - [Fact] - public void GivenAnInvalidRule_WhenTheRuleIsVerified_ThenTheResult_ShouldNotBeSuccessfull() - { - //Given - var engine = new RuleEngine(Substitute.For(), new [] { "(" }, SaveMode.Batch, true); - - //When - var result = engine.VerifyRule(); - - //Then - Assert.False(result.success); - Assert.NotEmpty(result.diagnostics); - } - } -} \ No newline at end of file diff --git a/src/unittests-ruleng/RuleFileParserTests.cs b/src/unittests-ruleng/RuleFileParserTests.cs index 57f33025..86bb17a8 100644 --- a/src/unittests-ruleng/RuleFileParserTests.cs +++ b/src/unittests-ruleng/RuleFileParserTests.cs @@ -1,4 +1,6 @@ -using aggregator.Engine.Language; +using System; +using System.Linq; +using aggregator.Engine.Language; using Xunit; @@ -14,7 +16,7 @@ public void RuleLanguageDefaultsCSharp_Succeeds() return $""Hello { self.WorkItemType } #{ self.Id } - { self.Title }!""; "; - (IRuleDirectives directives, bool parsingSuccess, _) = RuleFileParser.Read(ruleCode.Mince()); + (IRuleDirectives directives, bool parsingSuccess) = RuleFileParser.Read(ruleCode.Mince()); Assert.Empty(directives.References); Assert.Empty(directives.Imports); @@ -30,7 +32,7 @@ public void RuleLanguageDefaultsCSharp_Succeeds() [InlineData(".language=CS")] public void RuleLanguageDirectiveParse_Succeeds(string ruleCode, RuleLanguage expectedLanguage = RuleLanguage.Csharp) { - (IRuleDirectives directives, bool parsingSuccess, _) = RuleFileParser.Read(ruleCode.Mince()); + (IRuleDirectives directives, bool parsingSuccess) = RuleFileParser.Read(ruleCode.Mince()); Assert.True(parsingSuccess); Assert.Equal(expectedLanguage, directives.Language); @@ -42,7 +44,7 @@ public void RuleLanguageDirectiveParse_Succeeds(string ruleCode, RuleLanguage ex [InlineData(".lang=C#\r\n.unrecognized=directive\r\nreturn string.Empty;\r\n", RuleLanguage.Csharp)] public void RuleLanguageDirectiveParse_Fails(string ruleCode, RuleLanguage expectedLanguage = RuleLanguage.Unknown) { - (IRuleDirectives directives, bool parsingSuccess, _) = RuleFileParser.Read(ruleCode.Mince()); + (IRuleDirectives directives, bool parsingSuccess) = RuleFileParser.Read(ruleCode.Mince()); Assert.False(parsingSuccess); Assert.Equal(expectedLanguage, directives.Language); @@ -55,7 +57,7 @@ public void RuleLanguageDirectiveParse_Fails(string ruleCode, RuleLanguage expec [InlineData(".reference=System.Xml.XDocument", 1)] public void RuleReferenceDirectiveParse_Succeeds(string ruleCode, int expectedReferenceCount) { - (IRuleDirectives directives, bool parsingSuccess, _) = RuleFileParser.Read(ruleCode.Mince()); + (IRuleDirectives directives, bool parsingSuccess) = RuleFileParser.Read(ruleCode.Mince()); Assert.True(parsingSuccess); Assert.Equal(expectedReferenceCount, directives.References.Count); @@ -67,7 +69,7 @@ public void RuleReferenceDirectiveParse_Succeeds(string ruleCode, int expectedRe [InlineData(".namespace=System.Diagnostics", 1)] public void RuleImportDirectiveParse_Succeeds(string ruleCode, int expectedImportCount) { - (IRuleDirectives directives, bool parsingSuccess, _) = RuleFileParser.Read(ruleCode.Mince()); + (IRuleDirectives directives, bool parsingSuccess) = RuleFileParser.Read(ruleCode.Mince()); Assert.True(parsingSuccess); Assert.Equal(expectedImportCount, directives.Imports.Count); @@ -79,10 +81,27 @@ public void RuleImpersonateDirectiveParse_Succeeds() string ruleCode = @".impersonate=onBehalfOfInitiator "; - (IRuleDirectives directives, bool parsingSuccess, _) = RuleFileParser.Read(ruleCode.Mince()); + (IRuleDirectives directives, bool parsingSuccess) = RuleFileParser.Read(ruleCode.Mince()); Assert.True(parsingSuccess); Assert.True(directives.Impersonate); } + + [Fact] + public void RuleLanguageReadWrite_Succeeds() + { + string ruleCode = @".language=C# +.reference=System.Xml.XDocument +.import=System.Diagnostics + +return $""Hello { self.WorkItemType } #{ self.Id } - { self.Title }!""; +"; + + (IRuleDirectives directives, bool parsingSuccess) = RuleFileParser.Read(ruleCode.Mince()); + + var ruleCode2 = RuleFileParser.Write(directives); + + Assert.Equal(ruleCode, string.Join(Environment.NewLine, ruleCode2), StringComparer.OrdinalIgnoreCase); + } } } diff --git a/src/unittests-ruleng/RuleTests.cs b/src/unittests-ruleng/RuleTests.cs index 44a834d3..8fde70b9 100644 --- a/src/unittests-ruleng/RuleTests.cs +++ b/src/unittests-ruleng/RuleTests.cs @@ -29,6 +29,7 @@ public class RuleTests private readonly IAggregatorLogger logger; private readonly WorkHttpClient workClient; private readonly WorkItemTrackingHttpClient witClient; + private readonly RuleEngine engine; private readonly TestClientsContext clientsContext; public RuleTests() @@ -40,6 +41,8 @@ public RuleTests() workClient = clientsContext.WorkClient; witClient = clientsContext.WitClient; witClient.ExecuteBatchRequest(default).ReturnsForAnyArgs(info => new List()); + + engine = new RuleEngine(logger, SaveMode.Default, dryRun: true); } [Fact] @@ -61,65 +64,13 @@ public async Task HelloWorldRule_Succeeds() return $""Hello { self.WorkItemType } #{ self.Id } - { self.Title }!""; "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); Assert.Equal("Hello Bug #42 - Hello!", result); await witClient.DidNotReceive().GetWorkItemAsync(Arg.Any(), expand: Arg.Any()); } - [Fact] - public async Task LanguageDirective_Succeeds() - { - int workItemId = 42; - WorkItem workItem = new WorkItem - { - Id = workItemId, - Fields = new Dictionary - { - { "System.WorkItemType", "Bug" }, - { "System.Title", "Hello" }, - { "System.TeamProject", clientsContext.ProjectName }, - } - }; - witClient.GetWorkItemAsync(workItemId, expand: WorkItemExpand.All).Returns(workItem); - string ruleCode = @".lang=CS -return string.Empty; -"; - - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); - - Assert.Equal(EngineState.Success, engine.State); - Assert.Equal(string.Empty, result); - await witClient.DidNotReceive().GetWorkItemAsync(Arg.Any(), expand: Arg.Any()); - } - - [Fact] - public async Task LanguageDirective_Fails() - { - int workItemId = 42; - WorkItem workItem = new WorkItem - { - Id = workItemId, - Fields = new Dictionary - { - { "System.WorkItemType", "Bug" }, - { "System.Title", "Hello" }, - { "System.TeamProject", clientsContext.ProjectName }, - } - }; - witClient.GetWorkItemAsync(workItemId, expand: WorkItemExpand.All).Returns(workItem); - string ruleCode = @".lang=WHAT -return string.Empty; -"; - - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); - - Assert.Equal(EngineState.Error, engine.State); - } - [Fact] public async Task Parent_Succeeds() { @@ -171,8 +122,8 @@ public async Task Parent_Succeeds() return message; "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); Assert.Equal("Parent is 1", result); await witClient.Received(1).GetWorkItemAsync(Arg.Is(workItemId1), expand: Arg.Is(WorkItemExpand.All)); @@ -198,8 +149,8 @@ public async Task New_Succeeds() wi.Title = ""Brand new""; "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); Assert.Null(result); logger.Received().WriteInfo($"Found a request for a new Task workitem in {clientsContext.ProjectName}"); @@ -229,9 +180,9 @@ public async Task AddChild_Succeeds() parent.Relations.AddChild(newChild); "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); - + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); + Assert.Null(result); logger.Received().WriteInfo($"Found a request for a new Task workitem in {clientsContext.ProjectName}"); logger.Received().WriteInfo($"Found a request to update workitem {workItemId} in {clientsContext.ProjectName}"); @@ -258,8 +209,8 @@ public async Task TouchDescription_Succeeds() return self.Description; "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); Assert.Equal("Hello.", result); logger.Received().WriteInfo($"Found a request to update workitem {workItemId} in {clientsContext.ProjectName}"); @@ -287,10 +238,9 @@ public async Task ReferenceDirective_Succeeds() return string.Empty; "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); - Assert.Equal(EngineState.Success, engine.State); Assert.Equal(string.Empty, result); } @@ -314,10 +264,9 @@ public async Task ImportDirective_Succeeds() return string.Empty; "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); - Assert.Equal(EngineState.Success, engine.State); Assert.Equal(string.Empty, result); } @@ -341,9 +290,9 @@ public async Task ImportDirective_Fail() return string.Empty; "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); await Assert.ThrowsAsync( - () => engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None) + () => engine.RunAsync(rule, clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None) ); } @@ -382,9 +331,9 @@ public async Task DeleteWorkItem() return string.Empty; "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); await Assert.ThrowsAsync( - () => engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None) + () => engine.RunAsync(rule, clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None) ); } @@ -398,8 +347,8 @@ public async Task HelloWorldRuleOnUpdate_Succeeds() return $""Hello #{ selfChanges.WorkItemId } - Update { selfChanges.Id } changed Title from { selfChanges.Fields[""System.Title""].OldValue } to { selfChanges.Fields[""System.Title""].NewValue }!""; "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, new WorkItemData(workItem, workItemUpdate), clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, new WorkItemData(workItem, workItemUpdate), clientsContext, CancellationToken.None); Assert.Equal("Hello #22 - Update 3 changed Title from Initial Title to Hello!", result); await witClient.DidNotReceive().GetWorkItemAsync(Arg.Any(), expand: Arg.Any()); @@ -424,8 +373,8 @@ public async Task DocumentationRule_OnUpdateExample_Succeeds() } "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, new WorkItemData(workItem, workItemUpdate), clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, new WorkItemData(workItem, workItemUpdate), clientsContext, CancellationToken.None); Assert.Equal("Title was changed from 'Initial Title' to 'Hello'", result); await witClient.DidNotReceive().GetWorkItemAsync(Arg.Any(), expand: Arg.Any()); @@ -452,8 +401,8 @@ public async Task CustomStringField_HasValue_Succeeds() return customField; "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); Assert.Equal("some value", result); } @@ -477,8 +426,8 @@ public async Task CustomStringField_NoValue_ReturnsDefault() return customField; "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); Assert.Equal("MyDefault", result); } @@ -503,8 +452,8 @@ public async Task CustomNumericField_HasValue_Succeeds() return customField.ToString(""N"", System.Globalization.CultureInfo.InvariantCulture); "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); Assert.Equal("42.00", result); } @@ -528,8 +477,8 @@ public async Task CustomNumericField_NoValue_ReturnsDefault() return customField.ToString(""N"", System.Globalization.CultureInfo.InvariantCulture); "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); Assert.Equal("3.00", result); } @@ -588,8 +537,8 @@ public async Task DocumentationRule_BacklogWorkItemsActivateParent_Succeeds() workClient.GetProcessConfigurationAsync(clientsContext.ProjectName).Returns(ExampleTestData.ProcessConfigDefaultAgile); witClient.GetWorkItemAsync(workItemFeature.Id.Value, expand: WorkItemExpand.All).Returns(workItemFeature); - var engine = new RuleEngine(logger, ExampleRuleCode.ActivateParent, SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, new WorkItemData(workItemUS, workItemUpdate), clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ExampleRuleCode.ActivateParent); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, new WorkItemData(workItemUS, workItemUpdate), clientsContext, CancellationToken.None); Assert.Equal("updated Parent Feature #1 to State='Active'", result); } @@ -606,8 +555,8 @@ public async Task DocumentationRule_BacklogWorkItemsResolveParent_Succeeds() workClient.GetProcessConfigurationAsync(clientsContext.ProjectName).Returns(ExampleTestData.ProcessConfigDefaultAgile); witClient.GetWorkItemAsync(workItemFeature.Id.Value, expand: WorkItemExpand.All).Returns(workItemFeature); - var engine = new RuleEngine(logger, ExampleRuleCode.ResolveParent, SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, new WorkItemData(workItemUS, workItemUpdate), clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ExampleRuleCode.ResolveParent); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, new WorkItemData(workItemUS, workItemUpdate), clientsContext, CancellationToken.None); Assert.Equal("updated Parent #1 to State='Resolved'", result); } @@ -627,8 +576,8 @@ public async Task DocumentationRule_BacklogWorkItemsResolveParent_FailsDueToChil witClient.GetWorkItemAsync(workItemFeature.Id.Value, expand: WorkItemExpand.All).Returns(workItemFeature); witClient.GetWorkItemsAsync(Arg.Is>(ints => ints.Single() == workItemUS3.Id.Value), expand: WorkItemExpand.All).Returns(new List() { workItemUS3 }); - var engine = new RuleEngine(logger, ExampleRuleCode.ResolveParent, SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, new WorkItemData(workItemUS2, workItemUpdate), clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ExampleRuleCode.ResolveParent); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, new WorkItemData(workItemUS2, workItemUpdate), clientsContext, CancellationToken.None); Assert.Equal("Not all child work items or : #2=Closed,#3=Active", result); } diff --git a/src/unittests-ruleng/ScriptedRuleWrapperTests.cs b/src/unittests-ruleng/ScriptedRuleWrapperTests.cs new file mode 100644 index 00000000..5bda1fb3 --- /dev/null +++ b/src/unittests-ruleng/ScriptedRuleWrapperTests.cs @@ -0,0 +1,53 @@ +using aggregator.Engine; +using Xunit; + +namespace unittests_ruleng +{ + public class ScriptedRuleWrapperTests + { + [Fact] + public void GivenAnValidRule_WhenTheRuleIsVerified_ThenTheResult_ShouldBeSuccessfull() + { + //Given + var rule = new ScriptedRuleWrapper("dummy", new[] { "" }); + + + //When + var result = rule.Verify(); + + //Then + Assert.True(result.success); + Assert.Empty(result.diagnostics); + } + + [Fact] + public void GivenAnNotCompilableRule_WhenTheRuleIsVerified_ThenTheResult_ShouldNotBeSuccessfull() + { + //Given + var rule = new ScriptedRuleWrapper("dummy", new[] { "(" }); + + + //When + var result = rule.Verify(); + + //Then + Assert.False(result.success); + Assert.NotEmpty(result.diagnostics); + } + + [Fact] + public void GivenAnNotParsableRule_WhenTheRuleIsVerified_ThenTheResult_ShouldNotBeSuccessfull() + { + //Given + var rule = new ScriptedRuleWrapper("dummy", new[] { ".invalid=directive" }); + + + //When + var result = rule.Verify(); + + //Then + Assert.False(result.success); + Assert.Empty(result.diagnostics); + } + } +} \ No newline at end of file From 1c49df1abc54ae20019d3d53fc11a9c8fc738778 Mon Sep 17 00:00:00 2001 From: BobSilent Date: Mon, 9 Sep 2019 00:16:36 +0200 Subject: [PATCH 16/20] Show impersonation information when listing rules therefore changed signature of ListRules from KuduFunction to RuleOutputData --- src/aggregator-cli/Rules/AggregatorRules.cs | 58 ++++++++++++++++---- src/aggregator-cli/Rules/ListRulesCommand.cs | 4 +- src/aggregator-cli/Rules/RuleOutputData.cs | 14 +++-- 3 files changed, 59 insertions(+), 17 deletions(-) diff --git a/src/aggregator-cli/Rules/AggregatorRules.cs b/src/aggregator-cli/Rules/AggregatorRules.cs index 8b31cb37..a379214c 100644 --- a/src/aggregator-cli/Rules/AggregatorRules.cs +++ b/src/aggregator-cli/Rules/AggregatorRules.cs @@ -29,29 +29,67 @@ public AggregatorRules(IAzure azure, ILogger logger) _logger = logger; } - internal async Task> ListAsync(InstanceName instance, CancellationToken cancellationToken) + internal async Task> ListAsync(InstanceName instance, CancellationToken cancellationToken) { var kudu = new KuduApi(instance, _azure, _logger); _logger.WriteInfo($"Retrieving Functions in {instance.PlainName}..."); using (var client = new HttpClient()) - using (var request = await kudu.GetRequestAsync(HttpMethod.Get, $"api/functions", cancellationToken)) - using (var response = await client.SendAsync(request, cancellationToken)) { - var stream = await response.Content.ReadAsStreamAsync(); - - if (response.IsSuccessStatusCode) + KuduFunction[] kuduFunctions; + using (var request = await kudu.GetRequestAsync(HttpMethod.Get, $"api/functions", cancellationToken)) + using (var response = await client.SendAsync(request, cancellationToken)) { + var stream = await response.Content.ReadAsStreamAsync(); + + if (!response.IsSuccessStatusCode) + { + _logger.WriteError($"{response.ReasonPhrase} {await response.Content.ReadAsStringAsync()}"); + return Enumerable.Empty(); + } + using (var sr = new StreamReader(stream)) using (var jtr = new JsonTextReader(sr)) { var js = new JsonSerializer(); - var functionList = js.Deserialize(jtr); - return functionList; + kuduFunctions = js.Deserialize(jtr); + } + } + + List ruleData = new List(); + foreach (var kuduFunction in kuduFunctions) + { + var ruleName = kuduFunction.Name; + var ruleFileUrl = $"api/vfs/site/wwwroot/{ruleName}/{ruleName}.rule"; + _logger.WriteInfo($"Retrieving Function Rule Details {ruleName}..."); + + using (var request = await kudu.GetRequestAsync(HttpMethod.Get, ruleFileUrl, cancellationToken)) + using (var response = await client.SendAsync(request, cancellationToken)) + { + var stream = await response.Content.ReadAsStreamAsync(); + + if (!response.IsSuccessStatusCode) + { + _logger.WriteError($"{response.ReasonPhrase} {await response.Content.ReadAsStringAsync()}"); + continue; + } + + + var ruleCode = new List(); + using (var sr = new StreamReader(stream)) + { + string line; + while ((line = sr.ReadLine()) != null) + { + ruleCode.Add(line); + } + } + + var (ruleDirectives, _) = RuleFileParser.Read(ruleCode.ToArray()); + ruleData.Add(new RuleOutputData(instance, kuduFunction, ruleDirectives.Impersonate)); } } - _logger.WriteError($"{response.ReasonPhrase} {await response.Content.ReadAsStringAsync()}"); - return new KuduFunction[0]; + return ruleData; } } diff --git a/src/aggregator-cli/Rules/ListRulesCommand.cs b/src/aggregator-cli/Rules/ListRulesCommand.cs index 1e4ecc57..2cb92e39 100644 --- a/src/aggregator-cli/Rules/ListRulesCommand.cs +++ b/src/aggregator-cli/Rules/ListRulesCommand.cs @@ -24,10 +24,10 @@ internal override async Task RunAsync(CancellationToken cancellationToken) var instance = new InstanceName(Instance, ResourceGroup); var rules = new AggregatorRules(context.Azure, context.Logger); bool any = false; - foreach (var item in await rules.ListAsync(instance, cancellationToken)) + foreach (var ruleInformation in await rules.ListAsync(instance, cancellationToken)) { cancellationToken.ThrowIfCancellationRequested(); - context.Logger.WriteOutput(new RuleOutputData(instance, item)); + context.Logger.WriteOutput(ruleInformation); any = true; } diff --git a/src/aggregator-cli/Rules/RuleOutputData.cs b/src/aggregator-cli/Rules/RuleOutputData.cs index cf50e526..0f0aea61 100644 --- a/src/aggregator-cli/Rules/RuleOutputData.cs +++ b/src/aggregator-cli/Rules/RuleOutputData.cs @@ -6,18 +6,22 @@ namespace aggregator.cli { internal class RuleOutputData : ILogDataObject { - string instanceName; - KuduFunction function; + private readonly string instanceName; + private readonly string ruleName; + private readonly bool isDisabled; + private readonly bool isImpersonated; - internal RuleOutputData(InstanceName instance, KuduFunction function) + internal RuleOutputData(InstanceName instance, KuduFunction function, bool isImpersonated) { this.instanceName = instance.PlainName; - this.function = function; + this.ruleName = function.Name; + this.isDisabled = function.Config.Disabled; + this.isImpersonated = isImpersonated; } public string AsHumanReadable() { - return $"Rule {instanceName}/{function.Name} {(function.Config.Disabled ? "(disabled)" : string.Empty)}"; + return $"Rule {instanceName}/{ruleName} {(isImpersonated ? "*execute impersonated*" : string.Empty)} {(isDisabled ? "(disabled)" : string.Empty)}"; } } } From 5e6f59115d9fcaee9a46db1723fd2866624a4e5e Mon Sep 17 00:00:00 2001 From: BobSilent Date: Sun, 15 Sep 2019 00:23:26 +0200 Subject: [PATCH 17/20] Add possibility to impersonate when adding or updating a rule --- .../Instances/FunctionRuntimePackage.cs | 12 +++++++----- src/aggregator-cli/Rules/AddRuleCommand.cs | 5 ++++- src/aggregator-cli/Rules/AggregatorRules.cs | 10 +++++++--- src/aggregator-cli/Rules/UpdateRuleCommand.cs | 5 ++++- src/aggregator-ruleng/Language/IRuleDirectives.cs | 2 +- src/aggregator-ruleng/Language/RuleDirectives.cs | 2 +- 6 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/aggregator-cli/Instances/FunctionRuntimePackage.cs b/src/aggregator-cli/Instances/FunctionRuntimePackage.cs index 740ffa51..932ea601 100644 --- a/src/aggregator-cli/Instances/FunctionRuntimePackage.cs +++ b/src/aggregator-cli/Instances/FunctionRuntimePackage.cs @@ -215,9 +215,14 @@ internal async Task GetDeployedRuntimeVersion(InstanceName instance, private async Task GetLocalPackageVersionAsync(string runtimePackageFile) { - if (File.Exists(runtimePackageFile)) + if (!File.Exists(runtimePackageFile)) + { + // this default allows SemVer to parse and compare + return new SemVersion(0, 0); + } + + using (var zip = ZipFile.OpenRead(runtimePackageFile)) { - var zip = ZipFile.OpenRead(runtimePackageFile); var manifestEntry = zip.GetEntry("aggregator-manifest.ini"); using (var byteStream = manifestEntry.Open()) using (var reader = new StreamReader(byteStream)) @@ -227,9 +232,6 @@ private async Task GetLocalPackageVersionAsync(string runtimePackage return info.Version; } } - - // this default allows SemVer to parse and compare - return new SemVersion(0, 0); } private async Task<(string name, DateTimeOffset? when, string url)> FindVersionInGitHubAsync(string tag = "latest") diff --git a/src/aggregator-cli/Rules/AddRuleCommand.cs b/src/aggregator-cli/Rules/AddRuleCommand.cs index bd6c056c..a93f7298 100644 --- a/src/aggregator-cli/Rules/AddRuleCommand.cs +++ b/src/aggregator-cli/Rules/AddRuleCommand.cs @@ -22,6 +22,9 @@ class AddRuleCommand : CommandBase [Option('f', "file", Required = true, HelpText = "Aggregator rule code.")] public string File { get; set; } + [Option("impersonate", Required = false, HelpText = "Do rule changes on behalf of the person triggered the rule execution. See wiki for details, requires special account privileges.")] + public bool ImpersonateExecution { get; set; } + internal override async Task RunAsync(CancellationToken cancellationToken) { var context = await Context @@ -29,7 +32,7 @@ internal override async Task RunAsync(CancellationToken cancellationToken) .BuildAsync(cancellationToken); var instance = new InstanceName(Instance, ResourceGroup); var rules = new AggregatorRules(context.Azure, context.Logger); - bool ok = await rules.AddAsync(instance, Name, File, cancellationToken); + bool ok = await rules.AddAsync(instance, Name, File, ImpersonateExecution, cancellationToken); return ok ? 0 : 1; } } diff --git a/src/aggregator-cli/Rules/AggregatorRules.cs b/src/aggregator-cli/Rules/AggregatorRules.cs index a379214c..9c5e6e41 100644 --- a/src/aggregator-cli/Rules/AggregatorRules.cs +++ b/src/aggregator-cli/Rules/AggregatorRules.cs @@ -130,12 +130,16 @@ internal static Uri GetInvocationUrl(InstanceName instance, string rule) } } - internal async Task AddAsync(InstanceName instance, string name, string filePath, CancellationToken cancellationToken) + internal async Task AddAsync(InstanceName instance, string name, string filePath, bool impersonate, CancellationToken cancellationToken) { _logger.WriteInfo($"Validate rule file {filePath}"); var engineLogger = new EngineWrapperLogger(_logger); var (ruleDirectives, _) = await RuleFileParser.ReadFile(filePath, engineLogger, cancellationToken); + if (impersonate) + { + ruleDirectives.Impersonate = true; + } try { var rule = new Engine.ScriptedRuleWrapper(name, ruleDirectives, engineLogger); @@ -317,14 +321,14 @@ internal async Task EnableAsync(InstanceName instance, string name, bool d return true; } - internal async Task UpdateAsync(InstanceName instance, string name, string filePath, string requiredVersion, string sourceUrl, CancellationToken cancellationToken) + internal async Task UpdateAsync(InstanceName instance, string name, string filePath, bool impersonate, string requiredVersion, string sourceUrl, CancellationToken cancellationToken) { // check runtime package var package = new FunctionRuntimePackage(_logger); bool ok = await package.UpdateVersionAsync(requiredVersion, sourceUrl, instance, _azure, cancellationToken); if (ok) { - ok = await AddAsync(instance, name, filePath, cancellationToken); + ok = await AddAsync(instance, name, filePath, impersonate, cancellationToken); } return ok; diff --git a/src/aggregator-cli/Rules/UpdateRuleCommand.cs b/src/aggregator-cli/Rules/UpdateRuleCommand.cs index 55e4ed7f..5cecedfc 100644 --- a/src/aggregator-cli/Rules/UpdateRuleCommand.cs +++ b/src/aggregator-cli/Rules/UpdateRuleCommand.cs @@ -22,6 +22,9 @@ class UpdateRuleCommand : CommandBase [Option('f', "file", Required = true, HelpText = "Aggregator rule code.")] public string File { get; set; } + [Option("impersonate", Required = false, HelpText = "Do rule changes on behalf of the person triggered the rule execution. See wiki for details, requires special account privileges.")] + public bool ImpersonateExecution { get; set; } + [Option("requiredVersion", SetName = "nourl", Required = false, HelpText = "Version of Aggregator Runtime required.")] public string RequiredVersion { get; set; } @@ -35,7 +38,7 @@ internal override async Task RunAsync(CancellationToken cancellationToken) .BuildAsync(cancellationToken); var instance = new InstanceName(Instance, ResourceGroup); var rules = new AggregatorRules(context.Azure, context.Logger); - bool ok = await rules.UpdateAsync(instance, Name, File, RequiredVersion, SourceUrl, cancellationToken); + bool ok = await rules.UpdateAsync(instance, Name, File, ImpersonateExecution, RequiredVersion, SourceUrl, cancellationToken); return ok ? 0 : 1; } } diff --git a/src/aggregator-ruleng/Language/IRuleDirectives.cs b/src/aggregator-ruleng/Language/IRuleDirectives.cs index 25b37b85..cc7b30bd 100644 --- a/src/aggregator-ruleng/Language/IRuleDirectives.cs +++ b/src/aggregator-ruleng/Language/IRuleDirectives.cs @@ -4,7 +4,7 @@ namespace aggregator.Engine.Language { public interface IRuleDirectives { - bool Impersonate { get; } + bool Impersonate { get; set; } RuleLanguage Language { get; } IReadOnlyList References { get; } IReadOnlyList Imports { get; } diff --git a/src/aggregator-ruleng/Language/RuleDirectives.cs b/src/aggregator-ruleng/Language/RuleDirectives.cs index dde335dc..c872a77f 100644 --- a/src/aggregator-ruleng/Language/RuleDirectives.cs +++ b/src/aggregator-ruleng/Language/RuleDirectives.cs @@ -14,7 +14,7 @@ public RuleDirectives() RuleCode = new List(); } - public bool Impersonate { get; internal set; } + public bool Impersonate { get; set; } public RuleLanguage Language { get; internal set; } From fa7aeb9bb5ea724f722787d1afbcc602a228af6e Mon Sep 17 00:00:00 2001 From: BobSilent Date: Sun, 15 Sep 2019 13:43:38 +0200 Subject: [PATCH 18/20] fix: Display disabled rules, see ea9d05f5c5b77a66d09908ee88cc5d3e8f3f0ce1 --- src/aggregator-cli/Rules/AggregatorRules.cs | 32 ++++++++++++++++----- src/aggregator-cli/Rules/RuleOutputData.cs | 6 ++-- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/aggregator-cli/Rules/AggregatorRules.cs b/src/aggregator-cli/Rules/AggregatorRules.cs index 9c5e6e41..f3761522 100644 --- a/src/aggregator-cli/Rules/AggregatorRules.cs +++ b/src/aggregator-cli/Rules/AggregatorRules.cs @@ -9,6 +9,7 @@ using System.Threading; using System.Threading.Tasks; using aggregator.Engine.Language; +using Microsoft.Azure.Management.AppService.Fluent; using Microsoft.Azure.Management.Fluent; using Microsoft.TeamFoundation.Core.WebApi; using Microsoft.TeamFoundation.WorkItemTracking.WebApi.Models; @@ -33,6 +34,10 @@ internal async Task> ListAsync(InstanceName instance { var kudu = new KuduApi(instance, _azure, _logger); _logger.WriteInfo($"Retrieving Functions in {instance.PlainName}..."); + + var webFunctionApp = await GetWebApp(instance, cancellationToken); + cancellationToken.ThrowIfCancellationRequested(); + using (var client = new HttpClient()) { KuduFunction[] kuduFunctions; @@ -84,8 +89,9 @@ internal async Task> ListAsync(InstanceName instance } } + var isDisabled = IsDisabled(webFunctionApp, ruleName); var (ruleDirectives, _) = RuleFileParser.Read(ruleCode.ToArray()); - ruleData.Add(new RuleOutputData(instance, kuduFunction, ruleDirectives.Impersonate)); + ruleData.Add(new RuleOutputData(instance, ruleName, isDisabled, ruleDirectives.Impersonate)); } } @@ -93,6 +99,12 @@ internal async Task> ListAsync(InstanceName instance } } + private static bool IsDisabled(IWebApp webFunctionApp, string ruleName) + { + var value = webFunctionApp.GetAppSettings().GetValueOrDefault($"AzureWebJobs.{ruleName}.Disabled")?.Value ?? "false"; + return Boolean.TryParse(value, out bool result) && result; + } + internal static Uri GetInvocationUrl(InstanceName instance, string rule) { var url = $"{instance.FunctionAppUrl}/api/{rule}"; @@ -305,6 +317,17 @@ internal async Task RemoveAsync(InstanceName instance, string name, Cancel } internal async Task EnableAsync(InstanceName instance, string name, bool disable, CancellationToken cancellationToken) + { + var webFunctionApp = await GetWebApp(instance, cancellationToken); + webFunctionApp + .Update() + .WithAppSetting($"AzureWebJobs.{name}.Disabled", disable.ToString().ToLower()) + .Apply(); + + return true; + } + + private async Task GetWebApp(InstanceName instance, CancellationToken cancellationToken) { var webFunctionApp = await _azure .AppServices @@ -313,12 +336,7 @@ internal async Task EnableAsync(InstanceName instance, string name, bool d instance.ResourceGroupName, instance.FunctionAppName, cancellationToken); cancellationToken.ThrowIfCancellationRequested(); - webFunctionApp - .Update() - .WithAppSetting($"AzureWebJobs.{name}.Disabled", disable.ToString().ToLower()) - .Apply(); - - return true; + return webFunctionApp; } internal async Task UpdateAsync(InstanceName instance, string name, string filePath, bool impersonate, string requiredVersion, string sourceUrl, CancellationToken cancellationToken) diff --git a/src/aggregator-cli/Rules/RuleOutputData.cs b/src/aggregator-cli/Rules/RuleOutputData.cs index 0f0aea61..7351bdf1 100644 --- a/src/aggregator-cli/Rules/RuleOutputData.cs +++ b/src/aggregator-cli/Rules/RuleOutputData.cs @@ -11,11 +11,11 @@ internal class RuleOutputData : ILogDataObject private readonly bool isDisabled; private readonly bool isImpersonated; - internal RuleOutputData(InstanceName instance, KuduFunction function, bool isImpersonated) + internal RuleOutputData(InstanceName instance, string ruleName, bool isDisabled, bool isImpersonated) { this.instanceName = instance.PlainName; - this.ruleName = function.Name; - this.isDisabled = function.Config.Disabled; + this.ruleName = ruleName; + this.isDisabled = isDisabled; this.isImpersonated = isImpersonated; } From 4d3ea3622291da3ddcaacda16e5cfb240a558cd6 Mon Sep 17 00:00:00 2001 From: BobSilent Date: Mon, 16 Sep 2019 13:18:11 +0200 Subject: [PATCH 19/20] Change Impersonation Configuration RuleCode allows setting default via directive Configure Impersonation via rule.configure remove impersonation option from add.rule, update.rule reduce compiler messages Extract some types into separate files and beautify code --- doc/command-examples.md | 20 +- doc/rule-language.md | 2 +- doc/setup.md | 5 +- src/.editorconfig | 197 ++++++++++++++++++ src/aggregator-cli/AzureBaseClass.cs | 41 ++++ .../Instances/AggregatorInstances.cs | 32 ++- src/aggregator-cli/Logon/DevOpsLogon.cs | 1 + .../Mappings/AggregatorMappings.cs | 2 +- src/aggregator-cli/Rules/AddRuleCommand.cs | 5 +- src/aggregator-cli/Rules/AggregatorRules.cs | 126 ++++++----- .../Rules/ConfigureRuleCommand.cs | 50 ++++- src/aggregator-cli/Rules/InvokeRuleCommand.cs | 1 + src/aggregator-cli/Rules/RuleOutputData.cs | 11 +- src/aggregator-cli/Rules/UpdateRuleCommand.cs | 5 +- .../AzureFunctionHandler.cs | 4 +- src/aggregator-ruleng/IRule.cs | 27 +++ src/aggregator-ruleng/IRuleResult.cs | 31 +++ .../Language/RuleDirectivesExtensions.cs | 2 +- src/aggregator-ruleng/RuleEngine.cs | 8 +- .../{Globals.cs => RuleExecutionContext.cs} | 2 +- src/aggregator-ruleng/RuleExecutor.cs | 4 +- src/aggregator-ruleng/ScriptedRuleWrapper.cs | 76 ++----- .../AggregatorConfiguration.cs | 140 +++++++++++-- src/aggregator-shared/InvokeOptions.cs | 57 +++-- .../Model/AggregatorConfiguration.cs | 27 +++ .../Model/RuleConfiguration.cs | 17 ++ .../AzureFunctionHandlerTests.cs | 2 +- src/unittests-ruleng/RuleFileParserTests.cs | 2 +- src/unittests-ruleng/RuleTests.cs | 12 +- .../TestData/ExampleTestData.cs | 5 +- 30 files changed, 689 insertions(+), 225 deletions(-) create mode 100644 src/.editorconfig create mode 100644 src/aggregator-cli/AzureBaseClass.cs create mode 100644 src/aggregator-ruleng/IRule.cs create mode 100644 src/aggregator-ruleng/IRuleResult.cs rename src/aggregator-ruleng/{Globals.cs => RuleExecutionContext.cs} (88%) create mode 100644 src/aggregator-shared/Model/AggregatorConfiguration.cs create mode 100644 src/aggregator-shared/Model/RuleConfiguration.cs diff --git a/doc/command-examples.md b/doc/command-examples.md index 09e1fdd2..c3ffd5df 100644 --- a/doc/command-examples.md +++ b/doc/command-examples.md @@ -82,13 +82,29 @@ list.mappings --instance my1 --project SampleProject ``` -### Disable and enable rules -Disabling a broken rule leaves any mappings in place. +### Configuring rules + +Configuring a rule leaves any mappings in place. + +#### Disable and enable rules + +e.g. disabling a broken rule ```Batchfile configure.rule --verbose --instance my1 --name test1 --disable configure.rule --verbose --instance my1 --name test1 --enable ``` +#### Execute Impersonated + +configure a rule to run impersonated +**Attention:** To use this the identify accessing Azure DevOps needs special permissions, +see [Rule Examples](setup.md#azure-devops-personal-access-token--PAT-). + +```Batchfile +configure.rule --verbose --instance my1 --name test1 --disableImpersonate +configure.rule --verbose --instance my1 --name test1 --enableImpersonate +``` + ### Update the code and runtime of a rule This command updates the code and potentially the Aggregator runtime diff --git a/doc/rule-language.md b/doc/rule-language.md index ef3eb2b4..6a0e1b44 100644 --- a/doc/rule-language.md +++ b/doc/rule-language.md @@ -25,7 +25,7 @@ Equivalent to C# namespace Aggregator uses credentials for accessing Azure DevOps. By default the changes which were saved back to Azure DevOps are done with the credentials provided for accessing Azure DevOps. -In order to do the changes on behalf of the account who initiated an event, which Aggregator will handle, +In order to do the changes on behalf of the account who initiated an event, which Aggregator is going to handle, specify `.impersonate=onBehalfOfInitiator` diff --git a/doc/setup.md b/doc/setup.md index 2025a477..3042001d 100644 --- a/doc/setup.md +++ b/doc/setup.md @@ -67,7 +67,10 @@ In Azure Portal you can check the permissons in the IAM menu for the selected Re A PAT has the same or less permissions than the person/identity that creates it. We recommend that the PAT is issued by an Azure DevOps Organization Administrator Identity. -When using the [impersonate directive](rule-language.md#impersonate-directive) +When using the [impersonate directive](rule-language.md#impersonate-directive), +[mapping a rule](command-examples.md#adds-two-service-hooks-to-azure-devops--each-invoking-a-different-rule) +to execute impersonated or +[configuring a rule impersonated](command-examples.md#), the used identity for creating the PAT must have the permission: "Bypass rules on work item updates" diff --git a/src/.editorconfig b/src/.editorconfig new file mode 100644 index 00000000..0f6a7bac --- /dev/null +++ b/src/.editorconfig @@ -0,0 +1,197 @@ +# Remove the line below if you want to inherit .editorconfig settings from higher directories +root = true + +# C# files +[*.cs] + +#### Core EditorConfig Options #### + +# Indentation and spacing +indent_size = 4 +indent_style = space +tab_width = 4 + +# New line preferences +end_of_line = crlf +insert_final_newline = false + +#### .NET Coding Conventions #### + +# Organize usings +dotnet_separate_import_directive_groups = false +dotnet_sort_system_directives_first = false + +# this. and Me. preferences +dotnet_style_qualification_for_event = false:silent +dotnet_style_qualification_for_field = false:silent +dotnet_style_qualification_for_method = false:silent +dotnet_style_qualification_for_property = false:silent + +# Language keywords vs BCL types preferences +dotnet_style_predefined_type_for_locals_parameters_members = true:silent +dotnet_style_predefined_type_for_member_access = true:silent + +# Parentheses preferences +dotnet_style_parentheses_in_arithmetic_binary_operators = always_for_clarity:silent +dotnet_style_parentheses_in_other_binary_operators = always_for_clarity:silent +dotnet_style_parentheses_in_other_operators = never_if_unnecessary:silent +dotnet_style_parentheses_in_relational_binary_operators = always_for_clarity:silent + +# Modifier preferences +dotnet_style_require_accessibility_modifiers = for_non_interface_members:silent + +# Expression-level preferences +csharp_style_deconstructed_variable_declaration = true:suggestion +csharp_style_inlined_variable_declaration = true:suggestion +csharp_style_throw_expression = true:suggestion +dotnet_style_coalesce_expression = true:suggestion +dotnet_style_collection_initializer = true:suggestion +dotnet_style_explicit_tuple_names = true:suggestion +dotnet_style_null_propagation = true:suggestion +dotnet_style_object_initializer = true:suggestion +dotnet_style_prefer_auto_properties = true:suggestion +dotnet_style_prefer_compound_assignment = true:suggestion +dotnet_style_prefer_conditional_expression_over_assignment = true:silent +dotnet_style_prefer_conditional_expression_over_return = true:silent +dotnet_style_prefer_inferred_anonymous_type_member_names = true:suggestion +dotnet_style_prefer_inferred_tuple_names = true:suggestion +dotnet_style_prefer_is_null_check_over_reference_equality_method = true:suggestion + +# Field preferences +dotnet_style_readonly_field = true:suggestion + +# Parameter preferences +dotnet_code_quality_unused_parameters = all:suggestion + +#### C# Coding Conventions #### + +# var preferences +csharp_style_var_elsewhere = false:silent +csharp_style_var_for_built_in_types = false:silent +csharp_style_var_when_type_is_apparent = false:silent + +# Expression-bodied members +csharp_style_expression_bodied_accessors = true:silent +csharp_style_expression_bodied_constructors = false:silent +csharp_style_expression_bodied_indexers = true:silent +csharp_style_expression_bodied_lambdas = true:silent +csharp_style_expression_bodied_local_functions = false:silent +csharp_style_expression_bodied_methods = false:silent +csharp_style_expression_bodied_operators = false:silent +csharp_style_expression_bodied_properties = true:silent + +# Pattern matching preferences +csharp_style_pattern_matching_over_as_with_null_check = true:suggestion +csharp_style_pattern_matching_over_is_with_cast_check = true:suggestion +csharp_style_prefer_switch_expression = true:suggestion + +# Null-checking preferences +csharp_style_conditional_delegate_call = true:suggestion + +# Modifier preferences +csharp_prefer_static_local_function = true:suggestion +csharp_preferred_modifier_order = public,private,protected,internal,static,extern,new,virtual,abstract,sealed,override,readonly,unsafe,volatile,async + +# Code-block preferences +csharp_prefer_braces = true:warning +csharp_prefer_simple_using_statement = false:silent + +# Expression-level preferences +csharp_prefer_simple_default_expression = true:suggestion +csharp_style_pattern_local_over_anonymous_function = true:suggestion +csharp_style_prefer_index_operator = true:suggestion +csharp_style_prefer_range_operator = true:suggestion +csharp_style_unused_value_assignment_preference = discard_variable:suggestion +csharp_style_unused_value_expression_statement_preference = discard_variable:silent + +# 'using' directive preferences +csharp_using_directive_placement = outside_namespace:suggestion + +#### C# Formatting Rules #### + +# New line preferences +csharp_new_line_before_catch = true +csharp_new_line_before_else = true +csharp_new_line_before_finally = true +csharp_new_line_before_members_in_anonymous_types = true +csharp_new_line_before_members_in_object_initializers = true +csharp_new_line_before_open_brace = all +csharp_new_line_between_query_expression_clauses = true + +# Indentation preferences +csharp_indent_block_contents = true +csharp_indent_braces = false +csharp_indent_case_contents = true +csharp_indent_case_contents_when_block = true +csharp_indent_labels = no_change +csharp_indent_switch_labels = true + +# Space preferences +csharp_space_after_cast = false +csharp_space_after_colon_in_inheritance_clause = true +csharp_space_after_comma = true +csharp_space_after_dot = false +csharp_space_after_keywords_in_control_flow_statements = true +csharp_space_after_semicolon_in_for_statement = true +csharp_space_around_binary_operators = before_and_after +csharp_space_around_declaration_statements = false +csharp_space_before_colon_in_inheritance_clause = true +csharp_space_before_comma = false +csharp_space_before_dot = false +csharp_space_before_open_square_brackets = false +csharp_space_before_semicolon_in_for_statement = false +csharp_space_between_empty_square_brackets = false +csharp_space_between_method_call_empty_parameter_list_parentheses = false +csharp_space_between_method_call_name_and_opening_parenthesis = false +csharp_space_between_method_call_parameter_list_parentheses = false +csharp_space_between_method_declaration_empty_parameter_list_parentheses = false +csharp_space_between_method_declaration_name_and_open_parenthesis = false +csharp_space_between_method_declaration_parameter_list_parentheses = false +csharp_space_between_parentheses = false +csharp_space_between_square_brackets = false + +# Wrapping preferences +csharp_preserve_single_line_blocks = true +csharp_preserve_single_line_statements = true + +#### Naming styles #### + +# Naming rules + +dotnet_naming_rule.interface_should_be_begins_with_i.severity = suggestion +dotnet_naming_rule.interface_should_be_begins_with_i.symbols = interface +dotnet_naming_rule.interface_should_be_begins_with_i.style = begins_with_i + +dotnet_naming_rule.types_should_be_pascal_case.severity = suggestion +dotnet_naming_rule.types_should_be_pascal_case.symbols = types +dotnet_naming_rule.types_should_be_pascal_case.style = pascal_case + +dotnet_naming_rule.non_field_members_should_be_pascal_case.severity = suggestion +dotnet_naming_rule.non_field_members_should_be_pascal_case.symbols = non_field_members +dotnet_naming_rule.non_field_members_should_be_pascal_case.style = pascal_case + +# Symbol specifications + +dotnet_naming_symbols.interface.applicable_kinds = interface +dotnet_naming_symbols.interface.applicable_accessibilities = public, internal, private, protected, protected_internal, private_protected +dotnet_naming_symbols.interface.required_modifiers = + +dotnet_naming_symbols.types.applicable_kinds = class, struct, interface, enum +dotnet_naming_symbols.types.applicable_accessibilities = public, internal, private, protected, protected_internal, private_protected +dotnet_naming_symbols.types.required_modifiers = + +dotnet_naming_symbols.non_field_members.applicable_kinds = property, event, method +dotnet_naming_symbols.non_field_members.applicable_accessibilities = public, internal, private, protected, protected_internal, private_protected +dotnet_naming_symbols.non_field_members.required_modifiers = + +# Naming styles + +dotnet_naming_style.pascal_case.required_prefix = +dotnet_naming_style.pascal_case.required_suffix = +dotnet_naming_style.pascal_case.word_separator = +dotnet_naming_style.pascal_case.capitalization = pascal_case + +dotnet_naming_style.begins_with_i.required_prefix = I +dotnet_naming_style.begins_with_i.required_suffix = +dotnet_naming_style.begins_with_i.word_separator = +dotnet_naming_style.begins_with_i.capitalization = pascal_case diff --git a/src/aggregator-cli/AzureBaseClass.cs b/src/aggregator-cli/AzureBaseClass.cs new file mode 100644 index 00000000..6e29bbed --- /dev/null +++ b/src/aggregator-cli/AzureBaseClass.cs @@ -0,0 +1,41 @@ +using System.Threading; +using System.Threading.Tasks; + +using Microsoft.Azure.Management.AppService.Fluent; +using Microsoft.Azure.Management.Fluent; + + +namespace aggregator.cli { + internal abstract class AzureBaseClass + { + protected IAzure _azure; + + protected ILogger _logger; + + protected AzureBaseClass(IAzure azure, ILogger logger) + { + _azure = azure; + _logger = logger; + } + + + protected async Task GetWebApp(InstanceName instance, CancellationToken cancellationToken) + { + var webFunctionApp = await _azure + .AppServices + .WebApps + .GetByResourceGroupAsync( + instance.ResourceGroupName, + instance.FunctionAppName, cancellationToken); + cancellationToken.ThrowIfCancellationRequested(); + return webFunctionApp; + } + + + protected KuduApi GetKudu(InstanceName instance) + { + var kudu = new KuduApi(instance, _azure, _logger); + return kudu; + } + } +} \ No newline at end of file diff --git a/src/aggregator-cli/Instances/AggregatorInstances.cs b/src/aggregator-cli/Instances/AggregatorInstances.cs index 0ee62345..b41992d9 100644 --- a/src/aggregator-cli/Instances/AggregatorInstances.cs +++ b/src/aggregator-cli/Instances/AggregatorInstances.cs @@ -12,16 +12,13 @@ namespace aggregator.cli { - class AggregatorInstances + class AggregatorInstances : AzureBaseClass { private readonly IAzure azure; private readonly ILogger logger; - public AggregatorInstances(IAzure azure, ILogger logger) - { - this.azure = azure; - this.logger = logger; - } + public AggregatorInstances(IAzure azure, ILogger logger) : base(azure, logger) + { } public async Task> ListAllAsync(CancellationToken cancellationToken) { @@ -218,19 +215,14 @@ private async Task DeployArmTemplateAsync(InstanceName instance, string lo internal async Task ChangeAppSettingsAsync(InstanceName instance, DevOpsLogon devopsLogonData, SaveMode saveMode, CancellationToken cancellationToken) { - var webFunctionApp = await azure - .AppServices - .WebApps - .GetByResourceGroupAsync( - instance.ResourceGroupName, - instance.FunctionAppName, cancellationToken); - var configuration = new AggregatorConfiguration - { - DevOpsTokenType = devopsLogonData.Mode, - DevOpsToken = devopsLogonData.Token, - SaveMode = saveMode - }; - configuration.Write(webFunctionApp); + var webFunctionApp = await GetWebApp(instance, cancellationToken); + var configuration = await AggregatorConfiguration.ReadConfiguration(webFunctionApp); + + configuration.DevOpsTokenType = devopsLogonData.Mode; + configuration.DevOpsToken = devopsLogonData.Token; + configuration.SaveMode = saveMode; + + configuration.WriteConfiguration(webFunctionApp); return true; } @@ -297,7 +289,7 @@ internal async Task ChangeAppSettingsAsync(InstanceName instance, string l internal async Task StreamLogsAsync(InstanceName instance, CancellationToken cancellationToken) { - var kudu = new KuduApi(instance, azure, logger); + var kudu = GetKudu(instance); logger.WriteVerbose($"Connecting to {instance.PlainName}..."); // Main takes care of resetting color diff --git a/src/aggregator-cli/Logon/DevOpsLogon.cs b/src/aggregator-cli/Logon/DevOpsLogon.cs index b52a2e1e..b83cfe4d 100644 --- a/src/aggregator-cli/Logon/DevOpsLogon.cs +++ b/src/aggregator-cli/Logon/DevOpsLogon.cs @@ -7,6 +7,7 @@ using System.Threading; using System.Threading.Tasks; + namespace aggregator.cli { class DevOpsLogon : LogonDataBase diff --git a/src/aggregator-cli/Mappings/AggregatorMappings.cs b/src/aggregator-cli/Mappings/AggregatorMappings.cs index 9e2a39e2..2a642b82 100644 --- a/src/aggregator-cli/Mappings/AggregatorMappings.cs +++ b/src/aggregator-cli/Mappings/AggregatorMappings.cs @@ -58,7 +58,7 @@ internal async Task> ListAsync(InstanceName insta string ruleName = ruleUrl.Segments.LastOrDefault() ?? string.Empty; string ruleFullName = $"{InstanceName.FromFunctionAppUrl(ruleUrl).PlainName}/{ruleName}"; result.Add( - new MappingOutputData(instance, ruleFullName, ruleUrl.IsImpersonatationEnabled(), foundProject.Name, subscription.EventType, subscription.Status.ToString()) + new MappingOutputData(instance, ruleFullName, ruleUrl.IsImpersonationEnabled(), foundProject.Name, subscription.EventType, subscription.Status.ToString()) ); } return result; diff --git a/src/aggregator-cli/Rules/AddRuleCommand.cs b/src/aggregator-cli/Rules/AddRuleCommand.cs index a93f7298..bd6c056c 100644 --- a/src/aggregator-cli/Rules/AddRuleCommand.cs +++ b/src/aggregator-cli/Rules/AddRuleCommand.cs @@ -22,9 +22,6 @@ class AddRuleCommand : CommandBase [Option('f', "file", Required = true, HelpText = "Aggregator rule code.")] public string File { get; set; } - [Option("impersonate", Required = false, HelpText = "Do rule changes on behalf of the person triggered the rule execution. See wiki for details, requires special account privileges.")] - public bool ImpersonateExecution { get; set; } - internal override async Task RunAsync(CancellationToken cancellationToken) { var context = await Context @@ -32,7 +29,7 @@ internal override async Task RunAsync(CancellationToken cancellationToken) .BuildAsync(cancellationToken); var instance = new InstanceName(Instance, ResourceGroup); var rules = new AggregatorRules(context.Azure, context.Logger); - bool ok = await rules.AddAsync(instance, Name, File, ImpersonateExecution, cancellationToken); + bool ok = await rules.AddAsync(instance, Name, File, cancellationToken); return ok ? 0 : 1; } } diff --git a/src/aggregator-cli/Rules/AggregatorRules.cs b/src/aggregator-cli/Rules/AggregatorRules.cs index f3761522..30ae5f70 100644 --- a/src/aggregator-cli/Rules/AggregatorRules.cs +++ b/src/aggregator-cli/Rules/AggregatorRules.cs @@ -9,34 +9,30 @@ using System.Threading; using System.Threading.Tasks; using aggregator.Engine.Language; -using Microsoft.Azure.Management.AppService.Fluent; + using Microsoft.Azure.Management.Fluent; using Microsoft.TeamFoundation.Core.WebApi; using Microsoft.TeamFoundation.WorkItemTracking.WebApi.Models; using Microsoft.VisualStudio.Services.Common; +using Microsoft.VisualStudio.Services.ServiceHooks.WebApi; using Microsoft.VisualStudio.Services.WebApi; using Newtonsoft.Json; namespace aggregator.cli { - internal class AggregatorRules + internal class AggregatorRules : AzureBaseClass { - private readonly IAzure _azure; - private readonly ILogger _logger; - - public AggregatorRules(IAzure azure, ILogger logger) - { - _azure = azure; - _logger = logger; - } + public AggregatorRules(IAzure azure, ILogger logger) : base(azure, logger) + { } internal async Task> ListAsync(InstanceName instance, CancellationToken cancellationToken) { - var kudu = new KuduApi(instance, _azure, _logger); + var kudu = GetKudu(instance); _logger.WriteInfo($"Retrieving Functions in {instance.PlainName}..."); var webFunctionApp = await GetWebApp(instance, cancellationToken); cancellationToken.ThrowIfCancellationRequested(); + var configuration = await AggregatorConfiguration.ReadConfiguration(webFunctionApp); using (var client = new HttpClient()) { @@ -89,9 +85,9 @@ internal async Task> ListAsync(InstanceName instance } } - var isDisabled = IsDisabled(webFunctionApp, ruleName); + var ruleConfiguration = configuration.GetRuleConfiguration(ruleName); var (ruleDirectives, _) = RuleFileParser.Read(ruleCode.ToArray()); - ruleData.Add(new RuleOutputData(instance, ruleName, isDisabled, ruleDirectives.Impersonate)); + ruleData.Add(new RuleOutputData(instance, ruleConfiguration, ruleDirectives.LanguageAsString())); } } @@ -99,12 +95,6 @@ internal async Task> ListAsync(InstanceName instance } } - private static bool IsDisabled(IWebApp webFunctionApp, string ruleName) - { - var value = webFunctionApp.GetAppSettings().GetValueOrDefault($"AzureWebJobs.{ruleName}.Disabled")?.Value ?? "false"; - return Boolean.TryParse(value, out bool result) && result; - } - internal static Uri GetInvocationUrl(InstanceName instance, string rule) { var url = $"{instance.FunctionAppUrl}/api/{rule}"; @@ -113,7 +103,7 @@ internal static Uri GetInvocationUrl(InstanceName instance, string rule) internal async Task<(Uri url, string key)> GetInvocationUrlAndKey(InstanceName instance, string rule, CancellationToken cancellationToken = default) { - var kudu = new KuduApi(instance, _azure, _logger); + var kudu = GetKudu(instance); // see https://github.com/projectkudu/kudu/wiki/Functions-API using (var client = new HttpClient()) @@ -142,20 +132,16 @@ internal static Uri GetInvocationUrl(InstanceName instance, string rule) } } - internal async Task AddAsync(InstanceName instance, string name, string filePath, bool impersonate, CancellationToken cancellationToken) + internal async Task AddAsync(InstanceName instance, string ruleName, string filePath, CancellationToken cancellationToken) { _logger.WriteInfo($"Validate rule file {filePath}"); var engineLogger = new EngineWrapperLogger(_logger); var (ruleDirectives, _) = await RuleFileParser.ReadFile(filePath, engineLogger, cancellationToken); - if (impersonate) - { - ruleDirectives.Impersonate = true; - } try { - var rule = new Engine.ScriptedRuleWrapper(name, ruleDirectives, engineLogger); - (var success, var diagnostics) = rule.Verify(); + var rule = new Engine.ScriptedRuleWrapper(ruleName, ruleDirectives, engineLogger); + var (success, diagnostics) = rule.Verify(); if (success) { _logger.WriteInfo($"Rule file is valid"); @@ -179,14 +165,24 @@ internal async Task AddAsync(InstanceName instance, string name, string fi } _logger.WriteVerbose($"Layout rule files"); - var inMemoryFiles = await PackagingFilesAsync(name, ruleDirectives); - _logger.WriteInfo($"Packaging rule {name} complete."); + var inMemoryFiles = await PackagingFilesAsync(ruleName, ruleDirectives); + _logger.WriteInfo($"Packaging rule {ruleName} complete."); _logger.WriteVerbose($"Uploading rule files to {instance.PlainName}"); - bool ok = await UploadRuleFilesAsync(instance, name, inMemoryFiles, cancellationToken); + bool ok = await UploadRuleFilesAsync(instance, ruleName, inMemoryFiles, cancellationToken); if (ok) { - _logger.WriteInfo($"All {name} files successfully uploaded to {instance.PlainName}."); + _logger.WriteInfo($"All {ruleName} files successfully uploaded to {instance.PlainName}."); + } + + if (ruleDirectives.Impersonate) + { + _logger.WriteInfo($"Configure {ruleName} to execute impersonated."); + ok &= await ConfigureAsync(instance, ruleName, impersonate: true, cancellationToken: cancellationToken); + if (ok) + { + _logger.WriteInfo($"Updated {ruleName} configuration successfully."); + } } return ok; @@ -234,7 +230,7 @@ Puts a file at path. Note: when updating or deleting a file, ETag behavior will apply. You can pass a If-Match: "*" header to disable the ETag check. */ - var kudu = new KuduApi(instance, _azure, _logger); + var kudu = GetKudu(instance); var relativeUrl = $"api/vfs/site/wwwroot/{name}/"; using (var client = new HttpClient()) @@ -299,7 +295,7 @@ Puts a file at path. internal async Task RemoveAsync(InstanceName instance, string name, CancellationToken cancellationToken) { - var kudu = new KuduApi(instance, _azure, _logger); + var kudu = GetKudu(instance); // undocumented but works, see https://github.com/projectkudu/kudu/wiki/Functions-API _logger.WriteInfo($"Removing Function {name} in {instance.PlainName}..."); using (var client = new HttpClient()) @@ -314,39 +310,40 @@ internal async Task RemoveAsync(InstanceName instance, string name, Cancel return ok; } + + //TODO BobSilent remove configuration (Enable/Disable or Impersonate) } - internal async Task EnableAsync(InstanceName instance, string name, bool disable, CancellationToken cancellationToken) + internal async Task ConfigureAsync(InstanceName instance, string name, bool? disable = null, bool? impersonate = null, CancellationToken cancellationToken = default) { var webFunctionApp = await GetWebApp(instance, cancellationToken); - webFunctionApp - .Update() - .WithAppSetting($"AzureWebJobs.{name}.Disabled", disable.ToString().ToLower()) - .Apply(); + var configuration = await AggregatorConfiguration.ReadConfiguration(webFunctionApp); + var ruleConfig = configuration.GetRuleConfiguration(name); + + if (disable.HasValue) + { + ruleConfig.IsDisabled = disable.Value; + } + + if (impersonate.HasValue) + { + ruleConfig.Impersonate = impersonate.Value; + } + + ruleConfig.WriteConfiguration(webFunctionApp); return true; } - private async Task GetWebApp(InstanceName instance, CancellationToken cancellationToken) - { - var webFunctionApp = await _azure - .AppServices - .WebApps - .GetByResourceGroupAsync( - instance.ResourceGroupName, - instance.FunctionAppName, cancellationToken); - cancellationToken.ThrowIfCancellationRequested(); - return webFunctionApp; - } - internal async Task UpdateAsync(InstanceName instance, string name, string filePath, bool impersonate, string requiredVersion, string sourceUrl, CancellationToken cancellationToken) + internal async Task UpdateAsync(InstanceName instance, string name, string filePath, string requiredVersion, string sourceUrl, CancellationToken cancellationToken) { // check runtime package var package = new FunctionRuntimePackage(_logger); bool ok = await package.UpdateVersionAsync(requiredVersion, sourceUrl, instance, _azure, cancellationToken); if (ok) { - ok = await AddAsync(instance, name, filePath, impersonate, cancellationToken); + ok = await AddAsync(instance, name, filePath, cancellationToken); } return ok; @@ -451,24 +448,21 @@ internal async Task InvokeRemoteAsync(string account, string project, stri using (var client = new HttpClient()) { - using (var request = new HttpRequestMessage(HttpMethod.Post, ruleUrl)) - { - request.Headers.UserAgent.Add(new ProductInfoHeaderValue("aggregator", "3.0")); - request.Headers.Add("x-functions-key", ruleKey); - request.Content = new StringContent(body, Encoding.UTF8, "application/json"); + client.DefaultRequestHeaders.UserAgent.Add(new ProductInfoHeaderValue("aggregator", "3.0")); + client.DefaultRequestHeaders.Add("x-functions-key", ruleKey); + var content = new StringContent(body, Encoding.UTF8, "application/json"); - using (var response = await client.SendAsync(request, cancellationToken)) + using (var response = await client.PostAsync(ruleUrl, content, cancellationToken)) + { + if (response.IsSuccessStatusCode) { - if (response.IsSuccessStatusCode) - { - string result = await response.Content.ReadAsStringAsync(); - _logger.WriteInfo($"{result}"); - return true; - } - - _logger.WriteError($"Failed with {response.ReasonPhrase}"); - return false; + string result = await response.Content.ReadAsStringAsync(); + _logger.WriteInfo($"{result}"); + return true; } + + _logger.WriteError($"Failed with {response.ReasonPhrase}"); + return false; } } } diff --git a/src/aggregator-cli/Rules/ConfigureRuleCommand.cs b/src/aggregator-cli/Rules/ConfigureRuleCommand.cs index 51526555..8d763349 100644 --- a/src/aggregator-cli/Rules/ConfigureRuleCommand.cs +++ b/src/aggregator-cli/Rules/ConfigureRuleCommand.cs @@ -20,9 +20,15 @@ class ConfigureRuleCommand : CommandBase public string Name { get; set; } [Option('d', "disable", SetName = "disable", HelpText = "Disable the rule.")] - public bool Disable { get; set; } + public bool? Disable { get; set; } [Option('e', "enable", SetName = "enable", HelpText = "Enable the rule.")] - public bool Enable { get; set; } + public bool? Enable { get; set; } + + [Option("disableImpersonate", Required = false, HelpText = "Disable do rule changes impersonated.")] + public bool? DisableImpersonateExecution { get; set; } + + [Option("enableImpersonate", Required = false, HelpText = "Enable do rule changes on behalf of the person triggered the rule execution. See wiki for details, requires special account privileges.")] + public bool? EnableImpersonateExecution { get; set; } internal override async Task RunAsync(CancellationToken cancellationToken) { @@ -31,12 +37,42 @@ internal override async Task RunAsync(CancellationToken cancellationToken) .BuildAsync(cancellationToken); var instance = new InstanceName(Instance, ResourceGroup); var rules = new AggregatorRules(context.Azure, context.Logger); - bool ok = false; - if (Disable || Enable) - { - ok = await rules.EnableAsync(instance, Name, Disable, cancellationToken); - } + + var disable = GetDisableStatus(Disable, Enable); + var impersonate = GetEnableStatus(DisableImpersonateExecution, EnableImpersonateExecution); + + var ok = await rules.ConfigureAsync(instance, Name, disable, impersonate, cancellationToken); return ok ? 0 : 1; } + + + /// + /// in case of neither disableSetting nor enableSetting is set return null + /// Otherwise return value of disableSetting + /// + /// + /// + /// + private static bool? GetDisableStatus(bool? disableSetting, bool? enableSetting) + { + return GetEnableDisableStatus(disableSetting, enableSetting, disableSetting); + } + + /// + /// in case of neither disableSetting nor enableSetting is set return null + /// Otherwise return value of enableSetting + /// + /// + /// + /// + private static bool? GetEnableStatus(bool? disableSetting, bool? enableSetting) + { + return GetEnableDisableStatus(disableSetting, enableSetting, enableSetting); + } + + private static bool? GetEnableDisableStatus(bool? disableSetting, bool? enableSetting, bool? defaultSetting) + { + return disableSetting.HasValue || enableSetting.HasValue ? (bool?)(defaultSetting ?? false) : null; + } } } diff --git a/src/aggregator-cli/Rules/InvokeRuleCommand.cs b/src/aggregator-cli/Rules/InvokeRuleCommand.cs index edd104ed..45dd3e27 100644 --- a/src/aggregator-cli/Rules/InvokeRuleCommand.cs +++ b/src/aggregator-cli/Rules/InvokeRuleCommand.cs @@ -5,6 +5,7 @@ using System.Threading; using System.Threading.Tasks; + namespace aggregator.cli { [Verb("invoke.rule", HelpText = "Executes a rule locally or in an existing Aggregator instance.")] diff --git a/src/aggregator-cli/Rules/RuleOutputData.cs b/src/aggregator-cli/Rules/RuleOutputData.cs index 7351bdf1..334a64c6 100644 --- a/src/aggregator-cli/Rules/RuleOutputData.cs +++ b/src/aggregator-cli/Rules/RuleOutputData.cs @@ -2,21 +2,24 @@ using System.Collections.Generic; using System.Text; + namespace aggregator.cli { internal class RuleOutputData : ILogDataObject { private readonly string instanceName; private readonly string ruleName; + private readonly string ruleLanguage; private readonly bool isDisabled; private readonly bool isImpersonated; - internal RuleOutputData(InstanceName instance, string ruleName, bool isDisabled, bool isImpersonated) + internal RuleOutputData(InstanceName instance, IRuleConfiguration ruleConfiguration, string ruleLanguage) { this.instanceName = instance.PlainName; - this.ruleName = ruleName; - this.isDisabled = isDisabled; - this.isImpersonated = isImpersonated; + this.ruleName = ruleConfiguration.RuleName; + this.isDisabled = ruleConfiguration.IsDisabled; + this.isImpersonated = ruleConfiguration.Impersonate; + this.ruleLanguage = ruleLanguage; } public string AsHumanReadable() diff --git a/src/aggregator-cli/Rules/UpdateRuleCommand.cs b/src/aggregator-cli/Rules/UpdateRuleCommand.cs index 5cecedfc..55e4ed7f 100644 --- a/src/aggregator-cli/Rules/UpdateRuleCommand.cs +++ b/src/aggregator-cli/Rules/UpdateRuleCommand.cs @@ -22,9 +22,6 @@ class UpdateRuleCommand : CommandBase [Option('f', "file", Required = true, HelpText = "Aggregator rule code.")] public string File { get; set; } - [Option("impersonate", Required = false, HelpText = "Do rule changes on behalf of the person triggered the rule execution. See wiki for details, requires special account privileges.")] - public bool ImpersonateExecution { get; set; } - [Option("requiredVersion", SetName = "nourl", Required = false, HelpText = "Version of Aggregator Runtime required.")] public string RequiredVersion { get; set; } @@ -38,7 +35,7 @@ internal override async Task RunAsync(CancellationToken cancellationToken) .BuildAsync(cancellationToken); var instance = new InstanceName(Instance, ResourceGroup); var rules = new AggregatorRules(context.Azure, context.Logger); - bool ok = await rules.UpdateAsync(instance, Name, File, ImpersonateExecution, RequiredVersion, SourceUrl, cancellationToken); + bool ok = await rules.UpdateAsync(instance, Name, File, RequiredVersion, SourceUrl, cancellationToken); return ok ? 0 : 1; } } diff --git a/src/aggregator-function/AzureFunctionHandler.cs b/src/aggregator-function/AzureFunctionHandler.cs index 1203671c..45804495 100644 --- a/src/aggregator-function/AzureFunctionHandler.cs +++ b/src/aggregator-function/AzureFunctionHandler.cs @@ -60,8 +60,8 @@ public async Task RunAsync(HttpRequestMessage req, Cancella } var configContext = GetConfigurationContext(); - var configuration = AggregatorConfiguration.Read(configContext) - .UpdateFromUrl(req.RequestUri); + var configuration = AggregatorConfiguration.ReadConfiguration(configContext) + .UpdateFromUrl(ruleName, req.RequestUri); var logger = new ForwarderLogger(_log); var ruleProvider = new AzureFunctionRuleProvider(logger, _context.FunctionDirectory); diff --git a/src/aggregator-ruleng/IRule.cs b/src/aggregator-ruleng/IRule.cs new file mode 100644 index 00000000..c2dd4371 --- /dev/null +++ b/src/aggregator-ruleng/IRule.cs @@ -0,0 +1,27 @@ +using System.Threading; +using System.Threading.Tasks; + + +namespace aggregator.Engine { + public interface IRule + { + /// + /// RuleName + /// + string Name { get; } + + /// + /// The history will show the changes made by person who triggered the event + /// Assumes PAT or Account Permission is high enough + /// + bool ImpersonateExecution { get; set; } + + /// + /// Apply the rule to executionContext + /// + /// + /// + /// + Task ApplyAsync(RuleExecutionContext executionContext, CancellationToken cancellationToken); + } +} \ No newline at end of file diff --git a/src/aggregator-ruleng/IRuleResult.cs b/src/aggregator-ruleng/IRuleResult.cs new file mode 100644 index 00000000..bd54792e --- /dev/null +++ b/src/aggregator-ruleng/IRuleResult.cs @@ -0,0 +1,31 @@ +namespace aggregator.Engine +{ + public enum RuleExecutionOutcome + { + Unknown, + Success, + Error + } + + public interface IRuleResult + { + /// + /// Result Value Message + /// + string Value { get; } + + /// + /// Execution Outcome + /// + RuleExecutionOutcome Outcome { get; } + } + + public class RuleResult : IRuleResult + { + /// + public string Value { get; set; } + + /// + public RuleExecutionOutcome Outcome { get; set; } + } +} \ No newline at end of file diff --git a/src/aggregator-ruleng/Language/RuleDirectivesExtensions.cs b/src/aggregator-ruleng/Language/RuleDirectivesExtensions.cs index 4f6e0246..0bd462cb 100644 --- a/src/aggregator-ruleng/Language/RuleDirectivesExtensions.cs +++ b/src/aggregator-ruleng/Language/RuleDirectivesExtensions.cs @@ -28,7 +28,7 @@ internal static bool IsSupportedLanguage(this IRuleDirectives ruleDirectives) } - internal static string LanguageAsString(this IRuleDirectives ruleDirectives) + public static string LanguageAsString(this IRuleDirectives ruleDirectives) { switch (ruleDirectives.Language) { diff --git a/src/aggregator-ruleng/RuleEngine.cs b/src/aggregator-ruleng/RuleEngine.cs index 7b3ffd6b..f05dabfe 100644 --- a/src/aggregator-ruleng/RuleEngine.cs +++ b/src/aggregator-ruleng/RuleEngine.cs @@ -43,9 +43,9 @@ public async Task RunAsync(IRule rule, Guid projectId, WorkItemData work return result; } - protected abstract Task ExecuteRuleAsync(IRule rule, Globals executionContext, CancellationToken cancellationToken = default); + protected abstract Task ExecuteRuleAsync(IRule rule, RuleExecutionContext executionContext, CancellationToken cancellationToken = default); - protected Globals CreateRuleExecutionContext(Guid projectId, WorkItemData workItemPayload, IClientsContext clients) + protected RuleExecutionContext CreateRuleExecutionContext(Guid projectId, WorkItemData workItemPayload, IClientsContext clients) { var workItem = workItemPayload.WorkItem; var context = new EngineContext(clients, projectId, workItem.GetTeamProject(), logger); @@ -54,7 +54,7 @@ protected Globals CreateRuleExecutionContext(Guid projectId, WorkItemData workIt var selfChanges = new WorkItemUpdateWrapper(workItemPayload.WorkItemUpdate); logger.WriteInfo($"Initial WorkItem {self.Id} retrieved from {clients.WitClient.BaseAddress}"); - var globals = new Globals + var globals = new RuleExecutionContext { self = self, selfChanges = selfChanges, @@ -72,7 +72,7 @@ public RuleEngine(IAggregatorLogger logger, SaveMode saveMode, bool dryRun) : ba { } - protected override async Task ExecuteRuleAsync(IRule rule, Globals executionContext, CancellationToken cancellationToken = default) + protected override async Task ExecuteRuleAsync(IRule rule, RuleExecutionContext executionContext, CancellationToken cancellationToken = default) { var result = await rule.ApplyAsync(executionContext, cancellationToken); diff --git a/src/aggregator-ruleng/Globals.cs b/src/aggregator-ruleng/RuleExecutionContext.cs similarity index 88% rename from src/aggregator-ruleng/Globals.cs rename to src/aggregator-ruleng/RuleExecutionContext.cs index 37719d6a..e1bd49c9 100644 --- a/src/aggregator-ruleng/Globals.cs +++ b/src/aggregator-ruleng/RuleExecutionContext.cs @@ -4,7 +4,7 @@ namespace aggregator.Engine { - public class Globals + public class RuleExecutionContext { public WorkItemWrapper self; public WorkItemUpdateWrapper selfChanges; diff --git a/src/aggregator-ruleng/RuleExecutor.cs b/src/aggregator-ruleng/RuleExecutor.cs index d325cc4f..3dae7084 100644 --- a/src/aggregator-ruleng/RuleExecutor.cs +++ b/src/aggregator-ruleng/RuleExecutor.cs @@ -8,11 +8,11 @@ namespace aggregator.Engine { public class RuleExecutor { - protected readonly AggregatorConfiguration configuration; + protected readonly IAggregatorConfiguration configuration; protected readonly IAggregatorLogger logger; - public RuleExecutor(IAggregatorLogger logger, AggregatorConfiguration configuration) + public RuleExecutor(IAggregatorLogger logger, IAggregatorConfiguration configuration) { this.configuration = configuration; this.logger = logger; diff --git a/src/aggregator-ruleng/ScriptedRuleWrapper.cs b/src/aggregator-ruleng/ScriptedRuleWrapper.cs index 1592f045..dbbe1c4b 100644 --- a/src/aggregator-ruleng/ScriptedRuleWrapper.cs +++ b/src/aggregator-ruleng/ScriptedRuleWrapper.cs @@ -13,53 +13,6 @@ namespace aggregator.Engine { - public enum RuleExecutionOutcome - { - Unknown, - Success, - Error - } - - public interface IRuleResult - { - /// - /// Result Value Message - /// - string Value { get; } - - /// - /// Execution Outcome - /// - RuleExecutionOutcome Outcome { get; } - } - - - public class RuleResult : IRuleResult - { - /// - public string Value { get; set; } - - /// - public RuleExecutionOutcome Outcome { get; set; } - } - - public interface IRule - { - /// - /// RuleName - /// - string Name { get; } - - /// - /// The history will show the changes made by person who triggered the event - /// Assumes PAT or Account Permission is high enough - /// - bool ImpersonateExecution { get; set; } - - - Task ApplyAsync(Globals executionContext, CancellationToken cancellationToken); - } - /// /// CSharp Scripted Rule Facade /// @@ -85,10 +38,10 @@ private ScriptedRuleWrapper(string ruleName, IAggregatorLogger logger) internal ScriptedRuleWrapper(string ruleName, string[] ruleCode) : this(ruleName, new NullLogger()) { - var parseResult = RuleFileParser.Read(ruleCode); - _ruleFileParseSuccess = parseResult.parseSuccess; + (IRuleDirectives ruleDirectives, bool parseSuccess) = RuleFileParser.Read(ruleCode); + _ruleFileParseSuccess = parseSuccess; - Initialize(parseResult.ruleDirectives); + Initialize(ruleDirectives); } public ScriptedRuleWrapper(string ruleName, IRuleDirectives ruleDirectives) : this(ruleName, ruleDirectives, new NullLogger()) @@ -119,7 +72,7 @@ private void Initialize(IRuleDirectives ruleDirectives) _roslynScript = CSharpScript.Create( code: RuleDirectives.GetRuleCode(), options: scriptOptions, - globalsType: typeof(Globals)); + globalsType: typeof(RuleExecutionContext)); } else { @@ -156,7 +109,7 @@ private static IEnumerable DefaultImports() } /// - public async Task ApplyAsync(Globals executionContext, CancellationToken cancellationToken) + public async Task ApplyAsync(RuleExecutionContext executionContext, CancellationToken cancellationToken) { var result = await _roslynScript.RunAsync(executionContext, cancellationToken); @@ -178,7 +131,12 @@ public async Task ApplyAsync(Globals executionContext, Cancellation }; } - public (bool success, ImmutableArray diagnostics) Verify() + /// + /// Verify the script rule code by trying to compile and return compile errors + /// if parsing rule code already fails, success will also be false + /// + /// + public (bool success, IReadOnlyList diagnostics) Verify() { if (_ruleFileParseSuccess.HasValue && !_ruleFileParseSuccess.Value) { @@ -186,16 +144,8 @@ public async Task ApplyAsync(Globals executionContext, Cancellation } // if parsing succeeded try to compile the script and look for errors - ImmutableArray diagnostics = _roslynScript.Compile(); - (bool, ImmutableArray) result; - if (diagnostics.Any()) - { - result = (false, diagnostics); - } - else - { - result = (true, ImmutableArray.Create()); - } + var diagnostics = _roslynScript.Compile(); + var result = diagnostics.Any() ? (false, diagnostics) : (true, ImmutableArray.Create()); return result; } diff --git a/src/aggregator-shared/AggregatorConfiguration.cs b/src/aggregator-shared/AggregatorConfiguration.cs index 324511ac..c0c31e6a 100644 --- a/src/aggregator-shared/AggregatorConfiguration.cs +++ b/src/aggregator-shared/AggregatorConfiguration.cs @@ -1,4 +1,12 @@ using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; + +using aggregator.Model; + +using Microsoft.VisualStudio.Services.Common; + namespace aggregator { @@ -16,14 +24,67 @@ public enum SaveMode TwoPhases = 3 } - /// - /// This class tracks the configuration data that CLI writes and Function runtime reads - /// - public class AggregatorConfiguration + public interface IAggregatorConfiguration + { + DevOpsTokenType DevOpsTokenType { get; set; } + string DevOpsToken { get; set; } + SaveMode SaveMode { get; set; } + bool DryRun { get; set; } + + IDictionary RulesConfiguration { get; } + } + + public interface IRuleConfiguration { - public static AggregatorConfiguration Read(Microsoft.Extensions.Configuration.IConfiguration config) + string RuleName { get; } + bool IsDisabled { get; set; } + bool Impersonate { get; set; } + } + + + public static class AggregatorConfiguration + { + const string RULE_SETTINGS_PREFIX = "AzureWebJobs."; + + public static async Task ReadConfiguration(Microsoft.Azure.Management.AppService.Fluent.IWebApp webApp) { - var ac = new AggregatorConfiguration(); + (string ruleName, string key) SplitRuleNameKey(string input) + { + int idx = input.LastIndexOf('.'); + return (input.Substring(0, idx), input.Substring(idx + 1)); + } + + var settings = await webApp.GetAppSettingsAsync(); + var ac = new Model.AggregatorConfiguration(); + foreach (var ruleSetting in settings.Where(kvp => kvp.Key.StartsWith(RULE_SETTINGS_PREFIX)).Select(kvp => new { ruleNameKey = kvp.Key.Substring(RULE_SETTINGS_PREFIX.Length), value = kvp.Value.Value })) + { + var (ruleName, key) = SplitRuleNameKey(ruleSetting.ruleNameKey); + + var ruleConfig = ac.GetRuleConfiguration(ruleName); + if (string.Equals("Disabled", key, StringComparison.OrdinalIgnoreCase)) + { + ruleConfig.IsDisabled = Boolean.TryParse(ruleSetting.value, out bool result) && result; + } + + if (string.Equals("Impersonate", key, StringComparison.OrdinalIgnoreCase)) + { + ruleConfig.Impersonate = string.Equals("onBehalfOfInitiator", ruleSetting.value, StringComparison.OrdinalIgnoreCase); + } + } + + Enum.TryParse(settings.GetValueOrDefault("Aggregator_VstsTokenType")?.Value, out DevOpsTokenType vtt); + ac.DevOpsTokenType = vtt; + ac.DevOpsToken = settings.GetValueOrDefault("Aggregator_VstsToken")?.Value; + ac.SaveMode = Enum.TryParse(settings.GetValueOrDefault("Aggregator_SaveMode")?.Value, out SaveMode sm) + ? sm + : SaveMode.Default; + ac.DryRun = false; + return ac; + } + + public static IAggregatorConfiguration ReadConfiguration(Microsoft.Extensions.Configuration.IConfiguration config) + { + var ac = new Model.AggregatorConfiguration(); Enum.TryParse(config["Aggregator_VstsTokenType"], out DevOpsTokenType vtt); ac.DevOpsTokenType = vtt; ac.DevOpsToken = config["Aggregator_VstsToken"]; @@ -34,20 +95,67 @@ public static AggregatorConfiguration Read(Microsoft.Extensions.Configuration.IC return ac; } - public void Write(Microsoft.Azure.Management.AppService.Fluent.IWebApp webApp) + public static void WriteConfiguration(this IAggregatorConfiguration config, Microsoft.Azure.Management.AppService.Fluent.IWebApp webApp) + { + var settings = new Dictionary() + { + {"Aggregator_VstsTokenType", config.DevOpsTokenType.ToString()}, + {"Aggregator_VstsToken", config.DevOpsToken}, + {"Aggregator_SaveMode", config.SaveMode.ToString()}, + }; + + foreach (var ruleSetting in config.RulesConfiguration.Select(kvp => kvp.Value)) + { + settings.AddRuleSettings(ruleSetting); + } + + webApp.ApplyWithAppSettings(settings); + } + + public static void WriteConfiguration(this IRuleConfiguration config, Microsoft.Azure.Management.AppService.Fluent.IWebApp webApp) + { + var settings = new Dictionary(); + + settings.AddRuleSettings(config); + + webApp.ApplyWithAppSettings(settings); + } + + public static void Delete(this IRuleConfiguration config, Microsoft.Azure.Management.AppService.Fluent.IWebApp webApp) + { + var settings = new Dictionary(); + + settings.AddRuleSettings(config); + + var update = webApp.Update(); + + foreach (var key in settings.Keys) + { + update.WithoutAppSetting(key); + } + + update.Apply(); + } + + public static IRuleConfiguration GetRuleConfiguration(this IAggregatorConfiguration config, string ruleName) + { + var ruleConfig = config.RulesConfiguration.GetValueOrDefault(ruleName) ?? (config.RulesConfiguration[ruleName] = new RuleConfiguration(ruleName)); + + return ruleConfig; + } + + private static void AddRuleSettings(this Dictionary settings, IRuleConfiguration ruleSetting) + { + settings[$"{RULE_SETTINGS_PREFIX}{ruleSetting.RuleName}.Disabled"] = ruleSetting.IsDisabled.ToString(); + settings[$"{RULE_SETTINGS_PREFIX}{ruleSetting.RuleName}.Impersonate"] = ruleSetting.Impersonate ? "onBehalfOfInitiator" : "false"; + } + + private static void ApplyWithAppSettings(this Microsoft.Azure.Management.AppService.Fluent.IWebApp webApp, Dictionary settings) { webApp .Update() - .WithAppSetting("Aggregator_VstsTokenType", DevOpsTokenType.ToString()) - .WithAppSetting("Aggregator_VstsToken", DevOpsToken) - .WithAppSetting("Aggregator_SaveMode", SaveMode.ToString()) + .WithAppSettings(settings) .Apply(); } - - public DevOpsTokenType DevOpsTokenType { get; set; } - public string DevOpsToken { get; set; } - public SaveMode SaveMode { get; set; } - public bool DryRun { get; internal set; } - public bool Impersonate { get; internal set; } } } diff --git a/src/aggregator-shared/InvokeOptions.cs b/src/aggregator-shared/InvokeOptions.cs index 5a5b62df..16b3c45b 100644 --- a/src/aggregator-shared/InvokeOptions.cs +++ b/src/aggregator-shared/InvokeOptions.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Collections.Specialized; using System.Globalization; @@ -9,44 +10,51 @@ namespace aggregator { public static class InvokeOptions { + /// + /// extend Url with configuration information + /// + /// + /// + /// + /// + /// public static Uri AddToUrl(this Uri ruleUrl, bool dryRun = false, SaveMode saveMode = SaveMode.Default, bool impersonate = false) { var queryBuilder = new UriQueryBuilder(); - queryBuilder.Add("dryRun", dryRun.ToString(CultureInfo.InvariantCulture)); - queryBuilder.Add("saveMode", saveMode.ToString()); - if (impersonate) - { - queryBuilder.Add("execute", "impersonated"); - } + queryBuilder.AddIfNotDefault("dryRun", dryRun) + .AddIfNotDefault("saveMode", saveMode) + .AddIfNotDefault("execute", impersonate, valueString: "impersonated"); return queryBuilder.AddToUri(ruleUrl); } - public static AggregatorConfiguration UpdateFromUrl(this AggregatorConfiguration configuration, Uri requestUri) + /// + /// + /// + /// + /// + /// + /// + public static IAggregatorConfiguration UpdateFromUrl(this IAggregatorConfiguration configuration, string ruleName, Uri requestUri) { var parameters = System.Web.HttpUtility.ParseQueryString(requestUri.Query); configuration.DryRun = IsDryRunEnabled(parameters); - - if (Enum.TryParse(parameters["saveMode"], out SaveMode saveMode)) - { - configuration.SaveMode = saveMode; - } - - configuration.Impersonate = IsImpersonatationEnabled(parameters); + configuration.SaveMode = GetSaveMode(parameters); + configuration.GetRuleConfiguration(ruleName).Impersonate = IsImpersonationEnabled(parameters); return configuration; } - public static bool IsImpersonatationEnabled(this Uri ruleUrl) + public static bool IsImpersonationEnabled(this Uri ruleUrl) { var parameters = System.Web.HttpUtility.ParseQueryString(ruleUrl.Query); - return IsImpersonatationEnabled(parameters); + return IsImpersonationEnabled(parameters); } - private static bool IsImpersonatationEnabled(NameValueCollection parameters) + private static bool IsImpersonationEnabled(NameValueCollection parameters) { return string.Equals(parameters["execute"], "impersonated", StringComparison.OrdinalIgnoreCase); } @@ -56,5 +64,20 @@ private static bool IsDryRunEnabled(NameValueCollection parameters) bool dryRun = bool.TryParse(parameters["dryRun"], out dryRun) && dryRun; return dryRun; } + + private static SaveMode GetSaveMode(NameValueCollection parameters) + { + return Enum.TryParse(parameters["saveMode"], out SaveMode saveMode) ? saveMode : SaveMode.Default; + } + + private static UriQueryBuilder AddIfNotDefault(this UriQueryBuilder queryBuilder, string name, T value, T defaultValue = default, string valueString = null) + { + if (!EqualityComparer.Default.Equals(defaultValue, value)) + { + queryBuilder.Add(name, valueString ?? Convert.ToString(value, CultureInfo.InvariantCulture)); + } + + return queryBuilder; + } } } diff --git a/src/aggregator-shared/Model/AggregatorConfiguration.cs b/src/aggregator-shared/Model/AggregatorConfiguration.cs new file mode 100644 index 00000000..0de68f9e --- /dev/null +++ b/src/aggregator-shared/Model/AggregatorConfiguration.cs @@ -0,0 +1,27 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Diagnostics; + + +namespace aggregator.Model +{ + /// + /// This class tracks the configuration data that CLI writes and Function runtime reads + /// + [DebuggerDisplay("SaveMode={SaveMode,nq}, DryRun={DryRun}")] + internal class AggregatorConfiguration : IAggregatorConfiguration + { + public AggregatorConfiguration() + { + RulesConfiguration = new ConcurrentDictionary(StringComparer.OrdinalIgnoreCase); + } + + public DevOpsTokenType DevOpsTokenType { get; set; } + public string DevOpsToken { get; set; } + public SaveMode SaveMode { get; set; } + public bool DryRun { get; set; } + + public IDictionary RulesConfiguration { get; } + } +} \ No newline at end of file diff --git a/src/aggregator-shared/Model/RuleConfiguration.cs b/src/aggregator-shared/Model/RuleConfiguration.cs new file mode 100644 index 00000000..7cac2e85 --- /dev/null +++ b/src/aggregator-shared/Model/RuleConfiguration.cs @@ -0,0 +1,17 @@ +using System.Diagnostics; + + +namespace aggregator.Model +{ + [DebuggerDisplay("{RuleName,nq}, Disabled={IsDisabled}, Impersonate={Impersonate}")] + internal class RuleConfiguration : IRuleConfiguration + { + public RuleConfiguration(string ruleName) + { + RuleName = ruleName; + } + public string RuleName { get; } + public bool IsDisabled { get; set; } + public bool Impersonate { get; set; } + } +} \ No newline at end of file diff --git a/src/unittests-function/AzureFunctionHandlerTests.cs b/src/unittests-function/AzureFunctionHandlerTests.cs index b3162242..e9f26ab3 100644 --- a/src/unittests-function/AzureFunctionHandlerTests.cs +++ b/src/unittests-function/AzureFunctionHandlerTests.cs @@ -46,7 +46,7 @@ public AzureFunctionHandlerTests() [Fact] public async void HandleTestEvent_ReturnAggregatorInformation_Succeeds() { - request.Content = new StringContent(ExampleTestData.TestEventAsString, Encoding.UTF8, "application/json"); + request.Content = new StringContent(ExampleEvents.TestEventAsString, Encoding.UTF8, "application/json"); var handler = new AzureFunctionHandler(logger, context); var response = await handler.RunAsync(request, CancellationToken.None); diff --git a/src/unittests-ruleng/RuleFileParserTests.cs b/src/unittests-ruleng/RuleFileParserTests.cs index 86bb17a8..fab13e27 100644 --- a/src/unittests-ruleng/RuleFileParserTests.cs +++ b/src/unittests-ruleng/RuleFileParserTests.cs @@ -97,7 +97,7 @@ public void RuleLanguageReadWrite_Succeeds() return $""Hello { self.WorkItemType } #{ self.Id } - { self.Title }!""; "; - (IRuleDirectives directives, bool parsingSuccess) = RuleFileParser.Read(ruleCode.Mince()); + (IRuleDirectives directives, _) = RuleFileParser.Read(ruleCode.Mince()); var ruleCode2 = RuleFileParser.Write(directives); diff --git a/src/unittests-ruleng/RuleTests.cs b/src/unittests-ruleng/RuleTests.cs index 8fde70b9..630e53a6 100644 --- a/src/unittests-ruleng/RuleTests.cs +++ b/src/unittests-ruleng/RuleTests.cs @@ -182,7 +182,7 @@ public async Task AddChild_Succeeds() var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); string result = await engine.RunAsync(rule, clientsContext.ProjectId, workItem, clientsContext, CancellationToken.None); - + Assert.Null(result); logger.Received().WriteInfo($"Found a request for a new Task workitem in {clientsContext.ProjectName}"); logger.Received().WriteInfo($"Found a request to update workitem {workItemId} in {clientsContext.ProjectName}"); @@ -304,8 +304,8 @@ public void Diagnostic_Location_Returned_Correctly() return string.Empty "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - var (success, diagnostics) = engine.VerifyRule(); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + var (success, diagnostics) = rule.Verify(); Assert.False(success); Assert.Single(diagnostics); Assert.Equal(2, diagnostics[0].Location.GetLineSpan().StartLinePosition.Line); @@ -520,8 +520,8 @@ public async Task SuccesorLink_Test() } "; - var engine = new RuleEngine(logger, ruleCode.Mince(), SaveMode.Default, dryRun: true); - string result = await engine.ExecuteAsync(clientsContext.ProjectId, predecessor, clientsContext, CancellationToken.None); + var rule = new ScriptedRuleWrapper("Test", ruleCode.Mince()); + string result = await engine.RunAsync(rule, clientsContext.ProjectId, predecessor, clientsContext, CancellationToken.None); Assert.Equal("Successor", result); } @@ -566,7 +566,7 @@ public async Task DocumentationRule_BacklogWorkItemsResolveParent_FailsDueToChil { var workItemFeature = ExampleTestData.BacklogFeatureTwoChildren; var workItemUS2 = ExampleTestData.BacklogUserStoryClosed; - var workItemUS3= ExampleTestData.BacklogUserStoryActive; + var workItemUS3 = ExampleTestData.BacklogUserStoryActive; workItemUS3.Id = 3; var workItemUpdate = ExampleTestData.WorkItemUpdateFields; diff --git a/src/unittests-ruleng/TestData/ExampleTestData.cs b/src/unittests-ruleng/TestData/ExampleTestData.cs index 568db758..841ba7ae 100644 --- a/src/unittests-ruleng/TestData/ExampleTestData.cs +++ b/src/unittests-ruleng/TestData/ExampleTestData.cs @@ -50,7 +50,7 @@ internal static class ExampleRuleCode public static string[] ResolveParent => Helper.GetFromResource("advanced.resolve-parent.rulecode"); } - public static class ExampleTestData + internal static class ExampleTestData { public static WorkItem DeltedWorkItem => Helper.GetFromResource("DeletedWorkItem.json"); public static WorkItem WorkItem => Helper.GetFromResource("WorkItem.22.json"); @@ -68,8 +68,11 @@ public static class ExampleTestData public static ProcessConfiguration ProcessConfigDefaultAgile => Helper.GetFromResource("WorkClient.ProcessConfiguration.Agile.json"); public static ProcessConfiguration ProcessConfigDefaultScrum => Helper.GetFromResource("WorkClient.ProcessConfiguration.Scrum.json"); public static WorkItemStateColor[] WorkItemStateColorDefault => Helper.GetFromResource("WitClient.WorkItemStateColor.EpicFeatureUserStory.json"); + } + public static class ExampleEvents + { public static WebHookEvent TestEvent => Helper.GetFromResource("TestEvent.json"); public static string TestEventAsString => Helper.GetEmbeddedResourceContent("TestEvent.json"); } From cc0a905300d6fdcaf8db129ef23328ef1a53fa61 Mon Sep 17 00:00:00 2001 From: BobSilent Date: Fri, 20 Dec 2019 14:13:20 +0100 Subject: [PATCH 20/20] update for #85 Strip directives by commenting out rather than omitting --- src/aggregator-cli.sln | 3 ++- src/aggregator-ruleng/Language/IRuleDirectives.cs | 1 + src/aggregator-ruleng/Language/RuleDirectives.cs | 3 +++ src/aggregator-ruleng/Language/RuleFileParser.cs | 4 +++- 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/aggregator-cli.sln b/src/aggregator-cli.sln index e682f1e9..6e4d8dc2 100644 --- a/src/aggregator-cli.sln +++ b/src/aggregator-cli.sln @@ -29,13 +29,14 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "unittests-ruleng", "unittes EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{020591FD-B34A-49E6-98E5-D8DD2A838997}" ProjectSection(SolutionItems) = preProject + .editorconfig = .editorconfig ..\azure-pipelines-CI.yaml = ..\azure-pipelines-CI.yaml ..\azure-pipelines-Release.yaml = ..\azure-pipelines-Release.yaml EndProjectSection EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "unittests-core", "unittests-core\unittests-core.csproj", "{6AD42A49-2EA4-4659-855D-0C011D368794}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "unittests-function", "unittests-function\unittests-function.csproj", "{A875638A-8E95-47BE-AEDD-BAD5113692B3}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "unittests-function", "unittests-function\unittests-function.csproj", "{A875638A-8E95-47BE-AEDD-BAD5113692B3}" EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution diff --git a/src/aggregator-ruleng/Language/IRuleDirectives.cs b/src/aggregator-ruleng/Language/IRuleDirectives.cs index cc7b30bd..423ff67a 100644 --- a/src/aggregator-ruleng/Language/IRuleDirectives.cs +++ b/src/aggregator-ruleng/Language/IRuleDirectives.cs @@ -8,6 +8,7 @@ public interface IRuleDirectives RuleLanguage Language { get; } IReadOnlyList References { get; } IReadOnlyList Imports { get; } + int RuleCodeOffset { get; } IReadOnlyList RuleCode { get; } } } \ No newline at end of file diff --git a/src/aggregator-ruleng/Language/RuleDirectives.cs b/src/aggregator-ruleng/Language/RuleDirectives.cs index c872a77f..eb25795d 100644 --- a/src/aggregator-ruleng/Language/RuleDirectives.cs +++ b/src/aggregator-ruleng/Language/RuleDirectives.cs @@ -11,6 +11,7 @@ public RuleDirectives() Language = RuleLanguage.Unknown; References = new List(); Imports = new List(); + RuleCodeOffset = 0; RuleCode = new List(); } @@ -29,6 +30,8 @@ public RuleDirectives() public IList Imports { get; } + public int RuleCodeOffset { get; set; } + public IList RuleCode { get; } } } \ No newline at end of file diff --git a/src/aggregator-ruleng/Language/RuleFileParser.cs b/src/aggregator-ruleng/Language/RuleFileParser.cs index dcfb1086..49e860fd 100644 --- a/src/aggregator-ruleng/Language/RuleFileParser.cs +++ b/src/aggregator-ruleng/Language/RuleFileParser.cs @@ -122,9 +122,11 @@ void FailParsingWithMessage(string message) } }//switch + ruleDirectives.RuleCode.Add($"//{directive}"); directiveLineIndex++; }//while + ruleDirectives.RuleCodeOffset = directiveLineIndex; ruleDirectives.RuleCode.AddRange(ruleCode.Skip(directiveLineIndex)); var parseSuccessful = !parsingIssues; return (ruleDirectives, parseSuccessful); @@ -151,7 +153,7 @@ public static string[] Write(IRuleDirectives ruleDirectives) content.AddRange(ruleDirectives.References.Select(reference => $".reference={reference}")); content.AddRange(ruleDirectives.Imports.Select(import => $".import={import}")); - content.AddRange(ruleDirectives.RuleCode); + content.AddRange(ruleDirectives.RuleCode.Skip(ruleDirectives.RuleCodeOffset)); return content.ToArray(); }