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

Use headers-only for api and app keys with exceptions #189

Merged
merged 17 commits into from
Oct 16, 2019
Merged
28 changes: 24 additions & 4 deletions lib/dogapi/common.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

require 'rubygems'
require 'multi_json'
require 'set'

module Dogapi

Expand Down Expand Up @@ -69,6 +70,7 @@ def request(method, url, params)

# Superclass that deals with the details of communicating with the DataDog API
class APIService
attr_reader :api_key, :application_key
def initialize(api_key, application_key, silent=true, timeout=nil, endpoint=nil)
@api_key = api_key
@application_key = application_key
Expand Down Expand Up @@ -116,8 +118,10 @@ def request(method, url, extra_params, body, send_json, with_app_key=true)
resp = nil
connect do |conn|
begin
current_url = url + prepare_params(extra_params, with_app_key)
current_url = url + prepare_params(extra_params, url, with_app_key)
req = method.new(current_url)
req['DD-API-KEY'] = @api_key
req['DD-APPLICATION-KEY'] = @application_key if with_app_key

if send_json
req.content_type = 'application/json'
Expand All @@ -132,15 +136,31 @@ def request(method, url, extra_params, body, send_json, with_app_key=true)
end
end

def prepare_params(extra_params, with_app_key)
params = { api_key: @api_key }
params[:application_key] = @application_key if with_app_key
def prepare_params(extra_params, url, with_app_key)
params = set_api_and_app_keys_in_params(url, with_app_key)
params = extra_params.merge params unless extra_params.nil?
qs_params = params.map { |k, v| CGI.escape(k.to_s) + '=' + CGI.escape(v.to_s) }
qs = '?' + qs_params.join('&')
qs
end

def set_api_and_app_keys_in_params(url, with_app_key)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot for this 👏
Would it be possible to add a unit test for set_api_and_app_keys_in_params?

Good point. Added a unit test!

set_of_urls = Set.new ['/api/v1/series',
'/api/v1/check_run',
'/api/v1/events',
'/api/v1/screen']

include_in_params = set_of_urls.include?(url)

if include_in_params
params = { api_key: @api_key }
params[:application_key] = @application_key if with_app_key
else
params = {}
end
return params
end

def handle_response(resp)
if resp.code == 204 || resp.body == '' || resp.body == 'null' || resp.body.nil?
return resp.code, {}
Expand Down
2 changes: 1 addition & 1 deletion lib/dogapi/version.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
module Dogapi
VERSION = '1.36.0'
VERSION = '1.37.0'
end
4 changes: 1 addition & 3 deletions spec/integration/comment_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@
stub_request(:put, /#{url}/).to_return(body: '{}').then.to_raise(StandardError)
expect(dog.send(:update_comment, COMMENT_ID, options)).to eq ['200', {}]

expect(WebMock).to have_requested(:put, url).with(
query: default_query
)
expect(WebMock).to have_requested(:put, url)
end
end

Expand Down
8 changes: 2 additions & 6 deletions spec/integration/common_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,15 @@
stub_request(:get, /#{url}/).to_return(body: '{}').then.to_raise(StandardError)
expect(service.request(Net::HTTP::Get, '/api/v1/awesome', nil, nil, true, true)).to eq(['200', {}])

expect(WebMock).to have_requested(:get, url).with(
query: default_query
)
expect(WebMock).to have_requested(:get, url)
end
end
context 'and it is down' do
it 'only queries one endpoint' do
stub_request(:get, /#{url}/).to_timeout
expect(service.request(Net::HTTP::Get, '/api/v1/awesome', nil, nil, true, true)).to eq([-1, {}])

expect(WebMock).to have_requested(:get, url).with(
query: default_query
)
expect(WebMock).to have_requested(:get, url)
end
end
end
Expand Down
12 changes: 4 additions & 8 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ module SpecDog
body = MultiJson.dump(body) if body

expect(WebMock).to have_requested(request, /#{url}|#{old_url}/).with(
query: default_query,
body: body
)
end
Expand All @@ -55,7 +54,6 @@ module SpecDog
body = MultiJson.dump(body ? (body.merge options) : options)

expect(WebMock).to have_requested(request, /#{url}|#{old_url}/).with(
query: default_query,
body: body
)
end
Expand All @@ -67,7 +65,7 @@ module SpecDog
stub_request(request, /#{url}/).to_return(body: '{}').then.to_raise(StandardError)
expect(dog.send(command, *args, *params.values)).to eq ['200', {}]
params.each { |k, v| params[k] = v.join(',') if v.is_a? Array }
params = params.merge default_query
params = params

expect(WebMock).to have_requested(request, url).with(
query: params
Expand All @@ -83,7 +81,7 @@ module SpecDog
expect(dog.send(command, *args, opt_params)).to eq ['200', {}]

opt_params.each { |k, v| opt_params[k] = v.join(',') if v.is_a? Array }
params = opt_params.merge default_query
params = opt_params

expect(WebMock).to have_requested(request, url).with(
query: params
Expand All @@ -101,7 +99,6 @@ module SpecDog
body = MultiJson.dump(body) if body

expect(WebMock).to have_requested(request, url).with(
query: default_query,
body: body
)
end
Expand All @@ -117,7 +114,6 @@ module SpecDog
body = MultiJson.dump(body ? (body.merge options) : options)

expect(WebMock).to have_requested(request, url).with(
query: default_query,
body: body
)
end
Expand All @@ -129,7 +125,7 @@ module SpecDog
stub_request(request, /#{url}/).to_return(body: '{}').then.to_raise(StandardError)
expect(dog2.send(command, *args, *params.values)).to eq ['200', {}]
params.each { |k, v| params[k] = v.join(',') if v.is_a? Array }
params = params.merge default_query
params = params

expect(WebMock).to have_requested(request, url).with(
query: params
Expand All @@ -144,7 +140,7 @@ module SpecDog
stub_request(request, /#{url}/).to_return(body: '{}').then.to_raise(StandardError)
expect(dog2.send(command, *args, opt_params)).to eq ['200', {}]
opt_params.each { |k, v| opt_params[k] = v.join(',') if v.is_a? Array }
params = opt_params.merge default_query
params = opt_params

expect(WebMock).to have_requested(request, url).with(
query: params
Expand Down
38 changes: 38 additions & 0 deletions spec/unit/common_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,44 @@
expect(conn.port).to eq 443
end
end

it 'respects http headers' do
service = Dogapi::APIService.new('api_key', 'app_key', true, nil, 'https://app.example.com')

expect(service.api_key).to eq 'api_key'
expect(service.application_key).to eq 'app_key'
end

it 'sets api and app keys in params' do
service = Dogapi::APIService.new('api_key', 'app_key', true, nil, 'https://app.example.com')

urls = ['/api/v1/series',
'/api/v1/check_run',
'/api/v1/events',
'/api/v1/screen']

urls.each do |url|
params = service.set_api_and_app_keys_in_params(url, true)
expect(params).to have_key(:api_key)
expect(params[:api_key]).to eq service.api_key
expect(params).to have_key(:application_key)
expect(params[:application_key]).to eq service.application_key
end
end

it 'does not set api and app keys in params' do
service = Dogapi::APIService.new('api_key', 'app_key', true, nil, 'https://app.example.com')

urls = ['/api/v2/series',
'/api/v1/random_endpoint',
'/api/v1/dashboards',
'/api/v2/users']

urls.each do |url|
params = service.set_api_and_app_keys_in_params(url, true)
expect(params).to eq({})
end
end
end
end

Expand Down