Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use StepHost when evaluating inputs to actions #1762

Merged
merged 15 commits into from
Apr 11, 2022

Conversation

nikola-jokic
Copy link
Contributor

@nikola-jokic nikola-jokic commented Mar 17, 2022

Closes #716

@nikola-jokic nikola-jokic requested a review from a team as a code owner March 17, 2022 11:16
thboop
thboop previously approved these changes Mar 21, 2022
Copy link
Collaborator

@thboop thboop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thboop thboop dismissed their stale review March 21, 2022 13:53

Let me double check how we convert these for other variables, it this not getting converted for some reason?

@ChristopherHX
Copy link
Contributor

it this not getting converted for some reason?

You are only converting the environment variables, no single expressions contexts has ever been translated to a container path.
Even paths of custom environment variables get converted
e.g.
The following prints host path

- run: echo '${{github.workspace}}'
  shell: bash

The following two prints the container path

- run: echo  $WP
  env:
    WP: ${{github.workspace}}
  shell: bash
- run: echo  $GITHUB_WORKSPACE
  shell: bash

GITHUB_ACTION_PATH already holds the container path, but the composite docs suggest to use ${{ github.action_path }} in a run step to call a bash script, which leads to problems.

@thboop
Copy link
Collaborator

thboop commented Mar 21, 2022

So, this does solve the issue for this specific context variable, however this problem exists for ALL path based context variables. For example consider the composite action:

  using: "composite"
  steps:
    - run: printenv
      shell: bash
    - run: |
        echo ${{github.action_path}}
        echo ${{github.event_path}}
        echo ${{github.workspace}}
      shell: bash

The env's will get translated, the echo statements will not. What we should really do is in Actionsrunner where we evaluate step inputs, use the step host to modify the context

            // TODO: Translate expression values into container expression values and pass that in here.
            // TODO: Temp create another dictionary for expression values and feed it in here, we need to map the path based containers to push it in.
            var inputs = templateEvaluator.EvaluateStepInputs(Action.Inputs, ExecutionContext.ExpressionValues, ExecutionContext.ExpressionFunctions);

@thboop thboop changed the title composite action github.action_path set based on the StepHost Use StepHost when evaluating inputs to actions Mar 23, 2022
@TingluoHuang
Copy link
Member

Can we update the PR description to include an output of dumping github and runner context with your changes? want to make sure all values are expected. 🙇

@nikola-jokic
Copy link
Contributor Author

@TingluoHuang of course! I will fix all of the suggestions as soon as I can 👍

Copy link
Contributor

@fhammerl fhammerl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@fhammerl fhammerl enabled auto-merge (squash) April 11, 2022 12:37
@fhammerl fhammerl merged commit c8cb600 into actions:main Apr 11, 2022
@nikola-jokic nikola-jokic deleted the nikola-jokic/bug/1221 branch April 11, 2022 12:48
@faubion-hbo
Copy link

Hi,
i'm running on the latest github runner (2.290.1) that should have this fix in it, but am still seeing a difference between the outputs of $GITHUB_ACTION_PATH and ${{ github.action_path }} when running a job within a container where ${{ github.action_path }} doesn't map to the right directory. i.e.

${{ github.action_path }} == /home/runner/work/_actions/<org>/<action>/<tag>
$GITHUB_ACTION_PATH == /__w/_actions/<org>/<action>/<tag>

where the container image is being pulled from ECR

@fhammerl
Copy link
Contributor

@faubion-hbo We recognised this as a sensitive change and rolling it out gradually.
Thanks for reporting the issue here, you can expect the fix to land in a few days.

@AllanOricil
Copy link

AllanOricil commented Apr 3, 2023

does not work. I had to change echo "${{ github.action_path }}" >> $GITHUB_PATH to echo "$GITHUB_ACTION_PATH" >> $GITHUB_PATH

philss pushed a commit to philss/rustler-precompiled-action that referenced this pull request Nov 3, 2023
Put the script path using both the env var, and the context value
for the "github.action_path". This should make the containers
see the script.

Reference:
actions/runner#1762 (comment)
@zachspar
Copy link

zachspar commented Jun 6, 2024

The problem still exists on self-hosted runners (actions-runner-controller) using defined container value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

${{ github.action_path }} makes no sense when action is run inside self-hosted custom container.
8 participants