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

feat: support config env expansion #2063

Merged
merged 4 commits into from
Dec 17, 2023

Conversation

raffis
Copy link
Contributor

@raffis raffis commented Oct 24, 2023

Current situation

Using an .actrc config it is not possible to utilize envs as there is no env expansion since its not invoked from a shell.

I'd like to have a config like this which is currently not working:

--detect-event
-s GITUB_TOKEN=$GITHUB_TOKEN
-s GH_ACTIONS=$GITHUB_TOKEN
--container-options --privileged --mount type=bind,source=/home/$USER/.docker-act,target=/home/runner/.docker

Proposal

Expand any envs while appending the caller args.

Signed-off-by: Raffael Sahli <raffael.sahli@doodle.com>
@raffis raffis requested a review from a team as a code owner October 24, 2023 12:23
Copy link
Contributor

@ChristopherHX ChristopherHX left a comment

Choose a reason for hiding this comment

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

I think this is useful, but another maintainer or the owner need to decide

Copy link

codecov bot commented Nov 12, 2023

Codecov Report

Attention: 872 lines in your changes are missing coverage. Please review.

Comparison is base (4989f44) 61.22% compared to head (9a7d7fc) 61.64%.
Report is 288 commits behind head on master.

Files Patch % Lines
pkg/artifactcache/handler.go 65.46% 102 Missing and 42 partials ⚠️
pkg/runner/run_context.go 73.37% 75 Missing and 19 partials ⚠️
pkg/runner/expression.go 55.17% 66 Missing and 12 partials ⚠️
pkg/runner/action_cache.go 50.74% 49 Missing and 17 partials ⚠️
pkg/container/docker_network.go 0.00% 56 Missing ⚠️
pkg/container/docker_run.go 2.00% 48 Missing and 1 partial ⚠️
pkg/model/workflow.go 43.37% 40 Missing and 7 partials ⚠️
pkg/common/outbound_ip.go 0.00% 44 Missing ⚠️
pkg/container/host_environment.go 0.00% 43 Missing ⚠️
pkg/artifactcache/storage.go 60.22% 24 Missing and 11 partials ⚠️
... and 25 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2063      +/-   ##
==========================================
+ Coverage   61.22%   61.64%   +0.42%     
==========================================
  Files          46       53       +7     
  Lines        7141     8808    +1667     
==========================================
+ Hits         4372     5430    +1058     
- Misses       2462     2948     +486     
- Partials      307      430     +123     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@wolfogre wolfogre left a comment

Choose a reason for hiding this comment

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

LGTM. I don't see any reason why users would want to keep the original string $XXX in args. If they do, they can use $$XXX.

@ChristopherHX
Copy link
Contributor

image
What happended here?
mergify pushed, then the pr owner pushed a few seconds later? Or is that a GitHub bug.

Copy link
Contributor

mergify bot commented Dec 17, 2023

@raffis this pull request has failed checks 🛠

@mergify mergify bot added needs-work Extra attention is needed and removed needs-work Extra attention is needed labels Dec 17, 2023
@mergify mergify bot merged commit 0806c8b into nektos:master Dec 17, 2023
10 checks passed
jmikedupont2 pushed a commit to meta-introspector/act that referenced this pull request Mar 10, 2024
Signed-off-by: Raffael Sahli <raffael.sahli@doodle.com>
Co-authored-by: ChristopherHX <christopher.homberger@web.de>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
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.

None yet

3 participants