Final version of the external validation EP

jmrodri · jmrodri · commit ef2e918e0596 · 2022-01-30T23:42:08.000-05:00
Signed-off-by: jesus m. rodriguez &lt;jesusr@redhat.com&gt;
diff --git a/enhancements/sdk-external-and-pluggable-validations.md b/enhancements/sdk-external-and-pluggable-validations.md
@@ -27,20 +27,21 @@ see-also:
 
 ## Open Questions [optional]
 
- 1. what validations do we have today?
  1. can we use scorecard to replace all of these validations?
     * 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.
  1. what would it take to convert an existing validator to an executable format?
+    Simply add a `main.go` to wrap existing validators.
+ 1. do we need to add `json` tags to [these structs][errors-go]?
 
 ## Summary
 
-Today, validations used by CVP are compiled into the operator-sdk. Any changes
+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
+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.
+requiring new releases of the `operator-sdk`.
 
 ## Motivation
 
@@ -66,8 +67,6 @@ validators to be created allowing for greater flexibility.
 * allow validations to be hosted in their own repos
 * allow validations to be external to operator-sdk
 
-TODO: are there any other goals?
-
 ### Non-Goals
 
 * existing validations do NOT have be migrated unless they want to
@@ -84,8 +83,6 @@ TODO: are there any other goals?
 * as a validation author, I can write a validation for a bundle and use it
   without having to get a new operator-sdk release
 
-TODO: Stories from the epic; consolidate with the above stories
-
 #### Story 3
 * Users want to edit/add/remove a set of validation rules for Operator bundle validation.
 
@@ -122,9 +119,6 @@ shell script.
 From the operator-sdk's point of view, we don't care what you use
 to create your validator only that we can run it and pass it a bundle directory.
 
-
-TODO: how will validators be found?
-
 #### Validators
 
 ##### Running validators
@@ -220,8 +214,6 @@ be found at [validator-poc][validator-poc1]
 Another example of a migration can be find at [ocp-olm-catalog-validator][camila-poc].
 This particular example does NOT yet output `ManifestResult` format.
 
-TODO: do we need to add `json` tags to these structs?
-https://github.com/operator-framework/api/blob/master/pkg/validation/errors/error.go#L9-L16
 
 
 #### CLI
@@ -238,6 +230,7 @@ Usage:
 ...
 
 Flags:
+      --alpha-select-external string                         Selector to select external validators to run. It should be set to a Unix path list ("/path/to/e1.sh:/path/to/e2")
   -h, --help                                                 help for validate
   -b, --image-builder string                                 Tool to pull and unpack bundle images. Only used when validating a bundle image. One of: [docker, podman, none] (default "docker")
       --list-optional                                        List all optional validators available. When set, no validators will be run
@@ -253,93 +246,12 @@ Global Flags:
 * *--help* works as is
 * *--image-builder* works as is
 * *--list-optional* would need to be updated to locate external validators.
-
-  TODO: How do we locate these validators? Do we use the XDG_CONFIG like Phase
-  2? Or allow users to pass in the location of the validators?
-
 * *--optional-values* would continue to be passed to the validators
 * *--output* indicates how we want to output the results.
+* *--select-optional* works as is
+* *-- alpha-select-external* added; takes in the location of the executable to run as
+  the validator, i.e. `/path/to/validator/the-executable:/path/to/another`
 
-  TODO: what types do we want to support?
-
-* *--select-optional* takes in a label selector to specify which optional test
-  to run. We could allow a new label to indicate the location of the executable
-  to run as the validator. i.e. `validator-path=/path/to/validator/the-executable`
-
-TODO: need to define what the CLI for operator-sdk will look like. Are there
-any new flags? Do we use the environment variable?
-
-NOTE: what would the CLI look like?
-
-
-##### Default validators run by operator-sdk
-
-List of current validators:
-TODO: do we need these validators called out like this?
-
-* BundleValidator
-  * validates the bundle
-  * looks for duplicate keys in bundle
-  * verifies all owned keys match a CRD in the bundle
-  * verifies that all CRDs present in the bundle are in the CSV
-  * validates the service account
-
-* ClusterServiceVersionValidator
-  * operator-sdk checks to see if the CSV is nil on the bundle (that's the first
-    check)
-  * checks that the CSV name is a valid format
-    * DNS1123
-    * valid label
-  * verifies replaces name is also a valid format
-    * DNS1123
-    * valid label
-  * ensures that both `alm-examples` and `olm.examples` are not both present
-  * decodes the example yaml, to validate its format
-  * checks provided APIs
-  * validates the `InstallModes`
-    * verifies that conversion CRDs support `AllNamespaces`
-  * checks for missing mandatory fields
-  * validates the annotation names
-  * validates the version and kind
-
-* CustomResourceDefinitionValidator
-  * operator-sdk puts the v1beta1 and v1 CRDs together for validation
-  * validates v1beta1 CRDs
-  * validates v1 CRDs
-  * validates internal CRDs
-    * https://github.com/kubernetes/apiextensions-apiserver/blob/master/pkg/apis/apiextensions/validation/validation.go#L49-L78
-
-##### Optional validators
-
-* PackageManifestValidator
-
-* OperatorHubValidator
-
-* ObjectValidator
-
-* OperatorGroupValidator
-
-* CommunityOperatorValidator
-
-* AlphaDeprecatedAPIsValidator
-
-* AllValidators
-  * Uses ALL of the validators
-    * BundleValidator
-    * ClusterServiceVersionValidator
-    * CustomResourceDefinitionValidator
-    * PackageManifestValidator
-    * OperatorHubValidator
-    * ObjectValidator
-    * OperatorGroupValidator
-    * CommunityOperatorValidator
-    * AlphaDeprecatedAPIsValidator
-
-* DefaultBundleValidators
-  * Uses the following 3 validators
-    * BundleValidator
-    * ClusterServiceVersionValidator
-    * CustomResourceDefinitionValidator
 ### Risks and Mitigations
 
 There is little risk, if this does not pan out we keep going on the current path
