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

* Use headers only for api and app keys * Add changelog and version * Remove unused params * Add unit test to check api and app keys * Remove puts * Refactor unit test * Check if api and app keys need to be set in url * Indentation * Indentation * Remove space * Add more unit test * Linting * Linting * Linting
DataDog · Oct 16, 2019 · 82510d6 · 82510d6
1 parent 4a7e6fa
commit 82510d6
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 22 deletions.
diff --git a/lib/dogapi/common.rb b/lib/dogapi/common.rb
@@ -6,6 +6,7 @@
 
 require 'rubygems'
 require 'multi_json'
+require 'set'
 
 module Dogapi
 
@@ -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
@@ -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'
@@ -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)
+      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, {}

diff --git a/lib/dogapi/version.rb b/lib/dogapi/version.rb
@@ -1,3 +1,3 @@
 module Dogapi
-  VERSION = '1.36.0'
+  VERSION = '1.37.0'
 end
diff --git a/spec/integration/comment_spec.rb b/spec/integration/comment_spec.rb
@@ -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
 

diff --git a/spec/integration/common_spec.rb b/spec/integration/common_spec.rb
@@ -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

diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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

diff --git a/spec/unit/common_spec.rb b/spec/unit/common_spec.rb
@@ -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