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

Environment variables not loading on travis for external PRs #491

Closed
kaustubh-nair opened this issue Apr 5, 2019 · 18 comments
Closed

Environment variables not loading on travis for external PRs #491

kaustubh-nair opened this issue Apr 5, 2019 · 18 comments

Comments

@kaustubh-nair
Copy link
Member

Travis does not load environment variables due to security concerns, (https://docs.travis-ci.com/user/pull-requests#pull-requests-and-security-restrictions)
This is a valid security concern because anyone can send a PR that prints and exposes the env variables to the logs.
It is however a setback for us since it restrains us from sending coverage reports to Coveralls/Codeclimate. Because of this we can't see coverage change on external prs. (This is important for us because we might want to encourage contributors to send prs with sufficient tests so that test coverage does not decrease)
For example compare #467 and #462

The same issue exists for plots2 as well. Notice how a repo can't be found to send report to coveralls in this build. https://travis-ci.org/publiclab/plots2/builds/512970864#L3498
Currently only push builds are sending coverage reports to coveralls.

I can't find a good solution for this that doesn't include making the environment variables public, which should be our last resort.

@kaustubh-nair
Copy link
Member Author

Someone please add the discussion label 😅

@jywarren
Copy link
Member

jywarren commented Apr 5, 2019

Done, also adding you as a collaborator you you can too! 👍

@kaustubh-nair
Copy link
Member Author

@icarito do you have any ideas about this?
Thanks

@alaxalves
Copy link
Member

alaxalves commented Apr 7, 2019

@kaustubh-nair If if have understood well the problem here I think that we could use Travis CLI to encrypt any kind of file we want(In our case a .env file) and then decrypt the file in travis.yml, export those needed variables and use them inside our build. Maybe something like I have set here:
https://raw.githubusercontent.com/falko-org/Falko-API/1f2bd28964a82ede6c9ea56084fab29589ffe8d3/.travis.yml

Note that in before_install: job I decrypt a previously encrypted private key in ~/.travis/alax-digitalocean-key.enc path. After that I use this key to auto deploy in a DO vps.

@kaustubh-nair
Copy link
Member Author

kaustubh-nair commented Apr 9, 2019

@alaxalves Travis documentation says encrypted files are not available for pull requests across forks https://docs.travis-ci.com/user/encrypting-files/
So your solution doesn't solve our problem

@kaustubh-nair
Copy link
Member Author

kaustubh-nair commented Apr 9, 2019

@jywarren @alaxalves I guess whatever solution we implement, the env variables will always be available be during the build, thus open to being accessed. So I don't really think there is any point in encrypting these. What do you think? Should we put the coveralls token in travis.yml?

@jywarren
Copy link
Member

jywarren commented Apr 9, 2019 via email

@kaustubh-nair
Copy link
Member Author

@jywarren No we can't do that. It's mentioned here that Travis does not load env variables from settings for external PRs. That's why coveralls is working for internal but not for external PRs

@alaxalves
Copy link
Member

@kaustubh-nair Isn't there a space in Mapknitter's project settings where we could set env variables, just like we have in Gitlab?

@kaustubh-nair
Copy link
Member Author

@alaxalves Hmm, I don't think Github has that feature. If I am right, only gitlab has that.

@jywarren
Copy link
Member

Hi all, sorry i hadn't had time to chime in here. Just to clarify, what makes a PR internal or external? #467 and #462 are both by project collaborators, no?

In any case, I'm OK with putting them in travis.yml, and we can see how that works. We can always revoke and revert. What should the format be? I can add to a PR with a travis.yml update...

@kaustubh-nair
Copy link
Member Author

Internal PRs are created from a Mapknitter branch and external ones are sent from a branch across a fork.
If you think it's okay to make the token public, I'll send a pr!
Cheers

@jywarren
Copy link
Member

jywarren commented Apr 14, 2019 via email

@sashadev-sky
Copy link
Member

@jywarren @alaxalves was this resolved with the switch to codecov(#543)? can it be closed?

@alaxalves
Copy link
Member

@jywarren @alaxalves was this resolved with the switch to codecov(#543)? can it be closed?

I think that f88fa70 already solved this but I might be mislead, @kaustubh-nair Can you answer this?

@kaustubh-nair
Copy link
Member Author

kaustubh-nair commented May 20, 2019

This isn't really resolved, the point of this issue was that it was a prerequisite to getting Coveralls working.
But since we've got Codecov set up, this issue is not important anymore
So we can close this.

@kaustubh-nair
Copy link
Member Author

Or we can keep it open, in case it is required for something else in the future?

@kaustubh-nair kaustubh-nair reopened this May 20, 2019
@sashadev-sky
Copy link
Member

@kaustubh-nair @alaxalves if it was specific to coveralls lets just close it. It's likely in the future it'd just be easier to start clean and the information in the issue is still available when its closed. Im all for less clutter

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

Successfully merging a pull request may close this issue.

4 participants