search

mapzen / eraser-map

Privacy-focused mapping application for Android
GNU General Public License v3.0
75 stars 24 forks source link

Fork pull request builds fail on Circle CI; unable to access AWS key #797

Closed ecgreb closed 7 years ago

ecgreb commented 7 years ago

Development APKs are deployed from Circle CI to http://android.mapzen.com/erasermap/ as a convenience to facilitate manual testing of dev builds. AWS credentials are stored in secure environment variables configured via the Circle CI web interface.

Builds from a forked pull request on Circle CI are unable to access the AWS key stored in a secure environment variable.

https://circleci.com/gh/mapzen/eraser-map/1859

Building the same commits from a non-fork branch in this repository fixes the issue.

https://circleci.com/gh/mapzen/eraser-map/1864

The Circle CI docs are somewhat unclear on this matter.

Non-sensitive environment variables for your project can be set in circle.yml; values configured via the web UI are stored encrypted at rest and injected into the build environment during regular builds.

The variables configured via the circle.yml will be available in the fork PR builds (as they have access to the same circle.yml file); the values configured through the UI will not be visible to the fork PRs.

https://circleci.com/docs/fork-pr-builds/

My original understanding of the docs was that "not visible" means the output of secure environment variables would be obscured in the console output. However it seems "not visible" actually means "not available" and is causing the APK upload to fail.

msmollin commented 7 years ago

This does make some sense, given they're one of the recommended ways to store deployment credentials for the app store and web servers.

Maybe for forked PRs, would it be good enough to leave the artifacts in Circle and just download them from the build page to install?

ecgreb commented 7 years ago

I agree we should probably just skip this step for fork PRs and make sure we copy the APK into the correct folder on Circle CI so it is downloadable.

https://circleci.com/docs/build-artifacts/

ecgreb commented 7 years ago

Maybe we want to start doing this for all dev builds and only deploy master builds and releases to android.mapzen.com?

msmollin commented 7 years ago

Depends on how you want to treat long-running feature branches. I would be fine with that approach though.

msmollin commented 7 years ago

Looks like Circle supports regex branch names, so you could cover the "long running feature branch" case by having a build step that executes for anything prefixed feature

sarahsnow1 commented 7 years ago

Per a discussion with dev ops, we turned off fork PR builds for EM