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

Fix WIF api call options #174

Merged
merged 6 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
114 changes: 57 additions & 57 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -10,65 +10,66 @@ PATH
GEM
remote: https://rubygems.org/
specs:
actioncable (8.0.0)
actionpack (= 8.0.0)
activesupport (= 8.0.0)
actioncable (7.2.2)
actionpack (= 7.2.2)
activesupport (= 7.2.2)
nio4r (~> 2.0)
websocket-driver (>= 0.6.1)
zeitwerk (~> 2.6)
actionmailbox (8.0.0)
actionpack (= 8.0.0)
activejob (= 8.0.0)
activerecord (= 8.0.0)
activestorage (= 8.0.0)
activesupport (= 8.0.0)
actionmailbox (7.2.2)
actionpack (= 7.2.2)
activejob (= 7.2.2)
activerecord (= 7.2.2)
activestorage (= 7.2.2)
activesupport (= 7.2.2)
mail (>= 2.8.0)
actionmailer (8.0.0)
actionpack (= 8.0.0)
actionview (= 8.0.0)
activejob (= 8.0.0)
activesupport (= 8.0.0)
actionmailer (7.2.2)
actionpack (= 7.2.2)
actionview (= 7.2.2)
activejob (= 7.2.2)
activesupport (= 7.2.2)
mail (>= 2.8.0)
rails-dom-testing (~> 2.2)
actionpack (8.0.0)
actionview (= 8.0.0)
activesupport (= 8.0.0)
actionpack (7.2.2)
actionview (= 7.2.2)
activesupport (= 7.2.2)
nokogiri (>= 1.8.5)
rack (>= 2.2.4)
racc
rack (>= 2.2.4, < 3.2)
rack-session (>= 1.0.1)
rack-test (>= 0.6.3)
rails-dom-testing (~> 2.2)
rails-html-sanitizer (~> 1.6)
useragent (~> 0.16)
actiontext (8.0.0)
actionpack (= 8.0.0)
activerecord (= 8.0.0)
activestorage (= 8.0.0)
activesupport (= 8.0.0)
actiontext (7.2.2)
actionpack (= 7.2.2)
activerecord (= 7.2.2)
activestorage (= 7.2.2)
activesupport (= 7.2.2)
globalid (>= 0.6.0)
nokogiri (>= 1.8.5)
actionview (8.0.0)
activesupport (= 8.0.0)
actionview (7.2.2)
activesupport (= 7.2.2)
builder (~> 3.1)
erubi (~> 1.11)
rails-dom-testing (~> 2.2)
rails-html-sanitizer (~> 1.6)
activejob (8.0.0)
activesupport (= 8.0.0)
activejob (7.2.2)
activesupport (= 7.2.2)
globalid (>= 0.3.6)
activemodel (8.0.0)
activesupport (= 8.0.0)
activerecord (8.0.0)
activemodel (= 8.0.0)
activesupport (= 8.0.0)
activemodel (7.2.2)
activesupport (= 7.2.2)
activerecord (7.2.2)
activemodel (= 7.2.2)
activesupport (= 7.2.2)
timeout (>= 0.4.0)
activestorage (8.0.0)
actionpack (= 8.0.0)
activejob (= 8.0.0)
activerecord (= 8.0.0)
activesupport (= 8.0.0)
activestorage (7.2.2)
actionpack (= 7.2.2)
activejob (= 7.2.2)
activerecord (= 7.2.2)
activesupport (= 7.2.2)
marcel (~> 1.0)
activesupport (8.0.0)
activesupport (7.2.2)
base64
benchmark (>= 0.3)
bigdecimal
Expand All @@ -80,7 +81,6 @@ GEM
minitest (>= 5.1)
securerandom (>= 0.3)
tzinfo (~> 2.0, >= 2.0.5)
uri (>= 0.13.1)
addressable (2.8.7)
public_suffix (>= 2.0.2, < 7.0)
appraisal (2.5.0)
Expand Down Expand Up @@ -286,30 +286,30 @@ GEM
rack (>= 1.3)
rackup (2.2.0)
rack (>= 3)
rails (8.0.0)
actioncable (= 8.0.0)
actionmailbox (= 8.0.0)
actionmailer (= 8.0.0)
actionpack (= 8.0.0)
actiontext (= 8.0.0)
actionview (= 8.0.0)
activejob (= 8.0.0)
activemodel (= 8.0.0)
activerecord (= 8.0.0)
activestorage (= 8.0.0)
activesupport (= 8.0.0)
rails (7.2.2)
actioncable (= 7.2.2)
actionmailbox (= 7.2.2)
actionmailer (= 7.2.2)
actionpack (= 7.2.2)
actiontext (= 7.2.2)
actionview (= 7.2.2)
activejob (= 7.2.2)
activemodel (= 7.2.2)
activerecord (= 7.2.2)
activestorage (= 7.2.2)
activesupport (= 7.2.2)
bundler (>= 1.15.0)
railties (= 8.0.0)
railties (= 7.2.2)
rails-dom-testing (2.2.0)
activesupport (>= 5.0.0)
minitest
nokogiri (>= 1.6)
rails-html-sanitizer (1.6.0)
loofah (~> 2.21)
nokogiri (~> 1.14)
railties (8.0.0)
actionpack (= 8.0.0)
activesupport (= 8.0.0)
railties (7.2.2)
actionpack (= 7.2.2)
activesupport (= 7.2.2)
irb (~> 1.13)
rackup (>= 1.0.0)
rake (>= 12.2)
Expand Down Expand Up @@ -341,7 +341,7 @@ GEM
rspec-mocks (3.13.2)
diff-lcs (>= 1.2.0, < 2.0)
rspec-support (~> 3.13.0)
rspec-rails (7.0.1)
rspec-rails (7.1.0)
actionpack (>= 7.0)
activesupport (>= 7.0)
railties (>= 7.0)
Expand Down Expand Up @@ -432,7 +432,7 @@ GEM
with_model (2.1.7)
activerecord (>= 6.0)
yard (0.9.37)
zeitwerk (2.7.1)
zeitwerk (2.6.18)

