refactor: remove composite action runcontext workaround by KnisterPeter · Pull Request #1085 · nektos/act

KnisterPeter · 2022-03-29T13:10:58Z

The RunContext is cloned to execute a composite action with all its
steps in a similar context. This required some workaround, since
the command handler has kept a reference to the original RunContext.

This is solved now, by replacing the docker LogWriter with a proper
scoped LogWriter.

This prepares for a simpler setup of composite actions to be able
to create and re-create the composite RunContext for pre/main/post
action steps.

github-actions · 2022-03-29T13:13:07Z

MegaLinter status: ✅ SUCCESS

Descriptor	Linter	Files	Errors	Elapsed time
✅ CREDENTIALS	secretlint	yes	no	0.97s
✅ EDITORCONFIG	editorconfig-checker	10	0	0.6s
✅ GIT	git_diff	yes	no	0.01s

See errors details in artifact MegaLinter reports on CI Job page
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

KnisterPeter · 2022-03-29T15:49:25Z

Ops, this has to wait for #1083

codecov · 2022-03-29T15:50:55Z

Codecov Report

Merging #1085 (b3cd099) into master (4f8da0a) will increase coverage by 2.53%.
The diff coverage is 78.74%.

@@            Coverage Diff             @@
##           master    #1085      +/-   ##
==========================================
+ Coverage   57.50%   60.04%   +2.53%     
==========================================
  Files          32       39       +7     
  Lines        4594     4990     +396     
==========================================
+ Hits         2642     2996     +354     
- Misses       1729     1746      +17     
- Partials      223      248      +25

Impacted Files	Coverage Δ
pkg/model/action.go	`0.00% <ø> (ø)`
pkg/model/github_context.go	`79.51% <ø> (ø)`
pkg/model/planner.go	`50.73% <ø> (+0.32%)`	⬆️
pkg/model/workflow.go	`50.91% <ø> (ø)`
pkg/container/docker_run.go	`5.04% <2.08%> (-0.51%)`	⬇️
pkg/common/git.go	`49.31% <28.57%> (-0.52%)`	⬇️
pkg/container/file_collector.go	`44.85% <44.85%> (ø)`
pkg/runner/runner.go	`73.72% <73.68%> (-2.75%)`	⬇️
pkg/exprparser/interpreter.go	`75.29% <75.00%> (+1.89%)`	⬆️
pkg/runner/logger.go	`69.56% <75.47%> (+4.13%)`	⬆️
... and 20 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

ChristopherHX · 2022-03-29T18:05:12Z

We need more tests for nodejs and docker actions before this PR can be merged, or we see heavy regressions.
I think the following is broken now:

GITHUB_ACTION_PATH for nodejs and docker actions
${{ github.action_path }} for ~~nodejs and~~ docker actions

I only have added tests for GITHUB_ACTION_PATH in composite actions, which seem to be still working if I can trust my tests.

KnisterPeter · 2022-03-29T18:23:05Z

I will add tests tomorrow

KnisterPeter · 2022-03-30T07:41:21Z

According to the docs the GITHUB_ACTION_PATH is only supported in composite actions.

I'll add tests for GITHUB_ACTION and GITHUB_ACTION_REPOSITORY

ChristopherHX · 2022-03-30T07:57:05Z

Ok, GITHUB_ACTION_PATH is only for composite.
What about ${{github.action_ref}} for docker actions?
GITHUB_ACTION_REF seems not to be documented, but provided.

KnisterPeter · 2022-03-30T08:00:52Z

I will add tests for GITHUB_ACTION_REF as well

KnisterPeter · 2022-03-30T11:34:47Z

@nektos/act-maintainers I'd consider this refactoring fine now. It already fixes regressions related to the env-var in actions.
This changes makes creating the RunContext for composite actions simpler. I will further refactor if this is merged, to have the pre and post steps available before and after the action is executed.

ChristopherHX

Hmm I have no idea why I don't see any log output of run steps inside composite actions.
Due to this I couldn't test if ::add-mask:: is still working.

ChristopherHX · 2022-04-05T19:03:54Z

I have issues with ::add-mask:: inside composite actions. If you echo the secret value after the command your secret get exposed to the log, while in v0.2.26 it is replaced by ***.

This make me think we need tests which tests secret masking.

KnisterPeter · 2022-04-05T19:11:00Z

Right. I will add a test for that tomorrow

Since we create a new line writer, we need to log the raw_output as well. Otherwise no output will be available from the log-writer

To allow masking of values from composite actions, we need to use a custom job logger with a reference to the masked values for the composite run context.

To not introduce another new logger while still be able to use the masking from the composite action, we add the masks to the go context. To leverage that context, we also add the context to the log entries where the valueMasker then could get the actual mask values. With this way to 'inject' the masked values into the logger, we do - keep the logger - keep the coloring - stay away from inconsistencies due to parallel jobs

This one should have never removed :-)

This adds a test and fix for the parent environment. It should be inherited by the composite environment.

We must not expose the secrets to composite actions, but the `github.token` is available inside composite actions. To provide this we store the token in the config and create it in the GithubContext from there. The token can be used with `github.token` but is not available as `secrets.GITHUB_TOKEN`. This implements the same behavior as on GitHub. Co-authored-by: Björn Brauer <bjoern.brauer@new-work.se> Co-authored-by: Marcus Noll <markus.noll@new-work.se>

