Explicitly define what is allowed for a composite action

[ethanchewy]

In this PR, we explicitly define what is allowed for our composite action feature.

For each composite run step, we explicitly remove support for if, timeout-minutes, and continue-on-error. We'll support, id, shell, run, working-directory, and env all of which are optional except for run.

We don't want the defaults to flow to the composite action. Here is proof of them not flowing: https://github.com/ethanchewy/testing-actions/runs/895343022?check_suite_focus=true

Here is an example of the step level working-directory and env working: https://github.com/ethanchewy/testing-actions/runs/896147997?check_suite_focus=true

Shell requirement: https://github.com/ethanchewy/testing-actions/actions/runs/184525896

Example outputs demo working with explicit parameters: https://github.com/ethanchewy/testing-actions/runs/915495023?check_suite_focus=true

ID Context Expressions Not Allowed: https://github.com/ethanchewy/testing-actions/actions/runs/184536224, https://github.com/ethanchewy/testing-actions/actions/runs/184534064

Shell expression not allowed: https://github.com/ethanchewy/testing-actions/runs/915600890

[ethanchewy]

Tested if the defaults shell and working-dir values override the composite action steps' shell and working-dir. They do not override the steps inside of a composite action's shell or working-dir. This is want we want because we want the workflow to have no influence over the composite action in terms of defaults.

Why does this already work?

We don't check the ExecutionContext.Global.JobDefaults in ScriptHandler.cs unless the ExecutionContext.ScopeName is null for both the shell and working-dir:

• working-dir: https://github.com/actions/runner/blob/main/src/Runner.Worker/Handlers/ScriptHandler.cs#L152-L162
• shell: https://github.com/actions/runner/blob/main/src/Runner.Worker/Handlers/ScriptHandler.cs#L168-L179

cc @ericsciple

[ethanchewy]

Originally had planned to just null the ExectionContext.Global.JobDefaults for each composite action step ]'s ExecutionContext but this would have nullified all .Global.JobDefaults since we share the same pointer to Global for every instance of ExecutionContext.

So, I think my isComposite solution is better suited: aeae15d

[ethanchewy]

Reverting changes - if we change the run defaults logic in the future, we'll change it then.

[ethanchewy]

example of if not being allowed: https://github.com/ethanchewy/testing-actions/runs/895611092?check_suite_focus=true

(all others should not work too like "needs", etc.)

[ethanchewy]

Step level shell and working-directory not working in a composite action since the shell and working-directory are supposed to be mapped into the ActionStep.Inputs when we convert the step in the pipeline template converter. But for some reason it's not mapped when we process it in the Composite Action Handler.

The ActionRunner tries to retrieve these values from the ActionDefinitionData Inputs and then sets the handler's Inputs to this inputs dictionary.

Then, the ScriptHandler tries to get this working-directory and shell values from this handlerData.Inputs.

To fix this, I'm trying to figure out how to add the working-directory and shell values to the definition.Inputs dictionary. Currently debugging...

[ethanchewy]

This issue stems from the ConvertToSteps function in the pipeline template converter losing specificity. It originally returned a list of Steps which doesn't include the .Inputs attribute which is a dictionary that will contain working-directory and shell. Since we return it as a list of Steps and cast it back to a list of ActionSteps, we "lose" this .Inputs attribute.

[ethanchewy]

Thought there was a bug with setting the working-dir and shell but it works as intended (it was a bug with my test :/). Reverting back to original code.

[ethanchewy]

Ready for review.

[ethanchewy]

(leaving this unmerged till @ericsciple has the chance to look at it since this is a really important PR)

[ericsciple]

do we have validation on the server that at least one step is required?

[ethanchewy]

@ericsciple For server side,

• If there are no "steps" token used, it handles it: https://github.com/ethanchewy/testing-actions/runs/898870893?check_suite_focus=true.
• If there are no steps within the "steps" token, the user will get an unexpected value from the server side: https://github.com/ethanchewy/testing-actions/runs/898862578?check_suite_focus=true

[ethanchewy]

if the attribute needs a context, they'll have access (see "string-steps-context")

[ethanchewy]

Ready for final review. (tested all edge cases [see description for links]).