enhancement: External validations for Operator SDK #98

jmrodri · 2021-11-08T04:12:46Z

Today, validations used by CVP are compiled into the operator-sdk. Any changes
to the validation rules requires a release of operator-framework/api followed by
a release of operator-sdk. This proposal attempts to design a way where the
validations can be hosted in their own repos as well as updated without
requiring new releases of the operator-sdk.

enhancements/sdk-external-and-pluggable-validations.md

camilamacedo86 · 2021-11-08T14:39:57Z

I tried to contribute with some comments/nits.
However, otherwise, it seems amazing to me 🥇.

I think that can be very helpful for we are able to do specific checks and allow the users/community to create their own checks.

Terrific job @jmrodri!

enhancements/sdk-external-and-pluggable-validations.md

Signed-off-by: jesus m. rodriguez <jesusr@redhat.com>

jmrodri · 2022-01-31T22:37:16Z

Implementation operator-framework/operator-sdk#5525

ryantking · 2022-02-01T15:30:30Z

/hold Have an in-progress review that I'll finish in the next 3 hours

ryantking

Generally looks pretty good, just a few questions and things I think would improve the reading of the EP from someone with less context.

I opted to leave out any formatting issues, but there's some funky numbering and capitalization stuff going on. I'd proofread quickly then blast the document with prettier.

ryantking · 2022-02-01T17:29:11Z

enhancements/sdk-external-and-pluggable-validations.md

+    * scorecard uses the cluster to run the tests, these validations typically
+      run locally or in a pipeline. They are also done before the expensive
+      operator tests are run.


Scorecard uses kuttl, which supports a mocked control plane. Not sure if you want to couple validation to the scorecard, but that is an option to avoid needing a live cluster if you do want scorecard to handle this.

@ryantking Most of the validations are pretty static. We could revisit this idea of enhancing scorecard for this purpose in the future.

@ryantking,

The Scorecard tests require to be executed with a cluster and have a high cost to be executed. Its initial idea would also allow functional tests and the users to create their own custom tests.