mergify · 2022-04-25T11:15:12Z

@KnisterPeter this pull request has failed checks 🛠

mergify · 2022-04-25T11:20:48Z

@KnisterPeter this pull request has failed checks 🛠

KnisterPeter · 2022-04-27T12:57:34Z

@cplee This is waiting for a review/merge.

cplee

works for me locally ✅

KnisterPeter self-assigned this Mar 29, 2022

pull-request-size Bot added the size/L label Mar 29, 2022

KnisterPeter force-pushed the remove-composite-runcontext-workaround branch 4 times, most recently from ea70f9b to 5883f7e Compare March 29, 2022 15:33

KnisterPeter marked this pull request as ready for review March 29, 2022 15:42

KnisterPeter requested a review from a team as a code owner March 29, 2022 15:42

KnisterPeter marked this pull request as draft March 29, 2022 15:49

KnisterPeter removed the request for review from a team March 29, 2022 15:49

Base automatically changed from simplify-action-interfaces to master March 29, 2022 17:42

KnisterPeter force-pushed the remove-composite-runcontext-workaround branch from 5883f7e to 3017fa4 Compare March 30, 2022 07:24

KnisterPeter force-pushed the remove-composite-runcontext-workaround branch from 2940e0c to 02628aa Compare March 30, 2022 09:54

KnisterPeter marked this pull request as ready for review March 30, 2022 11:31

mergify Bot requested a review from a team March 30, 2022 11:31

ChristopherHX reviewed Mar 30, 2022

View reviewed changes

Comment thread pkg/runner/action.go Outdated

mergify Bot requested a review from a team March 30, 2022 13:14

ZauberNerd previously approved these changes Apr 4, 2022

View reviewed changes

KnisterPeter dismissed ZauberNerd’s stale review via a711610 April 6, 2022 11:41

KnisterPeter and others added 14 commits April 25, 2022 10:58

test: test remote docker and js actions

0f07904

fix: merge github context into env when read and setup

2f37c49

refacotr: simplify composite context setup

36d8ac3

test: use a map matcher to test input setup

ae15be2

fix: restore composite log output

d76c84d

Since we create a new line writer, we need to log the raw_output as well. Otherwise no output will be available from the log-writer

fix: add RunContext JobName to fill GITHUB_JOBNAME

056d840

test: use nektos/act-test-actions

b41d466

fix: allow masking values in composite actions

ff586d8

To allow masking of values from composite actions, we need to use a custom job logger with a reference to the masked values for the composite run context.

fix: re-add removed color increase

07823fd

This one should have never removed :-)

fix: add missing ExtraPath attribute

324037c

fix: merge run context env into composite run context env

bc4730c

This adds a test and fix for the parent environment. It should be inherited by the composite environment.

test: add missing test case

a88c876

ZauberNerd dismissed ChristopherHX’s stale review via fd76d7e April 25, 2022 11:12

ZauberNerd force-pushed the remove-composite-runcontext-workaround branch from 04822f8 to fd76d7e Compare April 25, 2022 11:12

mergify Bot added the needs-work Extra attention is needed label Apr 25, 2022

fixup! fix: allow masking values in composite actions

b1220cd

ZauberNerd force-pushed the remove-composite-runcontext-workaround branch from fd76d7e to b1220cd Compare April 25, 2022 11:18

style: use tabs instead of spaces to fix linter errors

0007215

mergify Bot removed the needs-work Extra attention is needed label Apr 25, 2022

mergify Bot requested a review from a team April 25, 2022 12:03

catthehacker added the stale-exempt Exempt from stale label Apr 30, 2022

cplee approved these changes May 11, 2022

View reviewed changes

Merge branch 'master' into remove-composite-runcontext-workaround

b3cd099

mergify Bot merged commit e360811 into master May 11, 2022

mergify Bot deleted the remove-composite-runcontext-workaround branch May 11, 2022 19:06

Uh oh!

Conversation

KnisterPeter commented Mar 29, 2022

Uh oh!

github-actions Bot commented Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

MegaLinter status: ✅ SUCCESS

Uh oh!

KnisterPeter commented Mar 29, 2022

Uh oh!

codecov Bot commented Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ChristopherHX commented Mar 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

KnisterPeter commented Mar 29, 2022

Uh oh!

KnisterPeter commented Mar 30, 2022

Uh oh!

ChristopherHX commented Mar 30, 2022

Uh oh!

KnisterPeter commented Mar 30, 2022

Uh oh!

KnisterPeter commented Mar 30, 2022

Uh oh!

ChristopherHX left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ChristopherHX commented Apr 5, 2022

Uh oh!

KnisterPeter commented Apr 5, 2022

Uh oh!

mergify Bot commented Apr 25, 2022

Uh oh!

mergify Bot commented Apr 25, 2022

Uh oh!

KnisterPeter commented Apr 27, 2022

Uh oh!

cplee left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions Bot commented Mar 29, 2022 •

edited

Loading

codecov Bot commented Mar 29, 2022 •

edited

Loading

ChristopherHX commented Mar 29, 2022 •

edited

Loading