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.

Sign up for GitHub

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

Jump to bottom

fix: consider version when getting CRDs for validating descriptors #6784

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix: consider version when getting CRDs for validating descriptors #6784

wants to merge 1 commit into from

Conversation

tthvo
Copy link

@tthvo tthvo commented Jul 15, 2024 •
edited
Loading

Description of the change:

An additional condition is included for matching apiVersion of example CRs with CRD version when searching for the CRD in the CSV.

Motivation for the change:

Previously, the olm-spec-descriptors scorecard test failed when multiple versions of CRD are included in the CSV. The CRs specified in alm-examples annotations are validated only against the first matched CRD (by kind), which is incorrect. This ensures the CRD with correct kind and version is selected for descriptor scorecard test.

Checklist

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

Fixes #6781

@acornett21
Copy link
Contributor

@tthvo At face value I don't think this PR works, since this info only comes from the alm-examples, and those do not have a versions. The code to get the CR from the bundle is here, maybe I'm missing something.

@tthvo
Copy link
Author

tthvo commented Jul 15, 2024 •
edited
Loading

@tthvo At face value I don't think this PR works, since this info only comes from the alm-examples, and those do not have a versions. The code to get the CR from the bundle is here, maybe I'm missing something.

Hi @acornett21, I guess what I meant is to compare the apiVersion of the object in alm-examples annotations with the owned CRD's version field.

operator-sdk/testdata/go/v4/memcached-operator/bundle/manifests/memcached-operator.clusterserviceversion.yaml

Line 8 in e95abdb

"apiVersion": "cache.example.com/v1alpha1",

operator-sdk/testdata/go/v4/memcached-operator/bundle/manifests/memcached-operator.clusterserviceversion.yaml

Line 47 in e95abdb

version: v1alpha1

This is a very similar issue/fix to:

@tthvo
Copy link
Author

tthvo commented Jul 15, 2024

Sorry I forgot to include a way to test these changes. You can try:

Run: operator-sdk scorecard -n default -s default quay.io/thvo/cryostat-operator-bundle:4.0.0-olm --selector=suite=olm. That should succeed. Previously, if you use the scorecard image on quay.io/operator-framework, it fails.

@tthvo tthvo mentioned this pull request Jul 19, 2024
Open
@ebaron
Copy link

ebaron commented Aug 15, 2024

Hi, any update on getting this fixed? Our workaround of removing the v1beta1 CRD from alm-examples is generating the following warning with each build/test:
Warning: Value operator.cryostat.io/v1beta1, Kind=Cryostat: provided API should have an example annotation

It would be nice to have clean test results, but that depends on this bug being fixed.

@tthvo
fix: consider version when getting CRDs for validating descriptors
e0dfbc1 
An additional condition is included for matching `apiVersion` of example CRs with CRD `version` when searching for the CRD in the CSV. This ensures the correct CRD version is selected for validations.

Closes operator-framework#6781

Signed-off-by: Thuan Vo <thuan.votann@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acornett21 acornett21 Awaiting requested review from acornett21

@oceanc80 oceanc80 Awaiting requested review from oceanc80

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

olm-spec-descriptors test validates alm-examples with the incorrect CRD
3 participants
@tthvo @acornett21 @ebaron