Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for external bundle validators #5525

Merged
merged 14 commits into from
Apr 22, 2022

Conversation

jmrodri
Copy link
Member

@jmrodri jmrodri commented Jan 31, 2022

Description of the change:
Adding support for external bundle validators.

Motivation for the change:
Today whenever there are validation changes specific to OpenShift, that often trickles to the upstream release. By adding external bundle validator support, this can allow bundle validator authors to create external binaries to be called by operator-sdk and allow business specific validators to not impact upstream releases.

Checklist

If the pull request includes user-facing changes, extra documentation is required:

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 31, 2022
@jmrodri jmrodri changed the title WIP: Add support for external bundle validators Add support for external bundle validators Feb 4, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 4, 2022
Copy link
Contributor

@ryantking ryantking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a few questions/suggestions

pkg/validate/external.go Show resolved Hide resolved
pkg/validate/external.go Outdated Show resolved Hide resolved
pkg/validate/result.go Outdated Show resolved Hide resolved
estroz and others added 11 commits April 17, 2022 02:00
Signed-off-by: Eric Stroczynski <ericstroczynski@gmail.com>
Signed-off-by: jesus m. rodriguez <jesusr@redhat.com>
Signed-off-by: jesus m. rodriguez <jesusr@redhat.com>
Signed-off-by: jesus m. rodriguez <jesusr@redhat.com>
Signed-off-by: jesus m. rodriguez <jesusr@redhat.com>
Signed-off-by: jesus m. rodriguez <jesusr@redhat.com>
Signed-off-by: jesus m. rodriguez <jesusr@redhat.com>
Signed-off-by: jesus m. rodriguez <jesusr@redhat.com>
Signed-off-by: jesus m. rodriguez <jesusr@redhat.com>
Signed-off-by: jesus m. rodriguez <jesusr@redhat.com>
Signed-off-by: jesus m. rodriguez <jesusr@redhat.com>
@jmrodri jmrodri force-pushed the validator-update branch 2 times, most recently from 51d627b to e7d71cc Compare April 17, 2022 06:11
jmrodri and others added 3 commits April 18, 2022 00:39
The PrintWithFormat method would hard exit if there was an error in the
test. Instead of returning a special error for this case, we added a
boolean to indicate if there is a failed result. This allows the cmd to
handle this case and we no longer have a hard exit in the middle of a
library function.

Signed-off-by: jesus m. rodriguez <jmrodri@gmail.com>
Signed-off-by: jesus m. rodriguez <jmrodri@gmail.com>
Signed-off-by: jesus m. rodriguez <jmrodri@gmail.com>
Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one nit. Everything else looks good to me.

I don't think it should block it from merging if you don't think the change is necessary so I am not requesting changes. Since I am still learning the ropes I will leave explicit approval to someone else.

Copy link
Contributor

@everettraven everettraven left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks good to me. I am approving and will leave it up to you whether or not you want to wait for another approval before merging.

@jmrodri jmrodri merged commit 95237b3 into operator-framework:master Apr 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants