Treat invalid "elementType" and "objectType" annotations as errors, not panics #1023
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1023 +/- ##
==========================================
+ Coverage 56.66% 56.97% +0.31%
==========================================
Files 28 28
Lines 1560 1569 +9
==========================================
+ Hits 884 894 +10
+ Misses 559 558 -1
Partials 117 117
Continue to review full report at Codecov.
|
sadlerap
force-pushed
the
dont-panic
branch
2 times, most recently
from
957d3ce to
1cc7977
Compare
sadlerap
force-pushed
the
dont-panic
branch
from
1cc7977 to
678672f
Compare
sadlerap
force-pushed
the
dont-panic
branch
from
25c52f6 to
86e2d08
Compare
|
/retest |
sadlerap
force-pushed
the
dont-panic
branch
from
620899b to
630bb25
Compare
In a service binding annotation on a service, elementType and objectType could be garbage (i.e. elementType=asdf), and SBO would panic. We should instead do the following: 1. Throw an error instead of panicking when either of these two are invalid. 2. Fail the binding if those errors are thrown. Signed-off-by: Andy Sadler <[email protected]>
sadlerap
force-pushed
the
dont-panic
branch
from
630bb25 to
a27aa03
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: pedjak The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Motivation
If a service has an invalid service binding annotation set, we shouldn't attempt to bind the service. However, current behavior catches most cases (such as an invalid path), but errors currently are ignored, and when either
elementTypeorobjectTypeisn't valid, we panic. This could cause issues for users.Changes
Rather than panicking, we should fail the binding request and report any errors that are thrown.
Testing
Attempting to bind against a service with an garbage annotation should now fail and report errors in the binding status. For example,
service.binding/credentials: path={.status.dbCredentials},elementType=asdfshould fail. This currently implements unit and acceptance tests for bothelementTypeandobjectTypeerrors. It also adds an acceptance test for invalid paths for the sake of regression testing.