HostObject Cleanup (#216)

* Don't disable nullable in shipping code

* One exception per class

* No need to override execute in the ToolTask

* Add const for env var names

* Capitalize consts

* Modify containerhttpexception message

* Remove warning when extracting host object

* Don't recalculate the json contents

* Remove extra HostObjectMagic function

* Lower-pri 'host object not found' message

Author: benvillalobos

Microsoft.NET.Build.Containers/AuthHandshakeMessageHandler.cs

@@ -88,8 +88,8 @@ private record TokenResponse(string? token, string? access_token, int? expires_i
     private async Task<string> GetTokenAsync(Uri realm, string service, string scope, CancellationToken cancellationToken)
     {
         // Allow overrides for auth via environment variables
-        string? credU = Environment.GetEnvironmentVariable("SDK_CONTAINER_REGISTRY_UNAME");
-        string? credP = Environment.GetEnvironmentVariable("SDK_CONTAINER_REGISTRY_PWORD");
+        string? credU = Environment.GetEnvironmentVariable(ContainerHelpers.HostObjectUser);
+        string? credP = Environment.GetEnvironmentVariable(ContainerHelpers.HostObjectPass);
 
         // fetch creds for the host
         DockerCredentials? privateRepoCreds;

Microsoft.NET.Build.Containers/ContainerHelpers.cs

@@ -20,6 +20,10 @@ public record Port(int number, PortType type);
 
 public static class ContainerHelpers
 {
+    public const string HostObjectUser = "SDK_CONTAINER_REGISTRY_UNAME";
+    
+    public const string HostObjectPass = "SDK_CONTAINER_REGISTRY_PWORD";
+
     private static Regex envVarRegex = new Regex(@"^[a-zA-Z_]+$");
 
     /// <summary>

Microsoft.NET.Build.Containers/ContainerHttpException.cs

@@ -0,0 +1,13 @@
+namespace Microsoft.NET.Build.Containers;
+public class ContainerHttpException : Exception
+{
+    private const string ErrorPrefix = "Containerize: error CONTAINER004:";
+    string? jsonResponse;
+    string? uri;
+    public ContainerHttpException(string message, string? targetUri, string? jsonResp)
+            : base($"{ErrorPrefix} {message}\nURI: {targetUri ?? "Unknown"}\nJson Response: {jsonResp ?? "None."}")
+    {
+        jsonResponse = jsonResp;
+        uri = targetUri;
+    }
+}

Microsoft.NET.Build.Containers/CreateNewImageToolTask.cs

@@ -139,29 +139,8 @@ public CreateNewImage()
         GeneratedContainerManifest = "";
     }
 
-    private void HostObjectMagic()
-    {
-        VSHostObject hostObj = new VSHostObject(HostObject as System.Collections.Generic.IEnumerable<ITaskItem>);
-        if (hostObj.ExtractCredentials(out string user, out string pass))
-        {
-            Log.LogWarning($"Host Object Retrieved.\nUser: {user}\nPass: {pass}");
-            extractionInfo = (true, user, pass);
-        }
-        else
-        {
-            Log.LogWarning("Host object failed to extract");
-        }
-            
-    }
-
     protected override string GenerateFullPathToTool() => Quote(Path.Combine(DotNetPath, ToolExe));
 
-    public override bool Execute()
-    {
-        HostObjectMagic();
-        return base.Execute();
-    }
-
     /// <summary>
     /// Workaround to avoid storing user/pass into the EnvironmentVariables property, which gets logged by the task.
     /// </summary>
@@ -176,12 +155,22 @@ protected override ProcessStartInfo GetProcessStartInfo
         string responseFileSwitch
     )
     {
+        VSHostObject hostObj = new VSHostObject(HostObject as System.Collections.Generic.IEnumerable<ITaskItem>);
+        if (hostObj.ExtractCredentials(out string user, out string pass))
+        {
+            extractionInfo = (true, user, pass);
+        }
+        else
+        {
+            Log.LogMessage(MessageImportance.Low, "No host object detected.");
+        }
+
         ProcessStartInfo startInfo = base.GetProcessStartInfo(pathToTool, commandLineCommands, responseFileSwitch)!;
 
         if (extractionInfo.success)
         {
-            startInfo.Environment["SDK_CONTAINER_REGISTRY_UNAME"] = extractionInfo.user;
-            startInfo.Environment["SDK_CONTAINER_REGISTRY_PWORD"] = extractionInfo.pass;
+            startInfo.Environment[ContainerHelpers.HostObjectUser] = extractionInfo.user;
+            startInfo.Environment[ContainerHelpers.HostObjectPass] = extractionInfo.pass;
         }
 
         return startInfo;

Microsoft.NET.Build.Containers/Exceptions.cs renamed to Microsoft.NET.Build.Containers/DockerLoadException.cs

@@ -20,16 +20,3 @@ protected DockerLoadException(SerializationInfo info, StreamingContext context)
     {
     }
 }
-
-public class ContainerHttpException : Exception
-{
-    private const string errorPrefix = "Containerize: error CONTAINER004:";
-    string? jsonResponse;
-    string? uri;
-    public ContainerHttpException(string message, string? targetUri, string? jsonResp) 
-            : base($"{errorPrefix} {message}\nURI: {targetUri ?? "None."}\nJson Response: {jsonResp ?? "None."}" )
-    {
-        jsonResponse = jsonResp;
-        uri = targetUri;
-    }
-}

Microsoft.NET.Build.Containers/Registry.cs

@@ -216,14 +216,14 @@ await Task.WhenAll(x.LayerDescriptors.Select(async descriptor => {
 
         var manifestDigest = x.GetDigest(x.manifest);
         logProgressMessage($"Uploading manifest to registry at blob {manifestDigest}");
-        HttpContent manifestUploadContent = new StringContent(x.manifest.ToJsonString());
+        string jsonString = x.manifest.ToJsonString();
+        HttpContent manifestUploadContent = new StringContent(jsonString);
         manifestUploadContent.Headers.ContentType = new MediaTypeHeaderValue(DockerManifestV2);
         var putResponse = await client.PutAsync(new Uri(BaseUri, $"/v2/{name}/manifests/{manifestDigest}"), manifestUploadContent);
 
         if (!putResponse.IsSuccessStatusCode)
         {
-            string jsonResponse = await putResponse.Content.ReadAsStringAsync();
-            throw new ContainerHttpException("Registry push failed.", putResponse.RequestMessage?.RequestUri?.ToString(), jsonResponse);
+            throw new ContainerHttpException("Registry push failed.", putResponse.RequestMessage?.RequestUri?.ToString(), jsonString);
         }
         logProgressMessage($"Uploaded manifest to registry");
 
@@ -232,8 +232,7 @@ await Task.WhenAll(x.LayerDescriptors.Select(async descriptor => {
 
         if (!putResponse2.IsSuccessStatusCode)
         {
-            string jsonResponse = await putResponse2.Content.ReadAsStringAsync();
-            throw new ContainerHttpException("Registry push failed.", putResponse2.RequestMessage?.RequestUri?.ToString(), jsonResponse);
+            throw new ContainerHttpException("Registry push failed.", putResponse2.RequestMessage?.RequestUri?.ToString(), jsonString);
         }
 
         logProgressMessage($"Uploaded tag to registry");

Microsoft.NET.Build.Containers/VSHostObject.cs

@@ -2,18 +2,16 @@
 using System.Linq;
 using Microsoft.Build.Framework;
 
-#nullable disable
-
 namespace Microsoft.NET.Build.Containers.Tasks;
 
 internal class VSHostObject
 {
     private const string CredentialItemSpecName = "MsDeployCredential";
     private const string UserMetaDataName = "UserName";
     private const string PasswordMetaDataName = "Password";
-    IEnumerable<ITaskItem> _hostObject;
+    IEnumerable<ITaskItem>? _hostObject;
 
-    public VSHostObject(IEnumerable<ITaskItem> hostObject)
+    public VSHostObject(IEnumerable<ITaskItem>? hostObject)
     {
         _hostObject = hostObject;
     }