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

chore(test): enable mongoid use db config from ENV #155

Merged

Conversation

crazyoptimist
Copy link
Collaborator

@crazyoptimist crazyoptimist commented Dec 16, 2022

Description

Not everyone use no-auth mongodb in the dev environment. For my case as an example, I have a mongodb container running on my local machine which is auth configured. So we need to be able to use mongo connection URI from ENV. It's already configured like that in slack-ruby-bot-server-events.
Remote URIs won't work in the test environment because database_cleaner prevents it, but auth should still be allowed.

@crazyoptimist
Copy link
Collaborator Author

crazyoptimist commented Dec 16, 2022

Danger fails:

bundler: failed to load command: danger (/home/runner/work/slack-ruby-bot-server/slack-ruby-bot-server/vendor/bundle/ruby/2.6.0/bin/danger)
/home/runner/work/slack-ruby-bot-server/slack-ruby-bot-server/vendor/bundle/ruby/2.6.0/gems/octokit-4.25.1/lib/octokit/response/raise_error.rb:14:in `on_complete': GET https://api.github.com/repos/slack-ruby/slack-ruby-bot-server/pulls/155: 401 - Bad credentials // See: https://docs.github.com/rest (Octokit::Unauthorized)

Is this related to the repository config maybe? I guess some credentials are missing.

And, some tests fail with this log(it's from my local machine, but CI log looks to be very similar):

  1) Teams oauth v1 oauth registers a team
     Failure/Error: required = zeitwerk_original_require(path)

     LoadError:
       cannot load such file -- rack/handler/webrick
     # /home/debian/.rbenv/versions/3.1.3/lib/ruby/gems/3.1.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
     # /home/debian/.rbenv/versions/3.1.3/lib/ruby/gems/3.1.0/gems/zeitwerk-2.6.6/lib/zeitwerk/kernel.rb:38:in `require'
     # /home/debian/.rbenv/versions/3.1.3/lib/ruby/gems/3.1.0/gems/capybara-3.36.0/lib/capybara/registrations/servers.rb:8:in `block in <top (required)>'
     # /home/debian/.rbenv/versions/3.1.3/lib/ruby/gems/3.1.0/gems/capybara-3.36.0/lib/capybara/server.rb:77:in `block in boot'
     #
     #   Showing full backtrace because every line was filtered out.
     #   See docs for RSpec::Configuration#backtrace_exclusion_patterns and
     #   RSpec::Configuration#backtrace_inclusion_patterns for more information.

@dblock

@dblock
Copy link
Collaborator

dblock commented Dec 19, 2022

The GitHub token for Danger was revoked. I'd rather not replace it with another one, can you help find us an CHANGELOG verifier app that doesn't require a token hard-coded and let's replace danger with it?

@crazyoptimist
Copy link
Collaborator Author

crazyoptimist commented Dec 19, 2022

Two possible solutions

  • Replace the hard-coded github token with github secret variable. It's as simple as just using ${{ secrets.GITHUB_TOKEN }}
  • Run danger in the precommit hook

I think you can create a non-expiring github token with limited permissions and use github actions secret variable without security concerns. And it's way better than figuring out the precommit hook.

@dblock
Copy link
Collaborator

dblock commented Dec 20, 2022

GitHub rotated this token for me and I have dozens of repos where it needs to be changed. The way it was stored was completely insecure (token had very limited permissions but still). Then, secrets cannot be read on a pull requests, you have to use pull_request_target which checks out the wrong source code. So is it really worth keeping the same thing?

We use changelog verifier in https://github.com/opensearch-project/OpenSearch/blob/main/.github/workflows/changelog_verifier.yml for changelogs. Maybe that's good enough?

@crazyoptimist
Copy link
Collaborator Author

crazyoptimist commented Dec 20, 2022

    steps:
      - uses: actions/checkout@v3
        with:
          token: ${{ secrets.GITHUB_TOKEN }}
          ref: ${{ github.event.pull_request.head.sha }}

      - uses: dangoslen/changelog-enforcer@v3
        with:
          skipLabels: "autocut, skip-changelog"

See above, the changelog verifier you referenced is still using a github token, and it checks out the correct code by pointing out the hash of the PR's head.

Replacing the changelog-enforcer action with run danger, we can keep the same thing.

Or let me know if I'm getting it wrong :)

Further, I guess there maybe a way to store the token org wise, not in every repo individually?

@dblock
Copy link
Collaborator

dblock commented Dec 20, 2022

You're wrong. Pull requests from forks don't have access to repo or org secrets. See https://github.com/danger/danger-js/blob/main/source/ci_source/providers/GitHubActions.ts#L152.

The reason the changelog verifier works is that it's not trying to leave comments, just pass/fail.

I changed my mind on trying to replace all this stuff anyway, took may repos, and rotated the token. If someone feels motivated to do better, I'll gladly take it. See #156 for the new token, I'll just merge that. There's a bunch of repos with it in the slack-ruby org, if you want to bulk replace them, that'd be awesome.

@dangerpr-bot
Copy link

dangerpr-bot commented Dec 20, 2022

1 Message
📖 We really appreciate pull requests that demonstrate issues, even without a fix. That said, the next step is to try and fix the failing tests!

Generated by 🚫 Danger

@crazyoptimist
Copy link
Collaborator Author

I got it.
Done with this PR.

I can find only one more repo (slack-ruby-bot-server-events) to do the replacement.
Please list more if you have any.

@dblock
Copy link
Collaborator

dblock commented Dec 20, 2022

@crazyoptimist https://github.com/slack-ruby/slack-ruby-client/blob/master/.github/workflows/pr_lint.yml#L20 almost all active repos in this project have Danger - feel free to rename the job names/files and copy-paste so they are all the same

@dblock dblock merged commit 4dafca7 into slack-ruby:master Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants