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.

Sign up for GitHub

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

Do not use secrets in test run #2201

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+4 −12
Open

Do not use secrets in test run #2201

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .env.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ DOMAIN="example.com"
STAGING_API_URL="bops-staging.services"
STAGING_API_BEARER="fjisdfjsdiofjdsoi"
OS_VECTOR_TILES_API_KEY="testtest"
NOTIFY_LETTER_API_KEY='testtest'
NOTIFY_API_KEY="test_key_for_ci-d64cc892-8848-4663-9544-487551484e9b-d7bc72d1-8e1f-4d5a-a703-de5beecaf4a8"
NOTIFY_LETTER_API_KEY="test_key_for_ci-d64cc892-8848-4663-9544-487551484e9b-d7bc72d1-8e1f-4d5a-a703-de5beecaf4a8"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these real test keys?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. (Have assumed that because it's specifically for testing and therefore doesn't have permission to do anything, it's less of a risk. Can revert and revoke if that's not the case.)

Problem is that we can't keep it in GH Secrets (if we want to allow PRs from forks) and we can't use a fake key because some of the code is actually instantiating Notify, which throws an error if just given a non-uuid like testtest.

We'd have to track down all of those places and mock Notify (which arguably we should have been doing all along.)

UPLOADS_HOSTNAME=uploads.example.com
UPLOADS_BASE_URL=http://uploads.example.com
USE_SIGNED_URLS=false
13 changes: 2 additions & 11 deletions .github/workflows/testing.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,6 @@ on:
required: false
default: ""

secrets:
NOTIFY_API_KEY:
required: true
OTP_SECRET_ENCRYPTION_KEY:
required: true

jobs:
tests:
name: ${{ inputs.name }}
Expand Down Expand Up @@ -123,9 +117,7 @@ jobs:
env:
DATABASE_URL: postgres://postgres:postgres@localhost:5432/test
RAILS_ENV: test
NOTIFY_API_KEY: ${{ secrets.NOTIFY_API_KEY }}
NOTIFY_LETTER_API_KEY: ${{ secrets.NOTIFY_API_KEY }}
OTP_SECRET_ENCRYPTION_KEY: ${{ secrets.OTP_SECRET_ENCRYPTION_KEY }}
OTP_SECRET_ENCRYPTION_KEY: testtest
SPEC_OPTS: '-f doc --force-color --exclude "${{ inputs.exclude }}" --pattern "${{ inputs.include }}" ${{ inputs.additional_spec_opts }}'
run: |
bundle exec rake spec
Expand Down Expand Up @@ -159,8 +151,7 @@ jobs:
CUCUMBER_FORMAT: pretty
DATABASE_URL: postgres://postgres:postgres@localhost:5432/test
RAILS_ENV: test
NOTIFY_API_KEY: ${{ secrets.NOTIFY_API_KEY }}
OTP_SECRET_ENCRYPTION_KEY: ${{ secrets.OTP_SECRET_ENCRYPTION_KEY }}
OTP_SECRET_ENCRYPTION_KEY: testtest
run: |
bundle exec cucumber

Expand Down
Loading