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

Error when changing JDK to something other than the default #28

Closed
nateha1984 opened this issue Dec 16, 2021 · 11 comments
Closed

Error when changing JDK to something other than the default #28

nateha1984 opened this issue Dec 16, 2021 · 11 comments
Labels
help wanted Extra attention is needed wontfix This will not be worked on

Comments

@nateha1984
Copy link

If JAVA_HOME is set on the runner to something other than the default, the action fails with:

ERROR: JAVA_HOME is set to an invalid directory: /opt/hostedtoolcache/Java_Adopt_jdk/11.0.11-9/x64

Removing the Set up JDK 11 step allows the workflow to proceed without issue.

Sample workflow causing the error:

            on:
              push:
            
            jobs:
              get-version:
                runs-on: ubuntu-20.04
                steps:
                  - uses: actions/checkout@v2
            
                  - name: Set up JDK 11
                    uses: actions/setup-java@v2
                    with:
                      java-version: '11'
                      distribution: 'adopt'
            
                  - name: Get version from
                    uses: madhead/read-java-properties@c2ad203dff7a56a06aab9e3586bef5c1e36409c2
                    id: version
                    with:
                      file: gradle.properties
                      property: version
@SettingDust
Copy link

SettingDust commented Dec 17, 2021

Due to https://github.com/madhead/tyzenhaus/blob/master/.github/workflows/master.yml#L18-L26. Use this action first
Or you can do some hack things like this https://github.com/SettingDust/GriefDefender/blob/a1a0300eb2e3eef2a7d3ce2506fa42bce3bc17d7/.github/workflows/gradle.yml#L48-L49. Note, you needn't use my fork(And it's deleted). The method above is more effective

@madhead
Copy link
Owner

madhead commented Dec 17, 2021

That's actually very interesting. This action is a Docker action and should be completely independent of the environment. I'll take a look, though.

@nateha1984
Copy link
Author

I looked into it more and it appears every user-set env variable in the runner is passed to Docker when it starts, so if JAVA_HOME is set at all it causes the failure. I even tried saving the original JAVA_HOME value so I could set it back, but that caused the same problem.

Ex Docker output that breaks from the Github debug: /usr/bin/docker run --name ghcriomadheadreadjavapropertieslatest_c12d86 --label 6a6825 --workdir /github/workspace --rm -e JAVA_HOME -e INPUT_FILE ...

Ex Docker output that succeeds: /usr/bin/docker run --name ghcriomadheadreadjavapropertieslatest_0d93b4 --label 6a6825 --workdir /github/workspace --rm -e INPUT_FILE -e INPUT_PROPERTY ...

I was eventually able to get this working as long as I ran this action first, then the setup java action as @SettingDust mentioned. Still seems odd though, but not sure if the variables passed to the container are in your control, I couldn't find anything about how to control that.

@madhead
Copy link
Owner

madhead commented Dec 18, 2021

That was my first guess, that variables set in the workflow / worker leak into the container. Of course, the container has another JAVA_HOME (Java is installed into another directory). And the shell script (generated by Gradle Application Plugin) reads that property when it tries to execute the code of this action. Thus, it fails.

However, this behavior seems super contr-intuitive to me and is shouldn't work that way! Probably it should be reported to GitHub Actions team or what. If the assumptions above are true, then there is not many things actions authors could do.

@madhead
Copy link
Owner

madhead commented Dec 19, 2021

Ok, environment definitely leaks into Docker actions :(

I've made a minimal reproducible example here. Take a look at the actions execution log: https://github.com/madhead/actions-env-leak/runs/4575382091?check_suite_focus=true#step:5:54

Notice that after actions/setup-java JAVA_HOME environment variable "leaks" into the container.

How do you think, @nateha1984, @SettingDust, maybe this one should be reported to the Actions team?

@SettingDust
Copy link

I can't imagine how can they "fix" this. It's not a bug. For the users want to modify the env of container. So, I think we need to ask someone for the best practice. Notice in README is enough.

@madhead
Copy link
Owner

madhead commented Dec 19, 2021

Well, actually I didn't expect that. This "problem" is not specific to my action. It's just a matter of bad luck. I expect Docker container actions to be fully isolated of the workflow. With the variables leaking inside, you could potentially break any action that relies on environment variables. At least, I'd expect this be explicit: if one wants to overwrite the variable, she should be explicit. It seems like Docker container actions just inherit the whole environment…

madhead added a commit that referenced this issue Dec 19, 2021
Add a few notes about `JAVA_HOME` leakage
madhead added a commit that referenced this issue Dec 19, 2021
@madhead madhead added help wanted Extra attention is needed wontfix This will not be worked on labels Dec 19, 2021
@madhead
Copy link
Owner

madhead commented Dec 19, 2021

I've added a few words in README about this issue. I've also created a topic on GitHub Community forum, but it was closed by a dang moderator bot as a spam :(

@madhead
Copy link
Owner

madhead commented Jan 14, 2022

This is what a support told me on my request to clarify the behavior:

Thank you for reaching out to GitHub Support and sorry to hear you've experienced this issue.

I've reached out to the Actions team and they confirmed that most (if not all) environment variables from the runner are passed through to a container when run to allow access to things like secrets and other custom environment variables.

I'm afraid there are currently no plans to change the behaviour around environment variables in these Actions - sorry about that.

We really appreciate feedback on how we can make GitHub even better and the best way to report any feature requests is directly to us through our feedback form: https://support.github.com/contact/feedback

Our roadmap is now publicly visible, so we recommend that you keep an eye on the GitHub Blog and roadmap for the latest announcements about new features.

@madhead
Copy link
Owner

madhead commented Jan 14, 2022

So, I won't be able to do anything here...

@madhead madhead closed this as completed Jan 14, 2022
@BrycensRanch
Copy link

rewrote this action in typescript like any other normal action so docker and other java stuff wouldn't effect anything.

100% compatible and fixes this issue since madhead cant do anything about it, godspeed

https://github.com/BrycensRanch/read-properties-action

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants