Merge pull request #655 from jardleex/use_x-consul-token_header

use X-Consul-Token header instead of token query parameter
voxpupuli · Mar 8, 2024 · 6ea9574 · 6ea9574
2 parents 9439999 + 53303c0
commit 6ea9574
Show file tree

Hide file tree

Showing 8 changed files with 70 additions and 63 deletions.
diff --git a/REFERENCE.md b/REFERENCE.md
@@ -1110,7 +1110,7 @@ The following parameters are available in the `consul_acl` type.
 
 Token for accessing the ACL API
 
-Default value: `''`
+Default value: `anonymous`
 
 ##### <a name="-consul_acl--api_tries"></a>`api_tries`
 
@@ -1179,7 +1179,7 @@ The following parameters are available in the `consul_key_value` type.
 
 Token for accessing the ACL API
 
-Default value: `''`
+Default value: `anonymous`
 
 ##### <a name="-consul_key_value--api_tries"></a>`api_tries`
 
@@ -1373,7 +1373,7 @@ The following parameters are available in the `consul_prepared_query` type.
 
 Token for accessing the ACL API
 
-Default value: `''`
+Default value: `anonymous`
 
 ##### <a name="-consul_prepared_query--api_tries"></a>`api_tries`
 

diff --git a/lib/puppet/provider/consul_acl/default.rb b/lib/puppet/provider/consul_acl/default.rb
@@ -41,8 +41,9 @@ def self.list_resources(acl_api_token, port, hostname, protocol, tries)
     http = Net::HTTP.new(uri.host, uri.port)
     http.use_ssl = true if uri.instance_of? URI::HTTPS
 
-    path = uri.request_uri + "/list?token=#{acl_api_token}"
-    req = Net::HTTP::Get.new(path)
+    path = "#{uri.request_uri}/list"
+    http_headers = { 'X-Consul-Token' => acl_api_token.to_s }
+    req = Net::HTTP::Get.new(path, http_headers)
     res = nil
     res_code = nil
 
@@ -58,7 +59,7 @@ def self.list_resources(acl_api_token, port, hostname, protocol, tries)
         res_code = res.code
         break if res_code == '200'
       rescue Errno::ECONNREFUSED => e
-        Puppet.debug("#{uri}/list?token=<redacted> #{e.class} #{e.message}")
+        Puppet.debug("#{uri}/list #{e.class} #{e.message}")
         res_code = e.class.to_s
       end
     end
@@ -94,8 +95,9 @@ def put_acl(method, body)
     http = Net::HTTP.new(uri.host, uri.port)
     http.use_ssl = true if uri.instance_of? URI::HTTPS
     acl_api_token = @resource[:acl_api_token]
-    path = uri.request_uri + "/#{method}?token=#{acl_api_token}"
-    req = Net::HTTP::Put.new(path)
+    path = uri.request_uri + "/#{method}"
+    http_headers = { 'X-Consul-Token' => acl_api_token.to_s }
+    req = Net::HTTP::Put.new(path, http_headers)
     req.body = body.to_json if body
     res = http.request(req)
     raise(Puppet::Error, "Session #{name} create: invalid return code #{res.code} uri: #{path} body: #{req.body}") if res.code != '200'

