Diff
Modified: trunk/Websites/perf.webkit.org/ChangeLog (177613 => 177614)
--- trunk/Websites/perf.webkit.org/ChangeLog 2014-12-20 01:21:38 UTC (rev 177613)
+++ trunk/Websites/perf.webkit.org/ChangeLog 2014-12-20 02:38:04 UTC (rev 177614)
@@ -1,3 +1,43 @@
+2014-12-19 Ryosuke Niwa <[email protected]>
+
+ Perf dashboard should support authentication via a slave password
+ https://bugs.webkit.org/show_bug.cgi?id=139837
+
+ Reviewed by Andreas Kling.
+
+ For historical reasons, perf dashboard conflated builders and build slaves. As a result we ended up
+ having to add multiple builders with the same password when a single build slave is shared among them.
+
+ This patch introduces the concept of build_slave into the perf dashboard to end this madness.
+
+ * init-database.sql: Added build_slave table as well as references to it in builds and reports.
+
+ * public/admin/build-slaves.php: Added.
+
+ * public/admin/builders.php: Added the support for updating passwords.
+
+ * public/include/admin-header.php:
+ (update_field): Takes an extra argument when a new value needs to be supplied by the caller instead of
+ being retrieved from $_POST.
+ (AdministrativePage::render_table): Use array_get to retrieve a value out of the database row since
+ the raw may not exist (e.g. new_password).
+ (AdministrativePage::render_form_to_add): Added the support for post_insertion. Don't render the form
+ control here when this flag evaluates to TRUE.
+
+ * public/include/report-processor.php:
+ (ReportProcessor::process): Added the logic to authenticate with slaveName and slavePassword if those
+ values are present in the report. In addition, try authenticating builderName with slavePassword if
+ builderPassword is not specified. When neither password is specified, exit with BuilderNotFound.
+ Also insert the slave or the builder whichever is missing after we've successfully authenticated.
+ (ReportProcessor::construct_build_data): Takes a builder ID and an optional slave ID instead of
+ a builder row.
+ (ReportProcessor::store_report): Store the slave ID with the report.
+ (ReportProcessor::resolve_build_id): Exit with MismatchingBuildSlave when the slave associated with
+ the matching build is different from what's being reported.
+
+ * tests/api-report.js: Added a bunch of tests to test the new features of /api/report.
+ (.addSlave): Added.
+
2014-12-18 Ryosuke Niwa <[email protected]>
New perf dashboard should not duplicate author information in each commit
Modified: trunk/Websites/perf.webkit.org/init-database.sql (177613 => 177614)
--- trunk/Websites/perf.webkit.org/init-database.sql 2014-12-20 01:21:38 UTC (rev 177613)
+++ trunk/Websites/perf.webkit.org/init-database.sql 2014-12-20 02:38:04 UTC (rev 177614)
@@ -7,6 +7,7 @@
DROP TABLE committers CASCADE;
DROP TABLE commits CASCADE;
DROP TABLE build_commits CASCADE;
+DROP TABLE build_slaves CASCADE;
DROP TABLE builders CASCADE;
DROP TABLE repositories CASCADE;
DROP TABLE platforms CASCADE;
@@ -46,12 +47,18 @@
CREATE TABLE builders (
builder_id serial PRIMARY KEY,
builder_name varchar(64) NOT NULL UNIQUE,
- builder_password_hash character(64) NOT NULL,
+ builder_password_hash character(64),
builder_build_url varchar(1024));
+CREATE TABLE build_slaves (
+ slave_id serial PRIMARY KEY,
+ slave_name varchar(64) NOT NULL UNIQUE,
+ slave_password_hash character(64));
+
CREATE TABLE builds (
build_id serial PRIMARY KEY,
build_builder integer REFERENCES builders ON DELETE CASCADE,
+ build_slave integer REFERENCES build_slaves ON DELETE CASCADE,
build_number integer NOT NULL,
build_time timestamp NOT NULL,
build_latest_revision timestamp,
@@ -135,6 +142,7 @@
CREATE TABLE reports (
report_id serial PRIMARY KEY,
report_builder integer NOT NULL REFERENCES builders ON DELETE RESTRICT,
+ report_slave integer REFERENCES build_slaves ON DELETE RESTRICT,
report_build_number integer,
report_build integer REFERENCES builds,
report_created_at timestamp NOT NULL DEFAULT (CURRENT_TIMESTAMP AT TIME ZONE 'UTC'),
Added: trunk/Websites/perf.webkit.org/public/admin/build-slaves.php (0 => 177614)
--- trunk/Websites/perf.webkit.org/public/admin/build-slaves.php (rev 0)
+++ trunk/Websites/perf.webkit.org/public/admin/build-slaves.php 2014-12-20 02:38:04 UTC (rev 177614)
@@ -0,0 +1,35 @@
+<?php
+
+require('../include/admin-header.php');
+
+if ($db) {
+
+ if ($action == 'add') {
+ if ($db->insert_row('build_slaves', 'slave', array('name' => $_POST['name'], 'password_hash' => hash('sha256', $_POST['password'])))) {
+ notice('Inserted the new slave.');
+ regenerate_manifest();
+ } else
+ notice('Could not add the slave.');
+ } else if ($action == 'update') {
+ if (update_field('build_slaves', 'slave', 'name'))
+ regenerate_manifest();
+ else if (update_field('build_slaves', 'slave', 'password_hash', hash('sha256', $_POST['new_password'])))
+ regenerate_manifest();
+ else
+ notice('Invalid parameters.');
+ }
+
+ $page = new AdministrativePage($db, 'build_slaves', 'slave', array(
+ 'name' => array('size' => 50, 'editing_mode' => 'string'),
+ 'password_hash' => array(),
+ 'password' => array('pre_insertion' => TRUE, 'editing_mode' => 'string'),
+ 'new_password' => array('post_insertion' => TRUE, 'editing_mode' => 'string'),
+ ));
+
+ $page->render_table('name');
+ $page->render_form_to_add();
+}
+
+require('../include/admin-footer.php');
+
+?>
Modified: trunk/Websites/perf.webkit.org/public/admin/builders.php (177613 => 177614)
--- trunk/Websites/perf.webkit.org/public/admin/builders.php 2014-12-20 01:21:38 UTC (rev 177613)
+++ trunk/Websites/perf.webkit.org/public/admin/builders.php 2014-12-20 02:38:04 UTC (rev 177614)
@@ -14,6 +14,8 @@
} else if ($action == 'update') {
if (update_field('builders', 'builder', 'name') || update_field('builders', 'builder', 'build_url'))
regenerate_manifest();
+ else if (update_field('build_slaves', 'slave', 'password_hash', hash('sha256', $_POST['new_password'])))
+ regenerate_manifest();
else
notice('Invalid parameters.');
}
@@ -22,6 +24,7 @@
'name' => array('size' => 50, 'editing_mode' => 'string'),
'password_hash' => array(),
'password' => array('pre_insertion' => TRUE, 'editing_mode' => 'string'),
+ 'new_password' => array('post_insertion' => TRUE, 'editing_mode' => 'string'),
'build_url' => array('label' => 'Build URL', 'size' => 100, 'editing_mode' => 'url'),
));
Modified: trunk/Websites/perf.webkit.org/public/include/admin-header.php (177613 => 177614)
--- trunk/Websites/perf.webkit.org/public/include/admin-header.php 2014-12-20 01:21:38 UTC (rev 177613)
+++ trunk/Websites/perf.webkit.org/public/include/admin-header.php 2014-12-20 02:38:04 UTC (rev 177614)
@@ -18,6 +18,7 @@
<li><a href=""
<li><a href=""
<li><a href=""
+ <li><a href=""
<li><a href=""
<li><a href="" Trackers</a></li>
</ul>
@@ -56,18 +57,24 @@
return false;
}
-function update_field($table, $prefix, $field_name) {
+function update_field($table, $prefix, $field_name, $new_value = NULL) {
global $db;
- if (!array_key_exists('id', $_POST) || (array_get($_POST, 'updated-column') != $field_name && !array_key_exists($field_name, $_POST)))
+ if (!array_key_exists('id', $_POST))
return FALSE;
$id = intval($_POST['id']);
$prefixed_field_name = $prefix . '_' . $field_name;
$id_field_name = $prefix . '_id';
+ if ($new_value == NULL) {
+ if (array_get($_POST, 'updated-column') != $field_name && !array_key_exists($field_name, $_POST))
+ return FALSE;
+ $new_value = array_get($_POST, $field_name);
+ }
+
execute_query_and_expect_one_row_to_be_affected("UPDATE $table SET $prefixed_field_name = \$2 WHERE $id_field_name = \$1",
- array($id, array_get($_POST, $field_name)),
+ array($id, $new_value),
"Updated the $prefix $id",
"Could not update $prefix $id");
@@ -226,7 +233,7 @@
continue;
}
- $value = htmlspecialchars($row[$this->prefix . '_' . $name], ENT_QUOTES);
+ $value = htmlspecialchars(array_get($row, $this->prefix . '_' . $name), ENT_QUOTES);
$editing_mode = array_get($this->column_info[$name], 'editing_mode');
if (!$editing_mode) {
echo "<td$rowspan_if_needed>$value</td>\n";
@@ -295,6 +302,8 @@
$editing_mode = array_get($this->column_info[$name], 'editing_mode');
if (array_get($this->column_info[$name], 'custom') || !$editing_mode)
continue;
+ if (array_get($this->column_info[$name], 'post_insertion'))
+ continue;
$label = htmlspecialchars($this->column_label($name));
echo "<label>$label<br>\n";
Modified: trunk/Websites/perf.webkit.org/public/include/report-processor.php (177613 => 177614)
--- trunk/Websites/perf.webkit.org/public/include/report-processor.php 2014-12-20 01:21:38 UTC (rev 177613)
+++ trunk/Websites/perf.webkit.org/public/include/report-processor.php 2014-12-20 02:38:04 UTC (rev 177614)
@@ -39,16 +39,42 @@
array_key_exists('buildTime', $report) or $this->exit_with_error('MissingBuildTime');
$builder_info = array('name' => $report['builderName']);
- if (!$existing_report_id)
- $builder_info['password_hash'] = hash('sha256', $report['builderPassword']);
+ $slave_name = array_get($report, 'slaveName', NULL);
+ $slave_id = NULL;
+ if (!$existing_report_id) {
+ $hash = NULL;
+ if ($slave_name && array_key_exists('slavePassword', $report)) {
+ $hash = hash('sha256', $report['slavePassword']);
+ $slave = $this->db->select_first_row('build_slaves', 'slave', array('name' => $slave_name, 'password_hash' => $hash));
+ if ($slave)
+ $slave_id = $slave['slave_id'];
+ } else if (array_key_exists('builderPassword', $report))
+ $hash = hash('sha256', $report['builderPassword']);
+
+ if (!$hash)
+ $this->exit_with_error('BuilderNotFound');
+ if (!$slave_id)
+ $builder_info['password_hash'] = $hash;
+ }
+
if (array_key_exists('builderPassword', $report))
unset($report['builderPassword']);
+ if (array_key_exists('slavePassword', $report))
+ unset($report['slavePassword']);
- $matched_builder = $this->db->select_first_row('builders', 'builder', $builder_info);
- if (!$matched_builder)
- $this->exit_with_error('BuilderNotFound', array('name' => $builder_info['name']));
+ $builder_id = NULL;
+ if ($slave_id)
+ $builder_id = $this->db->select_or_insert_row('builders', 'builder', $builder_info);
+ else {
+ $builder = $this->db->select_first_row('builders', 'builder', $builder_info);
+ if (!$builder)
+ $this->exit_with_error('BuilderNotFound', array('name' => $builder_info['name']));
+ $builder_id = $builder['builder_id'];
+ if ($slave_name)
+ $slave_id = $this->db->select_or_insert_row('build_slaves', 'slave', array('name' => $slave_name));
+ }
- $build_data = $this->construct_build_data($report, $matched_builder);
+ $build_data = $this->construct_build_data($report, $builder_id, $slave_id);
if (!$existing_report_id)
$this->store_report($report, $build_data);
@@ -67,16 +93,19 @@
$this->runs->commit($platform_id, $build_id);
}
- private function construct_build_data($report, $builder) {
+ private function construct_build_data($report, $builder_id, $slave_id) {
array_key_exists('buildNumber', $report) or $this->exit_with_error('MissingBuildNumber');
array_key_exists('buildTime', $report) or $this->exit_with_error('MissingBuildTime');
- return array('builder' => $builder['builder_id'], 'number' => $report['buildNumber'], 'time' => $report['buildTime']);
+ return array('builder' => $builder_id, 'slave' => $slave_id, 'number' => $report['buildNumber'], 'time' => $report['buildTime']);
}
private function store_report($report, $build_data) {
assert(!$this->report_id);
- $this->report_id = $this->db->insert_row('reports', 'report', array('builder' => $build_data['builder'], 'build_number' => $build_data['number'],
+ $this->report_id = $this->db->insert_row('reports', 'report', array(
+ 'builder' => $build_data['builder'],
+ 'slave' => $build_data['slave'],
+ 'build_number' => $build_data['number'],
'content' => json_encode($report)));
if (!$this->report_id)
$this->exit_with_error('FailedToStoreRunReport');
@@ -84,11 +113,15 @@
private function resolve_build_id($build_data, $revisions) {
// FIXME: This code has a race condition. See <rdar://problem/15876303>.
- $results = $this->db->query_and_fetch_all("SELECT build_id FROM builds WHERE build_builder = $1 AND build_number = $2 AND build_time <= $3 AND build_time + interval '1 day' > $3",
+ $results = $this->db->query_and_fetch_all("SELECT build_id, build_slave FROM builds
+ WHERE build_builder = $1 AND build_number = $2 AND build_time <= $3 AND build_time + interval '1 day' > $3",
array($build_data['builder'], $build_data['number'], $build_data['time']));
- if ($results)
- $build_id = $results[0]['build_id'];
- else
+ if ($results) {
+ $first_result = $results[0];
+ if ($first_result['build_slave'] != $build_data['slave'])
+ $this->exit_with_error('MismatchingBuildSlave', array('storedBuild' => $results, 'reportedBuildData' => $build_data));
+ $build_id = $first_result['build_id'];
+ } else
$build_id = $this->db->insert_row('builds', 'build', $build_data);
if (!$build_id)
$this->exit_with_error('FailedToInsertBuild', $build_data);
Modified: trunk/Websites/perf.webkit.org/tests/api-report.js (177613 => 177614)
--- trunk/Websites/perf.webkit.org/tests/api-report.js 2014-12-20 01:21:38 UTC (rev 177613)
+++ trunk/Websites/perf.webkit.org/tests/api-report.js 2014-12-20 02:38:04 UTC (rev 177614)
@@ -3,6 +3,7 @@
"buildNumber": "123",
"buildTime": "2013-02-28T10:12:03.388304",
"builderName": "someBuilder",
+ "slaveName": "someSlave",
"builderPassword": "somePassword",
"platform": "Mountain Lion",
"tests": {},
@@ -16,11 +17,35 @@
}
}}];
+ var emptySlaveReport = [{
+ "buildNumber": "123",
+ "buildTime": "2013-02-28T10:12:03.388304",
+ "builderName": "someBuilder",
+ "builderPassword": "somePassword",
+ "slaveName": "someSlave",
+ "slavePassword": "otherPassword",
+ "platform": "Mountain Lion",
+ "tests": {},
+ "revisions": {
+ "OS X": {
+ "revision": "10.8.2 12C60"
+ },
+ "WebKit": {
+ "revision": "141977",
+ "timestamp": "2013-02-06T08:55:20.9Z"
+ }
+ }}];
+
function addBuilder(report, callback) {
queryAndFetchAll('INSERT INTO builders (builder_name, builder_password_hash) values ($1, $2)',
[report[0].builderName, sha256(report[0].builderPassword)], callback);
}
+ function addSlave(report, callback) {
+ queryAndFetchAll('INSERT INTO build_slaves (slave_name, slave_password_hash) values ($1, $2)',
+ [report[0].slaveName, sha256(report[0].slavePassword)], callback);
+ }
+
it("should reject error when builder name is missing", function () {
postJSON('/api/report/', [{"buildTime": "2013-02-28T10:12:03.388304"}], function (response) {
assert.equal(response.statusCode, 200);
@@ -53,6 +78,28 @@
});
});
+ it("should reject a report without a builder password", function () {
+ addBuilder(emptyReport, function () {
+ var report = [{
+ "buildNumber": "123",
+ "buildTime": "2013-02-28T10:12:03.388304",
+ "builderName": "someBuilder",
+ "tests": {},
+ "revisions": {}}];
+ postJSON('/api/report/', report, function (response) {
+ assert.equal(response.statusCode, 200);
+ assert.notEqual(JSON.parse(response.responseText)['status'], 'OK');
+ assert.equal(JSON.parse(response.responseText)['failureStored'], false);
+ assert.equal(JSON.parse(response.responseText)['processedRuns'], 0);
+
+ queryAndFetchAll('SELECT COUNT(*) from reports', [], function (rows) {
+ assert.equal(rows[0].count, 0);
+ notifyDone();
+ });
+ });
+ });
+ });
+
it("should store a report from a valid builder", function () {
addBuilder(emptyReport, function () {
postJSON('/api/report/', emptyReport, function (response) {
@@ -68,6 +115,41 @@
});
});
+ it("should treat the slave password as the builder password if there is no matching slave", function () {
+ addBuilder(emptyReport, function () {
+ emptyReport[0]['slavePassword'] = emptyReport[0]['builderPassword'];
+ delete emptyReport[0]['builderPassword'];
+ postJSON('/api/report/', emptyReport, function (response) {
+ emptyReport[0]['builderPassword'] = emptyReport[0]['slavePassword'];
+ delete emptyReport[0]['slavePassword'];
+
+ assert.equal(response.statusCode, 200);
+ assert.equal(JSON.parse(response.responseText)['status'], 'OK');
+ assert.equal(JSON.parse(response.responseText)['failureStored'], false);
+ assert.equal(JSON.parse(response.responseText)['processedRuns'], 1);
+ queryAndFetchAll('SELECT COUNT(*) from reports', [], function (rows) {
+ assert.equal(rows[0].count, 1);
+ notifyDone();
+ });
+ });
+ });
+ });
+
+ it("should store a report from a valid slave", function () {
+ addSlave(emptySlaveReport, function () {
+ postJSON('/api/report/', emptySlaveReport, function (response) {
+ assert.equal(response.statusCode, 200);
+ assert.equal(JSON.parse(response.responseText)['status'], 'OK');
+ assert.equal(JSON.parse(response.responseText)['failureStored'], false);
+ assert.equal(JSON.parse(response.responseText)['processedRuns'], 1);
+ queryAndFetchAll('SELECT COUNT(*) from reports', [], function (rows) {
+ assert.equal(rows[0].count, 1);
+ notifyDone();
+ });
+ });
+ });
+ });
+
it("should store the builder name but not the builder password", function () {
addBuilder(emptyReport, function () {
postJSON('/api/report/', emptyReport, function (response) {
@@ -81,6 +163,28 @@
});
});
+ it("should add a slave if there isn't one and the report was authenticated by a builder", function () {
+ addBuilder(emptyReport, function () {
+ postJSON('/api/report/', emptyReport, function (response) {
+ queryAndFetchAll('SELECT * from build_slaves', [], function (rows) {
+ assert.strictEqual(rows[0].slave_name, emptyReport[0].slaveName);
+ notifyDone();
+ });
+ });
+ });
+ });
+
+ it("should add a builder if there isn't one and the report was authenticated by a slave", function () {
+ addSlave(emptySlaveReport, function () {
+ postJSON('/api/report/', emptySlaveReport, function (response) {
+ queryAndFetchAll('SELECT * from builders', [], function (rows) {
+ assert.strictEqual(rows[0].builder_name, emptyReport[0].builderName);
+ notifyDone();
+ });
+ });
+ });
+ });
+
it("should add a build", function () {
addBuilder(emptyReport, function () {
postJSON('/api/report/', emptyReport, function (response) {
