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 empty CRD description validator #5603

Closed

Conversation

ryantking
Copy link
Contributor

Operator-SDK has alpha support for running external validators.
Validators are implemented as binaries that are given a path to bundle
and print the result of the validation to the standard out. More
information is available at
operator-framework/enhancements#98.

This commit adds in an external validator that errors when a CRD
specified in the bundle ClusterServiceVersion does not have a
description. This is enforced for both owned and required CRDs.

Signed-off-by: Ryan King ryking@redhat.com

Operator-SDK has alpha support for running external validators.
Validators are implemented as binaries that are given a path to bundle
and print the result of the validation to the standard out. More
information is available at
operator-framework/enhancements#98.

This commit adds in an external validator that errors when a CRD
specified in the bundle `ClusterServiceVersion` does not have a
description. This is enforced for both owned and required CRDs.

Signed-off-by: Ryan King <ryking@redhat.com>
BadValue: "",
Detail: "CRD descriptions cannot be empty",
}
}
Copy link
Contributor

@camilamacedo86 camilamacedo86 Mar 17, 2022

Choose a reason for hiding this comment

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

/hold

I think we cannot move with this one here.
We need to check:
a) I the CRD description mandatory for the bundle works well on OLM?
OR
b) we would like to ask for it because is good practice and recommendation?

If we find out that is (a) then,

the check needs to be under the bundle validator OR csv validator

Otherwise, we cannot ensure that we are checking if the bundle/CSV is valid across all solutions that use OF. See that we have other tools which use the validators and we do not want them importing SDK as lib.

if we find out that is (b) then,

the checks would fit better in ; https://github.com/operator-framework/api/blob/master/pkg/validation/internal/good_practices.go

IHMO we cannot indeed have the external validators in this project since SDK itself is not responsible for the validations and these ones are used by many projects. I mean, we have OPM, Audit, as others importing these checks such as SDK from operator-framework/api.

c/c @jmrodri

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Mar 17, 2022
@ryantking ryantking closed this Apr 4, 2022
@ryantking ryantking deleted the empty-crd-description branch April 4, 2022 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants