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

Commit

Permalink
validate_all_responses feature, to be used in development and staging…
Browse files Browse the repository at this point in the history
… environments
  • Loading branch information
karenetheridge committed Nov 24, 2020
1 parent 2c0d74f commit 5ef663c
Show file tree
Hide file tree
Showing 29 changed files with 408 additions and 88 deletions.
12 changes: 12 additions & 0 deletions docs/json-schema/response.json
@@ -1,6 +1,15 @@
{
"$comment" : "NOTE: This file is for human reference ONLY. For programmatic use, use the GET '/json_schema/response/$schema_name' endpoints, or within conch itself, json-schema/response.yaml.",
"$defs" : {
"Anything" : {
"additionalProperties" : true,
"items" : true,
"type" : [
"null",
"object",
"array"
]
},
"Build" : {
"else" : {
"properties" : {
Expand Down Expand Up @@ -1795,6 +1804,9 @@
],
"type" : "object"
},
"Null" : {
"type" : "null"
},
"Organization" : {
"additionalProperties" : false,
"properties" : {
Expand Down
11 changes: 11 additions & 0 deletions docs/modules/Conch::Plugin::JSONValidator.md
Expand Up @@ -88,6 +88,17 @@ Performs more checks when this ["feature" in Conch::Plugin::Features](../modules
assumes the request body schema is [request.json#/$defs/Null](../json-schema/request.json#/$defs/Null) when not provided (for
`POST`, `PUT`, `DELETE` requests)

### after\_dispatch

Runs after dispatching is complete.

Performs more checks when this ["feature" in Conch::Plugin::Features](../modules/Conch%3A%3APlugin%3A%3AFeatures#feature) is enabled:

- `validate_all_responses`

When not provided, assumes the response body schema is [response.json#/$defs/Null](../json-schema/response.json#/$defs/Null)
(for all 2xx responses), or [response.json#/$defs/Error](../json-schema/response.json#/$defs/Error) (for 4xx responses).

## LICENSING

Copyright Joyent, Inc.
Expand Down
6 changes: 6 additions & 0 deletions json-schema/response.yaml
Expand Up @@ -82,6 +82,12 @@ $defs:
uniqueItems: true
items:
$ref: '#/$defs/JSONSchemaError'
'Null':
type: 'null'
Anything:
type: [ 'null', object, array ]
additionalProperties: true
items: true
Ping:
type: object
additionalProperties: false
Expand Down
1 change: 1 addition & 0 deletions lib/Conch.pm
Expand Up @@ -130,6 +130,7 @@ C<< $c->render >> as a side-effect.
$payload //= { error => 'Unauthorized' } if $code == 401;
$payload //= { error => 'Forbidden' } if $code == 403;
$payload //= { error => 'Entity Not Found' } if $code == 404;
$payload //= { error => 'Entity No Longer Available' } if $code == 410;
$payload //= { error => 'Unimplemented' } if $code == 501;

# https://tools.ietf.org/html/rfc7235#section-4.1
Expand Down
33 changes: 24 additions & 9 deletions lib/Conch/Controller/Build.pm
Expand Up @@ -717,9 +717,18 @@ sub get_devices ($c) {
$rs = $rs->search({ last_seen => { '>' => \[ 'now() - ?::interval', $params->{active_minutes}.' minutes' ] } })
if $params->{active_minutes};

$rs = $params->{ids_only} ? $rs->get_column('id')
: $params->{serials_only} ? $rs->get_column('serial_number')
: $rs->with_device_location->with_sku->with_build_name;
if ($params->{ids_only}) {
$rs = $rs->get_column('id');
$c->stash('response_schema', 'DeviceIds');
}
elsif ($params->{serials_only}) {
$rs = $rs->get_column('serial_number');
$c->stash('response_schema', 'DeviceSerials');
}
else {
$rs = $rs->with_device_location->with_sku->with_build_name;
$c->stash('response_schema', 'Devices');
}

$c->status(200, [ $rs->all ]);
}
Expand Down Expand Up @@ -930,12 +939,18 @@ sub get_racks ($c) {
my $rs = $c->stash('build_rs')->related_resultset('racks');
$rs = $rs->search({ 'racks.phase' => $params->{phase} }) if $params->{phase};

$rs = $params->{ids_only} ? $rs->get_column('id')
: $rs->add_columns({ build_name => 'build.name' })
->with_full_rack_name
->with_datacenter_room_alias
->with_rack_role_name
->order_by('racks.name');
if ($params->{ids_only}) {
$rs = $rs->get_column('id');
$c->stash('response_schema', 'RackIds');
}
else {
$rs = $rs->add_columns({ build_name => 'build.name' })
->with_full_rack_name
->with_datacenter_room_alias
->with_rack_role_name
->order_by('racks.name');
$c->stash('response_schema', 'Racks');
}

$c->status(200, [ $rs->all ]);
}
Expand Down
4 changes: 4 additions & 0 deletions lib/Conch/Controller/User.pm
Expand Up @@ -38,6 +38,7 @@ sub find_user ($c) {
}

if ($user->deactivated) {
$c->stash('response_schema', 'UserError') if $c->is_system_admin;
return $c->status(410, {
error => 'user is deactivated',
$c->is_system_admin ? ( user => { map +($_ => $user->$_), qw(id email name created deactivated) } ) : (),
Expand Down Expand Up @@ -381,6 +382,7 @@ sub update ($c) {
&& $c->db_user_accounts->active->find_by_email($input->{email}))
|| (exists $dirty_columns{name}
&& $c->db_user_accounts->active->search({ name => $input->{name} })->single) ) {
$c->stash('response_schema', 'UserError') if $is_system_admin;
return $c->status(409, {
error => 'duplicate user found',
$is_system_admin ? ( user => { map +($_ => $dupe_user->$_), qw(id email name created deactivated) } ) : (),
Expand Down Expand Up @@ -447,6 +449,7 @@ sub create ($c) {

if (my $dupe_user = $c->db_user_accounts->active->search({ name => $input->{name} })->single
|| $c->db_user_accounts->active->find_by_email($input->{email})) {
$c->stash('response_schema', 'UserError');
return $c->status(409, {
error => 'duplicate user found',
user => { map +($_ => $dupe_user->$_), qw(id email name created deactivated) },
Expand Down Expand Up @@ -508,6 +511,7 @@ sub deactivate ($c) {
->order_by($type.'.name');

if (my $thing = $rs->rows(1)->one_row) {
$c->stash('response_schema', 'UserError');
return $c->status(409, {
error => 'user is the only admin of the "'.$thing->name.'" '.$type.' ('.$thing->id.')',
user => { map +($_ => $user->$_), qw(id email name created deactivated) },
Expand Down
83 changes: 81 additions & 2 deletions lib/Conch/Plugin/JSONValidator.pm
Expand Up @@ -7,7 +7,7 @@ use JSON::Schema::Draft201909 '0.017';
use YAML::PP;
use Mojo::JSON 'to_json';
use Path::Tiny;
use List::Util qw(any none);
use List::Util qw(any none first);

=pod
Expand Down Expand Up @@ -74,6 +74,7 @@ Returns a boolean.
if (not $result) {
my @errors = $c->normalize_evaluation_result($result);
$c->log->warn("FAILED query_params validation for schema $schema_name: ".to_json(\@errors));
$c->stash('response_schema', 'QueryParamsValidationError');
return $c->status(400, {
error => 'query parameters did not match required format',
data => $data,
Expand Down Expand Up @@ -112,6 +113,7 @@ Returns a boolean.
if (not $result) {
my @errors = $c->normalize_evaluation_result($result);
$c->log->warn("FAILED request payload validation for schema $schema_name: ".to_json(\@errors));
$c->stash('response_schema', 'RequestValidationError');
return $c->status(400, {
error => 'request did not match required format',
details => \@errors,
Expand Down Expand Up @@ -234,8 +236,85 @@ C<POST>, C<PUT>, C<DELETE> requests)
}

return $next->();
});

=head2 after_dispatch
Runs after dispatching is complete.
Performs more checks when this L<Conch::Plugin::Features/feature> is enabled:
=over 4
=item * C<validate_all_responses>
When not provided, assumes the response body schema is F<response.yaml#/$defs/Null>
(for all 2xx responses), or F<response.yaml#/$defs/Error> (for 4xx responses).
=back
=cut

$app->hook(after_dispatch => sub ($c) {
return if ($c->res->headers->content_type // '')
!~ m{application/(?:schema(?:-instance)?\+)?json}i;

return if not $c->feature('validate_all_responses');

my $res_code = $c->res->code;
return if not ($res_code >= 200 and $res_code < 300)
and not ($res_code >= 400 and $res_code < 500);

my $response_schema = $c->stash('response_schema');
$response_schema = 'Error'
if (not defined $response_schema or $response_schema !~ /Error$/)
and $res_code >= 400 and $res_code < 500 and $c->res->json;
$response_schema //= 'Null';
$c->log->fatal('failed to specify exact response_schema used') if ref $response_schema;

my $validator = $c->json_schema_validator;
my $schema = !ref($response_schema)
? ($response_schema =~ /^http/ ? $response_schema
: 'response.yaml#/$defs/'.$response_schema)
: ref($response_schema) ne 'ARRAY' ? $response_schema
: { anyOf => [ map +{ '$ref' => 'response.yaml#/$defs/'.$_ }, $response_schema->@* ] };

# we don't track successes or failures when multiple schemas are provided, because we
# don't know which schema is intended to match

my @errors;
$c->stash('response_validation_errors', { $response_schema => \@errors })
if not ref $response_schema;
my $schema_description = ref($response_schema) eq 'ARRAY'
? 'anyOf: ['.join(',',$response_schema->@*).']'
: $response_schema;

if (not (my $result = $validator->evaluate($c->res->json, $schema))) {
@errors = $c->normalize_evaluation_result($result);

if (my $notfound = first { $_->{error} =~ /EXCEPTION: unable to find resource / } @errors) {
$c->log->fatal($notfound->{error} =~ s/^EXCEPTION: //r);
return;
}

my $level = $INC{'Test/Conch.pm'} ? 'fatal' : 'warn';
$c->log->$level('FAILED response payload validation for schema '.$schema_description.': '
.to_json(\@errors));

if ($c->feature('rollbar')) {
my $endpoint = join '#', map $_//'', ($c->match->stack->[-1]//{})->@{qw(controller action)};
$c->send_message_to_rollbar(
'error',
'failed response payload validation for schema '.$schema_description,
{ endpoint => $endpoint, url => $c->req->url, errors => \@errors },
[ 'response schema validation failed', $endpoint ],
);
}
return;
}

# TODO: we can also validate the response against its schema.
$c->log->debug('Passed data validation for response schema '.$schema_description);
return;
});
}

Expand Down
8 changes: 4 additions & 4 deletions lib/Conch/Route.pm
Expand Up @@ -113,16 +113,16 @@ Returns the root node.
$root->add_type(json_pointer_token => qr{[A-Za-z0-9_-]+});

# GET /ping
$root->get('/ping', sub ($c) { $c->status(200, { status => 'ok' }) });
$root->get('/ping', { response_schema => 'Ping' }, sub ($c) { $c->status(200, { status => 'ok' }) });

# GET /version
$root->get('/version', sub ($c) {
$root->get('/version', { response_schema => 'Version' }, sub ($c) {
$c->res->headers->last_modified(Mojo::Date->new($c->startup_time->epoch));
$c->status(200, { version => $c->version_tag })
});

# POST /login
$root->post('/login')->to('login#login', request_schema => 'Login');
$root->post('/login')->to('login#login', request_schema => 'Login', response_schema => 'LoginToken');

# * /json_schema/...
Conch::Route::JSONSchema->unsecured_routes($root->any('/json_schema'));
Expand All @@ -138,7 +138,7 @@ Returns the root node.
$secured->get('/me', sub ($c) { $c->status(204) });

# POST /refresh_token
$secured->post('/refresh_token')->to('login#refresh_token', request_schema => 'Null');
$secured->post('/refresh_token')->to('login#refresh_token', request_schema => 'Null', response_schema => 'LoginToken');

Conch::Route::Device->routes($secured->any('/device'), $app);
Conch::Route::DeviceReport->routes($secured->any('/device_report'));
Expand Down
17 changes: 10 additions & 7 deletions lib/Conch/Route/Build.pm
Expand Up @@ -23,7 +23,7 @@ sub routes {
$build->to({ controller => 'build' });

# GET /build
$build->get('/')->to('#get_all', query_params_schema => 'GetBuilds');
$build->get('/')->to('#get_all', query_params_schema => 'GetBuilds', response_schema => 'Builds');

# POST /build
$build->require_system_admin->post('/')->to('#create', request_schema => 'BuildCreate');
Expand All @@ -41,7 +41,7 @@ sub routes {
->to('#find_build', require_role => 'admin');

# GET /build/:build_id_or_name
$with_build_ro->get('/')->to('#get', query_params_schema => 'GetBuild');
$with_build_ro->get('/')->to('#get', query_params_schema => 'GetBuild', response_schema => 'Build');

# POST /build/:build_id_or_name
$with_build_admin->post('/')->to('#update', request_schema => 'BuildUpdate');
Expand All @@ -53,7 +53,7 @@ sub routes {
$with_build_admin->delete('/links')->to('#remove_links', request_schema => 'BuildLinksOrNull');

# GET /build/:build_id_or_name/user
$with_build_admin->get('/user')->to('#get_users');
$with_build_admin->get('/user')->to('#get_users', response_schema => 'BuildUsers');

# POST /build/:build_id_or_name/user?send_mail=<1|0>
$with_build_admin->find_user_from_payload->post('/user')
Expand All @@ -69,7 +69,7 @@ sub routes {
my $build_organization = $with_build_admin->any('/organization');

# GET /build/:build_id_or_name/organization
$build_organization->get('/')->to('#get_organizations');
$build_organization->get('/')->to('#get_organizations', response_schema => 'BuildOrganizations');

# POST /build/:build_id_or_name/organization?send_mail=<1|0>
$build_organization->post('/')
Expand All @@ -86,10 +86,12 @@ sub routes {
->to('#find_devices', query_params_schema => 'FindDevice');

# GET /build/:build_id_or_name/device
$build_devices->get('/')->to('#get_devices', query_params_schema => 'BuildDevices');
$build_devices->get('/')
->to('#get_devices', query_params_schema => 'BuildDevices',
response_schema => [ qw(Devices DeviceIds DeviceSerials) ]);

# GET /build/:build_id_or_name/device/pxe
$build_devices->get('/pxe')->to('#get_pxe_devices');
$build_devices->get('/pxe')->to('#get_pxe_devices', response_schema => 'DevicePXEs');

# POST /build/:build_id_or_name/device
$with_build_rw->post('/device')->to('#create_and_add_devices', require_role => 'rw',
Expand All @@ -106,7 +108,8 @@ sub routes {
->delete('/')->to('build#remove_device');

# GET /build/:build_id_or_name/rack
$with_build_ro->get('/rack')->to('#get_racks', query_params_schema => 'BuildRacks');
$with_build_ro->get('/rack')->to('#get_racks',
query_params_schema => 'BuildRacks', response_schema => [ qw(Racks RackIds) ]);

# POST /build/:build_id_or_name/rack/:rack_id_or_name
$with_build_rw->under('/rack/:rack_id_or_name')
Expand Down
6 changes: 3 additions & 3 deletions lib/Conch/Route/Datacenter.pm
Expand Up @@ -23,20 +23,20 @@ sub routes {
$dc = $dc->require_system_admin->to({ controller => 'datacenter' });

# GET /dc
$dc->get('/')->to('#get_all');
$dc->get('/')->to('#get_all', response_schema => 'Datacenters');
# POST /dc
$dc->post('/')->to('#create', request_schema => 'DatacenterCreate');

my $with_datacenter = $dc->under('/<datacenter_id:uuid>')->to('#find_datacenter');

# GET /dc/:datacenter_id
$with_datacenter->get('/')->to('#get_one');
$with_datacenter->get('/')->to('#get_one', response_schema => 'Datacenter');
# POST /dc/:datacenter_id
$with_datacenter->post('/')->to('#update', request_schema => 'DatacenterUpdate');
# DELETE /dc/:datacenter_id
$with_datacenter->delete('/')->to('#delete');
# GET /dc/:datacenter_id/rooms
$with_datacenter->get('/rooms')->to('#get_rooms');
$with_datacenter->get('/rooms')->to('#get_rooms', response_schema => 'DatacenterRoomsDetailed');
}

1;
Expand Down
6 changes: 3 additions & 3 deletions lib/Conch/Route/DatacenterRoom.pm
Expand Up @@ -25,7 +25,7 @@ sub routes {
my $room_with_system_admin = $room->require_system_admin;

# GET /room
$room_with_system_admin->get('/')->to('#get_all');
$room_with_system_admin->get('/')->to('#get_all', response_schema => 'DatacenterRoomsDetailed');
# POST /room
$room_with_system_admin->post('/')->to('#create', request_schema => 'DatacenterRoomCreate');

Expand All @@ -37,15 +37,15 @@ sub routes {
->to('#find_datacenter_room');

# GET /room/:datacenter_room_id_or_alias
$with_datacenter_room_ro->get('/')->to('#get_one');
$with_datacenter_room_ro->get('/')->to('#get_one', response_schema => 'DatacenterRoomDetailed');
# POST /room/:datacenter_room_id_or_alias
$with_datacenter_room_system_admin->post('/')->to('#update', request_schema => 'DatacenterRoomUpdate');
# DELETE /room/:datacenter_room_id_or_alias
$with_datacenter_room_system_admin->delete('/')->to('#delete');

# GET /room/:datacenter_room_id_or_alias/rack
$with_datacenter_room_ro->get('/racks', sub { shift->status(308, 'get_room_racks') });
$with_datacenter_room_ro->get('/rack', 'get_room_racks')->to('#racks');
$with_datacenter_room_ro->get('/rack', 'get_room_racks')->to('#racks', response_schema => 'Racks');

# GET /room/:datacenter_room_id_or_alias/rack/:rack_id_or_name
# POST /room/:datacenter_room_id_or_alias/rack/:rack_id_or_name
Expand Down

0 comments on commit 5ef663c

Please sign in to comment.