@@ -349,73 +261,17 @@ of compiling them in.
 
 ### Test Plan
 
-**Note:** *Section not required until targeted at a release.*
-
-Consider the following in developing a test plan for this enhancement:
-- Will there be e2e and integration tests, in addition to unit tests?
-- How will it be tested in isolation vs with other components?
-
-No need to outline all of the test cases, just the general strategy. Anything
-that would count as tricky in the implementation and anything particularly
-challenging to test should be called out.
-
-All code is expected to have adequate tests (eventually with coverage
-expectations).
+- unit tests for the feature will be added in the implementation
+- QE would need to create custom validations to be called by the new `--alpha-select-external` flag.
 
 ### Graduation Criteria
 
-**Note:** *Section not required until targeted at a release.*
-
-Define graduation milestones.
-
-These may be defined in terms of API maturity, or as something else. Initial proposal
-should keep this high-level with a focus on what signals will be looked at to
-determine graduation.
-
-Consider the following in developing the graduation criteria for this
-enhancement:
-- Maturity levels - `Dev Preview`, `Tech Preview`, `GA`
-- Deprecation
-
-Clearly define what graduation means.
-
-#### Examples
-
-These are generalized examples to consider, in addition to the aforementioned
-[maturity levels][maturity-levels].
+This feature will come out as alpha accessible via the `--alpha-select-external`
+flag to the `bundle validate` command. We will accept feedback on possible
+changes to the feature.
 
-##### Dev Preview -> Tech Preview
-
-- Ability to utilize the enhancement end to end
-- End user documentation, relative API stability
-- Sufficient test coverage
-- Gather feedback from users rather than just developers
-
-##### Tech Preview -> GA 
-
-- More testing (upgrade, downgrade, scale)
-- Sufficient time for feedback
-- Available by default
-
-**For non-optional features moving to GA, the graduation criteria must include
-end to end tests.**
-
-##### Removing a deprecated feature
-
-- Announce deprecation and support policy of the existing feature
-- Deprecate the feature
-
-### Upgrade / Downgrade Strategy
-
-If applicable, how will the component be upgraded and downgraded? Make sure this
-is in the test plan.
-
-Consider the following in developing an upgrade/downgrade strategy for this
-enhancement:
-- What changes (in invocations, configurations, API use, etc.) is an existing
-  cluster required to make on upgrade in order to keep previous behavior?
-- What changes (in invocations, configurations, API use, etc.) is an existing
-  cluster required to make on upgrade in order to make use of the enhancement?
+After a period of time, we can graduate to a GA with a more permanent flag, i.e.
+`--select-external`.
 
 ### Version Skew Strategy
 
@@ -426,8 +282,7 @@ enhancement:
 
 ## Implementation History
 
-Major milestones in the life cycle of a proposal should be tracked in `Implementation
-History`.
+N/A
 
 ## Drawbacks
 
@@ -446,20 +301,6 @@ compilation step being needed depending on the language used to implement them.
     * would need a cluster to run these validations
     * authors would have to create binaries of their validations anyway
 
-TODO: ---------- vvvvvv CUT vvvvvv ----------
-* wrap validations in their own executables
-  * validations would be wrapped in an executable that would be called from
-    operator-sdk
-  * pro:
-    * could reuse the existing go validations and put a main.go in front to make
-      them executable
-    * follows a similar phase 2 plugin path
-    * could reuse some of the phase 2 tech to run this
-    * would not need a cluster necessarily to run the validation
-  * con:
-    * authors would have to create binaries of their validations
-TODO: ---------- ^^^^^^ CUT ^^^^^^ ----------
-
 * use a language like JavaScript or CUE to define all validations
   * validations could be run from a git repo, i.e. operator-sdk could pull it
     and then evaluate it
@@ -480,15 +321,10 @@ TODO: ---------- ^^^^^^ CUT ^^^^^^ ----------
 
 ## Infrastructure Needed [optional]
 
-Use this section if you need things from the project. Examples include a new
-subproject, repos requested, github details, and/or testing infrastructure.
-
-Listing these here allows the community to get the process for these resources
-started right away.
-
-https://github.com/operator-framework/api/tree/master/pkg/validation
+N/A
 
 [phase-2]: https://github.com/kubernetes-sigs/kubebuilder/blob/master/designs/extensible-cli-and-scaffolding-plugins-phase-2.md
 [manifest-result]: https://github.com/operator-framework/api/blob/master/pkg/validation/errors/error.go#L9-L16
 [validator-poc1]: https://github.com/jmrodri/validator-poc/tree/poc1-manifestresults
 [camila-poc]: https://github.com/camilamacedo86/ocp-olm-catalog-validator
+[errors-go]: https://github.com/operator-framework/api/blob/master/pkg/validation/errors/error.go#L9-L16
