Skip to content
This repository has been archived by the owner on Jul 24, 2021. It is now read-only.

Commit

Permalink
properly handle a report using a different device's system_uuid
Browse files Browse the repository at this point in the history
If we do not query for the device properly, the wrong device will get updated;
this might also cause a database explosion if the system_uuid and/or
serial_number is already in use by another device.

(cherry picked from commit ed07cf5)
..with conflict resolution, which originally comes from v3.0.0-a6.
  • Loading branch information
karenetheridge committed Apr 15, 2020
1 parent 88e6e29 commit abb862d
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 14 deletions.
42 changes: 28 additions & 14 deletions lib/Conch/Controller/DeviceReport.pm
Original file line number Diff line number Diff line change
Expand Up @@ -83,19 +83,28 @@ sub process ($c) {
: $existing_device ? $existing_device->uptime_since
: undef;

my $device = $c->db_devices->update_or_create({
id => $c->stash('device_id'),
system_uuid => $unserialized_report->{system_uuid},
hardware_product_id => $hw->id,
state => $unserialized_report->{state},
health => 'unknown',
last_seen => \'now()',
uptime_since => $uptime,
hostname => $unserialized_report->{os}{hostname},
updated => \'now()',
deactivated => undef,
my $device = $c->txn_wrapper(sub ($c) {
$c->db_devices->update_or_create({
id => $c->stash('device_id'),
system_uuid => $unserialized_report->{system_uuid},
hardware_product_id => $hw->id,
state => $unserialized_report->{state},
health => 'unknown',
last_seen => \'now()',
uptime_since => $uptime,
hostname => $unserialized_report->{os}{hostname},
updated => \'now()',
deactivated => undef,
},
{ key => 'primary' });
});

if (not $device) {
return $c->status(400, { error => 'could not process report for device '
.$c->stash('device_id')
.($c->stash('exception') ? ': '.(split(/\n/, $c->stash('exception'), 2))[0] : '') });
}

$c->log->debug('Creating device report');
my $device_report = $device->create_related('device_reports', {
report => $c->req->text, # this is the raw json string
Expand Down Expand Up @@ -130,7 +139,9 @@ sub process ($c) {
device => $c->db_ro_devices->find($device->id),
device_report => $device_report,
);
return $c->status(400, { error => 'no validations ran' }) if not $validation_state;
return $c->status(400, { error => 'no validations ran'
.($c->stash('exception') ? ': '.(split(/\n/, $c->stash('exception'), 2))[0] : '') })
if not $validation_state;
$c->log->debug('Validations ran with result: '.$validation_state->status);

# calculate the device health based on the validation results.
Expand Down Expand Up @@ -430,7 +441,8 @@ sub validate_report ($c) {
hostname => $unserialized_report->{os}{hostname},
updated => \'now()',
deactivated => undef,
});
},
{ key => 'primary' });

# we do not call _record_device_configuration, because no validations
# should be using that information, instead choosing to respect the report data.
Expand All @@ -448,7 +460,9 @@ sub validate_report ($c) {
die 'rollback: device used for report validation should not be persisted';
});

return $c->status(400, { error => 'no validations ran' }) if not @validation_results;
return $c->status(400, { error => 'no validations ran'
.($c->stash('exception') ? ': '.(split(/\n/, $c->stash('exception'), 2))[0] : '') })
if not @validation_results;

$c->status(200, {
device_id => $unserialized_report->{serial_number},
Expand Down
14 changes: 14 additions & 0 deletions t/integration/device-reports.t
Original file line number Diff line number Diff line change
Expand Up @@ -79,4 +79,18 @@ subtest 'run report without an existing device' => sub {
});
};

subtest 'system_uuid collisions' => sub {
my $report_data = Mojo::JSON::from_json($report);
$report_data->{serial_number} = 'i_was_here_first';

my $existing_device = $t->generate_fixtures('device', { id => 'i_was_here_first' });

$t->post_ok('/device_report', json => $report_data)
->status_is(400)
->json_cmp_deeply({ error => re(qr/no validations ran: .*duplicate key value violates unique constraint "device_system_uuid_key"/) });

$t->post_ok('/device/i_was_here_first', json => $report_data)
->json_cmp_deeply({ error => re(qr/could not process report for device i_was_here_first.*duplicate key value violates unique constraint "device_system_uuid_key"/) });
};

done_testing;

0 comments on commit abb862d

Please sign in to comment.