operator-registry
operator-registry copied to clipboard
Remove redundant validateOwnedCRDs impl
Description of the change:
- Adjust RegistryBundleValidator to remove the duplicate implementation for validating Owned CRDs that operates on the operator registry bundle type.
- Leave RegistryBundleValidator skeleton in place for future registry bundle validation
- Adjust existing unit test to account for validation removal
- Make ValidateBundleContent use the registry bundle AND api bundle validators
- Fix bug in argument handling for ObjectValidator which would cause validation to never trigger because it was passing an array itself instead of expanding ... to pass to Variadic argument
- enhance test coverage for files that have changed
Signed-off-by: John Hunkins [email protected]
Motivation for the change:
Remove redundant "validate owned CRD" implementation from operator registry while keeping a future point of expansion for registry specific validation. This consolidates the "validate owned CRD" into a single implementation which already exists in the api project.
Also used this as an opportunity to enhance code coverage.
Reviewer Checklist
- [ ] Implementation matches the proposed design, or proposal is updated to match implementation
- [ ] Sufficient unit test coverage
- [ ] Sufficient end-to-end test coverage
- [ ] Docs updated or added to
/docs
- [ ] Commit messages sensible and descriptive
Closes https://github.com/operator-framework/operator-registry/issues/640
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: jchunkins
To complete the pull request process, please assign ecordell after the PR has been reviewed.
You can assign the PR to them by writing /assign @ecordell
in a comment when ready.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve
in a comment
Approvers can cancel approval by writing /approve cancel
in a comment
Hi @jchunkins. Thanks for your PR.
I'm waiting for a operator-framework member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test
on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.
Once the patch is verified, the new status will be reflected by the ok-to-test
label.
I understand the commands that are listed here.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
Codecov Report
Merging #650 (7ffa364) into master (cbcacd3) will increase coverage by
0.51%
. The diff coverage is86.66%
.
@@ Coverage Diff @@
## master #650 +/- ##
==========================================
+ Coverage 48.40% 48.91% +0.51%
==========================================
Files 87 87
Lines 7599 7562 -37
==========================================
+ Hits 3678 3699 +21
+ Misses 3195 3155 -40
+ Partials 726 708 -18
Impacted Files | Coverage Δ | |
---|---|---|
pkg/lib/bundle/validate.go | 74.57% <85.71%> (+14.22%) |
:arrow_up: |
pkg/lib/validation/bundle.go | 100.00% <100.00%> (+50.90%) |
:arrow_up: |
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.
/ok-to-test
/test all
@jchunkins: No presubmit jobs available for operator-framework/operator-registry@master
In response to this:
/test all
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
/test ?
@jchunkins: No presubmit jobs available for operator-framework/operator-registry@master
In response to this:
/test ?
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
I took a quick look at the changes and they seem reasonable but my brain is dead for the day. Going to try and carve out sometime later this week to take another look.
Thanks @timflannagan
@jchunkins: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
This PR has been open for over a year and seems stale. I am going to close this PR for now, but if the suggested features are still desired, please reopen.