Skip to content

Add reading environment from Docker image#771

Merged
mergify[bot] merged 4 commits intonektos:masterfrom
catthehacker:cat/fix/read-image-spec
Sep 26, 2021
Merged

Add reading environment from Docker image#771
mergify[bot] merged 4 commits intonektos:masterfrom
catthehacker:cat/fix/read-image-spec

Conversation

@catthehacker
Copy link
Copy Markdown
Member

@catthehacker catthehacker commented Aug 9, 2021

This change will add reading image definition and merging it's environment variables with our current env in style of:

  • missing envvar, creates new one
  • existing envvar is overwritten by our env
  • PATH envvar is merged, container PATH is last to not overwrite any tools installed during workflow

It's not required for running our main container but it will be probably more intuitive if it will work like standard docker and this change is required for nested containers started in workflow

Fixes #757

@catthehacker catthehacker requested a review from a team as a code owner August 9, 2021 18:16
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 9, 2021

Codecov Report

Merging #771 (93dc252) into master (0f04942) will increase coverage by 5.35%.
The diff coverage is 62.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #771      +/-   ##
==========================================
+ Coverage   49.27%   54.63%   +5.35%     
==========================================
  Files          23       23              
  Lines        2401     3725    +1324     
==========================================
+ Hits         1183     2035     +852     
- Misses       1090     1536     +446     
- Partials      128      154      +26     
Impacted Files Coverage Δ
pkg/container/docker_build.go 0.00% <0.00%> (ø)
pkg/container/docker_run.go 6.67% <21.35%> (+4.75%) ⬆️
pkg/common/git.go 49.82% <31.81%> (-9.97%) ⬇️
pkg/model/planner.go 49.78% <42.50%> (+16.70%) ⬆️
pkg/model/workflow.go 52.14% <63.63%> (+26.42%) ⬆️
pkg/container/docker_pull.go 36.36% <64.00%> (+18.18%) ⬆️
pkg/runner/runner.go 74.46% <70.37%> (-2.01%) ⬇️
pkg/runner/expression.go 85.71% <72.09%> (-0.93%) ⬇️
pkg/runner/step_context.go 79.48% <79.05%> (+10.53%) ⬆️
pkg/runner/run_context.go 81.85% <91.08%> (+5.44%) ⬆️
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d552e6...93dc252. Read the comment docs.

@catthehacker catthehacker force-pushed the cat/fix/read-image-spec branch from d5a8b8e to c648bb9 Compare August 11, 2021 06:04
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Aug 11, 2021

@catthehacker this pull request has failed checks 🛠

@mergify mergify Bot added the needs-work Extra attention is needed label Aug 11, 2021
@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Aug 13, 2021

@catthehacker this pull request has failed checks 🛠

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Aug 24, 2021

@catthehacker this pull request has failed checks 🛠

@mergify mergify Bot removed the needs-work Extra attention is needed label Aug 27, 2021
@joonas
Copy link
Copy Markdown

joonas commented Sep 16, 2021

Glad to see this PR, I just recently came across an instance myself where the golang:1.17 container sets up a PATH variable, which unfortunately currently gets overridden in act unless an explicit PATH environment variable is set.

In turn, this results in the go not being available on the path for steps to use as it would be when running in GitHub Actions.

After spelunking a bit into the actions/runner codebase, I found that they are parsing it out from the docker inspect --format "{{range .Config.Env}}{{println .}}{{end}}" ... output.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Environment variables defined in the container are not evaluatable

3 participants