Fix notification checks (#1707)

* Fix notification checks

- Ensures that the team name as well as pipeline id match
- If the name and id don't match then update one or the other so
  that we can keep updated with pipeline updates (i.e. renames or deletes)
- Ensure the pipeline in the notification settings matches the current pipeline name.

* Fix build warning/errors

eng/pipelines/notifications.yml

@@ -37,7 +37,7 @@ stages:
 
     steps:
 
-      - task: DotNetCoreInstaller@0
+      - task: UseDotNet@2
         displayName: 'Use .NET Core sdk $(DotNetCoreVersion)'
         inputs:
           version: '$(DotNetCoreVersion)'

tools/notification-configuration/common/Helpers/GitHubNameResolver.cs

@@ -82,6 +82,7 @@ public async Task<string> GetInternalUserPrincipal(string githubUserName)
             return result;
         }
 
+#pragma warning disable 1998
         public async Task<string> GetInternalUserPrincipalImpl(string githubUserName)
         {
             var query = $"{kustoTable} | where githubUserName == '{githubUserName}' | project aadUpn | limit 1;";
@@ -126,6 +127,7 @@ public async Task<IdentityDetail> GetMappingInformationFromAADName(string aadNam
                 return default;
             }
         }
+#pragma warning restore 1998
 
         /// <summary>
         /// Disposes the GitHubNameResolver

tools/notification-configuration/common/Services/AzureDevOpsService.cs

@@ -166,7 +166,22 @@ public async Task<WebApiTeam> CreateTeamForProjectAsync(string projectId, WebApi
 
             var result = await client.CreateTeamAsync(team, projectId);
             return result;
+        }
+
+        /// <summary>
+        /// Updates a team in the given project
+        /// </summary>
+        /// <param name="projectId">ID of project to associate with team</param>
+        /// <param name="team">Team to create</param>
+        /// <returns>Team with properties updated</returns>
+        public async Task<WebApiTeam> UpdateTeamForProjectAsync(string projectId, WebApiTeam team)
+        {
+            var client = await GetClientAsync<TeamHttpClient>();
+
+            logger.LogInformation("UpdateTeamForProjectAsync TeamName = {0} ProjectId = {1}", team.Name, projectId);
 
+            var result = await client.UpdateTeamAsync(team, projectId, team.Id.ToString());
+            return result;
         }
 
         /// <summary>
@@ -253,7 +268,7 @@ public async Task RemoveMember(string groupDescriptor, string memberDescriptor)
         {
             var client = await GetClientAsync<GraphHttpClient>();
 
-            logger.LogInformation("RemoveMember GroupDescriptor = {0}, MemberDescriptor = {1}");
+            logger.LogInformation("RemoveMember GroupDescriptor = {0}, MemberDescriptor = {1}", groupDescriptor, memberDescriptor);
             await client.RemoveMembershipAsync(memberDescriptor, groupDescriptor);
         }
 
@@ -275,19 +290,14 @@ public async Task<GraphMembership> AddToTeamAsync(string parent, string child)
         /// <summary>
         /// Gets subscriptions for a given target GUID
         /// </summary>
-        /// <remarks>
-        /// Some properties of the NotificationSubscription (like "Filter") are
-        /// not resolved in this API call. Expansion with a followup call may
-        /// be required
-        /// </remarks>
         /// <param name="targetId">GUID of the subscription target</param>
         /// <returns>Complete subscription objects</returns>
         public async Task<IEnumerable<NotificationSubscription>> GetSubscriptionsAsync(Guid targetId)
         {
             var client = await GetClientAsync<NotificationHttpClient>();
 
             logger.LogInformation("GetSubscriptionsAsync TargetId = {0}", targetId);
-            var result = await client.ListSubscriptionsAsync(targetId);
+            var result = await client.ListSubscriptionsAsync(targetId, queryFlags: SubscriptionQueryFlags.IncludeFilterDetails);
 
             return result;
         }
@@ -305,5 +315,19 @@ public async Task<NotificationSubscription> CreateSubscriptionAsync(Notification
             var result = await client.CreateSubscriptionAsync(newSubscription);
             return result;
         }
+
+        /// <summary>
+        /// Updates a subscription
+        /// </summary>
+        /// <param name="newSubscription">Subscription to update</param>
+        /// <returns>The updated subscription</returns>
+        public async Task<NotificationSubscription> UpdatedSubscriptionAsync(NotificationSubscriptionUpdateParameters updateParameters, string subscriptionId)
+        {
+            var client = await GetClientAsync<NotificationHttpClient>();
+
+            logger.LogInformation("UpdateSubscriptionAsync Id = {0}", subscriptionId);
+            var result = await client.UpdateSubscriptionAsync(updateParameters, subscriptionId);
+            return result;
+        }
     }
 }

tools/notification-configuration/common/common.csproj

