Add notice annotation level and support more annotation fields (#1175)

* Add Notice Command

* Add Feature Flag For Enhanced Annotations

* Fix tests

* Add validation for columns and lines

* Fix order to match service

* Remove console.write

* Make Validation Better

* Cleanup

* Handle empty/whitespace strings

* Add more validation for line/column ranges

* Make Validation Debug, Not Throw

* Change casing to 🐫 from 🐍

* Give notice a well known tag

* Cleanup

* Sanitize invalid commands rather than fail
This commit is contained in:
Luke Tomlinson
2021-07-13 11:38:16 -04:00
committed by GitHub
parent 6443fe8c97
commit fff31e11c5
8 changed files with 208 additions and 2 deletions

View File

@@ -51,6 +51,7 @@ namespace GitHub.Runner.Common
Add<T>(extensions, "GitHub.Runner.Worker.RemoveMatcherCommandExtension, Runner.Worker"); Add<T>(extensions, "GitHub.Runner.Worker.RemoveMatcherCommandExtension, Runner.Worker");
Add<T>(extensions, "GitHub.Runner.Worker.WarningCommandExtension, Runner.Worker"); Add<T>(extensions, "GitHub.Runner.Worker.WarningCommandExtension, Runner.Worker");
Add<T>(extensions, "GitHub.Runner.Worker.ErrorCommandExtension, Runner.Worker"); Add<T>(extensions, "GitHub.Runner.Worker.ErrorCommandExtension, Runner.Worker");
Add<T>(extensions, "GitHub.Runner.Worker.NoticeCommandExtension, Runner.Worker");
Add<T>(extensions, "GitHub.Runner.Worker.DebugCommandExtension, Runner.Worker"); Add<T>(extensions, "GitHub.Runner.Worker.DebugCommandExtension, Runner.Worker");
Add<T>(extensions, "GitHub.Runner.Worker.GroupCommandExtension, Runner.Worker"); Add<T>(extensions, "GitHub.Runner.Worker.GroupCommandExtension, Runner.Worker");
Add<T>(extensions, "GitHub.Runner.Worker.EndGroupCommandExtension, Runner.Worker"); Add<T>(extensions, "GitHub.Runner.Worker.EndGroupCommandExtension, Runner.Worker");

View File

@@ -544,6 +544,11 @@ namespace GitHub.Runner.Common
timelineRecord.WarningCount = rec.WarningCount; timelineRecord.WarningCount = rec.WarningCount;
} }
if (rec.NoticeCount != null && rec.NoticeCount > 0)
{
timelineRecord.NoticeCount = rec.NoticeCount;
}
if (rec.Issues.Count > 0) if (rec.Issues.Count > 0)
{ {
timelineRecord.Issues.Clear(); timelineRecord.Issues.Clear();

View File

@@ -75,6 +75,12 @@ namespace GitHub.Runner.Worker
return false; return false;
} }
if (!ActionCommandManager.EnhancedAnnotationsEnabled(context) && actionCommand.Command == "notice")
{
context.Debug($"Enhanced Annotations not enabled on the server: 'notice' command will not be processed.");
return false;
}
// Serialize order // Serialize order
lock (_commandSerializeLock) lock (_commandSerializeLock)
{ {
@@ -141,6 +147,10 @@ namespace GitHub.Runner.Worker
return true; return true;
} }
internal static bool EnhancedAnnotationsEnabled(IExecutionContext context) {
return context.Global.Variables.GetBoolean("DistributedTask.EnhancedAnnotations") ?? false;
}
} }
public interface IActionCommandExtension : IExtension public interface IActionCommandExtension : IExtension
@@ -498,6 +508,13 @@ namespace GitHub.Runner.Worker
public override string Command => "error"; public override string Command => "error";
} }
public sealed class NoticeCommandExtension : IssueCommandExtension
{
public override IssueType Type => IssueType.Notice;
public override string Command => "notice";
}
public abstract class IssueCommandExtension : RunnerService, IActionCommandExtension public abstract class IssueCommandExtension : RunnerService, IActionCommandExtension
{ {
public abstract IssueType Type { get; } public abstract IssueType Type { get; }
@@ -512,6 +529,11 @@ namespace GitHub.Runner.Worker
command.Properties.TryGetValue(IssueCommandProperties.Line, out string line); command.Properties.TryGetValue(IssueCommandProperties.Line, out string line);
command.Properties.TryGetValue(IssueCommandProperties.Column, out string column); command.Properties.TryGetValue(IssueCommandProperties.Column, out string column);
if (!ActionCommandManager.EnhancedAnnotationsEnabled(context))
{
context.Debug("Enhanced Annotations not enabled on the server. The 'title', 'end_line', and 'end_column' fields are unsupported.");
}
Issue issue = new Issue() Issue issue = new Issue()
{ {
Category = "General", Category = "General",
@@ -563,13 +585,73 @@ namespace GitHub.Runner.Worker
context.AddIssue(issue); context.AddIssue(issue);
} }
public static void ValidateLinesAndColumns(ActionCommand command, IExecutionContext context)
{
command.Properties.TryGetValue(IssueCommandProperties.Line, out string line);
command.Properties.TryGetValue(IssueCommandProperties.EndLine, out string endLine);
command.Properties.TryGetValue(IssueCommandProperties.Column, out string column);
command.Properties.TryGetValue(IssueCommandProperties.EndColumn, out string endColumn);
var hasStartLine = int.TryParse(line, out int lineNumber);
var hasEndLine = int.TryParse(endLine, out int endLineNumber);
var hasStartColumn = int.TryParse(column, out int columnNumber);
var hasEndColumn = int.TryParse(endColumn, out int endColumnNumber);
var hasColumn = hasStartColumn || hasEndColumn;
if (hasEndLine && !hasStartLine)
{
context.Debug($"Invalid {command.Command} command value. '{IssueCommandProperties.EndLine}' can only be set if '{IssueCommandProperties.Line}' is provided");
command.Properties[IssueCommandProperties.Line] = endLine;
hasStartLine = true;
line = endLine;
}
if (hasEndColumn && !hasStartColumn)
{
context.Debug($"Invalid {command.Command} command value. '{IssueCommandProperties.EndColumn}' can only be set if '{IssueCommandProperties.Column}' is provided");
command.Properties[IssueCommandProperties.Column] = endColumn;
hasStartColumn = true;
column = endColumn;
}
if (!hasStartLine && hasColumn)
{
context.Debug($"Invalid {command.Command} command value. '{IssueCommandProperties.Column}' and '{IssueCommandProperties.EndColumn}' can only be set if '{IssueCommandProperties.Line}' value is provided.");
command.Properties.Remove(IssueCommandProperties.Column);
command.Properties.Remove(IssueCommandProperties.EndColumn);
}
if (hasEndLine && line != endLine && hasColumn)
{
context.Debug($"Invalid {command.Command} command value. '{IssueCommandProperties.Column}' and '{IssueCommandProperties.EndColumn}' cannot be set if '{IssueCommandProperties.Line}' and '{IssueCommandProperties.EndLine}' are different values.");
command.Properties.Remove(IssueCommandProperties.Column);
command.Properties.Remove(IssueCommandProperties.EndColumn);
}
if (hasStartLine && hasEndLine && endLineNumber < lineNumber)
{
context.Debug($"Invalid {command.Command} command value. '{IssueCommandProperties.EndLine}' cannot be less than '{IssueCommandProperties.Line}'.");
command.Properties.Remove(IssueCommandProperties.Line);
command.Properties.Remove(IssueCommandProperties.EndLine);
}
if (hasStartColumn && hasEndColumn && endColumnNumber < columnNumber)
{
context.Debug($"Invalid {command.Command} command value. '{IssueCommandProperties.EndColumn}' cannot be less than '{IssueCommandProperties.Column}'.");
command.Properties.Remove(IssueCommandProperties.Column);
command.Properties.Remove(IssueCommandProperties.EndColumn);
}
}
private static class IssueCommandProperties private static class IssueCommandProperties
{ {
public const String File = "file"; public const String File = "file";
public const String Line = "line"; public const String Line = "line";
public const String EndLine = "endLine";
public const String Column = "col"; public const String Column = "col";
public const String EndColumn = "endColumn";
public const String Title = "title";
} }
} }
public sealed class GroupCommandExtension : GroupingCommandExtension public sealed class GroupCommandExtension : GroupingCommandExtension