See that for example, we check all bundles using SDK in the Audit tool (https://github.com/operator-framework/audit), by using the scorecard we need like 4 hours when doing exactly the same with the static validators it takes 1 hour.

enhancements/sdk-external-and-pluggable-validations.md

ryantking · 2022-02-04T18:20:30Z

enhancements/sdk-external-and-pluggable-validations.md

+to the validation rules requires a release of operator-framework/api followed by
+a release of `operator-sdk`. This proposal attempts to design a way where the
+validations can be hosted in their own repos as well as updated without
+requiring new releases of the `operator-sdk`.


Can we add an example of a validator is currently defined in-code?

@ryantking added an example to the document.

ryantking · 2022-02-04T18:22:25Z

enhancements/sdk-external-and-pluggable-validations.md

+```json
+{
+    "Name": "gatekeeper-operator.v0.2.0-rc.3",
+    "Errors": null,
+    "Warnings": [
+        {
+            "Type": "CSVFileNotValid",
+            "Level": "Warning",
+            "Field": "",
+            "BadValue": "",
+            "Detail": "(gatekeeper-operator.v0.2.0-rc.3) csv.Spec.minKubeVersion is not informed. It is recommended you provide this information. Otherwise, it would mean that your operator project can be distributed and installed in any cluster version available, which is not necessarily the case for all projects."
+        }
+    ]
+}
+```


Are we going to expose this as a go type that validator implementers will use?

@ryantking yes, we are going to expose it. There is a link to it in the EP: https://github.com/operator-framework/api/blob/master/pkg/validation/errors/error.go#L9-L16

That's already accessible from SDK.

Let me know if I need to call that out better.

enhancements/sdk-external-and-pluggable-validations.md

jmrodri · 2022-02-04T21:12:25Z

@ryantking I think I addressed your comments. Let me know if you want me to add a more explicit link to the ManifestResult struct or anything else.

jmrodri · 2022-02-04T21:12:45Z

I will add the validator example on Monday.

jmrodri · 2022-02-04T21:20:02Z

@ryantking I don't see the funky formatting when I view the rendered file in github:
validatorn-ep.pdf

camilamacedo86 · 2022-02-13T19:18:01Z

enhancements/sdk-external-and-pluggable-validations.md

+
+#### Story 5
+* Users can define the target set of validation rules for SDK's bundle validation command (either from local or remote source).
+


By generating the bundle via SDK we need to check it to ensure that the spec is correct.
Also, all checks under operator-sdk bundle validate ./bundle --select-optional suite=operatorframework are valid checks for any Operator using the OF solutions which would like to be distributed via OLM.

In this way, all tests shipped under operator-framework/api must still be provided via SDK by default.
However, we can allow users to consume the operator-framework/api bin sooner as well.

I think we have the following uses cases that we still need to comply with:

I am as an Operator Author would like to still have my bundle validated when I run make bundle to create/update it for my project so that, I can ensure that the spec generated/updated by the tool using my configuration is valid.

I am as an Operator Author would like to still have the possibility to check a bundle against all criteria to distribute my project in OLM by informing only the bundle so that, I am still able to use operator-sdk bundle validate to check bundles that were not built using the default SDK scaffold.

@camilamacedo86 I still think there is value for the compiled in checks. Though we may have to think of a better way to update those.

enhancements/sdk-external-and-pluggable-validations.md

camilamacedo86 · 2022-02-13T19:27:28Z

enhancements/sdk-external-and-pluggable-validations.md

+    * all existing validations would have to be re-written in a new language
+      structure which could introduce new bugs
+    * unproven technology
+    * would have to write the engine to know how to execute these


Suggested change

* would have to write the engine to know how to execute these

* would have to write the engine to know how to execute these

* the validators checks are static checks with a very low cost to be executed. By requiring a cluster the tests require like 3 minutes to be executed instead of milliseconds which make it very hard to achieve the goal of re-test all bundle of an index as we do via Audit Tool for example.

this is a repeat and I don't think it needs to be added as a con to the JavaScript or CUE alternative. These would be equally fast.

camilamacedo86 · 2022-02-13T19:28:50Z

enhancements/sdk-external-and-pluggable-validations.md

+
+Another example of a migration can be find at [ocp-olm-catalog-validator][camila-poc].
+This particular example does NOT yet output `ManifestResult` format.
+


I think we could also move the CLI implementation from SDK to the https://github.com/operator-framework/apia and export it as a lib, so that:

all external validators can follow the same standard (see that we are copying and pasting the SDK code implementation in ocp-olm-catalog-validator/pkg/result/ and here; bundle-validator/pkg/result/)

we can make SDK also consume it

we can make the OF/api CLI(https://github.com/operator-framework/api/tree/master/cmd/operator-verify
) use it

So that, we can ensure a standard and a centralised code to be kept maintained.

@camilamacedo86 pkg/result shouldn't be in the external validators. I do believe we can have a future enhancement to consolidate some of the logic particular logic on opening and working through bundles.

camilamacedo86 · 2022-02-13T20:05:35Z

enhancements/sdk-external-and-pluggable-validations.md

+      --plugins strings   plugin keys to be used for this subcommand execution
+      --verbose           Enable verbose logging
+```
+


Currently, we do not have many external validators. However, in the future, if it grows we might face complexities such as:

To test your project against the criteria to publish in the index X:

download external validator A, B, C

test your bundle against operator-sdk bundle validate ./bundle --select-optional suite=operatorframework and then against A,B,C validators as well.

So, shows that would be very nice if we could just copy and paste a config and run just 1 command that would check the bundle against the common criteria + all external validators.

See that we download OPM CLI and use it (https://github.com/operator-framework/operator-sdk/blob/master/testdata/go/v3/memcached-operator/Makefile#L183-L198). Then, if we could make SDK download and use all external validators configured then, we maybe make it easier / possible SDK only uses this config with a bin provided by operator-framework/api instead of importing it to execute the checks. That could address the main motivations of this PE. (avoid SDK releases only to ship new versions of the of/api bundle validators implementation)

Then, if a user requires a new version of the default checks (of/api) we could only update the config for its upper version as we do in the case of OPM in the makefile target.

This config is going down the route of "discoverability".

jmrodri · 2022-02-28T18:05:58Z

@camilamacedo86 if you want discoverability to be part of this EP, let me know so we can discuss how we think we should do that. @ryantking is currently working on some validators.

jmrodri · 2022-02-28T18:06:07Z

/hold cancel

jmrodri · 2022-03-07T14:45:24Z

@camilamacedo86 @ryantking I'm merging this EP. If we need to add more we can definitely enhance this enhancement (just updating the same one would be fine).

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.

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>

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 Nov 8, 2021

jmrodri requested review from tlwu2013, camilamacedo86, estroz and joelanford November 8, 2021 04:14