Add provisioned service support #978
Conversation
pedjak
force-pushed
the
provisioned-service-support
branch
from
425ec2e
to
675e125
Compare
| return i.bindingItems | ||
| var allItems pipeline.BindingItems | ||
| for _, b := range i.bindings { | ||
| items, err := b.Items() |
There was a problem hiding this comment.
Is it possible to drop the error return value here?
There was a problem hiding this comment.
not sure what do you mean - would you like that return error to the caller, instead of trying to collect as much as possible items?
There was a problem hiding this comment.
The error is captured but ignored, and also the fact that Items() doesn't read as a method that might return an error.
There was a problem hiding this comment.
you are right - according to the current contract Items() does not return error, it is just a getter.
|
What if the Provisioned Service also got CR/CRD annotations? Does the binding Secret resource contain values from Provisioned Service Secret and the values extracted through CR/CRD annotations? We need to document the behavior. Probably we can add some acceptance tests also. |
Application can be bound to a provisioned service: https://github.com/k8s-service-bindings/spec#provisioned-service * collect.ProvisionedService handler detects if referred service contains a reference to secret containing the bindings * unit and acceptance tests demonstrate the contact and exposed functionality Signed-off-by: Predrag Knezevic <pknezevi@redhat.com>
pedjak
force-pushed
the
provisioned-service-support
branch
from
675e125
to
3ca7333
Compare
@baijum the code submitted in this PR allows that - I have added the acceptance test demonstrating this. The documentation will be added as the part of #863 |
|
/retest |
1 similar comment
|
/retest |
Codecov Report
@@ Coverage Diff @@
## master #978 +/- ##
==========================================
+ Coverage 55.56% 56.42% +0.86%
==========================================
Files 23 24 +1
Lines 1240 1315 +75
==========================================
+ Hits 689 742 +53
- Misses 462 477 +15
- Partials 89 96 +7
Continue to review full report at Codecov.
|
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: baijum 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 |
Application can be bound to a provisioned service:
https://github.com/k8s-service-bindings/spec#provisioned-service
Fixes #548