diff --git a/src/Runner.Common/Constants.cs b/src/Runner.Common/Constants.cs index 583958981a9..003a5a929c1 100644 --- a/src/Runner.Common/Constants.cs +++ b/src/Runner.Common/Constants.cs @@ -176,6 +176,7 @@ public static class Features public static readonly string SetOrchestrationIdEnvForActions = "actions_set_orchestration_id_env_for_actions"; public static readonly string SendJobLevelAnnotations = "actions_send_job_level_annotations"; public static readonly string EmitCompositeMarkers = "actions_runner_emit_composite_markers"; + public static readonly string ValidateRequiredActionInputs = "actions_runner_validate_required_action_inputs"; } // Node version migration related constants diff --git a/src/Runner.Worker/ActionManager.cs b/src/Runner.Worker/ActionManager.cs index 38c2ab8b320..9e0baf2a2e9 100644 --- a/src/Runner.Worker/ActionManager.cs +++ b/src/Runner.Worker/ActionManager.cs @@ -1267,6 +1267,8 @@ public sealed class ActionDefinitionData public ActionExecutionData Execution { get; set; } public Dictionary Deprecated { get; set; } + + public HashSet RequiredInputs { get; set; } } public enum ActionExecutionType diff --git a/src/Runner.Worker/ActionManifestManager.cs b/src/Runner.Worker/ActionManifestManager.cs index a70592381c7..2c7c34b7b7b 100644 --- a/src/Runner.Worker/ActionManifestManager.cs +++ b/src/Runner.Worker/ActionManifestManager.cs @@ -538,6 +538,15 @@ private void ConvertInputs( hasDefault = true; actionDefinition.Inputs.Add(inputName, metadata.Value); } + else if (string.Equals(metadataName, "required", StringComparison.OrdinalIgnoreCase)) + { + var requiredValue = metadata.Value.AssertBoolean("input required"); + if (requiredValue.Value) + { + actionDefinition.RequiredInputs ??= new HashSet(StringComparer.OrdinalIgnoreCase); + actionDefinition.RequiredInputs.Add(inputName.Value); + } + } else if (string.Equals(metadataName, "deprecationMessage", StringComparison.OrdinalIgnoreCase)) { if (actionDefinition.Deprecated == null) @@ -568,6 +577,8 @@ public sealed class ActionDefinitionDataNew public ActionExecutionData Execution { get; set; } public Dictionary Deprecated { get; set; } + + public HashSet RequiredInputs { get; set; } } public sealed class ContainerActionExecutionDataNew : ActionExecutionData diff --git a/src/Runner.Worker/ActionManifestManagerWrapper.cs b/src/Runner.Worker/ActionManifestManagerWrapper.cs index 6d893fd8252..6f8e45f3277 100644 --- a/src/Runner.Worker/ActionManifestManagerWrapper.cs +++ b/src/Runner.Worker/ActionManifestManagerWrapper.cs @@ -118,7 +118,8 @@ private ActionDefinitionData ConvertToLegacyActionDefinitionData(ActionDefinitio Description = newData.Description, Inputs = ConvertToLegacyToken(newData.Inputs), Deprecated = newData.Deprecated, - Execution = ConvertToLegacyExecution(newData.Execution) + Execution = ConvertToLegacyExecution(newData.Execution), + RequiredInputs = newData.RequiredInputs }; } diff --git a/src/Runner.Worker/ActionRunner.cs b/src/Runner.Worker/ActionRunner.cs index 4c84c19424a..2fef9d9bc5a 100644 --- a/src/Runner.Worker/ActionRunner.cs +++ b/src/Runner.Worker/ActionRunner.cs @@ -235,7 +235,18 @@ Action.Reference is Pipelines.RepositoryPathReference repoAction && ExecutionContext.Warning($"Unexpected input(s) '{string.Join("', '", unexpectedInputs)}', valid inputs are ['{string.Join("', '", validInputs)}']"); } } - + // Validate required inputs when feature flag is enabled + if ((ExecutionContext.Global.Variables.GetBoolean(Constants.Runner.Features.ValidateRequiredActionInputs) ?? false) + && definition.Data?.RequiredInputs != null) + { + foreach (var requiredKey in definition.Data.RequiredInputs) + { + if (!inputs.TryGetValue(requiredKey, out var requiredValue) || string.IsNullOrEmpty(requiredValue)) + { + ExecutionContext.Error($"Input '{requiredKey}' is required for action '{definition.Data.Name}' but was not provided."); + } + } + } // Load the action environment. ExecutionContext.Debug("Loading env"); var environment = new Dictionary(VarUtil.EnvironmentVariableKeyComparer); diff --git a/src/Test/L0/Worker/ActionRunnerL0.cs b/src/Test/L0/Worker/ActionRunnerL0.cs index 2c1bd46bc28..1de9fd1eac7 100644 --- a/src/Test/L0/Worker/ActionRunnerL0.cs +++ b/src/Test/L0/Worker/ActionRunnerL0.cs @@ -517,6 +517,118 @@ public async void SetGitHubContextActionRepoRef() _ec.Verify(x => x.SetGitHubContext("action_ref", null), Times.Once); } + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async void RequiredInputProvidedDoesNotError() + { + // Arrange + Setup(); + + // Enable the feature flag + _ec.Object.Global.Variables.Set(Constants.Runner.Features.ValidateRequiredActionInputs, "true"); + + // Set up action definition with a required input + var actionInputs = new MappingToken(null, null, null); + actionInputs.Add(new StringToken(null, null, null, "input1"), new StringToken(null, null, null, "input1")); + actionInputs.Add(new StringToken(null, null, null, "input2"), new StringToken(null, null, null, "")); + actionInputs.Add(new StringToken(null, null, null, "input3"), new StringToken(null, null, null, "github")); + var actionDefinition = new Definition() + { + Directory = _hc.GetDirectory(WellKnownDirectory.Work), + Data = new ActionDefinitionData() + { + Name = "TestAction", + Description = "TestAction", + Inputs = actionInputs, + Execution = new ScriptActionExecutionData(), + RequiredInputs = new HashSet(StringComparer.OrdinalIgnoreCase) { "input1" } + } + }; + _actionManager.Setup(x => x.LoadAction(It.IsAny(), It.IsAny())).Returns(actionDefinition); + + var actionId = Guid.NewGuid(); + var stepInputs = new MappingToken(null, null, null); + stepInputs.Add(new StringToken(null, null, null, "input1"), new StringToken(null, null, null, "provided-value")); + var action = new Pipelines.ActionStep() + { + Name = "action", + Id = actionId, + Reference = new Pipelines.RepositoryPathReference() + { + Name = "actions/test", + Ref = "v1" + }, + Inputs = stepInputs + }; + _actionRunner.Action = action; + + _handlerFactory.Setup(x => x.Create(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>(), It.IsAny>(), It.IsAny(), It.IsAny(), It.IsAny>())) + .Returns(new Mock().Object); + + // Act + await _actionRunner.RunAsync(); + + // Assert: no error should be raised for a required input that was provided + _ec.Verify(x => x.AddIssue(It.Is(s => s.Type == IssueType.Error && s.Message.Contains("input1")), It.IsAny()), Times.Never); + } + + [Fact] + [Trait("Level", "L0")] + [Trait("Category", "Worker")] + public async void RequiredInputMissingRaisesError() + { + // Arrange + Setup(); + + // Enable the feature flag + _ec.Object.Global.Variables.Set(Constants.Runner.Features.ValidateRequiredActionInputs, "true"); + + // Set up action definition with a required input + var actionInputs = new MappingToken(null, null, null); + actionInputs.Add(new StringToken(null, null, null, "input1"), new StringToken(null, null, null, "input1")); + actionInputs.Add(new StringToken(null, null, null, "input2"), new StringToken(null, null, null, "")); + actionInputs.Add(new StringToken(null, null, null, "input3"), new StringToken(null, null, null, "github")); + var actionDefinition = new Definition() + { + Directory = _hc.GetDirectory(WellKnownDirectory.Work), + Data = new ActionDefinitionData() + { + Name = "TestAction", + Description = "TestAction", + Inputs = actionInputs, + Execution = new ScriptActionExecutionData(), + RequiredInputs = new HashSet(StringComparer.OrdinalIgnoreCase) { "required-but-missing" } + } + }; + _actionManager.Setup(x => x.LoadAction(It.IsAny(), It.IsAny())).Returns(actionDefinition); + + var actionId = Guid.NewGuid(); + var stepInputs = new MappingToken(null, null, null); + stepInputs.Add(new StringToken(null, null, null, "input1"), new StringToken(null, null, null, "provided-value")); + var action = new Pipelines.ActionStep() + { + Name = "action", + Id = actionId, + Reference = new Pipelines.RepositoryPathReference() + { + Name = "actions/test", + Ref = "v1" + }, + Inputs = stepInputs + }; + _actionRunner.Action = action; + + _handlerFactory.Setup(x => x.Create(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>(), It.IsAny>(), It.IsAny(), It.IsAny(), It.IsAny>())) + .Returns(new Mock().Object); + + // Act + await _actionRunner.RunAsync(); + + // Assert: an error should be raised for the missing required input + _ec.Verify(x => x.AddIssue(It.Is(s => s.Type == IssueType.Error && s.Message.Contains("required-but-missing")), It.IsAny()), Times.Once); + } + private void Setup([CallerMemberName] string name = "") { _ecTokenSource?.Dispose();