Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
25 changes: 0 additions & 25 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,6 @@ name: Test workflow

on:
workflow_call:
secrets:
SECRET_KEY_BASE:
required: true
SESSION_ENCRYPTED_COOKIE_SALT:
required: true
JWT_PRIVATE_KEY:
required: true
JWT_PUBLIC_KEY:
required: true
MDS_USERNAME:
required: true
MDS_PASSWORD:
required: true
AWS_REGION:
required: true

jobs:
test:
Expand All @@ -42,13 +27,6 @@ jobs:
MYSQL_HOST: "127.0.0.1"
MYSQL_DATABASE: metrics_test
MYSQL_USER: root
SECRET_KEY_BASE: ${{ secrets.SECRET_KEY_BASE }}
SESSION_ENCRYPTED_COOKIE_SALT: ${{ secrets.SESSION_ENCRYPTED_COOKIE_SALT }}
JWT_PRIVATE_KEY: ${{ secrets.JWT_PRIVATE_KEY }}
JWT_PUBLIC_KEY: ${{ secrets.JWT_PUBLIC_KEY }}
MDS_USERNAME: ${{ secrets.MDS_USERNAME }}
MDS_PASSWORD: ${{ secrets.MDS_PASSWORD }}
AWS_REGION: ${{ secrets.AWS_REGION }}
steps:
- uses: actions/checkout@v3
- name: Set up Ruby 4.0.1
Expand Down Expand Up @@ -76,9 +54,6 @@ jobs:
env:
MYSQL_PORT: ${{ job.services.mysql.ports[3306] }}
MEMCACHE_SERVERS: "localhost:11211"
AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
AWS_S3_BUCKET: 'metrics-api.stage.datacite.org'
run: |
bundle exec rspec
echo $?
22 changes: 7 additions & 15 deletions config/environments/test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
# While tests run files are not watched, reloading is not necessary.
config.enable_reloading = false

config.log_level = :warn
# Eager loading loads your entire application. When running a single test locally,
# this is usually not necessary, and can slow down your test suite. However, it's
# recommended that you enable it in continuous integration systems to ensure eager
Expand Down Expand Up @@ -40,19 +41,10 @@
# Raise error when a before_action's only/except options reference missing actions.
config.action_controller.raise_on_missing_callback_actions = true

# kt-paperclip global defaults - note bucket is different per environment.
config.paperclip_defaults = {
storage: :s3,
s3_protocol: "https",
# s3_host_alias: 's3-eu-west-1.amazonaws.com',
s3_credentials: {
access_key_id: ENV["AWS_ACCESS_KEY_ID"].to_s,
secret_access_key: ENV["AWS_SECRET_ACCESS_KEY"].to_s,
s3_region: ENV["AWS_REGION"] || "eu-west-1",
},
bucket: ENV["AWS_S3_BUCKET"] || "metrics-api.test.datacite.org",
path: "/report_files/:filename",
url: ":s3_domain_url",
use_timestamp: false,
}
config.paperclip_defaults = {
storage: :filesystem,
path: ":rails_root/tmp/report_files/:filename",
url: "/system/report_files/:filename",
use_timestamp: false,
}
end
26 changes: 11 additions & 15 deletions spec/rails_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,16 @@

require File.expand_path('../../config/environment', __FILE__)

# Test JWT keys and other support load after boot so they override .env.
Dir[Rails.root.join('spec/support/**/*.rb')].each { |f| require f }

%w[AWS_REGION AWS_ACCESS_KEY_ID AWS_SECRET_ACCESS_KEY AWS_S3_BUCKET].each do |key|
ENV.delete(key)
end

require "aws-sdk-sqs"
Aws.config.update(stub_responses: true)

require "rspec/rails"
require "shoulda-matchers"
require "webmock/rspec"
Expand Down Expand Up @@ -45,18 +53,8 @@

ActiveJob::Base.queue_adapter = :test

## To do: find another way to track and delete test files.
## Dev/Test/Stage- {Rails.root}/public/report_files.
## Production - AWS S3 BUCKET (report_files directory)
# Cleans up generated reports files after test run.
# Make sure we are not doing this in production.
config.after(:each) do # :suite or :each or :all
# Never delete files in production. This is dangerous, should find another way to clean up.
if Rails.env.development? || Rails.env.test? || Rails.env.stage?
Dir["#{Rails.root}/public/report_files/**"].each do |file|
# File.delete(file)
end
end
config.after(:suite) do
FileUtils.rm_rf(Rails.root.join("tmp/report_files"))
end
end

Expand All @@ -67,7 +65,5 @@
c.ignore_hosts "codeclimate.com"
c.configure_rspec_metadata!
c.default_cassette_options = { :match_requests_on => [:method, :path] }
# Allow AWS S3 requests to go through in testing without VCR, for now.
c.allow_http_connections_when_no_cassette = true
c.allow_http_connections_when_no_cassette = false
end

46 changes: 46 additions & 0 deletions spec/support/test_jwt_keys.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# Test-only RSA keypair for JWT encode/decode in specs.
# Not used in any deployed environment.
# rubocop:disable Layout/LineLength -- PEM blocks
ENV["JWT_PRIVATE_KEY"] = <<~PEM

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I don't think we should ever embed in our public repositories private/public key pairs, even if this is only ever used for the tests, it exposes a key that while currently won't be used in production, things can change and we could accidentally expose details we don't expect to.

Using github secrets is a better alternative because it's never then exposed. An alternative to this might instead be to just generate a key pair dynamically in the code.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