@@ -7,9 +7,9 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.Azure.Kusto.Data" Version="7.1.0" />
     <PackageReference Include="Microsoft.Extensions.Logging" Version="2.2.0" />
-    <PackageReference Include="Microsoft.TeamFoundationServer.Client" Version="16.153.0-preview" />
-    <PackageReference Include="Microsoft.TeamFoundationServer.ExtendedClient" Version="16.153.0-preview" />
-    <PackageReference Include="Microsoft.VisualStudio.Services.Notifications.WebApi" Version="16.153.0-preview" />
+    <PackageReference Include="Microsoft.TeamFoundationServer.Client" Version="16.170.0" />
+    <PackageReference Include="Microsoft.TeamFoundationServer.ExtendedClient" Version="16.170.0" />
+    <PackageReference Include="Microsoft.VisualStudio.Services.Notifications.WebApi" Version="16.170.0" />
     <PackageReference Include="YamlDotNet" Version="6.1.1" />
   </ItemGroup>
 

tools/notification-configuration/identity-resolver/Program.cs

@@ -10,7 +10,7 @@ class Program
     {
 
         /// <summary>
-        /// Retrieves github<->ms mapping information given the full name of an employee.
+        /// Retrieves github-to-ms mapping information given the full name of an employee.
         /// </summary>
         /// <param name="aadAppIdVar">AAD App ID environment variable name (Kusto access)</param>
         /// <param name="aadAppSecretVar">AAD App Secret environment variable name (Kusto access)</param>

tools/notification-configuration/identity-resolver/identity-resolver.csproj

@@ -9,7 +9,7 @@
   <ItemGroup>
     <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="5.0.0" />
     <PackageReference Include="System.CommandLine.DragonFruit" Version="0.3.0-alpha.19317.1" />
-    <PackageReference Include="Newtonsoft.Json" Version="11.0.2" />
+    <PackageReference Include="Newtonsoft.Json" Version="12.0.3" />
   </ItemGroup>
 
   <ItemGroup>

tools/notification-configuration/notification-creator/NotificationConfigurator.cs

@@ -73,14 +73,40 @@ private async Task<WebApiTeam> EnsureTeamExists(
             IEnumerable<WebApiTeam> teams, 
             bool persistChanges)
         {
+            // Ensure team name fits within maximum 64 character limit
+            // https://docs.microsoft.com/en-us/azure/devops/organizations/settings/naming-restrictions?view=azure-devops#teams
+            string teamName = StringHelper.MaxLength($"{pipeline.Name} -- {suffix}", MaxTeamNameLength);
+            bool updateMetadataAndName = false;
             var result = teams.FirstOrDefault(
                 team =>
                 {
                     // Swallowing exceptions because parse errors on
                     // free form text fields which might be non-yaml text
                     // are not exceptional
                     var metadata = YamlHelper.Deserialize<TeamMetadata>(team.Description, swallowExceptions: true);
-                    return metadata?.PipelineId == pipeline.Id && metadata?.Purpose == purpose;
+                    bool metadataMatches = (metadata?.PipelineId == pipeline.Id && metadata?.Purpose == purpose);
+                    bool nameMatches = (team.Name == teamName);
+
+                    if (metadataMatches && nameMatches)
+                    {
+                        return true;
+                    }
+
+                    if (metadataMatches)
+                    {
+                        logger.LogInformation("Found team with matching pipeline id {0} but different name '{1}', expected '{2}'. Purpose = '{3}'", metadata?.PipelineId, team.Name, teamName, metadata?.Purpose);
+                        updateMetadataAndName = true;
+                        return true;
+                    }
+
+                    if (nameMatches)
+                    {
+                        logger.LogInformation("Found team with matching name {0} but different pipeline id {1}, expected {2}. Purpose = '{3}'", team.Name, metadata?.PipelineId, pipeline.Id, metadata?.Purpose);
+                        updateMetadataAndName = true;
+                        return true;
+                    }
+
+                    return false;
                 });
 
             if (result == default)
@@ -94,17 +120,31 @@ private async Task<WebApiTeam> EnsureTeamExists(
                 var newTeam = new WebApiTeam
                 {
                     Description = YamlHelper.Serialize(teamMetadata),
-                    // Ensure team name fits within maximum 64 character limit
-                    // https://docs.microsoft.com/en-us/azure/devops/organizations/settings/naming-restrictions?view=azure-devops#teams
-                    Name = StringHelper.MaxLength($"{pipeline.Name} -- {suffix}", MaxTeamNameLength),
+                    Name = teamName
                 };
 
-                logger.LogInformation("Create Team for Pipeline PipelineId = {0} Purpose = {1}", pipeline.Id, purpose);
+                logger.LogInformation("Create Team for Pipeline PipelineId = {0} Purpose = {1} Name = '{2}'", pipeline.Id, purpose, teamName);
                 if (persistChanges)
                 {
                     result = await service.CreateTeamForProjectAsync(pipeline.Project.Id.ToString(), newTeam);
                 }
             }
+            else if (updateMetadataAndName)
+            {
+                var teamMetadata = new TeamMetadata
+                {
+                    PipelineId = pipeline.Id,
+                    Purpose = purpose,
+                };
+                result.Description = YamlHelper.Serialize(teamMetadata);
+                result.Name = teamName;
+
+                logger.LogInformation("Update Team for Pipeline PipelineId = {0} Purpose = {1} Name = '{2}'", pipeline.Id, purpose, teamName);
+                if (persistChanges)
+                {
+                    result = await service.UpdateTeamForProjectAsync(pipeline.Project.Id.ToString(), result);
+                }
+            }
 
             return result;
         }
@@ -117,13 +157,11 @@ private async Task<IEnumerable<BuildDefinition>> GetPipelinesAsync(string projec
             {
                 case PipelineSelectionStrategy.All:
                     return definitions;
-                    break;
                 case PipelineSelectionStrategy.Scheduled:
                 default:
                     return definitions.Where(
                         def => def.Triggers.Any(
                             trigger => trigger.TriggerType == DefinitionTriggerType.Schedule));
-                    break;
             }
         }
 
@@ -152,18 +190,19 @@ private async Task EnsureScheduledBuildFailSubscriptionExists(BuildDefinition pi
             const string BuildFailureNotificationTag = "#AutomaticBuildFailureNotification";
             var subscriptions = await service.GetSubscriptionsAsync(team.Id);
 
-            var hasSubscription = subscriptions.Any(sub => sub.Description.Contains(BuildFailureNotificationTag));
+            var subscription = subscriptions.FirstOrDefault(sub => sub.Description.Contains(BuildFailureNotificationTag));
 
-            logger.LogInformation("Team Is Subscribed TeamId = {0} PipelineId = {1} HasSubscription = {2}", team.Id, pipeline.Id, hasSubscription);
+            logger.LogInformation("Team Is Subscribed TeamName = {0} PipelineId = {1}", team.Name, pipeline.Id);
 
-            if (!hasSubscription)
+            string definitionName = $"\\{pipeline.Project.Name}\\{pipeline.Name}";
+            if (subscription == default)
             {
                 var filterModel = new ExpressionFilterModel
                 {
                     Clauses = new ExpressionFilterClause[]
                     {
                         new ExpressionFilterClause { Index = 1, LogicalOperator = "", FieldName = "Status", Operator = "=", Value = "Failed" },
-                        new ExpressionFilterClause { Index = 2, LogicalOperator = "And", FieldName = "Definition name", Operator = "=", Value = $"\\{pipeline.Project.Name}\\{pipeline.Name}" },
+                        new ExpressionFilterClause { Index = 2, LogicalOperator = "And", FieldName = "Definition name", Operator = "=", Value = definitionName },
                         new ExpressionFilterClause { Index = 3, LogicalOperator = "And", FieldName = "Build reason", Operator = "=", Value = "Scheduled" }
                     }
                 };
@@ -187,7 +226,42 @@ private async Task EnsureScheduledBuildFailSubscriptionExists(BuildDefinition pi
                 logger.LogInformation("Creating Subscription PipelineId = {0}, TeamId = {1}", pipeline.Id, team.Id);
                 if (persistChanges)
                 {
-                    var subscription = await service.CreateSubscriptionAsync(newSubscription);
+                    subscription = await service.CreateSubscriptionAsync(newSubscription);
+                }
+            }
+            else
+            {
+                var filter = subscription.Filter as ExpressionFilter;
+                if (filter == null)
+                {
+                    logger.LogWarning("Subscription expression is not correct for of team {0}", team.Name);
+                    return;
+                }
+
+                var definitionClause = filter.FilterModel.Clauses.FirstOrDefault(c => c.FieldName == "Definition name");
+
+                if (definitionClause == null)
+                {
+                    logger.LogWarning("Subscription doesn't have correct expression filters for of team {0}", team.Name);
+                    return;
+                }
+
+                if (definitionClause.Value != definitionName)
+                {
+                    definitionClause.Value = definitionName;
+
+                    if (persistChanges)
+                    {
+                        var updateParameters = new NotificationSubscriptionUpdateParameters()
+                        {
+                            Channel = subscription.Channel,
+                            Description = subscription.Description,
+                            Filter = subscription.Filter,
+                            Scope = subscription.Scope,
+                        };
+                        logger.LogInformation("Updating Subscription expression for team {0} with correct definition name {1}", team.Name, definitionName);
+                        subscription = await service.UpdatedSubscriptionAsync(updateParameters, subscription.Id.ToString());
+                    }
                 }
             }
         }