From 4d7988e0ad30584f7c61d8b669a6babf04d0e486 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Thu, 10 Jun 2021 17:39:40 -0400 Subject: [PATCH 1/6] draft of composite adr --- docs/adrs/0000-composite-actions.md | 97 +++++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 docs/adrs/0000-composite-actions.md diff --git a/docs/adrs/0000-composite-actions.md b/docs/adrs/0000-composite-actions.md new file mode 100644 index 00000000000..1c26faf859b --- /dev/null +++ b/docs/adrs/0000-composite-actions.md @@ -0,0 +1,97 @@ +**Date**: 2021-06-10 + +**Status**: Accepted + +## Context + +We released [composite run steps](https://github.com/actions/runner/pull/554) last year which started our journey of reusing steps across different workflow files. To continue that journey, we want to expand composite run steps into composite actions. + +We want to support the `uses` steps from workflows in composite actions, including: + - Container actions + - javascript actions + - and even other Composite actions (up to a limit of course!) + - The pre and post steps these actions can generate + +## Guiding Principles + +- Composite Actions should function as a single step or action, no matter how many steps it is composed of or how many levels of recursion it has +- A workflow author should not need to understand the inner workings of a composite action in order to use it +- Composite actions should leverage inputs to get values they need, they will not have full access to the `context` objects. The secrets context will **not** be available to composite actions, users will need to pass these values in as an input. +- Other Actions should **just work** inside a composite action, without any code changes + +## Decisions + +### Composite Recursion Limit + +We will start with supporting a recursion limit of `3` +That means you can have +``` +- Composite Action + - Nested Composite Action + - Deep Nested Composite Action + - uses action step + - runs action step +``` +- We are free to bump this limit in the future, the code will be written to just require updating a variable. If the graph evaluates beyond the recursion limit, the job will fail in the pre-job phase (The `Set up job` step). +- A composite actions interface is its inputs and outputs, nothing else is carried over when invoking recursively. + +### Pre/Post Steps in nested Actions + +- We do not plan on adding the ability to configure a customizable pre or post step for composite actions at this time. However, we will execute the pre and post steps of any actions referenced in a composite action. +- Composite actions will generate a single pre-step and post-step for the entire composite action, even if there are multiple pre-steps and post-steps in the referenced actions. + - These steps will execute following the same ordering rules we have today, first to run has their pre step run first and their post step run last. + - For example, if you had a composite action with two pre steps and two posts steps: + + ``` + - uses: action1 + - uses: composite1 + - uses: action2 + ``` + + The order of execution would be: + + ``` + - prestep-action1 + - prestep-composite1 + - prestep-composite1-first-action-referenced + - prestep-composite1-second-action-referenced + - prestep-action2 + - the job steps + - poststep-action2 + - poststep-composite1 + - poststep-composite1-the-second-action-referenced + - poststep-composite1-first-action-referenced + - poststep-action1 + ``` + +#### Set-state + +- While the composite action has an individual combined pre/post action, the `set-state` command will not be shared. +- If the `set-state` command is used during a composite step, only the action that originally called `set-state` will have access to the env variable during the post run step. + - This prevents multible actions that set the same state from interfering with the execution of another action's post step. + +### Resolve Action Endpoint changes + +- The resolve actions endpoint will now validate policy to ensure that the given workflow run has access to download that action. + - It will return a value indicated security was checked. Older server versions would lack this boolean. For composite actions that try to resolve an action, if the server does not respond with this value, we will fail the job as we could violate access policy if we allowed the composite action to continue. + - Oder GHES/GHAE customers with newer runners will be locked out of composite uses steps until they upgrade their instance. + +### If, continue-on-error, timeout-minutes - Not being considered at this time + +- `if`, `continue-on-error`, `timeout-minutes` could be supported in composite run/uses steps. These values were not originally supported in our composite run steps implementation. + - Browsing the community forums and runner repo, there hasn't been a lot of noise asking for these features, so we will hold off on them. +- These values passed as input into the composite action will **not** be carried over as input into the individual steps the composite action runs. + +### Defaults - Not being considered at this time + +- In actions, we have the idea of [defaults](https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#defaultsrun) , which allow you to specify a shell and working directory in one location, rather then on each step. +- However, `shell` is currently required in composite run steps + - In regular run steps, it is optional, and defaults to a different value based on the OS. +- We could make it optional in composite run steps, and follow the trend of workflow run steps and add the `defaults` functionality + - However, encouraging action authors to explicitly state the `shell` forces them to do the right thing regarding considering cross platform support. + - We can change this in the future as needed, but for now its largely a convenience feature that adds no new functionality, let's wait on more user feedback before we consider this further. + +## Consequences + +- Workflows are now more reusable across multiple workflow files +- Composite actions implement most of the existing workflow run steps, with room to expand these in the future \ No newline at end of file From 6fd6b16af2824375090b80ba378beb5c8df7888e Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Fri, 11 Jun 2021 17:38:55 -0400 Subject: [PATCH 2/6] adr feedback --- docs/adrs/0000-composite-actions.md | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/docs/adrs/0000-composite-actions.md b/docs/adrs/0000-composite-actions.md index 1c26faf859b..c1f717e9dbb 100644 --- a/docs/adrs/0000-composite-actions.md +++ b/docs/adrs/0000-composite-actions.md @@ -23,15 +23,7 @@ We want to support the `uses` steps from workflows in composite actions, includi ### Composite Recursion Limit -We will start with supporting a recursion limit of `3` -That means you can have -``` -- Composite Action - - Nested Composite Action - - Deep Nested Composite Action - - uses action step - - runs action step -``` +- We will start with supporting a recursion limit of `10` composite actions deep - We are free to bump this limit in the future, the code will be written to just require updating a variable. If the graph evaluates beyond the recursion limit, the job will fail in the pre-job phase (The `Set up job` step). - A composite actions interface is its inputs and outputs, nothing else is carried over when invoking recursively. @@ -68,7 +60,7 @@ That means you can have - While the composite action has an individual combined pre/post action, the `set-state` command will not be shared. - If the `set-state` command is used during a composite step, only the action that originally called `set-state` will have access to the env variable during the post run step. - - This prevents multible actions that set the same state from interfering with the execution of another action's post step. + - This prevents multiple actions that set the same state from interfering with the execution of another action's post step. ### Resolve Action Endpoint changes @@ -76,6 +68,11 @@ That means you can have - It will return a value indicated security was checked. Older server versions would lack this boolean. For composite actions that try to resolve an action, if the server does not respond with this value, we will fail the job as we could violate access policy if we allowed the composite action to continue. - Oder GHES/GHAE customers with newer runners will be locked out of composite uses steps until they upgrade their instance. +### Local actions +- TODO describe this more +- We should support pre and post actions for local actions + + ### If, continue-on-error, timeout-minutes - Not being considered at this time - `if`, `continue-on-error`, `timeout-minutes` could be supported in composite run/uses steps. These values were not originally supported in our composite run steps implementation. @@ -87,11 +84,10 @@ That means you can have - In actions, we have the idea of [defaults](https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#defaultsrun) , which allow you to specify a shell and working directory in one location, rather then on each step. - However, `shell` is currently required in composite run steps - In regular run steps, it is optional, and defaults to a different value based on the OS. -- We could make it optional in composite run steps, and follow the trend of workflow run steps and add the `defaults` functionality - - However, encouraging action authors to explicitly state the `shell` forces them to do the right thing regarding considering cross platform support. - - We can change this in the future as needed, but for now its largely a convenience feature that adds no new functionality, let's wait on more user feedback before we consider this further. +- We want to prioritize the right experience for the consumer, and make the action author continue to explicitly set these values. We can consider improving this experience in the future. ## Consequences - Workflows are now more reusable across multiple workflow files -- Composite actions implement most of the existing workflow run steps, with room to expand these in the future \ No newline at end of file +- Composite actions implement most of the existing workflow run steps, with room to expand these in the future +- Feature flags will control this rollout \ No newline at end of file From 4a98aabdf49e846bfff013a1d3bc8b50dac38302 Mon Sep 17 00:00:00 2001 From: Thomas Boop Date: Mon, 14 Jun 2021 11:22:21 -0400 Subject: [PATCH 3/6] minor updates --- docs/adrs/0000-composite-actions.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/adrs/0000-composite-actions.md b/docs/adrs/0000-composite-actions.md index c1f717e9dbb..d0a51dbd816 100644 --- a/docs/adrs/0000-composite-actions.md +++ b/docs/adrs/0000-composite-actions.md @@ -8,13 +8,14 @@ We released [composite run steps](https://github.com/actions/runner/pull/554) la We want to support the `uses` steps from workflows in composite actions, including: - Container actions - - javascript actions - - and even other Composite actions (up to a limit of course!) + - Javascript actions + - Other Composite actions (up to a limit of course!) - The pre and post steps these actions can generate ## Guiding Principles - Composite Actions should function as a single step or action, no matter how many steps it is composed of or how many levels of recursion it has + - In the future we may add a configurable option to make this no longer the case - A workflow author should not need to understand the inner workings of a composite action in order to use it - Composite actions should leverage inputs to get values they need, they will not have full access to the `context` objects. The secrets context will **not** be available to composite actions, users will need to pass these values in as an input. - Other Actions should **just work** inside a composite action, without any code changes From a70380ff029041f8e814c156c09ab39221a8d28b Mon Sep 17 00:00:00 2001 From: Thomas Boop <52323235+thboop@users.noreply.github.com> Date: Mon, 26 Jul 2021 20:40:07 -0400 Subject: [PATCH 4/6] Minor update --- docs/adrs/0000-composite-actions.md | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/docs/adrs/0000-composite-actions.md b/docs/adrs/0000-composite-actions.md index d0a51dbd816..d1d7eed73fc 100644 --- a/docs/adrs/0000-composite-actions.md +++ b/docs/adrs/0000-composite-actions.md @@ -67,12 +67,11 @@ We want to support the `uses` steps from workflows in composite actions, includi - The resolve actions endpoint will now validate policy to ensure that the given workflow run has access to download that action. - It will return a value indicated security was checked. Older server versions would lack this boolean. For composite actions that try to resolve an action, if the server does not respond with this value, we will fail the job as we could violate access policy if we allowed the composite action to continue. - - Oder GHES/GHAE customers with newer runners will be locked out of composite uses steps until they upgrade their instance. + - Older GHES/GHAE customers with newer runners will be locked out of composite uses steps until they upgrade their instance. ### Local actions -- TODO describe this more -- We should support pre and post actions for local actions - +- Local actions will expand the tree, perform policy checks, and download actions Just in Time when the step is running. +- Like current local actions, we will not support presteps. If an action is running local, by the time we know that, the time to run presteps have already passed. ### If, continue-on-error, timeout-minutes - Not being considered at this time @@ -91,4 +90,4 @@ We want to support the `uses` steps from workflows in composite actions, includi - Workflows are now more reusable across multiple workflow files - Composite actions implement most of the existing workflow run steps, with room to expand these in the future -- Feature flags will control this rollout \ No newline at end of file +- Feature flags will control this rollout From bc9f54f40d7cf9aaeaa15434f303ae83a1c78b43 Mon Sep 17 00:00:00 2001 From: Thomas Boop <52323235+thboop@users.noreply.github.com> Date: Mon, 26 Jul 2021 20:49:52 -0400 Subject: [PATCH 5/6] Rename 0000-composite-actions.md to 1144-composite-actions.md --- .../adrs/{0000-composite-actions.md => 1144-composite-actions.md} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename docs/adrs/{0000-composite-actions.md => 1144-composite-actions.md} (100%) diff --git a/docs/adrs/0000-composite-actions.md b/docs/adrs/1144-composite-actions.md similarity index 100% rename from docs/adrs/0000-composite-actions.md rename to docs/adrs/1144-composite-actions.md From a80f9c4374fc453c06e29cab3d7ab5d2d2c8e37a Mon Sep 17 00:00:00 2001 From: Thomas Boop <52323235+thboop@users.noreply.github.com> Date: Tue, 27 Jul 2021 09:07:21 -0400 Subject: [PATCH 6/6] Update 1144-composite-actions.md --- docs/adrs/1144-composite-actions.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/adrs/1144-composite-actions.md b/docs/adrs/1144-composite-actions.md index d1d7eed73fc..2e1c30461ab 100644 --- a/docs/adrs/1144-composite-actions.md +++ b/docs/adrs/1144-composite-actions.md @@ -66,8 +66,7 @@ We want to support the `uses` steps from workflows in composite actions, includi ### Resolve Action Endpoint changes - The resolve actions endpoint will now validate policy to ensure that the given workflow run has access to download that action. - - It will return a value indicated security was checked. Older server versions would lack this boolean. For composite actions that try to resolve an action, if the server does not respond with this value, we will fail the job as we could violate access policy if we allowed the composite action to continue. - - Older GHES/GHAE customers with newer runners will be locked out of composite uses steps until they upgrade their instance. + - Older GHES/GHAE customers with newer runners will be locked out of composite uses steps until they upgrade their instance. ### Local actions - Local actions will expand the tree, perform policy checks, and download actions Just in Time when the step is running.