PLATFORMS
aarch64-linux
Expand Down
6 changes: 4 additions & 2 deletions lib/dfe/analytics/azure_federated_auth.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module Analytics
class AzureFederatedAuth
DEFAULT_AZURE_SCOPE = 'api://AzureADTokenExchange/.default'
DEFAULT_GCP_SCOPE = 'https://www.googleapis.com/auth/cloud-platform'
ACCESS_TOKEN_EXPIRE_TIME_LEEWAY = 10.seconds
ACCESS_TOKEN_EXPIRE_TIME_LEEWAY = 10

def self.gcp_client_credentials
return @gcp_client_credentials if @gcp_client_credentials && !@gcp_client_credentials.expired?
Expand All @@ -19,7 +19,9 @@ def self.gcp_client_credentials

google_token, expire_time = google_access_token(azure_google_exchange_token)

expire_time_with_leeway = expire_time.to_datetime - ACCESS_TOKEN_EXPIRE_TIME_LEEWAY
# Expire time with leeway that includes the max time for any retries
expire_time_with_leeway =
expire_time.to_datetime - ACCESS_TOKEN_EXPIRE_TIME_LEEWAY.seconds - DfE::Analytics::BigQueryApi::ALL_RETRIES_MAX_ELASPED_TIME.seconds

@gcp_client_credentials = Google::Auth::UserRefreshCredentials.new(access_token: google_token, expires_at: expire_time_with_leeway)
end
Expand Down
21 changes: 17 additions & 4 deletions lib/dfe/analytics/big_query_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,14 @@

module DfE
module Analytics
# For use with for workload identity federeation
# For use with for workload identity federation
class BigQueryApi
# All times are in seconds
ALL_RETRIES_MAX_ELASPED_TIME = 120
RETRY_INITIAL_BASE_INTERVAL = 15
RETRY_MAX_INTERVAL = 60
RETRY_INTERVAL_MULTIPLIER = 2

