Skip to content

Commit

Permalink
more exhaustive port testing and error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
@baronfel
baronfel committed Aug 22, 2022
1 parent 9dfd402 commit 1229a38
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 31 deletions.
61 changes: 53 additions & 8 deletions Microsoft.NET.Build.Containers/ContainerHelpers.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,18 +146,63 @@ public static bool NormalizeImageName(string containerImageName, [NotNullWhen(fa
}
}

public static bool TryParsePort(string input, [NotNullWhen(true)] out Port? port)
[Flags]
public enum ParsePortError
{
MissingPortNumber,
InvalidPortNumber,
InvalidPortType,
UnknownPortFormat
}

public record ParsePortResult(bool success, Port? port, ParsePortError? parseErrors);

public static ParsePortResult ParsePort(string? portNumber, string? portType)
{
ParsePortError? errors = null;
int port = 0;
if (String.IsNullOrEmpty(portNumber))
{
errors = ParsePortError.MissingPortNumber;
}
else if (!int.TryParse(portNumber, out port))
{
errors = ParsePortError.InvalidPortNumber;
}

if (!Enum.TryParse<PortType>(portType, out PortType t))
{
if (portType is not null)
{
errors = (errors ?? ParsePortError.InvalidPortType) | ParsePortError.InvalidPortType;
}
else
{
t = PortType.tcp;
}
}

if (errors is null)
{
return new(true, new(port, t), errors);
}
else
{
return new(false, null, errors);
}

}

public static ParsePortResult TryParsePort(string input)
{
var parts = input.Split('/');
Port? p =
ParsePortResult p =
parts switch
{
[var portNumber, var type] when int.TryParse(portNumber, out var portInt)
&& Enum.TryParse<PortType>(type, out var portType) => new(portInt, portType),
[var portNumber] when int.TryParse(portNumber, out var portInt) => new(portInt, PortType.tcp),
_ => null
[var portNumber, var type] => ParsePort(portNumber, type),
[var portNumber] => ParsePort(portNumber, null),
_ => new(false, null, ParsePortError.UnknownPortFormat)
};
port = p;
return p != null;
return p;
}
}
72 changes: 63 additions & 9 deletions Microsoft.NET.Build.Containers/CreateNewImage.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ public class CreateNewImage : Microsoft.Build.Utilities.Task
/// </summary>
public ITaskItem[] Labels { get; set; }

private bool IsDockerPush { get => OutputRegistry == "docker://"; }

public CreateNewImage()
{
BaseRegistry = "";
Expand All @@ -94,6 +96,53 @@ public CreateNewImage()
ExposedPorts = Array.Empty<ITaskItem>();
}

private void SetPorts(Image image, ITaskItem[] exposedPorts)
{
foreach (var port in exposedPorts)
{
var portNo = port.ItemSpec;
var portTy = port.GetMetadata("Type");
var parsePortResult = ContainerHelpers.ParsePort(portNo, portTy);
if (!parsePortResult.success)
{
ContainerHelpers.ParsePortError errors = (ContainerHelpers.ParsePortError)parsePortResult.parseErrors!;
var portString = portTy == null ? portNo : $"{portNo}/{portTy}";
if (errors.HasFlag(ContainerHelpers.ParsePortError.MissingPortNumber))
{
Log.LogError("A ContainerPort item was provided without an Include metadata specifying the port number. Please provide a ContainerPort item with an ItemSpec: <ContainerPort Include=\"80\" />");
}
else
{
var message = "A ContainerPort item was provided with ";
var arguments = new List<string>(2);
if (errors.HasFlag(ContainerHelpers.ParsePortError.InvalidPortNumber) && errors.HasFlag(ContainerHelpers.ParsePortError.InvalidPortNumber))
{
message += "an invalid port number '{0}' and an invalid port type '{1}'";
arguments.Add(portNo);
arguments.Add(portTy!);
}
else if (errors.HasFlag(ContainerHelpers.ParsePortError.InvalidPortNumber))
{
message += "an invalid port number '{0}'";
arguments.Add(portNo);
}
else if (errors.HasFlag(ContainerHelpers.ParsePortError.InvalidPortNumber))
{
message += "an invalid port type '{0}'";
arguments.Add(portTy!);
}
message += ". ContainerPort items must have an Include value that is an integer, and a Type value that is either 'tcp' or 'udp'";

Log.LogError(message, arguments);
}
}
else
{
image.ExposePort(parsePortResult.port!.number, parsePortResult.port.type);
}
}

}

