Commit e627352b authored by Wyatt Alt's avatar Wyatt Alt

(PDB-2230) support submission of certname/command/version via post params

This allows us to do more granular logging at the wire without parsing the
command payload itself.
parent 2daa71ac
......@@ -32,19 +32,24 @@ The entire command MUST be encoded as UTF-8.
## Command submission
Commands are submitted via HTTP to the `/pdb/cmd/v1` URL and must
conform to the following rules:
Commands must be submitted via HTTP to the `/pdb/cmd/v1` endpoint via one of
two mechanisms:
* A `POST` is used
* The `POST` body must contain the JSON payload.
* There is an `Accept` header that matches `application/json`.
* The content-type is `application/json`.
* Payload and parameters: This method entails POSTing the certname, command name,
command version, and optionally the checksum as parameters, with the POST body
containing the given command's wire format. This mechanism allows PuppetDB to
provide better validation and feedback at time of POSTing without inspecting the
command payload itself, and should be preferred over the alternative due to
lower memory consumption.
Optionally, there may be a query parameter, `checksum`, that contains a SHA-1 hash of
the payload which will be used for verification.
* Payload only (deprecated): This method entails POSTing a single JSON body
containing the certname, command name, and command version along side a
`payload` key valued with the given command's wire format. The checksum is
optionally provided as a parameter.
When a command is successfully submitted, the submitter will
receive the following:
In either case, the checksum should contain a SHA-1 hash of the payload which
will be used for content verification with the server. When a command is
successfully submitted, the submitter will receive the following:
* A response code of 200
* A content-type of `application/json`
......@@ -52,7 +57,8 @@ receive the following:
value is a UUID corresponding to the submitted command. This can be used, for example, by
clients to correlate submitted commands with server-side logs.
The PuppetDB termini for puppet masters use this command API to update facts, catalogs, and reports for nodes.
The PuppetDB termini for Puppet masters use this command API to update facts,
catalogs, and reports for nodes, and will always include the checksum.
### Blocking command submission (EXPERIMENTAL)
......@@ -157,6 +163,14 @@ Puppet resources. It is structured as a JSON object, conforming to the
To post a `replace facts` command you can use the following curl command:
curl -X POST \
-H 'Content-Type:application/json' \
-H 'Accept:application/json' \
-d '{"certname":"test1","environment":"DEV","values":{"myfact":"myvalue"},"producer_timestamp":"2015-01-01"}' \
"http://localhost:8080/pdb/cmd/v1?command=replace-facts&version=4&certname=test1"
or equivalently (with the deprecated mechanism):
curl -X POST \
-H "Accept: application/json" \
-H "Content-Type: application/json" \
......@@ -165,6 +179,14 @@ To post a `replace facts` command you can use the following curl command:
An example of `deactivate node`:
curl -X POST \
-H 'Content-Type:application/json' \
-H 'Accept:application/json' \
-d '{"certname":"test1","producer_timestamp":"2015-01-01"}' \
"http://localhost:8080/pdb/cmd/v1?certname=test1&command=deactivate-node&version=3"
or equivalently:
curl -X POST \
-H "Accept: application/json" \
-H "Content-Type: application/json" \
......
......@@ -65,7 +65,7 @@ module CharEncoding
# @param bad_char_range a range indicating a block of invalid characters
# @return String
def self.error_char_context(str, bad_char_range)
gap = bad_char_range.to_a.length
start_char = [0, bad_char_range.begin-100].max
......@@ -106,9 +106,10 @@ module CharEncoding
# information using error_context_str
#
# @param str A string coming from to_pson, likely a command to be submitted to PDB
# @param error_context_str information about where this string came from for use in error messages
# @param error_context_str information about where this string came from for
# use in error messages. Defaults to nil, in which case no error is reported.
# @return Str
def self.coerce_to_utf8(str, error_context_str)
def self.coerce_to_utf8(str, error_context_str=nil)
str_copy = str.dup
# This code is passed in a string that was created by
# to_pson. to_pson calls force_encoding('ASCII-8BIT') on the
......@@ -127,11 +128,16 @@ module CharEncoding
# byte related issues that could arise from mis-interpreting a
# random extra byte as part of a multi-byte UTF-8 character
str_copy.force_encoding("US-ASCII")
warn_if_invalid_chars(str_copy.encode!("UTF-8",
:invalid => :replace,
:undef => :replace,
:replace => DEFAULT_INVALID_CHAR),
error_context_str)
str_lossy = str_copy.encode!("UTF-8",
:invalid => :replace,
:undef => :replace,
:replace => DEFAULT_INVALID_CHAR)
if !error_context_str.nil?
warn_if_invalid_chars(str_lossy, error_context_str)
else
str_lossy
end
end
end
......
......@@ -24,13 +24,11 @@ class Puppet::Util::Puppetdb::Command
# primitive (numeric type, string, array, or hash) that is natively supported
# by JSON serialization / deserialization libraries.
def initialize(command, version, certname, payload)
@command = command
@version = version
@certname = certname
profile("Format payload", [:puppetdb, :payload, :format]) do
@payload = Puppet::Util::Puppetdb::CharEncoding.utf8_string({
@checksum_payload = Puppet::Util::Puppetdb::CharEncoding.utf8_string({
:command => command,
:version => version,
:certname => certname,
:payload => payload,
# We use to_pson still here, to work around the support for shifting
# binary data from a catalog to PuppetDB. Attempting to use to_json
......@@ -44,21 +42,25 @@ class Puppet::Util::Puppetdb::Command
# Puppet 4.1.0. We need a better answer to non-utf8 data end-to-end.
}.to_pson, "Error encoding a '#{command}' command for host '#{certname}'")
end
@command = Puppet::Util::Puppetdb::CharEncoding.coerce_to_utf8(command).gsub(" ", "_")
@version = version
@certname = Puppet::Util::Puppetdb::CharEncoding.coerce_to_utf8(certname)
@payload = Puppet::Util::Puppetdb::CharEncoding.coerce_to_utf8(payload.to_pson)
end
attr_reader :command, :version, :certname, :payload
attr_reader :command, :version, :certname, :payload, :checksum_payload
# Submit the command, returning the result hash.
#
# @return [Hash <String, String>]
def submit
checksum = Digest::SHA1.hexdigest(payload)
checksum = Digest::SHA1.hexdigest(checksum_payload)
for_whom = " for #{certname}" if certname
params = "checksum=#{checksum}&version=#{version}&certname=#{certname}&command=#{command}"
begin
response = profile("Submit command HTTP post", [:puppetdb, :command, :submit]) do
Http.action("#{CommandsUrl}?checksum=#{checksum}", :command) do |http_instance, path|
Http.action("#{CommandsUrl}?#{params}", :command) do |http_instance, path|
http_instance.post(path, payload, headers)
end
end
......
......@@ -38,15 +38,10 @@ describe Puppet::Resource::Catalog::Puppetdb do
end
it "should POST the catalog command as a JSON string" do
command_payload = subject.munge_catalog(catalog, options)
payload = {
:command => Puppet::Util::Puppetdb::CommandNames::CommandReplaceCatalog,
:version => 7,
:payload => command_payload,
}.to_json
command_payload = subject.munge_catalog(catalog, options).to_json
http.expects(:post).with do |uri, body, headers|
expect(body).to eq(payload)
expect(body).to eq(command_payload)
end.returns response
save
......
......@@ -43,17 +43,11 @@ describe Puppet::Node::Facts::Puppetdb do
trusted_data = {"foo" => "foobar", "certname" => "testing_posting"}
subject.stubs(:get_trusted_info).returns trusted_data
f = {
payload = {
"certname" => facts.name,
"values" => facts.values.merge({"trusted" => trusted_data}),
"environment" => "my_environment",
"producer_timestamp" => "a test",
}
payload = {
:command => CommandReplaceFacts,
:version => 4,
:payload => f,
}.to_pson
http.expects(:post).with do |uri, body, headers|
......@@ -75,11 +69,10 @@ describe Puppet::Node::Facts::Puppetdb do
save
message = JSON.parse(sent_payload)
sent_facts = message['payload']
# We shouldn't modify the original instance
facts.values['something'].should == 100
sent_facts['values']['something'].should == 100
message['values']['something'].should == 100
end
end
......
......@@ -31,12 +31,8 @@ describe Puppet::Node::Puppetdb do
it "should POST a '#{CommandDeactivateNode}' command" do
response.stubs(:body).returns '{"uuid": "a UUID"}'
payload = {
:command => CommandDeactivateNode,
:version => 3,
:payload => { :certname => node,
:producer_timestamp => producer_timestamp },
}.to_json
payload = { :certname => node,
:producer_timestamp => producer_timestamp }.to_json
http.expects(:post).with do |uri,body,headers|
expect(body).to eq(payload)
......
......@@ -26,7 +26,7 @@ describe processor do
def without_producer_timestamp(json_body)
parsed = JSON.parse(json_body)
parsed["payload"].delete("producer_timestamp")
parsed.delete("producer_timestamp")
parsed.to_json
end
......@@ -34,11 +34,7 @@ describe processor do
httpok.stubs(:body).returns '{"uuid": "a UUID"}'
subject.stubs(:run_duration).returns(10)
expected_body = {
:command => Puppet::Util::Puppetdb::CommandNames::CommandStoreReport,
:version => 6,
:payload => subject.send(:report_to_hash)
}.to_json
expected_body = subject.send(:report_to_hash).to_json
Puppet::Network::HttpPool.expects(:http_instance).returns(http)
http.expects(:post).with {|path, body, headers|
......
......@@ -87,7 +87,7 @@ describe Puppet::Util::Puppetdb::Command do
end
Puppet.expects(:debug).with do |msg|
msg =~ /Error encoding a 'command-1' command for host 'foo.localdomain'/ &&
msg =~ Regexp.new(Regexp.quote('"command":"command-1","version":1,"payload":{"foo"')) &&
msg =~ Regexp.new(Regexp.quote('"command":"command-1","version":1,"certname":"foo.localdomain","payload":{"foo"')) &&
msg =~ /1 invalid\/undefined/
end
cmd = described_class.new("command-1", 1, "foo.localdomain", {"foo" => [192].pack('c*')})
......
......@@ -19,23 +19,29 @@
[clojure.java.io :as io]
[clojure.set :as set]))
(defrecord RawJsonString [data])
;; Alias coerce/to-string to avoid reflection
(def ^String to-string coerce/to-string)
(defn add-common-json-encoders!*
"Non-memoize version of add-common-json-encoders!"
[]
(generate/add-encoder
org.postgresql.util.PGobject
(fn [^PGobject data ^JsonGenerator jsonGenerator]
;; The .getPrettyPrinter method on the Jackson jsonGenerator will return
;; nil if `:pretty` is not set as a cheshire option
(if (.getPrettyPrinter jsonGenerator)
(generate/encode-map (core/parse-string (.getValue data)) jsonGenerator)
(.writeRawValue jsonGenerator (.getValue data)))))
org.postgresql.util.PGobject
(fn [^PGobject data ^JsonGenerator jsonGenerator]
;; The .getPrettyPrinter method on the Jackson jsonGenerator will return
;; nil if `:pretty` is not set as a cheshire option
(if (.getPrettyPrinter jsonGenerator)
(generate/encode-map (core/parse-string (.getValue data)) jsonGenerator)
(.writeRawValue jsonGenerator (.getValue data)))))
(generate/add-encoder
org.joda.time.DateTime
(fn [data ^JsonGenerator jsonGenerator]
(.writeString jsonGenerator (to-string data))))
(generate/add-encoder
org.joda.time.DateTime
(fn [data ^JsonGenerator jsonGenerator]
(.writeString jsonGenerator (to-string data)))))
RawJsonString
(fn [^String data ^JsonGenerator jsonGenerator]
(.writeRawValue jsonGenerator (:data data)))))
(def
^{:doc "Registers some common encoders for cheshire JSON encoding.
......
......@@ -23,19 +23,24 @@
(defn validate-command-version
[app]
(fn [{:keys [body-string] :as req}]
(let [{:keys [command version]} (json/parse-string body-string true)
(fn [{:keys [body-string params param-post?] :as req}]
(let [{:strs [command version]} (if param-post?
params
(json/parse-string body-string))
numeric-version (Integer. version)
min-supported (get min-supported-commands command ::invalid)]
(when-not param-post?
(log/warn "POSTing version and command in the body is deprecated. Consider using parameters instead."))
(cond
(= ::invalid min-supported)
(http/bad-request-response
(format "Supported commands are %s. Received '%s'."
valid-commands-str command))
(< version min-supported)
(< numeric-version min-supported)
(http/bad-request-response
(format "%s version %s is retired. The minimum supported version is %s."
command version min-supported))
command numeric-version min-supported))
:else (app req)))))
......@@ -117,7 +122,8 @@
(mid/fail-when-payload-too-large reject-large-commands? max-command-size)
mid/verify-accepts-json
mid/verify-checksum
(mid/validate-query-params {:optional ["checksum" "secondsToWaitForCompletion"]})
(mid/validate-query-params {:optional ["checksum" "secondsToWaitForCompletion"
"certname" "command" "version"]})
mid/payload-to-body-string
(mid/verify-content-type ["application/json"])
(mid/wrap-with-metrics (atom {}) http/leading-uris)
......
......@@ -284,15 +284,36 @@
"application/x-www-form-urlencoded"
(if-let [payload (params "payload")]
(app (assoc req :body-string payload))
(http/error-response (str "Missing required parameter 'payload'")))
(http/error-response "Missing required parameter 'payload'"))
"application/json"
(let [body-string (request/body-string req)]
(if (nil? body-string)
(http/error-response (str "Empty body for application/json submission"))
(app (assoc req :body-string body-string))))
(let [{:strs [certname version command]} params
body-string (request/body-string req)
param-post? (boolean (and certname version command))]
(cond
(and (not param-post?) (or certname version command))
(http/error-response
"Commands submitted via POST with parameters require certname, version, and command")
(nil? body-string)
(http/error-response "Empty body for application/json submission")
:else
(if param-post?
(let [command' (s/replace command "_" " ")
submission-body (-> params
(assoc "payload" body-string)
(assoc "command" command')
utils/cmd-params->json-str)]
(-> req
(assoc-in [:params "command"] command')
(assoc :body-string submission-body :param-post? param-post?)
app))
(app (assoc req :body-string body-string :param-post? param-post?)))))
(if-let [payload (params "payload")]
(app (assoc req :body-string payload))
(http/error-response (str "Missing required parameter 'payload'")))))))
(http/error-response "Missing required parameter 'payload'"))))))
(defn consume-and-close
"Consume all data from input stream and then close"
......
......@@ -310,6 +310,12 @@
#(update % 0 underscores->dashes)
m))
(defn cmd-params->json-str
[{:strs [command version certname payload]}]
(json/generate-string
{:command command :version (Integer. version)
:certname certname :payload (json/->RawJsonString payload)}))
(defmacro with-timeout [timeout-ms default & body]
`(let [f# (future (do ~@body))
result# (deref f# ~timeout-ms ~default)]
......
......@@ -9,6 +9,7 @@
[clojure.test :refer :all]
[puppetlabs.puppetdb.testutils :refer [block-until-results temp-file]]
[me.raynes.fs :as fs]
[puppetlabs.puppetdb.utils :as utils]
[puppetlabs.trapperkeeper.testutils.logging :refer [with-log-output logs-matching]]))
(deftest wrapping-metrics
......@@ -117,13 +118,26 @@
(let [test-req {:body (test-stream)
:headers {"content-type" "application/json"}}]
(is (= (wrapped-fn test-req)
(assoc test-req :body-string test-content)))))))
(assoc test-req :body-string test-content :param-post? false)))))))
(testing "url encoded payload should populate body-string"
(let [test-req {:params {"payload" test-content}
:headers {"content-type" "application/x-www-form-urlencoded"}}]
(is (= (wrapped-fn test-req)
(assoc test-req :body-string test-content)))))))
(assoc test-req :body-string test-content)))))
(testing "param-post? is true when params are included"
(let [test-req {:params {"certname" "foo.com"
"command" "fix drywall"
"version" "307"}
:body (test-stream)
:headers {"content-type" "application/json"}}]
(is (= (wrapped-fn test-req)
(assoc test-req
:body-string (-> (:params test-req)
(assoc "payload" test-content)
utils/cmd-params->json-str)
:param-post? true)))))))
(deftest verify-checksum-test
(let [test-content "test content"
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment