slack-ruby / slack-ruby-bot-server

A library that enables you to write a complete Slack bot service with Slack button integration, in Ruby.
MIT License
268 stars 74 forks source link

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

Closed crazyoptimist closed 1 year ago

crazyoptimist commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Two possible solutions

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 commented 1 year ago

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 commented 1 year ago
    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 commented 1 year ago

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 https://github.com/slack-ruby/slack-ruby-bot-server/pull/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 commented 1 year ago
1 Message
:book: 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 :no_entry_sign: Danger

crazyoptimist commented 1 year ago

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 commented 1 year ago

@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