public override bool Execute()
{
Expand Down Expand Up @@ -131,17 +180,15 @@ public override bool Execute()
image.Label(label.ItemSpec, label.GetMetadata("Value"));
}

foreach (var port in ExposedPorts)
SetPorts(image, ExposedPorts);

// at the end of this step, if any failed then bail out.
if (Log.HasLoggedErrors)
{
if (int.TryParse(port.ItemSpec, out int portNumber)
&& port.GetMetadata("Type") is { } portType
&& Enum.TryParse<PortType>(portType, out var parsedPortType))
{
image.ExposePort(portNumber, parsedPortType);
}
return false;
}

if (OutputRegistry.StartsWith("docker://"))
if (IsDockerPush)
{
try
{
Expand Down Expand Up @@ -172,7 +219,14 @@ public override bool Execute()

if (BuildEngine != null)
{
Log.LogMessage(MessageImportance.High, "Pushed container '{0}:{1}' to registry '{2}'", ImageName, ImageTag, OutputRegistry);
if (IsDockerPush)
{
Log.LogMessage(MessageImportance.High, "Pushed container '{0}:{1}' to local Docker daemon", ImageName, ImageTag);
}
else
{
Log.LogMessage(MessageImportance.High, "Pushed container '{0}:{1}' to registry '{2}'", ImageName, ImageTag, OutputRegistry);
}
}

return !Log.HasLoggedErrors;
Expand Down
4 changes: 2 additions & 2 deletions Microsoft.NET.Build.Containers/Image.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,9 @@ private static HashSet<Port> ReadPortsFromConfig(JsonNode inputConfig)
{
if (property.Key is { } propertyName
&& property.Value is JsonObject propertyValue
&& ContainerHelpers.TryParsePort(propertyName, out var port))
&& ContainerHelpers.TryParsePort(propertyName) is { success: true } result)
{
ports.Add(port);
ports.Add(result.port!);
}
}
return ports;
Expand Down
28 changes: 16 additions & 12 deletions Test.Microsoft.NET.Build.Containers/ContainerHelpersTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,21 +77,25 @@ public void IsValidImageTag_InvalidLength()
}

[TestMethod]
[DataRow("80/tcp", true, 80, PortType.tcp)]
[DataRow("80", true, 80, PortType.tcp)]
[DataRow("125/dup", false, 125, PortType.tcp)]
[DataRow("invalidNumber", false, null, null)]
[DataRow("80/unknowntype", false, null, null)]
public void CanParsePort(string input, bool shouldParse, int? expectedPortNumber, PortType? expectedType) {
var parseSuccess = ContainerHelpers.TryParsePort(input, out var parsedPort);
Assert.AreEqual(shouldParse, parseSuccess, $"Should have parsed {input} into a port");
[DataRow("80/tcp", true, 80, PortType.tcp, null)]
[DataRow("80", true, 80, PortType.tcp, null)]
[DataRow("125/dup", false, 125, PortType.tcp, ContainerHelpers.ParsePortError.InvalidPortType)]
[DataRow("invalidNumber", false, null, null, ContainerHelpers.ParsePortError.InvalidPortNumber)]
[DataRow("welp/unknowntype", false, null, null, (ContainerHelpers.ParsePortError)3)]
[DataRow("a/b/c", false, null, null, ContainerHelpers.ParsePortError.UnknownPortFormat)]
[DataRow("/tcp", false, null, null, ContainerHelpers.ParsePortError.MissingPortNumber)]
public void CanParsePort(string input, bool shouldParse, int? expectedPortNumber, PortType? expectedType, ContainerHelpers.ParsePortError? expectedError) {
var parseSuccess = ContainerHelpers.TryParsePort(input);
Assert.AreEqual<bool>(shouldParse, parseSuccess.success, $"{(shouldParse ? "Should" : "Shouldn't")} have parsed {input} into a port");
if (!shouldParse) {
Assert.IsNull(parsedPort);
Assert.IsNull(parseSuccess.port);
Assert.IsNotNull(parseSuccess.parseErrors);
Assert.AreEqual(expectedError, parseSuccess.parseErrors);
}
if (shouldParse) {
Assert.IsNotNull(parsedPort);
Assert.AreEqual(parsedPort.number, expectedPortNumber);
Assert.AreEqual(parsedPort.type, expectedType);
Assert.IsNotNull(parseSuccess.port);
Assert.AreEqual(parseSuccess.port.number, expectedPortNumber);
Assert.AreEqual(parseSuccess.port.type, expectedType);
}
}
}

0 comments on commit 1229a38

Please sign in to comment.