diff --git a/lib/puppet/provider/consul_key_value/default.rb b/lib/puppet/provider/consul_key_value/default.rb
@@ -38,9 +38,13 @@ def self.list_resources(acl_api_token, port, hostname, protocol, tries, datacent
 
     # this might be configurable by searching /etc/consul.d
     # but would break for anyone using nonstandard paths
-    consul_url = "#{protocol}://#{hostname}:#{port}/v1/kv/?dc=#{datacenter}&recurse&token=#{acl_api_token}"
+    consul_url = "#{protocol}://#{hostname}:#{port}/v1/kv/?dc=#{datacenter}&recurse"
 
     uri = URI(consul_url)
+    http = Net::HTTP.new(uri.host, uri.port)
+    http.use_ssl = true if uri.instance_of? URI::HTTPS
+    http_headers = { 'X-Consul-Token' => acl_api_token.to_s }
+    req = Net::HTTP::Get.new(uri.request_uri, http_headers)
     res = nil
 
     # retry Consul API query for ACLs, in case Consul has just started
@@ -49,7 +53,7 @@ def self.list_resources(acl_api_token, port, hostname, protocol, tries, datacent
         Puppet.debug("retrying Consul API query in #{i} seconds")
         sleep i
       end
-      res = Net::HTTP.get_response(uri)
+      res = http.request(req)
       break if res.code == '200'
     end
 
@@ -83,24 +87,25 @@ def self.reset
   end
 
   def get_path(name)
-    uri = URI("#{@resource[:protocol]}://#{@resource[:hostname]}:#{@resource[:port]}/v1/kv/#{name}?dc=#{@resource[:datacenter]}&token=#{@resource[:acl_api_token]}")
+    uri = URI("#{@resource[:protocol]}://#{@resource[:hostname]}:#{@resource[:port]}/v1/kv/#{name}?dc=#{@resource[:datacenter]}")
     http = Net::HTTP.new(uri.host, uri.port)
     http.use_ssl = true if uri.instance_of? URI::HTTPS
     acl_api_token = @resource[:acl_api_token]
-    [uri.request_uri, http]
+    http_headers = { 'X-Consul-Token' => acl_api_token.to_s }
+    [uri.request_uri, http, http_headers]
   end
 
   def create_or_update_key_value(name, value, flags)
-    path, http = get_path(name)
-    req = Net::HTTP::Put.new(path + "&flags=#{flags}")
+    path, http, http_headers = get_path(name)
+    req = Net::HTTP::Put.new(path + "&flags=#{flags}", http_headers)
     req.body = value
     res = http.request(req)
     raise(Puppet::Error, "Session #{name} create/update: invalid return code #{res.code} uri: #{path} body: #{req.body}") if res.code != '200'
   end
 
   def delete_key_value(name)
-    path, http = get_path(name)
-    req = Net::HTTP::Delete.new(path)
+    path, http, http_headers = get_path(name)
+    req = Net::HTTP::Delete.new(path, http_headers)
     res = http.request(req)
     raise(Puppet::Error, "Session #{name} delete: invalid return code #{res.code} uri: #{path} body: #{req.body}") if res.code != '200'
   end

diff --git a/lib/puppet/provider/consul_prepared_query/default.rb b/lib/puppet/provider/consul_prepared_query/default.rb
@@ -38,9 +38,8 @@ def self.list_resources(acl_api_token, port, hostname, protocol, tries)
     uri = URI("#{protocol}://#{hostname}:#{port}/v1/query")
     http = Net::HTTP.new(uri.host, uri.port)
     http.use_ssl = true if uri.instance_of? URI::HTTPS
-
-    path = uri.request_uri + "?token=#{acl_api_token}"
-    req = Net::HTTP::Get.new(path)
+    http_headers = { 'X-Consul-Token' => acl_api_token.to_s }
+    req = Net::HTTP::Get.new(uri.request_uri, http_headers)
     res = nil
 
     # retry Consul API query for ACLs, in case Consul has just started
@@ -82,20 +81,21 @@ def get_path(id)
     http = Net::HTTP.new(uri.host, uri.port)
     http.use_ssl = true if uri.instance_of? URI::HTTPS
     acl_api_token = @resource[:acl_api_token]
-    [uri.request_uri + "?token=#{acl_api_token}", http]
+    http_headers = { 'X-Consul-Token' => acl_api_token.to_s }
+    [uri.request_uri, http, http_headers]
   end
 
   def create_prepared_query(body)
-    path, http = get_path(false)
-    req = Net::HTTP::Post.new(path)
+    path, http, http_headers = get_path(false)
+    req = Net::HTTP::Post.new(path, http_headers)
     req.body = body.to_json if body
     res = http.request(req)
     raise(Puppet::Error, "Session #{name} create: invalid return code #{res.code} uri: #{path} body: #{req.body}") if res.code != '200'
   end
 
   def update_prepared_query(id, body)
-    path, http = get_path(id)
-    req = Net::HTTP::Put.new(path)
+    path, http, http_headers = get_path(id)
+    req = Net::HTTP::Put.new(path, http_headers)
     if body
       body[:id] = id
       req.body = body.to_json
@@ -105,8 +105,8 @@ def update_prepared_query(id, body)
   end
 
   def delete_prepared_query(id)
-    path, http = get_path(id)
-    req = Net::HTTP::Delete.new(path)
+    path, http, http_headers = get_path(id)
+    req = Net::HTTP::Delete.new(path, http_headers)
     res = http.request(req)
     raise(Puppet::Error, "Session #{name} delete: invalid return code #{res.code} uri: #{path} body: #{req.body}") if res.code != '200'
   end

diff --git a/lib/puppet/type/consul_acl.rb b/lib/puppet/type/consul_acl.rb
@@ -22,7 +22,7 @@
     validate do |value|
       raise ArgumentError, 'ACL API token must be a string' unless value.is_a?(String)
     end
-    defaultto ''
+    defaultto 'anonymous'
   end
 
   newproperty(:rules) do

diff --git a/lib/puppet/type/consul_key_value.rb b/lib/puppet/type/consul_key_value.rb
@@ -31,7 +31,7 @@
     validate do |value|
       raise ArgumentError, 'ACL API token must be a string' unless value.is_a?(String)
     end
-    defaultto ''
+    defaultto 'anonymous'
   end
 
   newparam(:datacenter) do

diff --git a/lib/puppet/type/consul_prepared_query.rb b/lib/puppet/type/consul_prepared_query.rb
@@ -26,7 +26,7 @@
     validate do |value|
       raise ArgumentError, 'ACL API token must be a string' unless value.is_a?(String)
     end
-    defaultto ''
+    defaultto 'anonymous'
   end
 
   newparam(:service_name) do

diff --git a/spec/unit/puppet/provider/consul_key_value_spec.rb b/spec/unit/puppet/provider/consul_key_value_spec.rb
@@ -27,8 +27,8 @@
             'ModifyIndex' => 1_350_503 },
         ]
 
