Commit 6aa7d847 authored by Nate Wolfe's avatar Nate Wolfe

(PE-959) Save resource event file/line properties

Prior to this commit, the file and line properties that exist on a
resource event were not being saved down to PuppetDB during report
processing. This commit changes that so we can show that information
in Burgundy.
parent 68abcca4
......@@ -76,7 +76,9 @@ A JSON Object of the following form:
"status": <string>,
"old-value": <string>,
"new-value": <string>,
"message": <string>
"message": <string>,
"file": <string>,
"line: <integer>
}
All keys are required.
......@@ -101,6 +103,10 @@ to the event.
`"message"` is a descriptive message providing extra information about the event.
This should be `null` if `status` is `success`.
`"file"` is the manifest in which the resource is defined.
`"line"` is the line number (within `"file"`) where the resource is defined.
## Gaps with this wire format
1. Binary data needs to be dealt with (hopefully only for the `old-value` and
......
......@@ -45,7 +45,7 @@ Puppet::Reports.register_report(:puppetdb) do
if ! (status.events.empty?)
events.concat(
status.events.map do |event|
event_to_hash(status.resource_type, status.title, event)
event_to_hash(status, event)
end)
elsif status.skipped == true
events.concat([resource_status_to_skipped_event_hash(status)])
......@@ -67,16 +67,18 @@ Puppet::Reports.register_report(:puppetdb) do
## Convert an instance of `Puppet::Transaction::Event` to a hash
## suitable for sending over the wire to PuppetDB
def event_to_hash(resource_type, resource_title, event)
def event_to_hash(status, event)
{
"status" => event.status,
"timestamp" => Puppet::Util::Puppetdb.to_wire_time(event.time),
"resource-type" => resource_type,
"resource-title" => resource_title,
"resource-type" => status.resource_type,
"resource-title" => status.title,
"property" => event.property,
"new-value" => event.desired_value,
"old-value" => event.previous_value,
"message" => event.message,
"file" => status.file,
"line" => status.line
}
end
......@@ -94,6 +96,8 @@ Puppet::Reports.register_report(:puppetdb) do
"new-value" => nil,
"old-value" => nil,
"message" => nil,
"file" => nil,
"line" => -1
}
end
......
......@@ -49,7 +49,7 @@ describe processor do
{ :pathbuilder => ["foo", "bar", "baz"],
:path => "foo",
:file => "foo",
:line => "foo",
:line => 1,
:tags => [],
:title => "foo",
:type => "foo" })
......@@ -116,7 +116,8 @@ describe processor do
res_event["new-value"].should == "fooval"
res_event["old-value"].should == "oldfooval"
res_event["message"].should == "foomessage"
res_event["file"].should == "foo"
res_event["line"].should == 1
end
end
......
......@@ -54,7 +54,7 @@
;; these fields allow NULL, which causes a change in semantics when
;; wrapped in a NOT(...) clause, so we have to be very explicit
;; about the NULL case.
[(field :when #{"property" "message"})]
[(field :when #{"property" "message" "file" "line"})]
{:where (format "resource_events.%s = ? AND resource_events.%s IS NOT NULL" field field)
:params [value] }
......@@ -88,7 +88,7 @@
;; these fields allow NULL, which causes a change in semantics when
;; wrapped in a NOT(...) clause, so we have to be very explicit
;; about the NULL case.
[(field :when #{"property" "message"})]
[(field :when #{"property" "message" "file" "line"})]
{:where (format "%s AND resource_events.%s IS NOT NULL"
(sql-regexp-match (format "resource_events.%s" field))
field)
......@@ -127,7 +127,9 @@
resource_events.property,
resource_events.new_value,
resource_events.old_value,
resource_events.message
resource_events.message,
resource_events.file,
resource_events.line
FROM resource_events
JOIN reports ON resource_events.report = reports.hash
WHERE %s")
......
......@@ -26,17 +26,21 @@
:resource-type :string
:resource-title :string
:property { :optional? true
:type :string}
:type :string }
:new-value { :optional? true
:type :jsonable }
:old-value { :optional? true
:type :jsonable }
:message { :optional? true
:type :string}})
:type :string }
:file { :optional? true
:type :string }
:line { :optional? true
:type :integer }})
;; TODO: update this as we add new fields
;; TODO: docs
(def v2-new-event-fields [])
(def v2-new-event-fields [:file :line])
(defn validate-and-add-v2-event-field!
;; TODO: docs
......
......@@ -315,6 +315,13 @@
(format "Unsupported database engine '%s'"
(sql-current-connection-database-name)))))))
(defn add-file-line-columns-to-events-table
"Add 'file' and 'line' columns to the event table."
[]
(sql/do-commands
"ALTER TABLE resource_events ADD COLUMN file VARCHAR(1024) DEFAULT NULL"
"ALTER TABLE resource_events ADD COLUMN line INTEGER DEFAULT NULL"))
;; The available migrations, as a map from migration version to migration
;; function.
(def migrations
......@@ -328,7 +335,8 @@
8 rename-fact-column
9 add-reports-tables
10 add-event-status-index
11 increase-puppet-version-field-length})
11 increase-puppet-version-field-length
12 add-file-line-columns-to-events-table})
(def desired-schema-version (apply max (keys migrations)))
......
......@@ -690,7 +690,7 @@ must be supplied as the value to be matched."
property, values, status, timestamp, etc.)
"
[{:keys [resource-type resource-title property timestamp status old-value
new-value message] :as resource-event}]
new-value message file line] :as resource-event}]
(-> (sort { :resource-type resource-type
:resource-title resource-title
:property property
......@@ -698,7 +698,9 @@ must be supplied as the value to be matched."
:status status
:old-value old-value
:new-value new-value
:message message})
:message message
:file file
:line line})
(pr-str)
(utils/utf8-string->sha1)))
......
......@@ -8,7 +8,9 @@
"message": "infundibulate acmite uranium disciplinarianism",
"resource-title": "undoubting",
"new-value": "ultraloyal",
"status": "success"
"status": "success",
"file": "/Users/foo/bar.pp",
"line": 11
},
{
"old-value": "vacantly",
......@@ -18,7 +20,9 @@
"message": "horizontalize noncoincident levity lucifugous nationalism undergirdle khutuktu somatosplanchnic",
"resource-title": "vitiator",
"new-value": "forecastlehead",
"status": "success"
"status": "success",
"file": null,
"line": null
}
],
"configuration-version": "02ae0b7",
......
......@@ -25,7 +25,9 @@
:property "message"
:new-value "notify, yo"
:old-value ["what" "the" "woah"]
:message "defined 'message' as 'notify, yo'"}
:message "defined 'message' as 'notify, yo'"
:file "foo.pp"
:line 1}
{:test-id 2
:certname "foo.local"
:status "success"
......@@ -35,7 +37,9 @@
:property "message"
:new-value {"absent" 5}
:old-value {"absent" true}
:message "defined 'message' as 'notify, yo'"}
:message "defined 'message' as 'notify, yo'"
:file nil
:line nil}
{:test-id 3
:certname "foo.local"
:status "skipped"
......@@ -45,5 +49,7 @@
:property nil
:new-value nil
:old-value nil
:message nil}]
:message nil
:file "bar"
:line 2}]
}})
......@@ -108,16 +108,20 @@
(testing "equality queries"
(doseq [[field value matches]
[[:resource-type "Notify" [1 2 3]]
[:resource-title "notify, yo" [1]]
[:status "success" [1 2]]
[:property "message" [1 2]]
[:old-value ["what" "the" "woah"] [1]]
[:new-value "notify, yo" [1]]
[[:resource-type "Notify" [1 2 3]]
[:resource-title "notify, yo" [1]]
[:status "success" [1 2]]
[:property "message" [1 2]]
[:old-value ["what" "the" "woah"] [1]]
[:new-value "notify, yo" [1]]
[:message "defined 'message' as 'notify, yo'" [1 2]]
[:resource-title "bunk" []]
[:certname "foo.local" [1 2 3]]
[:certname "bunk.remote" []]]]
[:resource-title "bunk" []]
[:certname "foo.local" [1 2 3]]
[:certname "bunk.remote" []]
[:file "foo.pp" [1]]
[:file "bar" [3]]
[:line 1 [1]]
[:line 2 [3]]]]
(testing (format "equality query on field '%s'" field)
(let [expected (expected-resource-events
(utils/select-values basic-events matches)
......@@ -129,16 +133,20 @@
(testing "'not' queries"
(doseq [[field value matches]
[[:resource-type "Notify" []]
[:resource-title "notify, yo" [2 3]]
[:status "success" [3]]
[:property "message" [3]]
[:old-value ["what" "the" "woah"] [2 3]]
[:new-value "notify, yo" [2 3]]
[[:resource-type "Notify" []]
[:resource-title "notify, yo" [2 3]]
[:status "success" [3]]
[:property "message" [3]]
[:old-value ["what" "the" "woah"] [2 3]]
[:new-value "notify, yo" [2 3]]
[:message "defined 'message' as 'notify, yo'" [3]]
[:resource-title "bunk" [1 2 3]]
[:certname "foo.local" []]
[:certname "bunk.remote" [1 2 3]]]]
[:resource-title "bunk" [1 2 3]]
[:certname "foo.local" []]
[:certname "bunk.remote" [1 2 3]]
[:file "foo.pp" [2 3]]
[:file "bar" [1 2]]
[:line 1 [2 3]]
[:line 2 [1 2]]]]
(testing (format "'not' query on field '%s'" field)
(let [expected (expected-resource-events
(utils/select-values basic-events matches)
......@@ -150,14 +158,16 @@
(testing "regex queries"
(doseq [[field value matches]
[[:resource-type "otify" [1 2 3]]
[:resource-title "^[Nn]otify,\\s*yo$" [1]]
[:status "^.ucces." [1 2]]
[:property "^[Mm][\\w\\s]+" [1 2]]
[:message "notify, yo" [1 2]]
[:resource-title "^bunk$" []]
[:certname "^foo\\." [1 2 3]]
[:certname "^.*\\.mydomain\\.com$" []]]]
[[:resource-type "otify" [1 2 3]]
[:resource-title "^[Nn]otify,\\s*yo$" [1]]
[:status "^.ucces." [1 2]]
[:property "^[Mm][\\w\\s]+" [1 2]]
[:message "notify, yo" [1 2]]
[:resource-title "^bunk$" []]
[:certname "^foo\\." [1 2 3]]
[:certname "^.*\\.mydomain\\.com$" []]
[:file ".*" [1 3]]
[:file "\\.pp" [1]]]]
(testing (format "regex query on field '%s'" field)
(let [expected (expected-resource-events
(utils/select-values basic-events matches)
......@@ -169,14 +179,16 @@
(testing "negated regex queries"
(doseq [[field value matches]
[[:resource-type "otify" []]
[:resource-title "^[Nn]otify,\\s*yo$" [2 3]]
[:status "^.ucces." [3]]
[:property "^[Mm][\\w\\s]+" [3]]
[:message "notify, yo" [3]]
[:resource-title "^bunk$" [1 2 3]]
[:certname "^foo\\." []]
[:certname "^.*\\.mydomain\\.com$" [1 2 3]]]]
[[:resource-type "otify" []]
[:resource-title "^[Nn]otify,\\s*yo$" [2 3]]
[:status "^.ucces." [3]]
[:property "^[Mm][\\w\\s]+" [3]]
[:message "notify, yo" [3]]
[:resource-title "^bunk$" [1 2 3]]
[:certname "^foo\\." []]
[:certname "^.*\\.mydomain\\.com$" [1 2 3]]
[:file ".*" [2]]
[:file "\\.pp" [2 3]]]]
(testing (format "negated regex query on field '%s'" field)
(let [expected (expected-resource-events
(utils/select-values basic-events matches)
......@@ -190,14 +202,16 @@
(testing "'or' equality queries"
(doseq [[terms matches]
[[[[:resource-title "notify, yo"]
[:status "skipped"]] [1 3]
[:status "skipped"]] [1 3]]
[[[:resource-type "bunk"]
[:resource-title "notify, yar"]] [2]]
[[[:resource-type "bunk"]
[:status "bunk"]] []]]
[:status "bunk"]] []]
[[[:new-value "notify, yo"]
[:resource-title "notify, yar"]
[:resource-title "hi"]] [1 2 3]]]]
[:resource-title "hi"]] [1 2 3]]
[[[:file "foo.pp"]
[:line 2]] [1 3]]]]
(let [expected (expected-resource-events
(utils/select-values basic-events matches)
report-hash)
......@@ -219,7 +233,9 @@
[:resource-type "Notify"]
[:certname "foo.local"]] [1]]
[[[:certname "foo.local"]
[:resource-type "Notify"]] [1 2 3]]]]
[:resource-type "Notify"]] [1 2 3]]
[[[:file "foo.pp"]
[:line 1]] [1]]]]
(let [expected (expected-resource-events
(utils/select-values basic-events matches)
report-hash)
......@@ -242,7 +258,12 @@
["=" "status" "success"]]
["and"
["=" "resource-type" "Notify"]
["=" "property" "message"]]] [1 2]]]]
["=" "property" "message"]]] [1 2]]
[["or"
["and"
["=" "file" "foo.pp"]
["=" "line" 1]]
["=" "line" 2]] [1 3]]]]
(let [expected (expected-resource-events
(utils/select-values basic-events matches)
report-hash)
......@@ -265,6 +286,3 @@
(is (= actual expected)
(format "Results didn't match for query '%s'" query)))))))
......@@ -20,16 +20,16 @@
(is (= v2-report (validate! 1 v1-report)))))
;; TODO: uncomment this as soon as we've added :file to v2-new-event-fields
; (testing "should fail when a v1 report has a v2 key"
; (let [add-key-fn (fn [event] (assoc event :file "/tmp/foo"))
; v1-report (munge-v2-example-report-to-v1 report)
; v1-report-with-v2-key (update-in
; v1-report
; [:resource-events]
; #(mapv add-key-fn %))]
; (is (thrown-with-msg?
; IllegalArgumentException #"ResourceEvent has unknown keys: :file.*version 1"
; (validate! 1 v1-report-with-v2-key)))))
(testing "should fail when a v1 report has a v2 key"
(let [add-key-fn (fn [event] (assoc event :file "/tmp/foo"))
v1-report (munge-v2-example-report-to-v1 report)
v1-report-with-v2-key (update-in
v1-report
[:resource-events]
#(mapv add-key-fn %))]
(is (thrown-with-msg?
IllegalArgumentException #"ResourceEvent has unknown keys: :file.*version 1"
(validate! 1 v1-report-with-v2-key)))))
(testing "should accept a valid v2 report"
(is (= report (validate! 2 report))))
......
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