View File

@@ -517,6 +517,24 @@ namespace GitHub.Runner.Worker
_record.WarningCount++; _record.WarningCount++;
} }
else if (issue.Type == IssueType.Notice)
{
// tracking line number for each issue in log file
// log UI use this to navigate from issue to log
if (!string.IsNullOrEmpty(logMessage))
{
long logLineNumber = Write(WellKnownTags.Notice, logMessage);
issue.Data["logFileLineNumber"] = logLineNumber.ToString();
}
if (_record.NoticeCount < _maxIssueCount)
{
_record.Issues.Add(issue);
}
_record.NoticeCount++;
}
_jobServerQueue.QueueTimelineRecordUpdate(_mainTimelineId, _record); _jobServerQueue.QueueTimelineRecordUpdate(_mainTimelineId, _record);
} }
@@ -841,6 +859,7 @@ namespace GitHub.Runner.Worker
_record.State = TimelineRecordState.Pending; _record.State = TimelineRecordState.Pending;
_record.ErrorCount = 0; _record.ErrorCount = 0;
_record.WarningCount = 0; _record.WarningCount = 0;
_record.NoticeCount = 0;
if (parentTimelineRecordId != null && parentTimelineRecordId.Value != Guid.Empty) if (parentTimelineRecordId != null && parentTimelineRecordId.Value != Guid.Empty)
{ {
@@ -1012,6 +1031,7 @@ namespace GitHub.Runner.Worker
public static readonly string Command = "##[command]"; public static readonly string Command = "##[command]";
public static readonly string Error = "##[error]"; public static readonly string Error = "##[error]";
public static readonly string Warning = "##[warning]"; public static readonly string Warning = "##[warning]";
public static readonly string Notice = "##[notice]";
public static readonly string Debug = "##[debug]"; public static readonly string Debug = "##[debug]";
} }
} }

