operator-framework / operator-courier

Build, verify and push operators
Apache License 2.0
41 stars 53 forks source link

courier: fix validation with multiple CRD versions #195

Closed ffromani closed 3 years ago

ffromani commented 3 years ago

If we ship a bundle which have multiple CRD version, without this patch the validation may fail with a false negative with

CRD.spec.version does not match CSV.spec.crd.owned.version

This is because we check that EVERY owned CRD matches with the supported version in the CSV. But this doesn't work if we supply a CSV which points to a CRD version (example: v1) which is in the bundle, but which also contains older CRD versions.

IOW the failed scenario is: CSV: owns: v1, v1alpha1 CRD: provides: v1, v1alpha1 supports: v1

In this case the need to check:

  1. the CRD version (v1) is owned by the CSV
  2. all the versions owned in the CSV are declared in the CRD

The check #2 was already guarantee by the code looping over CSV owned CRD versions; this patch fixes the code to fix the check #1.

Signed-off-by: Francesco Romani fromani@redhat.com

openshift-ci-robot commented 3 years ago

Hi @fromanirh. 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](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
kevinrizza commented 3 years ago

/ok-to-test /lgtm /approve

ffromani commented 3 years ago

ok, the intent was triggering the stale tests, it seems that closing/reopening in #194 did the trick, so closing this clone for now.