Fix agent lint errors (#4117)

* Fix CA2000 error with secrets masker * Fix CA1711 for ServiceBootFlag * Fix CA2000 for JobRunner * Resolve CA2000 in StepHost * Resolve CA2000 in Windows service * Resolve CA2000 for WorkerCommandManager * Resove CA2000 for CodeCoverageCommands * Remove unused namespaces * Add fixture for SecretMasker tests * Change Fixtures -> Disposable * Use dispose pattern to fix errors * Formatting
microsoft · Feb 1, 2023 · 37bb9cf · 37bb9cf
1 parent d503197
commit 37bb9cf
Show file tree

Hide file tree

Showing 14 changed files with 71 additions and 49 deletions.
diff --git a/src/Agent.Listener/Configuration.Windows/NativeWindowsServiceHelper.cs b/src/Agent.Listener/Configuration.Windows/NativeWindowsServiceHelper.cs
@@ -515,7 +515,7 @@ public void InstallService(string serviceName, string serviceDisplayName, string
                                         serviceDisplayName,
                                         ServiceRights.AllAccess,
                                         SERVICE_WIN32_OWN_PROCESS,
-                                        ServiceBootFlag.AutoStart,
+                                        ServiceStartType.AutoStart,
                                         ServiceError.Normal,
                                         agentServiceExecutable,
                                         null,
@@ -1332,9 +1332,9 @@ public enum ServiceError
             Critical = 0x00000003
         }
 
-        public enum ServiceBootFlag
+        public enum ServiceStartType
         {
-            Start = 0x00000000,
+            BootStart = 0x00000000,
             SystemStart = 0x00000001,
             AutoStart = 0x00000002,
             DemandStart = 0x00000003,
@@ -1425,7 +1425,7 @@ private static extern IntPtr CreateService(
             string lpDisplayName,
             ServiceRights dwDesiredAccess,
             int dwServiceType,
-            ServiceBootFlag dwStartType,
+            ServiceStartType dwStartType,
             ServiceError dwErrorControl,
             string lpBinaryPathName,
             string lpLoadOrderGroup,

diff --git a/src/Agent.Sdk/Util/ExceptionsUtil.cs b/src/Agent.Sdk/Util/ExceptionsUtil.cs
@@ -2,10 +2,7 @@
 // Licensed under the MIT License.
 
 using System;
-using System.Collections.Generic;
 using System.Net.Sockets;
-using System.Text;
-using Microsoft.VisualStudio.Services.Agent;
 using Microsoft.VisualStudio.Services.Agent.Util;
 
 namespace Agent.Sdk.Util

diff --git a/src/Agent.Sdk/Util/ILoggedSecretMasker.cs b/src/Agent.Sdk/Util/ILoggedSecretMasker.cs
@@ -1,8 +1,5 @@
 using Microsoft.TeamFoundation.DistributedTask.Logging;
 using System;
-using System.Collections.Generic;
-using System.Text;
-using System.Text.RegularExpressions;
 
 namespace Agent.Sdk.Util
 {

diff --git a/src/Agent.Sdk/Util/LoggedSecretMasker.cs b/src/Agent.Sdk/Util/LoggedSecretMasker.cs
@@ -1,8 +1,5 @@
 using Microsoft.TeamFoundation.DistributedTask.Logging;
 using System;
-using System.Collections.Generic;
-using System.Text;
-using System.Text.RegularExpressions;
 
 namespace Agent.Sdk.Util
 {

diff --git a/src/Agent.Sdk/Util/MaskingUtil.cs b/src/Agent.Sdk/Util/MaskingUtil.cs
@@ -1,7 +1,5 @@
 using Microsoft.VisualStudio.Services.ServiceEndpoints.WebApi;
 using System;
-using System.Collections.Generic;
-using System.Text;
 
 namespace Microsoft.VisualStudio.Services.Agent.Util
 {

diff --git a/src/Agent.Sdk/Util/PlatformUtil.cs b/src/Agent.Sdk/Util/PlatformUtil.cs
@@ -2,23 +2,16 @@
 // Licensed under the MIT License.
 
 using System;
-using System.Collections.Generic;
-using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using System.Net.Http;
-using System.Net.Sockets;
 using System.Reflection;
 using System.Runtime.InteropServices;
 using System.Text;
 using System.Text.RegularExpressions;
 using System.Threading.Tasks;
 using System.Xml.Linq;
 using Agent.Sdk.Knob;
-using Agent.Sdk.Util;
-using BuildXL.Cache.MemoizationStore.Interfaces.Caches;
-using BuildXL.Utilities;
-using Microsoft.TeamFoundation.Build.WebApi;
 using Microsoft.VisualStudio.Services.Agent.Util;
 using Microsoft.Win32;
 using Newtonsoft.Json;

diff --git a/src/Agent.Service/Windows/Program.cs b/src/Agent.Service/Windows/Program.cs
@@ -24,11 +24,13 @@ static int Main(String[] args)
                     return 1;
                 }
 
-                EventLog applicationLog = new EventLog("Application");
-                if (applicationLog.OverflowAction == OverflowAction.DoNotOverwrite)
+                using (EventLog applicationLog = new EventLog("Application"))
                 {
-                    Console.WriteLine("[WARNING] The retention policy for Application event log is set to \"Do not overwrite events\".");
-                    Console.WriteLine("[WARNING] Make sure manually clear logs as needed, otherwise AgentService will stop writing output to event log.");
+                    if (applicationLog.OverflowAction == OverflowAction.DoNotOverwrite)
+                    {
+                        Console.WriteLine("[WARNING] The retention policy for Application event log is set to \"Do not overwrite events\".");
+                        Console.WriteLine("[WARNING] Make sure manually clear logs as needed, otherwise AgentService will stop writing output to event log.");
+                    }
                 }
 
                 try

diff --git a/src/Agent.Worker/CodeCoverage/CodeCoverageCommands.cs b/src/Agent.Worker/CodeCoverage/CodeCoverageCommands.cs
@@ -149,9 +149,8 @@ private async Task PublishCodeCoverageAsync(
             }
             catch (SocketException ex)
             {
-#pragma warning disable CA2000 // Dispose objects before losing scope
-                ExceptionsUtil.HandleSocketException(ex, WorkerUtilities.GetVssConnection(executionContext).Uri.ToString(), executionContext.Warning);
-#pragma warning restore CA2000 // Dispose objects before losing scope
+                using var vssConnection = WorkerUtilities.GetVssConnection(executionContext);
+                ExceptionsUtil.HandleSocketException(ex, vssConnection.Uri.ToString(), executionContext.Warning);
             }
             catch (Exception ex)
             {

diff --git a/src/Agent.Worker/Handlers/StepHost.cs b/src/Agent.Worker/Handlers/StepHost.cs
@@ -211,7 +211,7 @@ public async Task<int> ExecuteAsync(string workingDirectory,
                     outputEncoding = Encoding.UTF8;
                 }
 
-                var redirectStandardIn = new InputQueue<string>();
+                using var redirectStandardIn = new InputQueue<string>();
                 var payloadJson = JsonUtility.ToString(payload);
                 redirectStandardIn.Enqueue(payloadJson);
                 HostContext.GetTrace(nameof(ContainerStepHost)).Info($"Payload: {payloadJson}");

diff --git a/src/Agent.Worker/JobRunner.cs b/src/Agent.Worker/JobRunner.cs
@@ -93,6 +93,9 @@ public async Task<TaskResult> RunAsync(Pipelines.AgentJobRequestMessage message,
 
             IExecutionContext jobContext = null;
             CancellationTokenRegistration? agentShutdownRegistration = null;
+            VssConnection taskConnection = null;
+            VssConnection legacyTaskConnection = null;
+
             try
             {
                 // Create the job execution context.
@@ -226,7 +229,9 @@ public async Task<TaskResult> RunAsync(Pipelines.AgentJobRequestMessage message,
                 if (taskServerUri != null)
                 {
                     Trace.Info($"Creating task server with {taskServerUri}");
-                    await taskServer.ConnectAsync(VssUtil.CreateConnection(taskServerUri, taskServerCredential, trace: Trace));
+
+                    taskConnection = VssUtil.CreateConnection(taskServerUri, taskServerCredential, Trace);
+                    await taskServer.ConnectAsync(taskConnection);
                 }
 
                 // for back compat TFS 2015 RTM/QU1, we may need to switch the task server url to agent config url
@@ -237,8 +242,10 @@ public async Task<TaskResult> RunAsync(Pipelines.AgentJobRequestMessage message,
                         Trace.Info($"Can't determine task download url from JobMessage or the endpoint doesn't exist.");
                         var configStore = HostContext.GetService<IConfigurationStore>();
                         taskServerUri = new Uri(configStore.GetSettings().ServerUrl);
+
                         Trace.Info($"Recreate task server with configuration server url: {taskServerUri}");
-                        await taskServer.ConnectAsync(VssUtil.CreateConnection(taskServerUri, taskServerCredential, trace: Trace));
+                        legacyTaskConnection = VssUtil.CreateConnection(taskServerUri, taskServerCredential, trace: Trace);
+                        await taskServer.ConnectAsync(legacyTaskConnection);
                     }
                 }
 
@@ -381,6 +388,10 @@ public async Task<TaskResult> RunAsync(Pipelines.AgentJobRequestMessage message,
                     agentShutdownRegistration = null;
                 }
 
+                legacyTaskConnection?.Dispose();
+                taskConnection?.Dispose();
+                jobConnection?.Dispose();
+
                 await ShutdownQueue(throwOnFailure: false);
             }
         }

diff --git a/src/Agent.Worker/WorkerCommandManager.cs b/src/Agent.Worker/WorkerCommandManager.cs
@@ -111,9 +111,9 @@ public bool TryProcessCommand(IExecutionContext context, string input)
                     }
                     catch (SocketException ex)
                     {
-#pragma warning disable CA2000 // Dispose objects before losing scope
-                        ExceptionsUtil.HandleSocketException(ex, WorkerUtilities.GetVssConnection(context).Uri.ToString(), context.Error);
-#pragma warning restore CA2000 // Dispose objects before losing scope
+                        using var vssConnection = WorkerUtilities.GetVssConnection(context);
+
+                        ExceptionsUtil.HandleSocketException(ex, vssConnection.Uri.ToString(), context.Error);
                         context.CommandResult = TaskResult.Failed;
                     }
                     catch (Exception ex)

diff --git a/src/Microsoft.VisualStudio.Services.Agent/HostContext.cs b/src/Microsoft.VisualStudio.Services.Agent/HostContext.cs
@@ -68,6 +68,7 @@ public class HostContext : EventListener, IObserver<DiagnosticListener>, IObserv
         private static int[] _vssHttpCredentialEventIds = new int[] { 11, 13, 14, 15, 16, 17, 18, 20, 21, 22, 27, 29 };
         private readonly ConcurrentDictionary<Type, object> _serviceInstances = new ConcurrentDictionary<Type, object>();
         protected readonly ConcurrentDictionary<Type, Type> ServiceTypes = new ConcurrentDictionary<Type, Type>();
+        SecretMasker _basicSecretMasker = new SecretMasker();
         private readonly ILoggedSecretMasker _secretMasker;
         private readonly ProductInfoHeaderValue _userAgent = new ProductInfoHeaderValue($"VstsAgentCore-{BuildConstants.AgentPackage.PackageName}", BuildConstants.AgentPackage.Version);
         private CancellationTokenSource _agentShutdownTokenSource = new CancellationTokenSource();
@@ -89,7 +90,8 @@ public class HostContext : EventListener, IObserver<DiagnosticListener>, IObserv
         public ProductInfoHeaderValue UserAgent => _userAgent;
         public HostContext(HostType hostType, string logFile = null)
         {
-            _secretMasker = new LoggedSecretMasker(new SecretMasker());
+            _secretMasker = new LoggedSecretMasker(_basicSecretMasker);
+
             // Validate args.
             if (hostType == HostType.Undefined)
             {
@@ -593,6 +595,8 @@ protected virtual void Dispose(bool disposing)
                 _trace = null;
                 _httpTrace?.Dispose();
                 _httpTrace = null;
+                _basicSecretMasker?.Dispose();
+                _basicSecretMasker = null;
 
                 _agentShutdownTokenSource?.Dispose();
                 _agentShutdownTokenSource = null;

diff --git a/src/Test/L0/SecretMaskerTests/LoggedSecretMaskerL0.cs b/src/Test/L0/SecretMaskerTests/LoggedSecretMaskerL0.cs
@@ -5,14 +5,22 @@
 
 namespace Microsoft.VisualStudio.Services.Agent.Tests
 {
-    public class LoggedSecretMaskerL0
+    public class LoggedSecretMaskerL0 : IDisposable
     {
+        SecretMasker _secretMasker;
+        private bool disposedValue;
+
+        public LoggedSecretMaskerL0()
+        {
+            _secretMasker = new SecretMasker();
+        }
+
         [Fact]
         [Trait("Level", "L0")]
         [Trait("Category", "SecretMasker")]
         public void LoggedSecretMasker_MaskingSecrets()
         {
-            var lsm = new LoggedSecretMasker(new SecretMasker())
+            var lsm = new LoggedSecretMasker(_secretMasker)
             {
                 MinSecretLength = 0
             };
@@ -29,7 +37,7 @@ public void LoggedSecretMasker_MaskingSecrets()
         [Trait("Category", "SecretMasker")]
         public void LoggedSecretMasker_ShortSecret_Removes_From_Dictionary()
         {
-            var lsm = new LoggedSecretMasker(new SecretMasker())
+            var lsm = new LoggedSecretMasker(_secretMasker)
             {
                 MinSecretLength = 0
             };
@@ -48,7 +56,7 @@ public void LoggedSecretMasker_ShortSecret_Removes_From_Dictionary()
         [Trait("Category", "SecretMasker")]
         public void LoggedSecretMasker_ShortSecret_Removes_From_Dictionary_BoundaryValue()
         {
-            var lsm = new LoggedSecretMasker(new SecretMasker())
+            var lsm = new LoggedSecretMasker(_secretMasker)
             {
                 MinSecretLength = 3
             };
@@ -66,7 +74,7 @@ public void LoggedSecretMasker_ShortSecret_Removes_From_Dictionary_BoundaryValue
         [Trait("Category", "SecretMasker")]
         public void LoggedSecretMasker_Skipping_ShortSecrets()
         {
-            var lsm = new LoggedSecretMasker(new SecretMasker())
+            var lsm = new LoggedSecretMasker(_secretMasker)
             {
                 MinSecretLength = 3
             };
@@ -82,7 +90,7 @@ public void LoggedSecretMasker_Skipping_ShortSecrets()
         [Trait("Category", "SecretMasker")]
         public void LoggedSecretMasker_Throws_Exception_If_Large_MinSecretLength_Specified()
         {
-            var lsm = new LoggedSecretMasker(new SecretMasker());
+            var lsm = new LoggedSecretMasker(_secretMasker);
 
             Assert.Throws<ArgumentException>(() => lsm.MinSecretLength = 5);
         }
@@ -92,7 +100,7 @@ public void LoggedSecretMasker_Throws_Exception_If_Large_MinSecretLength_Specifi
         [Trait("Category", "SecretMasker")]
         public void LoggedSecretMasker_Sets_MinSecretLength_To_MaxValue()
         {
-            var lsm = new LoggedSecretMasker(new SecretMasker());
+            var lsm = new LoggedSecretMasker(_secretMasker);
 
             try { lsm.MinSecretLength = 5; }
             catch (ArgumentException) { }
@@ -105,7 +113,7 @@ public void LoggedSecretMasker_Sets_MinSecretLength_To_MaxValue()
         [Trait("Category", "SecretMasker")]
         public void LoggedSecretMasker_NegativeValue_Passed()
         {
-            var lsm = new LoggedSecretMasker(new SecretMasker())
+            var lsm = new LoggedSecretMasker(_secretMasker)
             {
                 MinSecretLength = -2
             };
@@ -116,5 +124,26 @@ public void LoggedSecretMasker_NegativeValue_Passed()
 
             Assert.Equal("***2345", resultMessage);
         }
+
+        protected virtual void Dispose(bool disposing)
+        {
+            if (!disposedValue)
+            {
+                if (disposing)
+                {
+                }
+
+                _secretMasker.Dispose();
+                _secretMasker = null;
+
+                disposedValue = true;
+            }
+        }
+
+        public void Dispose()
+        {
+            Dispose(disposing: true);
+            GC.SuppressFinalize(this);
+        }
     }
 }
diff --git a/src/Test/L0/SecretMaskerTests/SecretMaskerL0.cs b/src/Test/L0/SecretMaskerTests/SecretMaskerL0.cs
@@ -2,11 +2,6 @@
 // Licensed under the MIT License.
 
 using Microsoft.TeamFoundation.DistributedTask.Logging;
-using Microsoft.VisualStudio.Services.Agent.Worker;
-using Microsoft.VisualStudio.Services.Agent.Worker.Build;
-using System;
-using System.Collections.Generic;
-using System.Linq;
 using Xunit;
 
 namespace Microsoft.VisualStudio.Services.Agent.Tests