View File

@@ -210,6 +210,10 @@ namespace GitHub.Runner.Worker.Handlers
{ {
issueType = DTWebApi.IssueType.Warning; issueType = DTWebApi.IssueType.Warning;
} }
else if (string.Equals(match.Severity, "notice", StringComparison.OrdinalIgnoreCase))
{
issueType = DTWebApi.IssueType.Notice;
}
else else
{ {
_executionContext.Debug($"Skipped logging an issue for the matched line because the severity '{match.Severity}' is not supported."); _executionContext.Debug($"Skipped logging an issue for the matched line because the severity '{match.Severity}' is not supported.");

View File

@@ -9,6 +9,9 @@ namespace GitHub.DistributedTask.WebApi
Error = 1, Error = 1,
[EnumMember] [EnumMember]
Warning = 2 Warning = 2,
[EnumMember]
Notice = 3
} }
} }

View File

@@ -38,6 +38,7 @@ namespace GitHub.DistributedTask.WebApi
this.RefName = recordToBeCloned.RefName; this.RefName = recordToBeCloned.RefName;
this.ErrorCount = recordToBeCloned.ErrorCount; this.ErrorCount = recordToBeCloned.ErrorCount;
this.WarningCount = recordToBeCloned.WarningCount; this.WarningCount = recordToBeCloned.WarningCount;
this.NoticeCount = recordToBeCloned.NoticeCount;
this.AgentPlatform = recordToBeCloned.AgentPlatform; this.AgentPlatform = recordToBeCloned.AgentPlatform;
if (recordToBeCloned.Log != null) if (recordToBeCloned.Log != null)
@@ -222,6 +223,13 @@ namespace GitHub.DistributedTask.WebApi
set; set;
} }
[DataMember(Order = 55)]
public Int32? NoticeCount
{
get;
set;
}
public List<Issue> Issues public List<Issue> Issues
{ {
get get

View File

@@ -188,6 +188,84 @@ namespace GitHub.Runner.Common.Tests.Worker
} }
} }
[Fact]
[Trait("Level", "L0")]
[Trait("Category", "Worker")]
public void IssueCommandInvalidColumns()
{
using (TestHostContext hc = CreateTestContext())
{
_ec.Setup(x => x.Write(It.IsAny<string>(), It.IsAny<string>()))
.Returns((string tag, string line) =>
{
hc.GetTrace().Info($"{tag} {line}");
return 1;
});
var registeredCommands = new HashSet<string>(new string[1]{ "warning" });
ActionCommand command;
// Columns when lines are different
ActionCommand.TryParseV2("::warning line=1,endLine=2,col=1,endColumn=2::this is a warning", registeredCommands, out command);
Assert.Equal("1", command.Properties["col"]);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.False(command.Properties.ContainsKey("col"));
// No lines with columns
ActionCommand.TryParseV2("::warning col=1,endColumn=2::this is a warning", registeredCommands, out command);
Assert.Equal("1", command.Properties["col"]);
Assert.Equal("2", command.Properties["endColumn"]);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.False(command.Properties.ContainsKey("col"));
Assert.False(command.Properties.ContainsKey("endColumn"));
// No line with endLine
ActionCommand.TryParseV2("::warning endLine=1::this is a warning", registeredCommands, out command);
Assert.Equal("1", command.Properties["endLine"]);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.Equal(command.Properties["endLine"], command.Properties["line"]);
// No column with endColumn
ActionCommand.TryParseV2("::warning line=1,endColumn=2::this is a warning", registeredCommands, out command);
Assert.Equal("2", command.Properties["endColumn"]);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.Equal(command.Properties["endColumn"], command.Properties["col"]);
// Empty Strings
ActionCommand.TryParseV2("::warning line=,endLine=3::this is a warning", registeredCommands, out command);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.Equal(command.Properties["line"], command.Properties["endLine"]);
// Nonsensical line values
ActionCommand.TryParseV2("::warning line=4,endLine=3::this is a warning", registeredCommands, out command);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.False(command.Properties.ContainsKey("line"));
Assert.False(command.Properties.ContainsKey("endLine"));
/// Nonsensical column values
ActionCommand.TryParseV2("::warning line=1,endLine=1,col=3,endColumn=2::this is a warning", registeredCommands, out command);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.False(command.Properties.ContainsKey("col"));
Assert.False(command.Properties.ContainsKey("endColumn"));
// Valid
ActionCommand.TryParseV2("::warning line=1,endLine=1,col=1,endColumn=2::this is a warning", registeredCommands, out command);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.Equal("1", command.Properties["line"]);
Assert.Equal("1", command.Properties["endLine"]);
Assert.Equal("1", command.Properties["col"]);
Assert.Equal("2", command.Properties["endColumn"]);
// Backwards compatibility
ActionCommand.TryParseV2("::warning line=1,col=1,file=test.txt::this is a warning", registeredCommands, out command);
IssueCommandExtension.ValidateLinesAndColumns(command, _ec.Object);
Assert.Equal("1", command.Properties["line"]);
Assert.False(command.Properties.ContainsKey("endLine"));
Assert.Equal("1", command.Properties["col"]);
Assert.False(command.Properties.ContainsKey("endColumn"));
}
}
[Fact] [Fact]
[Trait("Level", "L0")] [Trait("Level", "L0")]
[Trait("Category", "Worker")] [Trait("Category", "Worker")]
@@ -268,6 +346,7 @@ namespace GitHub.Runner.Common.Tests.Worker
new EchoCommandExtension(), new EchoCommandExtension(),
new InternalPluginSetRepoPathCommandExtension(), new InternalPluginSetRepoPathCommandExtension(),
new SetEnvCommandExtension(), new SetEnvCommandExtension(),
new WarningCommandExtension(),
}; };
foreach (var command in commands) foreach (var command in commands)
{ {
@@ -285,6 +364,10 @@ namespace GitHub.Runner.Common.Tests.Worker
_ec = new Mock<IExecutionContext>(); _ec = new Mock<IExecutionContext>();
_ec.SetupAllProperties(); _ec.SetupAllProperties();
_ec.Setup(x => x.Global).Returns(new GlobalContext()); _ec.Setup(x => x.Global).Returns(new GlobalContext());
_ec.Object.Global.Variables = new Variables(
hostContext,
new Dictionary<string, VariableValue>()
);
// Command manager // Command manager
_commandManager = new ActionCommandManager(); _commandManager = new ActionCommandManager();