✨ feat(olm): validate the bundle locally with the modern validator set - #312
Merged
Conversation
The bundle gate proved each config/crd/bases file is COPIED into the bundle and that alm-examples matches spec.customresourcedefinitions.owned, but nothing asserted owned[] itself covers config/crd/bases. A synthetic 12th CRD base added without touching owned[] kept the gate green: OLM would install the CRD while the console never lists the API, and the alm-examples check still passes because it compares against the very list that is missing the kind. Add a third set comparison between the CRD base kinds and owned[], on both the CSV template and the generated CSV, with both sides asserted non-empty so a mistyped yq path cannot make it pass vacuously.
Bundle defects were only discovered after pushing to community-operators and reading their pipeline output -- which is how the missing alm-examples were found. Add `make validate-olm-bundle`, running the validator set that upstream now recommends in place of the deprecated `operatorhub` validator, and wire it into the dist_* gate glob and the OperatorHub submission path. Two findings worth recording: * The CLI selector names differ from the deprecation message. `operatorhub/v2` is a fatal label-parse error; operator-sdk registers the three validators as `operatorhubv2`, `capabilities` and `categories` (`bundle validate --list-optional`). They map to OperatorHubV2Validator, StandardCapabilitiesValidator and StandardCategoriesValidator. * `--select-optional` is a plain string flag, not a repeatable slice. Passing it three times in ONE invocation silently keeps only the LAST selector, so the obvious one-liner would exit 0 on a green bundle while running a single validator. Each validator therefore gets its own invocation, and the gate asserts one selector per invocation so the shape cannot be "simplified" back. operator-sdk is installed from the checksum-verified release binary (hack/install-operator-sdk.sh) rather than `go install`, which needs cgo and pkg-config for gpgme. CI installs it in both the lint and release jobs so the gate genuinely runs; absent locally it fails with an install hint rather than skipping. Validation reads an on-disk bundle directory, so no cluster or registry is contacted. The declared `categories: Monitoring,Integration & Delivery` and `capabilities: Full Lifecycle` were verified against the upstream standard lists and are unchanged.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Headline discovery — why this design looks unusual
operator-sdk's--select-optionalis a plain string flag, not a slice. Passing it three times in onebundle validateinvocation silently keeps only the last selector. The obvious one-liner therefore exits 0 on a bundle with a real defect — it looks like it validates three things and validates one.Verified in both directions by the author and independently reproduced by the reviewer. Hence three separate
bundle validateinvocations, plus a shape gate assertingcount(bundle validate) == count(--select-optional)so nobody "simplifies" it back into a silently-vacuous single call.Second trap: the AC's names (
operatorhub/v2,standardcapabilities,standardcategories) are Go identities inoperator-framework/api, not CLI selectors.--select-optional name=operatorhub/v2is a fatal parse error. The working CLI names areoperatorhubv2,capabilities,categories.What this lands
DIST-OH-02 — local OLM bundle validation with the modern validator set
hack/install-operator-sdk.sh— new, installs a checksum-verified pinnedoperator-sdkv1.42.3 into./bin.make validate-olm-bundleruns the three validators as three invocations; gate asserts the 1:1 shape.operator-sdkis absent, rather than degrading to green-but-vacuous coverage. The reviewer attempted to defeat this and could not.hack/operatorhub-pr.shruns underset -Eeuo pipefailwith the validate call unwrapped and upstream of every push, so a failed installer aborts before any bytes reach the third-party repo.operator-sdkin thelintjob, upstream of thedist_*glob that consumes it.GATE-OWNED-01 (folded in) —
owned[]↔config/crd/basesset comparison, bidirectional, both directions perturbation-verified.CSV template unchanged.
Monitoring,Integration & Delivery,Full Lifecyclewere verified standard against the pinned binary, not modified.Verification
Full gate matrix re-run on this exact SHA with
operator-sdkpresent:hack/test/dist_*_test.sh— pass (incl. the real 3-validator run against a generated bundle)task verify,task lint,task test,task lint:shell— rc=0pre-commit run --files <8 changed>— passtask changelog:verifyis red because this lane's own commits are the drift; the changelog-sync bot self-heals post-merge.Follow-ups
Independent review: APPROVE, no P0/P1. One P2 and six P3, all explicitly non-blocking, captured as story GATE-HARDEN-01 — notably the CI-wiring gate compares file-global line numbers rather than steps within the
lintjob, so it can pass vacuously if the install step is moved to another job. Verified by eye on this diff that the step is genuinely insidelintand upstream of the glob.