While I don't 100% agree when these keys are kept in specs and not reused elsewhere, I think it is also fine to generate the pair dynamically. I don't think they should have to be kept in the environment whether that is provided in the github secrets or elsewhere.

I'll work on generating them dynamically

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Looks like we have gone through iterations on this idea before in other projects
datacite/lupo@c5c3c61

Lupo currently uses fixtures for keys.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for taking a look at this @richardhallett . I've updated the helper so that it generates the public private pair once per Rspec process.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

There are a couple of lines printed when the specs run; JWT decode ERROR lines that are the expected empty/abc token cases

-----BEGIN PRIVATE KEY-----
MIIEuwIBADANBgkqhkiG9w0BAQEFAASCBKUwggShAgEAAoIBAQC9GgZ7N3VRwhNX
zDYJugGV50CyabTx/aZYe0JIL5j1ZFJEwG7fhRz03EQAkD6897mhgpHM2MsmPtYx
AybxC7Qs9q7eKCbX/z9CVRNJF4ayQoGDJdiaVAhInmtBwl3zU0p4Di+Nms/zJQFL
rOikzy/m5lS5gnXN4B6k/6V/A9YGH+nFmTuegwk0+MkSyrVvODG47nUvo8TCn4NP
yHSg2GwYO8mJ5M9sTtVNWGJ0Bn8BXjMgLTD0j23cdcPO43xD70S7dI+5l+vfaF0j
TnoCdimyIPD/flJpVlwmzjjbC2kOV2SehiffNHcJDvB1PSTzI9pxn3AfGYBW68a9
RW4xir2LAgMBAAECgf8ZZuWUbtB+Vb5f4jlgFRx2v28cFojUar5RQOgmdOkI8UZU
vRMLSOSMqpg/CVpHssZniDDU05/Lyoil8GLnN0e1phgsv41by9+lUI9xUFQrSKdW
BOoiCe7aC1FdIHfTt9yQJ5CREYHdp5rogUNpw8CA4FOBQqGQhkOXkTGb0BLFTdc7
ePUnaY3cRhGiS9XMsiweJYl70NNc7Rn1Q8Z6a5qrRaxu0mGQKbw9QB18hrne9Rr8
sw19Yib9ftRq54rawGNQqGf5J9s0z5E8d1/kYBN7/9+eorDqZIufzy63mG1wOF7V
C5zxquKqWv66m3K/4q2Lss1+8tN+u+IdB0dZ7sECgYEA42uQvWACw+WrvPNBqGk3
nw8P0JAZUkYUZU6F8PS/BPH6vGMUxzCgr9T3so+Accl5KcdD8YI397D+UZ6/9C0E
XXvPysiggFvo2WOG0M8ldvBVt/C9D5AQnvMGuJpKeWafppjeYvhWSa9TibW5U1DL
JjCV6bFzKf+MH0fcsu/2/8sCgYEA1N2ytfZEaIAkoUg+AsxIvjv3i6qQAKo6vG/a
Ni4ogV3EFYz/vJRE8hZ3sJxWJmJd+aF/fbpdb0CzGYEb+LiA4PaDsPDrdGYhqkZa
exuJLjfs8+5t41ja6iegaOVaiNaXqpuUvR+BkQYNVZO+DddH38cgkqEhQG2sPk7H
AGP8AUECgYB4aoMoEsdEDL8DfFYeox+lY+c+yp3tt1ejUJGRRTZDmxDlYR0mXzQm
BgDlliAvn7s9CNqL7SRPTXNLql//HTr7eMxMljCN5pyDRrSiqZwvvw/UJm8k52Y1
k37Sjjey0kjkgHr4MoAV4fS8rPElEVmPrhgitifBbCM4iOduA6toQwKBgQCeFAms
Qbdpl4gcUcbjUwvOYkAic0a/h6tY1wPI6pxf/d8pVe1xAPb2hSLbgC4mETTStlE0
cFsEzJp0nUHFglcLOWZw12kzpsjnZ5m4BKnqLdQFDpKCnDDuZQEmBPUn250EMMfT
bvtMVJ9AZxhjtPUGRR12ZryH2Spplx0HuDikQQKBgDI8VJlgH2oGWD4Ojr3ooR9Z
gvAYEZ8jol9Ja4pQcPUVmmNc1CtvNHrQRLH3MdinNUOnKPbujjn9j+fJ+IMgfw+s
KHh/Pn1+8SUTD+RTTjakQFYTzryMrIpRy0GdksHGU+oTThrGjigBo4jpF0YJXv+9
KvQnQF4urs6knvgIZuW0
-----END PRIVATE KEY-----
PEM

ENV["JWT_PUBLIC_KEY"] = <<~PEM
-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAvRoGezd1UcITV8w2CboB
ledAsmm08f2mWHtCSC+Y9WRSRMBu34Uc9NxEAJA+vPe5oYKRzNjLJj7WMQMm8Qu0
LPau3igm1/8/QlUTSReGskKBgyXYmlQISJ5rQcJd81NKeA4vjZrP8yUBS6zopM8v
5uZUuYJ1zeAepP+lfwPWBh/pxZk7noMJNPjJEsq1bzgxuO51L6PEwp+DT8h0oNhs
GDvJieTPbE7VTVhidAZ/AV4zIC0w9I9t3HXDzuN8Q+9Eu3SPuZfr32hdI056AnYp
siDw/35SaVZcJs442wtpDldknoYn3zR3CQ7wdT0k8yPacZ9wHxmAVuvGvUVuMYq9
iwIDAQAB
-----END PUBLIC KEY-----
PEM
# rubocop:enable Layout/LineLength
Loading