-        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_return(status: 400, body: '', headers: {}).times(2).then.
           to_return(status: 200, body: JSON.dump(kv_content), headers: {})
 
@@ -40,8 +40,8 @@
 
     context 'when the first three responses are unexpected' do
       it 'silentlies fail to prefetch' do
-        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_return(status: 400, body: '', headers: {})
 
         described_class.reset
@@ -52,8 +52,8 @@
 
     context 'when a timeout is received' do
       it 'does not handle the timeout' do
-        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_timeout
 
         described_class.reset
@@ -98,12 +98,12 @@
             'ModifyIndex' => 1_350_503 },
         ]
 
-        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_return(status: 200, body: JSON.dump(kv_content), headers: {})
 
-        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc2&recurse&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc2&recurse').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_return(status: 404, body: '', headers: {})
 
         described_class.reset
@@ -117,8 +117,8 @@
   describe '#exists?' do
     context 'when resource does not exists' do
       it 'returns false' do
-        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_return(status: 404, body: '', headers: {})
 
         described_class.reset
@@ -138,8 +138,8 @@
             'ModifyIndex' => 1_350_503 },
         ]
 
-        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_return(status: 200, body: JSON.dump(kv_content), headers: {})
 
         described_class.reset
@@ -161,11 +161,11 @@
             'ModifyIndex' => 1_350_503 },
         ]
 
-        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_return(status: 200, body: JSON.dump(kv_content), headers: {})
 
-        stub_request(:put, 'http://localhost:8500/v1/kv/sample/key?dc=dc1&flags=0&token=sampleToken').
+        stub_request(:put, 'http://localhost:8500/v1/kv/sample/key?dc=dc1&flags=0').
           with(body: 'sampleValue',
                headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
           to_return(status: 200, body: '', headers: {})
@@ -188,11 +188,11 @@
             'ModifyIndex' => 1_350_503 },
         ]
 
-        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_return(status: 200, body: JSON.dump(kv_content), headers: {})
 
-        stub_request(:put, 'http://localhost:8500/v1/kv/sample/key?dc=dc1&flags=0&token=sampleToken').
+        stub_request(:put, 'http://localhost:8500/v1/kv/sample/key?dc=dc1&flags=0').
           with(body: 'sampleValue',
                headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
           to_return(status: 200, body: '', headers: {})
@@ -226,11 +226,11 @@
         )
         resources = { 'sample/key' => resource }
 
-        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_return(status: 200, body: JSON.dump(kv_content), headers: {})
 
-        stub_request(:put, 'http://localhost:8500/v1/kv/sample/key?dc=dc1&flags=2&token=sampleToken').
+        stub_request(:put, 'http://localhost:8500/v1/kv/sample/key?dc=dc1&flags=2').
           with(body: 'sampleValue',
                headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
           to_return(status: 200, body: '', headers: {})
@@ -253,11 +253,11 @@
             'ModifyIndex' => 1_350_503 },
         ]
 
-        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_return(status: 200, body: JSON.dump(kv_content), headers: {})
 
-        stub_request(:put, 'http://localhost:8500/v1/kv/sample/key?dc=dc1&flags=0&token=sampleToken').
+        stub_request(:put, 'http://localhost:8500/v1/kv/sample/key?dc=dc1&flags=0').
           with(body: 'sampleValue',
                headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
           to_return(status: 400, body: '', headers: {})
@@ -282,12 +282,12 @@
             'ModifyIndex' => 1_350_503 },
         ]
 
-        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_return(status: 200, body: JSON.dump(kv_content), headers: {})
 
-        stub_request(:delete, 'http://localhost:8500/v1/kv/sample/key?dc=dc1&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:delete, 'http://localhost:8500/v1/kv/sample/key?dc=dc1').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_return(status: 200, body: '', headers: {})
 
         described_class.reset
@@ -308,12 +308,12 @@
             'ModifyIndex' => 1_350_503 },
         ]
 
-        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:get, 'http://localhost:8500/v1/kv/?dc=dc1&recurse').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_return(status: 200, body: JSON.dump(kv_content), headers: {})
 
-        stub_request(:delete, 'http://localhost:8500/v1/kv/sample/key?dc=dc1&token=sampleToken').
-          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby' }).
+        stub_request(:delete, 'http://localhost:8500/v1/kv/sample/key?dc=dc1').
+          with(headers: { 'Accept' => '*/*', 'User-Agent' => 'Ruby', 'X-Consul-Token' => 'sampleToken' }).
           to_return(status: 400, body: '', headers: {})
 
         described_class.reset