docs icon indicating copy to clipboard operation
docs copied to clipboard

[ci] CircleCI Test skipping Danger due to access

Open janbrasna opened this issue 3 years ago • 8 comments

Missing DANGER_GITHUB_* tokens, Danger fails with:

"Could not set up API to Code Review site for Danger"

results of: bundle exec danger || echo "danger failed, moving on"

e. g. Test3130:

For your GitHub repo, you need to expose: DANGER_GITHUB_API_TOKEN, DANGER_GITHUB_BEARER_TOKEN
You may also need: DANGER_GITHUB_HOST, DANGER_GITHUB_API_BASE_URL, DANGER_OCTOKIT_VERIFY_SSL

Found these keys in your ENV: CIRCLE_WORKFLOW_JOB_ID, CIRCLE_PR_NUMBER, CIRCLE_PULL_REQUESTS, HOSTNAME, JAVA_HOME, SSH_AUTH_SOCK, CIRCLE_REPOSITORY_URL, CIRCLE_WORKING_DIRECTORY, CIRCLE_INTERNAL_CONFIG, CIRCLECI, RUBY_DOWNLOAD_SHA256, CI_PULL_REQUESTS, CIRCLE_PROJECT_REPONAME, CIRCLE_WORKFLOW_UPSTREAM_JOB_IDS, YARN_VERSION, RUBY_VERSION, PWD, CIRCLE_WORKFLOW_ID, BUNDLE_APP_CONFIG, RUBY_MAJOR, CIRCLE_PULL_REQUEST, CIRCLE_USERNAME, CIRCLE_BRANCH, HOME, LANG, CIRCLE_PROJECT_USERNAME, CIRCLE_BUILD_NUM, CA_CERTIFICATES_JAVA_VERSION, CIRCLE_NODE_TOTAL, BUNDLE_SILENCE_ROOT_WARNING, CIRCLE_SHA1, GEM_HOME, NO_PROXY, JAVA_DEBIAN_VERSION, CI_PULL_REQUEST, SHLVL, CIRCLE_BUILD_URL, BASH_ENV, CIRCLE_NODE_INDEX, CIRCLE_COMPARE_URL, CIRCLE_PREVIOUS_BUILD_NUM, CIRCLE_WORKFLOW_WORKSPACE_ID, CIRCLE_PR_USERNAME, CIRCLE_STAGE, CIRCLE_JOB, LC_ALL, CIRCLE_SHELL_ENV, XAR_VERSION, PATH, CIRCLE_INTERNAL_SCRATCH, CI, CIRCLE_INTERNAL_TASK_DATA, CIRCLE_PR_REPONAME, NODE_VERSION, DEBIAN_FRONTEND, JAVA_VERSION, _, BUNDLER_ORIG_BUNDLE_BIN_PATH, BUNDLER_ORIG_BUNDLE_GEMFILE, BUNDLER_ORIG_BUNDLER_VERSION, BUNDLER_ORIG_GEM_HOME, BUNDLER_ORIG_GEM_PATH, BUNDLER_ORIG_MANPATH, BUNDLER_ORIG_PATH, BUNDLER_ORIG_RB_USER_INSTALL, BUNDLER_ORIG_RUBYLIB, BUNDLER_ORIG_RUBYOPT, BUNDLE_BIN_PATH, BUNDLE_GEMFILE, BUNDLER_VERSION, RUBYOPT, RUBYLIB, MANPATH.

Failing the build, Danger cannot run without API access.
You can see more information at https://danger.systems/guides/getting_started.html
danger failed, moving on
CircleCI received exit code 0

janbrasna avatar Feb 24 '22 14:02 janbrasna

Just an observation. Not sure if anyone relies on it. I first noticed it in https://github.com/fastlane/docs/pull/1085#issuecomment-899497002 in what should've triggered a danger, but went silent. 🤷‍♂️

janbrasna avatar Feb 24 '22 14:02 janbrasna

Hey @janbrasna 👋

Interesting find. This was added here: https://github.com/fastlane/docs/pull/288 but I don't know the rationale behind it. It probably made sense back then, but it doesn't anymore. I'll invite @nafu to take a look at this and shed some light if he can 🙇

But I think we can safely move forward with the removal of that silent failure.

I checked and current master of this repo passes on Danger 👌

I'll go ahead and remove the fallback that causes the silent failure 😊

Nice spotting!

rogerluan avatar Feb 26 '22 23:02 rogerluan

Here it is https://github.com/fastlane/docs/pull/1144

Thanks for opening this issue @janbrasna ! 🤗

rogerluan avatar Feb 26 '22 23:02 rogerluan

Ah… Now I get what's going on in CI haha nevermind my comments above about it succeeding in danger. I ran a dry_run locally, only.

We'll need to figure out the GitHub Token situation, which's not trivial since this is an open source repo and danger kinda self-protects repos from exposing DANGER_GITHUB_API_TOKEN 😬

Do you know what needs to be changed there? I haven't dug further into danger's set up, but I'm assuming DANGER_GITHUB_API_TOKEN isn't set in our CI for that reason

rogerluan avatar Feb 26 '22 23:02 rogerluan

Honestly, IDK — otherwise I'd give more pointers where to look; i just see the silent fail when checking deploy logs (usually trying to find out what's this time netlify doesn't like about the python env and fails;D)

I just basically copied the log message about API keys in here for better visibility into the failover.

janbrasna avatar Jul 03 '22 20:07 janbrasna

From what I see in the PRs, those opened by a bot these days have a danger/danger test in the checks explicitly:

Screenshot 2022-07-03 at 22 24 27

others don't:

Screenshot 2022-07-03 at 22 24 13

🤷‍♂️

So maybe the right way is to move it outside the CircleCI and keep it as a separate check only? However I don't have enough visibility into when that check is triggered by the fastlane bot TBH…

janbrasna avatar Jul 03 '22 20:07 janbrasna

Or actually it seems that PRs opened by @fastlane members have the danger/danger run, PRs opened by others don't… 🤷‍♂️ weird huh?

janbrasna avatar Jul 06 '22 22:07 janbrasna

Hi @janbrasna , since my last comment in this thread I've acquired more knowledge on the topic and am capable of clarifying what we're seeing the behavior you described in your last comment here 😊 ☝️

PRs opened by fastlane members have the danger/danger run, PRs opened by others don't

This is a security feature. The technical difference is "PRs coming from forks" vs "PRs opened within this repo", I think. This prevents users from creating PRs from forks from executing arbitrary code (e.g. introducing a new script that echo $DANGER_GITHUB_API_TOKEN so they can read the env var, just as a simple example).

Each CI has their own mechanism to prevent this from happening, so the solution to this problem will be a solution specific for Circle CI, and not e.g. danger itself.

Looks like this could be a good starting point: https://circleci.com/blog/triggering-trusted-ci-jobs-on-untrusted-forks (I didn't read it yet, I just found this)

rogerluan avatar Jul 10 '22 12:07 rogerluan