-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Set runner environment in context and env #2518
Set runner environment in context and env #2518
Conversation
ed1209c
to
8a1090d
Compare
Extract runner_environment from the global context and expose in the `github.runner` context and env as `RUNNER_ENVIRONMENT`. Signed-off-by: Philip Harrison <philip@mailharrison.com>
8a1090d
to
6c40884
Compare
@@ -126,6 +126,11 @@ public async Task<TaskResult> RunAsync(AgentJobRequestMessage message, Cancellat | |||
_runnerSettings = HostContext.GetService<IConfigurationStore>().GetSettings(); | |||
jobContext.SetRunnerContext("name", _runnerSettings.AgentName); | |||
|
|||
if (jobContext.Global.Variables.TryGetValue(WellKnownDistributedTaskVariables.RunnerEnvironment, out var runnerEnvironment)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TingluoHuang presuming this tryGetValue should make it safe to deploy this change independently of the backend change?
Also had a poke around and doesn't look like there's any allow list for what goes in the RUNNER_{KEY}
ENV values: https://github.com/actions/runner/blob/main/src/Runner.Worker/RunnerContext.cs
Looks like the GitHub context does have an allow list: https://github.com/actions/runner/blob/main/src/Runner.Worker/GitHubContext.cs#L9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have an allow list before for the RUNNER_*
since before this change all RUNNER_*
are set by the runner, while most GITHUB_*
are coming from the service.
@@ -126,6 +126,11 @@ public async Task<TaskResult> RunAsync(AgentJobRequestMessage message, Cancellat | |||
_runnerSettings = HostContext.GetService<IConfigurationStore>().GetSettings(); | |||
jobContext.SetRunnerContext("name", _runnerSettings.AgentName); | |||
|
|||
if (jobContext.Global.Variables.TryGetValue(WellKnownDistributedTaskVariables.RunnerEnvironment, out var runnerEnvironment)) | |||
{ | |||
jobContext.SetRunnerContext("environment", runnerEnvironment); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TingluoHuang another q: Looking at these docs: https://docs.github.com/en/actions/learn-github-actions/variables#detecting-the-operating-system - it seems you can access the runner
context before the job runs, e.g. if: runner.os == 'Windows'
. From our sync yesterday, is this a problem for us? Presuming it should be ok to set it here as everything else is set here too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if: runner.os == 'Windows'
in the example in under steps (not before job land on the runner), which will be on the runner for sure.
Problem
runner_environment
was added to the OIDC id token claims but it's currently not exposed in the runner context or ENV.We would like to access the runner env value from the system ENV on the runner in order to include it in the provenance statement generated in the npm CLI when constructing the
builder.id
: https://github.com/npm/cli/blob/latest/workspaces/libnpmpublish/lib/provenance.js#L7