-
Notifications
You must be signed in to change notification settings - Fork 131
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
Jmax/lg 15676 passport api health check infrastructure #11891
Open
jmax-gsa
wants to merge
18
commits into
main
Choose a base branch
from
jmax/LG-15676-passport-api-health-check-infrastructure
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
7bfff01
First test.
jmax-gsa 15e1ff1
Basic fetch for health check api.
jmax-gsa 0ef310d
Response class
jmax-gsa f84be15
Handle Faraday errors.
jmax-gsa 8e5f6f7
Fix success?
jmax-gsa 8adbfc0
Better analytics logging
jmax-gsa aca8a66
refacatoring: Split HealthCheckresponse in two.
jmax-gsa 4587495
Specs for extra analytics params.
jmax-gsa 427fdfb
Spec cleanup
jmax-gsa 916a501
Lint
jmax-gsa 62380cc
More spec cleanup
jmax-gsa 033ae4a
Just moving things around a bit
solipet 4acc77c
Improved event logging.
jmax-gsa d5efe7c
Changed namespace nesting.
jmax-gsa b4ed521
Lint
jmax-gsa 4f90570
Renamed files.
jmax-gsa aba3764
Renamed classes and removed junk files.
jmax-gsa f772c2c
More junk files
jmax-gsa File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
35 changes: 35 additions & 0 deletions
35
app/services/doc_auth/dos/requests/general_health_check_request.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,35 @@ | ||||||||||||||||||||||||||||||||||||
# frozen_string_literal: true | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
module DocAuth | ||||||||||||||||||||||||||||||||||||
module Dos | ||||||||||||||||||||||||||||||||||||
module Requests | ||||||||||||||||||||||||||||||||||||
class GeneralHealthCheckRequest | ||||||||||||||||||||||||||||||||||||
def fetch(analytics) | ||||||||||||||||||||||||||||||||||||
begin | ||||||||||||||||||||||||||||||||||||
faraday_response = connection.get | ||||||||||||||||||||||||||||||||||||
response = Responses::HealthCheckSuccess.new(faraday_response) | ||||||||||||||||||||||||||||||||||||
rescue Faraday::Error => faraday_error | ||||||||||||||||||||||||||||||||||||
response = Responses::HealthCheckFailure.new(faraday_error) | ||||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||||
ensure | ||||||||||||||||||||||||||||||||||||
analytics.passport_api_health_check( | ||||||||||||||||||||||||||||||||||||
success: response.success?, | ||||||||||||||||||||||||||||||||||||
**response.extra, | ||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
private | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
attr_reader :analytics | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
def connection | ||||||||||||||||||||||||||||||||||||
@connection ||= Faraday::Connection.new( | ||||||||||||||||||||||||||||||||||||
url: IdentityConfig.store.passports_api_health_check_endpoint, | ||||||||||||||||||||||||||||||||||||
) do |builder| | ||||||||||||||||||||||||||||||||||||
builder.response :raise_error | ||||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||||
Comment on lines
+25
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||||
end | ||||||||||||||||||||||||||||||||||||
end |
25 changes: 25 additions & 0 deletions
25
app/services/doc_auth/dos/responses/health_check_failure.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
# frozen_string_literal: true | ||
|
||
module DocAuth | ||
module Dos | ||
module Responses | ||
class HealthCheckFailure < DocAuth::Response | ||
def initialize(faraday_error) | ||
errors = | ||
if faraday_error.respond_to?(:status) # some subclasses don't | ||
{ network: faraday_error.status } | ||
else | ||
{ network: true } | ||
end | ||
|
||
super( | ||
success: false, | ||
errors:, | ||
exception: faraday_error, | ||
extra: { error: faraday_error.inspect } | ||
) | ||
end | ||
end | ||
end | ||
end | ||
end |
23 changes: 23 additions & 0 deletions
23
app/services/doc_auth/dos/responses/health_check_success.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
# frozen_string_literal: true | ||
|
||
module DocAuth | ||
module Dos | ||
module Responses | ||
class HealthCheckSuccess < DocAuth::Response | ||
def initialize(faraday_response) | ||
extra = | ||
if faraday_response.body && !faraday_response.body.empty? | ||
{ body: faraday_response.body } | ||
else | ||
{} | ||
end | ||
|
||
super( | ||
success: faraday_response.success?, | ||
extra: | ||
) | ||
end | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -285,6 +285,7 @@ outbound_connection_check_retry_count: 2 | |||
outbound_connection_check_timeout: 5 | ||||
outbound_connection_check_url: 'https://checkip.amazonaws.com' | ||||
participate_in_dap: false | ||||
passports_api_health_check_endpoint: 'https://caapinpe.state.gov/api/passport-match-prc-api/v1/html' | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
password_max_attempts: 3 | ||||
password_pepper: | ||||
personal_key_retired: true | ||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -313,6 +313,7 @@ def self.store | |||
config.add(:outbound_connection_check_timeout, type: :integer) | ||||
config.add(:outbound_connection_check_url) | ||||
config.add(:participate_in_dap, type: :boolean) | ||||
config.add(:passports_api_health_check_endpoint, type: :string) | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
config.add(:password_max_attempts, type: :integer) | ||||
config.add(:password_pepper, type: :string) | ||||
config.add(:personal_key_retired, type: :boolean) | ||||
|
95 changes: 95 additions & 0 deletions
95
spec/services/doc_auth/dos/requests/general_health_check_request_spec.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,95 @@ | ||
require 'rails_helper' | ||
|
||
RSpec.describe DocAuth::Dos::Requests::GeneralHealthCheckRequest do | ||
include PassportApiHelpers | ||
|
||
subject(:health_check_request) { described_class.new } | ||
|
||
let(:analytics) { FakeAnalytics.new } | ||
|
||
let(:health_check_endpoint) do | ||
IdentityConfig.store.passports_api_health_check_endpoint | ||
end | ||
|
||
describe '#fetch' do | ||
let(:result) { health_check_request.fetch(analytics) } | ||
|
||
context 'happy path' do | ||
before do | ||
stub_api_up | ||
end | ||
|
||
it 'hits the endpoint' do | ||
result | ||
expect(WebMock).to have_requested(:get, health_check_endpoint) | ||
end | ||
|
||
it 'logs the request' do | ||
result | ||
expect(analytics).to have_logged_event( | ||
:passport_api_health_check, | ||
success: true, | ||
body: successful_api_health_check_body.to_json, | ||
) | ||
end | ||
|
||
describe 'the #fetch result' do | ||
it 'succeeds' do | ||
expect(result).to be_success | ||
end | ||
end | ||
end | ||
|
||
context 'when Faraday raises an error' do | ||
before do | ||
stub_request(:get, health_check_endpoint).to_raise(Faraday::Error) | ||
end | ||
|
||
it 'hits the endpoint' do | ||
result | ||
expect(WebMock).to have_requested(:get, health_check_endpoint) | ||
end | ||
|
||
it 'logs the request' do | ||
result | ||
expect(analytics).to have_logged_event( | ||
:passport_api_health_check, | ||
success: false, | ||
error: /Faraday::Error/, | ||
) | ||
end | ||
|
||
describe 'the #fetch result' do | ||
it 'does not succeed' do | ||
expect(result).not_to be_success | ||
end | ||
end | ||
end | ||
|
||
context 'when Faraday returns an HTTP error' do | ||
before do | ||
stub_request(:get, health_check_endpoint).to_return(status: 500) | ||
end | ||
|
||
it 'hits the endpoint' do | ||
result | ||
expect(WebMock).to have_requested(:get, health_check_endpoint) | ||
end | ||
|
||
it 'logs the request' do | ||
result | ||
expect(analytics).to have_logged_event( | ||
:passport_api_health_check, | ||
success: false, | ||
error: /Faraday::ServerError/, | ||
) | ||
end | ||
|
||
describe 'the #fetch result' do | ||
it 'does not succeed' do | ||
expect(result).not_to be_success | ||
end | ||
end | ||
end | ||
end | ||
end |
69 changes: 69 additions & 0 deletions
69
spec/services/doc_auth/dos/responses/general_health_check_success_spec.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
require 'rails_helper' | ||
|
||
RSpec.describe DocAuth::Dos::Responses::HealthCheckSuccess do | ||
subject(:health_check_result) { described_class.new(faraday_result) } | ||
|
||
let(:faraday_result) do | ||
Faraday.get(health_check_endpoint) | ||
end | ||
|
||
let(:health_check_endpoint) do | ||
IdentityConfig.store.passports_api_health_check_endpoint | ||
end | ||
|
||
context 'when initialized from a successful request' do | ||
let(:body) do | ||
{ | ||
name: 'Passport Match Process API', | ||
status: 'Up', | ||
environment: 'dev-share', | ||
comments: 'Ok', | ||
}.to_json | ||
end | ||
|
||
before do | ||
stub_request(:get, health_check_endpoint).to_return(body:) | ||
end | ||
|
||
it 'is successful' do | ||
expect(health_check_result).to be_success | ||
end | ||
|
||
it 'has the body in the extra event parameters' do | ||
expect(health_check_result.extra[:body]).to eq(body) | ||
end | ||
end | ||
|
||
# should not happen, because the connection options in | ||
# GeneralHealthCheckRequest prevent it, but let's stay sane if it does. | ||
# 403 is an arbitrary choice. | ||
context 'when initialized from an HTTP error response' do | ||
context 'with no body' do | ||
before do | ||
stub_request(:get, health_check_endpoint).to_return(status: 403) | ||
end | ||
|
||
it 'is not successful' do | ||
expect(health_check_result).not_to be_success | ||
end | ||
|
||
it 'does not include the body: key in the extras' do | ||
expect(health_check_result.extra).not_to have_key(:body) | ||
end | ||
end | ||
|
||
context 'with a body' do | ||
before do | ||
stub_request(:get, health_check_endpoint).to_return(status: 403, body: 'a 403 body') | ||
end | ||
|
||
it 'is not successful' do | ||
expect(health_check_result).not_to be_success | ||
end | ||
|
||
it 'includes the body in the extras' do | ||
expect(health_check_result.extra[:body]).to eq('a 403 body') | ||
end | ||
end | ||
end | ||
end |
36 changes: 36 additions & 0 deletions
36
spec/services/doc_auth/dos/responses/health_check_failure_spec.rb
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
require 'rails_helper' | ||
|
||
RSpec.describe DocAuth::Dos::Responses::HealthCheckFailure do | ||
subject(:health_check_result) { described_class.new(faraday_error) } | ||
|
||
let(:health_check_endpoint) do | ||
IdentityConfig.store.passports_api_health_check_endpoint | ||
end | ||
|
||
def make_faraday_error(status:) | ||
stub_request(:get, health_check_endpoint).to_return(status:) | ||
begin | ||
Faraday.get(health_check_endpoint) | ||
rescue FaradayError => faraday_error | ||
faraday_error | ||
end | ||
end | ||
|
||
[403, 404, 500].each do |http_status| | ||
context "when initialized from an HTTP #{http_status} error" do | ||
let(:faraday_error) { make_faraday_error(status: http_status) } | ||
|
||
it 'is not successful' do | ||
expect(health_check_result).not_to be_success | ||
end | ||
|
||
it 'has the correct errors hash' do | ||
expect(health_check_result.errors).to eq({ network: http_status }) | ||
end | ||
|
||
it 'has the faraday exception' do | ||
expect(health_check_result.exception).to eq(faraday_error) | ||
end | ||
end | ||
end | ||
end |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
module PassportApiHelpers | ||
def successful_api_health_check_body = { | ||
name: 'Passport Match Process API', | ||
status: 'Up', | ||
environment: 'dev-share', | ||
comments: 'Ok', | ||
} | ||
|
||
def stub_api_up | ||
stub_request(:get, health_check_endpoint) | ||
.to_return_json( | ||
body: successful_api_health_check_body, | ||
) | ||
end | ||
end |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.