Release/1.1 (#240)

* fix typos in messages * Squashed commit of the following: commit 80e7206 Author: George Roberts <[email protected]> Date: Mon Feb 8 10:51:21 2021 -0600 Remove test log line commit bf4d89d Author: George Roberts <[email protected]> Date: Sat Feb 6 23:26:55 2021 -0600 Remove some extraneous logging commit db994e3 Author: George Roberts <[email protected]> Date: Sat Feb 6 23:16:06 2021 -0600 Fix tests commit c5945cd Author: George Roberts <[email protected]> Date: Sat Feb 6 22:57:14 2021 -0600 Fix bypassrules value commit 59f338d Author: George Roberts <[email protected]> Date: Sat Feb 6 22:54:45 2021 -0600 Support for .bypassrules directive * fixed an issue which caused removing work item links to fail * Fix ".check revision false" directive disappearing on rule upload * change log * Add tests to check more double assignment cases * Fixes #229 - double assignment and reset to original value Alternative implementation to PR #236 * Fixes spurious upgrade message noted in #225 * trigger build * Fully async Co-authored-by: Rob Osborne <[email protected]> Co-authored-by: Alexander Omelchuk <[email protected]>
tfsaggregator · Jun 12, 2021 · 332c159 · 332c159
1 parent 9af9105
commit 332c159
Show file tree

Hide file tree

Showing 28 changed files with 254 additions and 186 deletions.
diff --git a/Next-Release-ChangeLog.md b/Next-Release-ChangeLog.md
@@ -3,34 +3,29 @@ This release fixes a number of issues and introduce an additional CLI command.
 
 CLI Commands and Options
 ========================
-- Removed annoying messages about new version check.
-- Deprecation warning on missing `--resourceGroup` option.
-- Changed `IDataProtectionProvider.CreateProtector` purpose string to prevent clashes.
-- New `update.mappings` command.
+- Fixes spurious upgrade message noted in #225.
 
 
 Docker and Azure Function Hosting
 ========================
-- Retry after Http 429 using Polly, address #71.
-- Harden Azure resources (see #225).
+No changes.
 
 
 Rule Language
 ========================
-No changes.
+- Added support for `.bypassrules` directive (#83, #228).
+- Fixes #231 (Directive check revision false missing after `update.rule`).
 
 
 Rule Interpreter Engine
 ========================
-- Address #185 by removing field when set to null value.
-- Impersonation does not work when a rule updates the same work item bug (#206).
+- Fixes #229 (Updating a work item field with impersonation enabled fails with the message: _Remove requires Value to be null_).
+- Fixes #234 (_Object reference not set_ when removing a work item link).
 
 
 Build, Test, Documentation
 ========================
-- Terraform and PowerShell scripts to setup a dev VM.
-- Use latest GitVersion.
-- Drop log streaming in favour of reading the application log file: integration tests should become more reliable.
+- SonarCloud now requires Java 11.
 
 
 File Hashes

diff --git a/src/.build-trigger b/src/.build-trigger
@@ -1 +0,0 @@
-.

diff --git a/src/aggregator-cli/Instances/FunctionRuntimePackage.cs b/src/aggregator-cli/Instances/FunctionRuntimePackage.cs
@@ -57,8 +57,8 @@ internal FunctionRuntimePackage(ILogger logger)
             logger.WriteVerbose($"Found {gitHubVersion.Name} on {gitHubVersion.When} in GitHub.");
 
             SemVersion.TryParse(gitHubVersion.Name.Substring(1), out var latest);
-            var current = new SemVersion(
-                Assembly.GetEntryAssembly().GetName().Version);
+            var asmVer = Assembly.GetEntryAssembly().GetName().Version;
+            var current = new SemVersion(asmVer.Major, asmVer.Minor, asmVer.Build);
 
             bool upgrade = latest.CompareTo(current) > 0;
             return upgrade

diff --git a/src/aggregator-cli/Mappings/UpdateMappingsCommand.cs b/src/aggregator-cli/Mappings/UpdateMappingsCommand.cs
@@ -5,13 +5,13 @@
 namespace aggregator.cli
 {
 
-    [Verb("update.mappings", HelpText = "Updates existing mappings from old to new Aggrergator Instance.")]
+    [Verb("update.mappings", HelpText = "Updates existing mappings from old to new Aggregator Instance.")]
     class UpdateMappingsCommand : CommandBase
     {
         [Option('s', "sourceInstance", Required = true, HelpText = "Source Aggregator instance name.")]
         public string SourceInstance { get; set; }
 
-        [Option('d', "destInstance", Required = true, HelpText = "Destionation Aggregator instance name.")]
+        [Option('d', "destInstance", Required = true, HelpText = "Destination Aggregator instance name.")]
         public string DestInstance { get; set; }
 
         [Option('g', "resourceGroup", Required = false, Default = "", HelpText = "Azure Resource Group hosting the Aggregator instance.")]

diff --git a/src/aggregator-cli/Program.cs b/src/aggregator-cli/Program.cs
@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Threading;
+using System.Threading.Tasks;
 using aggregator.cli.Instances;
 using CommandLine;
 using CommandLine.Text;
@@ -35,7 +36,7 @@ emulates the event on the rule
     */
     public static class Program
     {
-        public static int Main(string[] args)
+        public static async Task<int> Main(string[] args)
         {
             var mainTimer = new Stopwatch();
             mainTimer.Start();
@@ -63,7 +64,7 @@ void cancelEventHandler(object sender, ConsoleCancelEventArgs e)
                 {
                     var tempLogger = new ConsoleLogger(false);
                     var verChecker = new FunctionRuntimePackage(tempLogger);
-                    (bool upgrade, string newversion) = verChecker.IsCliUpgradable().Result;
+                    (bool upgrade, string newversion) = await verChecker.IsCliUpgradable();
                     if (upgrade)
                     {
                         // bug user

diff --git a/src/aggregator-cli/Rules/AggregatorRules.cs b/src/aggregator-cli/Rules/AggregatorRules.cs
@@ -164,7 +164,7 @@ internal async Task<bool> AddAsync(InstanceName instance, string ruleName, strin
             if (preprocessedRule.Impersonate)
             {
                 _logger.WriteInfo($"Configure {ruleName} to execute impersonated.");
-                ok &= await ConfigureAsync(instance, ruleName, impersonate: true, cancellationToken: cancellationToken);
+                ok &= await ConfigureAsync(instance, ruleName, impersonate: true, bypassrules: false, cancellationToken: cancellationToken);
                 if (ok)
                 {
                     _logger.WriteInfo($"Updated {ruleName} configuration successfully.");
@@ -339,7 +339,7 @@ internal async Task<bool> RemoveAsync(InstanceName instance, string name, Cancel
             return true;
         }
 
-        internal async Task<bool> ConfigureAsync(InstanceName instance, string name, bool? disable = null, bool? impersonate = null, CancellationToken cancellationToken = default)
+        internal async Task<bool> ConfigureAsync(InstanceName instance, string name, bool? disable = null, bool? impersonate = null, bool? bypassrules = null, CancellationToken cancellationToken = default)
         {
             var webFunctionApp = await GetWebApp(instance, cancellationToken);
             var configuration = await AggregatorConfiguration.ReadConfiguration(webFunctionApp);
@@ -355,6 +355,11 @@ internal async Task<bool> ConfigureAsync(InstanceName instance, string name, boo
                 ruleConfig.Impersonate = impersonate.Value;
             }
 
+            if (bypassrules.HasValue)
+            {
+                ruleConfig.BypassRules = impersonate.Value;
+            }
+
             ruleConfig.WriteConfiguration(webFunctionApp);
 
             return true;

diff --git a/src/aggregator-cli/Rules/ConfigureRuleCommand.cs b/src/aggregator-cli/Rules/ConfigureRuleCommand.cs
@@ -44,7 +44,7 @@ internal override async Task<int> RunAsync(CancellationToken cancellationToken)
             var disable = GetDisableStatus(Disable, Enable);
             var impersonate = GetEnableStatus(DisableImpersonateExecution, EnableImpersonateExecution);
 
-            var ok = await rules.ConfigureAsync(instance, Name, disable, impersonate, cancellationToken);
+            var ok = await rules.ConfigureAsync(instance, Name, disable, impersonate, false, cancellationToken);
             return ok ? ExitCodes.Success : ExitCodes.Failure;
         }
 

diff --git a/src/aggregator-ruleng/IRule.cs b/src/aggregator-ruleng/IRule.cs
@@ -18,6 +18,13 @@ public interface IRule
         /// <remarks>Setter public for CLI</remarks>
         bool ImpersonateExecution { get; set; }
 
+        /// <summary>
+        /// WorkItem creation/updates will bypass rules
+        /// Assumes PAT or Account Permission is high enough
+        /// </summary>
+        /// <remarks>Setter public for CLI</remarks>
+        bool BypassRules { get; set; }
+
         ///<summary>
         /// Configuration data picked by the directive parser that may influence a rule behaviour.
         ///</summary>

diff --git a/src/aggregator-ruleng/Language/IPreprocessedRule.cs b/src/aggregator-ruleng/Language/IPreprocessedRule.cs
@@ -5,6 +5,7 @@ namespace aggregator.Engine.Language
 {
     public interface IPreprocessedRule
     {
+        bool BypassRules { get; set; }
         bool Impersonate { get; set; }
         IRuleSettings Settings { get; }
         RuleLanguage Language { get; }

diff --git a/src/aggregator-ruleng/Language/PreprocessedRule.cs b/src/aggregator-ruleng/Language/PreprocessedRule.cs
@@ -8,6 +8,7 @@ internal class PreprocessedRule : IPreprocessedRule
     {
         public PreprocessedRule()
         {
+            BypassRules = false;
             Impersonate = false;
             Settings = new RuleSettings();
             Language = RuleLanguage.Unknown;
@@ -20,6 +21,8 @@ public PreprocessedRule()
 
         public bool Impersonate { get; set; }
 
+        public bool BypassRules { get; set; }
+
         public IRuleSettings Settings { get; private set; }
 
         public RuleLanguage Language { get; internal set; }

diff --git a/src/aggregator-ruleng/Language/RuleFileParser.cs b/src/aggregator-ruleng/Language/RuleFileParser.cs
@@ -56,7 +56,6 @@ void FailParsingWithMessage(string message)
 
         (IPreprocessedRule preprocessedRule, bool parseSuccess) Parse(string[] ruleCode)
         {
-
             var directiveLineIndex = 0;
             var preprocessedRule = new PreprocessedRule()
             {
@@ -123,7 +122,9 @@ void ParseDirective(PreprocessedRule preprocessedRule, string directive, string
                 case "impersonate":
                     ParseImpersonateDirective(preprocessedRule, directive, arguments);
                     break;
-
+                case "bypassrules":
+                    ParseBypassRulesDirective(preprocessedRule, directive, arguments);
+                    break;
                 case "check":
                     ParseCheckDirective(preprocessedRule, directive, arguments);
                     break;
@@ -214,6 +215,18 @@ private void ParseImpersonateDirective(PreprocessedRule preprocessedRule, string
             }
         }
 
+        private void ParseBypassRulesDirective(PreprocessedRule preprocessedRule, string directive, string arguments)
+        {
+            if (string.IsNullOrWhiteSpace(arguments))
+            {
+                FailParsingWithMessage($"Invalid bypassrules directive {directive}");
+            }
+            else
+            {
+                preprocessedRule.BypassRules = string.Equals("true", arguments.TrimEnd(), StringComparison.OrdinalIgnoreCase);
+            }
+        }
+
         private void ParseCheckDirective(PreprocessedRule preprocessedRule, string directive, string arguments)
         {
             if (string.IsNullOrWhiteSpace(arguments))
@@ -279,6 +292,14 @@ public static string[] Write(IPreprocessedRule preprocessedRule)
             {
                 content.Add($".impersonate=onBehalfOfInitiator");
             }
+            if (!preprocessedRule.Settings.EnableRevisionCheck)
+            {
+                content.Add($".check revision false");
+            }
+            if (preprocessedRule.BypassRules)
+            {
+                content.Add($".bypassrules=true");
+            }
 
             content.AddRange(preprocessedRule.References.Select(reference => $".reference={reference}"));
             content.AddRange(preprocessedRule.Imports.Select(import => $".import={import}"));

diff --git a/src/aggregator-ruleng/RuleEngine.cs b/src/aggregator-ruleng/RuleEngine.cs
@@ -69,7 +69,7 @@ protected override async Task<string> ExecuteRuleAsync(IRule rule, RuleExecution
             var result = await rule.ApplyAsync(executionContext, cancellationToken);
 
             var store = executionContext.store;
-            var (created, updated) = await store.SaveChanges(saveMode, !dryRun, rule.ImpersonateExecution, cancellationToken);
+            var (created, updated) = await store.SaveChanges(saveMode, !dryRun, rule.ImpersonateExecution, rule.BypassRules, cancellationToken);
             if (created + updated > 0)
             {
                 logger.WriteInfo($"Changes saved to Azure DevOps (mode {saveMode}): {created} created, {updated} updated.");

diff --git a/src/aggregator-ruleng/ScriptedRuleWrapper.cs b/src/aggregator-ruleng/ScriptedRuleWrapper.cs
@@ -30,6 +30,7 @@ public class ScriptedRuleWrapper : IRule
         internal IPreprocessedRule RuleDirectives { get; private set; }
 
         public IRuleSettings Settings { get; private set; }
+        public bool BypassRules { get; set; }
 
         private ScriptedRuleWrapper(string ruleName, IAggregatorLogger logger)
         {
@@ -72,8 +73,11 @@ public ScriptedRuleWrapper(string ruleName, IPreprocessedRule preprocessedRule,
 
         private void Initialize(IPreprocessedRule preprocessedRule)
         {
+
             RuleDirectives = preprocessedRule;
             ImpersonateExecution = RuleDirectives.Impersonate;
+            BypassRules = RuleDirectives.BypassRules;
+
             Settings = preprocessedRule.Settings;
 
             var references = new HashSet<Assembly>(DefaultAssemblyReferences().Concat(RuleDirectives.LoadAssemblyReferences()));

diff --git a/src/aggregator-ruleng/WorkItemRelationWrapper.cs b/src/aggregator-ruleng/WorkItemRelationWrapper.cs
@@ -49,7 +49,7 @@ internal WorkItemRelationWrapper(string type, string url, string comment) : this
 
         //TODO: quick filter for work item relations only
         internal static bool IsWorkItemRelation(WorkItemRelation relation) => IsWorkItemRelation(relation.Url);
-        internal static bool IsWorkItemRelation(RelationPatch relationPatch) => IsWorkItemRelation(relationPatch.url);
+        internal static bool IsWorkItemRelation(RelationPatch relationPatch) => IsWorkItemRelation(relationPatch?.url);
         private static bool IsWorkItemRelation(string url) => url?.Contains("/_apis/wit/workItems/", StringComparison.OrdinalIgnoreCase) ?? false;
     }
 }
diff --git a/src/aggregator-ruleng/WorkItemRelationWrapperCollection.cs b/src/aggregator-ruleng/WorkItemRelationWrapperCollection.cs
@@ -65,8 +65,7 @@ private bool RemoveRelation(WorkItemRelationWrapper item)
                 _pivotWorkItem.Changes.Add(new JsonPatchOperation()
                 {
                     Operation = Operation.Remove,
-                    Path = "/relations/-",
-                    Value = _original.IndexOf(item)
+                    Path = $"/relations/{_original.IndexOf(item)}"
                 });
                 _pivotWorkItem.IsDirty = true;
                 return true;

diff --git a/src/aggregator-ruleng/WorkItemStore.cs b/src/aggregator-ruleng/WorkItemStore.cs
@@ -162,7 +162,7 @@ private void ImpersonateChanges()
         }
 
         [System.Diagnostics.CodeAnalysis.SuppressMessage("Sonar Code Smell", "S907:\"goto\" statement should not be used")]
-        public async Task<(int created, int updated)> SaveChanges(SaveMode mode, bool commit, bool impersonate, CancellationToken cancellationToken)
+        public async Task<(int created, int updated)> SaveChanges(SaveMode mode, bool commit, bool impersonate, bool bypassrules, CancellationToken cancellationToken)
         {
             if (impersonate)
             {
@@ -175,20 +175,20 @@ private void ImpersonateChanges()
                     _context.Logger.WriteVerbose($"No save mode specified, assuming {SaveMode.TwoPhases}.");
                     goto case SaveMode.TwoPhases;
                 case SaveMode.Item:
-                    var resultItem = await SaveChanges_ByItem(commit, impersonate, cancellationToken);
+                    var resultItem = await SaveChanges_ByItem(commit, impersonate, bypassrules, cancellationToken);
                     return resultItem;
                 case SaveMode.Batch:
-                    var resultBatch = await SaveChanges_Batch(commit, impersonate, cancellationToken);
+                    var resultBatch = await SaveChanges_Batch(commit, impersonate, bypassrules, cancellationToken);
                     return resultBatch;
                 case SaveMode.TwoPhases:
-                    var resultTwoPhases = await SaveChanges_TwoPhases(commit, impersonate, cancellationToken);
+                    var resultTwoPhases = await SaveChanges_TwoPhases(commit, impersonate, bypassrules, cancellationToken);
                     return resultTwoPhases;
                 default:
                     throw new InvalidOperationException($"Unsupported save mode: {mode}.");
             }
         }
 
-        private async Task<(int created, int updated)> SaveChanges_ByItem(bool commit, bool impersonate, CancellationToken cancellationToken)
+        private async Task<(int created, int updated)> SaveChanges_ByItem(bool commit, bool impersonate, bool bypassrules, CancellationToken cancellationToken)
         {
             int createdCounter = 0;
             int updatedCounter = 0;
@@ -203,7 +203,7 @@ private void ImpersonateChanges()
                         item.Changes,
                         _context.ProjectName,
                         item.WorkItemType,
-                        bypassRules: impersonate,
+                        bypassRules: impersonate || bypassrules,
                         cancellationToken: cancellationToken
                     );
                 }
@@ -236,7 +236,7 @@ private void ImpersonateChanges()
                     _ = await _clients.WitClient.UpdateWorkItemAsync(
                         item.Changes,
                         item.Id,
-                        bypassRules: impersonate,
+                        bypassRules: impersonate || bypassrules,
                         cancellationToken: cancellationToken
                     );
                 }
@@ -251,7 +251,7 @@ private void ImpersonateChanges()
             return (createdCounter, updatedCounter);
         }
 
-        private async Task<(int created, int updated)> SaveChanges_Batch(bool commit, bool impersonate, CancellationToken cancellationToken)
+        private async Task<(int created, int updated)> SaveChanges_Batch(bool commit, bool impersonate, bool bypassrules, 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
@@ -280,7 +280,7 @@ private void ImpersonateChanges()
 
                 var request = _clients.WitClient.CreateWorkItemBatchRequest(item.Id,
                                                                             item.Changes,
-                                                                            bypassRules: impersonate,
+                                                                            bypassRules: impersonate || bypassrules,
                                                                             suppressNotifications: false);
                 batchRequests.Add(request);
             }
@@ -309,7 +309,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, bool impersonate, CancellationToken cancellationToken)
+        private async Task<(int created, int updated)> SaveChanges_TwoPhases(bool commit, bool impersonate, bool bypassrules, 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
@@ -337,7 +337,7 @@ private static bool IsSuccessStatusCode(int statusCode)
                 var request = _clients.WitClient.CreateWorkItemBatchRequest(_context.ProjectName,
                                                                             item.WorkItemType,
                                                                             document,
-                                                                            bypassRules: impersonate,
+                                                                            bypassRules: impersonate || bypassrules,
                                                                             suppressNotifications: false);
                 batchRequests.Add(request);
             }
@@ -366,10 +366,11 @@ private static bool IsSuccessStatusCode(int statusCode)
                     continue;
                 }
                 _context.Logger.WriteInfo($"Found a request to update workitem {item.Id} in {_context.ProjectName}");
+                _context.Logger.WriteVerbose(JsonConvert.SerializeObject(item.Changes));
 
                 var request = _clients.WitClient.CreateWorkItemBatchRequest(item.Id,
                                                                             item.Changes,
-                                                                            bypassRules: impersonate,
+                                                                            bypassRules: impersonate || bypassrules,
                                                                             suppressNotifications: false);
                 batchRequests.Add(request);
             }