def self.events_client
@events_client ||= begin
missing_config = %i[
Expand All @@ -27,9 +33,16 @@ def self.events_client
end

def self.insert(events)
rows = events.map { |event| { json: event } }
data_request = Google::Apis::BigqueryV2::InsertAllTableDataRequest.new(rows: rows, skip_invalid_rows: true)
options = Google::Apis::RequestOptions.new(retries: DfE::Analytics.config.bigquery_retries)
rows = events.map { |event| { json: event } }
data_request = Google::Apis::BigqueryV2::InsertAllTableDataRequest.new(rows: rows, skip_invalid_rows: true)
options = Google::Apis::RequestOptions.default

options.authorization = events_client.authorization
options.retries = DfE::Analytics.config.bigquery_retries
options.max_elapsed_time = ALL_RETRIES_MAX_ELASPED_TIME
options.base_interval = RETRY_INITIAL_BASE_INTERVAL
options.max_interval = RETRY_MAX_INTERVAL
options.multiplier = RETRY_INTERVAL_MULTIPLIER

response =
events_client.insert_all_table_data(
Expand Down
10 changes: 5 additions & 5 deletions lib/dfe/analytics/initialisation_events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ module Analytics
# - Event contains the dfe analytics version, config and other items
class InitialisationEvents
# Disable rubocop class variable warnings for class - class variable required to control sending of event
# rubocop:disable Style:ClassVars
@@initialisation_events_sent = false # rubocop:disable Style:ClassVars
# rubocop:disable Style/ClassVars
@@initialisation_events_sent = false # rubocop:disable Style/ClassVars

def self.trigger_initialisation_events
new.send_initialisation_events
Expand All @@ -19,7 +19,7 @@ def self.initialisation_events_sent?
end

def self.initialisation_events_sent=(value)
@@initialisation_events_sent = value # rubocop:disable Style:ClassVars
@@initialisation_events_sent = value # rubocop:disable Style/ClassVars
end

def send_initialisation_events
Expand All @@ -32,7 +32,7 @@ def send_initialisation_events

DfE::Analytics::SendEvents.perform_for([initialise_analytics_event])

@@initialisation_events_sent = true # rubocop:disable Style:ClassVars
@@initialisation_events_sent = true # rubocop:disable Style/ClassVars
end

private
Expand All @@ -45,7 +45,7 @@ def initialise_analytics_data
}
}
end
# rubocop:enable Style:ClassVars
# rubocop:enable Style/ClassVars
end
end
end
4 changes: 3 additions & 1 deletion lib/dfe/analytics/testing.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,11 @@ def events_client
class StubClient
Response = Struct.new(:insert_errors)

def insert(*)
def insert_all_table_data(*)
Response.new([])
end

def authorization; end
end

module TestOverrides
Expand Down
7 changes: 5 additions & 2 deletions spec/dfe/analytics/azure_federated_auth_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,11 @@
it 'returns the expected client credentials' do
expect(described_class.gcp_client_credentials).to be_an_instance_of(Google::Auth::UserRefreshCredentials)
expect(described_class.gcp_client_credentials.access_token).to eq(google_access_token)
expect(described_class.gcp_client_credentials.expires_at)
.to be_within(DfE::Analytics::AzureFederatedAuth::ACCESS_TOKEN_EXPIRE_TIME_LEEWAY).of(future_expire_time)

expiry_leeway_duration =
DfE::Analytics::BigQueryApi::ALL_RETRIES_MAX_ELASPED_TIME.seconds + DfE::Analytics::AzureFederatedAuth::ACCESS_TOKEN_EXPIRE_TIME_LEEWAY.seconds

expect(described_class.gcp_client_credentials.expires_at).to be_within(expiry_leeway_duration).of(future_expire_time)
end

context 'token expiry' do
Expand Down
1 change: 1 addition & 0 deletions spec/dfe/analytics/big_query_api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
allow(Google::Apis::BigqueryV2::BigqueryService).to receive(:new).and_return(events_client)
allow(DfE::Analytics::AzureFederatedAuth).to receive(:gcp_client_credentials).and_return(authorization)
allow(events_client).to receive(:authorization=).and_return(authorization)
allow(events_client).to receive(:authorization).and_return(authorization)

DfE::Analytics::Testing.webmock!
end
Expand Down
Loading