From be561a1da8896cbc1b568393c4f2c9f14ec5a348 Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Tue, 19 Nov 2019 08:56:42 -0800 Subject: [PATCH 01/15] more complete endpoint data for large/long message alerts --- lib/Conch.pm | 12 ++++++++++-- t/conch-rollbar.t | 18 ++++++++++-------- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/lib/Conch.pm b/lib/Conch.pm index 152548a69..c15c6a029 100644 --- a/lib/Conch.pm +++ b/lib/Conch.pm @@ -66,7 +66,11 @@ sub startup { $c->send_message_to_rollbar( 'info', 'response payload contains many elements: candidate for paging?', - { elements => scalar $args->{json}->@*, action => $c->stash('action'), url => $c->url_for }, + { + elements => scalar $args->{json}->@*, + endpoint => join('#', map $_//'', $c->match->stack->[-1]->@{qw(controller action)}), + url => $c->url_for, + }, ) if ref $args->{json} eq 'ARRAY' # TODO: skip if ?page_size is passed (and we actually used it). @@ -79,7 +83,11 @@ sub startup { $c->send_message_to_rollbar( 'info', 'response payload size is large: candidate for paging or refactoring?', - { bytes => $body_size, action => $c->stash('action'), url => $c->url_for }, + { + bytes => $body_size, + endpoint => join('#', map $_//'', $c->match->stack->[-1]->@{qw(controller action)}), + url => $c->url_for, + }, ) if $body_size >= ($c->app->config->{rollbar}{warn_payload_size} // 10000) and $c->feature('rollbar'); diff --git a/t/conch-rollbar.t b/t/conch-rollbar.t index 717f04ad9..b65b392d7 100644 --- a/t/conch-rollbar.t +++ b/t/conch-rollbar.t @@ -50,12 +50,8 @@ $r->post('/_send_message')->to(cb => sub ($c) { ); $c->status(204); }); -$r->get('/_long_response')->to(cb => sub ($c) { - $c->status(200, [ 0..4 ]); -}); -$r->get('/_large_response')->to(cb => sub ($c) { - $c->status(200, { 10..59 }); -}); +$r->get('/_long_response')->to('yo_momma#long_response'); +$r->get('/_large_response')->to('yo_momma#large_response'); $r->post('/_conflict')->to('user#conflict'); $t->add_routes($r); @@ -72,6 +68,12 @@ package Conch::Controller::User { } } +package Conch::Controller::YoMomma { + use Mojo::Base 'Mojolicious::Controller', -signatures; + sub long_response ($c) { $c->status(200, [ 0..4 ]); } + sub large_response ($c) { $c->status(200, { 10..59 }); } +} + package RollbarSimulator { use Conch::UUID 'create_uuid_str'; use Mojo::Base 'Mojolicious', -signatures; @@ -547,7 +549,7 @@ $t->do_and_wait_for_event( message => { body => 'response payload contains many elements: candidate for paging?', elements => 5, - action => undef, + endpoint => 'yo_momma#long_response', url => '/_long_response', }, }, @@ -586,7 +588,7 @@ $t->do_and_wait_for_event( message => { body => 'response payload size is large: candidate for paging or refactoring?', bytes => 201, - action => undef, + endpoint => 'yo_momma#large_response', url => '/_large_response', }, }, From 8c06ad2d505b0d6d4834df9f12c86bfd23b2a148 Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Tue, 19 Nov 2019 09:44:40 -0800 Subject: [PATCH 02/15] improve the occurrence grouping mechanism for long/large message alerts --- docs/modules/Conch::Plugin::Rollbar.md | 4 ++- lib/Conch.pm | 42 ++++++++++++-------------- lib/Conch/Plugin/Rollbar.pm | 15 ++++----- t/conch-rollbar.t | 38 ++++++++++++++++------- 4 files changed, 58 insertions(+), 41 deletions(-) diff --git a/docs/modules/Conch::Plugin::Rollbar.md b/docs/modules/Conch::Plugin::Rollbar.md index eba527307..da4e78b78 100644 --- a/docs/modules/Conch::Plugin::Rollbar.md +++ b/docs/modules/Conch::Plugin::Rollbar.md @@ -40,7 +40,9 @@ thus created. Asynchronously send a message to Rollbar (if the `rollbar` `access_token` is configured). Returns a unique uuid suitable for logging, to correlate with the Rollbar entry thus created. -Requires a message string. A hashref of additional data is optional. +Requires a message string. +A hashref of additional data is optional. +A string or data structure of fingerprint data for grouping occurrences is optional. # LICENSING diff --git a/lib/Conch.pm b/lib/Conch.pm index c15c6a029..5bd01815e 100644 --- a/lib/Conch.pm +++ b/lib/Conch.pm @@ -63,34 +63,32 @@ sub startup { } } - $c->send_message_to_rollbar( - 'info', - 'response payload contains many elements: candidate for paging?', - { - elements => scalar $args->{json}->@*, - endpoint => join('#', map $_//'', $c->match->stack->[-1]->@{qw(controller action)}), - url => $c->url_for, - }, - ) - if ref $args->{json} eq 'ARRAY' + if (ref $args->{json} eq 'ARRAY' # TODO: skip if ?page_size is passed (and we actually used it). and $args->{json}->@* >= ($c->app->config->{rollbar}{warn_payload_elements} // 35) - and $c->feature('rollbar'); + and $c->feature('rollbar')) { + my $endpoint = join '#', map $_//'', ($c->match->stack->[-1]//{})->@{qw(controller action)}; + $c->send_message_to_rollbar( + 'info', + 'response payload contains many elements: candidate for paging?', + { elements => scalar $args->{json}->@*, endpoint => $endpoint, url => $c->url_for }, + [ 'response payload size is large', $endpoint ], + ); + } # do this after the response has been sent $c->on(finish => sub ($c) { my $body_size = $c->res->body_size; - $c->send_message_to_rollbar( - 'info', - 'response payload size is large: candidate for paging or refactoring?', - { - bytes => $body_size, - endpoint => join('#', map $_//'', $c->match->stack->[-1]->@{qw(controller action)}), - url => $c->url_for, - }, - ) - if $body_size >= ($c->app->config->{rollbar}{warn_payload_size} // 10000) - and $c->feature('rollbar'); + if ($body_size >= ($c->app->config->{rollbar}{warn_payload_size} // 10000) + and $c->feature('rollbar')) { + my $endpoint = join '#', map $_//'', ($c->match->stack->[-1]//{})->@{qw(controller action)}; + $c->send_message_to_rollbar( + 'info', + 'response payload size is large: candidate for paging or refactoring?', + { bytes => $body_size, endpoint => $endpoint, url => $c->url_for }, + [ 'response payload size is large', $endpoint ], + ); + } }); }); diff --git a/lib/Conch/Plugin/Rollbar.pm b/lib/Conch/Plugin/Rollbar.pm index 9258dd1d1..0ac10a529 100644 --- a/lib/Conch/Plugin/Rollbar.pm +++ b/lib/Conch/Plugin/Rollbar.pm @@ -7,7 +7,7 @@ use Conch::UUID 'create_uuid_str'; use WebService::Rollbar::Notifier; use Digest::SHA (); use Config; -use Mojo::JSON 'to_json'; +use Mojo::JSON 'encode_json'; use List::Util qw(none any); use Carp; use Storable 'dclone'; @@ -151,11 +151,13 @@ thus created. Asynchronously send a message to Rollbar (if the C C is configured). Returns a unique uuid suitable for logging, to correlate with the Rollbar entry thus created. -Requires a message string. A hashref of additional data is optional. +Requires a message string. +A hashref of additional data is optional. +A string or data structure of fingerprint data for grouping occurrences is optional. =cut - $app->helper(send_message_to_rollbar => sub ($c, $severity, $message_string, $payload = {}) { + $app->helper(send_message_to_rollbar => sub ($c, $severity, $message_string, $payload = {}, $fingerprint = undef) { Carp::croak('severity must be one of: '.join(', ',@message_levels)) if !$ENV{MOJO_MODE} and none { $severity eq $_ } @message_levels; @@ -165,10 +167,9 @@ Requires a message string. A hashref of additional data is optional. my $rollbar_id = create_uuid_str(); # see https://docs.rollbar.com/docs/grouping-algorithm - my $fingerprint = join(',', - $message_string, - $payload ? to_json($payload) : (), - ); + # optionally use provided data to calculate the fingerprint + $fingerprint //= [ $message_string, $payload ]; + $fingerprint = encode_json($fingerprint) if ref $fingerprint; $fingerprint = Digest::SHA::sha1_hex($fingerprint) if length($fingerprint) > 40; # asynchronously post to Rollbar, logging if the request fails diff --git a/t/conch-rollbar.t b/t/conch-rollbar.t index b65b392d7..53a9ad649 100644 --- a/t/conch-rollbar.t +++ b/t/conch-rollbar.t @@ -28,7 +28,7 @@ my $t = Test::Conch->new( environment => 'custom_environment', error_match_header => { 'My-Buggy-Client' => qr/^1\.[0-9]$/ }, warn_payload_elements => 5, - warn_payload_size => 200, + warn_payload_size => 100, }, logging => { handle => $log_fh }, }, @@ -70,8 +70,12 @@ package Conch::Controller::User { package Conch::Controller::YoMomma { use Mojo::Base 'Mojolicious::Controller', -signatures; - sub long_response ($c) { $c->status(200, [ 0..4 ]); } - sub large_response ($c) { $c->status(200, { 10..59 }); } + sub long_response ($c) { $c->status(200, [ 1..$c->req->query_params->to_hash->{elements} ]) } + sub large_response ($c) { + my $elements = $c->req->query_params->to_hash->{elements}; + my %hash; @hash{map chr(ord(0)+$_), 0..$elements-1} = (0)x$elements; + $c->status(200, \%hash); + } } package RollbarSimulator { @@ -519,10 +523,13 @@ foreach my $request ( ); } +my %fingerprints; + +foreach my $elements (5, 10) { $t->do_and_wait_for_event( $rollbar_app->plugins, 'rollbar_sent', sub ($t) { - $t->get_ok('/_long_response') + $t->get_ok('/_long_response?elements='.$elements) ->status_is(200); }, sub ($payload) { @@ -537,7 +544,7 @@ $t->do_and_wait_for_event( superhashof({ method => 'GET', url => re(qr{/_long_response}), - query_string => '', + query_string => 'elements='.$elements, body => '', }), 'request details are included', @@ -548,20 +555,25 @@ $t->do_and_wait_for_event( { message => { body => 'response payload contains many elements: candidate for paging?', - elements => 5, + elements => $elements, endpoint => 'yo_momma#long_response', url => '/_long_response', }, }, 'got alert about long response', ); + + push $fingerprints{long_response}->@*, $payload->{data}{fingerprint}; }, -); +); } +is($fingerprints{long_response}->[0], $fingerprints{long_response}->[1], 'the two fingerprints are identical'); + +foreach my $elements (40, 44) { $t->do_and_wait_for_event( $rollbar_app->plugins, 'rollbar_sent', sub ($t) { - $t->get_ok('/_large_response') + $t->get_ok('/_large_response?elements='.$elements) ->status_is(200); }, sub ($payload) { @@ -576,7 +588,7 @@ $t->do_and_wait_for_event( superhashof({ method => 'GET', url => re(qr{/_large_response}), - query_string => '', + query_string => 'elements='.$elements, body => '', }), 'request details are included', @@ -587,15 +599,19 @@ $t->do_and_wait_for_event( { message => { body => 'response payload size is large: candidate for paging or refactoring?', - bytes => 201, + bytes => 6*$elements + 1, endpoint => 'yo_momma#large_response', url => '/_large_response', }, }, 'got alert about large response', ); + + push $fingerprints{large_response}->@*, $payload->{data}{fingerprint}; }, -); +); } + +is($fingerprints{large_response}->[0], $fingerprints{large_response}->[1], 'the two fingerprints are identical'); warnings(sub { memory_cycle_ok($t, 'no leaks in the Test::Conch object'); From 1c60d08156a71d305c7288c7f27d112f553adc9d Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Thu, 21 Nov 2019 15:41:17 -0800 Subject: [PATCH 03/15] explicitly return a value from chained action, rather than evaluating the resultset --- lib/Conch/Controller/Build.pm | 1 + lib/Conch/Controller/Organization.pm | 1 + 2 files changed, 2 insertions(+) diff --git a/lib/Conch/Controller/Build.pm b/lib/Conch/Controller/Build.pm index 9e1664cc6..a2b73714a 100644 --- a/lib/Conch/Controller/Build.pm +++ b/lib/Conch/Controller/Build.pm @@ -136,6 +136,7 @@ sub find_build ($c) { } $c->stash('build_rs', $rs); + return 1; } =head2 get diff --git a/lib/Conch/Controller/Organization.pm b/lib/Conch/Controller/Organization.pm index 6375152fd..f70cfe9cb 100644 --- a/lib/Conch/Controller/Organization.pm +++ b/lib/Conch/Controller/Organization.pm @@ -133,6 +133,7 @@ sub find_organization ($c) { } $c->stash('organization_rs', $rs); + return 1; } =head2 get From a0b9c8c9520ada970ce6775ca54617ee15104c2e Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Wed, 20 Nov 2019 13:01:28 -0800 Subject: [PATCH 04/15] clarify comment --- t/integration/crud/build.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/integration/crud/build.t b/t/integration/crud/build.t index 85708a094..ee5fe8b9e 100644 --- a/t/integration/crud/build.t +++ b/t/integration/crud/build.t @@ -1003,7 +1003,7 @@ $t->get_ok('/build/my first build/device') ->status_is(200) ->json_schema_is('Devices') ->json_cmp_deeply([ - # device.phase >= production, so its location is no longer canonical + # device1 phase >= production, so its location is no longer canonical superhashof({ (map +($_ => $device2->$_), qw(id serial_number)), build_id => $build->{id}, From b9cdb932adcf83063e8098c3f162b2bd8b7811e2 Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Wed, 27 Nov 2019 18:01:43 -0800 Subject: [PATCH 05/15] minor doc fixes --- docs/modules/Conch::Command::fix_usernames.md | 8 +++++ .../Conch::Command::force_password_change.md | 8 +++++ .../Conch::Command::workspace_to_build.md | 8 +++++ .../Conch::Controller::HardwareVendor.md | 2 +- docs/modules/Conch::Controller::Rack.md | 4 +-- .../Conch::DB::ResultSet::UserAccount.md | 2 ++ .../Conch::DB::ResultSet::UserSessionToken.md | 2 ++ docs/modules/Conch::Plugin::Features.md | 6 ++-- docs/modules/Conch::Plugin::Mail.md | 4 +-- docs/modules/Conch::Time.md | 14 ++++---- docs/modules/Test::Conch::Validation.md | 2 +- lib/Conch/Command/fix_usernames.pm | 14 ++++++++ lib/Conch/Command/force_password_change.pm | 14 ++++++++ lib/Conch/Command/workspace_to_build.pm | 14 ++++++++ lib/Conch/Controller/HardwareVendor.pm | 2 +- lib/Conch/Controller/Rack.pm | 4 +-- lib/Conch/DB/ResultSet/UserAccount.pm | 2 ++ lib/Conch/DB/ResultSet/UserSessionToken.pm | 2 ++ lib/Conch/Plugin/Features.pm | 6 ++-- lib/Conch/Plugin/Mail.pm | 4 +-- lib/Conch/Time.pm | 32 ++++++++----------- lib/Test/Conch/Validation.pm | 2 +- 22 files changed, 114 insertions(+), 42 deletions(-) diff --git a/docs/modules/Conch::Command::fix_usernames.md b/docs/modules/Conch::Command::fix_usernames.md index 2767682a9..3ab7b535d 100644 --- a/docs/modules/Conch::Command::fix_usernames.md +++ b/docs/modules/Conch::Command::fix_usernames.md @@ -11,3 +11,11 @@ bin/conch fix_usernames [long options...] --help print usage message and exit ``` + +# LICENSING + +Copyright Joyent, Inc. + +This Source Code Form is subject to the terms of the Mozilla Public License, +v.2.0. If a copy of the MPL was not distributed with this file, You can obtain +one at [http://mozilla.org/MPL/2.0/](http://mozilla.org/MPL/2.0/). diff --git a/docs/modules/Conch::Command::force_password_change.md b/docs/modules/Conch::Command::force_password_change.md index ee52e882b..c75d6bece 100644 --- a/docs/modules/Conch::Command::force_password_change.md +++ b/docs/modules/Conch::Command::force_password_change.md @@ -12,3 +12,11 @@ bin/conch force_password_change [long options...] --help print usage message and exit ``` + +# LICENSING + +Copyright Joyent, Inc. + +This Source Code Form is subject to the terms of the Mozilla Public License, +v.2.0. If a copy of the MPL was not distributed with this file, You can obtain +one at [http://mozilla.org/MPL/2.0/](http://mozilla.org/MPL/2.0/). diff --git a/docs/modules/Conch::Command::workspace_to_build.md b/docs/modules/Conch::Command::workspace_to_build.md index d7f15fd7e..e9cc5c75b 100644 --- a/docs/modules/Conch::Command::workspace_to_build.md +++ b/docs/modules/Conch::Command::workspace_to_build.md @@ -11,3 +11,11 @@ bin/conch workspace_to_build [long options...] [workspace name] --help print usage message and exit ``` + +# LICENSING + +Copyright Joyent, Inc. + +This Source Code Form is subject to the terms of the Mozilla Public License, +v.2.0. If a copy of the MPL was not distributed with this file, You can obtain +one at [http://mozilla.org/MPL/2.0/](http://mozilla.org/MPL/2.0/). diff --git a/docs/modules/Conch::Controller::HardwareVendor.md b/docs/modules/Conch::Controller::HardwareVendor.md index c5e9042e4..50a0f59fb 100644 --- a/docs/modules/Conch::Controller::HardwareVendor.md +++ b/docs/modules/Conch::Controller::HardwareVendor.md @@ -1,6 +1,6 @@ # NAME -Conch::Controller::User +Conch::Controller::HardwareVendor # METHODS diff --git a/docs/modules/Conch::Controller::Rack.md b/docs/modules/Conch::Controller::Rack.md index f4843f009..3b8ba600b 100644 --- a/docs/modules/Conch::Controller::Rack.md +++ b/docs/modules/Conch::Controller::Rack.md @@ -55,8 +55,8 @@ Assigns devices to rack layouts, also optionally updating serial\_numbers and as creating the device if needed). Existing devices in referenced slots will be unassigned as needed. Note: the assignment is still performed even if there is no physical room in the rack -for the new hardware (its rack\_unit\_size overlaps into a subsequent layout), or that -the device's hardware matches what the layout specifies. +for the new hardware (its rack\_unit\_size overlaps into a subsequent layout), or if the device's +hardware doesn't match what the layout specifies. ## delete\_assignment diff --git a/docs/modules/Conch::DB::ResultSet::UserAccount.md b/docs/modules/Conch::DB::ResultSet::UserAccount.md index 2355d3dbf..c451790ce 100644 --- a/docs/modules/Conch::DB::ResultSet::UserAccount.md +++ b/docs/modules/Conch::DB::ResultSet::UserAccount.md @@ -6,6 +6,8 @@ Conch::DB::ResultSet::UserAccount Interface to queries against the `user_account` table. +# METHODS + ## find\_by\_email Queries for user by (case-insensitive) email address. diff --git a/docs/modules/Conch::DB::ResultSet::UserSessionToken.md b/docs/modules/Conch::DB::ResultSet::UserSessionToken.md index 134d18066..19ee2ab0a 100644 --- a/docs/modules/Conch::DB::ResultSet::UserSessionToken.md +++ b/docs/modules/Conch::DB::ResultSet::UserSessionToken.md @@ -6,6 +6,8 @@ Conch::DB::ResultSet::UserSessionToken Interface to queries against the 'user\_session\_token' table. +# METHODS + ## expired Chainable resultset to limit results to those that are expired. diff --git a/docs/modules/Conch::Plugin::Features.md b/docs/modules/Conch::Plugin::Features.md index 8a352a214..ba5295558 100644 --- a/docs/modules/Conch::Plugin::Features.md +++ b/docs/modules/Conch::Plugin::Features.md @@ -2,9 +2,11 @@ Conch::Plugin::Features - Sets up a helper to access configured features -## DESCRIPTION +# HELPERS -Provides the helper sub 'feature' to the app and controllers: +## feature + +Checks if a given feature name is enabled. ``` if ($c->feature('rollbar') { ... } diff --git a/docs/modules/Conch::Plugin::Mail.md b/docs/modules/Conch::Plugin::Mail.md index 176824082..da6d8872b 100644 --- a/docs/modules/Conch::Plugin::Mail.md +++ b/docs/modules/Conch::Plugin::Mail.md @@ -2,11 +2,11 @@ Conch::Plugin::Mail -## DESCRIPTION +# DESCRIPTION Helper methods for sending emails -## HELPERS +# HELPERS These methods are made available on the `$c` object (the invocant of all controller methods, and therefore other helpers). diff --git a/docs/modules/Conch::Time.md b/docs/modules/Conch::Time.md index ef1c8cbb8..ad40a38f5 100644 --- a/docs/modules/Conch::Time.md +++ b/docs/modules/Conch::Time.md @@ -48,31 +48,29 @@ Conch::Time->from_epoch(1234567890, 123); See ["from\_epoch" in Time::Moment](https://metacpan.org/pod/Time%3A%3AMoment#from_epoch). -## CONVERSIONS - -### rfc3339 +## rfc3339 Return an RFC3339 compatible string as UTC. Sub-second precision will use 3, 6 or 9 digits as necessary. -### timestamp +## timestamp Return an RFC3339 compatible string. -### to\_string +## to\_string Render the timestamp as a RFC 3339 timestamp string. Used to overload string coercion. -### TO\_JSON +## TO\_JSON Renderer for Mojo, as a RFC 3339 timestamp string -### timestamptz +## timestamptz Render a string in PostgreSQL's timestamptz style -### iso8601 +## iso8601 Render the timestamp as an ISO8601 extended format, in UTC diff --git a/docs/modules/Test::Conch::Validation.md b/docs/modules/Test::Conch::Validation.md index e42b5673c..fb67bff84 100644 --- a/docs/modules/Test::Conch::Validation.md +++ b/docs/modules/Test::Conch::Validation.md @@ -2,7 +2,7 @@ Test::Conch::Validation - Test Conch Validations -## METHODS +# EXPORTABLE FUNCTIONS ## test\_validation diff --git a/lib/Conch/Command/fix_usernames.pm b/lib/Conch/Command/fix_usernames.pm index 90c5bb326..f5e5d1bc4 100644 --- a/lib/Conch/Command/fix_usernames.pm +++ b/lib/Conch/Command/fix_usernames.pm @@ -62,3 +62,17 @@ sub run ($self, @opts) { } 1; +__END__ + +=pod + +=head1 LICENSING + +Copyright Joyent, Inc. + +This Source Code Form is subject to the terms of the Mozilla Public License, +v.2.0. If a copy of the MPL was not distributed with this file, You can obtain +one at L. + +=cut +# vim: set ts=4 sts=4 sw=4 et : diff --git a/lib/Conch/Command/force_password_change.pm b/lib/Conch/Command/force_password_change.pm index c183ad761..411653559 100644 --- a/lib/Conch/Command/force_password_change.pm +++ b/lib/Conch/Command/force_password_change.pm @@ -56,3 +56,17 @@ sub run ($self, @opts) { } 1; +__END__ + +=pod + +=head1 LICENSING + +Copyright Joyent, Inc. + +This Source Code Form is subject to the terms of the Mozilla Public License, +v.2.0. If a copy of the MPL was not distributed with this file, You can obtain +one at L. + +=cut +# vim: set ts=4 sts=4 sw=4 et : diff --git a/lib/Conch/Command/workspace_to_build.pm b/lib/Conch/Command/workspace_to_build.pm index 8610d4755..33cf5c3fc 100644 --- a/lib/Conch/Command/workspace_to_build.pm +++ b/lib/Conch/Command/workspace_to_build.pm @@ -131,3 +131,17 @@ sub run ($self, @opts) { } 1; +__END__ + +=pod + +=head1 LICENSING + +Copyright Joyent, Inc. + +This Source Code Form is subject to the terms of the Mozilla Public License, +v.2.0. If a copy of the MPL was not distributed with this file, You can obtain +one at L. + +=cut +# vim: set ts=4 sts=4 sw=4 et : diff --git a/lib/Conch/Controller/HardwareVendor.pm b/lib/Conch/Controller/HardwareVendor.pm index 1630d87a6..c368b0b6e 100644 --- a/lib/Conch/Controller/HardwareVendor.pm +++ b/lib/Conch/Controller/HardwareVendor.pm @@ -8,7 +8,7 @@ use Conch::UUID 'is_uuid'; =head1 NAME -Conch::Controller::User +Conch::Controller::HardwareVendor =head1 METHODS diff --git a/lib/Conch/Controller/Rack.pm b/lib/Conch/Controller/Rack.pm index 256b8614f..f405d9d80 100644 --- a/lib/Conch/Controller/Rack.pm +++ b/lib/Conch/Controller/Rack.pm @@ -313,8 +313,8 @@ Assigns devices to rack layouts, also optionally updating serial_numbers and ass creating the device if needed). Existing devices in referenced slots will be unassigned as needed. Note: the assignment is still performed even if there is no physical room in the rack -for the new hardware (its rack_unit_size overlaps into a subsequent layout), or that -the device's hardware matches what the layout specifies. +for the new hardware (its rack_unit_size overlaps into a subsequent layout), or if the device's +hardware doesn't match what the layout specifies. =cut diff --git a/lib/Conch/DB/ResultSet/UserAccount.pm b/lib/Conch/DB/ResultSet/UserAccount.pm index f053fc48b..14897cc89 100644 --- a/lib/Conch/DB/ResultSet/UserAccount.pm +++ b/lib/Conch/DB/ResultSet/UserAccount.pm @@ -13,6 +13,8 @@ Conch::DB::ResultSet::UserAccount Interface to queries against the C table. +=head1 METHODS + =head2 find_by_email Queries for user by (case-insensitive) email address. diff --git a/lib/Conch/DB/ResultSet/UserSessionToken.pm b/lib/Conch/DB/ResultSet/UserSessionToken.pm index 071b8de89..8ee2dccdf 100644 --- a/lib/Conch/DB/ResultSet/UserSessionToken.pm +++ b/lib/Conch/DB/ResultSet/UserSessionToken.pm @@ -13,6 +13,8 @@ Conch::DB::ResultSet::UserSessionToken Interface to queries against the 'user_session_token' table. +=head1 METHODS + =head2 expired Chainable resultset to limit results to those that are expired. diff --git a/lib/Conch/Plugin/Features.pm b/lib/Conch/Plugin/Features.pm index d4c3f8146..6625a122f 100644 --- a/lib/Conch/Plugin/Features.pm +++ b/lib/Conch/Plugin/Features.pm @@ -9,9 +9,11 @@ use Mojo::Base 'Mojolicious::Plugin', -signatures; Conch::Plugin::Features - Sets up a helper to access configured features -=head2 DESCRIPTION +=head1 HELPERS -Provides the helper sub 'feature' to the app and controllers: +=head2 feature + +Checks if a given feature name is enabled. if ($c->feature('rollbar') { ... } diff --git a/lib/Conch/Plugin/Mail.pm b/lib/Conch/Plugin/Mail.pm index e1521618d..f85e618f8 100644 --- a/lib/Conch/Plugin/Mail.pm +++ b/lib/Conch/Plugin/Mail.pm @@ -16,11 +16,11 @@ use Safe::Isa; Conch::Plugin::Mail -=head2 DESCRIPTION +=head1 DESCRIPTION Helper methods for sending emails -=head2 HELPERS +=head1 HELPERS These methods are made available on the C<$c> object (the invocant of all controller methods, and therefore other helpers). diff --git a/lib/Conch/Time.pm b/lib/Conch/Time.pm index 51e1cf389..4b1cfb441 100644 --- a/lib/Conch/Time.pm +++ b/lib/Conch/Time.pm @@ -1,3 +1,11 @@ +package Conch::Time; + +use v5.26; +use strict; +use warnings; + +use parent 'Time::Moment'; + =pod =head1 NAME @@ -15,16 +23,6 @@ Conch::Time - format timestamps as RFC 3337 UTC timestamps =head1 METHODS -=cut - -package Conch::Time; - -use v5.26; -use strict; -use warnings; - -use parent 'Time::Moment'; - =head2 new Overloads the constructor to use C<< ->from_string >> when a single argument is passed, for example: @@ -68,9 +66,7 @@ sub now { return shift->now_utc } See L. -=head2 CONVERSIONS - -=head3 rfc3339 +=head2 rfc3339 Return an RFC3339 compatible string as UTC. Sub-second precision will use 3, 6 or 9 digits as necessary. @@ -81,7 +77,7 @@ sub rfc3339 { return shift->at_utc->strftime('%Y-%m-%dT%H:%M:%S.%N%Z'); } -=head3 timestamp +=head2 timestamp Return an RFC3339 compatible string. @@ -89,7 +85,7 @@ Return an RFC3339 compatible string. sub timestamp { goto &rfc3339 } -=head3 to_string +=head2 to_string Render the timestamp as a RFC 3339 timestamp string. Used to overload string coercion. @@ -98,7 +94,7 @@ overload string coercion. sub to_string { goto &rfc3339 } -=head3 TO_JSON +=head2 TO_JSON Renderer for Mojo, as a RFC 3339 timestamp string @@ -106,7 +102,7 @@ Renderer for Mojo, as a RFC 3339 timestamp string sub TO_JSON { goto &rfc3339 } -=head3 timestamptz +=head2 timestamptz Render a string in PostgreSQL's timestamptz style @@ -116,7 +112,7 @@ sub timestamptz { return shift->strftime('%Y-%m-%d %H:%M:%S%f%z'); } -=head3 iso8601 +=head2 iso8601 Render the timestamp as an ISO8601 extended format, in UTC diff --git a/lib/Test/Conch/Validation.pm b/lib/Test/Conch/Validation.pm index 4aa5ab36c..eee1d7156 100644 --- a/lib/Test/Conch/Validation.pm +++ b/lib/Test/Conch/Validation.pm @@ -19,7 +19,7 @@ our @EXPORT_OK = qw(test_validation); Test::Conch::Validation - Test Conch Validations -=head2 METHODS +=head1 EXPORTABLE FUNCTIONS =head2 test_validation From 16574228f8449b3e447f3c12de151dee6147c16e Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Fri, 22 Nov 2019 12:34:50 -0800 Subject: [PATCH 06/15] use faster access method for query params We are not interested in application/x-www-form-urlencoded request bodies, so do not bother parsing for them --- lib/Conch/Plugin/JsonValidator.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Conch/Plugin/JsonValidator.pm b/lib/Conch/Plugin/JsonValidator.pm index c59a1d3e3..2d831f258 100644 --- a/lib/Conch/Plugin/JsonValidator.pm +++ b/lib/Conch/Plugin/JsonValidator.pm @@ -52,7 +52,7 @@ QueryParamsValidationError json response schema. =cut - $app->helper(validate_query_params => sub ($c, $schema_name, $data = $c->req->params->to_hash) { + $app->helper(validate_query_params => sub ($c, $schema_name, $data = $c->req->query_params->to_hash) { my $validator = $c->get_query_params_validator; my $schema = $validator->get('/definitions/'.$schema_name); From 272e950e49f61c025663ab3bcecc25b7e99af0d2 Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Mon, 25 Nov 2019 14:12:16 -0800 Subject: [PATCH 07/15] $args->{exception} is no longer populated, since Mojolicious 7.82 --- lib/Conch/Plugin/Rollbar.pm | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/Conch/Plugin/Rollbar.pm b/lib/Conch/Plugin/Rollbar.pm index 0ac10a529..5f87352c0 100644 --- a/lib/Conch/Plugin/Rollbar.pm +++ b/lib/Conch/Plugin/Rollbar.pm @@ -41,7 +41,6 @@ sub register ($self, $app, $config) { if (my $exception = $c->stash('exception') or ($template and $template =~ /exception/)) { - $exception //= $args->{exception}; my $rollbar_id = $c->send_exception_to_rollbar($exception); $c->log->debug('exception sent to rollbar: id '.$rollbar_id); } From 9c8a8ec686866cf83d4936d5f7618805fdcab921 Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Tue, 26 Nov 2019 09:51:42 -0800 Subject: [PATCH 08/15] ensure strict utf8 checks are used in these tests --- t/database.t | 9 +++++++-- t/lib/Conch/Validation/MutateDevice.pm | 3 +++ t/validation-system/exceptions.t | 2 +- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/t/database.t b/t/database.t index 6943b9b27..4ce2c3ddb 100644 --- a/t/database.t +++ b/t/database.t @@ -1,5 +1,10 @@ -use v5.26; -use Mojo::Base -strict, -signatures; +use strict; +use warnings; +use warnings FATAL => 'utf8'; +use utf8; +use open ':std', ':encoding(UTF-8)'; # force stdin, stdout, stderr into utf8 +use experimental 'signatures'; + use Test::More; use Test::Conch; use Test::Fatal; diff --git a/t/lib/Conch/Validation/MutateDevice.pm b/t/lib/Conch/Validation/MutateDevice.pm index 82152bdea..392935abb 100644 --- a/t/lib/Conch/Validation/MutateDevice.pm +++ b/t/lib/Conch/Validation/MutateDevice.pm @@ -1,5 +1,8 @@ package Conch::Validation::MutateDevice; +use strict; +use warnings; +use warnings FATAL => 'utf8'; use utf8; use Mojo::Base 'Conch::Validation', -signatures; diff --git a/t/validation-system/exceptions.t b/t/validation-system/exceptions.t index d3beda5e4..99e1ac8d2 100644 --- a/t/validation-system/exceptions.t +++ b/t/validation-system/exceptions.t @@ -105,7 +105,7 @@ subtest '->run, blessed external exception containing a stack trace' => sub { status => 'error', message => re(qr/permission denied for relation device/), category => 'exception', - hint => 't/lib/Conch/Validation/MutateDevice.pm line 14', + hint => 't/lib/Conch/Validation/MutateDevice.pm line 17', }, ], 'correctly parsed an exception from an external library containing a stack trace', From cc25dafb45b98fbcf2a561d59453608b251e4cc7 Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Thu, 28 Nov 2019 10:57:27 -0800 Subject: [PATCH 09/15] fix documentation for these endpoints --- docs/modules/Conch::Route::Build.md | 2 +- docs/modules/Conch::Route::Datacenter.md | 2 +- docs/modules/Conch::Route::DatacenterRoom.md | 4 ++-- docs/modules/Conch::Route::Device.md | 2 +- docs/modules/Conch::Route::DeviceReport.md | 2 +- docs/modules/Conch::Route::HardwareProduct.md | 2 +- docs/modules/Conch::Route::HardwareVendor.md | 2 +- docs/modules/Conch::Route::Organization.md | 2 +- docs/modules/Conch::Route::Rack.md | 18 +++++++++--------- docs/modules/Conch::Route::RackLayout.md | 2 +- docs/modules/Conch::Route::RackRole.md | 2 +- docs/modules/Conch::Route::Relay.md | 2 +- docs/modules/Conch::Route::User.md | 2 +- docs/modules/Conch::Route::Validation.md | 2 +- docs/modules/Conch::Route::Workspace.md | 2 +- lib/Conch/Route/Build.pm | 2 +- lib/Conch/Route/Datacenter.pm | 2 +- lib/Conch/Route/DatacenterRoom.pm | 4 ++-- lib/Conch/Route/Device.pm | 2 +- lib/Conch/Route/DeviceReport.pm | 2 +- lib/Conch/Route/HardwareProduct.pm | 2 +- lib/Conch/Route/HardwareVendor.pm | 2 +- lib/Conch/Route/Organization.pm | 2 +- lib/Conch/Route/Rack.pm | 18 +++++++++--------- lib/Conch/Route/RackLayout.pm | 2 +- lib/Conch/Route/RackRole.pm | 2 +- lib/Conch/Route/Relay.pm | 2 +- lib/Conch/Route/User.pm | 2 +- lib/Conch/Route/Validation.pm | 2 +- lib/Conch/Route/Workspace.pm | 2 +- 30 files changed, 48 insertions(+), 48 deletions(-) diff --git a/docs/modules/Conch::Route::Build.md b/docs/modules/Conch::Route::Build.md index 48e1fb3cd..9565d29eb 100644 --- a/docs/modules/Conch::Route::Build.md +++ b/docs/modules/Conch::Route::Build.md @@ -8,7 +8,7 @@ Conch::Route::Build Sets up the routes for /build. -Unless otherwise noted, all routes require authentication. +All routes require authentication. ### `GET /build` diff --git a/docs/modules/Conch::Route::Datacenter.md b/docs/modules/Conch::Route::Datacenter.md index 5b75a1225..eaf793289 100644 --- a/docs/modules/Conch::Route::Datacenter.md +++ b/docs/modules/Conch::Route::Datacenter.md @@ -8,7 +8,7 @@ Conch::Route::Datacenter Sets up the routes for /dc: -Unless otherwise noted, all routes require authentication. +All routes require authentication. ### `GET /dc` diff --git a/docs/modules/Conch::Route::DatacenterRoom.md b/docs/modules/Conch::Route::DatacenterRoom.md index 91c81c480..626c3f068 100644 --- a/docs/modules/Conch::Route::DatacenterRoom.md +++ b/docs/modules/Conch::Route::DatacenterRoom.md @@ -8,7 +8,7 @@ Conch::Route::DatacenterRoom Sets up the routes for /room: -Unless otherwise noted, all routes require authentication. +All routes require authentication. ### `GET /room` @@ -42,7 +42,7 @@ Unless otherwise noted, all routes require authentication. - Requires system admin authorization - Response: [response.json#/definitions/Racks](../json-schema/response.json#/definitions/Racks) -### `GET /room/:datacenter_room_id_or_alias/:rack_name` +### `GET /room/:datacenter_room_id_or_alias/rack/:rack_name` - Requires system admin authorization - Response: [response.json#/definitions/Rack](../json-schema/response.json#/definitions/Rack) diff --git a/docs/modules/Conch::Route::Device.md b/docs/modules/Conch::Route::Device.md index 3a4a89a6e..d2d53315d 100644 --- a/docs/modules/Conch::Route::Device.md +++ b/docs/modules/Conch::Route::Device.md @@ -8,7 +8,7 @@ Conch::Route::Device Sets up the routes for /device: -Unless otherwise noted, all routes require authentication. +All routes require authentication. The user's role (required for most endpoints) is determined by the build the device is contained in (where users are assigned a [role](../modules/Conch%3A%3ADB%3A%3AResult%3A%3AUserBuildRole#role) in that diff --git a/docs/modules/Conch::Route::DeviceReport.md b/docs/modules/Conch::Route::DeviceReport.md index 5fef2ba6d..46d90d683 100644 --- a/docs/modules/Conch::Route::DeviceReport.md +++ b/docs/modules/Conch::Route::DeviceReport.md @@ -8,7 +8,7 @@ Conch::Route::DeviceReport Sets up the routes for /device\_report: -Unless otherwise noted, all routes require authentication. +All routes require authentication. ### `POST /device_report` diff --git a/docs/modules/Conch::Route::HardwareProduct.md b/docs/modules/Conch::Route::HardwareProduct.md index ebc302a6f..f3f42386a 100644 --- a/docs/modules/Conch::Route::HardwareProduct.md +++ b/docs/modules/Conch::Route::HardwareProduct.md @@ -8,7 +8,7 @@ Conch::Route::HardwareProduct Sets up the routes for /hardware\_product: -Unless otherwise noted, all routes require authentication. +All routes require authentication. ### `GET /hardware_product` diff --git a/docs/modules/Conch::Route::HardwareVendor.md b/docs/modules/Conch::Route::HardwareVendor.md index 03284e67b..baf156920 100644 --- a/docs/modules/Conch::Route::HardwareVendor.md +++ b/docs/modules/Conch::Route::HardwareVendor.md @@ -8,7 +8,7 @@ Conch::Route::HardwareVendor Sets up the routes for /hardware\_vendor: -Unless otherwise noted, all routes require authentication. +All routes require authentication. ### `GET /hardware_vendor` diff --git a/docs/modules/Conch::Route::Organization.md b/docs/modules/Conch::Route::Organization.md index a9eb3b8e2..6c0f2b5d2 100644 --- a/docs/modules/Conch::Route::Organization.md +++ b/docs/modules/Conch::Route::Organization.md @@ -8,7 +8,7 @@ Conch::Route::Organization Sets up the routes for /organization. -Unless otherwise noted, all routes require authentication. +All routes require authentication. ### `GET /organization` diff --git a/docs/modules/Conch::Route::Rack.md b/docs/modules/Conch::Route::Rack.md index 48e6e6bb9..6d94796a3 100644 --- a/docs/modules/Conch::Route::Rack.md +++ b/docs/modules/Conch::Route::Rack.md @@ -8,7 +8,7 @@ Conch::Route::Rack Sets up the routes for /rack: -Unless otherwise noted, all routes require authentication. +All routes require authentication. ### `GET /rack` @@ -23,12 +23,12 @@ Unless otherwise noted, all routes require authentication. ### `GET /rack/:rack_id` -- User requires the read-only role on a workspace that contains the rack +- User requires the read-only role on the rack - Response: [response.json#/definitions/Rack](../json-schema/response.json#/definitions/Rack) ### `POST /rack/:rack_id` -- User requires the read/write role on a workspace that contains the rack +- User requires the read/write role on the rack - Request: [request.json#/definitions/RackUpdate](../json-schema/request.json#/definitions/RackUpdate) - Response: Redirect to the updated rack @@ -39,23 +39,23 @@ Unless otherwise noted, all routes require authentication. ### `GET /rack/:rack_id/layouts` -- User requires the read-only role on a workspace that contains the rack +- User requires the read-only role on the rack - Response: [response.json#/definitions/RackLayouts](../json-schema/response.json#/definitions/RackLayouts) ### `POST /rack/:rack_id/layouts` -- User requires the read/write role on a workspace that contains the rack +- User requires the read/write role on the rack - Request: [request.json#/definitions/RackLayouts](../json-schema/request.json#/definitions/RackLayouts) - Response: Redirect to the rack's layouts ### `GET /rack/:rack_id/assignment` -- User requires the read-only role on a workspace that contains the rack +- User requires the read-only role on the rack - Response: [response.json#/definitions/RackAssignments](../json-schema/response.json#/definitions/RackAssignments) ### `POST /rack/:rack_id/assignment` -- User requires the read/write role on a workspace that contains the rack +- User requires the read/write role on the rack - Request: [request.json#/definitions/RackAssignmentUpdates](../json-schema/request.json#/definitions/RackAssignmentUpdates) - Response: Redirect to the updated rack assignment @@ -63,7 +63,7 @@ Unless otherwise noted, all routes require authentication. This method requires a request body. -- User requires the read/write role on a workspace that contains the rack +- User requires the read/write role on the rack - Request: [request.json#/definitions/RackAssignmentDeletes](../json-schema/request.json#/definitions/RackAssignmentDeletes) - Response: `204 NO CONTENT` @@ -72,7 +72,7 @@ This method requires a request body. The query parameter `rack_only` (defaults to `0`) specifies whether to update only the rack's phase, or all the rack's devices' phases as well. -- User requires the read/write role on a workspace that contains the rack +- User requires the read/write role on the rack - Request: [request.json#/definitions/RackPhase](../json-schema/request.json#/definitions/RackPhase) - Response: Redirect to the updated rack diff --git a/docs/modules/Conch::Route::RackLayout.md b/docs/modules/Conch::Route::RackLayout.md index ba51fd55a..b15bcee4e 100644 --- a/docs/modules/Conch::Route::RackLayout.md +++ b/docs/modules/Conch::Route::RackLayout.md @@ -8,7 +8,7 @@ Conch::Route::RackLayout Sets up the routes for /layout: -Unless otherwise noted, all routes require authentication. +All routes require authentication. ### `GET /layout` diff --git a/docs/modules/Conch::Route::RackRole.md b/docs/modules/Conch::Route::RackRole.md index 2202c40bc..a21a852fe 100644 --- a/docs/modules/Conch::Route::RackRole.md +++ b/docs/modules/Conch::Route::RackRole.md @@ -8,7 +8,7 @@ Conch::Route::RackRole Sets up the routes for /rack\_role: -Unless otherwise noted, all routes require authentication. +All routes require authentication. ### `GET /rack_role` diff --git a/docs/modules/Conch::Route::Relay.md b/docs/modules/Conch::Route::Relay.md index e2767558b..81178da61 100644 --- a/docs/modules/Conch::Route::Relay.md +++ b/docs/modules/Conch::Route::Relay.md @@ -8,7 +8,7 @@ Conch::Route::Relay Sets up the routes for /relay: -Unless otherwise noted, all routes require authentication. +All routes require authentication. ### `POST /relay/:relay_serial_number/register` diff --git a/docs/modules/Conch::Route::User.md b/docs/modules/Conch::Route::User.md index 891df8faf..da52a8e54 100644 --- a/docs/modules/Conch::Route::User.md +++ b/docs/modules/Conch::Route::User.md @@ -8,7 +8,7 @@ Conch::Route::User Sets up the routes for /user: -Unless otherwise noted, all routes require authentication. +All routes require authentication. ### `GET /user/me` diff --git a/docs/modules/Conch::Route::Validation.md b/docs/modules/Conch::Route::Validation.md index 546aa0178..9b24e6673 100644 --- a/docs/modules/Conch::Route::Validation.md +++ b/docs/modules/Conch::Route::Validation.md @@ -8,7 +8,7 @@ Conch::Route::Validation Sets up the routes for /validation, /validation\_plan and /validation\_state: -Unless otherwise noted, all routes require authentication. +All routes require authentication. ### `GET /validation` diff --git a/docs/modules/Conch::Route::Workspace.md b/docs/modules/Conch::Route::Workspace.md index da62f9c9c..b3e9f6568 100644 --- a/docs/modules/Conch::Route::Workspace.md +++ b/docs/modules/Conch::Route::Workspace.md @@ -11,7 +11,7 @@ Sets up the routes for /workspace. Note that in all routes using `:workspace_id_or_name`, the stash for `workspace_id` will be populated, as well as `workspace_name` if the identifier was not a UUID. -Unless otherwise noted, all routes require authentication. +All routes require authentication. Users will require access to the workspace (or one of its ancestors) at a minimum [role](../modules/Conch%3A%3ADB%3A%3AResult%3A%3AUserWorkspaceRole#role), as indicated. diff --git a/lib/Conch/Route/Build.pm b/lib/Conch/Route/Build.pm index d1e42efb8..356373c3b 100644 --- a/lib/Conch/Route/Build.pm +++ b/lib/Conch/Route/Build.pm @@ -103,7 +103,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. =head3 C diff --git a/lib/Conch/Route/Datacenter.pm b/lib/Conch/Route/Datacenter.pm index 16858bb8d..0a1cf90db 100644 --- a/lib/Conch/Route/Datacenter.pm +++ b/lib/Conch/Route/Datacenter.pm @@ -44,7 +44,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. =head3 C diff --git a/lib/Conch/Route/DatacenterRoom.pm b/lib/Conch/Route/DatacenterRoom.pm index 4080ff691..9a936a479 100644 --- a/lib/Conch/Route/DatacenterRoom.pm +++ b/lib/Conch/Route/DatacenterRoom.pm @@ -49,7 +49,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. =head3 C @@ -116,7 +116,7 @@ Unless otherwise noted, all routes require authentication. =back -=head3 C +=head3 C =over 4 diff --git a/lib/Conch/Route/Device.pm b/lib/Conch/Route/Device.pm index b6c85bf76..9d0fd9284 100644 --- a/lib/Conch/Route/Device.pm +++ b/lib/Conch/Route/Device.pm @@ -121,7 +121,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. The user's role (required for most endpoints) is determined by the build the device is contained in (where users are assigned a L in that diff --git a/lib/Conch/Route/DeviceReport.pm b/lib/Conch/Route/DeviceReport.pm index e5f6a7841..3d0ff1aa4 100644 --- a/lib/Conch/Route/DeviceReport.pm +++ b/lib/Conch/Route/DeviceReport.pm @@ -40,7 +40,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. =head3 C diff --git a/lib/Conch/Route/HardwareProduct.pm b/lib/Conch/Route/HardwareProduct.pm index d6ef00882..55387d084 100644 --- a/lib/Conch/Route/HardwareProduct.pm +++ b/lib/Conch/Route/HardwareProduct.pm @@ -60,7 +60,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. =head3 C diff --git a/lib/Conch/Route/HardwareVendor.pm b/lib/Conch/Route/HardwareVendor.pm index c575027f9..0ce8f5ae5 100644 --- a/lib/Conch/Route/HardwareVendor.pm +++ b/lib/Conch/Route/HardwareVendor.pm @@ -45,7 +45,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. =head3 C diff --git a/lib/Conch/Route/Organization.pm b/lib/Conch/Route/Organization.pm index dbec6c1c9..aeb98443b 100644 --- a/lib/Conch/Route/Organization.pm +++ b/lib/Conch/Route/Organization.pm @@ -57,7 +57,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. =head3 C diff --git a/lib/Conch/Route/Rack.pm b/lib/Conch/Route/Rack.pm index d0956c2f0..9f5d8e3b0 100644 --- a/lib/Conch/Route/Rack.pm +++ b/lib/Conch/Route/Rack.pm @@ -57,7 +57,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. =head3 C @@ -85,7 +85,7 @@ Unless otherwise noted, all routes require authentication. =over 4 -=item * User requires the read-only role on a workspace that contains the rack +=item * User requires the read-only role on the rack =item * Response: F @@ -95,7 +95,7 @@ Unless otherwise noted, all routes require authentication. =over 4 -=item * User requires the read/write role on a workspace that contains the rack +=item * User requires the read/write role on the rack =item * Request: F @@ -117,7 +117,7 @@ Unless otherwise noted, all routes require authentication. =over 4 -=item * User requires the read-only role on a workspace that contains the rack +=item * User requires the read-only role on the rack =item * Response: F @@ -127,7 +127,7 @@ Unless otherwise noted, all routes require authentication. =over 4 -=item * User requires the read/write role on a workspace that contains the rack +=item * User requires the read/write role on the rack =item * Request: F @@ -139,7 +139,7 @@ Unless otherwise noted, all routes require authentication. =over 4 -=item * User requires the read-only role on a workspace that contains the rack +=item * User requires the read-only role on the rack =item * Response: F @@ -149,7 +149,7 @@ Unless otherwise noted, all routes require authentication. =over 4 -=item * User requires the read/write role on a workspace that contains the rack +=item * User requires the read/write role on the rack =item * Request: F @@ -163,7 +163,7 @@ This method requires a request body. =over 4 -=item * User requires the read/write role on a workspace that contains the rack +=item * User requires the read/write role on the rack =item * Request: F @@ -178,7 +178,7 @@ only the rack's phase, or all the rack's devices' phases as well. =over 4 -=item * User requires the read/write role on a workspace that contains the rack +=item * User requires the read/write role on the rack =item * Request: F diff --git a/lib/Conch/Route/RackLayout.pm b/lib/Conch/Route/RackLayout.pm index 3a51a54b3..7521f8b0d 100644 --- a/lib/Conch/Route/RackLayout.pm +++ b/lib/Conch/Route/RackLayout.pm @@ -42,7 +42,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. =head3 C diff --git a/lib/Conch/Route/RackRole.pm b/lib/Conch/Route/RackRole.pm index d6c9ce2fe..dfed454e4 100644 --- a/lib/Conch/Route/RackRole.pm +++ b/lib/Conch/Route/RackRole.pm @@ -42,7 +42,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. =head3 C diff --git a/lib/Conch/Route/Relay.pm b/lib/Conch/Route/Relay.pm index 89b6ae906..1cebb0aa1 100644 --- a/lib/Conch/Route/Relay.pm +++ b/lib/Conch/Route/Relay.pm @@ -42,7 +42,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. =head3 C diff --git a/lib/Conch/Route/User.pm b/lib/Conch/Route/User.pm index 07fcfe538..9d920447b 100644 --- a/lib/Conch/Route/User.pm +++ b/lib/Conch/Route/User.pm @@ -126,7 +126,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. =head3 C diff --git a/lib/Conch/Route/Validation.pm b/lib/Conch/Route/Validation.pm index c8c96e495..249b716b3 100644 --- a/lib/Conch/Route/Validation.pm +++ b/lib/Conch/Route/Validation.pm @@ -66,7 +66,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. =head3 C diff --git a/lib/Conch/Route/Workspace.pm b/lib/Conch/Route/Workspace.pm index 137c5579b..8be0741ee 100644 --- a/lib/Conch/Route/Workspace.pm +++ b/lib/Conch/Route/Workspace.pm @@ -94,7 +94,7 @@ __END__ =pod -Unless otherwise noted, all routes require authentication. +All routes require authentication. Users will require access to the workspace (or one of its ancestors) at a minimum L, as indicated. From f487b784687c162abf16bb5283996411fe91d264 Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Tue, 3 Dec 2019 16:25:07 -0800 Subject: [PATCH 10/15] remove unnecessary collapse clause --- lib/Conch/DB/ResultSet/RackLayout.pm | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/Conch/DB/ResultSet/RackLayout.pm b/lib/Conch/DB/ResultSet/RackLayout.pm index 02db173be..11d989f18 100644 --- a/lib/Conch/DB/ResultSet/RackLayout.pm +++ b/lib/Conch/DB/ResultSet/RackLayout.pm @@ -25,7 +25,6 @@ sub with_rack_unit_size ($self) { $self->search(undef, { join => 'hardware_product', '+columns' => { rack_unit_size => 'hardware_product.rack_unit_size' }, - collapse => 1, }); } From fa2b8cd1d92d3c34721a1a18d0861411e676977b Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Wed, 4 Dec 2019 12:40:15 -0800 Subject: [PATCH 11/15] fix these json schema type definitions, path specifications any string used in an endpoint path must conform to the standard placeholder pattern.. except for rack.name which must allow for existing data that contains '.' --- docs/json-schema/request.json | 10 +++++----- docs/json-schema/response.json | 22 +++++++++++----------- json-schema/request.yaml | 10 +++++----- json-schema/response.yaml | 22 +++++++++++----------- lib/Conch/Route/DatacenterRoom.pm | 2 +- lib/Test/Conch/Fixtures.pm | 2 +- t/integration/crud/racks.t | 4 ++-- t/integration/crud/relay.t | 4 ++-- t/integration/crud/rooms.t | 8 ++++---- t/integration/crud/workspace-rack.t | 6 +++--- 10 files changed, 45 insertions(+), 45 deletions(-) diff --git a/docs/json-schema/request.json b/docs/json-schema/request.json index 22ef77338..b31318356 100644 --- a/docs/json-schema/request.json +++ b/docs/json-schema/request.json @@ -233,7 +233,7 @@ "additionalProperties" : false, "properties" : { "alias" : { - "$ref" : "common.json#/definitions/non_empty_string" + "$ref" : "common.json#/definitions/mojo_standard_placeholder" }, "az" : { "$ref" : "common.json#/definitions/non_empty_string" @@ -257,7 +257,7 @@ "minProperties" : 1, "properties" : { "alias" : { - "$ref" : "common.json#/definitions/non_empty_string" + "$ref" : "common.json#/definitions/mojo_standard_placeholder" }, "az" : { "$ref" : "common.json#/definitions/non_empty_string" @@ -872,7 +872,7 @@ "$ref" : "common.json#/definitions/uuid" }, "name" : { - "$ref" : "common.json#/definitions/non_empty_string" + "$ref" : "common.json#/definitions/mojo_relaxed_placeholder" }, "phase" : { "$ref" : "common.json#/definitions/device_phase" @@ -982,7 +982,7 @@ "minProperties" : 1, "properties" : { "name" : { - "$ref" : "common.json#/definitions/non_empty_string" + "$ref" : "common.json#/definitions/mojo_standard_placeholder" }, "rack_size" : { "$ref" : "common.json#/definitions/positive_integer" @@ -1011,7 +1011,7 @@ "$ref" : "common.json#/definitions/uuid" }, "name" : { - "$ref" : "common.json#/definitions/non_empty_string" + "$ref" : "common.json#/definitions/mojo_relaxed_placeholder" }, "phase" : { "$ref" : "common.json#/definitions/device_phase" diff --git a/docs/json-schema/response.json b/docs/json-schema/response.json index ab6ec748e..08dc316ac 100644 --- a/docs/json-schema/response.json +++ b/docs/json-schema/response.json @@ -63,7 +63,7 @@ "$ref" : "common.json#/definitions/uuid" }, "name" : { - "$ref" : "common.json#/definitions/mojo_relaxed_placeholder" + "$ref" : "common.json#/definitions/mojo_standard_placeholder" }, "started" : { "oneOf" : [ @@ -219,7 +219,7 @@ "additionalProperties" : false, "properties" : { "alias" : { - "type" : "string" + "$ref" : "common.json#/definitions/mojo_standard_placeholder" }, "az" : { "type" : "string" @@ -785,7 +785,7 @@ "rack_name" : { "oneOf" : [ { - "type" : "string" + "$ref" : "common.json#/definitions/mojo_relaxed_placeholder" }, { "type" : "null" @@ -1862,7 +1862,7 @@ "$ref" : "common.json#/definitions/uuid" }, "name" : { - "$ref" : "common.json#/definitions/mojo_relaxed_placeholder" + "$ref" : "common.json#/definitions/mojo_standard_placeholder" }, "users" : { "items" : { @@ -2016,7 +2016,7 @@ "$ref" : "common.json#/definitions/uuid" }, "name" : { - "type" : "string" + "$ref" : "common.json#/definitions/mojo_relaxed_placeholder" }, "phase" : { "$ref" : "common.json#/definitions/device_phase" @@ -2158,7 +2158,7 @@ "$ref" : "common.json#/definitions/uuid" }, "name" : { - "type" : "string" + "$ref" : "common.json#/definitions/mojo_standard_placeholder" }, "rack_size" : { "$ref" : "common.json#/definitions/positive_integer" @@ -2415,7 +2415,7 @@ "$ref" : "common.json#/definitions/uuid" }, "name" : { - "$ref" : "common.json#/definitions/mojo_relaxed_placeholder" + "$ref" : "common.json#/definitions/mojo_standard_placeholder" }, "role" : { "$ref" : "common.json#/definitions/role" @@ -3146,13 +3146,13 @@ "$ref" : "common.json#/definitions/uuid" }, "name" : { - "type" : "string" + "$ref" : "common.json#/definitions/mojo_relaxed_placeholder" }, "phase" : { "$ref" : "common.json#/definitions/device_phase" }, "rack_role_name" : { - "type" : "string" + "$ref" : "common.json#/definitions/mojo_standard_placeholder" }, "rack_size" : { "$ref" : "common.json#/definitions/positive_integer" @@ -3213,10 +3213,10 @@ "$ref" : "common.json#/definitions/uuid" }, "rack_name" : { - "type" : "string" + "$ref" : "common.json#/definitions/mojo_relaxed_placeholder" }, "rack_role_name" : { - "type" : "string" + "$ref" : "common.json#/definitions/mojo_standard_placeholder" }, "rack_unit_start" : { "$ref" : "common.json#/definitions/positive_integer" diff --git a/json-schema/request.yaml b/json-schema/request.yaml index a50f5d2a3..ee5979b0f 100644 --- a/json-schema/request.yaml +++ b/json-schema/request.yaml @@ -45,7 +45,7 @@ definitions: az: $ref: common.yaml#/definitions/non_empty_string alias: - $ref: common.yaml#/definitions/non_empty_string + $ref: common.yaml#/definitions/mojo_standard_placeholder vendor_name: $ref: common.yaml#/definitions/non_empty_string DatacenterRoomUpdate: @@ -58,7 +58,7 @@ definitions: az: $ref: common.yaml#/definitions/non_empty_string alias: - $ref: common.yaml#/definitions/non_empty_string + $ref: common.yaml#/definitions/mojo_standard_placeholder vendor_name: $ref: common.yaml#/definitions/non_empty_string DeviceReport: @@ -73,7 +73,7 @@ definitions: - build_id properties: name: - $ref: common.yaml#/definitions/non_empty_string + $ref: common.yaml#/definitions/mojo_relaxed_placeholder datacenter_room_id: $ref: common.yaml#/definitions/uuid rack_role_id: @@ -92,7 +92,7 @@ definitions: minProperties: 1 properties: name: - $ref: common.yaml#/definitions/non_empty_string + $ref: common.yaml#/definitions/mojo_relaxed_placeholder datacenter_room_id: $ref: common.yaml#/definitions/uuid rack_role_id: @@ -178,7 +178,7 @@ definitions: minProperties: 1 properties: name: - $ref: common.yaml#/definitions/non_empty_string + $ref: common.yaml#/definitions/mojo_standard_placeholder rack_size: $ref: common.yaml#/definitions/positive_integer RackLayoutCreate: diff --git a/json-schema/response.yaml b/json-schema/response.yaml index a814bbbfb..47654fdd6 100644 --- a/json-schema/response.yaml +++ b/json-schema/response.yaml @@ -410,7 +410,7 @@ definitions: - type: 'null' rack_name: oneOf: - - type: string + - $ref: common.yaml#/definitions/mojo_relaxed_placeholder - type: 'null' rack_unit_start: oneOf: @@ -931,11 +931,11 @@ definitions: id: $ref: common.yaml#/definitions/uuid name: - type: string + $ref: common.yaml#/definitions/mojo_relaxed_placeholder phase: $ref: common.yaml#/definitions/device_phase rack_role_name: - type: string + $ref: common.yaml#/definitions/mojo_standard_placeholder rack_size: $ref: common.yaml#/definitions/positive_integer device_progress: @@ -1192,11 +1192,11 @@ definitions: rack_id: $ref: common.yaml#/definitions/uuid rack_name: - type: string + $ref: common.yaml#/definitions/mojo_relaxed_placeholder rack_unit_start: $ref: common.yaml#/definitions/positive_integer rack_role_name: - type: string + $ref: common.yaml#/definitions/mojo_standard_placeholder az: type: string num_devices: @@ -1316,7 +1316,7 @@ definitions: az: type: string alias: - type: string + $ref: common.yaml#/definitions/mojo_standard_placeholder vendor_name: description: the vendor's name for the room oneOf: @@ -1409,7 +1409,7 @@ definitions: id: $ref: common.yaml#/definitions/uuid name: - type: string + $ref: common.yaml#/definitions/mojo_relaxed_placeholder datacenter_room_id: $ref: common.yaml#/definitions/uuid rack_role_id: @@ -1447,7 +1447,7 @@ definitions: id: $ref: common.yaml#/definitions/uuid name: - type: string + $ref: common.yaml#/definitions/mojo_standard_placeholder rack_size: $ref: common.yaml#/definitions/positive_integer created: @@ -1629,7 +1629,7 @@ definitions: id: $ref: common.yaml#/definitions/uuid name: - $ref: common.yaml#/definitions/mojo_relaxed_placeholder + $ref: common.yaml#/definitions/mojo_standard_placeholder description: oneOf: - type: 'null' @@ -1868,7 +1868,7 @@ definitions: id: $ref: common.yaml#/definitions/uuid name: - $ref: common.yaml#/definitions/mojo_relaxed_placeholder + $ref: common.yaml#/definitions/mojo_standard_placeholder description: oneOf: - type: 'null' @@ -1973,7 +1973,7 @@ definitions: id: $ref: common.yaml#/definitions/uuid name: - $ref: common.yaml#/definitions/mojo_relaxed_placeholder + $ref: common.yaml#/definitions/mojo_standard_placeholder description: oneOf: - type: 'null' diff --git a/lib/Conch/Route/DatacenterRoom.pm b/lib/Conch/Route/DatacenterRoom.pm index 9a936a479..283eb191a 100644 --- a/lib/Conch/Route/DatacenterRoom.pm +++ b/lib/Conch/Route/DatacenterRoom.pm @@ -41,7 +41,7 @@ sub routes { $with_datacenter_room->get('/racks')->to('#racks'); # GET /room/:datacenter_room_id_or_alias/rack/:rack_name - $with_datacenter_room->get('/rack/:rack_name')->to('#find_rack'); + $with_datacenter_room->get('/rack/#rack_name')->to('#find_rack'); } 1; diff --git a/lib/Test/Conch/Fixtures.pm b/lib/Test/Conch/Fixtures.pm index 1c95570ba..eff5d1790 100644 --- a/lib/Test/Conch/Fixtures.pm +++ b/lib/Test/Conch/Fixtures.pm @@ -345,7 +345,7 @@ sub generate_set ($self, $set_name, @args) { }, "rack_${num}a" => { new => 'rack', - using => { name => "rack ${num}a" }, + using => { name => "rack.${num}a" }, requires => { "datacenter_room_${num}a" => { our => 'datacenter_room_id', their => 'id' }, rack_role_42u => { our => 'rack_role_id', their => 'id' }, diff --git a/t/integration/crud/racks.t b/t/integration/crud/racks.t index bc6046cfa..6084e788b 100644 --- a/t/integration/crud/racks.t +++ b/t/integration/crud/racks.t @@ -26,12 +26,12 @@ my $rack = $t->load_fixture('rack_0a'); $t->get_ok('/rack') ->status_is(200) ->json_schema_is('Racks') - ->json_cmp_deeply([ superhashof({ name => 'rack 0a' }) ]); + ->json_cmp_deeply([ superhashof({ name => 'rack.0a' }) ]); $t->get_ok('/rack/'.$rack->id) ->status_is(200) ->json_schema_is('Rack') - ->json_cmp_deeply(superhashof({ name => 'rack 0a' })); + ->json_cmp_deeply(superhashof({ name => 'rack.0a' })); $t->post_ok('/rack', json => { wat => 'wat' }) ->status_is(400) diff --git a/t/integration/crud/relay.t b/t/integration/crud/relay.t index 05cec7796..707a568f0 100644 --- a/t/integration/crud/relay.t +++ b/t/integration/crud/relay.t @@ -266,7 +266,7 @@ subtest list => sub { updated => re(qr/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3,9}Z$/), location => { $rack_layouts[1][2]->%{qw(rack_id rack_unit_start)}, - rack_name => 'rack 1a', + rack_name => 'rack.1a', rack_role_name => 'rack_role 42U', az => 'room-1a', }, @@ -284,7 +284,7 @@ subtest list => sub { updated => re(qr/^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}\.\d{3,9}Z$/), location => { $rack_layouts[0][0]->%{qw(rack_id rack_unit_start)}, - rack_name => 'rack 0a', + rack_name => 'rack.0a', rack_role_name => 'rack_role 42U', az => 'room-0a', }, diff --git a/t/integration/crud/rooms.t b/t/integration/crud/rooms.t index f69dcdca1..48b03c386 100644 --- a/t/integration/crud/rooms.t +++ b/t/integration/crud/rooms.t @@ -39,17 +39,17 @@ $t->get_ok('/room/'.$room->alias) $t->get_ok('/room/'.$room->id.'/racks') ->status_is(200) ->json_schema_is('Racks') - ->json_cmp_deeply([ superhashof({ name => 'rack 0a' }) ]); + ->json_cmp_deeply([ superhashof({ name => 'rack.0a' }) ]); $t->get_ok('/room/'.$room->alias.'/racks') ->status_is(200) ->json_schema_is('Racks') - ->json_cmp_deeply([ superhashof({ name => 'rack 0a' }) ]); + ->json_cmp_deeply([ superhashof({ name => 'rack.0a' }) ]); -$t->get_ok('/room/'.$room->alias.'/rack/rack 0a') +$t->get_ok('/room/'.$room->alias.'/rack/rack.0a') ->status_is(200) ->json_schema_is('Rack') - ->json_cmp_deeply(superhashof({ name => 'rack 0a' })); + ->json_cmp_deeply(superhashof({ name => 'rack.0a' })); $t->post_ok('/room', json => { wat => 'wat' }) ->status_is(400) diff --git a/t/integration/crud/workspace-rack.t b/t/integration/crud/workspace-rack.t index 5ea55ec73..011606db0 100644 --- a/t/integration/crud/workspace-rack.t +++ b/t/integration/crud/workspace-rack.t @@ -39,7 +39,7 @@ $t->get_ok("/workspace/$global_ws_id/rack") 'room-0a' => [ { id => $rack_id, - name => 'rack 0a', + name => 'rack.0a', phase => 'integration', rack_role_name => 'rack_role 42U', rack_size => 42, @@ -76,7 +76,7 @@ subtest 'Add rack to workspace' => sub { 'room-0a' => [ { id => $rack_id, - name => 'rack 0a', + name => 'rack.0a', phase => 'integration', rack_role_name => 'rack_role 42U', rack_size => 42, @@ -163,7 +163,7 @@ subtest 'Assign device to a location' => sub { device_progress => { unknown => 1, valid => 1 }, id => $rack_id, phase => 'integration', - name => 'rack 0a', + name => 'rack.0a', rack_role_name => 'rack_role 42U', rack_size => 42, } From 3fb6a4a91a9ea786b53c3bfca14b415393c7c5c4 Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Thu, 28 Nov 2019 16:06:05 -0800 Subject: [PATCH 12/15] fix controller action documentation --- docs/modules/Conch::Controller::Build.md | 6 +++--- docs/modules/Conch::Controller::Datacenter.md | 3 ++- docs/modules/Conch::Controller::DatacenterRoom.md | 4 +++- docs/modules/Conch::Controller::Device.md | 7 ++++--- docs/modules/Conch::Controller::DeviceInterface.md | 4 +++- docs/modules/Conch::Controller::DeviceReport.md | 7 +++++-- docs/modules/Conch::Controller::HardwareProduct.md | 7 +++++-- docs/modules/Conch::Controller::HardwareVendor.md | 3 ++- docs/modules/Conch::Controller::Organization.md | 8 +++++--- docs/modules/Conch::Controller::Rack.md | 6 +++++- docs/modules/Conch::Controller::RackLayout.md | 3 ++- docs/modules/Conch::Controller::RackRole.md | 3 ++- docs/modules/Conch::Controller::Relay.md | 7 +++++-- docs/modules/Conch::Controller::User.md | 4 ++-- docs/modules/Conch::Controller::Validation.md | 3 ++- docs/modules/Conch::Controller::ValidationPlan.md | 3 ++- docs/modules/Conch::Controller::Workspace.md | 10 +++++----- docs/modules/Conch::Controller::WorkspaceRack.md | 4 ++-- lib/Conch/Controller/Build.pm | 6 +++--- lib/Conch/Controller/Datacenter.pm | 3 ++- lib/Conch/Controller/DatacenterRoom.pm | 4 +++- lib/Conch/Controller/Device.pm | 7 ++++--- lib/Conch/Controller/DeviceInterface.pm | 4 +++- lib/Conch/Controller/DeviceReport.pm | 7 +++++-- lib/Conch/Controller/HardwareProduct.pm | 7 +++++-- lib/Conch/Controller/HardwareVendor.pm | 3 ++- lib/Conch/Controller/Organization.pm | 8 +++++--- lib/Conch/Controller/Rack.pm | 6 +++++- lib/Conch/Controller/RackLayout.pm | 3 ++- lib/Conch/Controller/RackRole.pm | 3 ++- lib/Conch/Controller/Relay.pm | 7 +++++-- lib/Conch/Controller/User.pm | 4 ++-- lib/Conch/Controller/Validation.pm | 3 ++- lib/Conch/Controller/ValidationPlan.pm | 3 ++- lib/Conch/Controller/Workspace.pm | 10 +++++----- lib/Conch/Controller/WorkspaceRack.pm | 4 ++-- 36 files changed, 118 insertions(+), 66 deletions(-) diff --git a/docs/modules/Conch::Controller::Build.md b/docs/modules/Conch::Controller::Build.md index 800fdddd8..09a915e18 100644 --- a/docs/modules/Conch::Controller::Build.md +++ b/docs/modules/Conch::Controller::Build.md @@ -19,11 +19,11 @@ Requires the user to be a system admin. ## find\_build -Chainable action that validates the `build_id` or `build_name` provided in the -path, and stashes the query to get to it in `build_rs`. +Chainable action that uses the `build_id_or_name` value provided in the stash (usually via the +request URL) to look up a build, and stashes the query to get to it in `build_rs`. If `require_role` is provided, it is used as the minimum required role for the user to -continue; otherwise the user must be a system admin. +continue; otherwise the user must have the 'admin' role. ## get diff --git a/docs/modules/Conch::Controller::Datacenter.md b/docs/modules/Conch::Controller::Datacenter.md index 6f37068f6..e0aa83f0e 100644 --- a/docs/modules/Conch::Controller::Datacenter.md +++ b/docs/modules/Conch::Controller::Datacenter.md @@ -6,7 +6,8 @@ Conch::Controller::Datacenter ## find\_datacenter -Handles looking up the object by id. +Chainable action that uses the `datacenter_id` value provided in the stash (usually via the +request URL) to look up a datacenter, and stashes the result in `datacenter`. ## get\_all diff --git a/docs/modules/Conch::Controller::DatacenterRoom.md b/docs/modules/Conch::Controller::DatacenterRoom.md index 0751e944c..749539185 100644 --- a/docs/modules/Conch::Controller::DatacenterRoom.md +++ b/docs/modules/Conch::Controller::DatacenterRoom.md @@ -6,7 +6,9 @@ Conch::Controller::DatacenterRoom ## find\_datacenter\_room -Handles looking up the object by id or alias. +Chainable action that uses the `datacenter_room_id_or_alias` value provided in the stash +(usually via the request URL) to look up a datacenter\_room, and stashes the result in +`datacenter_room`. ## get\_all diff --git a/docs/modules/Conch::Controller::Device.md b/docs/modules/Conch::Controller::Device.md index 70f75f97b..550cc9244 100644 --- a/docs/modules/Conch::Controller::Device.md +++ b/docs/modules/Conch::Controller::Device.md @@ -6,11 +6,12 @@ Conch::Controller::Device ## find\_device -Chainable action that uses the `device_id_or_serial_number` provided in the path -to find the device and verify the user has the required role to operate on it. +Chainable action that uses the `device_id`, `device_serial_number` or +`device_id_or_serial_number` provided in the stash (usually via the request URL) to look up a +device, and stashes the query to get to it in `device_rs`. If `require_role` is provided, it is used as the minimum required role for the user to -continue. +continue; otherwise the user must be a registered relay user or a system admin. If `phase_earlier_than` is provided, `409 CONFLICT` is returned if the device is in the provided phase (or later). diff --git a/docs/modules/Conch::Controller::DeviceInterface.md b/docs/modules/Conch::Controller::DeviceInterface.md index adf85ac8c..01a91819c 100644 --- a/docs/modules/Conch::Controller::DeviceInterface.md +++ b/docs/modules/Conch::Controller::DeviceInterface.md @@ -6,7 +6,9 @@ Conch::Controller::Device ## find\_device\_interface -Chainable action that looks up the device interface by its id or name. +Chainable action that uses the `interface_name` value provided in the stash (usually via the +request URL) to look up a device interface, and stashes the query to get to it in +`device_interface_rs`. ## get\_one\_field diff --git a/docs/modules/Conch::Controller::DeviceReport.md b/docs/modules/Conch::Controller::DeviceReport.md index 034f5a744..7ef439138 100644 --- a/docs/modules/Conch::Controller::DeviceReport.md +++ b/docs/modules/Conch::Controller::DeviceReport.md @@ -21,8 +21,11 @@ Uses a device report to populate configuration information about the given devic ## find\_device\_report -Chainable action that validates the 'device\_report\_id' provided in the path. -Stores the device\_id and device\_report resultset to the stash for later retrieval. +Chainable action that uses the `device_report_id` value provided in the stash (usually via the +request URL) to look up a device report, and stashes the query to get to it in +`device_report_rs`. + +`device_id` is also saved to the stash. Role checks are done in the next controller action in the chain. diff --git a/docs/modules/Conch::Controller::HardwareProduct.md b/docs/modules/Conch::Controller::HardwareProduct.md index 3759fb8eb..925ffaef5 100644 --- a/docs/modules/Conch::Controller::HardwareProduct.md +++ b/docs/modules/Conch::Controller::HardwareProduct.md @@ -12,8 +12,11 @@ Response uses the HardwareProducts json schema. ## find\_hardware\_product -Chainable action that looks up the object by id, sku, name or alias depending on the url -pattern, stashing the query to get to it in `hardware_product_rs`. +Chainable action that uses the `hardware_product_id` or `hardware_product_key` and +`hardware_product_value` values provided in the stash (usually via the request URL) to look up +a hardware\_product, and stashes the query to get to it in `hardware_product_rs`. + +Supported keys are: `sku`, `name`, and `alias`. ## get diff --git a/docs/modules/Conch::Controller::HardwareVendor.md b/docs/modules/Conch::Controller::HardwareVendor.md index 50a0f59fb..0ae9b85c0 100644 --- a/docs/modules/Conch::Controller::HardwareVendor.md +++ b/docs/modules/Conch::Controller::HardwareVendor.md @@ -6,7 +6,8 @@ Conch::Controller::HardwareVendor ## find\_hardware\_vendor -Handles looking up the object by id or name. +Chainable action that uses the `hardware_vendor_id_or_name` value provided in the stash +(usually via the request URL) to look up a build, and stashes the result in `hardware_vendor`. ## get\_all diff --git a/docs/modules/Conch::Controller::Organization.md b/docs/modules/Conch::Controller::Organization.md index d967d2074..cb40b63c7 100644 --- a/docs/modules/Conch::Controller::Organization.md +++ b/docs/modules/Conch::Controller::Organization.md @@ -24,10 +24,12 @@ Requires the user to be a system admin. ## find\_organization -Chainable action that validates the `organization_id` or `organization_name` provided in the -path, and stashes the query to get to it in `organization_rs`. +Chainable action that uses the `organization_id_or_name` value provided in the stash (usually +via the request URL) to look up a build, and stashes the query to get to it in +`organization_rs`. -Requires the 'admin' role on the organization (or the user to be a system admin). +If `require_role` is provided, it is used as the minimum required role for the user to +continue; otherwise the user must have the 'admin' role. ## get diff --git a/docs/modules/Conch::Controller::Rack.md b/docs/modules/Conch::Controller::Rack.md index 3b8ba600b..5214c8b46 100644 --- a/docs/modules/Conch::Controller::Rack.md +++ b/docs/modules/Conch::Controller::Rack.md @@ -6,7 +6,11 @@ Conch::Controller::Rack ## find\_rack -Supports rack lookups by uuid. +Chainable action that uses the `rack_id` value provided in the stash (usually via the +request URL) to look up a rack, and stashes the query to get to it in `rack_rs`. + +If `require_role` is provided, it is used as the minimum required role for the user to +continue; otherwise the user must be a system admin. ## create diff --git a/docs/modules/Conch::Controller::RackLayout.md b/docs/modules/Conch::Controller::RackLayout.md index 1401e0045..2fbfc1e9c 100644 --- a/docs/modules/Conch::Controller::RackLayout.md +++ b/docs/modules/Conch::Controller::RackLayout.md @@ -6,7 +6,8 @@ Conch::Controller::RackLayout ## find\_rack\_layout -Supports rack layout lookups by id. +Chainable action that uses the `layout_id` value provided in the stash (usually via the +request URL) to look up a build, and stashes the query to get to it in `layout_rs`. ## create diff --git a/docs/modules/Conch::Controller::RackRole.md b/docs/modules/Conch::Controller::RackRole.md index 744035555..2bfb3b09c 100644 --- a/docs/modules/Conch::Controller::RackRole.md +++ b/docs/modules/Conch::Controller::RackRole.md @@ -6,7 +6,8 @@ Conch::Controller::RackRole ## find\_rack\_role -Supports rack role lookups by uuid and name. +Chainable action that uses the `rack_role_id_or_name` value provided in the stash (usually via +the request URL) to look up a build, and stashes the result in `rack_role`. ## create diff --git a/docs/modules/Conch::Controller::Relay.md b/docs/modules/Conch::Controller::Relay.md index d7c5da041..e5554e069 100644 --- a/docs/modules/Conch::Controller::Relay.md +++ b/docs/modules/Conch::Controller::Relay.md @@ -17,8 +17,11 @@ Response uses the Relays json schema. ## find\_relay -Chainable action that looks up the relay by id or serial\_number, -stashing the query to get to it in `relay_rs`. +Chainable action that uses the `relay_id_or_serial_number` provided in the stash (usually +via the request URL), and stashes the query to get to it in `relay_rs`. + +The relay must have been registered by the user to continue; otherwise the user must be a +system admin. ## get diff --git a/docs/modules/Conch::Controller::User.md b/docs/modules/Conch::Controller::User.md index 6093f9e52..7490d40be 100644 --- a/docs/modules/Conch::Controller::User.md +++ b/docs/modules/Conch::Controller::User.md @@ -6,8 +6,8 @@ Conch::Controller::User ## find\_user -Chainable action that validates the `target_user_id_or_email` provided in the path, and -stashes the corresponding user row in `target_user`. +Chainable action that uses the `target_user_id_or_email` value provided in the stash (usually +via the request URL) to look up a user, and stashes the result in `target_user`. ## revoke\_user\_tokens diff --git a/docs/modules/Conch::Controller::Validation.md b/docs/modules/Conch::Controller::Validation.md index f0be0994c..79a94c0d9 100644 --- a/docs/modules/Conch::Controller::Validation.md +++ b/docs/modules/Conch::Controller::Validation.md @@ -14,7 +14,8 @@ Response uses the Validations json schema (including deactivated ones). ## find\_validation -Find the Validation specified by uuid or name, and stashes the query to get to it in +Chainable action that uses the `validation_id_or_name` value provided in the stash (usually +via the request URL) to look up a validation, and stashes the query to get to it in `validation_rs`. ## get diff --git a/docs/modules/Conch::Controller::ValidationPlan.md b/docs/modules/Conch::Controller::ValidationPlan.md index d28d5ef22..5fcd2812f 100644 --- a/docs/modules/Conch::Controller::ValidationPlan.md +++ b/docs/modules/Conch::Controller::ValidationPlan.md @@ -14,7 +14,8 @@ Response uses the ValidationPlans json schema. ## find\_validation\_plan -Find the Validation Plan specified by uuid or name and put it in the stash as +Chainable action that uses the `validation_plan_id_or_name` provided in the stash +(usually via the request URL) to look up a validation\_plan, and stashes the result in `validation_plan`. ## get diff --git a/docs/modules/Conch::Controller::Workspace.md b/docs/modules/Conch::Controller::Workspace.md index c3b4aa76d..b78006f92 100644 --- a/docs/modules/Conch::Controller::Workspace.md +++ b/docs/modules/Conch::Controller::Workspace.md @@ -6,13 +6,13 @@ Conch::Controller::Workspace ## find\_workspace -Chainable action that validates the `workspace_id` or `workspace_name` provided in the path, -and stashes the query to get to it in `workspace_rs`. +Chainable action that uses the `workspace_id_or__name` provided in the stash (usually via +the request URL) to look up a workspace, and stashes the query to get to it in `workspace_rs`. -If `workspace_name` is provided, `workspace_id` is looked up and stashed. +If the workspace name is provided, `workspace_id` is looked up and stashed. -If `require_role` is provided, it is used as the minimum required role for the user to -continue. +`require_role` is used as the minimum required role for the user to continue; otherwise the +user must be a system admin. ## list diff --git a/docs/modules/Conch::Controller::WorkspaceRack.md b/docs/modules/Conch::Controller::WorkspaceRack.md index 24f3e81c7..f7dc3f590 100644 --- a/docs/modules/Conch::Controller::WorkspaceRack.md +++ b/docs/modules/Conch::Controller::WorkspaceRack.md @@ -12,8 +12,8 @@ Response uses the WorkspaceRackSummary json schema. ## find\_workspace\_rack -Chainable action that uses the `workspace_rs` and `rack_id` stash values and confirms the -rack is a (direct or indirect) member of the workspace. +Chainable action that uses the `workspace_id` and `rack_id` values provided in the stash +to confirm the rack is a (direct or indirect) member of the workspace. Relies on ["find\_workspace" in Conch::Controller::Workspace](../modules/Conch%3A%3AController%3A%3AWorkspace#find_workspace) and ["find\_rack" in Conch::Controller::Rack](../modules/Conch%3A%3AController%3A%3ARack#find_rack) to have already run, verified user roles, and populated diff --git a/lib/Conch/Controller/Build.pm b/lib/Conch/Controller/Build.pm index a2b73714a..c84584d83 100644 --- a/lib/Conch/Controller/Build.pm +++ b/lib/Conch/Controller/Build.pm @@ -97,11 +97,11 @@ sub create ($c) { =head2 find_build -Chainable action that validates the C or C provided in the -path, and stashes the query to get to it in C. +Chainable action that uses the C value provided in the stash (usually via the +request URL) to look up a build, and stashes the query to get to it in C. If C is provided, it is used as the minimum required role for the user to -continue; otherwise the user must be a system admin. +continue; otherwise the user must have the 'admin' role. =cut diff --git a/lib/Conch/Controller/Datacenter.pm b/lib/Conch/Controller/Datacenter.pm index c3667be25..42080a62a 100644 --- a/lib/Conch/Controller/Datacenter.pm +++ b/lib/Conch/Controller/Datacenter.pm @@ -12,7 +12,8 @@ Conch::Controller::Datacenter =head2 find_datacenter -Handles looking up the object by id. +Chainable action that uses the C value provided in the stash (usually via the +request URL) to look up a datacenter, and stashes the result in C. =cut diff --git a/lib/Conch/Controller/DatacenterRoom.pm b/lib/Conch/Controller/DatacenterRoom.pm index fe49c7587..ae2b43a04 100644 --- a/lib/Conch/Controller/DatacenterRoom.pm +++ b/lib/Conch/Controller/DatacenterRoom.pm @@ -14,7 +14,9 @@ Conch::Controller::DatacenterRoom =head2 find_datacenter_room -Handles looking up the object by id or alias. +Chainable action that uses the C value provided in the stash +(usually via the request URL) to look up a datacenter_room, and stashes the result in +C. =cut diff --git a/lib/Conch/Controller/Device.pm b/lib/Conch/Controller/Device.pm index 458d2a727..2007af6a1 100644 --- a/lib/Conch/Controller/Device.pm +++ b/lib/Conch/Controller/Device.pm @@ -17,11 +17,12 @@ Conch::Controller::Device =head2 find_device -Chainable action that uses the C provided in the path -to find the device and verify the user has the required role to operate on it. +Chainable action that uses the C, C or +C provided in the stash (usually via the request URL) to look up a +device, and stashes the query to get to it in C. If C is provided, it is used as the minimum required role for the user to -continue. +continue; otherwise the user must be a registered relay user or a system admin. If C is provided, C<409 CONFLICT> is returned if the device is in the provided phase (or later). diff --git a/lib/Conch/Controller/DeviceInterface.pm b/lib/Conch/Controller/DeviceInterface.pm index 0bc8fa6e5..bb76453b6 100644 --- a/lib/Conch/Controller/DeviceInterface.pm +++ b/lib/Conch/Controller/DeviceInterface.pm @@ -12,7 +12,9 @@ Conch::Controller::Device =head2 find_device_interface -Chainable action that looks up the device interface by its id or name. +Chainable action that uses the C value provided in the stash (usually via the +request URL) to look up a device interface, and stashes the query to get to it in +C. =cut diff --git a/lib/Conch/Controller/DeviceReport.pm b/lib/Conch/Controller/DeviceReport.pm index e820f6fef..d6958ab66 100644 --- a/lib/Conch/Controller/DeviceReport.pm +++ b/lib/Conch/Controller/DeviceReport.pm @@ -336,8 +336,11 @@ sub _add_reboot_count ($device) { =head2 find_device_report -Chainable action that validates the 'device_report_id' provided in the path. -Stores the device_id and device_report resultset to the stash for later retrieval. +Chainable action that uses the C value provided in the stash (usually via the +request URL) to look up a device report, and stashes the query to get to it in +C. + +C is also saved to the stash. Role checks are done in the next controller action in the chain. diff --git a/lib/Conch/Controller/HardwareProduct.pm b/lib/Conch/Controller/HardwareProduct.pm index abdf4070e..1d34fa9bb 100644 --- a/lib/Conch/Controller/HardwareProduct.pm +++ b/lib/Conch/Controller/HardwareProduct.pm @@ -29,8 +29,11 @@ sub list ($c) { =head2 find_hardware_product -Chainable action that looks up the object by id, sku, name or alias depending on the url -pattern, stashing the query to get to it in C. +Chainable action that uses the C or C and +C values provided in the stash (usually via the request URL) to look up +a hardware_product, and stashes the query to get to it in C. + +Supported keys are: C, C, and C. =cut diff --git a/lib/Conch/Controller/HardwareVendor.pm b/lib/Conch/Controller/HardwareVendor.pm index c368b0b6e..021f69337 100644 --- a/lib/Conch/Controller/HardwareVendor.pm +++ b/lib/Conch/Controller/HardwareVendor.pm @@ -14,7 +14,8 @@ Conch::Controller::HardwareVendor =head2 find_hardware_vendor -Handles looking up the object by id or name. +Chainable action that uses the C value provided in the stash +(usually via the request URL) to look up a build, and stashes the result in C. =cut diff --git a/lib/Conch/Controller/Organization.pm b/lib/Conch/Controller/Organization.pm index f70cfe9cb..06f59186b 100644 --- a/lib/Conch/Controller/Organization.pm +++ b/lib/Conch/Controller/Organization.pm @@ -100,10 +100,12 @@ sub create ($c) { =head2 find_organization -Chainable action that validates the C or C provided in the -path, and stashes the query to get to it in C. +Chainable action that uses the C value provided in the stash (usually +via the request URL) to look up a build, and stashes the query to get to it in +C. -Requires the 'admin' role on the organization (or the user to be a system admin). +If C is provided, it is used as the minimum required role for the user to +continue; otherwise the user must have the 'admin' role. =cut diff --git a/lib/Conch/Controller/Rack.pm b/lib/Conch/Controller/Rack.pm index f405d9d80..bdb8582e9 100644 --- a/lib/Conch/Controller/Rack.pm +++ b/lib/Conch/Controller/Rack.pm @@ -14,7 +14,11 @@ Conch::Controller::Rack =head2 find_rack -Supports rack lookups by uuid. +Chainable action that uses the C value provided in the stash (usually via the +request URL) to look up a rack, and stashes the query to get to it in C. + +If C is provided, it is used as the minimum required role for the user to +continue; otherwise the user must be a system admin. =cut diff --git a/lib/Conch/Controller/RackLayout.pm b/lib/Conch/Controller/RackLayout.pm index 8ab5f56ef..8653ec1f5 100644 --- a/lib/Conch/Controller/RackLayout.pm +++ b/lib/Conch/Controller/RackLayout.pm @@ -14,7 +14,8 @@ Conch::Controller::RackLayout =head2 find_rack_layout -Supports rack layout lookups by id. +Chainable action that uses the C value provided in the stash (usually via the +request URL) to look up a build, and stashes the query to get to it in C. =cut diff --git a/lib/Conch/Controller/RackRole.pm b/lib/Conch/Controller/RackRole.pm index 1edb10ac3..6d2a5e90b 100644 --- a/lib/Conch/Controller/RackRole.pm +++ b/lib/Conch/Controller/RackRole.pm @@ -12,7 +12,8 @@ Conch::Controller::RackRole =head2 find_rack_role -Supports rack role lookups by uuid and name. +Chainable action that uses the C value provided in the stash (usually via +the request URL) to look up a build, and stashes the result in C. =cut diff --git a/lib/Conch/Controller/Relay.pm b/lib/Conch/Controller/Relay.pm index 36b0d62e1..46ab1d08e 100644 --- a/lib/Conch/Controller/Relay.pm +++ b/lib/Conch/Controller/Relay.pm @@ -62,8 +62,11 @@ sub list ($c) { =head2 find_relay -Chainable action that looks up the relay by id or serial_number, -stashing the query to get to it in C. +Chainable action that uses the C provided in the stash (usually +via the request URL), and stashes the query to get to it in C. + +The relay must have been registered by the user to continue; otherwise the user must be a +system admin. =cut diff --git a/lib/Conch/Controller/User.pm b/lib/Conch/Controller/User.pm index c6d2f0fbf..b5920e4e9 100644 --- a/lib/Conch/Controller/User.pm +++ b/lib/Conch/Controller/User.pm @@ -18,8 +18,8 @@ Conch::Controller::User =head2 find_user -Chainable action that validates the C provided in the path, and -stashes the corresponding user row in C. +Chainable action that uses the C value provided in the stash (usually +via the request URL) to look up a user, and stashes the result in C. =cut diff --git a/lib/Conch/Controller/Validation.pm b/lib/Conch/Controller/Validation.pm index e3bafcba0..5b6de5b2f 100644 --- a/lib/Conch/Controller/Validation.pm +++ b/lib/Conch/Controller/Validation.pm @@ -29,7 +29,8 @@ sub list ($c) { =head2 find_validation -Find the Validation specified by uuid or name, and stashes the query to get to it in +Chainable action that uses the C value provided in the stash (usually +via the request URL) to look up a validation, and stashes the query to get to it in C. =cut diff --git a/lib/Conch/Controller/ValidationPlan.pm b/lib/Conch/Controller/ValidationPlan.pm index 999421f25..be30c28d6 100644 --- a/lib/Conch/Controller/ValidationPlan.pm +++ b/lib/Conch/Controller/ValidationPlan.pm @@ -30,7 +30,8 @@ sub list ($c) { =head2 find_validation_plan -Find the Validation Plan specified by uuid or name and put it in the stash as +Chainable action that uses the C provided in the stash +(usually via the request URL) to look up a validation_plan, and stashes the result in C. =cut diff --git a/lib/Conch/Controller/Workspace.pm b/lib/Conch/Controller/Workspace.pm index bcd7419b3..b9a0505a7 100644 --- a/lib/Conch/Controller/Workspace.pm +++ b/lib/Conch/Controller/Workspace.pm @@ -15,13 +15,13 @@ Conch::Controller::Workspace =head2 find_workspace -Chainable action that validates the C or C provided in the path, -and stashes the query to get to it in C. +Chainable action that uses the C provided in the stash (usually via +the request URL) to look up a workspace, and stashes the query to get to it in C. -If C is provided, C is looked up and stashed. +If the workspace name is provided, C is looked up and stashed. -If C is provided, it is used as the minimum required role for the user to -continue. +C is used as the minimum required role for the user to continue; otherwise the +user must be a system admin. =cut diff --git a/lib/Conch/Controller/WorkspaceRack.pm b/lib/Conch/Controller/WorkspaceRack.pm index 27f1f5d04..3dbc85070 100644 --- a/lib/Conch/Controller/WorkspaceRack.pm +++ b/lib/Conch/Controller/WorkspaceRack.pm @@ -74,8 +74,8 @@ sub list ($c) { =head2 find_workspace_rack -Chainable action that uses the C and C stash values and confirms the -rack is a (direct or indirect) member of the workspace. +Chainable action that uses the C and C values provided in the stash +to confirm the rack is a (direct or indirect) member of the workspace. Relies on L and L to have already run, verified user roles, and populated From 67e5c8a06bfb7d1bf367f6a900cc068b845af3ba Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Thu, 5 Dec 2019 11:58:02 -0800 Subject: [PATCH 13/15] log when an object is not found This lets us distinguish in tests an endpoint not existing and an object not existing --- lib/Conch/Controller/Build.pm | 15 +++++++-- lib/Conch/Controller/Datacenter.pm | 2 +- lib/Conch/Controller/DatacenterRoom.pm | 2 +- lib/Conch/Controller/Device.pm | 5 +-- lib/Conch/Controller/DeviceInterface.pm | 2 +- lib/Conch/Controller/DeviceReport.pm | 8 ++--- lib/Conch/Controller/DeviceSettings.pm | 7 +++-- lib/Conch/Controller/HardwareProduct.pm | 2 +- lib/Conch/Controller/HardwareVendor.pm | 2 +- lib/Conch/Controller/Organization.pm | 10 ++++-- lib/Conch/Controller/Rack.pm | 9 +++--- lib/Conch/Controller/RackRole.pm | 4 +-- lib/Conch/Controller/Relay.pm | 5 ++- lib/Conch/Controller/Schema.pm | 5 ++- lib/Conch/Controller/User.pm | 20 +++++++++--- lib/Conch/Controller/Validation.pm | 2 +- lib/Conch/Controller/ValidationPlan.pm | 2 +- lib/Conch/Controller/ValidationState.pm | 2 +- t/integration/crud/build.t | 4 ++- t/integration/crud/datacenter.t | 3 +- t/integration/crud/devices.t | 38 +++++++++++++--------- t/integration/crud/hardware-product.t | 9 ++++-- t/integration/crud/hardware-vendor.t | 6 ++-- t/integration/crud/organization.t | 9 ++++-- t/integration/crud/rack-layouts.t | 3 +- t/integration/crud/rack-roles.t | 3 +- t/integration/crud/racks.t | 14 ++++++--- t/integration/crud/relay.t | 3 +- t/integration/crud/rooms.t | 3 +- t/integration/device-reports.t | 6 ++-- t/integration/users.t | 42 ++++++++++++++++--------- t/schema.t | 6 ++-- 32 files changed, 165 insertions(+), 88 deletions(-) diff --git a/lib/Conch/Controller/Build.pm b/lib/Conch/Controller/Build.pm index c84584d83..874331924 100644 --- a/lib/Conch/Controller/Build.pm +++ b/lib/Conch/Controller/Build.pm @@ -117,7 +117,10 @@ sub find_build ($c) { $rs = $rs->search({ 'build.name' => $identifier }); } - return $c->status(404) if not $rs->exists; + if (not $rs->exists) { + $c->log->debug('could not find build '.$identifier); + return $c->status(404); + } CHECK_ACCESS: { if ($c->is_system_admin) { @@ -271,7 +274,10 @@ sub add_user ($c) { my $user = $input->{user_id} ? $user_rs->find($input->{user_id}) : $input->{email} ? $user_rs->find_by_email($input->{email}) : undef; - return $c->status(404) if not $user; + if (not $user) { + $c->log->debug('Could not find user '.$input->@{qw(user_id email)}); + return $c->status(404); + } $c->stash('target_user', $user); my $build_name = $c->stash('build_name') // $c->stash('build_rs')->get_column('name')->single; @@ -462,7 +468,10 @@ sub add_organization ($c) { return if not $input; my $organization = $c->db_organizations->active->find($input->{organization_id}); - return $c->status(404) if not $organization; + if (not $organization) { + $c->log->debug('Could not find organization '.$input->{organization_id}); + return $c->status(404); + } my $build_id = $c->stash('build_id'); diff --git a/lib/Conch/Controller/Datacenter.pm b/lib/Conch/Controller/Datacenter.pm index 42080a62a..9bc142487 100644 --- a/lib/Conch/Controller/Datacenter.pm +++ b/lib/Conch/Controller/Datacenter.pm @@ -22,7 +22,7 @@ sub find_datacenter ($c) { my $datacenter = $c->db_datacenters->find($datacenter_id); if (not $datacenter) { - $c->log->debug('Unable to find datacenter '.$datacenter_id); + $c->log->debug('Could not find datacenter '.$datacenter_id); return $c->status(404); } diff --git a/lib/Conch/Controller/DatacenterRoom.pm b/lib/Conch/Controller/DatacenterRoom.pm index ae2b43a04..dad1dadda 100644 --- a/lib/Conch/Controller/DatacenterRoom.pm +++ b/lib/Conch/Controller/DatacenterRoom.pm @@ -36,7 +36,7 @@ sub find_datacenter_room ($c) { my $room = $rs->single; if (not $room) { - $c->log->debug('Could not find datacenter room'); + $c->log->debug('Could not find datacenter room '.$identifier); return $c->status(404); } diff --git a/lib/Conch/Controller/Device.pm b/lib/Conch/Controller/Device.pm index 2007af6a1..d3616653f 100644 --- a/lib/Conch/Controller/Device.pm +++ b/lib/Conch/Controller/Device.pm @@ -46,6 +46,7 @@ sub find_device ($c) { }); } else { + $c->log->error('missing identifier for #find_device'); return $c->status(404); } @@ -54,7 +55,7 @@ sub find_device ($c) { # if the device id cannot be fetched, we can bail out right now if (not $device_id) { - $c->log->debug('Failed to find device '.$identifier); + $c->log->debug('Could not find device '.$identifier); return $c->status(404); } @@ -241,7 +242,7 @@ sub lookup_by_other_attribute ($c) { # save ourselves a more expensive query if there are no matches if (not $device_rs->exists) { - $c->log->debug('Failed to find devices matching '.$key.'='.$value); + $c->log->debug('Could not find devices matching '.$key.'='.$value); return $c->status(404); } diff --git a/lib/Conch/Controller/DeviceInterface.pm b/lib/Conch/Controller/DeviceInterface.pm index bb76453b6..f16595366 100644 --- a/lib/Conch/Controller/DeviceInterface.pm +++ b/lib/Conch/Controller/DeviceInterface.pm @@ -28,7 +28,7 @@ sub find_device_interface ($c) { ->search_related('device_nics', { iface_name => $interface_name }) ->active; if (not $nic_rs->exists) { - $c->log->debug("Failed to find interface $interface_name for device ".$c->stash('device_id')); + $c->log->debug('Could not find interface '.$interface_name.' for device '.$c->stash('device_id')); return $c->status(404); } diff --git a/lib/Conch/Controller/DeviceReport.pm b/lib/Conch/Controller/DeviceReport.pm index d6958ab66..e29411e0f 100644 --- a/lib/Conch/Controller/DeviceReport.pm +++ b/lib/Conch/Controller/DeviceReport.pm @@ -34,7 +34,7 @@ sub process ($c) { # Make sure that the remote side is telling us about a hardware product we understand my $hardware_product_id = $c->db_hardware_products->active->search({ sku => $unserialized_report->{sku} })->get_column('id')->single; - return $c->status(409, { error => 'Could not locate hardware product for sku '.$unserialized_report->{sku} }) if not $hardware_product_id; + return $c->status(409, { error => 'Could not find hardware product with sku '.$unserialized_report->{sku} }) if not $hardware_product_id; if ($unserialized_report->{relay} and my $relay_serial = $unserialized_report->{relay}{serial}) { return $c->status(409, { error => 'relay serial '.$relay_serial.' is not registered' }) @@ -43,7 +43,7 @@ sub process ($c) { my $device = $c->db_devices->find({ serial_number => $c->stash('device_serial_number') }); if (not $device) { - $c->log->error('Failed to find device '.$c->stash('device_serial_number')); + $c->log->error('Could not find device '.$c->stash('device_serial_number')); return $c->status(404); } @@ -352,7 +352,7 @@ sub find_device_report ($c) { my $device_id = $device_report_rs->get_column('device_id')->single; if (not $device_id) { - $c->log->debug('Failed to find device_report id \''.$c->stash('device_report_id').'\''); + $c->log->debug('Could not find device report '.$c->stash('device_report_id')); return $c->status(404); } @@ -392,7 +392,7 @@ sub validate_report ($c) { } my $hardware_product_id = $c->db_hardware_products->active->search({ sku => $unserialized_report->{sku} })->get_column('id')->single; - return $c->status(409, { error => 'Could not locate hardware product for sku '.$unserialized_report->{sku} }) if not $hardware_product_id; + return $c->status(409, { error => 'Could not find hardware product with sku '.$unserialized_report->{sku} }) if not $hardware_product_id; if (my $current_hardware_product_id = $c->db_devices->search({ serial_number => $unserialized_report->{serial_number} })->get_column('hardware_product_id')->single) { return $c->status(409, { error => 'Report sku does not match expected hardware_product for device '.$unserialized_report->{serial_number} }) diff --git a/lib/Conch/Controller/DeviceSettings.pm b/lib/Conch/Controller/DeviceSettings.pm index 5ac633bf7..2c19a1375 100644 --- a/lib/Conch/Controller/DeviceSettings.pm +++ b/lib/Conch/Controller/DeviceSettings.pm @@ -126,7 +126,10 @@ sub get_single ($c) { ->rows(1) ->single; - return $c->status(404) if not $setting; + if (not $setting) { + $c->log->debug('Could not find device setting '.$setting_key.' for device '.$c->stash('device_id')); + return $c->status(404); + } $c->status(200, { $setting_key => $setting->value }); } @@ -154,7 +157,7 @@ sub delete_single ($c) { ->search_related('device_settings', { name => $setting_key }) ->active ->deactivate <= 0) { - $c->log->debug("No such setting '$setting_key'"); + $c->log->debug('Could not find device setting '.$setting_key.' for device '.$c->stash('device_id')); return $c->status(404); } diff --git a/lib/Conch/Controller/HardwareProduct.pm b/lib/Conch/Controller/HardwareProduct.pm index 1d34fa9bb..589111e19 100644 --- a/lib/Conch/Controller/HardwareProduct.pm +++ b/lib/Conch/Controller/HardwareProduct.pm @@ -57,7 +57,7 @@ sub find_hardware_product ($c) { } if (not $hardware_product_rs->exists) { - $c->log->debug('Could not locate a valid hardware product with ' + $c->log->debug('Could not find hardware product with ' .($c->stash('hardware_product_id') ? ('id '.$c->stash('hardware_product_id')) : ($c->stash('hardware_product_key').' '.$c->stash('hardware_product_value')))); return $c->status(404); diff --git a/lib/Conch/Controller/HardwareVendor.pm b/lib/Conch/Controller/HardwareVendor.pm index 021f69337..1df37d829 100644 --- a/lib/Conch/Controller/HardwareVendor.pm +++ b/lib/Conch/Controller/HardwareVendor.pm @@ -31,7 +31,7 @@ sub find_hardware_vendor ($c) { } if (not $hardware_vendor_rs->exists) { - $c->log->debug('Could not locate a valid hardware vendor'); + $c->log->debug('Could not find hardware vendor '.$c->stash('hardware_vendor_id_or_name')); return $c->status(404); } diff --git a/lib/Conch/Controller/Organization.pm b/lib/Conch/Controller/Organization.pm index 06f59186b..b08864268 100644 --- a/lib/Conch/Controller/Organization.pm +++ b/lib/Conch/Controller/Organization.pm @@ -121,7 +121,10 @@ sub find_organization ($c) { $rs = $rs->search({ 'organization.name' => $identifier }); } - return $c->status(404) if not $rs->exists; + if (not $rs->exists) { + $c->log->debug('Could not find organization '.$identifier); + return $c->status(404); + } $rs = $rs->active; return $c->status(410) if not $rs->exists; @@ -255,7 +258,10 @@ sub add_user ($c) { my $user = $input->{user_id} ? $user_rs->find($input->{user_id}) : $input->{email} ? $user_rs->find_by_email($input->{email}) : undef; - return $c->status(404) if not $user; + if (not $user) { + $c->log->debug('Could not find user '.$input->@{qw(user_id email)}); + return $c->status(404); + } $c->stash('target_user', $user); my $organization_name = $c->stash('organization_name') // $c->stash('organization_rs')->get_column('name')->single; diff --git a/lib/Conch/Controller/Rack.pm b/lib/Conch/Controller/Rack.pm index bdb8582e9..8e556031c 100644 --- a/lib/Conch/Controller/Rack.pm +++ b/lib/Conch/Controller/Rack.pm @@ -28,7 +28,7 @@ sub find_rack ($c) { ->search({ 'rack.id' => $c->stash('rack_id') }); if (not $rack_rs->exists) { - $c->log->debug('Could not find rack ',$c->stash('rack_id')); + $c->log->debug('Could not find rack '.$c->stash('rack_id')); return $c->status(404); } @@ -390,7 +390,7 @@ sub set_assignment ($c) { next if $device_locations_rs->search({ device_id => $device->id, $entry->%{rack_unit_start} })->exists; } elsif ($entry->{device_id}) { - $c->log->error('no device corresponding to device id '.$entry->{device_id}); + $c->log->error('Could not find device '.$entry->{device_id}); $c->res->code(404); die 'rollback'; } @@ -441,13 +441,12 @@ sub delete_assignment ($c) { my $ru = $_; none { $ru == $_->rack_unit_start } @layouts; } map $_->{rack_unit_start}, $input->@*; - $c->log->debug('cannot delete nonexistent layout'.(@missing > 1 ? 's' : '').' for rack_unit_start '.join(', ', @missing)); + $c->log->debug('Cannot delete nonexistent layout for rack_unit_start '.join(', ', @missing)); return $c->status(404); } if (my @unoccupied = grep !$_->device_location, @layouts) { - $c->log->debug('cannot delete assignments for unoccupied slot'. - (@unoccupied > 1 ? 's' : '').': rack_unit_start ' + $c->log->debug('Cannot delete assignment for unoccupied slot at rack_unit_start ' .join(', ', map $_->rack_unit_start, @unoccupied)); return $c->status(404); } diff --git a/lib/Conch/Controller/RackRole.pm b/lib/Conch/Controller/RackRole.pm index 6d2a5e90b..1e2c9f3a3 100644 --- a/lib/Conch/Controller/RackRole.pm +++ b/lib/Conch/Controller/RackRole.pm @@ -22,7 +22,7 @@ sub find_rack_role ($c) { if ($c->stash('rack_role_id_or_name') =~ /^(.+?)\=(.+)$/) { my ($key, $value) = ($1, $2); if ($key ne 'name') { - $c->log->warn("Unknown identifier '$key'"); + $c->log->error("Unknown identifier '$key'"); return $c->status(404); } @@ -35,7 +35,7 @@ sub find_rack_role ($c) { } if (not $rack_role) { - $c->log->debug('Failed to find rack role'); + $c->log->debug('Could not find rack_role '.$c->stash('rack_role_id_or_name')); return $c->status(404); } diff --git a/lib/Conch/Controller/Relay.pm b/lib/Conch/Controller/Relay.pm index 46ab1d08e..0090aba2c 100644 --- a/lib/Conch/Controller/Relay.pm +++ b/lib/Conch/Controller/Relay.pm @@ -75,7 +75,10 @@ sub find_relay ($c) { my $rs = $c->db_relays ->search({ is_uuid($identifier) ? 'id' : 'serial_number' => $identifier }); - return $c->status(404) if not $rs->exists; + if (not $rs->exists) { + $c->log->debug('Could not find relay '.$identifier); + return $c->status(404); + } $rs = $rs->active; return $c->status(410) if not $rs->exists; diff --git a/lib/Conch/Controller/Schema.pm b/lib/Conch/Controller/Schema.pm index af1544d7a..c345af800 100644 --- a/lib/Conch/Controller/Schema.pm +++ b/lib/Conch/Controller/Schema.pm @@ -35,7 +35,10 @@ sub get ($c) { return $c->status(400, { error => 'Cannot find validator' }) if not $validator; my $schema = _extract_schema_definition($validator, $name); - return $c->status(404) if not $schema; + if (not $schema) { + $c->log->debug('Could not find '.$type.' schema '.$name); + return $c->status(404); + } # ensure $id is unique $schema->{'$id'} =~ s/^urn:\K/$type./; diff --git a/lib/Conch/Controller/User.pm b/lib/Conch/Controller/User.pm index b5920e4e9..f3e76f19a 100644 --- a/lib/Conch/Controller/User.pm +++ b/lib/Conch/Controller/User.pm @@ -32,7 +32,10 @@ sub find_user ($c) { : Email::Valid->address($identifier) ? $user_rs->find_by_email($identifier) : return $c->status(400, { error => 'invalid identifier format for '.$identifier }); - return $c->status(404) if not $user; + if (not $user) { + $c->log->debug('Could not find user '.$identifier); + return $c->status(404); + } if ($user->deactivated) { return $c->status(410, { @@ -194,7 +197,11 @@ sub get_setting ($c) { ->order_by({ -desc => 'created' }) ->one_row; - return $c->status(404) if not $setting; + if (not $setting) { + $c->log->debug('Could not find user setting '.$key.' for user '.$c->stash('target_user')->email); + return $c->status(404); + } + $c->status(200, { $key => $setting->value }); } @@ -627,15 +634,18 @@ Only api tokens may be retrieved by this flow. =cut sub find_api_token ($c) { - return $c->status(404) if $c->stash('token_name') =~ /^login_jwt_/; + if ($c->stash('token_name') =~ /^login_jwt_/) { + $c->log->error('Lookup of login tokens not supported'); + return $c->status(404); + } my $token_rs = $c->stash('target_user') ->user_session_tokens ->unexpired ->search({ name => $c->stash('token_name') }); if (not $token_rs->exists) { - $c->log->debug('Could not find token named "'.$c->stash('token_name') - .' for user_id '.$c->stash('target_user')->id); + $c->log->debug('Could not find token '.$c->stash('token_name') + .' for user '.$c->stash('target_user')->email); return $c->status(404); } diff --git a/lib/Conch/Controller/Validation.pm b/lib/Conch/Controller/Validation.pm index 5b6de5b2f..d8d0d6c99 100644 --- a/lib/Conch/Controller/Validation.pm +++ b/lib/Conch/Controller/Validation.pm @@ -43,7 +43,7 @@ sub find_validation($c) { }); if (not $validation_rs->exists) { - $c->log->debug("Failed to find validation for '$identifier'"); + $c->log->debug('Could not find validation '.$identifier); return $c->status(404); } diff --git a/lib/Conch/Controller/ValidationPlan.pm b/lib/Conch/Controller/ValidationPlan.pm index be30c28d6..0c2ff3334 100644 --- a/lib/Conch/Controller/ValidationPlan.pm +++ b/lib/Conch/Controller/ValidationPlan.pm @@ -44,7 +44,7 @@ sub find_validation_plan($c) { })->single; if (not $validation_plan) { - $c->log->debug("Failed to find validation plan for '$identifier'"); + $c->log->debug('Could not find validation plan '.$identifier); return $c->status(404); } diff --git a/lib/Conch/Controller/ValidationState.pm b/lib/Conch/Controller/ValidationState.pm index b9d2a9da3..90da93c6c 100644 --- a/lib/Conch/Controller/ValidationState.pm +++ b/lib/Conch/Controller/ValidationState.pm @@ -30,7 +30,7 @@ sub get ($c) { ->all; if (not $validation_state) { - $c->log->debug('Failed to find validation for \''.$c->stash('validation_state_id').'\''); + $c->log->debug('Could not find validation state '.$c->stash('validation_state_id')); return $c->status(404); } diff --git a/t/integration/crud/build.t b/t/integration/crud/build.t index ee5fe8b9e..de34b66fd 100644 --- a/t/integration/crud/build.t +++ b/t/integration/crud/build.t @@ -453,8 +453,10 @@ $t->delete_ok('/user/'.$admin_user->id) user => { map +($_ => $admin_user->$_), qw(id email name created deactivated) }, }); +# wtf is this? $t->delete_ok('/build/my first build/user/foo@bar.com') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find user foo@bar.com'); $t->get_ok('/build/our second build/user') ->status_is(200) diff --git a/t/integration/crud/datacenter.t b/t/integration/crud/datacenter.t index 28e3496be..6aabede14 100644 --- a/t/integration/crud/datacenter.t +++ b/t/integration/crud/datacenter.t @@ -85,7 +85,8 @@ $t->delete_ok("/dc/$idd") ->status_is(204); $t->get_ok("/dc/$idd") - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find datacenter '.$idd); done_testing; # vim: set ts=4 sts=4 sw=4 et : diff --git a/t/integration/crud/devices.t b/t/integration/crud/devices.t index eb973b6c5..b189b52aa 100644 --- a/t/integration/crud/devices.t +++ b/t/integration/crud/devices.t @@ -43,7 +43,8 @@ $build2->create_related('user_build_roles', { user_id => $build_user->id, role = $t->authenticate(email => $ro_user->email); $t->get_ok('/device/nonexistent') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find device nonexistent'); my $test_device_id; @@ -584,11 +585,10 @@ subtest 'device network interfaces' => sub { ->status_is(200) ->json_schema_is('DeviceNic'); - $t->get_ok('/device/TEST/interface/ipmi1/device_id') - ->status_is(404); - - $t->get_ok('/device/TEST/interface/ipmi1/created') - ->status_is(404); + $t->get_ok('/device/TEST/interface/ipmi1/'.$_) + ->status_is(404) + ->log_error_is('no endpoint found for: GET /device/TEST/interface/ipmi1/'.$_) + foreach qw(device_id created); $t->get_ok('/device/TEST/interface/ipmi1/mac') ->status_is(200) @@ -702,12 +702,14 @@ subtest 'get by device attributes' => sub { ->json_is('', [ $undetailed_device ], 'got device by link'); $t->get_ok('/device?key=value') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find devices matching key=value'); $test_device->create_related('device_settings', { name => 'key', value => 'value' }); $t->get_ok('/device?key=ugh') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find devices matching key=ugh'); $t->get_ok('/device?key=value') ->status_is(200) @@ -738,8 +740,9 @@ subtest 'get by device attributes' => sub { }; subtest 'mutate device attributes' => sub { - $t->post_ok('/device/nonexistent/validate') - ->status_is(404); + $t->post_ok('/device/nonexistent/validated') + ->status_is(404) + ->log_debug_is('Could not find device nonexistent'); $t->post_ok('/device/TEST/asset_tag') ->status_is(400) @@ -877,7 +880,8 @@ subtest 'Device settings' => sub { ->json_is({}); $t->get_ok('/device/LOCATED_DEVICE/settings/foo') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find device setting foo for device '.$located_device_id); $t->post_ok('/device/LOCATED_DEVICE/settings') ->status_is(400) @@ -895,7 +899,8 @@ subtest 'Device settings' => sub { ->json_cmp_deeply('/details', [ { path => '/', message => re(qr/too many properties/i) } ]); $t->post_ok('/device/LOCATED_DEVICE/settings/FOO/BAR', json => { 'FOO/BAR' => 'foo' }) - ->status_is(404); + ->status_is(404) + ->log_error_is('no endpoint found for: POST /device/LOCATED_DEVICE/settings/FOO/BAR'); $t->post_ok('/device/LOCATED_DEVICE/settings', json => { foo => 'baz' }) ->status_is(204); @@ -947,10 +952,12 @@ subtest 'Device settings' => sub { ->status_is(204); $t->get_ok('/device/LOCATED_DEVICE/settings/fizzle') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find device setting fizzle for device '.$located_device_id); $t->delete_ok('/device/LOCATED_DEVICE/settings/fizzle') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find device setting fizzle for device '.$located_device_id); $t->post_ok('/device/LOCATED_DEVICE/settings', json => { 'tag.foo' => 'foo', 'tag.bar' => 'bar' }) ->status_is(204); @@ -967,7 +974,8 @@ subtest 'Device settings' => sub { ->status_is(204); $t->get_ok('/device/LOCATED_DEVICE/settings/tag.bar') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find device setting tag.bar for device '.$located_device_id); $t->get_ok('/device/LOCATED_DEVICE') ->status_is(200) diff --git a/t/integration/crud/hardware-product.t b/t/integration/crud/hardware-product.t index a6ccec554..e403ca930 100644 --- a/t/integration/crud/hardware-product.t +++ b/t/integration/crud/hardware-product.t @@ -30,7 +30,8 @@ my $vendor_id = $products->[0]{hardware_vendor_id}; my $validation_plan_id = $products->[0]{validation_plan_id}; $t->get_ok('/hardware_product/'.create_uuid_str()) - ->status_is(404); + ->status_is(404) + ->log_debug_like(qr/^Could not find hardware product with id ${\Conch::UUID::UUID_FORMAT}$/); $t->get_ok("/hardware_product/$hw_id") ->status_is(200) @@ -176,10 +177,12 @@ $t->get_ok($t->tx->res->headers->location) ->json_cmp_deeply($new_product); $t->get_ok('/hardware_product/foo=sungo') - ->status_is(404); + ->status_is(404) + ->log_error_is('no endpoint found for: GET /hardware_product/foo=sungo'); $t->get_ok('/hardware_product/name=sungo') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find hardware product with name sungo'); $t->get_ok('/hardware_product/name=sungo2') ->status_is(200) diff --git a/t/integration/crud/hardware-vendor.t b/t/integration/crud/hardware-vendor.t index fab5ab904..0d2c7afb5 100644 --- a/t/integration/crud/hardware-vendor.t +++ b/t/integration/crud/hardware-vendor.t @@ -19,10 +19,12 @@ $t->get_ok('/hardware_vendor') my $vendors = $t->tx->res->json; $t->get_ok('/hardware_vendor/'.create_uuid_str()) - ->status_is(404); + ->status_is(404) + ->log_debug_like(qr/^Could not find hardware vendor ${\Conch::UUID::UUID_FORMAT}$/); $t->get_ok('/hardware_vendor/foo') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find hardware vendor foo'); $t->get_ok('/hardware_vendor/'.$vendors->[0]{id}) ->status_is(200) diff --git a/t/integration/crud/organization.t b/t/integration/crud/organization.t index 946782dfc..3d361bd21 100644 --- a/t/integration/crud/organization.t +++ b/t/integration/crud/organization.t @@ -152,7 +152,8 @@ $t2->get_ok('/organization/my first organization') ->log_debug_is('User lacks the required role (admin) for organization my first organization'); $t2->delete_ok('/organization/foo') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find organization foo'); $t2->delete_ok('/organization/my first organization') ->status_is(403) @@ -896,7 +897,8 @@ $t->get_ok('/organization') ->json_is([ $organization, $organization2 ]); $t->delete_ok('/organization/my first organization/user/foo@bar.com') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find user foo@bar.com'); $t->get_ok('/organization/our second organization') ->status_is(200) @@ -904,7 +906,8 @@ $t->get_ok('/organization/our second organization') ->json_is($organization2); $t->delete_ok('/organization/foo') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find organization foo'); $t->delete_ok('/organization/our second organization') ->status_is(204) diff --git a/t/integration/crud/rack-layouts.t b/t/integration/crud/rack-layouts.t index acc42bfc5..6da57895b 100644 --- a/t/integration/crud/rack-layouts.t +++ b/t/integration/crud/rack-layouts.t @@ -321,7 +321,8 @@ $t->delete_ok('/layout/'.$layout_20_23->id) $t->delete_ok('/layout/'.$layout_3_6->id) ->status_is(204); $t->get_ok('/layout/'.$layout_3_6->id) - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find rack layout '.$layout_3_6->id); # now we have these assigned slots: # start 1, width 2 diff --git a/t/integration/crud/rack-roles.t b/t/integration/crud/rack-roles.t index 7d387cef8..b98aaa971 100644 --- a/t/integration/crud/rack-roles.t +++ b/t/integration/crud/rack-roles.t @@ -109,7 +109,8 @@ $t->delete_ok("/rack_role/$idr") ->status_is(204); $t->get_ok("/rack_role/$idr") - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find rack_role '.$idr); $t->get_ok('/rack_role') ->status_is(200) diff --git a/t/integration/crud/racks.t b/t/integration/crud/racks.t index 6084e788b..ba1dbe0f0 100644 --- a/t/integration/crud/racks.t +++ b/t/integration/crud/racks.t @@ -194,7 +194,8 @@ $t->delete_ok("/rack/$new_rack_id") ->status_is(204); $t->get_ok("/rack/$new_rack_id") - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find rack '.$new_rack_id); my $hardware_product_compute = $t->load_fixture('hardware_product_compute'); my $hardware_product_storage = $t->load_fixture('hardware_product_storage'); @@ -344,7 +345,7 @@ $t->post_ok('/rack/'.$rack->id.'/assignment', json => [ { device_id => $new_id, rack_unit_start => 11 }, ]) ->status_is(404) - ->log_is('no device corresponding to device id '.$new_id); + ->log_is('Could not find device '.$new_id); $t->post_ok('/rack/'.$rack->id.'/assignment', json => [ { device_id => $foo->id, device_serial_number => 'BAR', rack_unit_start => 11 }, @@ -460,7 +461,8 @@ $t->delete_ok('/rack/'.$rack->id.'/assignment', json => [ rack_unit_start => 2, # this rack_unit_start doesn't exist }, ]) - ->status_is(404); + ->status_is(404) + ->log_debug_is('Cannot delete nonexistent layout for rack_unit_start 2'); $t->delete_ok('/rack/'.$rack->id.'/assignment', json => [ { @@ -468,7 +470,8 @@ $t->delete_ok('/rack/'.$rack->id.'/assignment', json => [ rack_unit_start => 3, # this slot isn't occupied }, ]) - ->status_is(404); + ->status_is(404) + ->log_debug_is('Cannot delete assignment for unoccupied slot at rack_unit_start 3'); $t->delete_ok('/rack/'.$rack->id.'/assignment', json => [ { @@ -476,7 +479,8 @@ $t->delete_ok('/rack/'.$rack->id.'/assignment', json => [ rack_unit_start => 11, # wrong slot for this device }, ]) - ->status_is(404); + ->status_is(404) + ->log_debug_is('rack_unit_start 11 occupied by device_id '.$baz->id.' but was expecting device_id '.$foo->id); $t->delete_ok('/rack/'.$rack->id.'/assignment', json => [ { diff --git a/t/integration/crud/relay.t b/t/integration/crud/relay.t index 707a568f0..43af3b95d 100644 --- a/t/integration/crud/relay.t +++ b/t/integration/crud/relay.t @@ -103,7 +103,8 @@ is($new_relay_data->{updated}, $relay_data->{updated}, 'relay update timestamp d $relay0->last_seen(Conch::Time->from_string($new_relay_data->{last_seen}, lenient => 1)); $t->get_ok('/relay/'.create_uuid_str()) - ->status_is(404); + ->status_is(404) + ->log_debug_like(qr/^Could not find relay ${\Conch::UUID::UUID_FORMAT}$/); $t->get_ok('/relay/'.$relay0->id) ->status_is(200) diff --git a/t/integration/crud/rooms.t b/t/integration/crud/rooms.t index 48b03c386..adb79728e 100644 --- a/t/integration/crud/rooms.t +++ b/t/integration/crud/rooms.t @@ -91,6 +91,7 @@ $t->delete_ok("/room/$idr") ->status_is(204); $t->get_ok("/room/$idr") - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find datacenter room '.$idr); done_testing; diff --git a/t/integration/device-reports.t b/t/integration/device-reports.t index 1dd49d9e4..9cf5ecd86 100644 --- a/t/integration/device-reports.t +++ b/t/integration/device-reports.t @@ -33,11 +33,11 @@ subtest preliminaries => sub { $t->post_ok('/device/TEST', json => $report_data) ->status_is(409) - ->json_is({ error => 'Could not locate hardware product for sku '.$report_data->{sku} }); + ->json_is({ error => 'Could not find hardware product with sku '.$report_data->{sku} }); $t->post_ok('/device_report', json => $report_data) ->status_is(409) - ->json_is({ error => 'Could not locate hardware product for sku '.$report_data->{sku} }); + ->json_is({ error => 'Could not find hardware product with sku '.$report_data->{sku} }); $hardware_product = first { $_->isa('Conch::DB::Result::HardwareProduct') } $t->load_fixture('hardware_product_compute'); @@ -51,7 +51,7 @@ subtest preliminaries => sub { $t->post_ok('/device/TEST', json => $report_data) ->status_is(404) - ->log_error_is('Failed to find device TEST'); + ->log_error_is('Could not find device TEST'); my $device = $t->generate_fixtures('device', { serial_number => 'TEST', diff --git a/t/integration/users.t b/t/integration/users.t index fcfa61731..2ce732bc3 100644 --- a/t/integration/users.t +++ b/t/integration/users.t @@ -74,7 +74,8 @@ subtest 'User' => sub { ->json_is({}); $t->get_ok('/user/me/settings/BAD') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find user setting BAD for user '.$ro_user->email); $t->post_ok('/user/me/settings/TEST', json => { NOTTEST => 'test' }) ->status_is(400) @@ -86,7 +87,8 @@ subtest 'User' => sub { ->json_cmp_deeply('/details', [ { path => '/', message => re(qr/too many properties/i) } ]); $t->post_ok('/user/me/settings/FOO/BAR', json => { 'FOO/BAR' => 'TEST' }) - ->status_is(404); + ->status_is(404) + ->log_error_is('no endpoint found for: POST /user/me/settings/FOO/BAR'); $t->post_ok('/user/me/settings/TEST', json => { TEST => 'TEST' }) ->status_is(204); @@ -137,7 +139,8 @@ subtest 'User' => sub { ->json_is({}); $t->get_ok('/user/me/settings/TEST') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find user setting TEST for user '.$ro_user->email); $t->post_ok('/user/me/settings/dot.setting', json => { 'dot.setting' => 'set' }) ->status_is(204); @@ -686,7 +689,8 @@ subtest 'modify another user' => sub { ->json_is({ error => 'invalid identifier format for foobar' }); $t_super->delete_ok('/user/foobar@conch.joyent.us/password') - ->status_is(404, 'attempted to reset the password for a non-existent user'); + ->status_is(404) + ->log_debug_is('Could not find user foobar@conch.joyent.us'); $t_super->delete_ok("/user/$new_user_id/password") ->status_is(204, 'reset the new user\'s password') @@ -782,7 +786,8 @@ subtest 'modify another user' => sub { $t_super->delete_ok('/user/foobar@joyent.conch.us') - ->status_is(404, 'attempted to deactivate a non-existent user'); + ->status_is(404) + ->log_debug_is('Could not find user foobar@joyent.conch.us'); $new_user->create_related('user_workspace_roles', { workspace_id => $child_ws->id, role => 'rw' }); @@ -794,7 +799,8 @@ subtest 'modify another user' => sub { ->status_is(410); $t_super->get_ok('/user/'.create_uuid_str) - ->status_is(404); + ->status_is(404) + ->log_debug_like(qr/^Could not find user ${\Conch::UUID::UUID_FORMAT}$/); # we haven't cleared the user's session yet... $t2->get_ok('/me') @@ -910,7 +916,8 @@ subtest 'user tokens (our own)' => sub { ]); $t->get_ok('/user/me/token/'.$login_tokens[0]->name) - ->status_is(404, 'cannot retrieve login tokens'); + ->status_is(404) + ->log_error_is('Lookup of login tokens not supported'); $t->get_ok('/user/me/token/my first 💩 // to.ken @@') ->status_is(200) @@ -949,7 +956,8 @@ subtest 'user tokens (our own)' => sub { ->status_is(204); $t->get_ok('/user/me/token/my first 💩 // to.ken @@') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find token my first 💩 // to.ken @@ for user '.$ro_user->email); $t->get_ok('/user/me/token') ->status_is(200) @@ -957,10 +965,12 @@ subtest 'user tokens (our own)' => sub { ->json_is([]); $t->get_ok('/user/me/token/my first 💩 // to.ken @@') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find token my first 💩 // to.ken @@ for user '.$ro_user->email); $t->delete_ok('/user/me/token/my first 💩 // to.ken @@') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find token my first 💩 // to.ken @@ for user '.$ro_user->email); $t2 = Test::Conch->new(pg => $t->pg); $t2->get_ok('/user/me', { Authorization => 'Bearer '.$jwt }) @@ -1052,7 +1062,8 @@ subtest 'user tokens (someone else\'s)' => sub { ->json_is([ $tokens[1] ]); $t_super->get_ok('/user/'.$email.'/token/'.$tokens[0]->{name}) - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find token '.$tokens[0]->{name}.' for user '.$email); $t_other_user->reset_session; # force JWT to be used to authenticate @@ -1070,7 +1081,8 @@ subtest 'user tokens (someone else\'s)' => sub { ]); $t_other_user->get_ok('/user/me/token/'.$tokens[0]->{name}, { Authorization => 'Bearer '.$jwts[1] }) - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find token '.$tokens[0]->{name}.' for user '.$email); $t_super->post_ok('/user/'.$email.'/revoke') ->status_is(204) @@ -1098,10 +1110,12 @@ subtest 'user tokens (someone else\'s)' => sub { ); $t_super->delete_ok('/user/'.$email.'/token/'.$tokens[0]->{name}) - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find token '.$tokens[0]->{name}.' for user '.$email); $t_super->delete_ok('/user/'.$email.'/token/'.$tokens[1]->{name}) - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find token '.$tokens[1]->{name}.' for user '.$email); $t_super->get_ok('/user/'.$email.'/token') ->status_is(200) diff --git a/t/schema.t b/t/schema.t index 8be62a730..086716274 100644 --- a/t/schema.t +++ b/t/schema.t @@ -287,10 +287,12 @@ my $json_spec_schema = $_validator->schema->data; $t->get_ok('/schema/REQUEST/hello') ->status_is(404) - ->json_is({ error => 'Not Found' }); + ->json_is({ error => 'Not Found' }) + ->log_error_is('no endpoint found for: GET /schema/REQUEST/hello'); $t->get_ok('/schema/request/hello') - ->status_is(404); + ->status_is(404) + ->log_debug_is('Could not find request schema Hello'); $t->get_ok('/schema/response/Ping' => { 'If-Modified-Since' => 'Sun, 01 Jan 2040 00:00:00 GMT' }) ->header_is('Last-Modified', $t->app->startup_time->strftime('%a, %d %b %Y %T GMT')) From 8eb97d28bdde208670d5cdaa09514705df7fff9b Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Mon, 9 Dec 2019 11:35:21 -0800 Subject: [PATCH 14/15] rewrite ->exists helper for efficiency largely using the code in DBIx::Class::Helper::ResultSet::Shortcut::ResultsExist 0.034001, plus the modifications in https://github.com/frioux/DBIx-Class-Helpers/issues/54 --- ...ch::DB::Helper::ResultSet::ResultsExist.md | 2 +- lib/Conch/DB/Helper/ResultSet/ResultsExist.pm | 26 +++++++++--------- t/database.t | 27 +++++++++++++++++++ 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/docs/modules/Conch::DB::Helper::ResultSet::ResultsExist.md b/docs/modules/Conch::DB::Helper::ResultSet::ResultsExist.md index 48f7af4e7..94d6ada59 100644 --- a/docs/modules/Conch::DB::Helper::ResultSet::ResultsExist.md +++ b/docs/modules/Conch::DB::Helper::ResultSet::ResultsExist.md @@ -25,7 +25,7 @@ Efficiently determines if a result exists, without needing to do a `->count`. Essentially does: ``` -select exists (select 1 from ... rest of your query ...); +select * from ( select exists (select 1 from ... your query ... ) ) as _existence_subq; ``` Returns a value that you can treat as a boolean. diff --git a/lib/Conch/DB/Helper/ResultSet/ResultsExist.pm b/lib/Conch/DB/Helper/ResultSet/ResultsExist.pm index 29766ae2a..09a5f9dd8 100644 --- a/lib/Conch/DB/Helper/ResultSet/ResultsExist.pm +++ b/lib/Conch/DB/Helper/ResultSet/ResultsExist.pm @@ -27,29 +27,29 @@ This code is postgres-specific. Efficiently determines if a result exists, without needing to do a C<< ->count >>. Essentially does: - select exists (select 1 from ... rest of your query ...); + select * from ( select exists (select 1 from ... your query ... ) ) as _existence_subq; Returns a value that you can treat as a boolean. =cut sub exists ($self) { - my $inner = $self->search(undef, { select => [ \1 ] })->as_query; - - my $statement = 'select exists '.$inner->$*->[0]; - my @binds = map +($_->[1]), $inner->$*->@[1 .. $inner->$*->$#*]; - - my ($exists) = $self->result_source->schema->storage->dbh_do(sub ($storage, $dbh) { - # cribbed from DBIx::Class::Storage::DBI::_format_for_trace - $storage->debugobj->query_start($statement, map +(defined($_) ? qq{'$_'} : 'NULL'), @binds) - if $storage->debug; - - $dbh->selectrow_array($statement, undef, @binds); - }); + my $inner_q = $self->_results_exist_as_query; + my (undef, $sth) = $self->result_source + ->schema + ->storage + ->_select($inner_q, \'*', {}, {}); + my ($exists) = $sth->fetchrow_array; return $exists; } +sub _results_exist_as_query ($self) { + my $inner_q = $self->search(undef, { select => [ \1 ] })->as_query; + $$inner_q->[0] = '( select exists '.$$inner_q->[0].' ) as _existence_subq'; + $inner_q; +} + 1; __END__ diff --git a/t/database.t b/t/database.t index 4ce2c3ddb..87dcc0ff6 100644 --- a/t/database.t +++ b/t/database.t @@ -308,5 +308,32 @@ subtest 'database constraints' => sub { ); }; +subtest 'results exist' => sub { + my ($pgsql, $schema) = Test::Conch->init_db; + my $rs = $schema->resultset('build'); + + ok(!$rs->search({ created => { '!=', undef } })->exists, 'results do not exist'); + + my $query = $rs->search({ created => { '!=', undef } })->_results_exist_as_query; + my (undef, $sth) = $rs->result_source + ->schema + ->storage + ->_select($query, \'*', {}, {}); + my (@results) = $sth->fetchrow_array; + is(@results, 1, 'only one value is fetched'); + + + $rs->populate([ map +{ name => $_ }, qw(build1 build2 build3) ]); + ok($rs->search({ created => { '!=', undef } })->exists, 'results do exist'); + + (undef, $sth) = $rs->result_source + ->schema + ->storage + ->_select($query, \'*', {}, {}); + + (@results) = $sth->fetchrow_array; + is(@results, 1, 'only one value is fetched even when multiple rows exist'); +}; + done_testing; # vim: set ts=4 sts=4 sw=4 et : From 7fa28e1a2c25343ceb53548da83e080c22b5b69a Mon Sep 17 00:00:00 2001 From: Karen Etheridge Date: Mon, 9 Dec 2019 12:16:38 -0800 Subject: [PATCH 15/15] when creating builds from workspaces, also populate user_build_roles --- lib/Conch/Command/workspace_to_build.pm | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/lib/Conch/Command/workspace_to_build.pm b/lib/Conch/Command/workspace_to_build.pm index 33cf5c3fc..87b0c975a 100644 --- a/lib/Conch/Command/workspace_to_build.pm +++ b/lib/Conch/Command/workspace_to_build.pm @@ -66,9 +66,11 @@ sub run ($self, @opts) { }], }); + my $workspace_rs = $self->app->db_workspaces; + foreach my $workspace_name (@workspace_names) { $self->app->schema->txn_do(sub { - my $workspace = $self->app->db_workspaces->find({ name => $workspace_name }); + my $workspace = $workspace_rs->find({ name => $workspace_name }); die 'cannot find workspace '.$workspace_name if not $workspace; my $build = $self->app->db_builds->find({ name => $workspace_name }); @@ -94,14 +96,27 @@ sub run ($self, @opts) { ->hri ->get_column('created'); + # some of these may be collapsed into organization_build_role entries + # later on, but for now, just copy all user_workspace_role -> user_build_role + my @user_roles = $self->app->db_workspaces + ->and_workspaces_above($workspace->id) + ->search_related('user_workspace_roles', { user_id => { '!=' => $admin_id } }) + ->columns([ 'user_id', { role => { max => 'role' } } ]) + ->group_by(['user_id']) + ->hri + ->all; + $build = $self->app->db_builds->create({ name => $workspace_name, description => $workspace->description, started => minstr($device_created_rs->single, $rack_created_rs->single), - user_build_roles => [{ - user_id => $admin_id, - role => 'admin', - }], + user_build_roles => [ + { + user_id => $admin_id, + role => 'admin', + }, + @user_roles, + ], organization_build_roles => [{ organization_id => $company_org->id, role => 'ro',